From 5abf8a40f556bd9066a150caa7de19c3645d63a7 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 3 Sep 2020 13:22:14 -0700 Subject: [PATCH 1/4] Make channel_creds required in bootstrap file. --- .../main/java/io/grpc/xds/Bootstrapper.java | 24 +++++++++---------- .../java/io/grpc/xds/BootstrapperTest.java | 11 +++++++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/Bootstrapper.java b/xds/src/main/java/io/grpc/xds/Bootstrapper.java index a0bacd4190b..4efa03dc11f 100644 --- a/xds/src/main/java/io/grpc/xds/Bootstrapper.java +++ b/xds/src/main/java/io/grpc/xds/Bootstrapper.java @@ -101,19 +101,19 @@ public static BootstrapInfo parseConfig(String rawData) throws IOException { logger.log(XdsLogLevel.INFO, "xDS server URI: {0}", serverUri); List channelCredsOptions = new ArrayList<>(); List rawChannelCredsList = JsonUtil.getList(serverConfig, "channel_creds"); - // List of channel creds is optional. - if (rawChannelCredsList != null) { - List> channelCredsList = JsonUtil.checkObjectList(rawChannelCredsList); - for (Map channelCreds : channelCredsList) { - String type = JsonUtil.getString(channelCreds, "type"); - if (type == null) { - throw new IOException("Invalid bootstrap: 'xds_servers' contains server with " - + "unknown type 'channel_creds'."); - } - logger.log(XdsLogLevel.INFO, "Channel credentials option: {0}", type); - ChannelCreds creds = new ChannelCreds(type, JsonUtil.getObject(channelCreds, "config")); - channelCredsOptions.add(creds); + if (rawChannelCredsList == null) { + throw new IOException("Invalid bootstrap: 'channel_creds' required"); + } + List> channelCredsList = JsonUtil.checkObjectList(rawChannelCredsList); + for (Map channelCreds : channelCredsList) { + String type = JsonUtil.getString(channelCreds, "type"); + if (type == null) { + throw new IOException("Invalid bootstrap: 'xds_servers' contains server with " + + "unknown type 'channel_creds'."); } + logger.log(XdsLogLevel.INFO, "Channel credentials option: {0}", type); + ChannelCreds creds = new ChannelCreds(type, JsonUtil.getObject(channelCreds, "config")); + channelCredsOptions.add(creds); } List serverFeatures = JsonUtil.getListOfStrings(serverConfig, "server_features"); if (serverFeatures != null) { diff --git a/xds/src/test/java/io/grpc/xds/BootstrapperTest.java b/xds/src/test/java/io/grpc/xds/BootstrapperTest.java index 17f42aad73b..13fbb6232cc 100644 --- a/xds/src/test/java/io/grpc/xds/BootstrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/BootstrapperTest.java @@ -24,6 +24,7 @@ import io.grpc.internal.GrpcUtil; import io.grpc.internal.GrpcUtil.GrpcBuildVersion; import io.grpc.xds.Bootstrapper.BootstrapInfo; +import io.grpc.xds.Bootstrapper.ChannelCreds; import io.grpc.xds.Bootstrapper.ServerInfo; import io.grpc.xds.EnvoyProtoData.Locality; import io.grpc.xds.EnvoyProtoData.Node; @@ -234,7 +235,10 @@ public void parseBootstrap_minimalUsableData() throws IOException { String rawData = "{\n" + " \"xds_servers\": [\n" + " {\n" - + " \"server_uri\": \"trafficdirector.googleapis.com:443\"\n" + + " \"server_uri\": \"trafficdirector.googleapis.com:443\",\n" + + " \"channel_creds\": [\n" + + " {\"type\": \"insecure\"}\n" + + " ]\n" + " }\n" + " ]\n" + "}"; @@ -243,7 +247,10 @@ public void parseBootstrap_minimalUsableData() throws IOException { assertThat(info.getServers()).hasSize(1); ServerInfo serverInfo = Iterables.getOnlyElement(info.getServers()); assertThat(serverInfo.getServerUri()).isEqualTo("trafficdirector.googleapis.com:443"); - assertThat(serverInfo.getChannelCredentials()).isEmpty(); + assertThat(serverInfo.getChannelCredentials()).hasSize(1); + ChannelCreds creds = Iterables.getOnlyElement(serverInfo.getChannelCredentials()); + assertThat(creds.getType()).isEqualTo("insecure"); + assertThat(creds.getConfig()).isNull(); assertThat(info.getNode()).isEqualTo(getNodeBuilder().build()); } From d47423b72632908ae8531bf51e39762f91b2ef7c Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 10 Sep 2020 14:52:36 -0700 Subject: [PATCH 2/4] Do not allow empty list of channel creds. --- xds/src/main/java/io/grpc/xds/Bootstrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/Bootstrapper.java b/xds/src/main/java/io/grpc/xds/Bootstrapper.java index 4efa03dc11f..2f184c1a460 100644 --- a/xds/src/main/java/io/grpc/xds/Bootstrapper.java +++ b/xds/src/main/java/io/grpc/xds/Bootstrapper.java @@ -101,7 +101,7 @@ public static BootstrapInfo parseConfig(String rawData) throws IOException { logger.log(XdsLogLevel.INFO, "xDS server URI: {0}", serverUri); List channelCredsOptions = new ArrayList<>(); List rawChannelCredsList = JsonUtil.getList(serverConfig, "channel_creds"); - if (rawChannelCredsList == null) { + if (rawChannelCredsList == null || rawChannelCredsList.isEmpty()) { throw new IOException("Invalid bootstrap: 'channel_creds' required"); } List> channelCredsList = JsonUtil.checkObjectList(rawChannelCredsList); From 8f760a03f2ea19185aeb11a742bf4d6c97d5c79a Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 10 Sep 2020 17:17:04 -0700 Subject: [PATCH 3/4] Fix test that has empty channel_creds --- xds/src/test/java/io/grpc/xds/BootstrapperTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/BootstrapperTest.java b/xds/src/test/java/io/grpc/xds/BootstrapperTest.java index 13fbb6232cc..75b250f4f04 100644 --- a/xds/src/test/java/io/grpc/xds/BootstrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/BootstrapperTest.java @@ -122,7 +122,9 @@ public void parseBootstrap_validData_multipleXdsServers() throws IOException { + " },\n" + " {\n" + " \"server_uri\": \"trafficdirector-bar.googleapis.com:443\",\n" - + " \"channel_creds\": []\n" + + " \"channel_creds\": [\n" + + " {\"type\": \"insecure\"}" + + " ]\n" + " }\n" + " ]\n" + "}"; @@ -143,7 +145,9 @@ public void parseBootstrap_validData_multipleXdsServers() throws IOException { assertThat(serverInfoList.get(0).getServerFeatures()).contains("xds_v3"); assertThat(serverInfoList.get(1).getServerUri()) .isEqualTo("trafficdirector-bar.googleapis.com:443"); - assertThat(serverInfoList.get(1).getChannelCredentials()).isEmpty(); + assertThat(serverInfoList.get(1).getChannelCredentials().get(0).getType()) + .isEqualTo("insecure"); + assertThat(serverInfoList.get(0).getChannelCredentials().get(0).getConfig()).isNull(); assertThat(info.getNode()).isEqualTo( getNodeBuilder() .setId("ENVOY_NODE_ID") From 38112461273932325ff17546c5942ac7c9989817 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Thu, 10 Sep 2020 17:17:17 -0700 Subject: [PATCH 4/4] Improve exception message. --- xds/src/main/java/io/grpc/xds/Bootstrapper.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/Bootstrapper.java b/xds/src/main/java/io/grpc/xds/Bootstrapper.java index 2f184c1a460..ea59498a272 100644 --- a/xds/src/main/java/io/grpc/xds/Bootstrapper.java +++ b/xds/src/main/java/io/grpc/xds/Bootstrapper.java @@ -102,14 +102,15 @@ public static BootstrapInfo parseConfig(String rawData) throws IOException { List channelCredsOptions = new ArrayList<>(); List rawChannelCredsList = JsonUtil.getList(serverConfig, "channel_creds"); if (rawChannelCredsList == null || rawChannelCredsList.isEmpty()) { - throw new IOException("Invalid bootstrap: 'channel_creds' required"); + throw new IOException( + "Invalid bootstrap: server " + serverUri + " 'channel_creds' required"); } List> channelCredsList = JsonUtil.checkObjectList(rawChannelCredsList); for (Map channelCreds : channelCredsList) { String type = JsonUtil.getString(channelCreds, "type"); if (type == null) { - throw new IOException("Invalid bootstrap: 'xds_servers' contains server with " - + "unknown type 'channel_creds'."); + throw new IOException( + "Invalid bootstrap: server " + serverUri + " with 'channel_creds' type unspecified"); } logger.log(XdsLogLevel.INFO, "Channel credentials option: {0}", type); ChannelCreds creds = new ChannelCreds(type, JsonUtil.getObject(channelCreds, "config"));