From 2c0f108fed33f5c6f93f51290397b47a29818354 Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Wed, 10 Apr 2024 15:09:39 +0200 Subject: [PATCH 1/2] crd: allow specifying spec.preserveUnknownFields Reinstate crd:preserveUnknownFields option removed by #607 to allow specifying the value of deprecated spec.preserveUnknownFields CRD field. This is useful for updating CRDs that were automatically converted from v1beta1 version which had true as a default value and resulted in: ``` $ kubectl get crd foo.bar -o yaml | grep preserveUnknownFields preserveUnknownFields: true message: 'spec.preserveUnknownFields: Invalid value: true: must be false' ``` For #476 --- pkg/crd/gen.go | 27 +++++++++++++++++++++++- pkg/crd/gen_integration_test.go | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/pkg/crd/gen.go b/pkg/crd/gen.go index 9eb4126bc..6c317f257 100644 --- a/pkg/crd/gen.go +++ b/pkg/crd/gen.go @@ -85,6 +85,13 @@ type Generator struct { // Year specifies the year to substitute for " YEAR" in the header file. Year string `marker:",optional"` + + // PreserveUnknownFields indicates whether or not we should turn off pruning. + // + // Specifies spec.preserveUnknownFields value that is false and omitted by default. + // + // It's required to be false for v1 CRDs. + PreserveUnknownFields *bool `marker:",optional"` } func (Generator) CheckFilter() loader.NodeFilter { @@ -100,6 +107,16 @@ func transformRemoveCRDStatus(obj map[string]interface{}) error { return nil } +// transformPreserveUnknownFields adds spec.preserveUnknownFields=value. +func transformPreserveUnknownFields(value bool) func(map[string]interface{}) error { + return func(obj map[string]interface{}) error { + if spec, ok := obj["spec"].(map[interface{}]interface{}); ok { + spec["preserveUnknownFields"] = value + } + return nil + } +} + func (g Generator) Generate(ctx *genall.GenerationContext) error { parser := &Parser{ Collector: ctx.Collector, @@ -146,6 +163,14 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { } headerText = strings.ReplaceAll(headerText, " YEAR", " "+g.Year) + yamlOpts := []*genall.WriteYAMLOptions{ + genall.WithTransform(transformRemoveCRDStatus), + genall.WithTransform(genall.TransformRemoveCreationTimestamp), + } + if g.PreserveUnknownFields != nil { + yamlOpts = append(yamlOpts, genall.WithTransform(transformPreserveUnknownFields(*g.PreserveUnknownFields))) + } + for _, groupKind := range kubeKinds { parser.NeedCRDFor(groupKind, g.MaxDescLen) crdRaw := parser.CustomResourceDefinitions[groupKind] @@ -171,7 +196,7 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { } else { fileName = fmt.Sprintf("%s_%s.%s.yaml", crdRaw.Spec.Group, crdRaw.Spec.Names.Plural, crdVersions[i]) } - if err := ctx.WriteYAML(fileName, headerText, []interface{}{crd}, genall.WithTransform(transformRemoveCRDStatus), genall.WithTransform(genall.TransformRemoveCreationTimestamp)); err != nil { + if err := ctx.WriteYAML(fileName, headerText, []interface{}{crd}, yamlOpts...); err != nil { return err } } diff --git a/pkg/crd/gen_integration_test.go b/pkg/crd/gen_integration_test.go index e5dc41640..925c53f89 100644 --- a/pkg/crd/gen_integration_test.go +++ b/pkg/crd/gen_integration_test.go @@ -119,6 +119,43 @@ var _ = Describe("CRD Generation proper defaulting", func() { expectedOut := string(expectedFileFoos) + string(expectedFileZoos) Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut)) }) + + It("should add preserveUnknownFields=false when specified", func() { + By("calling Generate") + no := false + gen := &crd.Generator{ + CRDVersions: []string{"v1"}, + PreserveUnknownFields: &no, + } + Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) + + By("searching preserveUnknownFields") + Expect(out.buf.String()).To(ContainSubstring("preserveUnknownFields: false")) + }) + + It("should add preserveUnknownFields=true when specified", func() { + By("calling Generate") + yes := true + gen := &crd.Generator{ + CRDVersions: []string{"v1"}, + PreserveUnknownFields: &yes, + } + Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) + + By("searching preserveUnknownFields") + Expect(out.buf.String()).To(ContainSubstring("preserveUnknownFields: true")) + }) + + It("should not add preserveUnknownFields when not specified", func() { + By("calling Generate") + gen := &crd.Generator{ + CRDVersions: []string{"v1"}, + } + Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) + + By("searching preserveUnknownFields") + Expect(out.buf.String()).NotTo(ContainSubstring("preserveUnknownFields")) + }) }) // fixAnnotations fixes the attribution annotation for tests. From ef487eb0f08be665c06a4d7a48563dac9fab06d3 Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Tue, 16 Apr 2024 10:27:18 +0200 Subject: [PATCH 2/2] crd: rename marker to DeprecatedV1beta1CompatibilityPreserveUnknownFields --- pkg/crd/gen.go | 17 ++++++++++++----- pkg/crd/gen_integration_test.go | 8 ++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/pkg/crd/gen.go b/pkg/crd/gen.go index 6c317f257..ac8eb566d 100644 --- a/pkg/crd/gen.go +++ b/pkg/crd/gen.go @@ -86,12 +86,19 @@ type Generator struct { // Year specifies the year to substitute for " YEAR" in the header file. Year string `marker:",optional"` - // PreserveUnknownFields indicates whether or not we should turn off pruning. + // DeprecatedV1beta1CompatibilityPreserveUnknownFields indicates whether + // or not we should turn off field pruning for this resource. // // Specifies spec.preserveUnknownFields value that is false and omitted by default. + // This value can only be specified for CustomResourceDefinitions that were created with + // `apiextensions.k8s.io/v1beta1`. // - // It's required to be false for v1 CRDs. - PreserveUnknownFields *bool `marker:",optional"` + // The field can be set for compatiblity reasons, although strongly discouraged, resource + // authors should move to a structural OpenAPI schema instead. + // + // See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#field-pruning + // for more information about field pruning and v1beta1 resources compatibility. + DeprecatedV1beta1CompatibilityPreserveUnknownFields *bool `marker:",optional"` } func (Generator) CheckFilter() loader.NodeFilter { @@ -167,8 +174,8 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { genall.WithTransform(transformRemoveCRDStatus), genall.WithTransform(genall.TransformRemoveCreationTimestamp), } - if g.PreserveUnknownFields != nil { - yamlOpts = append(yamlOpts, genall.WithTransform(transformPreserveUnknownFields(*g.PreserveUnknownFields))) + if g.DeprecatedV1beta1CompatibilityPreserveUnknownFields != nil { + yamlOpts = append(yamlOpts, genall.WithTransform(transformPreserveUnknownFields(*g.DeprecatedV1beta1CompatibilityPreserveUnknownFields))) } for _, groupKind := range kubeKinds { diff --git a/pkg/crd/gen_integration_test.go b/pkg/crd/gen_integration_test.go index 925c53f89..d59e3e923 100644 --- a/pkg/crd/gen_integration_test.go +++ b/pkg/crd/gen_integration_test.go @@ -124,8 +124,8 @@ var _ = Describe("CRD Generation proper defaulting", func() { By("calling Generate") no := false gen := &crd.Generator{ - CRDVersions: []string{"v1"}, - PreserveUnknownFields: &no, + CRDVersions: []string{"v1"}, + DeprecatedV1beta1CompatibilityPreserveUnknownFields: &no, } Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) @@ -137,8 +137,8 @@ var _ = Describe("CRD Generation proper defaulting", func() { By("calling Generate") yes := true gen := &crd.Generator{ - CRDVersions: []string{"v1"}, - PreserveUnknownFields: &yes, + CRDVersions: []string{"v1"}, + DeprecatedV1beta1CompatibilityPreserveUnknownFields: &yes, } Expect(gen.Generate(ctx)).NotTo(HaveOccurred())