Wrong default values prevent basic install k8s ingress support

[gravufo]

Bug Description

Background

We were using Istio 1.15.3 installed through the helm charts and decided to upgrade to the latest currently available (1.17.2).
We have cert-manager installed and have a certificate that needs the HTTP01 challenge mechanism which was working fine out-of-the-box with istio 1.15.3.

Upgrade fun

While upgrading, we noticed that the istio-ingress and istio-egress charts were not published anymore and seemed to be deprecated in favor of the new unified chart simply called gateway. By doing so, we also had to adjust our Gateway CRD to match the new app label (istio-gateway rather than istio-ingressgateway). Rather simple fix, so we decided to go ahead and do it.

Real Problem

Once our certificates were getting close to expiry and needed to be renewed, the HTTP01 challenge was spawned, the associated ingress was created with the proper ingress.class annotation set to istio...but nothing worked. It was simply not getting picked up by istio.

Troubleshooting

We enabled all the logs we could think of and all we could find was the gateway's envoy saying there was no route found for the incoming challenge requests which meant that the ingress wasn't taken into account. Unfortunately we were unable to find anything more pertinent...maybe there could be improvements on the logging side or the logs exist and we simply didn't know where to enable them.

Final find

After many hours of research, we finally figured out that the issue was due to the gateway's name being changed in the new merged helm chart and the default values in istio's meshConfig not being updated to reflect this.
So the fix was to reinstall istiod's helm chart by adding the following parameters: --set meshConfig.ingressService=istio-gateway --set meshConfig.ingressSelector=gateway.
After that, the challenges were almost immediately completed and the certificate was issued successfully.

Takeaway

I think this whole ordeal was due to a rename that was done half-way. But, I also believe that finding the documentation to properly use k8s ingress resources was hard. There is this page that says it should just work out-of-the-box, but that's not always the case and then finding how to properly configure istiod was not easy...I basically stumbled upon the meshConfig page by chance and realised there were ingress options there and then had to reverse engineer to figure out how to provide them through the helm chart.

Version

❯ istioctl version
client version: 1.17.2
control plane version: 1.17.2
data plane version: 1.17.2 (15 proxies)

❯ kubectl version --short
Client Version: v1.27.0
Kustomize Version: v5.0.1
Server Version: v1.25.5

Additional Information

There are a few parts that should probably be changed to reflect the new gateway name:

• istio/pkg/config/constants/constants.go

  Line 85 in 081302a

  IstioIngressLabelValue = "ingressgateway"

  • should be renamed to just "gateway"

• istio/pilot/pkg/config/kube/ingress/conversion.go

  Line 367 in b437d02

  } else if ingressService != "istio-ingressgateway" && ingressService != "" {

  • should probably use a constant rather than a hardcoded string, but otherwise should be updated to "istio-gateway"
  • the logic also seems flawed, because it doesn't seem to be so true that the label will match the exact name of the service where you will usually have an "istio-" prefix but not on the label...

• istio/pkg/config/mesh/mesh.go

  Line 85 in a44c42b

  IngressService: "istio-ingressgateway",

  • should be renamed to "istio-gateway"
• https://istio.io/latest/docs/reference/config/istio.mesh.v1alpha1/
  • documentation should be updated to reflect the new values

Affected product area

• Ambient
• Docs
• Installation
• Networking
• Performance and Scalability
• Extensions and Telemetry
• Security
• Test and Release
• User Experience
• Developer Infrastructure
• Upgrade
• Multi Cluster
• Virtual Machine
• Control Plane Revisions

[howardjohn]

There was no "rename". There is no name at all. When you install the chart you pick them name and can put whatever you want there. If you do helm template helloworld istio/gateway you get a helloworld service.

So yes, some legacy code has legacy baked in assumptions about a particular naming scheme - but we should not change these. Instead we can document on the page for Ingress how these configurations work and how to set them.

[gravufo]

That is fine with me, since these hardcoded values are just assumptions and wild guesses anyway. I think the main part here is that the documentation was not enough to get this solved easily as it should have been.

Thanks for the quick response.

[howardjohn]

yep, 100% agree on the docs concern

…

On Fri, Apr 14, 2023, 2:31 PM Christian Artin ***@***.***> wrote: That is fine with me, since these hardcoded values are just assumptions and wild guesses anyway. I think the main part here is that the documentation was not enough to get this solved easily as it should have been. Thanks for the quick response. — Reply to this email directly, view it on GitHub <#44402 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEYGXMAWTKAWQO6COJGKATXBG63NANCNFSM6AAAAAAW64CVQM> . You are receiving this because you commented.Message ID: ***@***.***>

[istio-policy-bot]

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2023-04-14. 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.