-
Notifications
You must be signed in to change notification settings - Fork 83
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
add individual ingress configuration per service #2183
Conversation
enablePerHostIngress: false | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this per-host ingress configuration should be the only way to do it. I don't think we should allow the old way of doing it. If we switch to this new method then we reduce the amount of code we have to maintain and test while still providing the same features, just in a different way that is compatible with more environments.
Is there some reason we should keep the old way as an option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slack thread https://astronomer.slack.com/archives/C02J8JT58CC/p1715623166453439
We agreed to keep the old way in, behind a feature flag, and run it through a deprecation phase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can default this to true @pgvishnuram
* add individual ingress per service * update templating * added feature flag * fix pre-commit * re-work test cases * rework ingress template * update test cases * fix pre-commit * update test cases * Generalize some tests, add some test cases. --------- Co-authored-by: Daniel Hoherd <daniel.hoherd@gmail.com>
Description
This PR segreagates the common ingress rules to individual rules per service to avoid policy issues
Related Issues
Testing
Yet to update
Merging
NA