istio / istio

Connect, secure, control, and observe services.
https://istio.io
Apache License 2.0
35.71k stars 7.69k forks source link

ValidationWebhook 'failurePolicy' is changed to 'Fail' after deployment #52785

Open nandar-p opened 3 weeks ago

nandar-p commented 3 weeks ago

Is this the right place to submit this?

Bug Description

When installing Istio version 1.22.1 with Helm, the validation webook is created with a failure policy of Fail.

$ kubectl get validatingwebhookconfiguration istio-validator-cluster -n cluster  
-o yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"admissionregistration.k8s.io/v1","kind":"ValidatingWebhookConfiguration","metadata":{"annotations":{},"labels":{"app":"istiod","argocd.argoproj.io/instance":"cluster","istio":"istiod","istio.io/rev":"default","release":"cluster"},"name":"istio-validator-cluster"},"webhooks":[{"admissionReviewVersions":["v1beta1","v1"],"clientConfig":{"service":{"name":"istiod","namespace":"cluster","path":"/validate"}},"failurePolicy":"Ignore","name":"rev.validation.istio.io","objectSelector":{"matchExpressions":[{"key":"istio.io/rev","operator":"In","values":["default"]}]},"rules":[{"apiGroups":["security.istio.io","networking.istio.io","telemetry.istio.io","extensions.istio.io"],"apiVersions":["*"],"operations":["CREATE","UPDATE"],"resources":["*"]}],"sideEffects":"None"}]}
    meta.helm.sh/release-name: cluster
    meta.helm.sh/release-namespace: cluster
  creationTimestamp: "2024-08-21T02:01:52Z"
  generation: 3642
  labels:
    app: istiod
    app.kubernetes.io/managed-by: Helm
    argocd.argoproj.io/instance: cluster
    istio: istiod
    istio.io/rev: default
    release: cluster
  name: istio-validator-cluster
  resourceVersion: "8515774"
  uid: 8ff1a57b-3743-415f-8bdd-d6be18d87724
webhooks:
- admissionReviewVersions:
  - v1beta1
  - v1
  clientConfig:
    caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMvRENDQWVTZ0F3SUJBZ0lRSGpWdlY0ZitXSHAzSWdlM0hzeHV6akFOQmdrcWhraUc5dzBCQVFzRkFEQVkKTVJZd0ZBWURWUVFLRXcxamJIVnpkR1Z5TG14dlkyRnNNQjRYRFRJME1EY3lOREExTlRneU9Wb1hEVE0wTURjeQpNakExTlRneU9Wb3dHREVXTUJRR0ExVUVDaE1OWTJ4MWMzUmxjaTVzYjJOaGJEQ0NBU0l3RFFZSktvWklodmNOCkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFNUzJqYWZOZElBd0l3Q0w4T1l3MHBCUTNNOHl0MkJkNUZ4Rml6RW0KeTdYQlJZbVhnTnJhY1BMSHErc1VhK2JHcDM2b2xyY2kydjJ2NE9Ba25JanBVcFRkaytZTmM1UGZMcXVmc3FtVQpZb0xIVXI3K01DTnhzdDljYzFqTWdubU9UalkreEJWZ2VvTGRKUzN6dzl4TzkxbmZmaVlkOU9KYjZLam5ObXRKCjdNTlJuZkM5TW53WUJFR3BMUEdWNjdrcUFYSFUvSmRzbXArK2R5WU54ai83Q2RPYk0yTVloL3huUHZJa3krS3kKY2lEdHVuR2RQNCt1UlJUTU1uVjlERXYzdlpLekJlV0l4bWRvbGtLUXd0LzVrSGpjY1VYclQ5ejFTbGtpd3YyVwplZ3V3aFFmMWZ4N1hieC8wZ1BvbFprbWJoUExub0dnYzdnYTQyemphTzJtMDBoMENBd0VBQWFOQ01FQXdEZ1lEClZSMFBBUUgvQkFRREFnSUVNQThHQTFVZEV3RUIvd1FGTUFNQkFmOHdIUVlEVlIwT0JCWUVGR0Z0bVdFdWxWYUgKYXRrNDgwWVBMbXNlS1FNWU1BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQlVKS1VpRENpdVdqNlBNVnl2alE5Ngp3NmZmNlh1MDh0cWx5dXRNdDBOeWFxbjJ5ODVoNXJRblZpZmRRWGswQ1NmSEZMeTVmSU13cjkvcnJQMkhySzdvCkVjZ3JTdHA2enVOWEZRWi9tU0Q3OTJIR0lsZ2xTUU52b3RXRGltUjBNTWNCQmxzSllRZmdJTDdpNzhJR3NBZDYKMC8xSUM5dHVISVo0Mkg3NmZrdk1pZmMwVUVnc1ZaOFFIV08vNmdpVGwxYUNkcU5KdmoyMk5pUjFXeGh5MXhqUwoxcndJbDdPZFVEdjlsSW5LNEtsdVRjb2ZWcGhrUlNiTHY0T215djJsSHlFNUVOaWpIcVhsWUpsanhNQkRuT1NHCnVXREVWQUhLcVlzSi9BMW5LOEFvVHp2V3NpSE5tak9BU0VGOFZpdnZKUGFzRi92ZkpYWk9tYk9CcHd6T0ZEYjIKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=
    service:
      name: istiod
      namespace: cluster
      path: /validate
      port: 443
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: rev.validation.istio.io
  namespaceSelector: {}
  objectSelector:
    matchExpressions:
    - key: istio.io/rev
      operator: In
      values:
      - default
  rules:
  - apiGroups:
    - security.istio.io
    - networking.istio.io
    - telemetry.istio.io
    - extensions.istio.io
    apiVersions:
    - '*'
    operations:
    - CREATE
    - UPDATE
    resources:
    - '*'
    scope: '*'
  sideEffects: None
  timeoutSeconds: 10

Expection

According to the following istio's helm templates, the failurePolicy should be Ignore if {{.Values.base.validationCABundle}} is empty. https://github.com/istio/istio/blob/release-1.22/manifests/charts/base/templates/default.yaml#L43

We expect failurePolicy not to be changed to Fail when we do not define {{.Values.base.validationCABundle}} in helm/values.yaml as follow.

helm/values.yaml

base:
  defaults:
    global:
      # Used to locate istiod.
      istioNamespace: cluster

Version

$ kubectl version
Client Version: v1.30.2
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.28.11-eks-db838b0

$ helm version --short
v3.15.2+g1a500d5

Additional Information

howardjohn commented 3 weeks ago

When you define validationCABundle, it is immediately set to fail. When you don't set it, we automatically update it.

There is no option to keep it to Ignore permanently because it is unsafe.

nandar-p commented 2 weeks ago

When you define validationCABundle, it is immediately set to fail. When you don't set it, we automatically update it.

There is no option to keep it to Ignore permanently because it is unsafe.

@howardjohn Thanks for your explanation. This helps me a lot.

If we deployed it using GitOps tool(Argocd), this tool noticed the differences and causes an OutOfSync error as shown in the following image. May I know is there any solution to solve this differences in Argocd? istio-validator-cluster

zirain commented 2 weeks ago

cc @hanxiaop

hanxiaop commented 2 weeks ago

@nandar-p you can try using https://argo-cd.readthedocs.io/en/stable/user-guide/diffing/ to ignore the differences. Or server-side apply with managed fields may work as well, but that would require changes to Istio.

oliverpark999 commented 2 weeks ago

@nandar-p you can try using https://argo-cd.readthedocs.io/en/stable/user-guide/diffing/ to ignore the differences. Or server-side apply with managed fields may work as well, but that would require changes to Istio.

I have the same problem {ArgoCD}. I hate seeing OutOfSync.. I added the below values to istio chart but it doesn't work.

spec:
  ignoreDifferences:
    - group: admissionregistration.k8s.io
      kind: ValidatingWebhookConfiguration
      name: istiod-default-validator
      jqPathExpressions:
        - .webhooks[] | select(.name == "validation.istio.io") | .failurePolicy
zirain commented 2 weeks ago

can you try with https://istio.slack.com/archives/C37A4KAAD/p1724650498576219?thread_ts=1724650124.044929&cid=C37A4KAAD

nandar-p commented 1 week ago

@nandar-p you can try using https://argo-cd.readthedocs.io/en/stable/user-guide/diffing/ to ignore the differences. Or server-side apply with managed fields may work as well, but that would require changes to Istio.

Thank you @hanxiaop I will try it..