Skip to content

Commit

Permalink
Resolve remaining path conflicts in v3 (#3855)
Browse files Browse the repository at this point in the history
Resolves #2495
  • Loading branch information
thomas11 authored Jan 23, 2025
1 parent ee1cc82 commit f73a1a3
Show file tree
Hide file tree
Showing 8 changed files with 230 additions and 160 deletions.
77 changes: 0 additions & 77 deletions provider/pkg/gen/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -822,80 +822,6 @@ func (g *packageGenerator) findResourceVariants(resource *openapi.ResourceSpec)
return result, nil
}

// dedupResourceNameByPath returns a modified resource name (`typeName`) if the resource is mapped to multiple API
// paths. For instance, the deprecated "single server" resources in `dbformysql` and `dbforpostgresql` are renamed
// to `SingleServerResource`.
// TODO,tkappler check each one if we can just get rid of an old API version instead of doing this.
func dedupResourceNameByPath(moduleName openapi.ModuleName, typeName, canonPath string) string {
result := typeName

prefix := func(prefix string) string {
if !strings.HasPrefix(typeName, prefix) {
return prefix + typeName
}
return typeName
}

switch moduleName.Lowered() {
case "cache":
if strings.Contains(canonPath, "/redis/") {
result = prefix("Redis")
} else if strings.Contains(canonPath, "/redisenterprise/") {
result = prefix("RedisEnterprise")
}
// $ rg --only-matching --no-filename --glob '!examples' 'providers/Microsoft.DBforMySQL/.+?/' azure-rest-api-specs/specification/ | sort | uniq
// providers/Microsoft.DBforMySQL/flexibleServers/
// providers/Microsoft.DBforMySQL/servers/
case "dbformysql":
if strings.Contains(canonPath, "/servers/") {
result = prefix("SingleServer")
}
// $ rg --only-matching --no-filename --glob '!examples' 'providers/Microsoft.DBforPostgreSQL/.+?/' azure-rest-api-specs/specification/ | sort | uniq
// providers/Microsoft.DBforPostgreSQL/flexibleServers/
// providers/Microsoft.DBforPostgreSQL/serverGroupsv2/
// providers/Microsoft.DBforPostgreSQL/servers/
case "dbforpostgresql":
if strings.Contains(canonPath, "/servers/") {
result = prefix("SingleServer")
} else if strings.Contains(canonPath, "/servergroupsv2/") {
result = prefix("ServerGroup")
}
case "documentdb":
if strings.Contains(canonPath, "/mongoclusters/") {
prefix("MongoCluster")
}
case "hdinsight":
if strings.Contains(canonPath, "/clusterpools/") {
result = prefix("ClusterPool")
}
case "hybridcontainerservice":
if strings.Contains(canonPath, "/provisionedclusterinstances/") {
result = prefix("ClusterInstance")
}
case "labservices":
// /labaccounts is an old API that only occurs in 2018 but we support it in v2
if strings.Contains(canonPath, "/labaccounts/") {
result = prefix("LabAccount")
}
case "migrate":
if strings.Contains(canonPath, "/assessmentprojects/") {
result = prefix("AssessmentProjects")
}
case "mobilenetwork":
if strings.Contains(canonPath, "/simgroups/") {
result = prefix("SimGroup")
}
case "netapp":
if strings.Contains(canonPath, "/backupvaults/") {
result = prefix("BackupVault")
} else if strings.Contains(canonPath, "/capacitypools/") {
result = prefix("CapacityPool")
}
}

return result
}

// recordPath adds path to keep track of all API paths a resource is mapped to.
func (g *packageGenerator) recordPath(typeName openapi.ResourceName, canonPath string, apiVersion openapi.ApiVersion) {
// Some resources have a /default path, e.g., azure-native:azurestackhci:GuestAgent has conflicting paths
Expand Down Expand Up @@ -935,9 +861,6 @@ func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, res
canonPath := paths.NormalizePath(resource.Path)

typeName := resource.typeName
if g.majorVersion > 3 {
typeName = dedupResourceNameByPath(g.moduleName, resource.typeName, canonPath)
}

resourceTok := generateTok(g.moduleName, typeName, g.sdkVersion)
if !g.versioning.ShouldInclude(g.moduleName, g.apiVersion, typeName, resourceTok) {
Expand Down
22 changes: 0 additions & 22 deletions provider/pkg/gen/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,28 +361,6 @@ func TestGoModuleName(t *testing.T) {
})
}

