xds: add support for mTLS Credentials in xDS bootstrap #6757

atollena · 2023-10-31T11:06:38Z

Implement gRFC "A65: mTLS Credentials in xDS Bootstrap File".

https://github.com/grpc/proposal/blob/master/A65-xds-mtls-creds-in-bootstrap.md

RELEASE NOTES:

xds: add support for mTLS Credentials in xDS bootstrap (gRFC A65)

codecov · 2023-10-31T11:09:52Z

Codecov Report

Merging #6757 (01f8c82) into master (bb0d32f) will increase coverage by 0.05%.
Report is 1 commits behind head on master.
The diff coverage is 87.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6757      +/-   ##
==========================================
+ Coverage   83.65%   83.71%   +0.05%     
==========================================
  Files         286      287       +1     
  Lines       30756    30829      +73     
==========================================
+ Hits        25730    25808      +78     
- Misses       3963     3969       +6     
+ Partials     1063     1052      -11

Files	Coverage Δ
credentials/tls/certprovider/pemfile/builder.go	`100.00% <100.00%> (ø)`
xds/bootstrap/bootstrap.go	`100.00% <ø> (ø)`
xds/internal/xdsclient/bootstrap/bootstrap.go	`74.48% <100.00%> (+0.64%)`	⬆️
xds/internal/xdsclient/clientimpl.go	`89.65% <100.00%> (+5.44%)`	⬆️
xds/internal/xdsclient/tlscreds/bundle.go	`82.45% <82.45%> (ø)`

... and 25 files with indirect coverage changes

Implement A65: mTLS Credentials in xDS Bootstrap File described in https://github.com/grpc/proposal/blob/master/A65-xds-mtls-creds-in-bootstrap.md.

atollena · 2023-10-31T13:59:47Z

Regarding test coverage: I could add tests for Clone() and other untested and unused methods, but it feels a bit like gaming the objective...

xds/internal/xdsclient/bootstrap/bootstrap.go

xds/internal/xdsclient/creds/tls.go

arvindbr8 · 2023-11-01T20:45:31Z

Hey @atollena -- Thanks for the PR!!

I took a first pass at the patchset and left a few comments. PTAL

Regarding test coverage: I could add tests for Clone() and other untested and unused methods, but it feels a bit like gaming the objective...

I agree with your sentiment here. The coverage checks are not required anyways. So I wouldnt worry about this particular diff hit.

arvindbr8

LGTM! Thanks @atollena

@dfawley Could you take another look at this patch-set?

xds/internal/xdsclient/creds/tls.go

atollena · 2023-11-10T09:03:40Z

@dfawley gentle ping, do you have any feedback on this?

dfawley · 2023-11-10T17:10:58Z

Sorry! I need to do a context switch and get back up to speed on this code and the gRFC. Hopefully today or early next week...

- close credentials provider for each authority - Fatal tests where possible.

There are more changes that deserve another review.

xds/internal/xdsclient/authority.go

xds/internal/xdsclient/bootstrap/bootstrap.go

xds/internal/xdsclient/bootstrap/bootstrap_test.go

xds/internal/xdsclient/tlscreds/bundle.go

xds/internal/xdsclient/tlscreds/bundle_ext_test.go

- cleanup in clientimpl close rather than individual authority close - convert tests in bootstrap_test to table driven tests

atollena · 2023-12-22T12:15:41Z

I'll be off until early January, will pick this up after I come back.

easwars · 2023-12-26T23:57:11Z

I'll be off until early January, will pick this up after I come back.

I'm going to be off for a few months. See you then :)

Changes look good to me. I've approved it. Since we require two approvals for external contributors, and most of the team is out currently, someone will pick this up in January.

atollena · 2023-12-27T15:06:11Z

I'll be off until early January, will pick this up after I come back.

I'm going to be off for a few months. See you then :)

Changes look good to me. I've approved it. Since we require two approvals for external contributors, and most of the team is out currently, someone will pick this up in January.

Thanks you. Happy holidays and enjoy your time off then!

arvindbr8

LGTM, and like always thanks for the PR @atollena!

gRPC-Go as of [v1.61.0](https://github.com/grpc/grpc-go/releases/tag/v1.61.0) supports [gRFC A65: mTLS Credentials in xDS Bootstrap File](https://github.com/grpc/proposal/blob/8c31bfedded5f0a51c4933e9e9a8246122f9c41a/A65-xds-mtls-creds-in-bootstrap.md), see grpc/grpc-go#6757. These changes add control plane mTLS for the Go control plane and sample application implementations, controlled by flags in the `xds_features.yaml` configuration file.

atollena force-pushed the a65 branch 4 times, most recently from 9b46618 to 290e695 Compare October 31, 2023 11:39

xds/internal/xdsclient: A65 - mTLS Credentials

b214d59

Implement A65: mTLS Credentials in xDS Bootstrap File described in https://github.com/grpc/proposal/blob/master/A65-xds-mtls-creds-in-bootstrap.md.

atollena force-pushed the a65 branch from 290e695 to b214d59 Compare October 31, 2023 11:53

atollena marked this pull request as ready for review October 31, 2023 13:37

atollena mentioned this pull request Oct 31, 2023

Support tls xDS credentials defined in gRFC A65 #6690

Closed

ginayeh added the Type: Feature New features or improvements in behavior label Oct 31, 2023

ginayeh added this to the 1.60 Release milestone Oct 31, 2023

ginayeh requested review from dfawley and arvindbr8 October 31, 2023 17:31

ginayeh assigned arvindbr8 Oct 31, 2023

arvindbr8 reviewed Nov 1, 2023

View reviewed changes

xds/internal/xdsclient/creds/tls.go Outdated Show resolved Hide resolved

arvindbr8 assigned atollena and unassigned arvindbr8 Nov 1, 2023

arvindbr8 added the Status: Requires Reporter Clarification label Nov 1, 2023

arvindbr8 changed the title ~~xds/internal/xdsclient: A65 - mTLS Credentials~~ xds: add support for mTLS Credentials in xDS bootstrap Nov 1, 2023

feedback from Arvind

114af39

arvindbr8 removed the Status: Requires Reporter Clarification label Nov 2, 2023

arvindbr8 previously approved these changes Nov 2, 2023

View reviewed changes

arvindbr8 assigned dfawley and unassigned atollena Nov 2, 2023

arvindbr8 removed this from the 1.60 Release milestone Nov 14, 2023

atollena added 2 commits December 21, 2023 14:32

generic xds client cleanups

8f3472e

feedback from easwar:

e694f4c

- close credentials provider for each authority - Fatal tests where possible.

easwars assigned easwars and unassigned atollena and dfawley Dec 21, 2023

easwars reviewed Dec 21, 2023

View reviewed changes

easwars assigned atollena Dec 21, 2023

latest review from easwar

01f8c82

- cleanup in clientimpl close rather than individual authority close - convert tests in bootstrap_test to table driven tests

atollena force-pushed the a65 branch from dd4d212 to 01f8c82 Compare December 22, 2023 11:12

easwars removed the Status: Requires Reporter Clarification label Dec 26, 2023

easwars approved these changes Dec 26, 2023

View reviewed changes

easwars unassigned atollena and easwars Dec 26, 2023

ginayeh assigned arvindbr8 Jan 3, 2024

ginayeh removed the request for review from dfawley January 3, 2024 16:57

arvindbr8 approved these changes Jan 4, 2024

View reviewed changes

arvindbr8 merged commit 6bc1906 into grpc:master Jan 4, 2024
1 check passed

atollena deleted the a65 branch January 5, 2024 09:01

halvards mentioned this pull request Feb 2, 2024

feat(grpc-xds): Control plane mTLS for Go GoogleCloudPlatform/solutions-workshops#9

Merged

halvards mentioned this pull request Feb 2, 2024

feat(grpc-xds): Control plane mTLS for Go (#9) GoogleCloudPlatform/solutions-workshops#10

Merged

halvards mentioned this pull request Feb 2, 2024

feat(grpc-xds): Control plane mTLS for Go (#9) GoogleCloudPlatform/solutions-workshops#11

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jul 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xds: add support for mTLS Credentials in xDS bootstrap #6757

xds: add support for mTLS Credentials in xDS bootstrap #6757

atollena commented Oct 31, 2023 •

edited

Loading

codecov bot commented Oct 31, 2023 •

edited

Loading

atollena commented Oct 31, 2023

arvindbr8 commented Nov 1, 2023 •

edited

Loading

arvindbr8 left a comment

atollena commented Nov 10, 2023 •

edited

Loading

dfawley commented Nov 10, 2023

atollena commented Dec 22, 2023

easwars commented Dec 26, 2023

atollena commented Dec 27, 2023

arvindbr8 left a comment •

edited

Loading

xds: add support for mTLS Credentials in xDS bootstrap #6757

xds: add support for mTLS Credentials in xDS bootstrap #6757

Conversation

atollena commented Oct 31, 2023 • edited Loading

codecov bot commented Oct 31, 2023 • edited Loading

Codecov Report

atollena commented Oct 31, 2023

arvindbr8 commented Nov 1, 2023 • edited Loading

arvindbr8 left a comment

Choose a reason for hiding this comment

atollena commented Nov 10, 2023 • edited Loading

dfawley commented Nov 10, 2023

atollena commented Dec 22, 2023

easwars commented Dec 26, 2023

atollena commented Dec 27, 2023

arvindbr8 left a comment • edited Loading

Choose a reason for hiding this comment

atollena commented Oct 31, 2023 •

edited

Loading

codecov bot commented Oct 31, 2023 •

edited

Loading

arvindbr8 commented Nov 1, 2023 •

edited

Loading

atollena commented Nov 10, 2023 •

edited

Loading

arvindbr8 left a comment •

edited

Loading