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

Create conversion graph #1627

Merged
merged 17 commits into from
Jul 21, 2021
Merged
Show file tree
Hide file tree
Changes from 14 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
9 changes: 8 additions & 1 deletion hack/generator/pkg/astmodel/package_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,15 @@ func (v *versionComparer) isPreviewVersionLabel(identifier string) (int, bool) {
// special set, and if so returns its true. If the passed identifier does not contain one,
// returns false.
func containsPreviewVersionLabel(identifier string) bool {
// Work out where the API version starts in the identifier
// (We don't want to return preview just because the operator itself is in preview)
startOfAPIVersion := 0
if strings.HasPrefix(identifier, generatorVersionPrefix) {
startOfAPIVersion = len(generatorVersionPrefix)
}

for _, id := range previewVersionLabels {
if strings.LastIndex(identifier, id) > len(generatorVersionPrefix) {
if strings.LastIndex(identifier, id) > startOfAPIVersion {
return true
}
}
Expand Down
26 changes: 26 additions & 0 deletions hack/generator/pkg/astmodel/package_reference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,29 @@ func TestVersionComparerCompareNumeric(t *testing.T) {
})
}
}

func TestContainsPreviewVersionLabel(t *testing.T) {
t.Parallel()

cases := []struct {
name string
version string
expected bool
}{
{"Non-preview", "v20200201", false},
{"Alpha", "v20200201alpha", true},
{"Beta", "v20200201beta", true},
{"Preview", "v20200201preview", true},
}

for _, c := range cases {
c := c
t.Run(c.name, func(t *testing.T) {
t.Parallel()
g := NewGomegaWithT(t)

isPreview := containsPreviewVersionLabel(c.version)
g.Expect(isPreview).To(Equal(c.expected))
})
}
}
1 change: 1 addition & 0 deletions hack/generator/pkg/codegen/code_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration

// Create Storage types
// TODO: For now only used for ARM
pipeline.CreateConversionGraph().UsedFor(pipeline.ARMTarget),
pipeline.InjectOriginalVersionFunction(idFactory).UsedFor(pipeline.ARMTarget),
pipeline.CreateStorageTypes().UsedFor(pipeline.ARMTarget),
pipeline.InjectOriginalVersionProperty().UsedFor(pipeline.ARMTarget),
Expand Down
45 changes: 45 additions & 0 deletions hack/generator/pkg/codegen/pipeline/create_conversion_graph.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright (c) Microsoft Corporation.
* Licensed under the MIT license.
*/

package pipeline

import (
"context"

"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"
)

// CreateConversionGraphStageId is the unique identifier for this stage
const CreateConversionGraphStageId = "createConversionGraph"

// CreateConversionGraph walks the set of available types and creates a graph of conversions that will be used to
// convert resources to/from the designated storage (or hub) version
func CreateConversionGraph() Stage {
stage := MakeStage(
CreateConversionGraphStageId,
"Create the graph of conversions between versions of each resource group",
func(ctx context.Context, state *State) (*State, error) {
// Collect all distinct references
allReferences := astmodel.NewPackageReferenceSet()
for _, def := range state.Types() {
allReferences.AddReference(def.Name().PackageReference)
}

builder := storage.NewConversionGraphBuilder()
builder.AddAll(allReferences)
graph, err := builder.Build()
if err != nil {
// Shouldn't have any non-local references, if we do, abort
return nil, errors.Wrapf(err, "creating conversion graph")
}

return state.WithConversionGraph(graph), nil
})

return stage
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright (c) Microsoft Corporation.
* Licensed under the MIT license.
*/

package pipeline

import (
"context"
"testing"

. "github.com/onsi/gomega"

"github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel"
"github.com/Azure/azure-service-operator/hack/generator/pkg/test"
)

func TestCreateConversionGraph(t *testing.T) {
g := NewGomegaWithT(t)

person2020 := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.KnownAsProperty, test.FamilyNameProperty)
person2021 := test.CreateSpec(test.Pkg2021, "Person", test.FullNameProperty, test.KnownAsProperty, test.FamilyNameProperty)
person2022 := test.CreateSpec(test.Pkg2022, "Person", test.FullNameProperty, test.KnownAsProperty, test.FamilyNameProperty)

types := make(astmodel.Types)
types.AddAll(person2020, person2021, person2022)
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth having some unrelated types in here to make sure there aren't any links created between Person and OtherType?

Copy link
Member Author

Choose a reason for hiding this comment

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

Noted for a followup PR.


initialState := NewState().WithTypes(types)
stage := CreateConversionGraph()
finalState, err := stage.Run(context.TODO(), initialState)
g.Expect(err).To(Succeed())
g.Expect(finalState.Types()).To(Equal(types))
g.Expect(finalState.ConversionGraph()).NotTo(BeNil())

graph := finalState.ConversionGraph()

// Expect to have a link from Pkg2020 to a matching storage version
storage2020, ok := graph.LookupTransition(test.Pkg2020)
g.Expect(ok).To(BeTrue())
g.Expect(storage2020.String()).To(ContainSubstring(test.Pkg2020.Version()))

// Expect to have a link from Pkg2021 to a matching storage version
storage2021, ok := graph.LookupTransition(test.Pkg2021)
g.Expect(ok).To(BeTrue())
g.Expect(storage2021.String()).To(ContainSubstring(test.Pkg2021.Version()))

// Expect to have a link from Pkg2022 to a matching storage version
storage2022, ok := graph.LookupTransition(test.Pkg2022)
g.Expect(ok).To(BeTrue())
g.Expect(storage2022.String()).To(ContainSubstring(test.Pkg2022.Version()))

// Expect to have a link from Storage2020 to Storage2021
linkedFrom2020, ok := graph.LookupTransition(storage2020)
g.Expect(ok).To(BeTrue())
g.Expect(linkedFrom2020).To(Equal(storage2021))

// Expect to have a link from Storage2021 version Storage2022
linkedFrom2021, ok := graph.LookupTransition(storage2021)
g.Expect(ok).To(BeTrue())
g.Expect(linkedFrom2021).To(Equal(storage2022))

// Expect NOT to have a link from Storage2022
_, ok = graph.LookupTransition(storage2022)
g.Expect(ok).To(BeFalse())

// Finally, check that the five links we've verified are the only ones there
// We check this last, so that a failure doesn't block more detailed checks
g.Expect(graph.TransitionCount()).To(Equal(5))
}
17 changes: 8 additions & 9 deletions hack/generator/pkg/codegen/pipeline/create_storage_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ func CreateStorageTypes() Stage {
func(ctx context.Context, state *State) (*State, error) {

// Predicate to isolate both resources and complex objects
isPropertyContainer := func(def astmodel.TypeDefinition) bool {
_, ok := astmodel.AsPropertyContainer(def.Type())
return ok
isResourceOrObject := func(def astmodel.TypeDefinition) bool {
_, isResource := astmodel.AsResourceType(def.Type())
_, isObject := astmodel.AsObjectType(def.Type())
return isResource || isObject
}

// Predicate to filter out ARM types
Expand All @@ -37,29 +38,27 @@ func CreateStorageTypes() Stage {
}

// Filter to the types we want to process
typesToConvert := state.Types().Where(isPropertyContainer).Where(isNotARMType)
typesToConvert := state.Types().Where(isResourceOrObject).Where(isNotARMType)

storageTypes := make(astmodel.Types)
typeConverter := storage.NewTypeConverter(state.Types())
typeConverter := storage.NewTypeConverter(state.Types(), state.ConversionGraph())

// Create storage variants
conversionGraph := storage.NewConversionGraph()
for name, def := range typesToConvert {
storageDef, err := typeConverter.ConvertDefinition(def)
if err != nil {
return nil, errors.Wrapf(err, "creating storage variant of %q", name)
}

storageTypes.Add(storageDef)
conversionGraph.AddLink(name.PackageReference, storageDef.Name().PackageReference)
}

types := state.Types().Copy()
types.AddTypes(storageTypes)

return state.WithTypes(types).WithConversionGraph(conversionGraph), nil
return state.WithTypes(types), nil
})

result.RequiresPrerequisiteStages(injectOriginalVersionFunctionStageId)
result.RequiresPrerequisiteStages(InjectOriginalVersionFunctionStageId, CreateConversionGraphStageId)
return result
}
38 changes: 20 additions & 18 deletions hack/generator/pkg/codegen/pipeline/create_storage_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,33 +20,35 @@ func TestCreateStorageTypes(t *testing.T) {

// Test Resource V1

specV1 := test.CreateSpec(pkg2020, "Person", fullNameProperty, familyNameProperty, knownAsProperty)
statusV1 := test.CreateStatus(pkg2020, "Person")
resourceV1 := test.CreateResource(pkg2020, "Person", specV1, statusV1)
specV1 := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.FamilyNameProperty, test.KnownAsProperty)
statusV1 := test.CreateStatus(test.Pkg2020, "Person")
resourceV1 := test.CreateResource(test.Pkg2020, "Person", specV1, statusV1)

