Skip to content

✨ Add namespaceSelector and objectSelector to webhook markers #1339

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.

Sign up for GitHub

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

Open
camilamacedo86 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

✨ Add namespaceSelector and objectSelector to webhook markers #1339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
75 changes: 75 additions & 0 deletions pkg/webhook/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ limitations under the License.
package webhook

import (
"encoding/json"
"fmt"
"slices"
"strings"
Expand Down Expand Up @@ -159,6 +160,26 @@ type Config struct {
// The URL configuration should be between quotes.
// `url` cannot be specified when `path` is specified.
URL string `marker:"url,optional"`

// NamespaceSelector limits which namespaces trigger this webhook. The webhook runs only for
// requests in namespaces that match the selector. Value is a JSON object with the same shape
// as the Kubernetes LabelSelector (matchLabels and/or matchExpressions).
//
// Example:
//
// // +kubebuilder:webhook:...,namespaceSelector=`{"matchLabels":{"webhook-enabled":"true"}}`
// // +kubebuilder:webhook:...,namespaceSelector=`{"matchExpressions":[{"key":"environment","operator":"In","values":["dev","staging","prod"]}]}`
NamespaceSelector string `marker:"namespaceSelector,optional"`
Copy link
Member

@alvaroaleman alvaroaleman Feb 25, 2026

Choose a reason for hiding this comment

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

Can you instead of these two just add a patch marker that contains a strategic merge patch that will be applied over the rest of the webhook so any current and future field can be adjusted that way?


// ObjectSelector limits which objects trigger this webhook. The webhook runs only for requests
// whose object matches the selector. Value is a JSON object with the same shape as the
// Kubernetes LabelSelector (matchLabels and/or matchExpressions).
//
// Example:
//
// // +kubebuilder:webhook:...,objectSelector=`{"matchLabels":{"managed-by":"my-operator"}}`
// // +kubebuilder:webhook:...,objectSelector=`{"matchExpressions":[{"key":"app-type","operator":"In","values":["web","api","worker"]}]}`
ObjectSelector string `marker:"objectSelector,optional"`
}

// verbToAPIVariant converts a marker's verb to the proper value for the API.
Expand All @@ -180,6 +201,26 @@ func verbToAPIVariant(verbRaw string) admissionregv1.OperationType {
}
}

// parseLabelSelector parses a JSON string into a LabelSelector. The JSON must match the
// Kubernetes LabelSelector type (matchLabels and/or matchExpressions). It returns nil for empty input.
func parseLabelSelector(selectorStr string) (*metav1.LabelSelector, error) {
selectorStr = strings.TrimSpace(selectorStr)
if selectorStr == "" {
return nil, nil
}

var selector metav1.LabelSelector
if err := json.Unmarshal([]byte(selectorStr), &selector); err != nil {
return nil, fmt.Errorf("label selector must be valid JSON (e.g. {\"matchLabels\":{\"key\":\"value\"}}): %w", err)
}

if selector.MatchLabels == nil && len(selector.MatchExpressions) == 0 {
return nil, fmt.Errorf("label selector must specify at least one of matchLabels or matchExpressions")
}

return &selector, nil
}

// ToMutatingWebhookConfiguration converts this WebhookConfig to its Kubernetes API form.
func (c WebhookConfig) ToMutatingWebhookConfiguration() (admissionregv1.MutatingWebhookConfiguration, error) {
if !c.Mutating {
Expand Down Expand Up @@ -222,6 +263,16 @@ func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) {
return admissionregv1.MutatingWebhook{}, err
}

namespaceSelector, err := c.namespaceSelector()
if err != nil {
return admissionregv1.MutatingWebhook{}, fmt.Errorf("invalid namespaceSelector: %w", err)
}

objectSelector, err := c.objectSelector()
if err != nil {
return admissionregv1.MutatingWebhook{}, fmt.Errorf("invalid objectSelector: %w", err)
}

return admissionregv1.MutatingWebhook{
Name: c.Name,
Rules: c.rules(),
Expand All @@ -232,6 +283,8 @@ func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) {
TimeoutSeconds: c.timeoutSeconds(),
AdmissionReviewVersions: c.AdmissionReviewVersions,
ReinvocationPolicy: c.reinvocationPolicy(),
NamespaceSelector: namespaceSelector,
ObjectSelector: objectSelector,
}, nil
}

Expand All @@ -251,6 +304,16 @@ func (c Config) ToValidatingWebhook() (admissionregv1.ValidatingWebhook, error)
return admissionregv1.ValidatingWebhook{}, err
}

namespaceSelector, err := c.namespaceSelector()
if err != nil {
return admissionregv1.ValidatingWebhook{}, fmt.Errorf("invalid namespaceSelector: %w", err)
}

objectSelector, err := c.objectSelector()
if err != nil {
return admissionregv1.ValidatingWebhook{}, fmt.Errorf("invalid objectSelector: %w", err)
}

return admissionregv1.ValidatingWebhook{
Name: c.Name,
Rules: c.rules(),
Expand All @@ -260,6 +323,8 @@ func (c Config) ToValidatingWebhook() (admissionregv1.ValidatingWebhook, error)
SideEffects: c.sideEffects(),
TimeoutSeconds: c.timeoutSeconds(),
AdmissionReviewVersions: c.AdmissionReviewVersions,
NamespaceSelector: namespaceSelector,
ObjectSelector: objectSelector,
}, nil
}

Expand Down Expand Up @@ -402,6 +467,16 @@ func (c Config) reinvocationPolicy() *admissionregv1.ReinvocationPolicyType {
return &reinvocationPolicy
}

// namespaceSelector returns the LabelSelector for the webhook's namespace filter, or nil if unset.
func (c Config) namespaceSelector() (*metav1.LabelSelector, error) {
return parseLabelSelector(c.NamespaceSelector)
}

// objectSelector returns the LabelSelector for the webhook's object filter, or nil if unset.
func (c Config) objectSelector() (*metav1.LabelSelector, error) {
return parseLabelSelector(c.ObjectSelector)
}

// webhookVersions returns the target API versions of the {Mutating,Validating}WebhookConfiguration objects for a webhook.
func (c Config) webhookVersions() ([]string, error) {
// If WebhookVersions is not specified, we default it to `v1`.
Expand Down
140 changes: 140 additions & 0 deletions pkg/webhook/parser_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,146 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
Expect(err).To(HaveOccurred())
})

It("should properly generate webhook definition with namespaceSelector", func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expect(err).NotTo(HaveOccurred())
Expect(os.Chdir("./testdata/valid-namespaceselector")).To(Succeed())
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())
actualManifest := &admissionregv1.ValidatingWebhookConfiguration{}
Expect(yaml.UnmarshalStrict(actualFile, actualManifest)).To(Succeed())

By("loading the desired v1 YAML")
expectedFile, err := os.ReadFile("manifests.yaml")
Expect(err).NotTo(HaveOccurred())
expectedManifest := &admissionregv1.ValidatingWebhookConfiguration{}
Expect(yaml.UnmarshalStrict(expectedFile, expectedManifest)).To(Succeed())

By("comparing the two")
assertSame(actualManifest, expectedManifest)
})

It("should properly generate webhook definition with objectSelector", func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expect(err).NotTo(HaveOccurred())
Expect(os.Chdir("./testdata/valid-objectselector")).To(Succeed())
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())
actualManifest := &admissionregv1.MutatingWebhookConfiguration{}
Expect(yaml.UnmarshalStrict(actualFile, actualManifest)).To(Succeed())

By("loading the desired v1 YAML")
expectedFile, err := os.ReadFile("manifests.yaml")
Expect(err).NotTo(HaveOccurred())
expectedManifest := &admissionregv1.MutatingWebhookConfiguration{}
Expect(yaml.UnmarshalStrict(expectedFile, expectedManifest)).To(Succeed())

By("comparing the two")
assertSame(actualManifest, expectedManifest)
})

It("should properly generate webhook definition with matchExpressions in selectors", func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expect(err).NotTo(HaveOccurred())
Expect(os.Chdir("./testdata/valid-selectors-matchexpressions")).To(Succeed())
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)
})

})

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