Skip to content

xds/rbac: add additional handling for addresses with ports#8990

Open
mbissa wants to merge 1 commit intogrpc:masterfrom
mbissa:xds-rbac-handle-ipaddress-port
Open

xds/rbac: add additional handling for addresses with ports#8990
mbissa wants to merge 1 commit intogrpc:masterfrom
mbissa:xds-rbac-handle-ipaddress-port

Conversation

@mbissa
Copy link
Contributor

@mbissa mbissa commented Mar 20, 2026

This PR enhances the rbac matcher to handle IP address string with a port attached.

The fix introduces the net.SplitHostPort utility function, ensuring the port is properly stripped out of the underlying peerInfo.Addr.String() and localAddr.String() values before parsing them with netip.ParseAddr. A fallback mechanism is also included in case SplitHostPort fails due to a missing port.

RELEASE NOTES:

  • xds/rbac: Add additional handling for addresses with ports

@mbissa mbissa added this to the 1.80 Release milestone Mar 20, 2026
@codecov
Copy link

codecov bot commented Mar 20, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.00%. Comparing base (b6597b3) to head (323fae8).
⚠️ Report is 23 commits behind head on master.

Files with missing lines Patch % Lines
internal/xds/rbac/matchers.go 50.00% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8990      +/-   ##
==========================================
- Coverage   83.42%   83.00%   -0.42%     
==========================================
  Files         410      411       +1     
  Lines       32572    32902     +330     
==========================================
+ Hits        27172    27310     +138     
- Misses       4030     4191     +161     
- Partials     1370     1401      +31     
Files with missing lines Coverage Δ
internal/xds/rbac/matchers.go 75.71% <50.00%> (-1.79%) ⬇️

... and 57 files with indirect coverage changes

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

@@ -344,7 +345,15 @@ func newRemoteIPMatcher(cidrRange *v3corepb.CidrRange) (*remoteIPMatcher, error)
}

func (sim *remoteIPMatcher) match(data *rpcData) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

How big of a change would it be to make match return a (bool, error) to enable callers to handle parsing errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 11 implementations and each implementation has on an average of 4-5 references. This would need thorough testing and effort. I suggest we merge this and do that as a follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants