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

Allow setting the ingress class via spec.ingressClassName #118

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

mproffitt
Copy link
Contributor

Fixes: #114

The annotation kubernetes.io/ingress.class is deprecated and shouldn't be used for specifying the ingress class.

Instead the option ingressClassName should be used. This introduces that as an option to values and adds additional annotation comments for enabling SSL passthrough when nginx is used as an ingress controller

As a result of this change, in local testing the following side effects were observed:

Motivation

Changes

Fixes: localstack#114

The annotation `kubernetes.io/ingress.class` is deprecated and shouldn't
be used for specifying the ingress class.

Instead the option `ingressClassName` should be used. This introduces
that as an option to values and adds additional annotation comments for
enabling SSL passthrough when nginx is used as an ingress controller

As a result of this change, in local testing the following side effects
were observed:
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution, @mproffitt!
I only found two small nitpicks (cleaning up the old annotation comments for the ingress class, and a superfluous line break). Afterwards we are good to go, and I'll merge and release this change! 🚀

charts/localstack/templates/ingress.yaml Outdated Show resolved Hide resolved
charts/localstack/values.yaml Outdated Show resolved Hide resolved
@alexrashed alexrashed added the type: enhancement Improvement to usability or performance label Apr 24, 2024
- Remove comment block from values.yaml relating to deprecated settings
- Remove  superfluous line break
@mproffitt mproffitt requested a review from alexrashed April 24, 2024 14:11
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks a lot for addressing the comments, and in general for your contribution! 🚀 🏅

@alexrashed alexrashed merged commit 0281fb4 into localstack:main Apr 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Improvement to usability or performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add support for ingressClassName
2 participants