Merge pull request #1218 from alvaroaleman/fix-gv

🐛 Applyconfiguration: Use correct groupVersion

Author: k8s-ci-robot

pkg/applyconfiguration/applyconfiguration_integration_test.go

@@ -118,37 +118,46 @@ var _ = Describe("ApplyConfiguration generation from API types", func() {
 
 		rt.OutputRules = genall.OutputRules{Default: output}
 
-		originalFS := os.DirFS(filepath.Join(originalCWD, cronjobDir))
-		tmpFS := os.DirFS(".")
+		fixturePath := filepath.Join(originalCWD, cronjobDir)
 
 		By("Running the generator")
 		hadErrs := rt.Run()
 
 		By("Checking for generation errors")
 		Expect(hadErrs).To(BeFalse(), "Generator should run without errors")
 
+		tmpFS := os.DirFS(".")
+		if os.Getenv("UPDATE") != "" && outputPackage == "applyconfiguration" {
+			Expect(os.RemoveAll(fixturePath)).NotTo(HaveOccurred())
+			Expect(os.CopyFS(fixturePath, tmpFS)).NotTo(HaveOccurred())
+		}
+
+		originalFS := os.DirFS(fixturePath)
+
 		filesInOriginal := make(map[string][]byte)
 		originalFileNames := sets.New[string]()
-		Expect(fs.WalkDir(originalFS, filepath.Join("api/v1", applyConfigurationDir), func(path string, d fs.DirEntry, err error) error {
-			if err != nil {
-				return err
-			}
-
-			if d.IsDir() {
+		Expect(fs.WalkDir(originalFS, filepath.Join("api/v1", applyConfigurationDir),
+			func(path string, d fs.DirEntry, err error) error {
+				if err != nil {
+					return err
+				}
+
+				if d.IsDir() {
+					return nil
+				}
+
+				data, err := os.ReadFile(filepath.Join(originalCWD, cronjobDir, path))
+				if err != nil {
+					return fmt.Errorf("error reading file %s: %w", path, err)
+				}
+
+				// Record the path without the path prefix for comparison later.
+				path = strings.TrimPrefix(path, filepath.Join("api/v1", applyConfigurationDir)+"/")
+				originalFileNames.Insert(path)
+				filesInOriginal[path] = data
 				return nil
-			}
-
-			data, err := os.ReadFile(filepath.Join(originalCWD, cronjobDir, path))
-			if err != nil {
-				return fmt.Errorf("error reading file %s: %w", path, err)
-			}
-
-			// Record the path without the path prefix for comparison later.
-			path = strings.TrimPrefix(path, filepath.Join("api/v1", applyConfigurationDir)+"/")
-			originalFileNames.Insert(path)
-			filesInOriginal[path] = data
-			return nil
-		})).To(Succeed())
+			},
+		)).To(Succeed())
 
 		filesInOutput := make(map[string][]byte)
 		outputFileNames := sets.New[string]()

pkg/applyconfiguration/gen.go

@@ -17,6 +17,7 @@ limitations under the License.
 package applyconfiguration
 
 import (
+	"errors"
 	"fmt"
 	"go/ast"
 	"os"
@@ -33,6 +34,7 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
 	"sigs.k8s.io/controller-tools/pkg/genall"
+	"sigs.k8s.io/controller-tools/pkg/internal/crd"
 	"sigs.k8s.io/controller-tools/pkg/loader"
 	"sigs.k8s.io/controller-tools/pkg/markers"
 )
@@ -205,6 +207,18 @@ func (ctx *ObjectGenCtx) generateForPackage(root *loader.Package) error {
 		return fmt.Errorf("package %q not found in universe", root.Name)
 	}
 
+	pkgMarkers, err := markers.PackageMarkers(ctx.Collector, root)
+	if err != nil {
+		return fmt.Errorf("failed to get package markers: %w", err)
+	}
+
+	gv := crd.GroupVersionForPackage(pkgMarkers, root)
+	if gv.Empty() {
+		return errors.New("could not infer groupVersion for package - Is the `// +groupName` marker set?")
+	}
+
+	pkg.Comments = append(pkg.Comments, "+groupName="+gv.Group)
+
 	// For each type we think should be generated, make sure it has a genclient
 	// marker else the apply generator will not generate it.
 	if err := markers.EachType(ctx.Collector, root, func(info *markers.TypeInfo) {

pkg/applyconfiguration/testdata/cronjob/api/v1/applyconfiguration/api/v1/cronjob.go

@@ -16,7 +16,7 @@ limitations under the License.
 // TODO(directxman12): test this across both versions (right now we're just
 // trusting k/k conversion, which is probably fine though)
 
-//go:generate ../../../.run-controller-gen.sh crd:ignoreUnexportedFields=true,allowDangerousTypes=true paths=./;./deprecated;./unserved;./job/... output:dir=.
+//go:generate ../../../../../../.run-controller-gen.sh crd:ignoreUnexportedFields=true,allowDangerousTypes=true paths=./;./deprecated;./unserved;./job/... output:dir=.
 
 // +groupName=testdata.kubebuilder.io
 // +versionName=v1

pkg/applyconfiguration/testdata/cronjob/api/v1/applyconfiguration/utils.go

@@ -21,6 +21,7 @@ import (
 
 	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-tools/pkg/internal/crd"
 	"sigs.k8s.io/controller-tools/pkg/loader"
 	"sigs.k8s.io/controller-tools/pkg/markers"
 )
@@ -132,17 +133,8 @@ func (p *Parser) indexTypes(pkg *loader.Package) {
 		if skipPkg := pkgMarkers.Get("kubebuilder:skip"); skipPkg != nil {
 			return
 		}
-		if nameVal := pkgMarkers.Get("groupName"); nameVal != nil {
-			versionVal := pkg.Name // a reasonable guess
-			if versionMarker := pkgMarkers.Get("versionName"); versionMarker != nil {
-				versionVal = versionMarker.(string)
-			}
-
-			p.GroupVersions[pkg] = schema.GroupVersion{
-				Version: versionVal,
-				Group:   nameVal.(string),
-			}
-		}
+
+		p.GroupVersions[pkg] = crd.GroupVersionForPackage(pkgMarkers, pkg)
 	}
 
 	if err := markers.EachType(p.Collector, pkg, func(info *markers.TypeInfo) {

pkg/applyconfiguration/testdata/cronjob/api/v1/cronjob_types.go

@@ -0,0 +1,23 @@
+package crd
+
+import (
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-tools/pkg/loader"
+	"sigs.k8s.io/controller-tools/pkg/markers"
+)
+
+func GroupVersionForPackage(pkgMarkers markers.MarkerValues, pkg *loader.Package) schema.GroupVersion {
+	if nameVal := pkgMarkers.Get("groupName"); nameVal != nil {
+		versionVal := pkg.Name // a reasonable guess
+		if versionMarker := pkgMarkers.Get("versionName"); versionMarker != nil {
+			versionVal = versionMarker.(string)
+		}
+
+		return schema.GroupVersion{
+			Version: versionVal,
+			Group:   nameVal.(string),
+		}
+	}
+
+	return schema.GroupVersion{}
+}