Virtual service routes ignored when httpsRedirect: true in port 443 section

[ceastman-ibm]

(NOTE: This is used to report product bugs:
To report a security vulnerability, please visit https://istio.io/about/security-vulnerabilities/
To ask questions about how to use Istio, please visit https://discuss.istio.io
)

Bug description

Affected product area (please put an X in all that apply)

[ ] Docs
[ ] Installation
[X] Networking
[ ] Performance and Scalability
[ ] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Affected features (please put an X in all that apply)

[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane

Expected behavior
Don't ignore virtual service paths when https redirect is set (incorrectly on port 443). Just warn on the httpsredirect being set on port 443, since your already https you don.t need to redirect

Steps to reproduce the bug

probably related to #24958

in the gateway.yaml: have httpsRedirect:true on port 443

spec:
  selector:
    istio: ingressgateway
  servers:
  - port:
      number: 443
      name: https
      protocol: HTTPS
    hosts:
    - hostname
    tls:
      httpsRedirect: true
      mode: SIMPLE
      credentialName: sslSecretName

Version (include the output of istioctl version --remote and kubectl version --short and helm version if you used Helm)

istioctl version
client version: 1.7.0
control plane version: 1.7.0
data plane version: 1.7.0 (561 proxies), 1.6.8 (2 proxies)

kubectl version --short
Client Version: v1.17.7
Server Version: v1.17.11+IKS

How was Istio installed?
IBM managed istio

Environment where bug was observed (cloud vendor, OS, etc)
IBM Cloud

Additionally, please consider attaching a cluster state archive by attaching
the dump file to this issue.

[linsun]

@ceastman-ibm thanks for discovering/reporting this. I imagine it can take you a few hours to track this down.

@howardjohn @rshriram is this as designed, users should not be doing httpsredirect... and we should warn user properly on this? Seems good to fail at validation time except users may already have this (coming from 1.6 to 1.7), which they will need to run analyzer to discover it.

cc @esnible

[howardjohn]

I need to look into it a bit more but we can likely either fail in the webhook or add a warning if it's set. If we go with warning we should make the code just ignore it instead of break

[ceastman-ibm]

@linsun users should be able to do httsredirect. It just needs to be on port 80.

I had it set on port 443 which was wrong (it just didn't fail in istio < 1.7 ) . Httpsredirect doesn't make sense on port 443 since that is already https so there is no need to redirect to 443.

[linsun]

@ceastman-ibm Yes, agreed, only didn't make sense if it is already using https.
@howardjohn yep at least warning... i think fail would be good if we can detect it precisely, e.g. when a VS is bounded to such a GW but users can deploy VS first.

[esnible]

I am happy to write an analyzer for this.

I am uncertain what the exact triggering condition for the warning should be. Any Gateway with httpsRedirect AND port 443?

[linsun]

Thanks Ed!

Yes, pretty much any config like this where httpsRedirect: true is used on HTTPS protocol, which is port 443 by default for our gws.

  - port:
      number: 443
      name: https
      protocol: HTTPS
    hosts:
    - hostname
    tls:
      httpsRedirect: true

[howardjohn]

We shouldn't be so quick to write an analyzer IMO, lets first make sure we fully understand the problem and ensure Istiod is handling it correctly.

[pramodrj07]

HI @howardjohn @esnible - I am trying to understand what the "Analyzer" is related to this issue. Any pointer would be really useful.

[howardjohn]

@pramodrj07 we are referring to https://istio.io/latest/docs/ops/diagnostic-tools/istioctl-analyze/. But I strongly encourage us to NOT just create an analyzer and close this, but rather fix the root issue. And I also recommend we do not write a new analyzer and just instead add a warning (like #27445)

[pramodrj07]

@howardjohn - Yes Definitely. I was trying to understand the issue and what the comments were about. Basically trying to learn. Thanks for the pointer. This helps.

[istio-policy-bot]

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2020-09-24. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.