Skip to content

Commit

Permalink
Merge pull request #1002 from davidxia/webhook-name
Browse files Browse the repository at this point in the history 
✨ Allow customizing generated webhook configuration's name
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Sep 12, 2024
2 parents 715d27e + 287a105 commit c69110a
Show file tree
Hide file tree
Showing 8 changed files with 531 additions and 9 deletions.
107 changes: 98 additions & 9 deletions pkg/webhook/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-tools/pkg/genall"
"sigs.k8s.io/controller-tools/pkg/markers"
)
Expand All @@ -42,16 +43,31 @@ const (
)

var (
// ConfigDefinition s a marker for defining Webhook manifests.
// ConfigDefinition is a marker for defining Webhook manifests.
// Call ToWebhook on the value to get a Kubernetes Webhook.
ConfigDefinition = markers.Must(markers.MakeDefinition("kubebuilder:webhook", markers.DescribesPackage, Config{}))
// WebhookConfigDefinition is a marker for defining MutatingWebhookConfiguration or ValidatingWebhookConfiguration manifests.
WebhookConfigDefinition = markers.Must(markers.MakeDefinition("kubebuilder:webhookconfiguration", markers.DescribesPackage, WebhookConfig{}))
)

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

// +controllertools:marker:generateHelp

type WebhookConfig struct {
// Mutating marks this as a mutating webhook (it's validating only if false)
//
// Mutating webhooks are allowed to change the object in their response,
// and are called *before* all validating webhooks. Mutating webhooks may
// choose to reject an object, similarly to a validating webhook.
Mutating bool
// Name indicates the name of the K8s MutatingWebhookConfiguration or ValidatingWebhookConfiguration object.
Name string `marker:"name,optional"`
}

// +controllertools:marker:generateHelp:category=Webhook

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

// ToMutatingWebhookConfiguration converts this WebhookConfig to its Kubernetes API form.
func (c WebhookConfig) ToMutatingWebhookConfiguration() (admissionregv1.MutatingWebhookConfiguration, error) {
if !c.Mutating {
return admissionregv1.MutatingWebhookConfiguration{}, fmt.Errorf("%s is a validating webhook", c.Name)
}

return admissionregv1.MutatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: c.Name,
},
}, nil
}

// ToValidatingWebhookConfiguration converts this WebhookConfig to its Kubernetes API form.
func (c WebhookConfig) ToValidatingWebhookConfiguration() (admissionregv1.ValidatingWebhookConfiguration, error) {
if c.Mutating {
return admissionregv1.ValidatingWebhookConfiguration{}, fmt.Errorf("%s is a mutating webhook", c.Name)
}

return admissionregv1.ValidatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: c.Name,
},
}, nil
}

// ToMutatingWebhook converts this rule to its Kubernetes API form.
func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) {
if !c.Mutating {
Expand Down Expand Up @@ -362,20 +404,55 @@ func (Generator) RegisterMarkers(into *markers.Registry) error {
if err := into.Register(ConfigDefinition); err != nil {
return err
}
if err := into.Register(WebhookConfigDefinition); err != nil {
return err
}
into.AddHelp(ConfigDefinition, Config{}.Help())
into.AddHelp(WebhookConfigDefinition, Config{}.Help())
return nil
}

func (g Generator) Generate(ctx *genall.GenerationContext) error {
supportedWebhookVersions := supportedWebhookVersions()
mutatingCfgs := make(map[string][]admissionregv1.MutatingWebhook, len(supportedWebhookVersions))
validatingCfgs := make(map[string][]admissionregv1.ValidatingWebhook, len(supportedWebhookVersions))
var mutatingWebhookCfgs admissionregv1.MutatingWebhookConfiguration
var validatingWebhookCfgs admissionregv1.ValidatingWebhookConfiguration

for _, root := range ctx.Roots {
markerSet, err := markers.PackageMarkers(ctx.Collector, root)
if err != nil {
root.AddError(err)
}

webhookCfgs := markerSet[WebhookConfigDefinition.Name]
var hasValidatingWebhookConfig, hasMutatingWebhookConfig bool = false, false
for _, webhookCfg := range webhookCfgs {
webhookCfg := webhookCfg.(WebhookConfig)

if webhookCfg.Mutating {
if hasMutatingWebhookConfig {
return fmt.Errorf("duplicate mutating %s with name %s", WebhookConfigDefinition.Name, webhookCfg.Name)
}

if mutatingWebhookCfgs, err = webhookCfg.ToMutatingWebhookConfiguration(); err != nil {
return err
}

hasMutatingWebhookConfig = true
} else {
if hasValidatingWebhookConfig {
return fmt.Errorf("duplicate validating %s with name %s", WebhookConfigDefinition.Name, webhookCfg.Name)
}

if validatingWebhookCfgs, err = webhookCfg.ToValidatingWebhookConfiguration(); err != nil {
return err
}

hasValidatingWebhookConfig = true
}
}

cfgs := markerSet[ConfigDefinition.Name]
sort.SliceStable(cfgs, func(i, j int) bool {
return cfgs[i].(Config).Name < cfgs[j].(Config).Name
Expand Down Expand Up @@ -410,16 +487,22 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
versionedWebhooks := make(map[string][]interface{}, len(supportedWebhookVersions))
for _, version := range supportedWebhookVersions {
if cfgs, ok := mutatingCfgs[version]; ok {
// The only possible version in supportedWebhookVersions is v1,
// so use it for all versioned types in this context.
objRaw := &admissionregv1.MutatingWebhookConfiguration{}
var objRaw *admissionregv1.MutatingWebhookConfiguration
if mutatingWebhookCfgs.Name != "" {
objRaw = &mutatingWebhookCfgs
} else {
// The only possible version in supportedWebhookVersions is v1,
// so use it for all versioned types in this context.
objRaw = &admissionregv1.MutatingWebhookConfiguration{}
objRaw.SetName("mutating-webhook-configuration")
}
objRaw.SetGroupVersionKind(schema.GroupVersionKind{
Group: admissionregv1.SchemeGroupVersion.Group,
Version: version,
Kind: "MutatingWebhookConfiguration",
})
objRaw.SetName("mutating-webhook-configuration")
objRaw.Webhooks = cfgs

for i := range objRaw.Webhooks {
// SideEffects is required in admissionregistration/v1, if this is not set or set to `Some` or `Known`,
// return an error
Expand All @@ -441,16 +524,22 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
}

if cfgs, ok := validatingCfgs[version]; ok {
// The only possible version in supportedWebhookVersions is v1,
// so use it for all versioned types in this context.
objRaw := &admissionregv1.ValidatingWebhookConfiguration{}
var objRaw *admissionregv1.ValidatingWebhookConfiguration
if validatingWebhookCfgs.Name != "" {
objRaw = &validatingWebhookCfgs
} else {
// The only possible version in supportedWebhookVersions is v1,
// so use it for all versioned types in this context.
objRaw = &admissionregv1.ValidatingWebhookConfiguration{}
objRaw.SetName("validating-webhook-configuration")
}
objRaw.SetGroupVersionKind(schema.GroupVersionKind{
Group: admissionregv1.SchemeGroupVersion.Group,
Version: version,
Kind: "ValidatingWebhookConfiguration",
})
objRaw.SetName("validating-webhook-configuration")
objRaw.Webhooks = cfgs

for i := range objRaw.Webhooks {
// SideEffects is required in admissionregistration/v1, if this is not set or set to `Some` or `Known`,
// return an error
Expand Down
85 changes: 85 additions & 0 deletions pkg/webhook/parser_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
By("setting up the parser")
reg := &markers.Registry{}
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())

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

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

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

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

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

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

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

By("requesting that the manifest be generated")
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
Expand All @@ -326,6 +334,83 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
err = webhook.Generator{}.Generate(genCtx)
Expect(err).To(HaveOccurred())
})

It("should properly generate the webhook definition when a name is specified with the `kubebuilder:webhookconfiguration` marker", func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expect(err).NotTo(HaveOccurred())
Expect(os.Chdir("./testdata/valid-custom-name")).To(Succeed()) // go modules are directory-sensitive
defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()

By("loading the roots")
pkgs, err := loader.LoadRoots(".")
Expect(err).NotTo(HaveOccurred())
Expect(pkgs).To(HaveLen(1))

By("setting up the parser")
reg := &markers.Registry{}
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())

By("requesting that the manifest be generated")
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(outputDir)
genCtx := &genall.GenerationContext{
Collector: &markers.Collector{Registry: reg},
Roots: pkgs,
OutputRule: genall.OutputToDirectory(outputDir),
}
Expect(webhook.Generator{}.Generate(genCtx)).To(Succeed())
for _, r := range genCtx.Roots {
Expect(r.Errors).To(HaveLen(0))
}

By("loading the generated v1 YAML")
actualFile, err := os.ReadFile(path.Join(outputDir, "manifests.yaml"))
Expect(err).NotTo(HaveOccurred())
actualMutating, actualValidating := unmarshalBothV1(actualFile)

By("loading the desired v1 YAML")
expectedFile, err := os.ReadFile("manifests.yaml")
Expect(err).NotTo(HaveOccurred())
expectedMutating, expectedValidating := unmarshalBothV1(expectedFile)

By("comparing the two")
assertSame(actualMutating, expectedMutating)
assertSame(actualValidating, expectedValidating)
})

It("should fail to generate when there are multiple `kubebuilder:webhookconfiguration` markers of the same mutation type", func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expect(err).NotTo(HaveOccurred())
Expect(os.Chdir("./testdata/invalid-multiple-webhookconfigurations")).To(Succeed()) // go modules are directory-sensitive
defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()

By("loading the roots")
pkgs, err := loader.LoadRoots(".")
Expect(err).NotTo(HaveOccurred())
Expect(pkgs).To(HaveLen(1))

By("setting up the parser")
reg := &markers.Registry{}
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())
Expect(reg.Register(webhook.WebhookConfigDefinition)).To(Succeed())

By("requesting that the manifest be generated")
outputDir, err := os.MkdirTemp("", "webhook-integration-test")
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(outputDir)
genCtx := &genall.GenerationContext{
Collector: &markers.Collector{Registry: reg},
Roots: pkgs,
OutputRule: genall.OutputToDirectory(outputDir),
}
err = webhook.Generator{}.Generate(genCtx)
Expect(err).To(HaveOccurred())
})

})

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

0 comments on commit c69110a

Please sign in to comment.