Add new report to track properties that get overwritten during flattening

provider/pkg/gen/properties.go

@@ -97,6 +97,14 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput
 				return nil, err
 			}
 
+			// Check that none of the inner properties already exists on the outer type. This
+			// causes a conflict when flattening, and will probably need to be handled in v3.
+			for propName := range bag.properties {
+				if _, has := result.properties[propName]; has {
+					m.flattenedPropertyConflicts[fmt.Sprintf("%s.%s", name, propName)] = struct{}{}
+				}
+			}
+
 			// Adjust every property to mark them as flattened.
 			newProperties := map[string]resources.AzureAPIProperty{}
 			for n, value := range bag.properties {

provider/pkg/gen/schema.go

@@ -51,11 +51,12 @@ type Versioning interface {
 }
 
 type GenerationResult struct {
-	Schema            *pschema.PackageSpec
-	Metadata          *resources.AzureAPIMetadata
-	Examples          map[string][]resources.AzureAPIExample
-	ForceNewTypes     []ForceNewType
-	TypeCaseConflicts CaseConflicts
+	Schema                     *pschema.PackageSpec
+	Metadata                   *resources.AzureAPIMetadata
+	Examples                   map[string][]resources.AzureAPIExample
+	ForceNewTypes              []ForceNewType
+	TypeCaseConflicts          CaseConflicts
+	FlattenedPropertyConflicts map[string]map[string]struct{}
 }
 
 // PulumiSchema will generate a Pulumi schema for the given Azure providers and resources map.
@@ -269,10 +270,12 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
 	}
 	sort.Strings(providers)
 
+	// Track some global data
 	warnings := []string{}
 	var forceNewTypes []ForceNewType
-
 	caseSensitiveTypes := newCaseSensitiveTokens()
+	flattenedPropertyConflicts := map[string]map[string]struct{}{}
+
 	exampleMap := make(map[string][]resources.AzureAPIExample)
 	for _, providerName := range providers {
 		versionMap := providerMap[providerName]
@@ -284,15 +287,16 @@ func PulumiSchema(rootDir string, providerMap openapi.AzureProviders, versioning
 
 		for _, version := range versions {
 			gen := packageGenerator{
-				pkg:                &pkg,
-				metadata:           &metadata,
-				provider:           providerName,
-				apiVersion:         version,
-				allApiVersions:     versions,
-				examples:           exampleMap,
-				versioning:         versioning,
-				caseSensitiveTypes: caseSensitiveTypes,
-				rootDir:            rootDir,
+				pkg:                        &pkg,
+				metadata:                   &metadata,
+				provider:                   providerName,
+				apiVersion:                 version,
+				allApiVersions:             versions,
+				examples:                   exampleMap,
+				versioning:                 versioning,
+				caseSensitiveTypes:         caseSensitiveTypes,
+				rootDir:                    rootDir,
+				flattenedPropertyConflicts: flattenedPropertyConflicts,
 			}
 
 			// Populate C#, Java, Python and Go module mapping.
@@ -384,11 +388,12 @@ version using infrastructure as code, which Pulumi then uses to drive the ARM AP
 	})
 
 	return &GenerationResult{
-		Schema:            &pkg,
-		Metadata:          &metadata,
-		Examples:          exampleMap,
-		ForceNewTypes:     forceNewTypes,
-		TypeCaseConflicts: caseSensitiveTypes.findCaseConflicts(),
+		Schema:                     &pkg,
+		Metadata:                   &metadata,
+		Examples:                   exampleMap,
+		ForceNewTypes:              forceNewTypes,
+		TypeCaseConflicts:          caseSensitiveTypes.findCaseConflicts(),
+		FlattenedPropertyConflicts: flattenedPropertyConflicts,
 	}, nil
 }
 
