Add configuration to allow resolving name collisions when loading res…

…ources (#4346)

* Use slices.SortFunc to order files

* Fixup sort

* Add scope to ResourceDefinition used by Loader

* Return an error if something goes wrong

* Fixup

* Add typeLoaderRenames to configuration

* Use typeLoaderRenames to resolve name collisions

* Keep configuration for insights

* update crossplane

* Rename local for clarity

v2/azure-arm.yaml

@@ -129,6 +129,22 @@ typeFilters:
     group: costmanagement
     because: QueryFilter is a self-referential, infinitely recursive type. We can't easily unroll it and controller-gen doesn't support recursive types
 
+#
+# When loading resources, it's possible for us to end up with a name collision if two resources 
+# specify similar URLs (we derive the names from the structure of the ARM URL specified for the 
+# resource). This is an extremely rare event.
+# Because this happens so early in the operation of our pipeline, our existing mechanisms for 
+# disambiguation and renaming can't be applied, so we special case them here.
+#
+# This is configuration of last resort; in most cases, you should use the objectModelConfiguration
+# to rename things as it is much more flexible.
+#
+typeLoaderRenames:
+  - group: insights
+    name: DiagnosticSetting
+    scope: Location
+    renameTo: SubscriptionDiagnosticSetting
+
 # Exclusions for packages that currently produce types including AnyType.
 # TODO: get rid of these, either by chasing the teams generating
 # weird json or by handling them differently in the generator.

v2/tools/generator/internal/codegen/pipeline/load_types.go

@@ -11,7 +11,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"sort"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -91,8 +91,7 @@ func LoadTypes(
 
 				// add on ARM Type, URI, and supported operations
 				resourceType = resourceType.WithARMType(resourceInfo.ARMType).WithARMURI(resourceInfo.ARMURI)
-				scope := categorizeResourceScope(resourceInfo.ARMURI)
-				resourceType = resourceType.WithScope(scope)
+				resourceType = resourceType.WithScope(resourceInfo.Scope)
 				resourceType = resourceType.WithSupportedOperations(resourceInfo.SupportedOperations)
 
 				resourceDefinition := astmodel.MakeTypeDefinition(resourceName, resourceType)
@@ -125,31 +124,6 @@ func LoadTypes(
 		})
 }
 
-var (
-	resourceGroupScopeRegex = regexp.MustCompile(`(?i)^/subscriptions/[^/]+/resourcegroups/[^/]+/.*`)
-	locationScopeRegex      = regexp.MustCompile(`(?i)^/subscriptions/[^/]+/.*`)
-)
-
-func categorizeResourceScope(armURI string) astmodel.ResourceScope {
-	// this is a bit of a hack, eventually we should have better scope support.
-	// at the moment we assume that a resource is an extension if it can be applied to
-	// any scope or if armURI contains more than 1 provider:
-	if strings.HasPrefix(armURI, "/{scope}/") || strings.Count(armURI, "providers") > 1 {
-		return astmodel.ResourceScopeExtension
-	}
-
-	if resourceGroupScopeRegex.MatchString(armURI) {
-		return astmodel.ResourceScopeResourceGroup
-	}
-
-	if locationScopeRegex.MatchString(armURI) {
-		return astmodel.ResourceScopeLocation
-	}
-
-	// TODO: Not currently possible to generate a resource with scope Location, we should fix that
-	return astmodel.ResourceScopeTenant
-}
-
 var requiredSpecFields = astmodel.NewObjectType().WithProperties(
 	astmodel.NewPropertyDefinition(astmodel.NameProperty, "name", astmodel.StringType))
 
@@ -356,7 +330,7 @@ func loadSwaggerData(
 		"loaded", countLoaded,
 		"total", len(schemas))
 
-	return mergeSwaggerTypesByGroup(idFactory, typesByGroup)
+	return mergeSwaggerTypesByGroup(idFactory, typesByGroup, config)
 }
 
 var (
@@ -376,14 +350,22 @@ func logInfoSparse(log logr.Logger, message string, keysAndValues ...interface{}
 	}
 }
 
-func mergeSwaggerTypesByGroup(idFactory astmodel.IdentifierFactory, m map[astmodel.LocalPackageReference][]typesFromFile) (jsonast.SwaggerTypes, error) {
+func mergeSwaggerTypesByGroup(
+	idFactory astmodel.IdentifierFactory,
+	m map[astmodel.LocalPackageReference][]typesFromFile,
+	cfg *config.Configuration,
+) (jsonast.SwaggerTypes, error) {
 	result := jsonast.SwaggerTypes{
 		ResourceDefinitions: make(jsonast.ResourceDefinitionSet),
 		OtherDefinitions:    make(astmodel.TypeDefinitionSet),
 	}
 
 	for pkg, group := range m {
-		merged := mergeTypesForPackage(idFactory, group)
+		merged, err := mergeTypesForPackage(group, idFactory, cfg)
+		if err != nil {
+			return result, errors.Wrapf(err, "merging swagger types for %s", pkg)
+		}
+
 		for rn, rt := range merged.ResourceDefinitions {
 			if _, ok := result.ResourceDefinitions[rn]; ok {
 				panic("duplicate resource generated")
@@ -392,7 +374,7 @@ func mergeSwaggerTypesByGroup(idFactory astmodel.IdentifierFactory, m map[astmod
 			result.ResourceDefinitions[rn] = rt
 		}
 
-		err := result.OtherDefinitions.AddTypesAllowDuplicates(merged.OtherDefinitions)
+		err = result.OtherDefinitions.AddTypesAllowDuplicates(merged.OtherDefinitions)
 		if err != nil {
 			return result, errors.Wrapf(err, "when combining swagger types for %s", pkg)
 		}
@@ -406,17 +388,20 @@ type typesFromFile struct {
 	filePath string
 }
 
-type typesFromFilesSorter struct{ x []typesFromFile }
-
-var _ sort.Interface = typesFromFilesSorter{}
-
-func (s typesFromFilesSorter) Len() int           { return len(s.x) }
-func (s typesFromFilesSorter) Swap(i, j int)      { s.x[i], s.x[j] = s.x[j], s.x[i] }
-func (s typesFromFilesSorter) Less(i, j int) bool { return s.x[i].filePath < s.x[j].filePath }
-
-// mergeTypesForPackage merges the types for a single package from multiple files
-func mergeTypesForPackage(idFactory astmodel.IdentifierFactory, typesFromFiles []typesFromFile) jsonast.SwaggerTypes {
-	sort.Sort(typesFromFilesSorter{typesFromFiles})
+// mergeTypesForPackage merges the types for a single package from multiple files into our master set.
+// typesFromFiles is a slice of typesFromFile, each of which contains the types for a single file.
+// idFactory is used to generate new identifiers for types that collide.
+func mergeTypesForPackage(
+	typesFromFiles []typesFromFile,
+	idFactory astmodel.IdentifierFactory,
+	cfg *config.Configuration,
+) (*jsonast.SwaggerTypes, error) {
+	// Sort into order by filePath so we're deterministic
+	slices.SortFunc(
+		typesFromFiles,
+		func(left typesFromFile, right typesFromFile) int {
+			return strings.Compare(left.filePath, right.filePath)
+		})
 
 	typeNameCounts := make(map[astmodel.InternalTypeName]int)
 	for _, typesFromFile := range typesFromFiles {
@@ -452,7 +437,7 @@ func mergeTypesForPackage(idFactory astmodel.IdentifierFactory, typesFromFiles [
 		}
 	}
 
-	mergedResult := jsonast.SwaggerTypes{
+	mergedResult := &jsonast.SwaggerTypes{
 		ResourceDefinitions: make(jsonast.ResourceDefinitionSet),
 		OtherDefinitions:    make(astmodel.TypeDefinitionSet),
 	}
@@ -467,17 +452,94 @@ func mergeTypesForPackage(idFactory astmodel.IdentifierFactory, typesFromFiles [
 			// but they might be structurally equal
 		}
 
+		defs := mergedResult.OtherDefinitions
 		for rn, rt := range typesFromFile.ResourceDefinitions {
-			if foundRT, ok := mergedResult.ResourceDefinitions[rn]; ok &&
-				!(astmodel.TypeEquals(foundRT.SpecType, rt.SpecType) && astmodel.TypeEquals(foundRT.StatusType, rt.StatusType)) {
-				panic(fmt.Sprintf("While merging file %s: duplicate resource types generated", typesFromFile.filePath))
+			if foundRT, ok := mergedResult.ResourceDefinitions[rn]; ok {
+				// We have two resources with the same name, if they have exact same structure, they're the same
+				scopesEqual := foundRT.Scope == rt.Scope
+				specsEqual := structurallyIdentical(foundRT.SpecType, defs, rt.SpecType, defs)
+				statusesEqual := structurallyIdentical(foundRT.StatusType, defs, rt.StatusType, defs)
+				if scopesEqual && specsEqual && statusesEqual {
+					// They're the same resource, we're good.
+					continue
+				}
+
+				// Check for renames and apply them if available
+				delete(mergedResult.ResourceDefinitions, rn)
+				newNameA, renamedA := tryRename(rn, rt, cfg)
+				newNameB, renamedB := tryRename(rn, foundRT, cfg)
+
+				if _, collides := mergedResult.ResourceDefinitions[newNameA]; collides {
+					err := errors.Errorf(
+						"merging file %s: renaming %s to %s resulted in a collision with an existing type",
+						typesFromFile.filePath,
+						rn.Name(),
+						newNameA.Name())
+					return nil, err
+				}
+
+				if _, collides := mergedResult.ResourceDefinitions[newNameB]; collides {
+					err := errors.Errorf(
+						"merging file %s: renaming %s to %s resulted in a collision with an existing type",
+						typesFromFile.filePath,
+						rn.Name(),
+						newNameB.Name())
+					return nil, err
+				}
+
+				if newNameA != newNameB && (renamedA || renamedB) {
+					// One or other or both was successfully renamed, we can keep going
+					mergedResult.ResourceDefinitions[newNameA] = rt
+					mergedResult.ResourceDefinitions[newNameB] = foundRT
+					continue
+				}
+
+				// Names are still the same. We have a collision.
+				err := errors.Errorf(
+					"merging file %s: duplicate resource types generated with name %s",
+					typesFromFile.filePath,
+					rn.Name())
+				return nil, err
 			}
 
 			mergedResult.ResourceDefinitions[rn] = rt
 		}
 	}
 
-	return mergedResult
+	return mergedResult, nil
+}
+
+func tryRename(
+	name astmodel.InternalTypeName,
+	rsrc jsonast.ResourceDefinition,
+	cfg *config.Configuration,
+) (astmodel.InternalTypeName, bool) {
+	if cfg == nil {
+		// No renames configured
+		return name, false
+	}
+
+	for _, ren := range cfg.TypeLoaderRenames {
+		if !ren.AppliesToType(name) {
+			// Doesn't apply to us, not our name
+			continue
+		}
+
+		if ren.Scope != nil &&
+			!strings.EqualFold(*ren.Scope, string(rsrc.Scope)) {
+			// Doesn't apply to us, not our scope
+			continue
+		}
+
+		if ren.RenameTo == nil {
+			// No rename configured
+			return name, false
+		}
+
+		return name.WithName(*ren.RenameTo), true
+	}
+
+	return name, false
 }
 
 type typeAndSource struct {
@@ -596,6 +658,7 @@ func applyRenames(
 			ARMURI:              rt.ARMURI,
 			ARMType:             rt.ARMType,
 			SupportedOperations: rt.SupportedOperations,
+			Scope:               rt.Scope,
 		}
 	}
 

v2/tools/generator/internal/config/configuration.go

@@ -47,6 +47,8 @@ type Configuration struct {
 	AnyTypePackages []string `yaml:"anyTypePackages"`
 	// Filters used to control which types are created from the JSON schema
 	TypeFilters []*TypeFilter `yaml:"typeFilters"`
+	// Renamers used to resolve naming collisions during loading
+	TypeLoaderRenames []*TypeLoaderRename `yaml:"typeLoaderRenames"`
 	// Transformers used to remap types
 	Transformers []*TypeTransformer `yaml:"typeTransformers"`
 	// RootURL is the root URL for ASOv2 repo, paths are appended to this to generate resource links.

v2/tools/generator/internal/config/type_loader_rename.go

@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT license.
+ */
+
+package config
+
+// A TypeLoaderRename is used to resolve a naming conflict that happens during loading of types
+// right at the start of the code generator.
+type TypeLoaderRename struct {
+	TypeMatcher `yaml:",inline"`
+	Scope       *string `yaml:"scope,omitempty"`
+	RenameTo    *string `yaml:"renameTo,omitempty"`
+}

v2/tools/generator/internal/jsonast/swagger_type_extractor.go

@@ -70,6 +70,7 @@ type ResourceDefinition struct {
 	ARMType             string // e.g. Microsoft.XYZ/resourceThings
 	ARMURI              string
 	SupportedOperations set.Set[astmodel.ResourceOperation]
+	Scope               astmodel.ResourceScope
 	// TODO: use ARMURI for generating Resource URIs (only used for documentation & ownership at the moment)
 }
 
@@ -278,6 +279,7 @@ func (extractor *SwaggerTypeExtractor) extractOneResourceType(
 			ARMType:             armType,
 			ARMURI:              operationPath,
 			SupportedOperations: supportedOperations,
+			Scope:               categorizeResourceScope(operationPath),
 		}
 	}
 
@@ -953,3 +955,39 @@ func makeSchemaFromSimpleSchemaParam(param schemaAndValidations) *spec.Schema {
 
 	return schema
 }
+
+var (
+	resourceGroupScopeRegex = regexp.MustCompile(`(?i)^/subscriptions/[^/]+/resourcegroups/[^/]+/.*`)
+	locationScopeRegex      = regexp.MustCompile(`(?i)^/subscriptions/[^/]+/.*`)
+	extensionScopePrefixes  = []string{
+		"/{scope}/",
+		"/{resourceUri}/",
+	}
+)
+
+func categorizeResourceScope(armURI string) astmodel.ResourceScope {
+	// this is a bit of a hack, eventually we should have better scope support.
+	// at the moment we assume that a resource is an extension if it has a specific prefix
+	for _, prefix := range extensionScopePrefixes {
+		// Case-insensitive prefix check, as befits a heuristic
+		if strings.EqualFold(prefix, armURI[:len(prefix)]) {
+			return astmodel.ResourceScopeExtension
+		}
+	}
+
+	// Assume that a resource is an extension if armURI contains more than 1 provider:
+	if strings.Count(armURI, "providers") > 1 {
+		return astmodel.ResourceScopeExtension
+	}
+
+	if resourceGroupScopeRegex.MatchString(armURI) {
+		return astmodel.ResourceScopeResourceGroup
+	}
+
+	if locationScopeRegex.MatchString(armURI) {
+		return astmodel.ResourceScopeLocation
+	}
+
+	// TODO: Not currently possible to generate a resource with scope Location, we should fix that
+	return astmodel.ResourceScopeTenant
+}