Update ValidatingWebhook for Ingress to support --dry-run=server

[towolf]

What this PR does / why we need it:

Since the feature server dry run was introduced in Kubernetes, webhooks that potentially have "side-effects" in their operation are not executed during a "dry run". All webhooks have to declare that they do not trigger side effects behind the scenes to be able to run. Otherwise this error is returned:

Error from server (BadRequest): admission webhook "validate.nginx.ingress.kubernetes.io" does not support dry run

I declare no side effects but would like to confirm with the author of the webhook, if this is actually the case.

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)

Which issue/s this PR fixes

fixes #4767

How Has This Been Tested?

Edited my own ValidatingWebhookConfiguration locally to read sideEffects: None and the error disappears.

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]

Hi @towolf. 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.

[towolf]

@tjamet since you introduced this feature in #3802, could you review this PR in terms of the information from https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#side-effects

[cmluciano]

Can you please update the e2e tests in https://github.com/kubernetes/ingress-nginx/tree/master/charts/ingress-nginx/ci or https://github.com/kubernetes/ingress-nginx/tree/master/internal/admission/controller so that we can test the dry-run flag?

[towolf]

Can you please update the e2e tests in https://github.com/kubernetes/ingress-nginx/tree/master/charts/ingress-nginx/ci or https://github.com/kubernetes/ingress-nginx/tree/master/internal/admission/controller so that we can test the dry-run flag?

@cmluciano I looked at the tests and I'm not sure how the tests would capture the scenario, since that is taking place entirely outside of the controller on the side of Kubernetes. Unless we want to handle the case of a dry-run differently from the regular case. And from my cursory glance at how the admission works, this should happen just the same in the dry and non-dry case.

Or maybe I failed to recognize what we should test here and how?

[aledbf]

/ok-to-test

[aledbf]

/lgtm
/approve

[aledbf]

@towolf thanks!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, towolf

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:

• charts/ingress-nginx/OWNERS [aledbf]

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