-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Discuss rename of permanent-redirect and temporal-redirect annotation #2864
Comments
I have also just noticed that there is a ConfigMap setting:
This appears to only be used in http -> https redirects. So I believe this should be renamed to |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@fejta-bot: Closing this issue. In response to this:
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. |
Hey,
The work I did in #2834 allows you to control the status code used for permanent-redirects as there are a couple of valid options (301, 308 etc).
However the annotation we ended up using was called
http-redirect-code
which implies it could also be used to set short term redirects too (eg 302).Whilst digging in that code, I also noticed there is another undocumented annotation
temporal-redirect
which appears to302
.It's all getting a bit confusing, so I propose simply two annotations:
http-redirect
andhttp-redirect-code
and getting rid ofpermanent-redirect
andtemporal-redirect
which would then just allow people to use whichever of the 5 valid redirect codes they chose (301, 302, 303, 307, 308).Naturally however this would be a breaking change so this issue is to talk about that...
The text was updated successfully, but these errors were encountered: