Improve flattening

hack/generated/controllers/crd_cosmosdb_test.go

@@ -32,7 +32,7 @@ func Test_CosmosDB_CRUD(t *testing.T) {
 			Location:                 &tc.AzureRegion,
 			Owner:                    testcommon.AsOwner(rg.ObjectMeta),
 			Kind:                     &kind,
-			DatabaseAccountOfferType: documentdb.DatabaseAccountsSpecPropertiesDatabaseAccountOfferTypeStandard,
+			DatabaseAccountOfferType: documentdb.DatabaseAccountCreateUpdatePropertiesDatabaseAccountOfferTypeStandard,
 			Locations: []documentdb.Location{
 				{
 					LocationName: &tc.AzureRegion,

hack/generated/controllers/crd_disk_test.go

@@ -49,7 +49,7 @@ func Test_Disk_CRUD(t *testing.T) {
 
 	// Perform a simple patch.
 	patcher := tc.NewResourcePatcher(disk)
-	networkAccessPolicy := compute.DisksSpecPropertiesNetworkAccessPolicyDenyAll
+	networkAccessPolicy := compute.DiskPropertiesNetworkAccessPolicyDenyAll
 	disk.Spec.NetworkAccessPolicy = &networkAccessPolicy
 	patcher.Patch(disk)
 

hack/generated/controllers/crd_networking_loadbalancer_test.go

@@ -40,7 +40,7 @@ func Test_LoadBalancer_CRUD(t *testing.T) {
 			Sku: &network.PublicIPAddressSku{
 				Name: &sku,
 			},
-			PublicIPAllocationMethod: network.PublicIPAddressesSpecPropertiesPublicIPAllocationMethodStatic,
+			PublicIPAllocationMethod: network.PublicIPAddressPropertiesFormatPublicIPAllocationMethodStatic,
 		},
 	}
 

hack/generated/controllers/crd_networking_publicip_test.go

@@ -34,7 +34,7 @@ func Test_PublicIP_CRUD(t *testing.T) {
 			Sku: &network.PublicIPAddressSku{
 				Name: &sku,
 			},
-			PublicIPAllocationMethod: network.PublicIPAddressesSpecPropertiesPublicIPAllocationMethodStatic,
+			PublicIPAllocationMethod: network.PublicIPAddressPropertiesFormatPublicIPAllocationMethodStatic,
 		},
 	}
 

hack/generated/controllers/crd_storageaccount_test.go

@@ -33,8 +33,8 @@ func Test_StorageAccount_CRUD(t *testing.T) {
 			Location: tc.AzureRegion,
 			Owner:    testcommon.AsOwner(rg.ObjectMeta),
 			Kind:     storage.StorageAccountsSpecKindBlobStorage,
-			Sku: storage.StorageAccounts_Spec_Sku{
-				Name: storage.StorageAccountsSpecSkuNameStandardLRS,
+			Sku: storage.Sku{
+				Name: storage.SkuNameStandardLRS,
 			},
 			// TODO: They mark this property as optional but actually it is required
 			AccessTier: &accessTier,
@@ -100,7 +100,6 @@ func StorageAccount_BlobServices_CRUD(tc testcommon.KubePerTestContext, storageA
 }
 
 func StorageAccount_BlobServices_Container_CRUD(tc testcommon.KubePerTestContext, blobService metav1.ObjectMeta) {
-
 	blobContainer := &storage.StorageAccountsBlobServicesContainer{
 		ObjectMeta: tc.MakeObjectMeta("container"),
 		Spec: storage.StorageAccountsBlobServicesContainers_Spec{

hack/generated/controllers/crd_vmss_test.go

@@ -9,15 +9,16 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/Azure/go-autorest/autorest/to"
+	. "github.com/onsi/gomega"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	compute "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.compute/v1alpha1api20201201"
 	network "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.network/v1alpha1api20201101"
 	resources "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.resources/v1alpha1api20200601"
 	"github.com/Azure/azure-service-operator/hack/generated/pkg/genruntime"
 	"github.com/Azure/azure-service-operator/hack/generated/pkg/testcommon"
-	"github.com/Azure/go-autorest/autorest/to"
-	. "github.com/onsi/gomega"
-	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func newVNETForVMSS(tc testcommon.KubePerTestContext, owner genruntime.KnownResourceReference) *network.VirtualNetwork {
@@ -53,7 +54,7 @@ func newPublicIPAddressForVMSS(tc testcommon.KubePerTestContext, owner genruntim
 			Sku: &network.PublicIPAddressSku{
 				Name: &publicIPAddressSku,
 			},
-			PublicIPAllocationMethod: network.PublicIPAddressesSpecPropertiesPublicIPAllocationMethodStatic,
+			PublicIPAllocationMethod: network.PublicIPAddressPropertiesFormatPublicIPAllocationMethodStatic,
 		},
 	}
 }

hack/generated/controllers/deployment_reconciler_negative_test.go

@@ -11,13 +11,14 @@ import (
 	. "github.com/onsi/gomega"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/Azure/go-autorest/autorest/to"
+
 	compute "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.compute/v1alpha1api20201201"
 	resources "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.resources/v1alpha1api20200601"
 	storage "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.storage/v1alpha1api20210401"
 	"github.com/Azure/azure-service-operator/hack/generated/pkg/armclient"
 	"github.com/Azure/azure-service-operator/hack/generated/pkg/reconcilers"
 	"github.com/Azure/azure-service-operator/hack/generated/pkg/testcommon"
-	"github.com/Azure/go-autorest/autorest/to"
 )
 
 func newStorageAccountWithInvalidKeyExpiration(tc testcommon.KubePerTestContext, rg *resources.ResourceGroup) *storage.StorageAccount {
@@ -32,8 +33,8 @@ func newStorageAccountWithInvalidKeyExpiration(tc testcommon.KubePerTestContext,
 			Location: tc.AzureRegion,
 			Owner:    testcommon.AsOwner(rg.ObjectMeta),
 			Kind:     storage.StorageAccountsSpecKindBlobStorage,
-			Sku: storage.StorageAccounts_Spec_Sku{
-				Name: storage.StorageAccountsSpecSkuNameStandardLRS,
+			Sku: storage.Sku{
+				Name: storage.SkuNameStandardLRS,
 			},
 			AccessTier: &accessTier,
 			KeyPolicy: &storage.KeyPolicy{

hack/generated/controllers/edge_case_test.go

@@ -8,15 +8,16 @@ package controllers_test
 import (
 	"testing"
 
+	. "github.com/onsi/gomega"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
 	network "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.network/v1alpha1api20201101"
 	resources "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.resources/v1alpha1api20200601"
 	storage "github.com/Azure/azure-service-operator/hack/generated/_apis/microsoft.storage/v1alpha1api20210401"
 	"github.com/Azure/azure-service-operator/hack/generated/pkg/reconcilers"
 	"github.com/Azure/azure-service-operator/hack/generated/pkg/testcommon"
-	. "github.com/onsi/gomega"
-	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 func waitForOwnerMissingError(tc testcommon.KubePerTestContext, obj controllerutil.Object) {
@@ -50,8 +51,8 @@ func storageAccountAndResourceGroupProvisionedOutOfOrderHelper(t *testing.T, wai
 			Location: tc.AzureRegion,
 			Owner:    testcommon.AsOwner(rg.ObjectMeta),
 			Kind:     storage.StorageAccountsSpecKindBlobStorage,
-			Sku: storage.StorageAccounts_Spec_Sku{
-				Name: storage.StorageAccountsSpecSkuNameStandardLRS,
+			Sku: storage.Sku{
+				Name: storage.SkuNameStandardLRS,
 			},
 			AccessTier: &accessTier,
 		},
@@ -160,8 +161,8 @@ func Test_CreateStorageAccountThatAlreadyExists_ReconcilesSuccessfully(t *testin
 			Location: tc.AzureRegion,
 			Owner:    testcommon.AsOwner(rg.ObjectMeta),
 			Kind:     storage.StorageAccountsSpecKindBlobStorage,
-			Sku: storage.StorageAccounts_Spec_Sku{
-				Name: storage.StorageAccountsSpecSkuNameStandardLRS,
+			Sku: storage.Sku{
+				Name: storage.SkuNameStandardLRS,
 			},
 			AccessTier: &accessTier,
 		},

hack/generator/pkg/astmodel/property_definition.go

@@ -134,7 +134,7 @@ func (property *PropertyDefinition) WithType(newType Type) *PropertyDefinition {
 		panic("nil type provided to WithType")
 	}
 
-	if property.propertyType.Equals(newType) {
+	if property.propertyType == newType || property.propertyType.Equals(newType) {
 		return property
 	}
 
@@ -309,6 +309,10 @@ func (property *PropertyDefinition) renderedTags() string {
 
 // AsField generates a Go AST field node representing this property definition
 func (property *PropertyDefinition) AsField(codeGenerationContext *CodeGenerationContext) *dst.Field {
+	if property.flatten {
+		panic(fmt.Sprintf("property %s marked for flattening was not flattened", property.propertyName))
+	}
+
 	tags := property.renderedTags()
 
 	var names []*dst.Ident

hack/generator/pkg/astmodel/type_definition.go

@@ -6,11 +6,13 @@
 package astmodel
 
 import (
+	"fmt"
 	"go/token"
 
-	"github.com/Azure/azure-service-operator/hack/generator/pkg/astbuilder"
 	"github.com/dave/dst"
 	"github.com/pkg/errors"
+
+	"github.com/Azure/azure-service-operator/hack/generator/pkg/astbuilder"
 )
 
 // TypeDefinition is a name paired with a type
@@ -73,6 +75,12 @@ func (def TypeDefinition) AsDeclarations(codeGenerationContext *CodeGenerationCo
 		Description: def.description,
 	}
 
+	defer func() {
+		if p := recover(); p != nil {
+			panic(fmt.Sprintf("while generating %s/%s: %s", codeGenerationContext.currentPackage, def.name, p))
+		}
+	}()
+
 	return def.theType.AsDeclarations(codeGenerationContext, declContext)
 }
 

hack/generator/pkg/codegen/code_generator.go

@@ -79,7 +79,6 @@ func NewCodeGeneratorFromConfig(configuration *config.Configuration, idFactory a
 }
 
 func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration *config.Configuration) []pipeline.Stage {
-
 	// graph keeps track of the conversions we need between different API & Storage versions
 	graph := storage.NewConversionGraph()
 
@@ -157,7 +156,7 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration
 		pipeline.AddCrossplaneEmbeddedResourceStatus(idFactory).UsedFor(pipeline.CrossplaneTarget),
 
 		// Create Storage types
-		//TODO: For now only used for ARM
+		// TODO: For now only used for ARM
 		pipeline.InjectOriginalVersionFunction(idFactory).UsedFor(pipeline.ARMTarget),
 		pipeline.CreateStorageTypes(graph).UsedFor(pipeline.ARMTarget),
 		pipeline.InjectOriginalVersionProperty().UsedFor(pipeline.ARMTarget),

hack/generator/pkg/codegen/pipeline/create_arm_types.go

@@ -140,6 +140,10 @@ func removeValidations(t *astmodel.ObjectType) (*astmodel.ObjectType, error) {
 	return t, nil
 }
 
+func removeFlattening(t *astmodel.ObjectType) (*astmodel.ObjectType, error) {
+	return removeFlattenFromObject(t), nil
+}
+
 func (c *armTypeCreator) createARMTypeDefinition(isSpecType bool, def astmodel.TypeDefinition) (astmodel.TypeDefinition, error) {
 	convertObjectPropertiesForARM := func(t *astmodel.ObjectType) (*astmodel.ObjectType, error) {
 		return c.convertObjectPropertiesForARM(t, isSpecType)
@@ -155,7 +159,7 @@ func (c *armTypeCreator) createARMTypeDefinition(isSpecType bool, def astmodel.T
 	}
 
 	armName := astmodel.CreateARMTypeName(def.Name())
-	armDef, err := def.WithName(armName).ApplyObjectTransformations(removeValidations, convertObjectPropertiesForARM, addOneOfConversionFunctionIfNeeded)
+	armDef, err := def.WithName(armName).ApplyObjectTransformations(removeValidations, convertObjectPropertiesForARM, addOneOfConversionFunctionIfNeeded, removeFlattening)
 	if err != nil {
 		return astmodel.TypeDefinition{},
 			errors.Wrapf(err, "creating ARM prototype %v from Kubernetes definition %v", armName, def.Name())

hack/generator/pkg/codegen/pipeline/flatten_properties.go

@@ -56,7 +56,7 @@ func makeFlatteningVisitor(defs astmodel.Types) astmodel.TypeVisitor {
 			// safety check:
 			if err := checkForDuplicateNames(newProps); err != nil {
 				klog.Warningf("Skipping flattening for %s: %s", name, err)
-				return it, nil // nolint:nilerr
+				return removeFlattenFromObject(it), nil
 			}
 
 			if len(newProps) != len(it.Properties()) {
@@ -67,17 +67,28 @@ func makeFlatteningVisitor(defs astmodel.Types) astmodel.TypeVisitor {
 
 			return result, nil
 		},
-		VisitFlaggedType: func(this *astmodel.TypeVisitor, it *astmodel.FlaggedType, ctx interface{}) (astmodel.Type, error) {
-			// skip ARM types, do not flatten
-			if it.HasFlag(astmodel.ARMFlag) {
-				return it, nil
-			}
-
-			return astmodel.IdentityVisitOfFlaggedType(this, it, ctx)
-		},
 	}.Build()
 }
 
+func removeFlattenFromObject(tObj *astmodel.ObjectType) *astmodel.ObjectType {
+	var props []*astmodel.PropertyDefinition
+	for _, prop := range tObj.Properties() {
+		prop = prop.WithType(removeFlatten(prop.PropertyType()))
+		prop = prop.SetFlatten(false)
+		props = append(props, prop)
+	}
+
+	return tObj.WithProperties(props...)
+}
+
+func removeFlatten(t astmodel.Type) astmodel.Type {
+	if tObj, ok := t.(*astmodel.ObjectType); ok {
+		return removeFlattenFromObject(tObj)
+	}
+
+	return t
+}
+
 func checkForDuplicateNames(props []*astmodel.PropertyDefinition) error {
 	names := make(map[astmodel.PropertyName]struct{})
 	for _, p := range props {

hack/generator/pkg/codegen/pipeline/flatten_properties_test.go

@@ -30,9 +30,16 @@ func TestDuplicateNamesAreCaught(t *testing.T) {
 	types.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(placeholderPackage, "objType"), objType))
 
 	result, err := applyPropertyFlattening(context.Background(), types)
-	// We don't fail but flattening does not occur
+
+	// We don't fail but flattening does not occur, and flatten is set to false
 	g.Expect(err).ToNot(HaveOccurred())
-	g.Expect(result).To(Equal(types))
+
+	// identical to objType above but the "inner" prop is not set to flatten
+	newObjType := astmodel.NewObjectType().WithProperties(prop, innerObjProp.SetFlatten(false))
+	expectedTypes := make(astmodel.Types)
+	expectedTypes.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(placeholderPackage, "objType"), newObjType))
+
+	g.Expect(result).To(Equal(expectedTypes))
 }
 
 func TestFlatteningWorks(t *testing.T) {