Merge 393d2a7 into 1e458b2

hack/generator/pkg/astbuilder/composite_literals.go

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT license.
+ */
+
+package astbuilder
+
+import (
+	"github.com/dave/dst"
+)
+
+// CompositeLiteralDetails captures the information required to generate code for an inline struct initialization
+type CompositeLiteralDetails struct {
+	structType dst.Expr
+	elts       []dst.Expr
+}
+
+// NewCompositeLiteralDetails creates a new instance for initialization of the specified struct
+// structType is an expression to handle both structs from the current package and imported ones requiring qualification
+func NewCompositeLiteralDetails(structType dst.Expr) *CompositeLiteralDetails {
+	return &CompositeLiteralDetails{
+		structType: structType,
+	}
+}
+
+// AddField adds initialization of another field
+// Returns the receiver to allow method chaining when desired
+func (details *CompositeLiteralDetails) AddField(name string, value dst.Expr) {
+	expr := &dst.KeyValueExpr{
+		Key:   dst.NewIdent(name),
+		Value: dst.Clone(value).(dst.Expr),
+	}
+
+	expr.Decs.Before = dst.NewLine
+	expr.Decs.After = dst.NewLine
+
+	details.elts = append(details.elts, expr)
+}
+
+// Build constructs the actual dst.CompositeLit that's required
+func (details CompositeLiteralDetails) Build() *dst.CompositeLit {
+	return &dst.CompositeLit{
+		Type: details.structType,
+		Elts: details.elts,
+	}
+}

hack/generator/pkg/astmodel/errored_type.go

