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

Swap the order of the CORS filter and the ext_authz filter. #4172

Merged
merged 2 commits into from
May 24, 2022
Merged

Swap the order of the CORS filter and the ext_authz filter. #4172

merged 2 commits into from
May 24, 2022

Conversation

kflynn
Copy link
Member

@kflynn kflynn commented Mar 24, 2022 •
edited
Loading

Having auth happen before CORS prevents CORS preflights of authenticated resources from ever having a chance at working. Swapping them allows preflights and "real" requests to both work.

Signed-off-by: Flynn [email protected]

  • I made sure to update CHANGELOG.md.
  • This is unlikely to impact how Ambassador performs at scale.
  • My change is adequately tested.
  • I didn't need to update DEVELOPING.md.

@kflynn kflynn closed this Mar 25, 2022
@kflynn kflynn deleted the flynn/dev/cors-and-auth branch March 25, 2022 19:21
@kflynn kflynn restored the flynn/dev/cors-and-auth branch March 25, 2022 19:22
@kflynn kflynn reopened this Mar 25, 2022
@kflynn kflynn marked this pull request as ready for review March 25, 2022 20:18
@kflynn kflynn force-pushed the flynn/dev/cors-and-auth branch 2 times, most recently from ac81816 to b51d83d Compare May 23, 2022 15:07
@kflynn kflynn changed the base branch from release/v2.2 to release/v2.3 May 23, 2022 15:09
@kflynn kflynn requested review from Alice-Lilith and LukeShu May 23, 2022 15:10
Alice-Lilith
Alice-Lilith previously approved these changes May 23, 2022
View reviewed changes
Copy link
Member

@Alice-Lilith Alice-Lilith left a comment

Choose a reason for hiding this comment

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

LGTM, but lets figure out those commits that are not signed properly. Since those commits are not from this PR, that change doesn't need to happen in this PR.

Flynn added 2 commits May 23, 2022 20:24
Swap order of auth & cors
91f619e 
Signed-off-by: Flynn <[email protected]>
CHANGELOG
fec0899 
Signed-off-by: Flynn <[email protected]>
@kflynn kflynn force-pushed the flynn/dev/cors-and-auth branch from b51d83d to fec0899 Compare May 24, 2022 00:25
@kflynn kflynn merged commit ea05c49 into release/v2.3 May 24, 2022
@kflynn kflynn deleted the flynn/dev/cors-and-auth branch May 24, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alice-Lilith Alice-Lilith Alice-Lilith approved these changes

@LukeShu LukeShu Awaiting requested review from LukeShu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kflynn @Alice-Lilith