Call hub & spoke generation pipelines after the CRD generation pipeline

- Prevent execution of these pipelines multiple times for each available versions of an API group. - Improve conversion.RoundTrip paved conversion error message Signed-off-by: Alper Rifat Ulucinar <[email protected]>
crossplane · May 7, 2024 · b2b515f · b2b515f
1 parent b0089fb
commit b2b515f
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 70 deletions.
diff --git a/pkg/controller/conversion/functions.go b/pkg/controller/conversion/functions.go
@@ -48,7 +48,7 @@ func (r *registry) RoundTrip(dst, src resource.Terraformed) error { //nolint:goc
 	// convert the map[string]any representation of the conversion target back to
 	// the original type.
 	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(dstMap, dst); err != nil {
-		return errors.Wrap(err, "cannot convert the map[string]any representation of the conversion target object to the target object")
+		return errors.Wrap(err, "cannot convert the map[string]any representation of the conversion target back to the object itself")
 	}
 
 	// finally at the third stage, run the ManagedConverters

diff --git a/pkg/controller/external_tfpluginsdk.go b/pkg/controller/external_tfpluginsdk.go
@@ -258,7 +258,7 @@ func (c *TerraformPluginSDKConnector) Connect(ctx context.Context, mg xpresource
 		}
 		tfState, err = conversion.Convert(tfState, c.config.TFListConversionPaths(), conversion.ToSingletonList)
 		if err != nil {
-			return nil, err
+			return nil, errors.Wrap(err, "failed to run the API converters on the Terraform state")
 		}
 		copyParams := len(tfState) == 0
 		if err = resource.GetSensitiveParameters(ctx, &APISecretClient{kube: c.kube}, tr, tfState, tr.GetConnectionDetailsMapping()); err != nil {

diff --git a/pkg/pipeline/conversion_node.go b/pkg/pipeline/conversion_node.go
@@ -6,14 +6,15 @@ package pipeline
 
 import (
 	"fmt"
-	"go/types"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
 
 	"github.com/muvaf/typewriter/pkg/wrapper"
 	"github.com/pkg/errors"
+
+	"github.com/crossplane/upjet/pkg/config"
 )
 
 var (
@@ -23,59 +24,61 @@ var (
 // 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
+type generationPredicate func(c *config.Resource, fileAPIVersion string) bool
 
 // NewConversionNodeGenerator returns a new ConversionNodeGenerator.
-func NewConversionNodeGenerator(pkg *types.Package, rootDir, group, generatedFileName, fileTemplate string, p generationPredicate) *ConversionNodeGenerator {
+func NewConversionNodeGenerator(pcModulePath, rootDir, group, generatedFileName, fileTemplate string, p generationPredicate) *ConversionNodeGenerator {
+	shortGroup := strings.ToLower(strings.Split(group, ".")[0])
 	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,
-		generatedFileName:  generatedFileName,
-		fileTemplate:       fileTemplate,
-		predicate:          p,
+		apiGroupModule:    filepath.Join(pcModulePath, "apis", shortGroup),
+		apiGroupDir:       filepath.Join(rootDir, "apis", shortGroup),
+		licenseHeaderPath: filepath.Join(rootDir, "hack", "boilerplate.go.txt"),
+		nodeVersionsMap:   make(map[string][]string),
+		generatedFileName: generatedFileName,
+		fileTemplate:      fileTemplate,
+		predicate:         p,
 	}
 }
 
 // ConversionNodeGenerator generates conversion methods implementing the
 // conversion.Convertible interface on the CRD structs.
 type ConversionNodeGenerator struct {
-	localDirectoryPath string
-	licenseHeaderPath  string
-	nodeVersionsMap    map[string][]string
-	pkg                *types.Package
-	generatedFileName  string
-	fileTemplate       string
-	predicate          generationPredicate
+	apiGroupModule    string
+	apiGroupDir       string
+	licenseHeaderPath string
+	nodeVersionsMap   map[string][]string
+	generatedFileName string
+	fileTemplate      string
+	predicate         generationPredicate
 }
 
 // Generate writes generated conversion.Convertible interface functions
-func (cg *ConversionNodeGenerator) Generate(cfgs []*terraformedInput) error { //nolint:gocyclo
-	entries, err := os.ReadDir(cg.localDirectoryPath)
+func (cg *ConversionNodeGenerator) Generate(versionMap map[string]map[string]*config.Resource) error { //nolint:gocyclo
+	entries, err := os.ReadDir(cg.apiGroupDir)
 	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 functions", cg.apiGroupDir)
 	}
-
+	// iterate over the versions belonging to the API group
 	for _, e := range entries {
 		if !e.IsDir() {
 			continue
 		}
-		trFile := wrapper.NewFile(cg.pkg.Path(), cg.pkg.Name(), cg.fileTemplate,
+		version := e.Name()
+		convFile := wrapper.NewFile(filepath.Join(cg.apiGroupModule, version), version, cg.fileTemplate,
 			wrapper.WithGenStatement(GenStatement),
 			wrapper.WithHeaderPath(cg.licenseHeaderPath),
 		)
-		filePath := filepath.Join(cg.localDirectoryPath, e.Name(), cg.generatedFileName)
+		filePath := filepath.Join(cg.apiGroupDir, version, cg.generatedFileName)
 		vars := map[string]any{
-			"APIVersion": e.Name(),
+			"APIVersion": version,
 		}
 
-		var resources []map[string]any
-		versionDir := filepath.Join(cg.localDirectoryPath, e.Name())
+		versionDir := filepath.Join(cg.apiGroupDir, version)
 		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)
 		}
+		var resources []map[string]any
 		for _, f := range files {
 			if f.IsDir() {
 				continue
@@ -84,14 +87,14 @@ func (cg *ConversionNodeGenerator) Generate(cfgs []*terraformedInput) error { //
 			if len(m) < 2 {
 				continue
 			}
-			c := findKindTerraformedInput(cfgs, m[1])
+			c := findKindTerraformedInput(versionMap, m[1])
 			if c == nil {
 				// type may not be available in the new version =>
 				// no conversion is possible.
 				continue
 			}
 			// skip resource configurations that do not match the predicate
-			if !cg.predicate(c, e.Name()) {
+			if !cg.predicate(c, version) {
 				continue
 			}
 			resources = append(resources, map[string]any{
@@ -107,17 +110,19 @@ func (cg *ConversionNodeGenerator) Generate(cfgs []*terraformedInput) error { //
 		if len(resources) == 0 {
 			continue
 		}
-		if err := trFile.Write(filePath, vars, os.ModePerm); err != nil {
-			return errors.Wrapf(err, "cannot write the generated conversion Hub functions file %s", filePath)
+		if err := convFile.Write(filePath, vars, os.ModePerm); err != nil {
+			return errors.Wrapf(err, "cannot write the generated conversion functions file %s", filePath)
 		}
 	}
 	return nil
 }
 
-func findKindTerraformedInput(cfgs []*terraformedInput, name string) *terraformedInput {
-	for _, c := range cfgs {
-		if strings.EqualFold(name, c.Kind) {
-			return c
+func findKindTerraformedInput(versionMap map[string]map[string]*config.Resource, name string) *config.Resource {
+	for _, resources := range versionMap {
+		for _, r := range resources {
+			if strings.EqualFold(name, r.Kind) {
+				return r
+			}
 		}
 	}
 	return nil

diff --git a/pkg/pipeline/run.go b/pkg/pipeline/run.go
@@ -90,21 +90,12 @@ func Run(pc *config.Provider, rootDir string) { //nolint:gocyclo
 	}
 	count := 0
 	for group, versions := range resourcesGroups {
+		shortGroup := strings.Split(group, ".")[0]
 		for version, resources := range versions {
 			var tfResources []*terraformedInput
 			versionGen := NewVersionGenerator(rootDir, pc.ModulePath, group, version)
 			crdGen := NewCRDGenerator(versionGen.Package(), rootDir, pc.ShortName, group, version)
 			tfGen := NewTerraformedGenerator(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) {
@@ -125,8 +116,7 @@ func Run(pc *config.Provider, rootDir string) { //nolint:gocyclo
 				if err != nil {
 					panic(errors.Wrapf(err, "cannot generate controller for resource %s", name))
 				}
-				sGroup := strings.Split(group, ".")[0]
-				controllerPkgMap[sGroup] = append(controllerPkgMap[sGroup], ctrlPkgPath)
+				controllerPkgMap[shortGroup] = append(controllerPkgMap[shortGroup], ctrlPkgPath)
 				controllerPkgMap[config.PackageNameMonolith] = append(controllerPkgMap[config.PackageNameMonolith], ctrlPkgPath)
 				if err := exampleGen.Generate(group, version, resources[name]); err != nil {
 					panic(errors.Wrapf(err, "cannot generate example manifest for resource %s", name))
@@ -137,34 +127,44 @@ func Run(pc *config.Provider, rootDir string) { //nolint:gocyclo
 			if err := tfGen.Generate(tfResources, version); err != nil {
 				panic(errors.Wrapf(err, "cannot generate terraformed for resource %s", group))
 			}
-
-			if err := conversionHubGen.Generate(tfResources); err != nil {
-				panic(errors.Wrapf(err, "cannot generate the conversion.Hub function for the resource group %q", group))
-			}
-
-			if err := conversionSpokeGen.Generate(tfResources); err != nil {
-				panic(errors.Wrapf(err, "cannot generate the conversion.Convertible functions for the resource group %q", group))
-			}
-
 			if err := versionGen.Generate(); err != nil {
 				panic(errors.Wrap(err, "cannot generate version files"))
 			}
 			p := versionGen.Package().Path()
 			apiVersionPkgList = append(apiVersionPkgList, p)
-			for _, r := range resources {
-				// if there are spoke versions for the given group.Kind
-				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))
-					}
-				}
+		}
+		conversionHubGen := NewConversionNodeGenerator(pc.ModulePath, rootDir, group, "zz_generated.conversion_hubs.go", templates.ConversionHubTemplate,
+			func(c *config.Resource, fileAPIVersion string) bool {
+				// if this is the hub version, then mark it as a hub
+				return c.CRDHubVersion() == fileAPIVersion
+			})
+		if err := conversionHubGen.Generate(versions); err != nil {
+			panic(errors.Wrapf(err, "cannot generate the conversion.Hub function for the resource group %q", group))
+		}
+		conversionSpokeGen := NewConversionNodeGenerator(pc.ModulePath, rootDir, group, "zz_generated.conversion_spokes.go", templates.ConversionSpokeTemplate,
+			func(c *config.Resource, fileAPIVersion string) bool {
+				// if not the hub version, mark it as a spoke
+				return c.CRDHubVersion() != fileAPIVersion
+			})
+		if err := conversionSpokeGen.Generate(versions); err != nil {
+			panic(errors.Wrapf(err, "cannot generate the conversion.Convertible functions for the resource group %q", group))
+		}
 
-				// 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))
+		base := filepath.Join(pc.ModulePath, "apis", shortGroup)
+		for _, versions := range resourcesGroups {
+			for _, resources := range versions {
+				for _, r := range resources {
+					// if there are spoke versions for the given group.Kind
+					if spokeVersions := conversionSpokeGen.nodeVersionsMap[fmt.Sprintf("%s.%s", r.ShortGroup, r.Kind)]; spokeVersions != nil {
+						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 {
+						for _, sv := range hubVersions {
+							apiVersionPkgList = append(apiVersionPkgList, filepath.Join(base, sv))
+						}
 					}
 				}
 			}