-
Notifications
You must be signed in to change notification settings - Fork 8.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
Applying proxy-ssl-* directives on locations only #4981
Conversation
Add: new parameter in the ConfigMap to control whether the proxy-ssl parameters of an Ingress should be applied on server and location levels, or only on location level Add: logic in the config handling to work according to the new ConfigMap parameter Add: unit test case
Hi @janosi. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
Codecov Report
@@ Coverage Diff @@
## master #4981 +/- ##
=========================================
+ Coverage 58.54% 61.3% +2.75%
=========================================
Files 88 88
Lines 6737 8119 +1382
=========================================
+ Hits 3944 4977 +1033
- Misses 2363 2608 +245
- Partials 430 534 +104
Continue to review full report at Codecov.
|
/retest |
1 similar comment
/retest |
@aledbf Excuse me for my question, I am not familiar with the release process here, for example when a PR is merged. I can see that this one is removed from 0.29.0. Is there any blocking problem with this PR that I should solve? Thank you! |
/lgtm |
@janosi thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, janosi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
What this PR does / why we need it:
Currently we can define different proxy-ssl directives for the different locations of the same server if we have different Ingress definitions for those locations. However one of those proxy-ssl directives is also set on the common server level. It has the following problems:
This PR is o further enhance this logic, so it can provide a more user-controlled way of configuring the parameters either on location level, or on both server and location levels.
Types of changes
Which issue/s this PR fixes
This PR implements a solution for the problem drafted in #4831
How Has This Been Tested?
Unit tests are added
Checklist: