Allow Helm Chart to customize admission webhook's annotations, timeoutSeconds, namespaceSelector, objectSelector and cert files locations

[kolorful]

This PR allows Helm Chart to customize admission webhook's annotations, timeoutSeconds, namespaceSelector, objectSelector and cert files locations.

What this PR does / why we need it:

This PR adds following capabilities to the ingress-nginx Helm Chart:

• Allow users to set annotations, timeoutSeconds, namespaceSelector and objectSelector on ValidatingWebhookConfiguration
• Allow users to customize values of --validating-webhook-certificate and --validating-webhook-key in Deployment and DaemontSet.
• Fixed a duplicated command line argument --namespace $NAMESPACE in generate-deploy-scripts.sh

Why:

• We use cert-manager to manage webhook certs and it relies on an annotation on the ValidatingWebhookConfiguration resource, which currently cannot be set.
• The other improvement is making timeoutSeconds customizable, since it's a super useful setting.
• I also made namespaceSelector and objectSelector configurable due to it could be handy in big clusters.
• The last improvement is that during testing, I found there is a a duplicated command line argument --namespace $NAMESPACE in generate-deploy-scripts.sh, so I just went ahead and fixed it.
  

  ingress-nginx/hack/generate-deploy-scripts.sh

  Line 56 in 9c94d77

  cat << EOF | helm template $RELEASE_NAME ${DIR}/charts/ingress-nginx --namespace $NAMESPACE --namespace $NAMESPACE --values - | $DIR/hack/add-namespace.py $NAMESPACE > ${OUTPUT_FILE}

• In general, it's nicer to be able to customize resource as much as possible.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

How Has This Been Tested?

• I ran the e2e test.
• I ran the e2e test for helm-charts.
• I ran generate-deploy-scripts.sh to make sure generated static yamls has the correct webhook related values.
• I also ran helm install -n ingress-nginx-internal --dry-run ingress-nginx ./charts/ingress-nginx/ before and after my change with different sets values and verified via the diff results that annotations and timeoutSeconds are added correctly when specified and are not added when not specified.

With new value like:

  admissionWebhooks:
    annotations:
      foo: bar
      hello: world
    enabled: true
    failurePolicy: Fail
    timeoutSeconds: 30
    port: 8443
    certificate: "/usr/local/certificates/tls.crt"
    key: "/usr/local/certificates/tls.key"
    namespaceSelector:
      matchLabels:
        needValidation: true
    objectSelector: 
      matchExpressions:
      - key: owner
        operator: NotIn
        values:
        - noValidation

The new outputs are:

1. in ValidatingWebhookConfiguration resource

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  annotations: 
    foo: bar
    hello: world
  labels:
    helm.sh/chart: ingress-nginx-3.5.0
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/instance: ingress-nginx
    app.kubernetes.io/version: "0.40.2"
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/component: admission-webhook
  name: ingress-nginx-admission
webhooks:
  - name: validate.nginx.ingress.kubernetes.io
    rules:
      - apiGroups:
          - networking.k8s.io
        apiVersions:
          - v1beta1
          - v1
        operations:
          - CREATE
          - UPDATE
        resources:
          - ingresses
    failurePolicy: Fail
    sideEffects: None
    admissionReviewVersions:
      - v1
    clientConfig:
      service:
        namespace: ingress-nginx-internal
        name: ingress-nginx-controller-admission
        path: /networking/v1beta1/ingresses
    timeoutSeconds: 30
    namespaceSelector:
      matchLabels:
        needValidation: true
    objectSelector: 
      matchExpressions:
      - key: owner
        operator: NotIn
        values:
        - noValidation

2. In Deployment or DaemonSet resource

         args:
            ...
            - --validating-webhook-certificate=/usr/local/certificates/tls.crt
            - --validating-webhook-key=/usr/local/certificates/tls.key

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added tests to cover my changes.
• All new and existing tests passed.

[k8s-ci-robot]

Welcome @kolorful!

It looks like this is your first PR to kubernetes/ingress-nginx 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/ingress-nginx has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @kolorful. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[aledbf]

/hold

[kolorful]

Hi @ChiefAlexander and @ElvinEfendi, it would be very appreciated if you could take a look at this code change :)

[kolorful]

/assign @ChiefAlexander

[ChiefAlexander]

Why'd you put this onhold @aledbf?

[aledbf]

Why'd you put this onhold @aledbf?

Because when I am preparing a release (last Friday) and for a couple of days (just in case a new version is required) all PRs ready to merge are put on hold.

[ChiefAlexander]

Why'd you put this onhold @aledbf?

Because when I am preparing a release (last Friday) and for a couple of days (just in case a new version is required) all PRs ready to merge are put on hold.

Interesting. If I lgtm would that override your hold?

[ChiefAlexander]

/ok-to-test

[aledbf]

Interesting. If I lgtm would that override your hold?

No, tide waits until all the conditions are met

[ChiefAlexander]

/lgtm
Thanks for the PR!

[kolorful]

Thank you!

[aledbf]

@kolorful, please increase the version of the chart. Otherwise, the change will not publish a new release. Thanks

[aledbf]

This line https://github.com/kubernetes/ingress-nginx/blob/master/charts/ingress-nginx/Chart.yaml#L3
(3.5.0)

[kolorful]

@aledbf rebased master, bumped chart version to 3.4.2 and squashed commits.

[aledbf]

@aledbf rebased master, bumped chart version to 3.4.2 and squashed commits.

This is a new feature. Please use 3.5.0

[kolorful]

updated to 3.5.0 :)

[aledbf]

/lgtm
/approve

[aledbf]

@kolorful thanks!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, ChiefAlexander, kolorful

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [aledbf]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[aledbf]

/hold cancel

[kolorful]

@aledbf seems like helm chart publishing failed due to Error: GET https://api.github.com/repos/kubernetes/ingress-nginx/releases/tags/ingress-nginx-3.5.0: 404 Not Found, do I need to do anything to fix it?

update: should be resolved in #6299

[aledbf]

@kolorful fixed