Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new pipeline stage that merges groups #1579

Merged
merged 2 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
matthchr marked this conversation as resolved.
Show resolved Hide resolved
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
}
}
theunrepentantgeek marked this conversation as resolved.
Show resolved Hide resolved
}

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()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably in the case of a collision we'll panic rather than just overwriting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A collision would be err := result.AddAllowDuplicates(newDef) above returning an error - meaning we had two types that were named the same but are structurally different. Right now that'd just block the whole pipeline and we'd have to come and add handling to deal with it.

}
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