Skip to content

Commit

Permalink
Detect conflicting API paths for the same resource and de-conflict them
Browse files Browse the repository at this point in the history
  • Loading branch information
thomas11 committed Dec 31, 2024
1 parent ba3c439 commit a5fa911
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 2 deletions.
120 changes: 118 additions & 2 deletions provider/pkg/gen/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/pkg/errors"

"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/openapi"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/openapi/paths"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/resources"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/resources/customresources"
"github.com/pulumi/pulumi/pkg/v3/codegen"
Expand Down Expand Up @@ -288,6 +289,8 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
}
slices.Sort(versions)

pathConflicts := map[string]map[string]struct{}{}

for _, sdkVersion := range versions {
// Attempt to convert back to an API version for use elsewhere
var apiVersion *openapi.ApiVersion
Expand All @@ -298,6 +301,7 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
}
apiVersion = &apiVersionConverted
}

gen := packageGenerator{
pkg: &pkg,
metadata: &metadata,
Expand All @@ -311,6 +315,7 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
rootDir: rootDir,
flattenedPropertyConflicts: flattenedPropertyConflicts,
majorVersion: majorVersion,
pathConflicts: pathConflicts,
}

// Populate C#, Java, Python and Go module mapping.
Expand Down Expand Up @@ -640,6 +645,8 @@ type packageGenerator struct {
forceNewTypes []ForceNewType
flattenedPropertyConflicts map[string]map[string]struct{}
majorVersion int
// A tok -> path map to detect when the same resource tok is mapped to different API paths.
pathConflicts map[string]map[string]struct{}
}

func (g *packageGenerator) genResources(typeName string, resource *openapi.ResourceSpec, nestedResourceBodyRefs []string) error {
Expand Down Expand Up @@ -777,16 +784,125 @@ func (g *packageGenerator) makeTypeAlias(alias string, apiVersion openapi.SdkVer
return pschema.AliasSpec{Type: &fqAlias}
}

// 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(provider, typeName, canonPath string) string {
result := typeName

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

switch strings.ToLower(provider) {
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
}

// checkForPathConflicts detects when the same resource type token is mapped to different API paths.
func (g *packageGenerator) checkForPathConflicts(typeName, canonPath string) error {
// Some resources have a /default path, e.g., azure-native:azurestackhci:GuestAgent has conflicting paths
// /subscriptions/{}/resourcegroups/{}/providers/microsoft.azurestackhci/virtualmachines/{}/guestagents/{},
// /{}/providers/microsoft.azurestackhci/virtualmachineinstances/default/guestagents/default,
// also azure-native:hybridcontainerservice:HybridIdentityMetadatum
if strings.HasSuffix(canonPath, "/default") {
return nil
}

tokWithoutVersion := g.generateTok(typeName, "")
if paths, ok := g.pathConflicts[tokWithoutVersion]; ok {
paths[canonPath] = struct{}{}
if len(paths) > 1 {
// TODO,tkappler return error instead once we have all conflicts addressed.
log.Printf("Warning: resource %s has conflicting paths %s\n", tokWithoutVersion, strings.Join(codegen.SortedKeys(paths), ", "))
}
} else {
g.pathConflicts[tokWithoutVersion] = map[string]struct{}{canonPath: {}}
}

return nil
}

func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, resource *resourceVariant, nestedResourceBodyRefs []string, typeNameAliases ...string) error {
module := g.moduleName()
swagger := resource.Swagger
path := resource.PathItem
canonPath := paths.NormalizePath(resource.Path)

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

resourceTok := fmt.Sprintf(`%s:%s:%s`, g.pkg.Name, module, resource.typeName)
if !g.versioning.ShouldInclude(g.provider, g.apiVersion, resource.typeName, resourceTok) {
resourceTok := g.generateTok(typeName, g.sdkVersion)
if !g.versioning.ShouldInclude(g.provider, g.apiVersion, typeName, resourceTok) {
return nil
}

err := g.checkForPathConflicts(typeName, canonPath)
if err != nil {
return err
}

// Generate the resource.
gen := moduleGenerator{
pkg: g.pkg,
Expand Down
22 changes: 22 additions & 0 deletions provider/pkg/gen/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,25 @@ func TestResourceIsSingleton(t *testing.T) {
assert.False(t, isSingleton(res))
})
}

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/{}"))
})
}

0 comments on commit a5fa911

Please sign in to comment.