@@ -163,7 +163,11 @@ func (e *ErroredType) Unwrap() Type {
 // types is a dictionary for resolving named types
 func (e *ErroredType) WriteDebugDescription(builder *strings.Builder, types Types) {
 	builder.WriteString("Error[")
-	e.inner.WriteDebugDescription(builder, types)
+	if e.inner != nil {
+		e.inner.WriteDebugDescription(builder, types)
+	} else {
+		builder.WriteString("<missing>")
+	}
 
 	for _, e := range e.errors {
 		builder.WriteString("|")

hack/generator/pkg/astmodel/function_container.go

@@ -6,9 +6,25 @@
 package astmodel
 
 // FunctionContainer is implemented by Types that contain functions
-// Can't include the withers for modification until we have generics
+// Provides readonly access as we need to use a TypeVisitor for modifications to preserve type wrapping
 type FunctionContainer interface {
 	// Functions returns all the function implementations
 	// A sorted slice is returned to preserve immutability and provide determinism
 	Functions() []Function
+
+	// HasFunctionWithName determines if this resource has a function with the given name
+	HasFunctionWithName(name string) bool
+}
+
+// AsFunctionContainer converts a type into a function container
+// Only use this readonly access as we must use a TypeVisitor for modifications to preserve type wrapping
+func AsFunctionContainer(theType Type) (FunctionContainer, bool) {
+	switch t := theType.(type) {
+	case FunctionContainer:
+		return t, true
+	case MetaType:
+		return AsFunctionContainer(t.Unwrap())
+	default:
+		return nil, false
+	}
 }

hack/generator/pkg/astmodel/object_type.go

@@ -240,7 +240,10 @@ func (objectType *ObjectType) References() TypeNameSet {
 		results.AddAll(property.PropertyType().References())
 	}
 
-	// Not collecting types from functions deliberately.
+	for _, fn := range objectType.functions {
+		results.AddAll(fn.References())
+	}
+
 	return results
 }
 

hack/generator/pkg/astmodel/property_container.go

@@ -6,7 +6,7 @@
 package astmodel
 
 // PropertyContainer is implemented by Types that contain properties
-// Can't include the withers for modification until we have generics
+// Provides readonly access as we need to use a TypeVisitor for modifications to preserve type wrapping
 type PropertyContainer interface {
 	// Properties returns all the properties from this container
 	// A sorted slice is returned to preserve immutability and provide determinism
@@ -15,3 +15,16 @@ type PropertyContainer interface {
 	// Property returns the property and true if the named property is found, nil and false otherwise
 	Property(name PropertyName) (*PropertyDefinition, bool)
 }
+
+// AsPropertyContainer converts a type into a property container
+// Only use this readonly access as we must use a TypeVisitor for modifications to preserve type wrapping
+func AsPropertyContainer(theType Type) (PropertyContainer, bool) {
+	switch t := theType.(type) {
+	case PropertyContainer:
+		return t, true
+	case MetaType:
+		return AsPropertyContainer(t.Unwrap())
+	default:
+		return nil, false
+	}
+}

hack/generator/pkg/astmodel/resource_type.go

@@ -393,6 +393,12 @@ func (resource *ResourceType) Functions() []Function {
 	return functions
 }
 
+// HasFunctionWithName determines if this resource has a function with the given name
+func (resource *ResourceType) HasFunctionWithName(name string) bool {
+	_, ok := resource.functions[name]
+	return ok
+}
+
 // References returns the types referenced by Status or Spec parts of the resource
 func (resource *ResourceType) References() TypeNameSet {
 	spec := resource.spec.References()

hack/generator/pkg/astmodel/std_references.go

@@ -26,9 +26,13 @@ var (
 	APIExtensionsJSONReference   = MakeExternalPackageReference("k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/JSON")
 	APIMachineryErrorsReference  = MakeExternalPackageReference("k8s.io/apimachinery/pkg/util/errors")
 	APIMachineryRuntimeReference = MakeExternalPackageReference("k8s.io/apimachinery/pkg/runtime")
-	ClientGoSchemeReference      = MakeExternalPackageReference("k8s.io/client-go/kubernetes/scheme")
-	ControllerRuntimeAdmission   = MakeExternalPackageReference("sigs.k8s.io/controller-runtime/pkg/webhook/admission")
-	GitHubErrorsReference        = MakeExternalPackageReference("github.com/pkg/errors")
+	APIMachinerySchemaReference  = MakeExternalPackageReference("k8s.io/apimachinery/pkg/runtime/schema")
+
+	ClientGoSchemeReference     = MakeExternalPackageReference("k8s.io/client-go/kubernetes/scheme")
+	ControllerRuntimeAdmission  = MakeExternalPackageReference("sigs.k8s.io/controller-runtime/pkg/webhook/admission")
+	ControllerRuntimeConversion = MakeExternalPackageReference("sigs.k8s.io/controller-runtime/pkg/conversion")
+	ControllerSchemeReference   = MakeExternalPackageReference("sigs.k8s.io/controller-runtime/pkg/scheme")
+	GitHubErrorsReference       = MakeExternalPackageReference("github.com/pkg/errors")
 
 	// References to libraries used for testing
 	CmpReference        = MakeExternalPackageReference("github.com/google/go-cmp/cmp")
@@ -43,8 +47,17 @@ var (
 	// Imports with specified names
 	GomegaImport = NewPackageImport(GomegaReference).WithName(".")
 
-	// Type names
+	// Type names - GenRuntime
 	ResourceReferenceTypeName      = MakeTypeName(GenRuntimeReference, "ResourceReference")
 	KnownResourceReferenceTypeName = MakeTypeName(GenRuntimeReference, "KnownResourceReference")
-	JSONTypeName                   = MakeTypeName(APIExtensionsReference, "JSON")
+	ToARMConverterInterfaceType    = MakeTypeName(GenRuntimeReference, "ToARMConverter")
+
+	// Type names - API Machinery
+	GroupVersionKindTypeName = MakeTypeName(APIMachinerySchemaReference, "GroupVersionKind")
+	SchemeType               = MakeTypeName(APIMachineryRuntimeReference, "Scheme")
+	JSONTypeName             = MakeTypeName(APIExtensionsReference, "JSON")
+
+	// Type names - Controller Runtime
+	ConvertibleInterface = MakeTypeName(ControllerRuntimeConversion, "Convertible")
+	HubInterface         = MakeTypeName(ControllerRuntimeConversion, "Hub")
 )

hack/generator/pkg/astmodel/type_walker.go

@@ -74,6 +74,7 @@ func (t *TypeWalker) visitTypeName(this *TypeVisitor, it TypeName, ctx interface
 	if err != nil {
 		return nil, errors.Wrapf(err, "visitTypeName failed for name %q", it)
 	}
+
 	it, ok := visitedTypeName.(TypeName)
 	if !ok {
 		panic(fmt.Sprintf("TypeWalker visitor visitTypeName must return a TypeName, instead returned %T", visitedTypeName))

hack/generator/pkg/astmodel/types.go

@@ -58,7 +58,7 @@ func (types Types) FullyResolve(t Type) (Type, error) {
 }
 
 // AddAll adds multiple definitions to the set, with the same safety check as Add() to panic if a duplicate is included
-func (types Types) AddAll(otherDefinitions []TypeDefinition) {
+func (types Types) AddAll(otherDefinitions ...TypeDefinition) {
 	for _, t := range otherDefinitions {
 		types.Add(t)
 	}

hack/generator/pkg/astmodel/types_test.go

@@ -50,9 +50,7 @@ func Test_TypesAddAll_GivenTypes_ModifiesSet(t *testing.T) {
 	g := NewGomegaWithT(t)
 
 	types := createTestTypes(alphaDefinition, betaDefinition)
-	otherTypes := []TypeDefinition{gammaDefinition, deltaDefinition}
-
-	types.AddAll(otherTypes)
+	types.AddAll(gammaDefinition, deltaDefinition)
 
 	g.Expect(types).To(ContainElements(gammaDefinition, deltaDefinition))
 }
@@ -61,9 +59,7 @@ func Test_TypesAddAll_GivenOverlappingTypes_Panics(t *testing.T) {
 	g := NewGomegaWithT(t)
 
 	types := createTestTypes(alphaDefinition, betaDefinition)
-	otherTypes := []TypeDefinition{betaDefinition, deltaDefinition}
-
-	g.Expect(func() { types.AddAll(otherTypes) }).To(Panic())
+	g.Expect(func() { types.AddAll(betaDefinition, deltaDefinition) }).To(Panic())
 }
 
 /*

hack/generator/pkg/codegen/code_generator.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel"
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/codegen/pipeline"
+	"github.com/Azure/azure-service-operator/hack/generator/pkg/codegen/storage"
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/config"
 )
 
@@ -78,6 +79,10 @@ 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()
+
 	return []pipeline.Stage{
 
 		pipeline.LoadSchemaIntoTypes(idFactory, configuration, pipeline.DefaultSchemaLoader),
@@ -151,7 +156,14 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration
 		pipeline.AddCrossplaneEmbeddedResourceSpec(idFactory).UsedFor(pipeline.CrossplaneTarget),
 		pipeline.AddCrossplaneEmbeddedResourceStatus(idFactory).UsedFor(pipeline.CrossplaneTarget),
 
-		pipeline.CreateStorageTypes(idFactory).UsedFor(pipeline.ARMTarget), // TODO: For now only used for ARM
+		// Create Storage types
+		//TODO: For now only used for ARM
+		pipeline.InjectOriginalVersionFunction(idFactory).UsedFor(pipeline.ARMTarget),
+		pipeline.CreateStorageTypes(graph).UsedFor(pipeline.ARMTarget),
+		pipeline.InjectOriginalVersionProperty().UsedFor(pipeline.ARMTarget),
+		pipeline.InjectPropertyAssignmentFunctions(graph, idFactory).UsedFor(pipeline.ARMTarget),
+		pipeline.InjectOriginalGVKFunction(idFactory).UsedFor(pipeline.ARMTarget),
+
 		pipeline.SimplifyDefinitions(),
 		pipeline.InjectJsonSerializationTests(idFactory).UsedFor(pipeline.ARMTarget),
 
@@ -218,7 +230,11 @@ func (generator *CodeGenerator) verifyPipeline() error {
 		}
 
 		for _, postreq := range stage.Postrequisites() {
-			stagesExpected[postreq] = append(stagesExpected[postreq], stage.Id())
+			if _, ok := stagesSeen[postreq]; ok {
+				errs = append(errs, errors.Errorf("postrequisite %q of stage %q satisfied too early", postreq, stage.Id()))
+			} else {
+				stagesExpected[postreq] = append(stagesExpected[postreq], stage.Id())
+			}
 		}
 
 		stagesSeen[stage.Id()] = struct{}{}

hack/generator/pkg/codegen/golden_files_test.go

@@ -145,7 +145,15 @@ func NewTestCodeGenerator(testName string, path string, t *testing.T, testConfig
 	// TODO: This isn't as clean as would be liked -- should we remove panic from RemoveStages?
 	switch genPipeline {
 	case config.GenerationPipelineAzure:
-		codegen.RemoveStages("deleteGenerated", "rogueCheck", "createStorage", "reportTypesAndVersions")
+		codegen.RemoveStages(
+			"deleteGenerated",
+			"rogueCheck",
+			"createStorageTypes",
+			"injectOriginalGVKFunction",
+			"injectOriginalVersionFunction",
+			"injectOriginalVersionProperty",
+			"injectPropertyAssignmentFunctions",
+			"reportTypesAndVersions")
 		if !testConfig.HasARMResources {
 			codegen.RemoveStages("createArmTypes", "applyArmConversionInterface")
 			// These stages treat the collection of types as a graph of types rooted by a resource type.

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

@@ -8,69 +8,56 @@ package pipeline
 import (
 	"context"
 
-	kerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/klog/v2"
+	"github.com/pkg/errors"
 
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel"
 	"github.com/Azure/azure-service-operator/hack/generator/pkg/codegen/storage"
 )
 
+const CreateStorageTypesStageId = "createStorageTypes"
+
 // CreateStorageTypes returns a pipeline stage that creates dedicated storage types for each resource and nested object.
 // Storage versions are created for *all* API versions to allow users of older versions of the operator to easily
 // upgrade. This is of course a bit odd for the first release, but defining the approach from day one is useful.
-func CreateStorageTypes(idFactory astmodel.IdentifierFactory) Stage {
-	return MakeStage(
-		"createStorage",
+func CreateStorageTypes(conversionGraph *storage.ConversionGraph) Stage {
+	result := MakeStage(
+		CreateStorageTypesStageId,
 		"Create storage versions of CRD types",
 		func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
 
-			// Create a factory for each group (aka service) and divvy up the types
-			factories := make(map[string]*storage.StorageTypeFactory)
-			for name, def := range types {
+			// Predicate to isolate both resources and complex objects
+			isPropertyContainer := func(def astmodel.TypeDefinition) bool {
+				_, ok := astmodel.AsPropertyContainer(def.Type())
+				return ok
+			}
 
-				ref, ok := name.PackageReference.AsLocalPackage()
-				if !ok {
-					// Skip definitions from non-local packages
-					// (should never happen)
-					klog.Warningf("Skipping storage type generation for unexpected non-local package reference %q", name.PackageReference)
-					continue
-				}
+			// Predicate to filter out ARM types
+			isNotARMType := func(def astmodel.TypeDefinition) bool {
+				return !astmodel.ARMFlag.IsOn(def.Type())
+			}
 
-				factory, ok := factories[ref.Group()]
-				if !ok {
-					klog.V(3).Infof("Creating storage factory for %s", ref.Group())
-					factory = storage.NewStorageTypeFactory(ref.Group(), idFactory)
-					factories[ref.Group()] = factory
-				}
+			// Filter to the types we want to process
+			typesToConvert := types.Where(isPropertyContainer).Where(isNotARMType)
 
-				if astmodel.ARMFlag.IsOn(def.Type()) {
-					// skip ARM types as they don't need storage variants
-					continue
-				}
+			storageTypes := make(astmodel.Types)
+			typeConverter := storage.NewTypeConverter(types)
 
-				factory.Add(def)
-			}
-
-			// Collect up all the results
-			result := make(astmodel.Types)
-			var errs []error
-			for _, factory := range factories {
-				t, err := factory.Types()
+			// Create storage variants
+			for name, def := range typesToConvert {
+				storageDef, err := typeConverter.ConvertDefinition(def)
 				if err != nil {
-					errs = append(errs, err)
-					continue
+					return nil, errors.Wrapf(err, "creating storage variant of %q", name)
 				}
 
-				result.AddTypes(t)
+				storageTypes.Add(storageDef)
+				conversionGraph.AddLink(name.PackageReference, storageDef.Name().PackageReference)
 			}
 
-			err := kerrors.NewAggregate(errs)
-			if err != nil {
-				return nil, err
-			}
-
-			unmodified := types.Except(result)
-			result.AddTypes(unmodified)
+			result := types.Copy()
+			result.AddTypes(storageTypes)
 			return result, nil
 		})
+
+	result.RequiresPrerequisiteStages(injectOriginalVersionFunctionStageId)
+	return result
 }