-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Apply -enable-snippets cli arg to Ingresses #2124
Conversation
de405e4
to
a288b9f
Compare
17e7349
to
c2ad243
Compare
indirect=["ingress_controller"]) | ||
def test_snippet_annotation_ignored(self, kube_apis, ingress_controller_prerequisites, ingress_controller, test_namespace): | ||
file_name = f"{TEST_DATA}/annotations/standard/annotations-ingress-snippets.yaml" | ||
create_ingress_from_yaml(kube_apis.networking_v1, test_namespace, file_name) |
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.
Why do we not need to save the name of this ingress and then delete it after the test as we did in the previous test?
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 found that the ingress is not created as it is rejected as invalid ?
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.
Makes sense - I was mixing up ingress and ingress controller, thanks
ad5c0fe
to
c5e1549
Compare
c5e1549
to
0b00866
Compare
changed the base branch to release-2.0 |
* Apply -enable-snippets cli arg to Ingresses * Update docs * add snippet flag python tests * removing snippets check as we rely on validation * removing used param Co-authored-by: Sean O'Neill <s.oneill@f5.com>
* Apply -enable-snippets cli arg to Ingresses * Update docs * add snippet flag python tests * removing snippets check as we rely on validation * removing used param Co-authored-by: Sean O'Neill <s.oneill@f5.com>
FYI pushing breaking changes within a bugfix release is a bad practice. We had an outage in development environments due to this. From https://semver.org/
|
Also, can you please update the docs to reflect this CLI flags larger impact in 2.0.3? |
* fix: Updated theme hugo * Apply -enable-snippets cli arg to Ingresses (#2124) * Apply -enable-snippets cli arg to Ingresses * Update docs * add snippet flag python tests * removing snippets check as we rely on validation * removing used param Co-authored-by: Sean O'Neill <s.oneill@f5.com> * Install libcurl on OpenTracing for NGINX Plus (#2132) * DRAFT: Release v2.0.3 Co-authored-by: Ashutosh Pradhan <a.pradhan@f5.com> Co-authored-by: Michael Pleshakov <pleshakov@users.noreply.github.com> Co-authored-by: Sean O'Neill <s.oneill@f5.com> Co-authored-by: Ciara Stacke <c.stacke@f5.com>
Proposed changes
This PR extends the existing
-enable-snippets
cli argument to apply to Ingress resources. If snippets are not enabled, the Ingress Controller will reject any Ingress resources with snippets annotations.Previously, the argument only applied to VirtualServer, VirtualServerRoute and TransportServer resources.
Note: this is a breaking change. Because the default value of
-enable-snippets
isfalse
, If you're using snippets in Ingress resources, make sure to explicitly set the-enable-snippets
totrue
before upgrading the Ingress Controller, so that the new version of the Ingress Controller doesn't reject Ingresses with snippets annotations.TO-DO: