From dd213e64820759c8e0048efa8f3435bb450d7496 Mon Sep 17 00:00:00 2001 From: Camila Macedo Date: Sun, 10 Nov 2024 18:43:31 +0000 Subject: [PATCH] (kustomize/v2, go/v4): Add Hub and Spoke for conversion webhooks This PR introduces the initial implementation of the hub-and-spoke model for handling conversion webhooks. The goal is to streamline the conversion process by utilizing a central hub to speak a specific version of the same Group and Kind. - **Single Spoke Support (A to B, Same Kind and Group):** The system will allow only one spoke version for conversions (i.e., converting from Version A to Version B within the same kind and group). - **Future Expansion (List of GKV Spokes):** In the future, based on user feedback or demand, we can expand to support a list of **GKV spokes**, allowing for greater flexibility in conversions across different versions, kinds, and groups. - **Advanced Case Handling (Manual Steps):** For more advanced cases, users can proceed without specifying a spoke. In this scenario, the conversion process will still occur without the spoke, enabling users to continue. However, they will be required to complete specific advanced steps manually. Closes; https://github.com/kubernetes-sigs/kubebuilder/issues/2589 --- .../testdata/project/PROJECT | 4 +-- pkg/cli/alpha/internal/generate.go | 4 +-- pkg/model/resource/webhooks.go | 24 ++------------- pkg/model/resource/webhooks_test.go | 4 +-- pkg/plugins/golang/options.go | 2 +- .../scaffolds/internal/templates/api/spoke.go | 17 +++++------ pkg/plugins/golang/v4/scaffolds/webhook.go | 10 +------ pkg/plugins/golang/v4/webhook.go | 29 +++++++------------ test/e2e/v4/generate_test.go | 4 +-- testdata/project-v4-multigroup/PROJECT | 4 +-- testdata/project-v4-with-plugins/PROJECT | 4 +-- testdata/project-v4/PROJECT | 4 +-- 12 files changed, 32 insertions(+), 78 deletions(-) diff --git a/docs/book/src/multiversion-tutorial/testdata/project/PROJECT b/docs/book/src/multiversion-tutorial/testdata/project/PROJECT index 084ae740bff..cd969a46d87 100644 --- a/docs/book/src/multiversion-tutorial/testdata/project/PROJECT +++ b/docs/book/src/multiversion-tutorial/testdata/project/PROJECT @@ -20,9 +20,7 @@ resources: webhooks: conversion: true defaulting: true - spoke: - - v2 - - v2 + spoke: v2 validation: true webhookVersion: v1 - api: diff --git a/pkg/cli/alpha/internal/generate.go b/pkg/cli/alpha/internal/generate.go index 8ee434d109f..b9daddf2cf9 100644 --- a/pkg/cli/alpha/internal/generate.go +++ b/pkg/cli/alpha/internal/generate.go @@ -369,8 +369,8 @@ func getWebhookResourceFlags(resource resource.Resource) []string { } if resource.HasConversionWebhook() { args = append(args, "--conversion") - for _, spoke := range resource.Webhooks.Spoke { - args = append(args, "--spoke", spoke) + if resource.Webhooks.Spoke != "" { + args = append(args, "--spoke", resource.Webhooks.Spoke) } } return args diff --git a/pkg/model/resource/webhooks.go b/pkg/model/resource/webhooks.go index fd35c2372e3..5a2b766683b 100644 --- a/pkg/model/resource/webhooks.go +++ b/pkg/model/resource/webhooks.go @@ -34,7 +34,7 @@ type Webhooks struct { // Conversion specifies if a conversion webhook is associated to the resource. Conversion bool `json:"conversion,omitempty"` - Spoke []string `json:"spoke,omitempty"` + Spoke string `json:"spoke,omitempty"` } // Validate checks that the Webhooks is valid. @@ -80,7 +80,7 @@ func (webhooks *Webhooks) Update(other *Webhooks) error { webhooks.Conversion = webhooks.Conversion || other.Conversion // Update Spoke. - webhooks.Spoke = append(webhooks.Spoke, other.Spoke...) + webhooks.Spoke = other.Spoke return nil } @@ -89,23 +89,3 @@ func (webhooks *Webhooks) Update(other *Webhooks) error { func (webhooks Webhooks) IsEmpty() bool { return webhooks.WebhookVersion == "" && !webhooks.Defaulting && !webhooks.Validation && !webhooks.Conversion } - -// HasSpokeVersion returns true if the spoke version is present in the list of spoke versions. -func (webhooks Webhooks) HasSpokeVersion(version string) bool { - for _, v := range webhooks.Spoke { - if v == version { - return true - } - } - return false -} - -// HasAnySpokeVersionFrom returns true if any spoke versions are present in the list of spoke versions. -func (webhooks Webhooks) HasAnySpokeVersionFrom(values []string) bool { - for _, v := range values { - if webhooks.HasSpokeVersion(v) { - return true - } - } - return false -} diff --git a/pkg/model/resource/webhooks_test.go b/pkg/model/resource/webhooks_test.go index 1c136ba2419..757a395d1ca 100644 --- a/pkg/model/resource/webhooks_test.go +++ b/pkg/model/resource/webhooks_test.go @@ -52,14 +52,14 @@ var _ = Describe("Webhooks", func() { Defaulting: true, Validation: true, Conversion: true, - Spoke: []string{"v2"}, + Spoke: "v2", } Expect(webhook.Update(nil)).To(Succeed()) Expect(webhook.WebhookVersion).To(Equal(v1)) Expect(webhook.Defaulting).To(BeTrue()) Expect(webhook.Validation).To(BeTrue()) Expect(webhook.Conversion).To(BeTrue()) - Expect(webhook.Spoke).To(Equal([]string{"v2"})) + Expect(webhook.Spoke).To(Equal("v2")) }) Context("webhooks version", func() { diff --git a/pkg/plugins/golang/options.go b/pkg/plugins/golang/options.go index b7965a713d8..676712792e7 100644 --- a/pkg/plugins/golang/options.go +++ b/pkg/plugins/golang/options.go @@ -74,7 +74,7 @@ type Options struct { DoConversion bool // Spoke version for conversion webhook - Spoke []string + Spoke string } // UpdateResource updates the provided resource with the options diff --git a/pkg/plugins/golang/v4/scaffolds/internal/templates/api/spoke.go b/pkg/plugins/golang/v4/scaffolds/internal/templates/api/spoke.go index 076c44b1b32..1d790230dd8 100644 --- a/pkg/plugins/golang/v4/scaffolds/internal/templates/api/spoke.go +++ b/pkg/plugins/golang/v4/scaffolds/internal/templates/api/spoke.go @@ -32,17 +32,16 @@ type Spoke struct { machinery.BoilerplateMixin machinery.ResourceMixin - Force bool - SpokeVersion string + Force bool } // SetTemplateDefaults implements file.Template func (f *Spoke) SetTemplateDefaults() error { if f.Path == "" { if f.MultiGroup && f.Resource.Group != "" { - f.Path = filepath.Join("api", "%[group]", f.SpokeVersion, "%[kind]_conversion.go") + f.Path = filepath.Join("api", "%[group]", f.Resource.Webhooks.Spoke, "%[kind]_conversion.go") } else { - f.Path = filepath.Join("api", f.SpokeVersion, "%[kind]_conversion.go") + f.Path = filepath.Join("api", f.Resource.Webhooks.Spoke, "%[kind]_conversion.go") } } @@ -63,7 +62,7 @@ func (f *Spoke) SetTemplateDefaults() error { // nolint:lll const spokeTemplate = `{{ .Boilerplate }} -package {{ .SpokeVersion }} +package {{ .Resource.Webhooks.Spoke }} import ( "log" @@ -78,18 +77,18 @@ func (src *{{ .Resource.Kind }}) ConvertTo(dstRaw conversion.Hub) error { log.Printf("ConvertTo: converts this {{ .Resource.Kind }} to the Hub version ({{ .Resource.Version }});" + "source: %s/%s and target: %s/%s", src.Namespace, src.Name, dst.Namespace, dst.Name) - // TODO(user): Implement conversion logic from {{ .SpokeVersion }} to {{ .Resource.Version }} + // TODO(user): Implement conversion logic from {{ .Resource.Webhooks.Spoke }} to {{ .Resource.Version }} return nil } -// ConvertFrom converts the Hub version ({{ .Resource.Version }}) to this {{ .Resource.Kind }} ({{ .SpokeVersion}}). +// ConvertFrom converts the Hub version ({{ .Resource.Version }}) to this {{ .Resource.Kind }} ({{ .Resource.Webhooks.Spoke }}). func (dst *{{ .Resource.Kind }}) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}) - log.Printf("ConvertFrom: converts the Hub version ({{ .Resource.Version }}) to this {{ .Resource.Kind }} ({{ .SpokeVersion}});" + + log.Printf("ConvertFrom: converts the Hub version ({{ .Resource.Version }}) to this {{ .Resource.Kind }} ({{ .Resource.Webhooks.Spoke }});" + "source: %s/%s and target: %s/%s", src.Namespace, src.Name, dst.Namespace, dst.Name) - // TODO(user): Implement conversion logic from {{ .Resource.Version }} to {{ .SpokeVersion }} + // TODO(user): Implement conversion logic from {{ .Resource.Version }} to {{ .Resource.Webhooks.Spoke }} return nil } diff --git a/pkg/plugins/golang/v4/scaffolds/webhook.go b/pkg/plugins/golang/v4/scaffolds/webhook.go index 8861ae10774..21fbc194e24 100644 --- a/pkg/plugins/golang/v4/scaffolds/webhook.go +++ b/pkg/plugins/golang/v4/scaffolds/webhook.go @@ -124,18 +124,10 @@ func (s *webhookScaffolder) Scaffold() error { if err := scaffold.Execute( &api.Hub{Force: s.force}, + &api.Spoke{Force: s.force}, ); err != nil { return err } - - // Create the spoke version conversion file for each spoke version - for _, spokeVersion := range s.resource.Webhooks.Spoke { - if err := scaffold.Execute( - &api.Spoke{Force: s.force, SpokeVersion: spokeVersion}, - ); err != nil { - return err - } - } log.Println(`Webhook server has been set up for you. You need to implement the conversion.Hub and conversion.Convertible interfaces for your CRD types.`) } diff --git a/pkg/plugins/golang/v4/webhook.go b/pkg/plugins/golang/v4/webhook.go index a0996ad781d..fbf0a4134ac 100644 --- a/pkg/plugins/golang/v4/webhook.go +++ b/pkg/plugins/golang/v4/webhook.go @@ -66,7 +66,7 @@ validating and/or conversion webhooks. # Create conversion webhook for Group: ship, Version: v1beta1 # and Kind: Frigate - %[1]s create webhook --group ship --version v1beta1 --kind Frigate --conversion --spoke v1,v2,v3 + %[1]s create webhook --group ship --version v1beta1 --kind Frigate --conversion --spoke v1 `, cliMeta.CommandName) } @@ -84,9 +84,9 @@ func (p *createWebhookSubcommand) BindFlags(fs *pflag.FlagSet) { fs.BoolVar(&p.options.DoConversion, "conversion", false, "if set, scaffold the conversion webhook") - fs.StringSliceVar(&p.options.Spoke, "spoke", - []string{}, - "comma-separated list of spoke versions for conversion webhook (i.e. --spoke v1,v2,v3)") + fs.StringVar(&p.options.Spoke, "spoke", + "", + "if set, scaffold the spoke implementation (i.e. --spoke v1)") // TODO: remove for go/v5 fs.BoolVar(&p.isLegacyPath, "legacy", false, @@ -149,21 +149,12 @@ func (p *createWebhookSubcommand) InjectResource(res *resource.Resource) error { } if p.options.DoConversion { - if res.HasConversionWebhook() && - len(p.options.Spoke) > 0 && - res.Webhooks.HasAnySpokeVersionFrom(p.options.Spoke) { - return fmt.Errorf("conversion webhook already exists with one or more spoke versions informed") - } - - // Check if all spoke versions informed are api versions which exist in the resource - for _, spokeVersion := range p.options.Spoke { - spokeGKV := res.GVK - spokeGKV.Version = spokeVersion - _, err := p.config.GetResource(spokeGKV) - if err != nil { - return fmt.Errorf("resource does not have a version %s", - spokeVersion) - } + spokeGKV := res.GVK + spokeGKV.Version = p.options.Spoke + _, err := p.config.GetResource(spokeGKV) + if err != nil { + return fmt.Errorf("resource does not have a version %s", + p.options.Spoke) } } return nil diff --git a/test/e2e/v4/generate_test.go b/test/e2e/v4/generate_test.go index 78cf325d4f3..19a751ef511 100644 --- a/test/e2e/v4/generate_test.go +++ b/test/e2e/v4/generate_test.go @@ -421,12 +421,12 @@ func scaffoldConversionWebhook(kbc *utils.TestContext) { err = pluginutil.ReplaceInFile(filepath.Join(kbc.Dir, "api/v2/conversiontest_conversion.go"), "// TODO(user): Implement conversion logic from v1 to v2", - `dst.Spec.Size = src.Spec.Replicas`) + `src.Spec.Size = dst.Spec.Replicas`) Expect(err).NotTo(HaveOccurred(), "failed to implement conversion logic from v1 to v2") err = pluginutil.ReplaceInFile(filepath.Join(kbc.Dir, "api/v2/conversiontest_conversion.go"), "// TODO(user): Implement conversion logic from v2 to v1", - `dst.Spec.Replicas = src.Spec.Size`) + `src.Spec.Replicas = dst.Spec.Size`) Expect(err).NotTo(HaveOccurred(), "failed to implement conversion logic from v2 to v1") } diff --git a/testdata/project-v4-multigroup/PROJECT b/testdata/project-v4-multigroup/PROJECT index cf50c3a1515..aa1fdeec566 100644 --- a/testdata/project-v4-multigroup/PROJECT +++ b/testdata/project-v4-multigroup/PROJECT @@ -183,9 +183,7 @@ resources: version: v1 webhooks: conversion: true - spoke: - - v2 - - v2 + spoke: v2 webhookVersion: v1 - api: crdVersion: v1 diff --git a/testdata/project-v4-with-plugins/PROJECT b/testdata/project-v4-with-plugins/PROJECT index cd02c1d7706..6e1313b31a6 100644 --- a/testdata/project-v4-with-plugins/PROJECT +++ b/testdata/project-v4-with-plugins/PROJECT @@ -60,9 +60,7 @@ resources: version: v1 webhooks: conversion: true - spoke: - - v2 - - v2 + spoke: v2 webhookVersion: v1 - api: crdVersion: v1 diff --git a/testdata/project-v4/PROJECT b/testdata/project-v4/PROJECT index 193cbadc8c4..b764d9b0bf9 100644 --- a/testdata/project-v4/PROJECT +++ b/testdata/project-v4/PROJECT @@ -32,9 +32,7 @@ resources: version: v1 webhooks: conversion: true - spoke: - - v2 - - v2 + spoke: v2 webhookVersion: v1 - api: crdVersion: v1