From 998fa434299e0af2a2f2a63bbd31e6dd02946998 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Mon, 7 Dec 2020 15:53:14 -0800 Subject: [PATCH 1/2] xds: `server_features` should be a child of `xds_servers` --- xds/internal/client/bootstrap/bootstrap.go | 14 ++++----- .../client/bootstrap/bootstrap_test.go | 30 +++++++++---------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/xds/internal/client/bootstrap/bootstrap.go b/xds/internal/client/bootstrap/bootstrap.go index e9699717c956..e833a4c91f4f 100644 --- a/xds/internal/client/bootstrap/bootstrap.go +++ b/xds/internal/client/bootstrap/bootstrap.go @@ -91,8 +91,9 @@ type channelCreds struct { } type xdsServer struct { - ServerURI string `json:"server_uri"` - ChannelCreds []channelCreds `json:"channel_creds"` + ServerURI string `json:"server_uri"` + ChannelCreds []channelCreds `json:"channel_creds"` + ServerFeatures []string `json:"server_features"` } // NewConfig returns a new instance of Config initialized by reading the @@ -108,9 +109,9 @@ type xdsServer struct { // "config": // } // ], +// "server_features": [ ... ], // }, // "node": , -// "server_features": [ ... ], // "certificate_providers" : { // "default": { // "plugin_name": "default-plugin-name", @@ -188,12 +189,7 @@ func NewConfig() (*Config, error) { break } } - case "server_features": - var features []string - if err := json.Unmarshal(v, &features); err != nil { - return nil, fmt.Errorf("xds: json.Unmarshal(%v) for field %q failed during bootstrap: %v", string(v), k, err) - } - for _, f := range features { + for _, f := range xs.ServerFeatures { switch f { case serverFeaturesV3: serverSupportsV3 = true diff --git a/xds/internal/client/bootstrap/bootstrap_test.go b/xds/internal/client/bootstrap/bootstrap_test.go index b9d2b9ab63d9..9d5aec26df71 100644 --- a/xds/internal/client/bootstrap/bootstrap_test.go +++ b/xds/internal/client/bootstrap/bootstrap_test.go @@ -163,9 +163,9 @@ var ( "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ { "type": "google_default" } - ] - }], - "server_features" : ["foo", "bar"] + ], + "server_features" : ["foo", "bar"] + }] }`, "serverSupportsV3": ` { @@ -179,9 +179,9 @@ var ( "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ { "type": "google_default" } - ] - }], - "server_features" : ["foo", "bar", "xds_v3"] + ], + "server_features" : ["foo", "bar", "xds_v3"] + }] }`, } metadata = &structpb.Struct{ @@ -548,9 +548,9 @@ func TestNewConfigWithCertificateProviders(t *testing.T) { "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ { "type": "google_default" } - ] + ], + "server_features" : ["foo", "bar", "xds_v3"], }], - "server_features" : ["foo", "bar", "xds_v3"], "certificate_providers": "bad JSON" }`, "allUnknownCertProviders": ` @@ -565,9 +565,9 @@ func TestNewConfigWithCertificateProviders(t *testing.T) { "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ { "type": "google_default" } - ] + ], + "server_features" : ["foo", "bar", "xds_v3"] }], - "server_features" : ["foo", "bar", "xds_v3"], "certificate_providers": { "unknownProviderInstance1": { "plugin_name": "foo", @@ -591,9 +591,9 @@ func TestNewConfigWithCertificateProviders(t *testing.T) { "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ { "type": "google_default" } - ] + ], + "server_features" : ["foo", "bar", "xds_v3"], }], - "server_features" : ["foo", "bar", "xds_v3"], "certificate_providers": { "unknownProviderInstance": { "plugin_name": "foo", @@ -617,9 +617,9 @@ func TestNewConfigWithCertificateProviders(t *testing.T) { "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ { "type": "google_default" } - ] + ], + "server_features" : ["foo", "bar", "xds_v3"] }], - "server_features" : ["foo", "bar", "xds_v3"], "certificate_providers": { "unknownProviderInstance": { "plugin_name": "foo", @@ -692,7 +692,7 @@ func TestNewConfigWithCertificateProviders(t *testing.T) { c, err := NewConfig() if (err != nil) != test.wantErr { - t.Fatalf("NewConfig() returned: (%+v, %v), wantErr: %v", c.CertProviderConfigs, err, test.wantErr) + t.Fatalf("NewConfig() returned: %v, wantErr: %v", err, test.wantErr) } if test.wantErr { return From fc7de5bfa151e440d40bff3ef299d02365a4ab9b Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Mon, 7 Dec 2020 16:08:51 -0800 Subject: [PATCH 2/2] Fix e2e test --- xds/internal/testutils/e2e/bootstrap.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xds/internal/testutils/e2e/bootstrap.go b/xds/internal/testutils/e2e/bootstrap.go index a6a9f22fb44f..9cd5c51ab187 100644 --- a/xds/internal/testutils/e2e/bootstrap.go +++ b/xds/internal/testutils/e2e/bootstrap.go @@ -80,7 +80,7 @@ func SetupBootstrapFile(opts BootstrapOptions) (func(), error) { case TransportV2: // TODO: Add any v2 specific fields. case TransportV3: - cfg.ServerFeatures = append(cfg.ServerFeatures, "xds_v3") + cfg.XdsServers[0].ServerFeatures = append(cfg.XdsServers[0].ServerFeatures, "xds_v3") default: return nil, fmt.Errorf("unsupported xDS transport protocol version: %v", opts.Version) } @@ -103,14 +103,14 @@ func SetupBootstrapFile(opts BootstrapOptions) (func(), error) { } type bootstrapConfig struct { - XdsServers []server `json:"xds_servers,omitempty"` - Node node `json:"node,omitempty"` - ServerFeatures []string `json:"server_features,omitempty"` + XdsServers []server `json:"xds_servers,omitempty"` + Node node `json:"node,omitempty"` } type server struct { - ServerURI string `json:"server_uri,omitempty"` - ChannelCreds []creds `json:"channel_creds,omitempty"` + ServerURI string `json:"server_uri,omitempty"` + ChannelCreds []creds `json:"channel_creds,omitempty"` + ServerFeatures []string `json:"server_features,omitempty"` } type creds struct {