Skip to content

Conversation

@arjan-bal
Copy link
Contributor

@arjan-bal arjan-bal commented Jan 16, 2025

As per gRFC A82, xDS client should be able to use system root certificates to validate server identity if the use of system root certs is configured by the xDS management server. This is useful when the gRPC server is behind a reverse proxy. As per the gRFC, the feature is behind a feature flag till it is proven stable.

RELEASE NOTES:

  • xds: Client can use system root certificates to validate server identity when configures by the xDS management server. To enable this feature, set the environment variable GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS to true (case insensitive). See gRFC A82 (A82: xDS System Root Certificates proposal#436) for details.

@arjan-bal arjan-bal added Type: Feature New features or improvements in behavior Area: xDS Includes everything xDS related, including LB policies used with xDS. labels Jan 16, 2025
@arjan-bal arjan-bal added this to the 1.71 Release milestone Jan 16, 2025
@codecov
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 78.57143% with 18 lines in your changes missing coverage. Please review.

Project coverage is 82.32%. Comparing base (130c1d7) to head (985bbbd).
Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
xds/internal/xdsclient/xdsresource/type_cds.go 50.00% 12 Missing and 2 partials ⚠️
xds/internal/balancer/cdsbalancer/cdsbalancer.go 83.33% 2 Missing and 1 partial ⚠️
...ds/internal/xdsclient/xdsresource/unmarshal_cds.go 96.42% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8013      +/-   ##
==========================================
+ Coverage   81.93%   82.32%   +0.38%     
==========================================
  Files         381      383       +2     
  Lines       38581    38824     +243     
==========================================
+ Hits        31613    31963     +350     
+ Misses       5640     5539     -101     
+ Partials     1328     1322       -6     
Files with missing lines Coverage Δ
internal/testutils/xds/e2e/clientresources.go 98.21% <100.00%> (+0.03%) ⬆️
...ds/internal/xdsclient/xdsresource/unmarshal_cds.go 86.72% <96.42%> (+0.65%) ⬆️
xds/internal/balancer/cdsbalancer/cdsbalancer.go 81.64% <83.33%> (-0.11%) ⬇️
xds/internal/xdsclient/xdsresource/type_cds.go 50.00% <50.00%> (ø)

... and 48 files with indirect coverage changes

Copy link
Contributor

@easwars easwars left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Mainly minor comments.

systemRootCertsFunc = origSRCF
}()
envconfig.XDSSystemRootCertsEnabled = true
systemRootCertsFunc = origSRCF
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line is redundant, right, because we override systemRootCertsFunc right below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it's a copy/paste error. Removed it.

@easwars easwars assigned arjan-bal and unassigned easwars Jan 23, 2025
@arjan-bal arjan-bal changed the title xds: Implement system root cert support xds: Implement system root certs support Jan 24, 2025
@arjan-bal arjan-bal assigned easwars and unassigned arjan-bal Jan 24, 2025

func (systemRootCertsProvider) Close() {}

func (systemRootCertsProvider) KeyMaterial(_ context.Context) (*certprovider.KeyMaterial, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: since the context is the only parameter, can't we omit the _ parameter name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@easwars easwars assigned arjan-bal and unassigned easwars Jan 28, 2025
@arjan-bal arjan-bal merged commit e03960d into grpc:master Jan 28, 2025
15 checks passed
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request Jan 30, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 13, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 13, 2025
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Feb 15, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Area: xDS Includes everything xDS related, including LB policies used with xDS. Type: Feature New features or improvements in behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants