Add config.Resource.crd{Storage,Hub}Version to be able to configure

the storage & hub API versions independently. - The default values for both of the storage & hub versions is the version being generated, i.e., Resource.Version. - Replace pipeline.ConversionHubGenerator & pipeline.ConversionSpokeGenerator with a common pipeline.ConversionNodeGenerator implementation - Now the hub generator can also inspect the generated files to regenerate the hub versions according to the latest resource configuration and we have removed the assumption that the hub version is always the latest version generated. - Fix duplicated GKVs issue in zz_register.go. Signed-off-by: Alper Rifat Ulucinar <[email protected]>
crossplane · Apr 19, 2024 · 39b1286 · 39b1286
1 parent 017f4f2
commit 39b1286
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 111 deletions.
diff --git a/pkg/config/common.go b/pkg/config/common.go
@@ -91,7 +91,6 @@ func DefaultResource(name string, terraformSchema *schema.Resource, terraformPlu
 		UseAsync:                         true,
 		SchemaElementOptions:             make(SchemaElementOptions),
 		ServerSideApplyMergeStrategies:   make(ServerSideApplyMergeStrategies),
-		MarkStorageVersion:               true,
 		listConversionPaths:              make(map[string]string),
 	}
 	for _, f := range opts {

diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go
@@ -45,7 +45,6 @@ func TestDefaultResource(t *testing.T) {
 				UseAsync:                       true,
 				SchemaElementOptions:           SchemaElementOptions{},
 				ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
-				MarkStorageVersion:             true,
 			},
 		},
 		"TwoSectionsName": {
@@ -64,7 +63,6 @@ func TestDefaultResource(t *testing.T) {
 				UseAsync:                       true,
 				SchemaElementOptions:           SchemaElementOptions{},
 				ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
-				MarkStorageVersion:             true,
 			},
 		},
 		"NameWithPrefixAcronym": {
@@ -83,7 +81,6 @@ func TestDefaultResource(t *testing.T) {
 				UseAsync:                       true,
 				SchemaElementOptions:           SchemaElementOptions{},
 				ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
-				MarkStorageVersion:             true,
 			},
 		},
 		"NameWithSuffixAcronym": {
@@ -102,7 +99,6 @@ func TestDefaultResource(t *testing.T) {
 				UseAsync:                       true,
 				SchemaElementOptions:           SchemaElementOptions{},
 				ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
-				MarkStorageVersion:             true,
 			},
 		},
 		"NameWithMultipleAcronyms": {
@@ -121,7 +117,6 @@ func TestDefaultResource(t *testing.T) {
 				UseAsync:                       true,
 				SchemaElementOptions:           SchemaElementOptions{},
 				ServerSideApplyMergeStrategies: ServerSideApplyMergeStrategies{},
-				MarkStorageVersion:             true,
 			},
 		},
 	}
