validatingwebhook doesn't work as expected when enableAnnotationValidations is set to true

srikiz commented 10 months ago

What happened:

Create the below bad-ingress with proxy-next-upstream set to 300. It creates an ingress object even though the proxy-next-upstream value is in-correct.

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: bad-ingress
  namespace: default
  annotations:
    nginx.ingress.kubernetes.io/proxy-body-size: "0"
    nginx.ingress.kubernetes.io/proxy-next-upstream: "300"
spec:
  rules:
  - host: bad-ingress.k8s.mydomain
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: bad-ingress-service
            port:
              number: 8888

This issue occurs only when enableAnnotationValidations is set to true.

What you expected to happen:

Ideally, the validating webhook should reject creating this ingress object. Looks like the additional annotation validation that's introduced in v1.9.0 is not validating the errors as expected.

 kubectl apply -f bad-ingress.yaml
Error from server (BadRequest): error when creating "bad-ingress.yaml": admission webhook "validate.nginx.ingress.kubernetes.io" denied the request:
-------------------------------------------------------------------------------
Error: exit status 1
2023/11/23 08:15:04 [emerg] 297#297: invalid value "300" in /tmp/nginx/nginx-cfg910596062:568
nginx: [emerg] invalid value "300" in /tmp/nginx/nginx-cfg910596062:568
nginx: configuration file /tmp/nginx/nginx-cfg910596062 test failed

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

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

NGINX Ingress controller Release: v1.9.4 Build: 846d251814a09d8a5d8d28e2e604bfc7749bcb49 Repository: https://github.com/kubernetes/ingress-nginx nginx version: nginx/1.21.6

Kubernetes version (use kubectl version):

kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.3", GitCommit:"25b4e43193bcda6c7328a6d147b1fb73a33f1598", GitTreeState:"clean", BuildDate:"2023-06-14T09:53:42Z", GoVersion:"go1.20.5", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v5.0.1

Server Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.4", GitCommit:"fa3d7990104d7c1f16943a67f11b154b71f6a132", GitTreeState:"clean", BuildDate:"2023-07-19T12:14:49Z", GoVersion:"go1.20.6", Compiler:"gc", Platform:"linux/amd64"}

Environment: On-Prem installed via kubeadm

How was the ingress-nginx-controller installed:

helm list -n ingress-nginx
NAME            NAMESPACE       REVISION        UPDATED                                 STATUS          CHART                   APP VERSION
test-ingress     ingress-nginx   6               2023-11-23 01:47:48.101692406 +0530 IST deployed        ingress-nginx-4.8.3     1.9.4

k8s-ci-robot commented 10 months ago

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 commented 9 months ago

I think the webhookvalidation and the annotation value string validation are different code-paths and I guess non test is written to check for a value like 300. Probable that no upstream can be named as 300

longwuyuan commented 9 months ago

cc @rikatz @tao12345666333

nguyenthai0107 commented 8 months ago

Hello @srikiz Is that issue resolved ? thanks

jrhunger commented 8 months ago

Hello @srikiz Is that issue resolved ? thanks

Is a resolution expected in 1.9.5? I didn't my see a linked PR so we haven't tested 1.9.5 yet.

nguyenthai0107 commented 8 months ago

Hello everyone look like i facing off with enableAnnotationValidations too. please take a look

depend on CVE-2021-25742 ( This issue has been rated High ) but when I edited annotations-risk-level: High and set --enable-annotation-validation=true , look like this wrong and cannot create ingress object. Im not sure CVE-2021-25742 was rated correctly. Here are details

Configmap Ingress Nginx Controller

apiVersion: v1
kind: ConfigMap
metadata:
name: ingress-nginx-controller
namespace: ingress-nginx
data:
allow-snippet-annotations: "true"
annotations-risk-level: High

Deployment Ingress Nginx Controller

apiVersion: apps/v1
kind: Deployment
metadata:
name: ingress-nginx-controller
namespace: ingress-nginx
spec:
selector:
matchLabels:
  app.kubernetes.io/name: ingress-nginx
  app.kubernetes.io/instance: platform
  app.kubernetes.io/component: controller
replicas: 5
revisionHistoryLimit: 10
minReadySeconds: 0
template:
spec:
  containers:
  - name: controller
    image: registry.k8s.io-ingress-nginx-controller:v1.9.5
    imagePullPolicy: IfNotPresent
    lifecycle:
      preStop:
        exec:
          command:
          - /wait-shutdown
    args:
    - /nginx-ingress-controller
    - **--enable-annotation-validation=true**
    - --publish-service=$(POD_NAMESPACE)/ingress-nginx-controller
    - --election-id=ingress-nginx-leader
    - --controller-class=k8s.io/ingress-nginx
    - --ingress-class=nginx
    - --configmap=$(POD_NAMESPACE)/ingress-nginx-controller
    - --validating-webhook=:8443
    - --validating-webhook-certificate=/usr/local/certificates/cert
    - --validating-webhook-key=/usr/local/certificates/key
    - --enable-ssl-passthrough=true

Deploy app with Ingress object which got issue. Exactly app is deploy Kiali inside namespace Istio

ingress:
  class_name: "nginx"
  enabled: true
  # default: override_yaml is undefined
  override_yaml:
    metadata:
      annotations:
          nginx.ingress.kubernetes.io/configuration-snippet: |
          proxy_set_header "X-B3-Sampled" "1";
    spec:
      rules:
      - host: kiali.example.com
        http:
          paths:
          - backend:
              service:
                name: kiali
                port:
                  number: 20001
            path: /
            pathType: Prefix
      tls:
      - hosts:
        - kiali.example.com
        secretName: "kiali-tls"`

Components Used: Nginx Ingress Controller 1.9.5 Helm version 4.9.0 Kiali Operator v1.77.0

`/nginx-ingress-controller 
-------------------------------------------------------------------------------
NGINX Ingress controller
Release:       v1.9.5
Repository:    https://github.com/kubernetes/ingress-nginx
nginx version: nginx/1.21.6

Here is the issue which use command line kubectl describe kiali -n istio and ingress of Kiali wasn't created.

Status:
Conditions:
Message:               
Reason:                
Status:                False
Type:                  Successful
Message:               Running reconciliation
Reason:                Running
Status:                False
Type:                  Running
Ansible Result:
Changed:             5
Failures:            1
Ok:                  64
Skipped:             51
Message:               Failed to patch object: b'{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"admission webhook \\"validate.nginx.ingress.kubernetes.io\\" denied the request: annotation group ConfigurationSnippet contains risky annotation based on ingress configuration","reason":"BadRequest","code":400}\n'
Reason:                Failed
Status:                True
Type:                  Failure
Deployment:
Instance Name:  kiali
Namespace:      istio
Environment:
Is Kubernetes:       true
Kubernetes Version:  1.24.17
Operator Version:    v1.77.0
Progress:
Duration:    0:00:26
Message:     5. Creating core resources
Spec Version:  default
Events:          <none>

But when set to Critical or delete annotations-risk-level in configmap, the issue was gone and working fine. Just got stuck when set to High, Medium or Low. So please kindly for take a look. Thank you guys.

kubernetes / ingress-nginx

validatingwebhook doesn't work as expected when enableAnnotationValidations is set to true #10672