// Test Resource V2

specV2 := test.CreateSpec(
pkg2021,
test.Pkg2021,
"Person",
fullNameProperty,
familyNameProperty,
knownAsProperty,
residentialAddress2021,
postalAddress2021)
statusV2 := test.CreateStatus(pkg2021, "Person")
resourceV2 := test.CreateResource(pkg2021, "Person", specV2, statusV2)
test.FullNameProperty,
test.FamilyNameProperty,
test.KnownAsProperty,
test.ResidentialAddress2021,
test.PostalAddress2021)
statusV2 := test.CreateStatus(test.Pkg2021, "Person")
resourceV2 := test.CreateResource(test.Pkg2021, "Person", specV2, statusV2)

types := make(astmodel.Types)
types.AddAll(resourceV1, specV1, statusV1, resourceV2, specV2, statusV2, address2021)
types.AddAll(resourceV1, specV1, statusV1, resourceV2, specV2, statusV2, test.Address2021)
state := NewState().WithTypes(types)

// Run the stage
createStorageTypes := CreateStorageTypes()

// Don't need a context when testing
state := NewState()
finalState, err := createStorageTypes.Run(context.TODO(), state)
// Use CreateConversionGraph to create the conversion graph needed prior to creating storage types
createConversionGraphStage := CreateConversionGraph()
initialState, err := createConversionGraphStage.Run(context.TODO(), state)
g.Expect(err).To(Succeed())

// Now create storage types
createStorageTypesStage := CreateStorageTypes()
finalState, err := createStorageTypesStage.Run(context.TODO(), initialState)
g.Expect(err).To(Succeed())

test.AssertPackagesGenerateExpectedCode(t, finalState.Types(), t.Name())
Expand Down
12 changes: 6 additions & 6 deletions hack/generator/pkg/codegen/pipeline/inject_hub_function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ func TestInjectHubFunction_WhenResourceIsStorageVersion_GeneratesExpectedFile(t
idFactory := astmodel.NewIdentifierFactory()

// Define a test resource
spec := test.CreateSpec(pkg2020, "Person", fullNameProperty, familyNameProperty, knownAsProperty)
status := test.CreateStatus(pkg2020, "Person")
resource := test.CreateResource(pkg2020, "Person", spec, status)
spec := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.FamilyNameProperty, test.KnownAsProperty)
status := test.CreateStatus(test.Pkg2020, "Person")
resource := test.CreateResource(test.Pkg2020, "Person", spec, status)

resource = resource.WithType(
resource.Type().(*astmodel.ResourceType).MarkAsStorageVersion())
Expand All @@ -48,9 +48,9 @@ func TestInjectHubFunction_WhenResourceIsNotStorageVersion_GeneratesExpectedFile
idFactory := astmodel.NewIdentifierFactory()

// Define a test resource
spec := test.CreateSpec(pkg2020, "Person", fullNameProperty, familyNameProperty, knownAsProperty)
status := test.CreateStatus(pkg2020, "Person")
resource := test.CreateResource(pkg2020, "Person", spec, status)
spec := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.FamilyNameProperty, test.KnownAsProperty)
status := test.CreateStatus(test.Pkg2020, "Person")
resource := test.CreateResource(test.Pkg2020, "Person", spec, status)

types := make(astmodel.Types)
types.AddAll(resource, status, spec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@ func InjectOriginalGVKFunction(idFactory astmodel.IdentifierFactory) Stage {
return result, nil
})

stage.RequiresPrerequisiteStages(injectOriginalVersionFunctionStageId)
stage.RequiresPrerequisiteStages(InjectOriginalVersionFunctionStageId)
return stage
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ func TestInjectOriginalGVKFunction(t *testing.T) {
idFactory := astmodel.NewIdentifierFactory()

// Define a test resource
spec := test.CreateSpec(pkg2020, "Person", fullNameProperty, familyNameProperty, knownAsProperty)
status := test.CreateStatus(pkg2020, "Person")
resource := test.CreateResource(pkg2020, "Person", spec, status)
spec := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.FamilyNameProperty, test.KnownAsProperty)
status := test.CreateStatus(test.Pkg2020, "Person")
resource := test.CreateResource(test.Pkg2020, "Person", spec, status)

types := make(astmodel.Types)
types.AddAll(resource, status, spec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
"github.com/Azure/azure-service-operator/hack/generator/pkg/functions"
)

// injectOriginalVersionFunctionStageId is the unique identifier for this pipeline stage
const injectOriginalVersionFunctionStageId = "injectOriginalVersionFunction"
// InjectOriginalVersionFunctionStageId is the unique identifier for this pipeline stage
const InjectOriginalVersionFunctionStageId = "injectOriginalVersionFunction"

// InjectOriginalVersionFunction injects the function OriginalVersion() into each Spec type
// This function allows us to recover the original version used to create each custom resource, giving the operator the
Expand All @@ -25,7 +25,7 @@ const injectOriginalVersionFunctionStageId = "injectOriginalVersionFunction"
func InjectOriginalVersionFunction(idFactory astmodel.IdentifierFactory) Stage {

stage := MakeLegacyStage(
injectOriginalVersionFunctionStageId,
InjectOriginalVersionFunctionStageId,
"Inject the function OriginalVersion() into each Spec type",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
injector := storage.NewFunctionInjector()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ func TestInjectOriginalVersionFunction(t *testing.T) {
idFactory := astmodel.NewIdentifierFactory()

// Define a test resource
spec := test.CreateSpec(pkg2020, "Person", fullNameProperty, familyNameProperty, knownAsProperty)
status := test.CreateStatus(pkg2020, "Person")
resource := test.CreateResource(pkg2020, "Person", spec, status)
spec := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.FamilyNameProperty, test.KnownAsProperty)
status := test.CreateStatus(test.Pkg2020, "Person")
resource := test.CreateResource(test.Pkg2020, "Person", spec, status)

types := make(astmodel.Types)
types.AddAll(resource, status, spec)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ func InjectOriginalVersionProperty() Stage {
return result, nil
})

stage.RequiresPrerequisiteStages(injectOriginalVersionFunctionStageId)
stage.RequiresPrerequisiteStages(InjectOriginalVersionFunctionStageId)
return stage
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ func TestInjectOriginalVersionProperty_InjectsIntoSpec(t *testing.T) {
g := NewGomegaWithT(t)

// Define a test resource
spec := test.CreateSpec(pkg2020, "Person", fullNameProperty, familyNameProperty, knownAsProperty)
status := test.CreateStatus(pkg2020, "Person")
resource := test.CreateResource(pkg2020, "Person", spec, status)
spec := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.FamilyNameProperty, test.KnownAsProperty)
status := test.CreateStatus(test.Pkg2020, "Person")
resource := test.CreateResource(test.Pkg2020, "Person", spec, status)

types := make(astmodel.Types)
types.AddAll(resource, status, spec)
Expand All @@ -46,9 +46,9 @@ func TestInjectOriginalVersionProperty_WhenOriginalVersionFunctionFound_DoesNotI
fnInjector := storage.NewFunctionInjector()

// Define a test resource
spec := test.CreateSpec(pkg2020, "Person", fullNameProperty, familyNameProperty, knownAsProperty)
status := test.CreateStatus(pkg2020, "Person")
resource := test.CreateResource(pkg2020, "Person", spec, status)
spec := test.CreateSpec(test.Pkg2020, "Person", test.FullNameProperty, test.FamilyNameProperty, test.KnownAsProperty)
status := test.CreateStatus(test.Pkg2020, "Person")
resource := test.CreateResource(test.Pkg2020, "Person", spec, status)

spec, err := fnInjector.Inject(spec, functions.NewOriginalVersionFunction(idFactory))
g.Expect(err).To(Succeed())
Expand Down
Loading