-
Notifications
You must be signed in to change notification settings - Fork 110
Allow HTTP->HTTPS redirection on Istio Gateway #1888
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 HTTP->HTTPS redirection on Istio Gateway #1888
Conversation
|
|
|
Welcome @penguinush! It looks like this is your first PR to knative/operator 🎉 |
|
Hi @penguinush. Thanks for your PR. I'm waiting for a knative 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-sigs/prow repository. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1888 +/- ##
==========================================
+ Coverage 66.31% 66.49% +0.17%
==========================================
Files 53 54 +1
Lines 2084 2107 +23
==========================================
+ Hits 1382 1401 +19
- Misses 587 589 +2
- Partials 115 117 +2 ☔ View full report in Codecov by Sentry. |
|
@penguinush could you rebase this pr? |
|
/test all |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: houshengbo, penguinush 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 |
|
Please check this PR: #1912 |
Extends #1482.
One of the ServerTLSSettings on Istio Gatway is
httpsRedirectthat, according to docs, redirects HTTP traffic to HTTPS. Usually this is a desired behaviour for production setup, the only alternative would be to not create HTTP listener at all.PR leverages combination of
nullableproperty andoneOfwithrequiredfields - this works fine currently as there is 2 distinct options (mode: <>, which requirescredentialNameto be set, orhttpsRedirect, which is mutually exclusive with every other option), but it might become cumbersome if more options of different combinations are added.Tested with the following specs:
and
Both produce correct Gateways.
Kubernetes 1.30 (EKS), Knative Operator 1.15.4, Knative Serving 1.15.0, Istio 1.23.0.
Proposed Changes
Release Note