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

Update HTTPRequestRedirectPolicy StatusCode to use RedirectResponseCode #6789

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update HTTPRequestRedirectPolicy StatusCode to use RedirectResponseCode #6789

wants to merge 4 commits into from
+47 −18

Conversation

billyjs
Copy link

@billyjs billyjs commented Nov 26, 2024

To fix #6788

Allow 303, 307, 308 response status codes when using the request redirect policy.

@billyjs billyjs requested a review from a team as a code owner November 26, 2024 04:52
@billyjs billyjs requested review from tsaarni and sunjayBhatia and removed request for a team November 26, 2024 04:52
@sunjayBhatia sunjayBhatia requested review from a team, rajatvig and clayton-gonsalves and removed request for a team November 26, 2024 04:52
@github-actions GitHub Actions
Copy link

Hi @billyjs! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@tsaarni tsaarni added the release-note/small A small change that needs one line of explanation in the release notes. label Nov 26, 2024
@tsaarni
Copy link
Member

tsaarni commented Nov 26, 2024 •
edited
Loading

Hi @billyjs! You can runmake generate and commit the outcome to fix the build error and please also add one-liner changelog entry changelogs/unreleased/6789-billyjs-small.md and Signed-off-by to commit messages.

@billyjs
Update HTTPRequestRedirectPolicy StatusCode
4af6a40 
Use RedirectResponseCode instead of just 301/302 for HTTPRequestRedirectPolicy StatusCode

Fixes projectcontour#6788

Signed-off-by: Billy Schulze <code@billy.id.au>
@billyjs billyjs force-pushed the billyjs-redirect-status-code-fix branch from 8f39293 to 4af6a40 Compare November 26, 2024 05:31
@billyjs
Copy link
Author

billyjs commented Nov 26, 2024

@tsaarni what is the release cadence for this project?

@tsaarni
Copy link
Member

tsaarni commented Nov 26, 2024

what is the release cadence for this project?

Generally, we have aimed for quarterly releases but that schedule hasn't always been strictly followed.

By the way, I noticed there's a minor issue (#6791) that needs to be resolved before moving forward with this PR

@billyjs
Copy link
Author

billyjs commented Nov 26, 2024 •
edited
Loading

By the way, I noticed there's a minor issue (#6791) that needs to be resolved before moving forward with this PR

Thanks, i was trying to figure out why it was not updating that file.

@billyjs
Update HTTPRequestRedirectPolicy tests to use RedirectResponseCode
f4e4e33 
Signed-off-by: Billy Schulze <code@billy.id.au>
@billyjs billyjs force-pushed the billyjs-redirect-status-code-fix branch from 65b4c13 to f4e4e33 Compare November 26, 2024 07:21
@billyjs
Fix type in tests
faf2829 
Signed-off-by: Billy Schulze <code@billy.id.au>
@billyjs
Copy link
Author

billyjs commented Nov 28, 2024

@tsaarni I used your changes in #6791 to generate the deepcopy file and updated the types in the tests. I think it should pass the CI tests now as it passes the tests locally.

@github-actions GitHub Actions
Copy link

The Contour project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 14d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the PR is closed

You can:

  • Ensure your PR is passing all CI checks. PRs that are fully green are more likely to be reviewed. If you are having trouble with CI checks, reach out to the #contour channel in the Kubernetes Slack workspace.
  • Mark this PR as fresh by commenting or pushing a commit
  • Close this PR
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 16, 2024
@billyjs
Add 303, 307, 308 as valid envoy redirect route status codes
276ae32 
Signed-off-by: Billy Schulze <code@billy.id.au>
@billyjs
Copy link
Author

billyjs commented Dec 16, 2024

Anything else required to get this PR merged?

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsaarni tsaarni Awaiting requested review from tsaarni tsaarni is a code owner automatically assigned from projectcontour/maintainers

@sunjayBhatia sunjayBhatia Awaiting requested review from sunjayBhatia sunjayBhatia is a code owner automatically assigned from projectcontour/maintainers

@rajatvig rajatvig Awaiting requested review from rajatvig rajatvig was automatically assigned from projectcontour/contour-reviewers

@clayton-gonsalves clayton-gonsalves Awaiting requested review from clayton-gonsalves clayton-gonsalves was automatically assigned from projectcontour/contour-reviewers

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
release-note/small A small change that needs one line of explanation in the release notes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot use 307/308 for HTTPRequestRedirectPolicy StatusCode
2 participants
@billyjs @tsaarni