Skip to content

Commit 287a105

Browse files
committed
✨ Allow customizing generated webhook configuration's name
## example usage ``` ❯ GOBIN=(pwd)/bin go install ./cmd/* ❯ ./bin/controller-gen webhook -w Webhook +kubebuilder:webhook:admissionReviewVersions=<[]string>,failurePolicy=<string>,groups=<[]string>[,matchPolicy=<string>],mutating=<bool>,name=<string>[,path=<string>][,reinvocationPolicy=<string>],resources=<[]string>[,sideEffects=<string>][,timeoutSeconds=<int>][,url=<string>],verbs=<[]string>,versions=<[]string>[,webhookVersions=<[]string>] package specifies how a webhook should be served. +kubebuilder:webhookconfiguration:mutating=<bool>[,name=<string>] package specifies how a webhook should be served. ``` Signed-off-by: David Xia <[email protected]>
1 parent 46379c6 commit 287a105

File tree

8 files changed

+531
-9
lines changed

8 files changed

+531
-9
lines changed

pkg/webhook/parser.go

+98-9
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"k8s.io/apimachinery/pkg/runtime/schema"
3232
"k8s.io/apimachinery/pkg/util/sets"
3333

34+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3435
"sigs.k8s.io/controller-tools/pkg/genall"
3536
"sigs.k8s.io/controller-tools/pkg/markers"
3637
)
@@ -42,16 +43,31 @@ const (
4243
)
4344

4445
var (
45-
// ConfigDefinition s a marker for defining Webhook manifests.
46+
// ConfigDefinition is a marker for defining Webhook manifests.
4647
// Call ToWebhook on the value to get a Kubernetes Webhook.
4748
ConfigDefinition = markers.Must(markers.MakeDefinition("kubebuilder:webhook", markers.DescribesPackage, Config{}))
49+
// WebhookConfigDefinition is a marker for defining MutatingWebhookConfiguration or ValidatingWebhookConfiguration manifests.
50+
WebhookConfigDefinition = markers.Must(markers.MakeDefinition("kubebuilder:webhookconfiguration", markers.DescribesPackage, WebhookConfig{}))
4851
)
4952

5053
// supportedWebhookVersions returns currently supported API version of {Mutating,Validating}WebhookConfiguration.
5154
func supportedWebhookVersions() []string {
5255
return []string{defaultWebhookVersion}
5356
}
5457

58+
// +controllertools:marker:generateHelp
59+
60+
type WebhookConfig struct {
61+
// Mutating marks this as a mutating webhook (it's validating only if false)
62+
//
63+
// Mutating webhooks are allowed to change the object in their response,
64+
// and are called *before* all validating webhooks. Mutating webhooks may
65+
// choose to reject an object, similarly to a validating webhook.
66+
Mutating bool
67+
// Name indicates the name of the K8s MutatingWebhookConfiguration or ValidatingWebhookConfiguration object.
68+
Name string `marker:"name,optional"`
69+
}
70+
5571
// +controllertools:marker:generateHelp:category=Webhook
5672

5773
// Config specifies how a webhook should be served.
@@ -154,6 +170,32 @@ func verbToAPIVariant(verbRaw string) admissionregv1.OperationType {
154170
}
155171
}
156172

