Skip to content

xds: server_features should be a child of xds_servers and not a sibling #4087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
easwars merged 2 commits into from
Dec 8, 2020
Merged

xds: server_features should be a child of xds_servers and not a sibling #4087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
998fa43
xds: `server_features` should be a child of `xds_servers`
easwars Dec 7, 2020
fc7de5b
Fix e2e test
easwars Dec 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions xds/internal/client/bootstrap/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -108,9 +109,9 @@ type xdsServer struct {
// "config": <JSON object containing config for the type>
// }
// ],
// "server_features": [ ... ],
// },
// "node": <JSON form of Node proto>,
// "server_features": [ ... ],
// "certificate_providers" : {
// "default": {
// "plugin_name": "default-plugin-name",
Expand Down Expand Up @@ -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
Expand Down
30 changes: 15 additions & 15 deletions xds/internal/client/bootstrap/bootstrap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": `
{
Expand All @@ -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{
Expand Down Expand Up @@ -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": `
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions xds/internal/testutils/e2e/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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 {
Expand Down