xds/bootstrap: add trusted_xds_server server feature

[Pranjali-2501]

This PR implements the Bootstrap config changes for gRFC A81.

Authority rewriting is a security-sensitive feature that should only be enabled when the xDS server is explicitly trusted to provide such configuration. gRFC A81 specifies that this trust is indicated by adding trusted_xds_server to the server_features list for a given server in the bootstrap file.

RELEASE NOTES: None

[codecov]

Codecov Report

❌ Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.34%. Comparing base (c45d8e6) to head (680ebd3).
⚠️ Report is 16 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/xds/xdsclient/clientimpl.go	76.92%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8692      +/-   ##
==========================================
- Coverage   83.42%   83.34%   -0.08%     
==========================================
  Files         416      419       +3     
  Lines       32303    32443     +140     
==========================================
+ Hits        26948    27040      +92     
- Misses       3993     4031      +38     
- Partials     1362     1372      +10     

Files with missing lines	Coverage Δ
internal/xds/bootstrap/bootstrap.go	66.66% <100.00%> (+0.51%)	⬆️
internal/xds/clients/xdsclient/authority.go	79.51% <100.00%> (+0.26%)	⬆️
internal/xds/clients/xdsclient/xdsconfig.go	100.00% <100.00%> (ø)
internal/xds/xdsclient/clientimpl.go	85.45% <76.92%> (-0.96%)	⬇️

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[easwars]

The generic xDS client's configuration can be found here:

grpc-go/internal/xds/clients/xdsclient/xdsconfig.go

Line 75 in 7472d57

type ServerConfig struct {

We need to propagate this newly added server feature to the generic xDS client as well. In order to do that, we need to do the following:

• Either of the following:
  • Add a new field named TrustedXDSServer in the above mentioned ServerConfig struct, or
  • Add a new field named ServerFeatures which is of type []ServerFeature and remove the existing IgnoreResourceDeletion, or
  • Add a new field named ServerFeatures which is of type map[ServerFeature]bool which is the idiomatic way of representing a Set in Go. Again, remove the existing IgnoreResourceDeletion field.
  • I would prefer going with the second or the third approach since there is already a comment there saying IgnoreResourceDeletion will be removed in favor of two new fields when A88 is implemented. Instead of adding one field per server feature, we could instead have a container to store the server features.
• Add a method in the ServerConfig struct of the generic xDS client to check if a server feature exists. This could be something like:

func (s *ServerConfig) SupportsServerFeature(feature ServerFeature) bool { ... }

• Introduce exported constants for the supported server features in the generic xDS client. This could be internal/xds/clients/xdsclient/xdsconfig.go. Something like:

type ServerFeature int
const (
	ServerFeatureIgnoreResourceDeletion ServerFeature = iota
	ServerFeatureTrustedXDSServer
)

• Remove the existing direct reference to IgnoreResourceDeletion here:

  grpc-go/internal/xds/clients/xdsclient/authority.go

  Line 473 in 7472d57

  if serverConfig.IgnoreResourceDeletion {

  and replace it with a call to serverConfig.SupportsServerFeature(ServerFeatureIgnoreResourceDeletion)
• Add logic to populate the server features in the generic xDS client config here:

  grpc-go/internal/xds/xdsclient/clientimpl.go

  Line 166 in 7472d57

  func buildXDSClientConfig(config *bootstrap.Config, metricsRecorder estats.MetricsRecorder, target string, watchExpiryTimeout time.Duration) (xdsclient.Config, error) {

  • While you are here, you could also refactor the code a little to pull out the common logic to from an internal server config to an external (or generic) server config. That code is currently repeated twice. If you could pull it out into a separate function, that would be nicer
• Update tests

[easwars]

@Pranjali-2501 : Please assign it back to me once you've handled the comment. Thanks.

[easwars]

LGTM, modulo minor nits

[easwars]

Nit: You could get rid of the newline and add a trailing comment to this line saying this field stores a bitmap of supported features.

[Pranjali-2501]

Done.

[easwars]

Nit: Please use a separate line for individual fields (in sub-struct fields as well). I understand this is continuing to do what the existing code was doing. But that happened to be a part of a giant refactor, and it was often easy to miss things during the review. See: go/go-style/decisions#literal-formatting

Thanks.

[Pranjali-2501]

Done.