173+
// ToMutatingWebhookConfiguration converts this WebhookConfig to its Kubernetes API form.
174+
func (c WebhookConfig) ToMutatingWebhookConfiguration() (admissionregv1.MutatingWebhookConfiguration, error) {
175+
if !c.Mutating {
176+
return admissionregv1.MutatingWebhookConfiguration{}, fmt.Errorf("%s is a validating webhook", c.Name)
177+
}
178+
179+
return admissionregv1.MutatingWebhookConfiguration{
180+
ObjectMeta: metav1.ObjectMeta{
181+
Name: c.Name,
182+
},
183+
}, nil
184+
}
185+
186+
// ToValidatingWebhookConfiguration converts this WebhookConfig to its Kubernetes API form.
187+
func (c WebhookConfig) ToValidatingWebhookConfiguration() (admissionregv1.ValidatingWebhookConfiguration, error) {
188+
if c.Mutating {
189+
return admissionregv1.ValidatingWebhookConfiguration{}, fmt.Errorf("%s is a mutating webhook", c.Name)
190+
}
191+
192+
return admissionregv1.ValidatingWebhookConfiguration{
193+
ObjectMeta: metav1.ObjectMeta{
194+
Name: c.Name,
195+
},
196+
}, nil
197+
}
198+
157199
// ToMutatingWebhook converts this rule to its Kubernetes API form.
158200
func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) {
159201
if !c.Mutating {
@@ -362,20 +404,55 @@ func (Generator) RegisterMarkers(into *markers.Registry) error {
362404
if err := into.Register(ConfigDefinition); err != nil {
363405
return err
364406
}
407+
if err := into.Register(WebhookConfigDefinition); err != nil {
408+
return err
409+
}
365410
into.AddHelp(ConfigDefinition, Config{}.Help())
411+
into.AddHelp(WebhookConfigDefinition, Config{}.Help())
366412
return nil
367413
}
368414

369415
func (g Generator) Generate(ctx *genall.GenerationContext) error {
370416
supportedWebhookVersions := supportedWebhookVersions()
371417
mutatingCfgs := make(map[string][]admissionregv1.MutatingWebhook, len(supportedWebhookVersions))
372418
validatingCfgs := make(map[string][]admissionregv1.ValidatingWebhook, len(supportedWebhookVersions))
419+
var mutatingWebhookCfgs admissionregv1.MutatingWebhookConfiguration
420+
var validatingWebhookCfgs admissionregv1.ValidatingWebhookConfiguration
421+
373422
for _, root := range ctx.Roots {
374423
markerSet, err := markers.PackageMarkers(ctx.Collector, root)
375424
if err != nil {
376425
root.AddError(err)
377426
}
378427

428+
webhookCfgs := markerSet[WebhookConfigDefinition.Name]
429+
var hasValidatingWebhookConfig, hasMutatingWebhookConfig bool = false, false
430+
for _, webhookCfg := range webhookCfgs {
431+
webhookCfg := webhookCfg.(WebhookConfig)
432+
433+
if webhookCfg.Mutating {
434+
if hasMutatingWebhookConfig {
435+
return fmt.Errorf("duplicate mutating %s with name %s", WebhookConfigDefinition.Name, webhookCfg.Name)
436+
}
437+
438+
if mutatingWebhookCfgs, err = webhookCfg.ToMutatingWebhookConfiguration(); err != nil {
439+
return err
440+
}
441+
442+
hasMutatingWebhookConfig = true
443+
} else {
444+
if hasValidatingWebhookConfig {
445+
return fmt.Errorf("duplicate validating %s with name %s", WebhookConfigDefinition.Name, webhookCfg.Name)
446+
}
447+
448+
if validatingWebhookCfgs, err = webhookCfg.ToValidatingWebhookConfiguration(); err != nil {
449+
return err
450+
}
451+
452+
hasValidatingWebhookConfig = true
453+
}
454+
}
455+
379456
cfgs := markerSet[ConfigDefinition.Name]
380457
sort.SliceStable(cfgs, func(i, j int) bool {
381458
return cfgs[i].(Config).Name < cfgs[j].(Config).Name
@@ -410,16 +487,22 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
410487
versionedWebhooks := make(map[string][]interface{}, len(supportedWebhookVersions))
411488
for _, version := range supportedWebhookVersions {
412489
if cfgs, ok := mutatingCfgs[version]; ok {
413-
// The only possible version in supportedWebhookVersions is v1,
414-
// so use it for all versioned types in this context.
415-
objRaw := &admissionregv1.MutatingWebhookConfiguration{}
490+
var objRaw *admissionregv1.MutatingWebhookConfiguration
491+
if mutatingWebhookCfgs.Name != "" {
492+
objRaw = &mutatingWebhookCfgs
493+
} else {
494+
// The only possible version in supportedWebhookVersions is v1,
495+
// so use it for all versioned types in this context.
496+
objRaw = &admissionregv1.MutatingWebhookConfiguration{}
497+
objRaw.SetName("mutating-webhook-configuration")
498+
}
416499
objRaw.SetGroupVersionKind(schema.GroupVersionKind{
417500
Group: admissionregv1.SchemeGroupVersion.Group,
418501
Version: version,
419502
Kind: "MutatingWebhookConfiguration",
420503
})
421-
objRaw.SetName("mutating-webhook-configuration")
422504
objRaw.Webhooks = cfgs
505+
423506
for i := range objRaw.Webhooks {
424507
// SideEffects is required in admissionregistration/v1, if this is not set or set to `Some` or `Known`,
425508
// return an error
@@ -441,16 +524,22 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
441524
}
442525

443526
if cfgs, ok := validatingCfgs[version]; ok {
444-
// The only possible version in supportedWebhookVersions is v1,
445-
// so use it for all versioned types in this context.
446-
objRaw := &admissionregv1.ValidatingWebhookConfiguration{}
527+
var objRaw *admissionregv1.ValidatingWebhookConfiguration
528+
if validatingWebhookCfgs.Name != "" {
529+
objRaw = &validatingWebhookCfgs
530+
} else {
531+
// The only possible version in supportedWebhookVersions is v1,
532+
// so use it for all versioned types in this context.
533+
objRaw = &admissionregv1.ValidatingWebhookConfiguration{}
534+
objRaw.SetName("validating-webhook-configuration")
535+
}
447536
objRaw.SetGroupVersionKind(schema.GroupVersionKind{
448537
Group: admissionregv1.SchemeGroupVersion.Group,
449538
Version: version,
450539
Kind: "ValidatingWebhookConfiguration",
451540
})
452-
objRaw.SetName("validating-webhook-configuration")
453541
objRaw.Webhooks = cfgs
542+
454543
for i := range objRaw.Webhooks {
455544
// SideEffects is required in admissionregistration/v1, if this is not set or set to `Some` or `Known`,
456545
// return an error

pkg/webhook/parser_integration_test.go

+85
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
5656
By("setting up the parser")
5757
reg := &markers.Registry{}
5858
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
59+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
5960

6061
By("requesting that the manifest be generated")
6162
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -85,6 +86,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
8586
By("setting up the parser")
8687
reg := &markers.Registry{}
8788
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
89+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
8890

8991
By("requesting that the manifest be generated")
9092
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -116,6 +118,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
116118
By("setting up the parser")
117119
reg := &markers.Registry{}
118120
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
121+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
119122

120123
By("requesting that the manifest be generated")
121124
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -145,6 +148,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
145148
By("setting up the parser")
146149
reg := &markers.Registry{}
147150
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
151+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
148152

149153
By("requesting that the manifest be generated")
150154
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -174,6 +178,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
174178
By("setting up the parser")
175179
reg := &markers.Registry{}
176180
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
181+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
177182

178183
By("requesting that the manifest be generated")
179184
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -219,6 +224,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
219224
By("setting up the parser")
220225
reg := &markers.Registry{}
221226
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
227+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
222228

223229
By("requesting that the manifest be generated")
224230
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -268,6 +274,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
268274
By("setting up the parser")
269275
reg := &markers.Registry{}
270276
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
277+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
271278

272279
By("requesting that the manifest be generated")
273280
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -313,6 +320,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
313320
By("setting up the parser")
314321
reg := &markers.Registry{}
315322
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
323+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
316324

317325
By("requesting that the manifest be generated")
318326
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
@@ -326,6 +334,83 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
326334
err = webhook.Generator{}.Generate(genCtx)
327335
Expect(err).To(HaveOccurred())
328336
})
337+
338+
It("should properly generate the webhook definition when a name is specified with the `kubebuilder:webhookconfiguration` marker", func() {
339+
By("switching into testdata to appease go modules")
340+
cwd, err := os.Getwd()
341+
Expect(err).NotTo(HaveOccurred())
342+
Expect(os.Chdir("./testdata/valid-custom-name")).To(Succeed()) // go modules are directory-sensitive
343+
defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
344+
345+
By("loading the roots")
346+
pkgs, err := loader.LoadRoots(".")
347+
Expect(err).NotTo(HaveOccurred())
348+
Expect(pkgs).To(HaveLen(1))
349+
350+
By("setting up the parser")
351+
reg := &markers.Registry{}
352+
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
353+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
354+
355+
By("requesting that the manifest be generated")
356+
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
357+
Expect(err).NotTo(HaveOccurred())
358+
defer os.RemoveAll(outputDir)
359+
genCtx := &genall.GenerationContext{
360+
Collector: &markers.Collector{Registry: reg},
361+
Roots: pkgs,
362+
OutputRule: genall.OutputToDirectory(outputDir),
363+
}
364+
Expect(webhook.Generator{}.Generate(genCtx)).To(Succeed())
365+
for _, r := range genCtx.Roots {
366+
Expect(r.Errors).To(HaveLen(0))
367+
}
368+
369+
By("loading the generated v1 YAML")
370+
actualFile, err := os.ReadFile(path.Join(outputDir, "manifests.yaml"))
371+
Expect(err).NotTo(HaveOccurred())
372+
actualMutating, actualValidating := unmarshalBothV1(actualFile)
373+
374+
By("loading the desired v1 YAML")
375+
expectedFile, err := os.ReadFile("manifests.yaml")
376+
Expect(err).NotTo(HaveOccurred())
377+
expectedMutating, expectedValidating := unmarshalBothV1(expectedFile)
378+
379+
By("comparing the two")
380+
assertSame(actualMutating, expectedMutating)
381+
assertSame(actualValidating, expectedValidating)
382+
})
383+
384+
It("should fail to generate when there are multiple `kubebuilder:webhookconfiguration` markers of the same mutation type", func() {
385+
By("switching into testdata to appease go modules")
386+
cwd, err := os.Getwd()
387+
Expect(err).NotTo(HaveOccurred())
388+
Expect(os.Chdir("./testdata/invalid-multiple-webhookconfigurations")).To(Succeed()) // go modules are directory-sensitive
389+
defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
390+
391+
By("loading the roots")
392+
pkgs, err := loader.LoadRoots(".")
393+
Expect(err).NotTo(HaveOccurred())
394+
Expect(pkgs).To(HaveLen(1))
395+
396+
By("setting up the parser")
397+
reg := &markers.Registry{}
398+
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
399+
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())
400+
401+
By("requesting that the manifest be generated")
402+
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
403+
Expect(err).NotTo(HaveOccurred())
404+
defer os.RemoveAll(outputDir)
405+
genCtx := &genall.GenerationContext{
406+
Collector: &markers.Collector{Registry: reg},
407+
Roots: pkgs,
408+
OutputRule: genall.OutputToDirectory(outputDir),
409+
}
410+
err = webhook.Generator{}.Generate(genCtx)
411+
Expect(err).To(HaveOccurred())
412+
})
413+
329414
})
330415

331416
func unmarshalBothV1(in []byte) (mutating admissionregv1.MutatingWebhookConfiguration, validating admissionregv1.ValidatingWebhookConfiguration) {

0 commit comments

Comments
 (0)