Add ingress option

[thisisnotashwin]

Changes proposed in this PR:

• Based on Add ingress option to UI service. #130
• Added support to expose http port when global.tls.enabled is true and global.tls.httpsOnly is false.

The ingress resource always performs TLS termination at the point of ingress and can only communicate with the Consul server over the HTTP port. This can be avoided by certain ingress controllers that have the option of allowing ssl pass-through. Added a note to ensure that users have an ingress controller that can passthrough SSL and have it enabled if they want to enable TLS on Consul.

How I've tested this PR:

• Deployed an ingress controller on the kubernetes cluster (nginx).
• Performed a helm deploy with ingress enabled for UI. Verified the fields on the ingress resource created for non-TLS and TLS (with HTTP enabled) use cases and was able to access the UI via the created Ingress. (GKE creates a LoadBalancer for ingress)

How I expect reviewers to test this PR:

• Code review.

Checklist:

• Bats tests added
• CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)
• PR submitted to update consul.io Helm docs (see Generating Helm Reference Docs)
  • Link to PR: Update docs with fields for ingress support for consul UI consul#9612

[lkysow]

I'm wondering about the version of the object we're using:

W0121 13:39:33.134204   34932 warnings.go:70] extensions/v1beta1 Ingress is deprecated in v1.14+, unavailable in v1.22+; use networking.k8s.io/v1 Ingress

[lkysow]

I'm finding these test descriptions a bit hard to grok. Maybe "exposes single backed on port 80 when global.tls.enabled=false" and "exposes backends on ports 80 and 443 when ..."?

[thisisnotashwin]

omg i just read the test names and i admit they are horrible. copy-pasting with proof-reading. this is why we review!

[lkysow]

Should we tell them what secret to use? Do they need to use passthrough TLS if consul tls is enabled?

[thisisnotashwin]

So i realized in my tinkering that i hadn't tested the only TLS (https only enabled) options. ie, the 80 port was always open on the service. the ingress resource always performs TLS termination so we cant point traffic to 443 as the request that reached the service will always be HTTP. I guess it doesnt make sense for this PR to support port 443 in that case, and ill make the requisite changes.

[thisisnotashwin]

@lkysow re version: I can switch over to version v1. This was the version on the initial PR but i think it makes sense to up it given it has been supported since 1.14

[ryan-dyer-sp]

The only difference I'm seeing between L27-32 and L35-49 is the servicePort. So why not just make that the conditional instead of the entire block?

[thisisnotashwin]

I guess the only motivation there would be in a situation when TLS is enabled but both HTTP and HTTPs traffic is allowed, we would need both the blocks. I can look at DRY-ing this up though.

[ryan-dyer-sp]

I see your point from a what's allowed perspective, but not sure why someone would create an Ingress and allow port 80 traffic, if 443 is available for the UI. I would think that the primary reason for having 80 and 443 enabled on consul is because they are migrating from 80 to 443 and havent finished shoring up the clients still using 80.

[ryan-dyer-sp]

And in the case where they are both enabled how does the ingress know which backend to send to if they are defined the exact same way but with different servicePort? This is just a general curiousity question, if the path were different, I could see how it would work. But I'd almost think this would be an invalid configuration for an Ingress.

[thisisnotashwin]

In case the Ingress controller does not support TLS passthrough, only supporting port 443 will not work. In the case when both ports are exposed, the controller multiplexes the requests across the ports and responds with whichever response is successful.
That is a really good questions though. I tested it with both ports open without the passthrough and it worked like a treat.

[thisisnotashwin]

This allows for maximum flexibility but ensures the ability to expose just the one port (443) if that is so desired. Did I understand your question correctly?

[kschoche]

🧨