diff --git a/provider/pkg/gen/properties.go b/provider/pkg/gen/properties.go index 492dd3586380..c096433b35e6 100644 --- a/provider/pkg/gen/properties.go +++ b/provider/pkg/gen/properties.go @@ -25,6 +25,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" "github.com/pulumi/pulumi/pkg/v3/codegen/schema" pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema" @@ -41,6 +42,17 @@ type propertyBag struct { requiredContainers RequiredContainers } +// propertyIntersection returns the property names that are present in both bags. +func (p *propertyBag) propertyIntersection(other *propertyBag) []string { + result := []string{} + for propName := range p.properties { + if _, ok := other.properties[propName]; ok { + result = append(result, propName) + } + } + return result +} + type RequiredContainers [][]string // genPropertiesVariant is a set of flags that control the behavior of property generation @@ -100,11 +112,13 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, variants // Change the name to lowerCamelCase. sdkName = ToLowerCamel(sdkName) - flatten, ok := property.Extensions.GetBool(extensionClientFlatten) - // Flattened properties aren't modelled in the SDK explicitly: their sub-properties are merged directly to the parent. - // If the type is marked as a dictionary, ignore the extension and proceed with modeling this property explicitly. - // We can't flatten dictionaries in a type-safe manner. + // There are two exceptions to this rule, for which we skip the merging: + // 1. If the type is marked as a dictionary, ignore the extension and proceed with modeling this property explicitly. + // We can't flatten dictionaries in a type-safe manner. + // 2. Sometimes the Azure spec misuses the "x-ms-client-flatten" extension in cases where flattening would cause two + // properties to conflict because the outer and the inner type both have a property with the same name. + flatten, ok := property.Extensions.GetBool(extensionClientFlatten) isDict := resolvedProperty.AdditionalProperties != nil // TODO: Remove when https://github.com/Azure/azure-rest-api-specs/pull/14550 is rolled back @@ -112,52 +126,91 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, variants property.Ref.String() == "#/definitions/DelegatedSubnetProperties" || property.Ref.String() == "#/definitions/DelegatedControllerProperties" - // TODO: can be removed when https://github.com/pulumi/pulumi-azure-native/pull/3016 is merged - if m.resourceName == "DefenderForStorage" && (name == "malwareScanning" || name == "sensitiveDataDiscovery") { - flatten = false - } // See #3556 and https://github.com/Azure/azure-rest-api-specs/issues/30443 // It's ok if a new API version is fixed, this is a no-op then. if m.resourceName == "CIAMTenant" && name == "tier" && strings.HasPrefix(m.module, "azureactivedirectory") { flatten = false } - if (ok && flatten && !isDict) || workaroundDelegatedNetworkBreakingChange { - bag, err := m.genProperties(resolvedProperty, variants.noResponse()) - if err != nil { - return nil, err + // Prevent a property collision due to flattening. From v3, we detect and avoid this case automatically. + if version.GetVersion().Major < 3 { + if m.resourceName == "DefenderForStorage" && (name == "malwareScanning" || name == "sensitiveDataDiscovery") { + flatten = false } - // 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 { + if (ok && flatten && !isDict) || workaroundDelegatedNetworkBreakingChange { + bag, err := m.genProperties(resolvedProperty, variants.noResponse()) + if err != nil { + return nil, err + } + + // Check that none of the inner properties already exists on the outer type. This + // causes a conflict when flattening, and will be handled in v3. + for _, propName := range result.propertyIntersection(bag) { 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 { - // The order of containers is important, so we prepend the outermost name. - value.Containers = append([]string{name}, value.Containers...) - newProperties[n] = value - } - bag.properties = newProperties + // Adjust every property to mark them as flattened. + newProperties := map[string]resources.AzureAPIProperty{} + for n, value := range bag.properties { + // The order of containers is important, so we prepend the outermost name. + value.Containers = append([]string{name}, value.Containers...) + newProperties[n] = value + } + bag.properties = newProperties - newRequiredContainers := make(RequiredContainers, len(bag.requiredContainers)) - for i, containers := range bag.requiredContainers { - newRequiredContainers[i] = append([]string{name}, containers...) - } - for _, requiredName := range resolvedSchema.Required { - if requiredName == name { - newRequiredContainers = append(newRequiredContainers, []string{name}) + newRequiredContainers := make(RequiredContainers, len(bag.requiredContainers)) + for i, containers := range bag.requiredContainers { + newRequiredContainers[i] = append([]string{name}, containers...) + } + for _, requiredName := range resolvedSchema.Required { + if requiredName == name { + newRequiredContainers = append(newRequiredContainers, []string{name}) + } } + bag.requiredContainers = newRequiredContainers + + result.merge(bag) + continue } - bag.requiredContainers = newRequiredContainers + } else { + if (ok && flatten && !isDict) || workaroundDelegatedNetworkBreakingChange { + bag, err := m.genProperties(resolvedProperty, variants.noResponse()) + if err != nil { + return nil, err + } - result.merge(bag) - continue + // Check that none of the inner properties already exists on the outer type. + conflictingProperties := result.propertyIntersection(bag) + if len(conflictingProperties) == 0 { + // Adjust every property to mark them as flattened. + newProperties := map[string]resources.AzureAPIProperty{} + for n, value := range bag.properties { + // The order of containers is important, so we prepend the outermost name. + value.Containers = append([]string{name}, value.Containers...) + newProperties[n] = value + } + bag.properties = newProperties + + newRequiredContainers := make(RequiredContainers, len(bag.requiredContainers)) + for i, containers := range bag.requiredContainers { + newRequiredContainers[i] = append([]string{name}, containers...) + } + for _, requiredName := range resolvedSchema.Required { + if requiredName == name { + newRequiredContainers = append(newRequiredContainers, []string{name}) + } + } + bag.requiredContainers = newRequiredContainers + + result.merge(bag) + continue + } else { + for _, propName := range conflictingProperties { + m.flattenedPropertyConflicts[fmt.Sprintf("%s.%s", name, propName)] = struct{}{} + } + } + } } // Skip read-only properties for input types and write-only properties for output types. diff --git a/provider/pkg/gen/properties_test.go b/provider/pkg/gen/properties_test.go index 61c3ef246fb6..41f1cdc9f40a 100644 --- a/provider/pkg/gen/properties_test.go +++ b/provider/pkg/gen/properties_test.go @@ -194,3 +194,29 @@ func TestNonObjectInvokeResponses(t *testing.T) { assert.NotContains(t, props.properties, resources.SingleValueProperty) }) } + +func TestPropertyUnion(t *testing.T) { + t.Run("no conflict", func(t *testing.T) { + outer := propertyBag{properties: map[string]resources.AzureAPIProperty{ + "foo": {}, + }} + inner := propertyBag{properties: map[string]resources.AzureAPIProperty{ + "bar": {}, + "foo2": {}, + }} + assert.Empty(t, outer.propertyIntersection(&inner)) + }) + + t.Run("conflict", func(t *testing.T) { + outer := propertyBag{properties: map[string]resources.AzureAPIProperty{ + "foo": {}, + "foo2": {}, + "bla": {}, + }} + inner := propertyBag{properties: map[string]resources.AzureAPIProperty{ + "foo": {}, + "bar": {}, + }} + assert.Equal(t, []string{"foo"}, outer.propertyIntersection(&inner)) + }) +}