Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-check incorrectly highlighting IST0169 #50243

Closed
2 tasks done
Stono opened this issue Apr 4, 2024 · 4 comments · Fixed by #50254
Closed
2 tasks done

Pre-check incorrectly highlighting IST0169 #50243

Stono opened this issue Apr 4, 2024 · 4 comments · Fixed by #50254

Comments

@Stono
Copy link
Contributor

Stono commented Apr 4, 2024

Is this the right place to submit this?

  • This is not a security vulnerability or a crashing bug
  • This is not a question about how to use Istio

Bug Description

Using the new precheck command we get:

Warning [IST0169] (DestinationRule zendesk-controller/zendesk) The configuration "VERIFY_CERTIFICATE_AT_CLIENT" changed in release 1.20: previously, TLS verification was skipped. Set `insecureSkipVerify` if this behavior is desired. Or, install with `--set compatibilityVersion=1.20` to retain the old default.

On our destinationrule we explicitly set:

      tls:
        caCertificates: /etc/ssl/certs/ca-certificates.crt
        mode: SIMPLE
        sni: whatever.zendesk.com

My understanding was that this implicitly enabled TLS verification, so this warning feels incorrect?

Version

1.21.0

Additional Information

No response

@howardjohn
Copy link
Member

checkVerify := func(tls *networking.ClientTLSSettings) bool {
return tls != nil && tls.CaCertificates == "" && tls.CredentialName == "" &&
tls.Mode != networking.ClientTLSSettings_ISTIO_MUTUAL && !tls.InsecureSkipVerify.GetValue()
}
is the logic. I would think that is NOT impacted and should not log.

However, the TLS config can be in 4 places: {main,subset}x{global,port level}.. maybe one of them? Do you have the full DR so I can test?

@howardjohn
Copy link
Member

$ ./istio-1.21.0/bin/istioctl x precheck  --from-version 1.20
✔ No issues found when checking the cluster. Istio is safe to install or upgrade!
  To get started, check out https://istio.io/latest/docs/setup/getting-started/

with

apiVersion: networking.istio.io/v1alpha3
kind: DestinationRule
metadata:
  name: bookinfo-ratings
spec:
  host: ratings.prod.svc.cluster.local
  trafficPolicy:
    loadBalancer:
      simple: LEAST_REQUEST
    tls:
      caCertificates: /etc/ssl/certs/ca-certificates.crt
      mode: SIMPLE
      sni: whatever.zendesk.com

@Stono
Copy link
Contributor Author

Stono commented Apr 4, 2024

Ah it's because it's a complex DR (as we're doing tls origination) under portLevelSettings:

apiVersion: networking.istio.io/v1beta1
kind: DestinationRule
metadata:
  name: zendesk
  namespace: zendesk-controller
spec:
  exportTo:
  - .
  host: autotraderukpreprod.zendesk.com
  trafficPolicy:
    connectionPool:
      http:
        h2UpgradePolicy: DO_NOT_UPGRADE
      tcp:
        connectTimeout: 2s
        maxConnectionDuration: 5m
        tcpKeepalive:
          interval: 30s
          time: 30s
    portLevelSettings:
    - connectionPool:
        http:
          h2UpgradePolicy: DO_NOT_UPGRADE
        tcp:
          connectTimeout: 2s
          maxConnectionDuration: 5m
          tcpKeepalive:
            interval: 30s
            time: 30s
      port:
        number: 443
      tls:
        mode: DISABLE
    - connectionPool:
        http:
          h2UpgradePolicy: DO_NOT_UPGRADE
        tcp:
          connectTimeout: 2s
          maxConnectionDuration: 5m
          tcpKeepalive:
            interval: 30s
            time: 30s
      port:
        number: 80
      tls:
        caCertificates: /etc/ssl/certs/ca-certificates.crt
        mode: SIMPLE
        sni: autotraderukpreprod.zendesk.com

@howardjohn
Copy link
Member

thanks! i'll send a fix

howardjohn added a commit to howardjohn/istio that referenced this issue Apr 4, 2024
istio-testing pushed a commit to istio-testing/istio that referenced this issue Apr 12, 2024
istio-testing added a commit that referenced this issue Apr 12, 2024
fixes #50243

Co-authored-by: John Howard <john.howard@solo.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants