Skip to content

Conversation

@davem-git
Copy link
Contributor

@davem-git davem-git commented Oct 8, 2025

What type of PR is this?
Feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes ##4908

Release Notes: Yes/No
No

@davem-git davem-git requested a review from a team as a code owner October 8, 2025 21:06
…s clientIP and what defines a clientIP

Signed-off-by: davem-git <[email protected]>
Signed-off-by: davem-git <[email protected]>
Signed-off-by: davem-git <[email protected]>
Signed-off-by: davem-git <[email protected]>
…ty security policies, but that was determined as fine so the check to block it was removed

Signed-off-by: davem-git <[email protected]>
@codecov
Copy link

codecov bot commented Oct 11, 2025

Codecov Report

❌ Patch coverage is 91.66667% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.08%. Comparing base (c68db79) to head (7e601e5).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/securitypolicy.go 91.66% 3 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7171      +/-   ##
==========================================
+ Coverage   71.04%   71.08%   +0.03%     
==========================================
  Files         229      229              
  Lines       41099    41164      +65     
==========================================
+ Hits        29198    29260      +62     
- Misses      10181    10184       +3     
  Partials     1720     1720              

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davem-git davem-git force-pushed the feat-tcp-security-policy-gateway-api branch from 3a2057e to 67814ea Compare October 14, 2025 20:30
kkk777-7
kkk777-7 previously approved these changes Oct 17, 2025
@kkk777-7
Copy link
Member

LGTM thanks!

@kkk777-7
Copy link
Member

/retest

zhaohuabing
zhaohuabing previously approved these changes Oct 20, 2025
Copy link
Member

@zhaohuabing zhaohuabing 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!

@davem-git davem-git dismissed stale reviews from zhaohuabing and kkk777-7 via 6753bca October 20, 2025 15:46
@davem-git davem-git force-pushed the feat-tcp-security-policy-gateway-api branch from 85d61b6 to 6753bca Compare October 20, 2025 15:46
@davem-git
Copy link
Contributor Author

Fixed a test that was still referencing alpha2, which was removed in this PR. Sorry about that. The update cleared existing approvals.

@davem-git
Copy link
Contributor Author

the coverage test seems like a network timeout failure

kkk777-7
kkk777-7 previously approved these changes Oct 21, 2025
for _, listener := range parentRefCtx.listeners {
irListener := xdsIR[irKey].GetHTTPListener(irListenerName(listener))
if irListener != nil {
switch route.GetRouteType() {
Copy link
Contributor

Choose a reason for hiding this comment

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

curious why this code is needed when its NA for TCPRoute, which is checked in validateSecurityPolicyForTCP

Copy link
Contributor Author

@davem-git davem-git Oct 21, 2025

Choose a reason for hiding this comment

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

which part are you referring to, why we do switch.route? TCP is applied differently with expectedTCPRouteName := strings.TrimSuffix(prefix, "/") , but i don't see that getting applied just continueing if it doesn't match

then there's this line

if target.SectionName != nil && string(*target.SectionName) != r.Destination.Metadata.SectionName {
  continue
}```
vs this one for httproute
```go

if target.SectionName != nil && string(*target.SectionName) != r.Metadata.SectionName {
continue
}

those have to be different.

let me see if i can reduce the difference

@davem-git davem-git force-pushed the feat-tcp-security-policy-gateway-api branch from 7a6f78a to 7e601e5 Compare October 22, 2025 16:23
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

@arkodg arkodg merged commit 7c4bf61 into envoyproxy:main Oct 22, 2025
30 of 32 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.

4 participants