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

feat: allow TLS to be disabled for gateways #1030

Closed
wants to merge 5 commits into from
Closed

Conversation

willswire
Copy link
Member

@willswire willswire commented Nov 20, 2024

Description

When deploying UDS Core Base to EKS, we can use annotations to populate certificates from Amazon’s AWS Certificate Manager (ACM) on the ELBs provisioned by Istio (see here and here).

Unfortunately, this does not work if an entry in the Gateway's servers spec contains tls -- likely a result of the conflicting certs provided for *.uds.dev.

By exposing the option of enabling/disable TLS via a variable, we can opt to disable it, and therefore enable annotation-based cert management.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@willswire willswire requested a review from a team as a code owner November 20, 2024 20:55
@willswire willswire force-pushed the optional-tls-istio branch 2 times, most recently from a310342 to 0fb3183 Compare November 20, 2024 21:55
@willswire willswire changed the title allow TLS as optional for gateways allow TLS to be disabled for gateways Nov 20, 2024
@willswire willswire changed the title allow TLS to be disabled for gateways feat: allow TLS to be disabled for gateways Nov 20, 2024
Copy link
Member Author

@willswire willswire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented TLS configuration validation via a Helm chart hook.

src/istio/chart/templates/gateway.yaml Show resolved Hide resolved
src/istio/chart/templates/tls-validation-job.yaml Outdated Show resolved Hide resolved
src/istio/chart/templates/tls-validation-job.yaml Outdated Show resolved Hide resolved
src/istio/chart/templates/tls-validation-job.yaml Outdated Show resolved Hide resolved
src/istio/chart/templates/tls-validation-job.yaml Outdated Show resolved Hide resolved
src/istio/chart/templates/gateway.yaml Show resolved Hide resolved
@mjnagel
Copy link
Contributor

mjnagel commented Dec 6, 2024

@willswire just to check on this one - I think we determined that we would close this out due to some of the complexity/issues with non-tls traffic in the loop here?

@willswire
Copy link
Member Author

@willswire just to check on this one - I think we determined that we would close this out due to some of the complexity/issues with non-tls traffic in the loop here?

Yep! We're good to close this one out.

@willswire willswire closed this Dec 10, 2024
@willswire willswire deleted the optional-tls-istio branch December 10, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants