Skip to content

Commit

Permalink
Refactor to encapsulate logic more, type pathConflicts map more clearly
Browse files Browse the repository at this point in the history
  • Loading branch information
thomas11 committed Jan 9, 2025
1 parent d775a58 commit 2e1b64a
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 126 deletions.
79 changes: 56 additions & 23 deletions provider/pkg/gen/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ type GenerationResult struct {
ForceNewTypes []ForceNewType
TypeCaseConflicts CaseConflicts
FlattenedPropertyConflicts map[string]map[string]struct{}
PathConflicts map[string]map[string]struct{}
// A map of provider -> resource -> set of paths, to record resources that have conflicts where the same resource
// maps to more than one API path.
PathConflicts map[openapi.ProviderName]map[openapi.ResourceName]map[string]struct{}
}

// PulumiSchema will generate a Pulumi schema for the given Azure providers and resources map.
Expand Down Expand Up @@ -281,7 +283,7 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
caseSensitiveTypes := newCaseSensitiveTokens()
flattenedPropertyConflicts := map[string]map[string]struct{}{}
exampleMap := make(map[string][]resources.AzureAPIExample)
resourcePaths := map[string]map[string]struct{}{}
resourcesPathTracker := newResourcesPathTracker()

for _, providerName := range providers {
versionMap := providerMap[providerName]
Expand All @@ -291,6 +293,8 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
}
slices.Sort(versions)

resourcePaths := map[openapi.ResourceName]map[string]struct{}{}

for _, sdkVersion := range versions {
// Attempt to convert back to an API version for use elsewhere
var apiVersion *openapi.ApiVersion
Expand All @@ -315,7 +319,7 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
rootDir: rootDir,
flattenedPropertyConflicts: flattenedPropertyConflicts,
majorVersion: majorVersion,
resourcePaths: resourcePaths,
resourcePaths: map[openapi.ResourceName]string{},
}

// Populate C#, Java, Python and Go module mapping.
Expand Down Expand Up @@ -349,8 +353,17 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning

// Populate invokes.
gen.genInvokes(items.Invokes)

forceNewTypes = append(forceNewTypes, gen.forceNewTypes...)
gen.mergeResourcePathsInto(resourcePaths)
}

resourcesPathTracker.addPathConflictsForProvider(providerName, resourcePaths)
}

// When a resource maps to more than one API path, it's a conflict and we need to detect and report it. #2495
if majorVersion >= 3 && resourcesPathTracker.hasConflicts() {
return nil, fmt.Errorf("path conflicts detected: %v", resourcesPathTracker.pathConflicts)
}

err := genMixins(&pkg, &metadata)
Expand Down Expand Up @@ -405,28 +418,42 @@ version using infrastructure as code, which Pulumi then uses to drive the ARM AP
"packages": javaPackages,
})

// When a resource maps to more than one API path, it's a conflict and we need to detect and report it. #2495
pathConflicts := map[string]map[string]struct{}{}
for tok, paths := range resourcePaths {
if len(paths) > 1 {
pathConflicts[tok] = paths
}
}
if majorVersion >= 3 && len(pathConflicts) > 0 {
return nil, fmt.Errorf("path conflicts detected: %v", pathConflicts)
}

return &GenerationResult{
Schema: &pkg,
Metadata: &metadata,
Examples: exampleMap,
ForceNewTypes: forceNewTypes,
TypeCaseConflicts: caseSensitiveTypes.findCaseConflicts(),
FlattenedPropertyConflicts: flattenedPropertyConflicts,
PathConflicts: pathConflicts,
PathConflicts: resourcesPathTracker.pathConflicts,
}, nil
}

// resourcesPathConflicts tracks resource path conflicts by provider/module. Use newResourcesPathTracker to instantiate
type resourcesPathConflicts struct {
pathConflicts map[openapi.ProviderName]map[openapi.ResourceName]map[string]struct{}
}

func newResourcesPathTracker() *resourcesPathConflicts {
return &resourcesPathConflicts{pathConflicts: map[openapi.ProviderName]map[openapi.ResourceName]map[string]struct{}{}}
}

func (rpt *resourcesPathConflicts) addPathConflictsForProvider(providerName openapi.ProviderName, resourcePaths map[openapi.ResourceName]map[string]struct{}) {
providerPathConflicts := map[openapi.ResourceName]map[string]struct{}{}
for resource, paths := range resourcePaths {
if len(paths) > 1 {
providerPathConflicts[resource] = paths
}
}
if len(providerPathConflicts) > 0 {
rpt.pathConflicts[providerName] = providerPathConflicts
}
}

func (rpt *resourcesPathConflicts) hasConflicts() bool {
return len(rpt.pathConflicts) > 0
}

func (g *packageGenerator) genInvokes(invokes map[string]*openapi.ResourceSpec) {
var invokeNames []string
for invokeName := range invokes {
Expand Down Expand Up @@ -657,8 +684,9 @@ type packageGenerator struct {
forceNewTypes []ForceNewType
flattenedPropertyConflicts map[string]map[string]struct{}
majorVersion int
// A tok -> paths map to record API paths per resource and later detect conflicts.
resourcePaths map[string]map[string]struct{}
// A resource -> path map to record API paths per resource and later detect conflicts.
// packageGenerator is used for a single API version, so there won't be conflicting paths here.
resourcePaths map[openapi.ResourceName]string
}

func (g *packageGenerator) genResources(typeName string, resource *openapi.ResourceSpec, nestedResourceBodyRefs []string) error {
Expand Down Expand Up @@ -870,8 +898,8 @@ func dedupResourceNameByPath(provider, typeName, canonPath string) string {
return result
}

// recordPath keeps track of all API paths a resource is mapped to.
func (g *packageGenerator) recordPath(typeName, canonPath string) {
// recordPath adds path to keep track of all API paths a resource is mapped to.
func (g *packageGenerator) recordPath(typeName openapi.ResourceName, canonPath string) {
// 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,
Expand All @@ -880,12 +908,17 @@ func (g *packageGenerator) recordPath(typeName, canonPath string) {
return
}

tokWithoutVersion := g.generateTok(typeName, "")
g.resourcePaths[typeName] = canonPath
}

if _, ok := g.resourcePaths[tokWithoutVersion]; !ok {
g.resourcePaths[tokWithoutVersion] = map[string]struct{}{}
// mergeResourcePathsInto merges this packageGenerator's resource paths into the given map.
func (g *packageGenerator) mergeResourcePathsInto(resourcePaths map[openapi.ResourceName]map[string]struct{}) {
for resource, path := range g.resourcePaths {
if _, ok := resourcePaths[resource]; !ok {
resourcePaths[resource] = map[string]struct{}{}
}
resourcePaths[resource][path] = struct{}{}
}
g.resourcePaths[tokWithoutVersion][canonPath] = struct{}{}
}

func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, resource *resourceVariant, nestedResourceBodyRefs []string, typeNameAliases ...string) error {
Expand Down
49 changes: 49 additions & 0 deletions provider/pkg/gen/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,52 @@ func TestDedupResourceNameByPath(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 provider", func(t *testing.T) {
tracker := newResourcesPathTracker()
tracker.addPathConflictsForProvider("compute", map[openapi.ResourceName]map[string]struct{}{
"VirtualMachine": {"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Compute/virtualMachines/{}": struct{}{}},
})
assert.False(t, tracker.hasConflicts())
})

t.Run("conflicts, one provider", func(t *testing.T) {
tracker := newResourcesPathTracker()
tracker.addPathConflictsForProvider("compute", map[openapi.ResourceName]map[string]struct{}{
"VirtualMachine": {
"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Compute/virtualMachines/{}": struct{}{},
"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Compute/virtualMachinesFoo/{}": struct{}{},
},
})
assert.True(t, tracker.hasConflicts())
})

t.Run("no conflicts, multiple providers", func(t *testing.T) {
tracker := newResourcesPathTracker()
tracker.addPathConflictsForProvider("compute", map[openapi.ResourceName]map[string]struct{}{
"VirtualMachine": {"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Compute/virtualMachines/{}": struct{}{}},
})
tracker.addPathConflictsForProvider("storage", map[openapi.ResourceName]map[string]struct{}{
"StorageAccount": {"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Storage/storageAccounts/{}": struct{}{}},
})
assert.False(t, tracker.hasConflicts())
})

t.Run("conflicts, multiple providers", func(t *testing.T) {
tracker := newResourcesPathTracker()
tracker.addPathConflictsForProvider("storage", map[openapi.ResourceName]map[string]struct{}{
"StorageAccount": {"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Storage/storageAccounts/{}": struct{}{}},
})
tracker.addPathConflictsForProvider("compute", map[openapi.ResourceName]map[string]struct{}{
"VirtualMachine": {
"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Compute/virtualMachines/{}": struct{}{},
"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Compute/virtualMachinesFoo/{}": struct{}{},
},
})
tracker.addPathConflictsForProvider("migrate", map[openapi.ResourceName]map[string]struct{}{
"AssessmentProject": {"/subscriptions/{}/resourceGroups/{}/providers/Microsoft.Migrate/assessmentProjects/{}": struct{}{}},
})
assert.True(t, tracker.hasConflicts())
})
}
4 changes: 2 additions & 2 deletions provider/pkg/versioning/build_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ type BuildSchemaArgs struct {

type BuildSchemaReports struct {
PathChangesResult
// A tok -> paths map to record types that have conflicting paths.
PathConflicts map[string]map[string]struct{}
// providerName -> resourceName -> set of paths, to record resources that have conflicting paths.
PathConflicts map[openapi.ProviderName]map[openapi.ResourceName]map[string]struct{}
AllResourcesByVersion ProvidersVersionResources
AllResourceVersionsByResource ProviderResourceVersions
Pending openapi.ProviderVersionList
Expand Down
Loading

0 comments on commit 2e1b64a

Please sign in to comment.