From 445ed3d1107dea6655df8a310261c462cc769476 Mon Sep 17 00:00:00 2001 From: Adrian Orive Oneca Date: Fri, 22 Jan 2021 11:00:08 +0100 Subject: [PATCH] Add --plural flag (go/v3) Adds the flags and the required marker to the types file. Signed-off-by: Adrian Orive Oneca --- generate_testdata.sh | 24 +++++++-- pkg/model/resource/resource.go | 15 ++++-- pkg/model/resource/resource_test.go | 50 +++++++++++++++++++ pkg/plugins/golang/v3/api.go | 2 +- .../scaffolds/internal/templates/api/types.go | 8 ++- pkg/plugins/golang/v3/webhook.go | 2 +- testdata/project-v3/api/v1/admiral_types.go | 4 +- testdata/project-v3/api/v1/admiral_webhook.go | 2 +- .../project-v3/api/v1/webhook_suite_test.go | 4 +- ...ml => crew.testproject.org_admirales.yaml} | 6 +-- .../project-v3/config/crd/kustomization.yaml | 6 +-- ...als.yaml => cainjection_in_admirales.yaml} | 2 +- ...dmirals.yaml => webhook_in_admirales.yaml} | 2 +- .../config/rbac/admiral_editor_role.yaml | 6 +-- .../config/rbac/admiral_viewer_role.yaml | 6 +-- testdata/project-v3/config/rbac/role.yaml | 6 +-- .../project-v3/config/webhook/manifests.yaml | 2 +- .../controllers/admiral_controller.go | 6 +-- testdata/project-v3/main.go | 8 +-- 19 files changed, 121 insertions(+), 40 deletions(-) rename testdata/project-v3/config/crd/bases/{crew.testproject.org_admirals.yaml => crew.testproject.org_admirales.yaml} (93%) rename testdata/project-v3/config/crd/patches/{cainjection_in_admirals.yaml => cainjection_in_admirales.yaml} (86%) rename testdata/project-v3/config/crd/patches/{webhook_in_admirals.yaml => webhook_in_admirales.yaml} (89%) diff --git a/generate_testdata.sh b/generate_testdata.sh index 2b43d216dce..f66b16df5d9 100755 --- a/generate_testdata.sh +++ b/generate_testdata.sh @@ -59,14 +59,22 @@ scaffold_test_project() { $kb create api --group crew --version v1 --kind Captain --controller=true --resource=true --make=false $kb create api --group crew --version v1 --kind Captain --controller=true --resource=true --make=false --force $kb create webhook --group crew --version v1 --kind Captain --defaulting --programmatic-validation + if [ $project == "project-v3" ]; then + $kb create webhook --group crew --version v1 --kind Captain --defaulting --programmatic-validation --force + fi + $kb create api --group crew --version v1 --kind FirstMate --controller=true --resource=true --make=false $kb create webhook --group crew --version v1 --kind FirstMate --conversion - $kb create api --group crew --version v1 --kind Admiral --controller=true --resource=true --namespaced=false --make=false - $kb create webhook --group crew --version v1 --kind Admiral --defaulting - $kb create api --group crew --version v1 --kind Laker --controller=true --resource=false --make=false + if [ $project == "project-v3" ]; then - $kb create webhook --group crew --version v1 --kind Captain --defaulting --programmatic-validation --force + $kb create api --group crew --version v1 --kind Admiral --plural=admirales --controller=true --resource=true --namespaced=false --make=false + $kb create webhook --group crew --version v1 --kind Admiral --plural=admirales --defaulting + else + $kb create api --group crew --version v1 --kind Admiral --controller=true --resource=true --namespaced=false --make=false + $kb create webhook --group crew --version v1 --kind Admiral --defaulting fi + + $kb create api --group crew --version v1 --kind Laker --controller=true --resource=false --make=false elif [[ $project =~ multigroup ]]; then header_text 'Switching to multigroup layout ...' $kb edit --multigroup=true @@ -74,16 +82,24 @@ scaffold_test_project() { header_text 'Creating APIs ...' $kb create api --group crew --version v1 --kind Captain --controller=true --resource=true --make=false $kb create webhook --group crew --version v1 --kind Captain --defaulting --programmatic-validation + $kb create api --group ship --version v1beta1 --kind Frigate --controller=true --resource=true --make=false $kb create webhook --group ship --version v1beta1 --kind Frigate --conversion + $kb create api --group ship --version v1 --kind Destroyer --controller=true --resource=true --namespaced=false --make=false $kb create webhook --group ship --version v1 --kind Destroyer --defaulting + $kb create api --group ship --version v2alpha1 --kind Cruiser --controller=true --resource=true --namespaced=false --make=false $kb create webhook --group ship --version v2alpha1 --kind Cruiser --programmatic-validation + $kb create api --group sea-creatures --version v1beta1 --kind Kraken --controller=true --resource=true --make=false + $kb create api --group sea-creatures --version v1beta2 --kind Leviathan --controller=true --resource=true --make=false + $kb create api --group foo.policy --version v1 --kind HealthCheckPolicy --controller=true --resource=true --make=false + $kb create api --group apps --version v1 --kind Pod --controller=true --resource=false --make=false + if [ $project == "project-v3-multigroup" ]; then $kb create api --version v1 --kind Lakers --controller=true --resource=true --make=false $kb create webhook --version v1 --kind Lakers --defaulting --programmatic-validation diff --git a/pkg/model/resource/resource.go b/pkg/model/resource/resource.go index a9237fa7a4a..4499d7074f9 100644 --- a/pkg/model/resource/resource.go +++ b/pkg/model/resource/resource.go @@ -85,6 +85,11 @@ func (r Resource) HasConversionWebhook() bool { return r.Webhooks != nil && r.Webhooks.Conversion } +// IsRegularPlural returns true if the plural is the regular plural form for the kind. +func (r Resource) IsRegularPlural() bool { + return r.Plural == RegularPlural(r.Kind) +} + // Copy returns a deep copy of the Resource that can be safely modified without affecting the original. func (r Resource) Copy() Resource { // As this function doesn't use a pointer receiver, r is already a shallow copy. @@ -112,9 +117,13 @@ func (r *Resource) Update(other Resource) error { return fmt.Errorf("unable to update a Resource with another with non-matching GVK") } - // TODO: currently Plural & Path will always match. In the future, this may not be true (e.g. providing a - // --plural flag). In that case, we should yield an error in case of updating two resources with different - // values for these fields. + if r.Plural != other.Plural { + return fmt.Errorf("unable to update Resource with another with non-matching Plural") + } + + if r.Path != other.Path { + return fmt.Errorf("unable to update Resource with another with non-matching Path") + } // Update API. if r.API == nil && other.API != nil { diff --git a/pkg/model/resource/resource_test.go b/pkg/model/resource/resource_test.go index b0bd7fd7f8e..929e4951569 100644 --- a/pkg/model/resource/resource_test.go +++ b/pkg/model/resource/resource_test.go @@ -138,6 +138,16 @@ var _ = Describe("Resource", func() { Entry("no conversion", Resource{Webhooks: &Webhooks{Conversion: false}}), ) }) + + Context("IsRegularPlural", func() { + It("should return true if the regular plural form is used", func() { + Expect(Resource{GVK: GVK{Kind: "FirstMate"}, Plural: "firstmates"}.IsRegularPlural()).To(BeTrue()) + }) + + It("should return false if an irregular plural form is used", func() { + Expect(Resource{GVK: GVK{Kind: "FirstMate"}, Plural: "mates"}.IsRegularPlural()).To(BeFalse()) + }) + }) }) Context("Copy", func() { @@ -251,6 +261,46 @@ var _ = Describe("Resource", func() { Expect(r.Update(other)).NotTo(Succeed()) }) + It("should fail for different Plurals", func() { + r = Resource{ + GVK: GVK{ + Group: group, + Version: version, + Kind: kind, + }, + Plural: "kinds", + } + other = Resource{ + GVK: GVK{ + Group: group, + Version: version, + Kind: kind, + }, + Plural: "types", + } + Expect(r.Update(other)).NotTo(Succeed()) + }) + + It("should fail for different Paths", func() { + r = Resource{ + GVK: GVK{ + Group: group, + Version: version, + Kind: kind, + }, + Path: "api/v1", + } + other = Resource{ + GVK: GVK{ + Group: group, + Version: version, + Kind: kind, + }, + Path: "apis/group/v1", + } + Expect(r.Update(other)).NotTo(Succeed()) + }) + Context("API", func() { It("should work with nil APIs", func() { r = Resource{ diff --git a/pkg/plugins/golang/v3/api.go b/pkg/plugins/golang/v3/api.go index ab8a629f712..04d90c0209f 100644 --- a/pkg/plugins/golang/v3/api.go +++ b/pkg/plugins/golang/v3/api.go @@ -121,7 +121,7 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) { p.options.Domain = p.config.GetDomain() fs.StringVar(&p.options.Version, "version", "", "resource Version") fs.StringVar(&p.options.Kind, "kind", "", "resource Kind") - // p.options.Plural can be set to specify an irregular plural form + fs.StringVar(&p.options.Plural, "plural", "", "resource irregular plural form") fs.BoolVar(&p.options.DoAPI, "resource", true, "if set, generate the resource without prompting the user") diff --git a/pkg/plugins/golang/v3/scaffolds/internal/templates/api/types.go b/pkg/plugins/golang/v3/scaffolds/internal/templates/api/types.go index 0875b04df88..da32a090d07 100644 --- a/pkg/plugins/golang/v3/scaffolds/internal/templates/api/types.go +++ b/pkg/plugins/golang/v3/scaffolds/internal/templates/api/types.go @@ -91,7 +91,13 @@ type {{ .Resource.Kind }}Status struct { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -{{ if not .Resource.API.Namespaced }} //+kubebuilder:resource:scope=Cluster {{ end }} +{{- if and not .Resource.API.Namespaced not .Resource.IsRegular }} +//+kubebuilder:resource:path={{ .Resource.Plural }},scope=Cluster +{{- else if not .Resource.API.Namespaced }} +//+kubebuilder:resource:scope=Cluster +{{- else if not .Resource.IsRegularPlural }} +//+kubebuilder:resource:path={{ .Resource.Plural }} +{{- end }} // {{ .Resource.Kind }} is the Schema for the {{ .Resource.Plural }} API type {{ .Resource.Kind }} struct { diff --git a/pkg/plugins/golang/v3/webhook.go b/pkg/plugins/golang/v3/webhook.go index 229fb8579b7..ac64f39c02e 100644 --- a/pkg/plugins/golang/v3/webhook.go +++ b/pkg/plugins/golang/v3/webhook.go @@ -76,7 +76,7 @@ func (p *createWebhookSubcommand) BindFlags(fs *pflag.FlagSet) { p.options.Domain = p.config.GetDomain() fs.StringVar(&p.options.Version, "version", "", "resource Version") fs.StringVar(&p.options.Kind, "kind", "", "resource Kind") - fs.StringVar(&p.options.Plural, "resource", "", "resource irregular plural form") + fs.StringVar(&p.options.Plural, "plural", "", "resource irregular plural form") fs.StringVar(&p.options.WebhookVersion, "webhook-version", defaultWebhookVersion, "version of {Mutating,Validating}WebhookConfigurations to scaffold. Options: [v1, v1beta1]") diff --git a/testdata/project-v3/api/v1/admiral_types.go b/testdata/project-v3/api/v1/admiral_types.go index fe0d8308a70..20f58e77112 100644 --- a/testdata/project-v3/api/v1/admiral_types.go +++ b/testdata/project-v3/api/v1/admiral_types.go @@ -40,9 +40,9 @@ type AdmiralStatus struct { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -//+kubebuilder:resource:scope=Cluster +//+kubebuilder:resource:path=admirales,scope=Cluster -// Admiral is the Schema for the admirals API +// Admiral is the Schema for the admirales API type Admiral struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/testdata/project-v3/api/v1/admiral_webhook.go b/testdata/project-v3/api/v1/admiral_webhook.go index dca1250dcdc..47177bf43e4 100644 --- a/testdata/project-v3/api/v1/admiral_webhook.go +++ b/testdata/project-v3/api/v1/admiral_webhook.go @@ -33,7 +33,7 @@ func (r *Admiral) SetupWebhookWithManager(mgr ctrl.Manager) error { // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! -//+kubebuilder:webhook:path=/mutate-crew-testproject-org-v1-admiral,mutating=true,failurePolicy=fail,sideEffects=None,groups=crew.testproject.org,resources=admirals,verbs=create;update,versions=v1,name=madmiral.kb.io,admissionReviewVersions={v1,v1beta1} +//+kubebuilder:webhook:path=/mutate-crew-testproject-org-v1-admiral,mutating=true,failurePolicy=fail,sideEffects=None,groups=crew.testproject.org,resources=admirales,verbs=create;update,versions=v1,name=madmiral.kb.io,admissionReviewVersions={v1,v1beta1} var _ webhook.Defaulter = &Admiral{} diff --git a/testdata/project-v3/api/v1/webhook_suite_test.go b/testdata/project-v3/api/v1/webhook_suite_test.go index 21a406c4c13..1dabe80a540 100644 --- a/testdata/project-v3/api/v1/webhook_suite_test.go +++ b/testdata/project-v3/api/v1/webhook_suite_test.go @@ -109,10 +109,10 @@ var _ = BeforeSuite(func() { err = (&Captain{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) - err = (&Admiral{}).SetupWebhookWithManager(mgr) + err = (&Captain{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) - err = (&Captain{}).SetupWebhookWithManager(mgr) + err = (&Admiral{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:webhook diff --git a/testdata/project-v3/config/crd/bases/crew.testproject.org_admirals.yaml b/testdata/project-v3/config/crd/bases/crew.testproject.org_admirales.yaml similarity index 93% rename from testdata/project-v3/config/crd/bases/crew.testproject.org_admirals.yaml rename to testdata/project-v3/config/crd/bases/crew.testproject.org_admirales.yaml index e8612bd5328..8998d558726 100644 --- a/testdata/project-v3/config/crd/bases/crew.testproject.org_admirals.yaml +++ b/testdata/project-v3/config/crd/bases/crew.testproject.org_admirales.yaml @@ -6,20 +6,20 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.4.1 creationTimestamp: null - name: admirals.crew.testproject.org + name: admirales.crew.testproject.org spec: group: crew.testproject.org names: kind: Admiral listKind: AdmiralList - plural: admirals + plural: admirales singular: admiral scope: Cluster versions: - name: v1 schema: openAPIV3Schema: - description: Admiral is the Schema for the admirals API + description: Admiral is the Schema for the admirales API properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation diff --git a/testdata/project-v3/config/crd/kustomization.yaml b/testdata/project-v3/config/crd/kustomization.yaml index 481fe329551..61dcc48c5e5 100644 --- a/testdata/project-v3/config/crd/kustomization.yaml +++ b/testdata/project-v3/config/crd/kustomization.yaml @@ -4,7 +4,7 @@ resources: - bases/crew.testproject.org_captains.yaml - bases/crew.testproject.org_firstmates.yaml -- bases/crew.testproject.org_admirals.yaml +- bases/crew.testproject.org_admirales.yaml #+kubebuilder:scaffold:crdkustomizeresource patchesStrategicMerge: @@ -12,14 +12,14 @@ patchesStrategicMerge: # patches here are for enabling the conversion webhook for each CRD #- patches/webhook_in_captains.yaml #- patches/webhook_in_firstmates.yaml -#- patches/webhook_in_admirals.yaml +#- patches/webhook_in_admirales.yaml #+kubebuilder:scaffold:crdkustomizewebhookpatch # [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. # patches here are for enabling the CA injection for each CRD #- patches/cainjection_in_captains.yaml #- patches/cainjection_in_firstmates.yaml -#- patches/cainjection_in_admirals.yaml +#- patches/cainjection_in_admirales.yaml #+kubebuilder:scaffold:crdkustomizecainjectionpatch # the following config is for teaching kustomize how to do kustomization for CRDs. diff --git a/testdata/project-v3/config/crd/patches/cainjection_in_admirals.yaml b/testdata/project-v3/config/crd/patches/cainjection_in_admirales.yaml similarity index 86% rename from testdata/project-v3/config/crd/patches/cainjection_in_admirals.yaml rename to testdata/project-v3/config/crd/patches/cainjection_in_admirales.yaml index ba7fea6e88d..04882738e44 100644 --- a/testdata/project-v3/config/crd/patches/cainjection_in_admirals.yaml +++ b/testdata/project-v3/config/crd/patches/cainjection_in_admirales.yaml @@ -4,4 +4,4 @@ kind: CustomResourceDefinition metadata: annotations: cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) - name: admirals.crew.testproject.org + name: admirales.crew.testproject.org diff --git a/testdata/project-v3/config/crd/patches/webhook_in_admirals.yaml b/testdata/project-v3/config/crd/patches/webhook_in_admirales.yaml similarity index 89% rename from testdata/project-v3/config/crd/patches/webhook_in_admirals.yaml rename to testdata/project-v3/config/crd/patches/webhook_in_admirales.yaml index e7cef4898e3..2bde272e76a 100644 --- a/testdata/project-v3/config/crd/patches/webhook_in_admirals.yaml +++ b/testdata/project-v3/config/crd/patches/webhook_in_admirales.yaml @@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: - name: admirals.crew.testproject.org + name: admirales.crew.testproject.org spec: conversion: strategy: Webhook diff --git a/testdata/project-v3/config/rbac/admiral_editor_role.yaml b/testdata/project-v3/config/rbac/admiral_editor_role.yaml index 7f2cc5cd99f..08184d661d2 100644 --- a/testdata/project-v3/config/rbac/admiral_editor_role.yaml +++ b/testdata/project-v3/config/rbac/admiral_editor_role.yaml @@ -1,4 +1,4 @@ -# permissions for end users to edit admirals. +# permissions for end users to edit admirales. apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: @@ -7,7 +7,7 @@ rules: - apiGroups: - crew.testproject.org resources: - - admirals + - admirales verbs: - create - delete @@ -19,6 +19,6 @@ rules: - apiGroups: - crew.testproject.org resources: - - admirals/status + - admirales/status verbs: - get diff --git a/testdata/project-v3/config/rbac/admiral_viewer_role.yaml b/testdata/project-v3/config/rbac/admiral_viewer_role.yaml index ddbb0c2a85b..65df31b4b48 100644 --- a/testdata/project-v3/config/rbac/admiral_viewer_role.yaml +++ b/testdata/project-v3/config/rbac/admiral_viewer_role.yaml @@ -1,4 +1,4 @@ -# permissions for end users to view admirals. +# permissions for end users to view admirales. apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: @@ -7,7 +7,7 @@ rules: - apiGroups: - crew.testproject.org resources: - - admirals + - admirales verbs: - get - list @@ -15,6 +15,6 @@ rules: - apiGroups: - crew.testproject.org resources: - - admirals/status + - admirales/status verbs: - get diff --git a/testdata/project-v3/config/rbac/role.yaml b/testdata/project-v3/config/rbac/role.yaml index 1d105256811..a02d192e82e 100644 --- a/testdata/project-v3/config/rbac/role.yaml +++ b/testdata/project-v3/config/rbac/role.yaml @@ -9,7 +9,7 @@ rules: - apiGroups: - crew.testproject.org resources: - - admirals + - admirales verbs: - create - delete @@ -21,13 +21,13 @@ rules: - apiGroups: - crew.testproject.org resources: - - admirals/finalizers + - admirales/finalizers verbs: - update - apiGroups: - crew.testproject.org resources: - - admirals/status + - admirales/status verbs: - get - patch diff --git a/testdata/project-v3/config/webhook/manifests.yaml b/testdata/project-v3/config/webhook/manifests.yaml index 7c748d3a804..d3fc4ff3ffc 100644 --- a/testdata/project-v3/config/webhook/manifests.yaml +++ b/testdata/project-v3/config/webhook/manifests.yaml @@ -25,7 +25,7 @@ webhooks: - CREATE - UPDATE resources: - - admirals + - admirales sideEffects: None - admissionReviewVersions: - v1 diff --git a/testdata/project-v3/controllers/admiral_controller.go b/testdata/project-v3/controllers/admiral_controller.go index 5f7be6ac8c5..61b4456a335 100644 --- a/testdata/project-v3/controllers/admiral_controller.go +++ b/testdata/project-v3/controllers/admiral_controller.go @@ -34,9 +34,9 @@ type AdmiralReconciler struct { Scheme *runtime.Scheme } -//+kubebuilder:rbac:groups=crew.testproject.org,resources=admirals,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=crew.testproject.org,resources=admirals/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=crew.testproject.org,resources=admirals/finalizers,verbs=update +//+kubebuilder:rbac:groups=crew.testproject.org,resources=admirales,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=crew.testproject.org,resources=admirales/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=crew.testproject.org,resources=admirales/finalizers,verbs=update // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. diff --git a/testdata/project-v3/main.go b/testdata/project-v3/main.go index 0ae83837744..fc10f63f98e 100644 --- a/testdata/project-v3/main.go +++ b/testdata/project-v3/main.go @@ -98,6 +98,10 @@ func main() { setupLog.Error(err, "unable to create webhook", "webhook", "Captain") os.Exit(1) } + if err = (&crewv1.Captain{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "Captain") + os.Exit(1) + } if err = (&controllers.FirstMateReconciler{ Client: mgr.GetClient(), Log: ctrl.Log.WithName("controllers").WithName("FirstMate"), @@ -130,10 +134,6 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Laker") os.Exit(1) } - if err = (&crewv1.Captain{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "Captain") - os.Exit(1) - } //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {