Skip to content

fix: invalid basic auth data caused rds rejection#5683

Merged
arkodg merged 1 commit intoenvoyproxy:mainfrom
Xunzhuo:fix-security-policy-isolation
Jun 21, 2025
Merged

fix: invalid basic auth data caused rds rejection#5683
arkodg merged 1 commit intoenvoyproxy:mainfrom
Xunzhuo:fix-security-policy-isolation

Conversation

@Xunzhuo
Copy link
Member

@Xunzhuo Xunzhuo commented Apr 7, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #5581

Release Notes: Yes/No

@Xunzhuo Xunzhuo requested a review from a team as a code owner April 7, 2025 09:44
@Xunzhuo Xunzhuo marked this pull request as draft April 7, 2025 09:44
@Xunzhuo Xunzhuo force-pushed the fix-security-policy-isolation branch from 3a57e80 to c04c39d Compare April 8, 2025 02:34
@arkodg
Copy link
Contributor

arkodg commented May 8, 2025

I dont understand the fix here @Xunzhuo
the issue is that the file passed down to envoy was invalid, causing envoy to reject RDS

the fix here is to validate the basic auth data

usersSecretBytes, ok := usersSecret.Data[egv1a1.BasicAuthUsersSecretKey]

@Xunzhuo Xunzhuo force-pushed the fix-security-policy-isolation branch from c04c39d to d25f248 Compare May 8, 2025 02:42
@Xunzhuo Xunzhuo changed the title fix: security policy isolation fix: invalid basic auth data caused rds rejection May 8, 2025
@Xunzhuo Xunzhuo marked this pull request as ready for review May 8, 2025 02:47
@Xunzhuo Xunzhuo requested review from arkodg and zhaohuabing May 8, 2025 02:49
@codecov
Copy link

codecov bot commented May 8, 2025

Codecov Report

Attention: Patch coverage is 77.77778% with 6 lines in your changes missing coverage. Please review.

Project coverage is 70.72%. Comparing base (16747d4) to head (0783e2b).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/securitypolicy.go 77.77% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5683      +/-   ##
==========================================
- Coverage   70.76%   70.72%   -0.05%     
==========================================
  Files         220      220              
  Lines       37059    37086      +27     
==========================================
+ Hits        26225    26229       +4     
- Misses       9299     9318      +19     
- Partials     1535     1539       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Xunzhuo
Copy link
Member Author

Xunzhuo commented May 8, 2025

@arkodg get it, plz help review it : )

@arkodg arkodg added this to the v1.4.0 milestone May 8, 2025
@Xunzhuo Xunzhuo force-pushed the fix-security-policy-isolation branch from 875599f to dd7d9f4 Compare May 9, 2025 03:17
@arkodg arkodg modified the milestones: v1.4.0, Backlog May 13, 2025
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!

@github-actions github-actions bot added stale and removed stale labels Jun 12, 2025
arkodg
arkodg previously approved these changes Jun 13, 2025
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

can you rebase again ( 🙈 )

@arkodg arkodg requested review from a team June 13, 2025 01:04
@arkodg arkodg modified the milestones: Backlog, v1.5.0-rc.1 Release Jun 13, 2025
Signed-off-by: bitliu <bitliu@tencent.com>
@Xunzhuo Xunzhuo force-pushed the fix-security-policy-isolation branch from dd7d9f4 to 0783e2b Compare June 20, 2025 08:23
@Xunzhuo Xunzhuo requested a review from arkodg June 20, 2025 08:23
Copy link
Contributor

@shawnh2 shawnh2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@arkodg arkodg merged commit db09d27 into envoyproxy:main Jun 21, 2025
26 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All HTTPRoutes fail to resolve when a single route's SecurityPolicy fails to load

3 participants