From 7c1db6b37e2b89a7c83a29a833ad5b271442781c Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Thu, 11 Jul 2024 17:14:37 +0200 Subject: [PATCH] :sparkles: Make the anonymous field of the groupName marker optional Upstream introduced the use of empty groupName markers in k/k PR 125162. This breaks groupName parsing because its field is non-optional. Add an additional mustOptional function that takes anonymous field definitions and turns them into optional ones. Signed-off-by: Tom Wieczorek --- pkg/crd/markers/package.go | 2 +- pkg/crd/markers/register.go | 14 ++++++++ pkg/crd/parser_integration_test.go | 20 ++++++++++-- pkg/crd/testdata/nogroup/types.go | 29 +++++++++++++++++ pkg/crd/testdata/testdata._cronjobs.yaml | 41 ++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 pkg/crd/testdata/nogroup/types.go create mode 100644 pkg/crd/testdata/testdata._cronjobs.yaml diff --git a/pkg/crd/markers/package.go b/pkg/crd/markers/package.go index cebe8fa4b..86851b2c4 100644 --- a/pkg/crd/markers/package.go +++ b/pkg/crd/markers/package.go @@ -22,7 +22,7 @@ import ( func init() { AllDefinitions = append(AllDefinitions, - must(markers.MakeDefinition("groupName", markers.DescribesPackage, "")). + mustOptional(markers.MakeDefinition("groupName", markers.DescribesPackage, "")). WithHelp(markers.SimpleHelp("CRD", "specifies the API group name for this package.")), must(markers.MakeDefinition("versionName", markers.DescribesPackage, "")). diff --git a/pkg/crd/markers/register.go b/pkg/crd/markers/register.go index 5988a1b5b..b5a2760b0 100644 --- a/pkg/crd/markers/register.go +++ b/pkg/crd/markers/register.go @@ -17,6 +17,7 @@ limitations under the License. package markers import ( + "fmt" "reflect" "sigs.k8s.io/controller-tools/pkg/markers" @@ -56,6 +57,19 @@ func must(def *markers.Definition, err error) *definitionWithHelp { } } +func mustOptional(def *markers.Definition, err error) *definitionWithHelp { + def = markers.Must(def, err) + if !def.AnonymousField() { + def = markers.Must(def, fmt.Errorf("not an anonymous field: %v", def)) + } + field := def.Fields[""] + field.Optional = true + def.Fields[""] = field + return &definitionWithHelp{ + Definition: def, + } +} + // AllDefinitions contains all marker definitions for this package. var AllDefinitions []*definitionWithHelp diff --git a/pkg/crd/parser_integration_test.go b/pkg/crd/parser_integration_test.go index e6472e68d..7e020d07a 100644 --- a/pkg/crd/parser_integration_test.go +++ b/pkg/crd/parser_integration_test.go @@ -100,9 +100,9 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func } }) - assertCRD := func(pkg *loader.Package, kind, fileName string) { - By(fmt.Sprintf("requesting that the %s CRD be generated", kind)) - groupKind := schema.GroupKind{Kind: kind, Group: "testdata.kubebuilder.io"} + assertCRDForGroupKind := func(pkg *loader.Package, groupKind schema.GroupKind, fileName string) { + kind := groupKind.Kind + By(fmt.Sprintf("requesting that the %s CRD be generated in group %s", kind, groupKind.Group)) parser.NeedCRDFor(groupKind, nil) By(fmt.Sprintf("fixing top level ObjectMeta on the %s CRD", kind)) @@ -131,6 +131,10 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func ExpectWithOffset(1, parser.CustomResourceDefinitions[groupKind]).To(Equal(crd), "type not as expected, check pkg/crd/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(parser.CustomResourceDefinitions[groupKind], crd)) } + assertCRD := func(pkg *loader.Package, kind, fileName string) { + assertCRDForGroupKind(pkg, schema.GroupKind{Group: "testdata.kubebuilder.io", Kind: kind}, fileName) + } + assertError := func(pkg *loader.Package, kind, errorMsg string) { By(fmt.Sprintf("requesting that the %s CRD be generated", kind)) groupKind := schema.GroupKind{Kind: kind, Group: "testdata.kubebuilder.io"} @@ -183,6 +187,16 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func assertError(pkgs[0], "CronJob", "is not in 'xxx=xxx' format") }) }) + + Context("CronJob API without group", func() { + BeforeEach(func() { + pkgPaths = []string{"./nogroup"} + expPkgLen = 1 + }) + It("should successfully generate the CronJob CRD", func() { + assertCRDForGroupKind(pkgs[0], schema.GroupKind{Kind: "CronJob"}, "testdata._cronjobs.yaml") + }) + }) }) It("should generate plural words for Kind correctly", func() { diff --git a/pkg/crd/testdata/nogroup/types.go b/pkg/crd/testdata/nogroup/types.go new file mode 100644 index 000000000..0d6983c20 --- /dev/null +++ b/pkg/crd/testdata/nogroup/types.go @@ -0,0 +1,29 @@ +/* + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// +groupName= +package nogroup + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// +kubebuilder:object:root=true + +// CronJob is the Schema for the cronjobs API +type CronJob struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` +} diff --git a/pkg/crd/testdata/testdata._cronjobs.yaml b/pkg/crd/testdata/testdata._cronjobs.yaml new file mode 100644 index 000000000..218674c4a --- /dev/null +++ b/pkg/crd/testdata/testdata._cronjobs.yaml @@ -0,0 +1,41 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: (devel) + name: cronjobs. +spec: + group: "" + names: + kind: CronJob + listKind: CronJobList + plural: cronjobs + singular: cronjob + scope: Namespaced + versions: + - name: nogroup + schema: + openAPIV3Schema: + description: CronJob is the Schema for the cronjobs API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + type: object + served: true + storage: true