Merge branch 'master' into feature/storage-version-nonlocal-types

hack/generator/pkg/astmodel/types.go

@@ -111,6 +111,14 @@ func (types Types) Contains(name TypeName) bool {
 	return ok
 }
 
+// OverlayWith creates a new set containing all the type definitions from both this and the provided set. Any name
+// collisions are resolved in favour of the provided set. Returns a new independent set, leaving the original unmodified.
+func (types Types) OverlayWith(t Types) Types {
+	result := t.Copy()
+	result.AddTypes(types.Except(t))
+	return result
+}
+
 // TypesDisjointUnion merges this and other, with a safety check that no type is overwritten.
 // If an attempt is made to overwrite a type, this function panics
 func TypesDisjointUnion(s1 Types, s2 Types) Types {

hack/generator/pkg/astmodel/types_test.go

@@ -12,11 +12,12 @@ import (
 )
 
 var (
-	pkg             = MakeExternalPackageReference("foo")
-	alphaDefinition = createTestDefinition("alpha")
-	betaDefinition  = createTestDefinition("beta")
-	gammaDefinition = createTestDefinition("gamma")
-	deltaDefinition = createTestDefinition("delta")
+	pkg                = MakeExternalPackageReference("foo")
+	alphaDefinition    = createTestDefinition("alpha", StringType)
+	betaDefinition     = createTestDefinition("beta", StringType)
+	gammaDefinition    = createTestDefinition("gamma", StringType)
+	deltaDefinition    = createTestDefinition("delta", StringType)
+	deltaIntDefinition = createTestDefinition("delta", IntType)
 )
 
 /*
@@ -146,13 +147,45 @@ func Test_TypesExcept_GivenSubset_ReturnsExpectedSet(t *testing.T) {
 	g.Expect(set).To(ContainElement(deltaDefinition))
 }
 
+/*
+ * Overlay() tests
+ */
+
+func Test_TypesOverlayWith_GivenDisjointSets_ReturnsUnionSet(t *testing.T) {
+	g := NewGomegaWithT(t)
+	left := createTestTypes(alphaDefinition, betaDefinition)
+	right := createTestTypes(gammaDefinition, deltaDefinition)
+
+	set := left.OverlayWith(right)
+
+	g.Expect(len(set)).To(Equal(4))
+	g.Expect(set).To(ContainElement(alphaDefinition))
+	g.Expect(set).To(ContainElement(betaDefinition))
+	g.Expect(set).To(ContainElement(gammaDefinition))
+	g.Expect(set).To(ContainElement(deltaDefinition))
+}
+
+func Test_TypesOverlayWith_GivenOverlappingSets_PrefersTypeInOverlay(t *testing.T) {
+	g := NewGomegaWithT(t)
+	left := createTestTypes(alphaDefinition, deltaDefinition)
+	right := createTestTypes(gammaDefinition, deltaIntDefinition)
+
+	set := left.OverlayWith(right)
+
+	g.Expect(len(set)).To(Equal(3))
+	g.Expect(set).To(ContainElement(alphaDefinition))
+	g.Expect(set).To(ContainElement(gammaDefinition))
+	g.Expect(set).To(ContainElement(deltaIntDefinition))
+	g.Expect(set).NotTo(ContainElement(deltaDefinition))
+}
+
 /*
  * Utility functions
  */
 
-func createTestDefinition(name string) TypeDefinition {
+func createTestDefinition(name string, underlyingType Type) TypeDefinition {
 	n := MakeTypeName(pkg, name)
-	return MakeTypeDefinition(n, StringType)
+	return MakeTypeDefinition(n, underlyingType)
 }
 
 func createTestTypes(defs ...TypeDefinition) Types {

hack/generator/pkg/codegen/code_generator.go

@@ -119,6 +119,9 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration
 		// Strip out redundant type aliases:
 		removeTypeAliases(),
 
+		// Collapse cross group references
+		collapseCrossGroupReferences(),
+
 		// De-pluralize resource types
 		// (Must come after type aliases are resolved)
 		improveResourcePluralization().

hack/generator/pkg/codegen/golden_files_test.go

@@ -151,17 +151,20 @@ func NewTestCodeGenerator(testName string, path string, t *testing.T, testConfig
 		codegen.RemoveStages("deleteGenerated", "rogueCheck", "createStorage", "reportTypesAndVersions")
 		if !testConfig.HasARMResources {
 			codegen.RemoveStages("createArmTypes", "applyArmConversionInterface")
-			// removeEmbeddedResources treats the collection of types as a graph of types rooted by a resource type.
+			// These stages treat the collection of types as a graph of types rooted by a resource type.
 			// In the degenerate case where there are no resources it behaves the same as stripUnreferenced - removing
 			// all types. Remove it in phases that have no resources to avoid this.
-			codegen.RemoveStages("removeEmbeddedResources")
+			codegen.RemoveStages("removeEmbeddedResources", "collapseCrossGroupReferences")
+
 			codegen.ReplaceStage("stripUnreferenced", stripUnusedTypesPipelineStage())
 		} else {
 			codegen.ReplaceStage("addCrossResourceReferences", addCrossResourceReferencesForTest(idFactory))
 		}
 	case config.GenerationPipelineCrossplane:
 		codegen.RemoveStages("deleteGenerated", "rogueCheck")
-		codegen.ReplaceStage("stripUnreferenced", stripUnusedTypesPipelineStage())
+		if !testConfig.HasARMResources {
+			codegen.ReplaceStage("stripUnreferenced", stripUnusedTypesPipelineStage())
+		}
 
 	default:
 		return nil, errors.Errorf("unknown pipeline kind %q", string(pipeline))

hack/generator/pkg/codegen/pipeline_collapse_cross_group_refs.go

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT license.
+ */
+
+package codegen
+
+import (
+	"context"
+
+	"github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel"
+	"github.com/pkg/errors"
+)
+
+// collapseCrossGroupReferences finds and removes references between API groups. This isn't particularly common
+// but does occur in a few instances, for example from Microsoft.Compute -> Microsoft.Compute.Extensions.
+func collapseCrossGroupReferences() PipelineStage {
+	return MakePipelineStage(
+		"collapseCrossGroupReferences",
+		"Finds and removes cross group references",
+		func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
+			resources := astmodel.CollectResourceDefinitions(types)
+			result := make(astmodel.Types)
+
+			for resourceName := range resources {
+				walker := newTypeWalker(types, resourceName)
+				updatedTypes, err := walker.Walk(types[resourceName])
+				if err != nil {
+					return nil, errors.Wrapf(err, "failed walking types")
+				}
+
+				for _, newDef := range updatedTypes {
+					err := result.AddAllowDuplicates(newDef)
+					if err != nil {
+						return nil, err
+					}
+				}
+			}
+
+			return result, nil
+		})
+}
+
+func newTypeWalker(types astmodel.Types, resourceName astmodel.TypeName) *astmodel.TypeWalker {
+	visitor := astmodel.TypeVisitorBuilder{}.Build()
+	walker := astmodel.NewTypeWalker(types, visitor)
+	walker.AfterVisit = func(original astmodel.TypeDefinition, updated astmodel.TypeDefinition, ctx interface{}) (astmodel.TypeDefinition, error) {
+		if !resourceName.PackageReference.Equals(updated.Name().PackageReference) {
+			// Note: If we ever find this generating colliding names, we might need to introduce a unique suffix.
+			// For now though it doesn't seem to, so preserving the shorter names as they're clearer.
+			updated = updated.WithName(astmodel.MakeTypeName(resourceName.PackageReference, updated.Name().Name()))
+		}
+		return astmodel.IdentityAfterVisit(original, updated, ctx)
+	}
+
+	return walker
+}

hack/generator/pkg/codegen/testdata/ARMCodeGeneratorPipeline.golden

@@ -9,6 +9,7 @@ nameTypes                                      Name inner types for CRD
 propertyRewrites                               Applying type transformers to properties
 determineResourceOwnership                     Determine ARM resource relationships
 removeAliases                                  Remove type aliases
+collapseCrossGroupReferences                   Finds and removes cross group references
 pluralizeNames                                 Improve resource pluralization
 stripUnreferenced                              Strip unreferenced types
 assertTypesStructureValid                      Asserts that the types collection is valid

hack/generator/pkg/codegen/testdata/ArmResource/Arm_oneof_resource_conversion_on_arm_type_only_crossplane.golden

@@ -28,11 +28,6 @@ type FakeResourceList struct {
 	Items           []FakeResource `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	Test *FakeResource `json:"test,omitempty"`
-}
-
 type FakeResource_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           FakeResourceParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_dependent_resource_and_ownership_crossplane.golden

@@ -68,11 +68,24 @@ type CList struct {
 	Items           []C `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	A *A `json:"a,omitempty"`
-	B *B `json:"b,omitempty"`
-	C *C `json:"c,omitempty"`
+// +kubebuilder:rbac:groups=test.infra.azure.com,resources=ds,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=test.infra.azure.com,resources={ds/status,ds/finalizers},verbs=get;update;patch
+
+// +kubebuilder:object:root=true
+// +kubebuilder:storageversion
+//Generated from: https://test.test/schemas/2020-01-01/test.json#/resourceDefinitions/D
+type D struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+	Spec              D_Spec `json:"spec,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+//Generated from: https://test.test/schemas/2020-01-01/test.json#/resourceDefinitions/D
+type DList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []D `json:"items"`
 }
 
 type A_Spec struct {
@@ -90,6 +103,11 @@ type C_Spec struct {
 	ForProvider           CParameters `json:"forProvider"`
 }
 
+type D_Spec struct {
+	v1alpha1.ResourceSpec `json:",inline"`
+	ForProvider           DParameters `json:"forProvider"`
+}
+
 type AParameters struct {
 	// +kubebuilder:validation:Required
 	APIVersion ASpecAPIVersion `json:"apiVersion"`
@@ -143,6 +161,27 @@ type CParameters struct {
 	Type CSpecType `json:"type"`
 }
 
+type DParameters struct {
+	AName         string              `json:"aName"`
+	ANameRef      *v1alpha1.Reference `json:"aNameRef,omitempty"`
+	ANameSelector *v1alpha1.Selector  `json:"aNameSelector,omitempty"`
+
+	// +kubebuilder:validation:Required
+	APIVersion    DSpecAPIVersion     `json:"apiVersion"`
+	BName         string              `json:"bName"`
+	BNameRef      *v1alpha1.Reference `json:"bNameRef,omitempty"`
+	BNameSelector *v1alpha1.Selector  `json:"bNameSelector,omitempty"`
+
+	// +kubebuilder:validation:Required
+	Name                      string              `json:"name"`
+	ResourceGroupName         string              `json:"resourceGroupName"`
+	ResourceGroupNameRef      *v1alpha1.Reference `json:"resourceGroupNameRef,omitempty"`
+	ResourceGroupNameSelector *v1alpha1.Selector  `json:"resourceGroupNameSelector,omitempty"`
+
+	// +kubebuilder:validation:Required
+	Type DSpecType `json:"type"`
+}
+
 // +kubebuilder:validation:Enum={"2020-06-01"}
 type ASpecAPIVersion string
 
@@ -173,6 +212,16 @@ type CSpecType string
 
 const CSpecTypeMicrosoftAzureC = CSpecType("Microsoft.Azure/C")
 
+// +kubebuilder:validation:Enum={"2020-06-01"}
+type DSpecAPIVersion string
+
+const DSpecAPIVersion20200601 = DSpecAPIVersion("2020-06-01")
+
+// +kubebuilder:validation:Enum={"Microsoft.Azure/D"}
+type DSpecType string
+
+const DSpecTypeMicrosoftAzureD = DSpecType("Microsoft.Azure/D")
+
 func init() {
-	SchemeBuilder.Register(&A{}, &AList{}, &B{}, &BList{}, &C{}, &CList{})
+	SchemeBuilder.Register(&A{}, &AList{}, &B{}, &BList{}, &C{}, &CList{}, &D{}, &DList{})
 }

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_embedded_resource_empty_objecttype_removed_crossplane.golden

@@ -48,12 +48,6 @@ type BList struct {
 	Items           []B `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	A *A `json:"a,omitempty"`
-	B *B `json:"b,omitempty"`
-}
-
 type A_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           AParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_embedded_resource_has_embedded_resource_inside_crossplane.golden

@@ -68,13 +68,6 @@ type CList struct {
 	Items           []C `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	A *A `json:"a,omitempty"`
-	B *B `json:"b,omitempty"`
-	C *C `json:"c,omitempty"`
-}
-
 type A_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           AParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_embedded_resource_multiple_contexts_crossplane.golden

@@ -28,11 +28,6 @@ type AList struct {
 	Items           []A `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	A *A `json:"a,omitempty"`
-}
-
 type A_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           AParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_embedded_resource_removed_crossplane.golden

@@ -48,12 +48,6 @@ type BList struct {
 	Items           []B `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	A *A `json:"a,omitempty"`
-	B *B `json:"b,omitempty"`
-}
-
 type A_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           AParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_embedded_subresource_crossplane.golden

@@ -48,12 +48,6 @@ type BList struct {
 	Items           []B `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	A *A `json:"a,omitempty"`
-	B *B `json:"b,omitempty"`
-}
-
 type A_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           AParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_embedded_subresource_same_properties_crossplane.golden

@@ -48,12 +48,6 @@ type BList struct {
 	Items           []B `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	A *A `json:"a,omitempty"`
-	B *B `json:"b,omitempty"`
-}
-
 type A_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           AParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_id_resource_reference_crossplane.golden

@@ -28,11 +28,6 @@ type FakeResourceList struct {
 	Items           []FakeResource `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	Test *FakeResource `json:"test,omitempty"`
-}
-
 type FakeResource_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           FakeResourceParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_required_and_optional_resource_references_crossplane.golden

@@ -28,11 +28,6 @@ type FakeResourceList struct {
 	Items           []FakeResource `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	Test *FakeResource `json:"test,omitempty"`
-}
-
 type FakeResource_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           FakeResourceParameters `json:"forProvider"`

hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_array_properties_crossplane.golden

@@ -28,11 +28,6 @@ type FakeResourceList struct {
 	Items           []FakeResource `json:"items"`
 }
 
-//Generated from: https://test.test/schemas/2020-01-01/test.json
-type Test struct {
-	Test *FakeResource `json:"test,omitempty"`
-}
-
 type FakeResource_Spec struct {
 	v1alpha1.ResourceSpec `json:",inline"`
 	ForProvider           FakeResourceParameters `json:"forProvider"`