@@ -575,8 +580,9 @@ type packageGenerator struct {
 	caseSensitiveTypes caseSensitiveTokens
 	warnings           []string
 	// rootDir is used to resolve relative paths in the examples.
-	rootDir       string
-	forceNewTypes []ForceNewType
+	rootDir                    string
+	forceNewTypes              []ForceNewType
+	flattenedPropertyConflicts map[string]map[string]struct{}
 }
 
 func (g *packageGenerator) genResources(typeName string, resource *openapi.ResourceSpec, nestedResourceBodyRefs []string) error {
@@ -723,16 +729,17 @@ func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, res
 
 	// Generate the resource.
 	gen := moduleGenerator{
-		pkg:                    g.pkg,
-		metadata:               g.metadata,
-		module:                 module,
-		prov:                   g.provider,
-		resourceName:           resource.typeName,
-		resourceToken:          resourceTok,
-		visitedTypes:           make(map[string]bool),
-		caseSensitiveTypes:     g.caseSensitiveTypes,
-		inlineTypes:            map[*openapi.ReferenceContext]codegen.StringSet{},
-		nestedResourceBodyRefs: nestedResourceBodyRefs,
+		pkg:                        g.pkg,
+		metadata:                   g.metadata,
+		module:                     module,
+		prov:                       g.provider,
+		resourceName:               resource.typeName,
+		resourceToken:              resourceTok,
+		visitedTypes:               make(map[string]bool),
+		caseSensitiveTypes:         g.caseSensitiveTypes,
+		inlineTypes:                map[*openapi.ReferenceContext]codegen.StringSet{},
+		nestedResourceBodyRefs:     nestedResourceBodyRefs,
+		flattenedPropertyConflicts: map[string]struct{}{},
 	}
 
 	updateOp := path.Put
@@ -884,6 +891,9 @@ func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, res
 
 	g.generateExampleReferences(resourceTok, path, swagger)
 	g.forceNewTypes = append(g.forceNewTypes, gen.forceNewTypes...)
+	if len(gen.flattenedPropertyConflicts) > 0 {
+		g.flattenedPropertyConflicts[resourceTok] = gen.flattenedPropertyConflicts
+	}
 	return nil
 }
 
