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.

Sign up for GitHub

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

Jump to bottom

Added functions and testcases to handle ingress.kubernetes.io/force-s… #745

Merged
merged 4 commits into from
Jul 21, 2020
Merged

Added functions and testcases to handle ingress.kubernetes.io/force-s… #745

merged 4 commits into from
Jul 21, 2020

Conversation

SandeepVaman
Copy link
Contributor

…sl-redirect annotation

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

@CLAassistant
Copy link

CLAassistant commented Jun 24, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@hbagdi
Copy link
Member

hbagdi commented Jul 2, 2020

@SandeepVaman ping.

@SandeepVaman
Added function which updates the protocol of the route to either http…
89d0125 
…s or grpcs, or https and grpcs when force-ssl-redirectAnnotation is true
@SandeepVaman
Copy link
Contributor Author

SandeepVaman commented Jul 4, 2020 •
edited
Loading

Added a function named useSSLProtocol(), which sets route protocol to https (and/or grpcs).
If you think of a better name, please suggest me.
And I am not greatly convinced to set Protocols in overrideRouteHTTPSRedirectCode() function because the primary intention of this function is to override HTTPS redirection code. What are your thoughts on this?

@hbagdi
Copy link
Member

hbagdi commented Jul 13, 2020

Let's also upgrade this from draft to a regular PR.

@SandeepVaman
set default protocol to https when no protocol is specified in annota…
064389e 
…tions and when force-ssl-redirect is set true
@SandeepVaman SandeepVaman marked this pull request as ready for review July 14, 2020 06:18
@hbagdi hbagdi changed the base branch from master to next July 21, 2020 21:59
hbagdi
hbagdi approved these changes Jul 21, 2020
View reviewed changes
internal/ingress/controller/parser/parser_test.go
},
kong.Route{
Protocols: kong.StringSlice("grpcs", "https"),
},
Copy link
Member

Choose a reason for hiding this comment

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

nit: A mix of grpc and http is not possible. Have this test case here is fine but useless, meaning it adds unnecessary overheads.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will clean it up and raise another PR.

@hbagdi hbagdi added this to the 0.10.0 milestone Jul 21, 2020
@hbagdi hbagdi merged commit efe4c3c into Kong:next Jul 21, 2020
@hbagdi
Copy link
Member

hbagdi commented Jul 21, 2020

Thank you for your contribution. Please fill out the following form to claim your contributor swag:
https://github.com/Kong/kong/blob/master/CONTRIBUTING.md#contributor-t-shirt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hbagdi hbagdi hbagdi approved these changes

@rainest rainest Awaiting requested review from rainest

Assignees
No one assigned
Labels
pr/changes requested
Projects
None yet
Milestone
0.10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@SandeepVaman @CLAassistant @hbagdi @sandeeprapido