Skip to content

Commit

Permalink
Add new pipeline stage that merges groups
Browse files Browse the repository at this point in the history
  - In the case that group A references group B, we need to
    pull group B's types into group A.
  - Fixes #1578
  • Loading branch information
matthchr committed Jun 19, 2021
1 parent a6df6c7 commit 7bfe460
Show file tree
Hide file tree
Showing 19 changed files with 122 additions and 85 deletions.
3 changes: 3 additions & 0 deletions hack/generator/pkg/codegen/code_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
9 changes: 6 additions & 3 deletions hack/generator/pkg/codegen/golden_files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
57 changes: 57 additions & 0 deletions hack/generator/pkg/codegen/pipeline_collapse_cross_group_refs.go
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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"`
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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{})
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down

0 comments on commit 7bfe460

Please sign in to comment.