@@ -966,15 +976,16 @@ func (g *packageGenerator) generateExampleReferences(resourceTok string, path *s
 func (g *packageGenerator) genFunctions(typeName, path string, specParams []spec.Parameter, operation *spec.Operation, swagger *openapi.Spec) {
 	module := g.moduleName()
 	gen := moduleGenerator{
-		pkg:                g.pkg,
-		metadata:           g.metadata,
-		module:             module,
-		resourceToken:      fmt.Sprintf(`%s:%s:%s`, g.pkg.Name, module, typeName),
-		prov:               g.provider,
-		resourceName:       typeName,
-		visitedTypes:       make(map[string]bool),
-		caseSensitiveTypes: g.caseSensitiveTypes,
-		inlineTypes:        map[*openapi.ReferenceContext]codegen.StringSet{},
+		pkg:                        g.pkg,
+		metadata:                   g.metadata,
+		module:                     module,
+		resourceToken:              fmt.Sprintf(`%s:%s:%s`, g.pkg.Name, module, typeName),
+		prov:                       g.provider,
+		resourceName:               typeName,
+		visitedTypes:               make(map[string]bool),
+		caseSensitiveTypes:         g.caseSensitiveTypes,
+		inlineTypes:                map[*openapi.ReferenceContext]codegen.StringSet{},
+		flattenedPropertyConflicts: map[string]struct{}{},
 	}
 
 	// Generate the function to get this resource.
@@ -1219,17 +1230,18 @@ type ForceNewType struct {
 }
 
 type moduleGenerator struct {
-	pkg                    *pschema.PackageSpec
-	metadata               *resources.AzureAPIMetadata
-	module                 string
-	prov                   string
-	resourceName           string
-	resourceToken          string
-	visitedTypes           map[string]bool
-	caseSensitiveTypes     caseSensitiveTokens
-	inlineTypes            map[*openapi.ReferenceContext]codegen.StringSet
-	nestedResourceBodyRefs []string
-	forceNewTypes          []ForceNewType
+	pkg                        *pschema.PackageSpec
+	metadata                   *resources.AzureAPIMetadata
+	module                     string
+	prov                       string
+	resourceName               string
+	resourceToken              string
+	visitedTypes               map[string]bool
+	caseSensitiveTypes         caseSensitiveTokens
+	inlineTypes                map[*openapi.ReferenceContext]codegen.StringSet
+	nestedResourceBodyRefs     []string
+	forceNewTypes              []ForceNewType
+	flattenedPropertyConflicts map[string]struct{}
 }
 
 func (m *moduleGenerator) escapeCSharpNames(typeName string, resourceResponse *propertyBag) {

provider/pkg/versioning/build_schema.go

@@ -43,6 +43,7 @@ type BuildSchemaReports struct {
 	SkippedPOSTEndpoints          map[string]map[string]string
 	ForceNewTypes                 []gen.ForceNewType
 	TypeCaseConflicts             gen.CaseConflicts
+	FlattenedPropertyConflicts    map[string]map[string]struct{}
 }
 
 func (r BuildSchemaReports) WriteTo(outputDir string) ([]string, error) {
@@ -57,6 +58,7 @@ func (r BuildSchemaReports) WriteTo(outputDir string) ([]string, error) {
 		"skippedPOSTEndpoints.json":          r.SkippedPOSTEndpoints,
 		"forceNewTypes.json":                 r.ForceNewTypes,
 		"typeCaseConflicts.json":             r.TypeCaseConflicts,
+		"flattenedPropertyConflicts.json":    r.FlattenedPropertyConflicts,
 	})
 }
 
@@ -138,6 +140,7 @@ func BuildSchema(args BuildSchemaArgs) (*BuildSchemaResult, error) {
 		CurationViolations:            versionMetadata.CurationViolations,
 		ForceNewTypes:                 generationResult.ForceNewTypes,
 		TypeCaseConflicts:             generationResult.TypeCaseConflicts,
+		FlattenedPropertyConflicts:    generationResult.FlattenedPropertyConflicts,
 	}
 
 	return &BuildSchemaResult{

reports/README.md

@@ -34,3 +34,7 @@ Calculated by `findPathChanges`. Discussed in <https://github.com/pulumi/pulumi-
 ## `pending.json`
 
 A list of versions of each namespace which might be available to upgrade in the default version.
+
+## `flattenedPropertyConflicts.json`
+
+The Azure spec has an annotation [x-ms-client-flatten](https://github.com/Azure/autorest/blob/main/docs/extensions/readme.md#x-ms-client-flatten) which instructs the schema generator to flatten the property into the parent object. However, there are cases where this leads to overwriting a property, creating incorrect schema and SDKs, when inner and outer property have the same name.

reports/flattenedPropertyConflicts.json

@@ -0,0 +1,116 @@
+{
+  "azure-native:authorization/v20211201preview:AccessReviewHistoryDefinitionById": {
+    "range.type": {}
+  },
+  "azure-native:authorization/v20211201preview:AccessReviewScheduleDefinitionById": {
+    "range.type": {},
+    "scope.principalType": {}
+  },
+  "azure-native:authorization/v20211201preview:ScopeAccessReviewHistoryDefinitionById": {
+    "range.type": {}
+  },
+  "azure-native:authorization/v20211201preview:ScopeAccessReviewScheduleDefinitionById": {
+    "range.type": {},
+    "scope.principalType": {}
+  },
+  "azure-native:authorization:AccessReviewHistoryDefinitionById": {
+    "range.type": {}
+  },
+  "azure-native:authorization:AccessReviewScheduleDefinitionById": {
+    "range.type": {},
+    "scope.principalType": {}
+  },
+  "azure-native:authorization:ScopeAccessReviewHistoryDefinitionById": {
+    "range.type": {}
+  },
+  "azure-native:authorization:ScopeAccessReviewScheduleDefinitionById": {
+    "range.type": {},
+    "scope.principalType": {}
+  },
+  "azure-native:devhub/v20221011preview:Workflow": {
+    "githubWorkflowProfile.namespace": {}
+  },
+  "azure-native:devhub/v20230801:Workflow": {
+    "githubWorkflowProfile.namespace": {}
+  },
+  "azure-native:devhub:Workflow": {
+    "githubWorkflowProfile.namespace": {}
+  },
+  "azure-native:education/v20211201preview:Lab": {
+    "totalBudget.currency": {},
+    "totalBudget.value": {}
+  },
+  "azure-native:education:Lab": {
+    "totalBudget.currency": {},
+    "totalBudget.value": {}
+  },
+  "azure-native:insights/v20150501:Workbook": {
+    "properties.kind": {}
+  },
+  "azure-native:netapp/v20210401:BackupPolicy": {
+    "properties.name": {}
+  },
+  "azure-native:netapp/v20210401preview:BackupPolicy": {
+    "properties.name": {}
+  },
+  "azure-native:network/v20190701:P2sVpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network/v20190701:VirtualWan": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network/v20230201:VpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network/v20230401:VpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network/v20230501:VpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network/v20230601:NetworkVirtualApplianceConnection": {
+    "properties.name": {}
+  },
+  "azure-native:network/v20230601:VpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network/v20230901:NetworkVirtualApplianceConnection": {
+    "properties.name": {}
+  },
+  "azure-native:network/v20230901:VpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network:P2sVpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:network:VpnServerConfiguration": {
+    "properties.etag": {},
+    "properties.name": {}
+  },
+  "azure-native:security/v20221201preview:DefenderForStorage": {
+    "malwareScanning.isEnabled": {},
+    "sensitiveDataDiscovery.isEnabled": {},
+    "sensitiveDataDiscovery.operationStatus": {}
+  },
+  "azure-native:security:DefenderForStorage": {
+    "malwareScanning.isEnabled": {},
+    "sensitiveDataDiscovery.isEnabled": {},
+    "sensitiveDataDiscovery.operationStatus": {}
+  },
+  "azure-native:vmwarecloudsimple/v20190401:DedicatedCloudNode": {
+    "properties.id": {},
+    "properties.name": {}
+  },
+  "azure-native:vmwarecloudsimple:DedicatedCloudNode": {
+    "properties.id": {},
+    "properties.name": {}
+  }
+}