Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠ Add --plural flag (go/v3) #1967

Merged
merged 1 commit into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions generate_testdata.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,31 +59,47 @@ 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked that the changes here does not change the current scenarios and just add a new one to check a plural with an irregular form for an api and webhook


$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

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
Expand Down
15 changes: 12 additions & 3 deletions pkg/model/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
50 changes: 50 additions & 0 deletions pkg/model/resource/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/golang/v3/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.IsRegularPlural) }}
//+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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/golang/v3/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]")
Expand Down
4 changes: 2 additions & 2 deletions testdata/project-v3/api/v1/admiral_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
2 changes: 1 addition & 1 deletion testdata/project-v3/api/v1/admiral_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down
4 changes: 2 additions & 2 deletions testdata/project-v3/api/v1/webhook_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions testdata/project-v3/config/crd/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@
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:
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
# 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions testdata/project-v3/config/rbac/admiral_editor_role.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -7,7 +7,7 @@ rules:
- apiGroups:
- crew.testproject.org
resources:
- admirals
- admirales
verbs:
- create
- delete
Expand All @@ -19,6 +19,6 @@ rules:
- apiGroups:
- crew.testproject.org
resources:
- admirals/status
- admirales/status
verbs:
- get
6 changes: 3 additions & 3 deletions testdata/project-v3/config/rbac/admiral_viewer_role.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -7,14 +7,14 @@ rules:
- apiGroups:
- crew.testproject.org
resources:
- admirals
- admirales
verbs:
- get
- list
- watch
- apiGroups:
- crew.testproject.org
resources:
- admirals/status
- admirales/status
verbs:
- get
6 changes: 3 additions & 3 deletions testdata/project-v3/config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ rules:
- apiGroups:
- crew.testproject.org
resources:
- admirals
- admirales
verbs:
- create
- delete
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion testdata/project-v3/config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ webhooks:
- CREATE
- UPDATE
resources:
- admirals
- admirales
sideEffects: None
- admissionReviewVersions:
- v1
Expand Down
6 changes: 3 additions & 3 deletions testdata/project-v3/controllers/admiral_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions testdata/project-v3/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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 {
Expand Down