[admission-controller] Helm chart generates duplicate ValidatingWebhookConfiguration resources

[omaen]

Hi,

Previously we used terraform to deploy the sysdig-deploy helm chart to our cluster, but are now trying to move the setup to ArgoCD using Kustomize. This works for all our other charts, but it seems like the admission-controller sub-chart is generating multiple resources with the same name/id, which causes kustomization to fail with the following message

Error: may not add resource with an already registered id: admissionregistration.k8s.io_v1_ValidatingWebhookConfiguration|sysdig-agent|sysdig-cluster-agent-admissioncontroller-webhook

Our kustomization.yaml, and the values.yaml has set admissionController.enabled: true:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
helmCharts:
  - name: sysdig-deploy
    namespace: sysdig-agent
    repo: https://charts.sysdig.com/
    releaseName: sysdig-cluster-agent
    valuesFile: values.yaml
    version: "1.19.2"
    includeCRDs: true

Running helm template sysdig sysdig/sysdig-deploy --values values.yaml works, but I can see the two resources created.

# Source: sysdig-deploy/charts/admissionController/templates/webhook/admissionregistration.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  name: sysdig-admissioncontroller-webhook
  namespace: default
webhooks: []

and

# Source: sysdig-deploy/charts/admissionController/templates/webhook/admissionregistration.yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  name: sysdig-admissioncontroller-webhook
  namespace: default
  annotations:
    "helm.sh/hook": "post-install, post-upgrade"
    meta.helm.sh/release-name: sysdig
    meta.helm.sh/release-namespace: default
  labels:
    app.kubernetes.io/managed-by: Helm
webhooks:
- name: audit.secure.sysdig.com
  matchPolicy: Equivalent
  rules:
    <truncated>
  clientConfig:
    <truncated>
  admissionReviewVersions: ["v1", "v1beta1"]
  sideEffects: None
  timeoutSeconds: 5
  failurePolicy: Ignore

I don't know the reason for creating a empty ValidatingWebhookConfiguration and then overwriting it. It seemingly works with Helm, but causes problems for Kustomize. Let me know if you want me to open a PR with a suggested solution or not.

[mavimo]

Hi @omaen 👋

The reason is reported in #1217 but if you have better suggestion we will like to support also your use case.

[airadier]

Hi @omaen , as @mavimo said, reason is explained in #1217 . TLDR, we had to create ValidatingWebhookConfiguration in post-install hook to prevent it blocking the deployment of the Admission Controller itself. But using post-install hook, it was not part of the lifecycle, so it wasn't removed on uninstall, causing some issues.

The current approach, which works with helm, is creating an empty ValidatingWebhookConfiguration as part of the lifecycle, then adding the real webhook entries on post-install.

Now I see this can cause a conflict when rendering as a template, and then creating the resources. I wonder if "helm post-install" does an "apply", so it will update the resource, while ArgoCD + Kustomize might be doing a "create" instead, failing to update the existing resource.

I am open to suggestions. We might have a flag to control the behavior of the post-install hook. But please note that creating the ValidatingWebhookConfiguration along with all the other resources and not after the deployments, might cause delays or make the deployment fail, as it will try to validate them with an unreachable service.

[omaen]

Hi @mavimo and @airadier. Thanks for the explanation! 😃 A bit tricky to set up ValidatingWebhookConfiguration I understand. I'm no expert in either helm og kustomize, so I'll have to take a deeper dive to see if there are some other options that could work as a workaround.

Is the admission-controller dependant on other charts, or is it a standalone feature? Right now we are only using it for auditing-purposes, and not actually blocking any k8s actions. An alternative would be to install the admission-controller helm-chart separately from sysdig-deploy after the rest of the sysdig installation.

[airadier]

Hi @omaen , admission-controller chart does not have any dependencies. You can install it as a subchart of sysdig-deploy or deploy it as an independent chart.

Also, I can think of different workarounds to prevent the failure you describe:

• We could explore using kustomize in "declarative" way, using kubectl apply -k as described in https://kubernetes.io/docs/tasks/manage-kubernetes-objects/kustomization/. I think that would solve the issue, it shouldn't fail with the duplicate resource. But checking ArgoCD documentation I don't see a way to use the "declarative/apply" mode with kustomize
• We could add a flag to the helm template to prevent creating the resource as part of life-cycle and then in the post-install, which causes the duplication. If you are using helm to write the templates and then not applying with helm, lifecycle management is not relevant here.

Still, with the second approach you should be careful and try to create the Webhook registrations after deploying all the services. Otherwise might result in long delays while deploying.

Regards.

[omaen]

Hi @airadier,

If I'm not mistanken I believe Kustomize only works in a declarative way, and that may be the problem. As the underlying helm-chart produces an output that is dependent on mutating the object. Running kubectl kustomize --enable-helm also produces the following error:

error: may not add resource with an already registered id: ValidatingWebhookConfiguration.v1.admissionregistration.k8s.io/sysdig-cluster-agent-admissioncontroller-webhook.sysdig-agent

A flag to the helm chart to circumvent the issue should work for us, since ArgoCD will be managing the lifecycle of the resources and do a continuous repair. Although I'm a bit curious to how other helm charts with ValidatingWebhookConfigurations handles this.

Have you thought about excluding the namespace where the admission controller is installed from the ValidatingWebhookConfiguration? From the Kubernetes docs on avoiding dead-locks in self hosted webhooks it says

It is recommended to exclude the namespace where your webhook is running with a namespaceSelector.

https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#avoiding-deadlocks-in-self-hosted-webhooks

For example by adding something like this to the webhook template:

  webhooks:
    # Exclude namespaces
    - namespaceSelector:
        matchExpressions:
        - key: kubernetes.io/metadata.name
          operator: NotIn
          values:
            - {{ include "admissionController.namespace" . }}

[airadier]

Should be addressed by #1316 . Waiting for approvals and merge

[omaen]

Should be addressed by #1316 . Waiting for approvals and merge

Awesome! Any idea when @airadier?

[airadier]

Should be addressed by #1316 . Waiting for approvals and merge

Awesome! Any idea when @airadier?

I am waiting for approval from the agent team and it should be automatically merged. CC @mavimo for a review here.

[mavimo]

@omaen @airadier can we close this issue?

[omaen]

@omaen @airadier can we close this issue?

The solution is working for us, so it's fine by me :)