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

Why set namespace with kustomize for cluster-scoped Mutating|ValidatingWebhookConfiguration? #3999

Closed
davidxia opened this issue Jul 2, 2024 · 4 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@davidxia
Copy link
Contributor

davidxia commented Jul 2, 2024

What broke? What's expected?

What "broke"

Why does kubebuilder use a namespace kustomization transformer to set a namespace for Mutating|ValidatingWebhookConfiguration if these are cluster-scoped resources? Code here

Isn't this unnecessary?

What's expected

IIUC, lines of code above shouldn't exist as they don't do anything?

Expected output

# the following config is for teaching kustomize where to look at when substituting vars.
# It requires kustomize v2.1.0 or newer to work properly.
nameReference:
- kind: Service
  version: v1
  fieldSpecs:
  - kind: MutatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/name
  - kind: ValidatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/name

varReference:
- path: metadata/annotations

Actual output

# the following config is for teaching kustomize where to look at when substituting vars.
# It requires kustomize v2.1.0 or newer to work properly.
nameReference:
- kind: Service
  version: v1
  fieldSpecs:
  - kind: MutatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/name
  - kind: ValidatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/name

namespace:
- kind: MutatingWebhookConfiguration
  group: admissionregistration.k8s.io
  path: webhooks/clientConfig/service/namespace
  create: true
- kind: ValidatingWebhookConfiguration
  group: admissionregistration.k8s.io
  path: webhooks/clientConfig/service/namespace
  create: true

varReference:
- path: metadata/annotations

Reproducing this issue

Run kubebuilder create webhook --group batch --version v1 --kind CronJob --defaulting --programmatic-validation as described in Implementing defaulting/validating webhooks.

KubeBuilder (CLI) Version

4.0.0

PROJECT version

3

Plugin versions

go.kubebuilder.io/v4

Other versions

go 1.22.2
sigs.k8s.io/controller-runtime v0.18.2

kubectl version
Client Version: v1.29.3
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.29.3-gke.1282000

Extra Labels

/kind cleanup

@davidxia davidxia added the kind/bug Categorizes issue or PR as related to a bug. label Jul 2, 2024
@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jul 2, 2024
@davidxia
Copy link
Contributor Author

davidxia commented Jul 2, 2024

created this issue at the request of kubernetes-sigs/controller-tools#999 (comment) despite the issue template saying to create issues in controller-tools 🤷‍♂️

@camilamacedo86
Copy link
Member

It is not related to controller-tools at all.
Since the scaffold was implemented in Kubebuilder it has the namespaces transforms.
See: a3e6e25

However, it seems that is a bug and the code is unnecessary.
To fix it is required to:

  • 1 : Change the kubebuilder template:
    const kustomizeConfigWebhookTemplate = `# the following config is for teaching kustomize where to look at when substituting nameReference.
    # It requires kustomize v2.1.0 or newer to work properly.
    nameReference:
    - kind: Service
    version: v1
    fieldSpecs:
    - kind: MutatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/name
    - kind: ValidatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/name
    namespace:
    - kind: MutatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/namespace
    create: true
    - kind: ValidatingWebhookConfiguration
    group: admissionregistration.k8s.io
    path: webhooks/clientConfig/service/namespace
    create: true
    `
  • 2 - Run make generate to ensure that all samples and docs will properly updated

Please, feel free to open PR with the change.

@davidxia
Copy link
Contributor Author

davidxia commented Jul 3, 2024

@camilamacedo86 thanks, I'll attempt!

@davidxia
Copy link
Contributor Author

davidxia commented Jul 3, 2024

Closing as it's not a bug. These lines are setting the K8s Service's namespace not that of the Mutating|ValidatingWebhookConfiguration.

These lines

namespace:
- kind: MutatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/namespace
create: true
- kind: ValidatingWebhookConfiguration
group: admissionregistration.k8s.io
path: webhooks/clientConfig/service/namespace
create: true
are needed to update the Mutating|ValidatingWebhookConfiguration's webhooks/0/clientConfig/service/namespace value
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
name: mutating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-crew-testproject-org-v1-admiral
with that from here (Not sure why the path in kustomizeconfig.yaml doesn't have an array index path segment.)

Without these lines, the namespace value will always be system.

@davidxia davidxia closed this as completed Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

No branches or pull requests

3 participants