func TestDedupResourceNameByPath(t *testing.T) {
t.Run("no change", func(t *testing.T) {
assert.Equal(t, "Resource", dedupResourceNameByPath("compute", "Resource", "/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Compute/virtualmachines/{}"))
})

t.Run("dbformysql single server", func(t *testing.T) {
assert.Equal(t, "SingleServerResource", dedupResourceNameByPath("dbformysql", "Resource", "/subscriptions/{}/resourcegroups/{}/providers/Microsoft.DBforMySQL/servers/{}"))
})

t.Run("dbformysql flexible server", func(t *testing.T) {
assert.Equal(t, "Resource", dedupResourceNameByPath("dbformysql", "Resource", "/subscriptions/{}/resourcegroups/{}/providers/Microsoft.DBforMySQL/flexibleservers/{}"))
})

t.Run("dbforpostgresql single server", func(t *testing.T) {
assert.Equal(t, "SingleServerResource", dedupResourceNameByPath("dbforpostgresql", "Resource", "/subscriptions/{}/resourcegroups/{}/providers/Microsoft.DBforPostgreSQL/servers/{}"))
})

t.Run("dbforpostgresql flexible server", func(t *testing.T) {
assert.Equal(t, "Resource", dedupResourceNameByPath("dbforpostgresql", "Resource", "/subscriptions/{}/resourcegroups/{}/providers/Microsoft.DBforPostgreSQL/flexibleservers/{}"))
})
}

func TestResourcePathTracker(t *testing.T) {
t.Run("no conflicts, one module", func(t *testing.T) {
tracker := newResourcesPathConflictsTracker()
Expand Down
20 changes: 20 additions & 0 deletions provider/pkg/gen/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/convert"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/openapi"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/resources"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/version"
"github.com/pulumi/pulumi/pkg/v3/codegen"
pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema"
)
Expand Down Expand Up @@ -450,9 +451,17 @@ var typeNameOverrides = map[string]string{
// This one is not a disambiguation but a fix for a type name "String" that is not descriptive and leads to
// generating invalid Java.
"DatabaseWatcher.Target.String": "TargetCollectionStatus",
// SingleServer is different from just "Server", see the exception in resources.go ResourceName().
"DBforPostgreSQL.SingleServer.Sku": "SingleServerSku",
// Devices RP comes from "deviceprovisioningservices" and "iothub" which are similar but slightly different.
// In particular, the IP Filter Rule has more properties in the DPS version.
"Devices.IotDpsResource.IpFilterRule": "TargetIpFilterRule",
// See the exception for Microsoft.HDInsight in resources.go ResourceName().
"HDInsight.ClusterPoolCluster.Sku": "ClusterPoolSku",
"HDInsight.ClusterPoolCluster.ComputeProfile": "ClusterPoolComputeProfile",
"HDInsight.ClusterPoolCluster.SshProfile": "ClusterPoolSshProfile",

"HybridContainerService.ClusterInstanceAgentPool.Status": "AgentPoolProvisioningStatus",
// Workbook vs. MyWorkbook types are slightly different. Probably, a bug in the spec, but we have to disambiguate.
"Insights.MyWorkbook.ManagedIdentity": "MyManagedIdentity",
"Insights.MyWorkbook.UserAssignedIdentities": "MyUserAssignedIdentities",
Expand Down Expand Up @@ -480,11 +489,22 @@ var typeNameOverrides = map[string]string{
"SecurityInsights.WatchlistItem.UserInfo": "WatchlistUserInfo",
}

var typeNameOverridesV3 = map[string]string{
// DocumentDB.MongoCluster from /mongocluster uses a different private endpoint connection.
// The MongoCluster resource has the same name in v2 and v3, but the private endpoint connection was disambiguated in v3.
"DocumentDB.MongoCluster.PrivateEndpointConnection": "MongoClusterPrivateEndpointConnection",
}

func (m *moduleGenerator) typeNameOverride(typeName string) string {
key := fmt.Sprintf("%s.%s.%s", m.moduleName, m.resourceName, typeName)
if v, ok := typeNameOverrides[key]; ok {
return v
}
if version.GetVersion().Major >= 3 {
if v, ok := typeNameOverridesV3[key]; ok {
return v
}
}
return typeName
}

Expand Down
183 changes: 129 additions & 54 deletions provider/pkg/resources/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"regexp"
"sort"
"strings"
"unicode"

"github.com/gedex/inflector"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/version"
)

// SingleValueProperty is the name of the property that we insert into the schema for non-object type responses of invokes.
Expand Down Expand Up @@ -463,6 +465,17 @@ type NameDisambiguation struct {
// ResourceName constructs a name of a resource based on Get or List operation ID,
// e.g. "Managers_GetActivationKey" -> "ManagerActivationKey".
func ResourceName(operationID, path string) (string, *NameDisambiguation) {
return createResourceName(operationID, path, version.GetVersion().Major)
}

func createResourceName(operationID, path string, majorVersion uint64) (string, *NameDisambiguation) {
if majorVersion >= 3 {
// Uppercase the first character of operationID
r := []rune(operationID)
r[0] = unicode.ToUpper(r[0])
operationID = string(r)
}

normalizedID := strings.ReplaceAll(operationID, "-", "_")
parts := strings.Split(normalizedID, "_")
var name, verb string
Expand Down Expand Up @@ -493,66 +506,61 @@ func ResourceName(operationID, path string) (string, *NameDisambiguation) {

resourceName := name + subName

var nameDisambiuation *NameDisambiguation
return handleResourceNameSpecialCases(resourceName, operationID, path, majorVersion)
}

// Special cases
// handleResourceNameSpecialCases returns a modified resource name if the resource is mapped to multiple API paths. For
// instance, the deprecated "single server" resources in `dbformysql` and `dbforpostgresql` are renamed to `SingleServer`.
func handleResourceNameSpecialCases(resourceName, operationID, path string, majorVersion uint64) (string, *NameDisambiguation) {
newName := func(prefix, newName string) (string, *NameDisambiguation) {
// Don't generate "RedisRedis"
if prefix != "" && !strings.HasPrefix(resourceName, prefix) {
newName = prefix + newName
}

// Manual override to resolve ambiguity between public and private RecordSet.
// See https://github.com/pulumi/pulumi-azure-native/issues/583.
// To be removed with https://github.com/pulumi/pulumi-azure-native/issues/690.
if resourceName == "RecordSet" && strings.Contains(path, "/providers/Microsoft.Network/privateDnsZones/") {
newName := "PrivateRecordSet"
nameDisambiuation = &NameDisambiguation{
OperationID: operationID,
Path: path,
GeneratedName: resourceName,
DisambiguatedName: newName,
var nameDisambiguation *NameDisambiguation
if newName != resourceName {
nameDisambiguation = &NameDisambiguation{
OperationID: operationID,
Path: path,
GeneratedName: resourceName,
DisambiguatedName: newName,
}
}
resourceName = newName

return newName, nameDisambiguation
}

// Cognitive Services has global and per-account commitment plans with the same name.
var nameDisambiguation *NameDisambiguation
lowerPath := strings.ToLower(path)

// Microsoft.CognitiveServices has global and per-account commitment plans with the same name.
// The global ones are new, introduced in 2022-12-01, so we rename them.
// TODO,tkappler The global plan still has the description "Cognitive Services account
// commitment plan." - upstream issue?
// The global plan still has the description "Cognitive Services account commitment plan." - upstream issue?
if resourceName == "CommitmentPlan" && strings.Contains(path, "/providers/Microsoft.CognitiveServices/commitmentPlans/") {
newName := "SharedCommitmentPlan"
nameDisambiuation = &NameDisambiguation{
OperationID: operationID,
Path: path,
GeneratedName: resourceName,
DisambiguatedName: newName,
}
resourceName = newName
}

// Redis and RedisEnterprise are essentially distinct resources sharing the Microsoft.Cache
// namespace. It works out ok because each API version has only one of them, and of the shared
// types only PrivateEndpointConnection clashes.
if resourceName == "PrivateEndpointConnection" && strings.Contains(path, "/providers/Microsoft.Cache/redisEnterprise/") {
newName := "EnterprisePrivateEndpointConnection"
nameDisambiuation = &NameDisambiguation{
OperationID: operationID,
Path: path,
GeneratedName: resourceName,
DisambiguatedName: newName,
}
resourceName = newName
resourceName, nameDisambiguation = newName("", "SharedCommitmentPlan")
}

// Microsoft.NetApp
if strings.Contains(lowerPath, "/providers/microsoft.netapp/backupvaults/") {
resourceName, nameDisambiguation = newName("BackupVault", resourceName)
}

// Microsoft.Network
// Manual override to resolve ambiguity between public and private RecordSet.
// See https://github.com/pulumi/pulumi-azure-native/issues/583.
// To be removed with https://github.com/pulumi/pulumi-azure-native/issues/690.
if resourceName == "RecordSet" && strings.Contains(path, "/providers/Microsoft.Network/privateDnsZones/") {
resourceName, nameDisambiguation = newName("", "PrivateRecordSet")
}

// Microsoft.Network
// Both are virtual network links, but the other side of the link is a different resource and
// the links have different properties.
// https://learn.microsoft.com/en-us/azure/dns/dns-private-resolver-overview#virtual-network-links
// https://learn.microsoft.com/en-us/azure/dns/private-dns-virtual-network-links
if resourceName == "VirtualNetworkLink" && strings.Contains(path, "/providers/Microsoft.Network/dnsForwardingRulesets/") {
newName := "PrivateResolverVirtualNetworkLink"
nameDisambiuation = &NameDisambiguation{
OperationID: operationID,
Path: path,
GeneratedName: resourceName,
DisambiguatedName: newName,
}
resourceName = newName
resourceName, nameDisambiguation = newName("", "PrivateResolverVirtualNetworkLink")
}

// ServiceFabric introduced managed clusters in a new folder 'servicefabricmanagedclusters' but
Expand All @@ -562,17 +570,84 @@ func ResourceName(operationID, path string) (string, *NameDisambiguation) {
resourceName == "ApplicationType" ||
resourceName == "ApplicationTypeVersion" ||
resourceName == "Service") {
newName := "ManagedCluster" + resourceName
nameDisambiuation = &NameDisambiguation{
OperationID: operationID,
Path: path,
GeneratedName: resourceName,
DisambiguatedName: newName,
resourceName, nameDisambiguation = newName("ManagedCluster", resourceName)
}

if majorVersion < 3 {
if resourceName == "PrivateEndpointConnection" && strings.Contains(path, "/providers/Microsoft.Cache/redisEnterprise/") {
resourceName, nameDisambiguation = newName("", "EnterprisePrivateEndpointConnection")
}
}

if majorVersion >= 3 {
// Microsoft.Cache
if strings.Contains(lowerPath, "/providers/microsoft.cache/redis/") {
// resourceName, nameDisambiguation = newName("Redis", resourceName)
} else if strings.Contains(lowerPath, "/providers/microsoft.cache/redisenterprise/") {
if resourceName != "RedisEnterprise" {
resourceName, nameDisambiguation = newName("Enterprise", resourceName)
}
}

// Microsoft.DBforMySQL
if strings.Contains(lowerPath, "/providers/microsoft.dbformysql/servers/") {
if resourceName == "Server" {
resourceName, nameDisambiguation = newName("", "SingleServer")
} else {
resourceName, nameDisambiguation = newName("SingleServer", resourceName)
}
}

// Microsoft.DBforPostgreSQL
if strings.Contains(lowerPath, "/providers/microsoft.dbforpostgresql/servers/") {
if resourceName == "Server" {
resourceName, nameDisambiguation = newName("", "SingleServer")
} else {
resourceName, nameDisambiguation = newName("SingleServer", resourceName)
}
} else if strings.Contains(lowerPath, "/providers/microsoft.dbforpostgresql/servergroupsv2/") {
resourceName, nameDisambiguation = newName("ServerGroup", resourceName)
}

// Microsoft.DocumentDB
if strings.Contains(lowerPath, "/providers/microsoft.documentdb/mongoclusters/") {
resourceName, nameDisambiguation = newName("MongoCluster", resourceName)
}

// Microsoft.HDInsight
if strings.Contains(lowerPath, "/providers/microsoft.hdinsight/clusterpools/") {
resourceName, nameDisambiguation = newName("ClusterPool", resourceName)
}

// Microsoft.HybridContainerService
if strings.Contains(lowerPath, "/providers/microsoft.hybridcontainerservice/provisionedclusterinstances/") {
if !strings.Contains(resourceName, "ClusterInstance") {
resourceName, nameDisambiguation = newName("ClusterInstance", resourceName)
}
}

// Microsoft.LabServices
if strings.Contains(lowerPath, "/providers/microsoft.labservices/labaccounts/") {
resourceName, nameDisambiguation = newName("LabAccount", resourceName)
}

// Microsoft.Migrate
if strings.Contains(lowerPath, "/providers/microsoft.migrate/assessmentprojects/") {
resourceName, nameDisambiguation = newName("AssessmentProjects", resourceName)
}

// Microsoft.MobileNetwork
if strings.Contains(lowerPath, "/providers/microsoft.mobilenetwork/simgroups/") {
resourceName, nameDisambiguation = newName("SimGroup", resourceName)
}

// Microsoft.NetApp
if strings.Contains(lowerPath, "/providers/microsoft.netapp/netappaccounts/") && strings.Contains(lowerPath, "/capacitypools/") {
resourceName, nameDisambiguation = newName("CapacityPool", resourceName)
}
resourceName = newName
}

return resourceName, nameDisambiuation
return resourceName, nameDisambiguation
}

var referenceNameReplacer = strings.NewReplacer("CreateOrUpdateParameters", "", "Create", "", "Request", "")
Expand Down
Loading

0 comments on commit f73a1a3

Please sign in to comment.