@@ -135,6 +130,8 @@ func TestDefaultResource(t *testing.T) {
 		cmpopts.IgnoreFields(Resource{}, "useTerraformPluginFrameworkClient"),
 		cmpopts.IgnoreFields(Resource{}, "requiredFields"),
 		cmpopts.IgnoreFields(Resource{}, "listConversionPaths"),
+		cmpopts.IgnoreFields(Resource{}, "crdStorageVersion"),
+		cmpopts.IgnoreFields(Resource{}, "crdHubVersion"),
 	}
 
 	for name, tc := range cases {

diff --git a/pkg/config/resource.go b/pkg/config/resource.go
@@ -436,10 +436,19 @@ type Resource struct {
 	// SchemaElementOption for configuring options for schema elements.
 	SchemaElementOptions SchemaElementOptions
 
-	// MarkStorageVersion sets the generated CRD API version as
-	// the CRD storage version. The default value is true, i.e., the generated
-	// version is by default the storage version.
-	MarkStorageVersion bool
+	// crdStorageVersion is the CRD storage API version.
+	// Use Resource.CRDStorageVersion to read the configured storage version
+	// which implements a defaulting to the current version being generated
+	// for backwards compatibility. This field is not exported to enforce
+	// defaulting, which is needed for backwards-compatibility.
+	crdStorageVersion string
+
+	// crdHubVersion is the conversion hub API version for the generated CRD.
+	// Use Resource.CRDHubVersion to read the configured hub version
+	// which implements a defaulting to the current version being generated
+	// for backwards compatibility. This field is not exported to enforce
+	// the defaulting behavior, which is needed for backwards-compatibility.
+	crdHubVersion string
 
 	// listConversionPaths maps the Terraform field paths of embedded objects
 	// that need to be converted into singleton lists (lists of
@@ -576,6 +585,39 @@ func (r *Resource) CRDListConversionPaths() []string {
 	return l
 }
 
+// CRDStorageVersion returns the CRD storage version if configured. If not,
+// returns the Version being generated as the default value.
+func (r *Resource) CRDStorageVersion() string {
+	if r.crdStorageVersion != "" {
+		return r.crdStorageVersion
+	}
+	return r.Version
+}
+
+// SetCRDStorageVersion configures the CRD storage version for a Resource.
+// If unset, the default storage version is the current Version
+// being generated.
+func (r *Resource) SetCRDStorageVersion(v string) {
+	r.crdStorageVersion = v
+}
+
+// CRDHubVersion returns the CRD hub version if configured. If not,
+// returns the Version being generated as the default value.
+func (r *Resource) CRDHubVersion() string {
+	if r.crdHubVersion != "" {
+		return r.crdHubVersion
+	}
+	return r.Version
+}
+
+// SetCRDHubVersion configures the CRD API conversion hub version
+// for a Resource.
+// If unset, the default hub version is the current Version
+// being generated.
+func (r *Resource) SetCRDHubVersion(v string) {
+	r.crdHubVersion = v
+}
+
 // AddSingletonListConversion configures the list at the specified Terraform
 // field path and the specified CRD field path as an embedded object.
 // crdPath is the field path expression for the CRD schema and tfPath is

diff --git a/pkg/pipeline/conversion_hub.go b/pkg/pipeline/conversion_hub.go
diff --git a/pkg/pipeline/conversion_spoke.go → pkg/pipeline/conversion_node.go b/pkg/pipeline/conversion_spoke.go → pkg/pipeline/conversion_node.go
@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: 2023 The Crossplane Authors <https://crossplane.io>
+// SPDX-FileCopyrightText: 2024 The Crossplane Authors <https://crossplane.io>
 //
 // SPDX-License-Identifier: Apache-2.0
 
@@ -14,60 +14,64 @@ import (
 
 	"github.com/muvaf/typewriter/pkg/wrapper"
 	"github.com/pkg/errors"
-
-	"github.com/crossplane/upjet/pkg/pipeline/templates"
 )
 
 var (
 	regexTypeFile = regexp.MustCompile(`zz_(.+)_types.go`)
 )
 
-// NewConversionSpokeGenerator returns a new ConversionSpokeGenerator.
-func NewConversionSpokeGenerator(pkg *types.Package, rootDir, group, version string) *ConversionSpokeGenerator {
-	return &ConversionSpokeGenerator{
-		LocalDirectoryPath: filepath.Join(rootDir, "apis", strings.ToLower(strings.Split(group, ".")[0])),
-		LicenseHeaderPath:  filepath.Join(rootDir, "hack", "boilerplate.go.txt"),
-		SpokeVersionsMap:   make(map[string][]string),
+// generationPredicate controls whether a resource configuration will be marked
+// as a hub or spoke based on the API version of the resource file
+// being considered.
+type generationPredicate func(c *terraformedInput, fileAPIVersion string) bool
+
+// NewConversionNodeGenerator returns a new ConversionNodeGenerator.
+func NewConversionNodeGenerator(pkg *types.Package, rootDir, group, generatedFileName, fileTemplate string, p generationPredicate) *ConversionNodeGenerator {
+	return &ConversionNodeGenerator{
+		localDirectoryPath: filepath.Join(rootDir, "apis", strings.ToLower(strings.Split(group, ".")[0])),
+		licenseHeaderPath:  filepath.Join(rootDir, "hack", "boilerplate.go.txt"),
+		nodeVersionsMap:    make(map[string][]string),
 		pkg:                pkg,
-		version:            version,
+		generatedFileName:  generatedFileName,
+		fileTemplate:       fileTemplate,
+		predicate:          p,
 	}
 }
 
-// ConversionSpokeGenerator generates conversion methods implementing the
+// ConversionNodeGenerator generates conversion methods implementing the
 // conversion.Convertible interface on the CRD structs.
-type ConversionSpokeGenerator struct {
-	LocalDirectoryPath string
-	LicenseHeaderPath  string
-	SpokeVersionsMap   map[string][]string
-
-	pkg     *types.Package
-	version string
+type ConversionNodeGenerator struct {
+	localDirectoryPath string
+	licenseHeaderPath  string
+	nodeVersionsMap    map[string][]string
+	pkg                *types.Package
+	generatedFileName  string
+	fileTemplate       string
+	predicate          generationPredicate
 }
 
 // Generate writes generated conversion.Convertible interface functions
-func (cg *ConversionSpokeGenerator) Generate(cfgs []*terraformedInput) error { //nolint:gocyclo
-	entries, err := os.ReadDir(cg.LocalDirectoryPath)
+func (cg *ConversionNodeGenerator) Generate(cfgs []*terraformedInput) error { //nolint:gocyclo
+	entries, err := os.ReadDir(cg.localDirectoryPath)
 	if err != nil {
-		return errors.Wrapf(err, "cannot list the directory entries for the source folder %s while generating the conversion.Convertible interface functions", cg.LocalDirectoryPath)
+		return errors.Wrapf(err, "cannot list the directory entries for the source folder %s while generating the conversion.Convertible interface functions", cg.localDirectoryPath)
 	}
 
 	for _, e := range entries {
-		if !e.IsDir() || e.Name() == cg.version {
-			// we skip spoke generation for the current version as the assumption is
-			// the current CRD version is the hub version.
+		if !e.IsDir() {
 			continue
 		}
-		trFile := wrapper.NewFile(cg.pkg.Path(), cg.pkg.Name(), templates.ConversionSpokeTemplate,
+		trFile := wrapper.NewFile(cg.pkg.Path(), cg.pkg.Name(), cg.fileTemplate,
 			wrapper.WithGenStatement(GenStatement),
-			wrapper.WithHeaderPath(cg.LicenseHeaderPath),
+			wrapper.WithHeaderPath(cg.licenseHeaderPath),
 		)
-		filePath := filepath.Join(cg.LocalDirectoryPath, e.Name(), "zz_generated.conversion_spokes.go")
+		filePath := filepath.Join(cg.localDirectoryPath, e.Name(), cg.generatedFileName)
 		vars := map[string]any{
 			"APIVersion": e.Name(),
 		}
 
 		var resources []map[string]any
-		versionDir := filepath.Join(cg.LocalDirectoryPath, e.Name())
+		versionDir := filepath.Join(cg.localDirectoryPath, e.Name())
 		files, err := os.ReadDir(versionDir)
 		if err != nil {
 			return errors.Wrapf(err, "cannot list the directory entries for the source folder %s while looking for the generated types", versionDir)
@@ -86,13 +90,17 @@ func (cg *ConversionSpokeGenerator) Generate(cfgs []*terraformedInput) error { /
 				// no conversion is possible.
 				continue
 			}
+			// skip resource configurations that do not match the predicate
+			if !cg.predicate(c, e.Name()) {
+				continue
+			}
 			resources = append(resources, map[string]any{
 				"CRD": map[string]string{
 					"Kind": c.Kind,
 				},
 			})
 			sk := fmt.Sprintf("%s.%s", c.ShortGroup, c.Kind)
-			cg.SpokeVersionsMap[sk] = append(cg.SpokeVersionsMap[sk], filepath.Base(versionDir))
+			cg.nodeVersionsMap[sk] = append(cg.nodeVersionsMap[sk], filepath.Base(versionDir))
 		}
 
 		vars["Resources"] = resources

diff --git a/pkg/pipeline/crd.go b/pkg/pipeline/crd.go
@@ -89,7 +89,7 @@ func (cg *CRDGenerator) Generate(cfg *config.Resource) (string, error) {
 			"APIVersion":         cfg.Version,
 			"Group":              cg.Group,
 			"Kind":               cfg.Kind,
-			"MarkStorageVersion": strconv.FormatBool(cfg.MarkStorageVersion),
+			"MarkStorageVersion": strconv.FormatBool(cfg.CRDStorageVersion() == cfg.Version),
 			"ForProviderType":    gen.ForProviderType.Obj().Name(),
 			"InitProviderType":   gen.InitProviderType.Obj().Name(),
 			"AtProviderType":     gen.AtProviderType.Obj().Name(),

diff --git a/pkg/pipeline/register.go b/pkg/pipeline/register.go
@@ -7,7 +7,7 @@ package pipeline
 import (
 	"os"
 	"path/filepath"
-	"sort"
+	"slices"
 
 	"github.com/muvaf/typewriter/pkg/wrapper"
 	"github.com/pkg/errors"
@@ -38,7 +38,8 @@ func (rg *RegisterGenerator) Generate(versionPkgList []string) error {
 		wrapper.WithGenStatement(GenStatement),
 		wrapper.WithHeaderPath(rg.LicenseHeaderPath),
 	)
-	sort.Strings(versionPkgList)
+	slices.Sort(versionPkgList)
+	versionPkgList = slices.Compact(versionPkgList)
 	aliases := make([]string, len(versionPkgList))
 	for i, pkgPath := range versionPkgList {
 		aliases[i] = registerFile.Imports.UsePackage(pkgPath)

diff --git a/pkg/pipeline/run.go b/pkg/pipeline/run.go
@@ -15,6 +15,7 @@ import (
 
 	"github.com/crossplane/upjet/pkg/config"
 	"github.com/crossplane/upjet/pkg/examples"
+	"github.com/crossplane/upjet/pkg/pipeline/templates"
 )
 
 type terraformedInput struct {
@@ -94,8 +95,16 @@ func Run(pc *config.Provider, rootDir string) { //nolint:gocyclo
 			versionGen := NewVersionGenerator(rootDir, pc.ModulePath, group, version)
 			crdGen := NewCRDGenerator(versionGen.Package(), rootDir, pc.ShortName, group, version)
 			tfGen := NewTerraformedGenerator(versionGen.Package(), rootDir, group, version)
-			conversionHubGen := NewConversionHubGenerator(versionGen.Package(), rootDir, group, version)
-			conversionSpokeGen := NewConversionSpokeGenerator(versionGen.Package(), rootDir, group, version)
+			conversionHubGen := NewConversionNodeGenerator(versionGen.Package(), rootDir, group, "zz_generated.conversion_hubs.go", templates.ConversionHubTemplate,
+				func(c *terraformedInput, fileAPIVersion string) bool {
+					// if this is the hub version, then mark it as a hub
+					return c.CRDHubVersion() == fileAPIVersion
+				})
+			conversionSpokeGen := NewConversionNodeGenerator(versionGen.Package(), rootDir, group, "zz_generated.conversion_spokes.go", templates.ConversionSpokeTemplate,
+				func(c *terraformedInput, fileAPIVersion string) bool {
+					// if not the hub version, mark it as a spoke
+					return c.CRDHubVersion() != fileAPIVersion
+				})
 			ctrlGen := NewControllerGenerator(rootDir, pc.ModulePath, group)
 
 			for _, name := range sortedResources(resources) {
@@ -129,7 +138,7 @@ func Run(pc *config.Provider, rootDir string) { //nolint:gocyclo
 				panic(errors.Wrapf(err, "cannot generate terraformed for resource %s", group))
 			}
 
-			if err := conversionHubGen.Generate(tfResources, version); err != nil {
+			if err := conversionHubGen.Generate(tfResources); err != nil {
 				panic(errors.Wrapf(err, "cannot generate the conversion.Hub function for the resource group %q", group))
 			}
 
@@ -144,12 +153,20 @@ func Run(pc *config.Provider, rootDir string) { //nolint:gocyclo
 			apiVersionPkgList = append(apiVersionPkgList, p)
 			for _, r := range resources {
 				// if there are spoke versions for the given group.Kind
-				if spokeVersions := conversionSpokeGen.SpokeVersionsMap[fmt.Sprintf("%s.%s", r.ShortGroup, r.Kind)]; spokeVersions != nil {
+				if spokeVersions := conversionSpokeGen.nodeVersionsMap[fmt.Sprintf("%s.%s", r.ShortGroup, r.Kind)]; spokeVersions != nil {
 					base := filepath.Dir(p)
 					for _, sv := range spokeVersions {
 						apiVersionPkgList = append(apiVersionPkgList, filepath.Join(base, sv))
 					}
 				}
+
+				// if there are hub versions for the given group.Kind
+				if hubVersions := conversionHubGen.nodeVersionsMap[fmt.Sprintf("%s.%s", r.ShortGroup, r.Kind)]; hubVersions != nil {
+					base := filepath.Dir(p)
+					for _, sv := range hubVersions {
+						apiVersionPkgList = append(apiVersionPkgList, filepath.Join(base, sv))
+					}
+				}
 			}
 		}
 	}