Admission Webhook warns about faulty config, but does not block patching the configMap

[inge4pres]

What happened: Sending a patch with a faulty configuration in the ingress-nginx-controller configMap resulted in a Deployment being upgraded, despite the [emerg] message from the tested config.

What you expected to happen: The patch request should've been stopped by ValidatingAdmissionWebhook.

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

-------------------------------------------------------------------------------
NGINX Ingress controller
  Release:       v1.2.1
  Build:         08848d69e0c83992c89da18e70ea708752f21d7a
  Repository:    https://github.com/kubernetes/ingress-nginx
  nginx version: nginx/1.19.10

-------------------------------------------------------------------------------

Kubernetes version (use kubectl version):

version.Info{Major:"1", Minor:"22+", GitVersion:"v1.22.9-eks-a64ea69", GitCommit:"540410f9a2e24b7a2a870ebfacb3212744b5f878"

Environment:

• Cloud provider or hardware configuration: AWS / EKS

• OS (e.g. from /etc/os-release): Amazon Linux 2

• Kernel (e.g. uname -a): 5.4.117-58.216.amzn2.x86_64

• Install tools:

  • EKS

• How was the ingress-nginx-controller installed: -ingress-nginx ingress 31 2022-07-11 18:18:47.349848726 +0200 CEST deployed ingress-nginx-4.1.4 1.2.1

  • Helm values ( ❗ faulty configuration ❗)

    controller:
    admissionWebhooks:
    patch:
    tolerations:
    - effect: NoSchedule
    key: node-role.kubernetes.io/master
    operator: Equal
    value: ""
    config:
    enable-real-ip: "true"
    gzip-level: 5
    gzip-types: |
    text/css text/xml text/plain application/javascript application/x-javascript
    application/json application/atom+xml application/rss+xml application/font-woff
    application/x-web-app-manifest+json application/xhtml+xml application/xml
    http-snippet: |
    map $http_projectid $should_mirror {
     default   "";
     1         "";
     2         "";
     2         "1";
    }
    load-balance: ewma
    proxy-connect-timeout: 5
    proxy-next-upstream: error timeout http_502 non_idempotent
    proxy-next-upstream-tries: 5
    proxy-read-timeout: 10
    proxy-real-ip-cidr: <REDACTED>
    use-gzip: true
    use-http2: true
    ingressClass: nginx
    ingressClassResource:
    default: true

• Current State of the controller:

  • kubectl describe ingressclasses

Name:         nginx
Labels:       app.kubernetes.io/component=controller
              app.kubernetes.io/instance=ingress-nginx
              app.kubernetes.io/managed-by=Helm
              app.kubernetes.io/name=ingress-nginx
              app.kubernetes.io/part-of=ingress-nginx
              app.kubernetes.io/version=1.2.1
              helm.sh/chart=ingress-nginx-4.1.4
Annotations:  ingressclass.kubernetes.io/is-default-class: true
              meta.helm.sh/release-name: ingress-nginx
              meta.helm.sh/release-namespace: ingress
Controller:   k8s.io/ingress-nginx

All services and Pods are OK after a rollback to a sane configuration.

• Others:
  • Events from the webhook

    2022/07/11 14:22:41 [emerg] 2068#2068: conflicting parameter "2" in /tmp/nginx/nginx-cfg3575201305:271
    nginx: [emerg] conflicting parameter "2" in /tmp/nginx/nginx-cfg3575201305:271
    nginx: configuration file /tmp/nginx/nginx-cfg3575201305 test failed

How to reproduce this issue:

1. setup the configMap, configure the ingress-nginx-controller configMap with the custom http-snippet above, setting the same map key with 2 different values (this can happen when the list of keys is long, and its invalid in nginx.conf)
2. watch the admission webhook accept the config through a patch op, and the configmap-reloader try to reload the faulty config
3. if you really want to break the service, run a rollout restart deploy ingress-nginx-controller in the namespace where the ingress is deployed
4. watch the ingress Pods go into CrashLoopBackoff state, because the configuration is invalid

Anything else we need to know: The validating admission webhook is deployed, works perfectly and does its job when passing in a similar faulty configuration in a networking.k8s.io/ingress resource, but it doesn't when the configMap has it.

I asked about the support for testing configMap changes in the Kubernetes Slack

[k8s-ci-robot]

@inge4pres: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[longwuyuan]

Lets wait for other comments, but I would think that configMap is not part of the ingress api so would the admissionWebhook validate configmaps. Or is it expected to. And usually faulty values yaml changes are addressed with a helm upgrade execution with the corrected values.yaml (my opinion)

/remove-kind bug

[inge4pres]

good day @longwuyuan 😄 thanks for the reply, I had a similar memory of the webhook not working on the configMap, but I was suggested in Slack to raise an issue anyway 🤷🏼 it would be nice though if those changes could be intercepted by the webhook as well: I think it would be possible in theory to extend the webhook to check also a specific configMap?

[longwuyuan]

hello @inge4pres , the project has become difficult to maintain because developers are not available to support all the features. There was a announcement 2 weeks ago to freeze-feature development and focus on stabilization and bug fixing. So this feature can be looked at after 6 months. Sorry since its not expected result but there is a survey out to ask what users want and expect. You can respond to that as feedback so maintainers will get the message.

/kind feature

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[inge4pres]

/remove-lifecycle stale

[inge4pres]

/remove-lifecycle-stale

I still think this is an important enhancement for operating ingress-nginx, especially when iterating over global configs.

[longwuyuan]

hi @inge4pres , the volume of changes to the controller since November 2022 is too high to consider this issue at its face value.

Due to the security landscape now and the intention to move towards implementing the Gateway API, a undesired config test needs to be posted here with precise live real example data like the config yaml , the command used to insert it to the object , the logs the results etc etc. This has to be posted here with a test on the recent supported release of the controller.

I will close this issue for now just because its adding to the open issues tally without any trackable data. Please feel free to re-open the issue after you have posted data as I suggested in this post. Thanks

/close

[k8s-ci-robot]

@longwuyuan: Closing this issue.

In response to [this](https://github.com/kubernetes/ingress-nginx/issues/8818#issuecomment-2300664512): >hi @inge4pres , the volume of changes to the controller since November 2022 is too high to consider this issue at its face value. > >Due to the security landscape now and the intention to move towards implementing the Gateway API, a undesired config test needs to be posted here with precise live real example data like the config yaml , the command used to insert it to the object , the logs the results etc etc. This has to be posted here with a test on the recent supported release of the controller. > >I will close this issue for now just because its adding to the open issues tally without any trackable data. Please feel free to re-open the issue after you have posted data as I suggested in this post. Thanks > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.