From 1ac63293b0a58c0afdc978e234d989fda3f7bccc Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 13 Dec 2024 15:54:19 -0800 Subject: [PATCH 1/2] xds: Unexpected types in server_features should be ignored It was clearly defined in gRFC A30. The relevant text was copied as a comment in the code. As discovered due to grpc/grpc-go#7932 --- .../io/grpc/xds/client/BootstrapperImpl.java | 4 +++- .../io/grpc/xds/GrpcBootstrapperImplTest.java | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java index 7ef739c8048..c929da269fd 100644 --- a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java @@ -234,7 +234,9 @@ private List parseServerInfos(List rawServerConfigs, XdsLogger lo Object implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri); boolean ignoreResourceDeletion = false; - List serverFeatures = JsonUtil.getListOfStrings(serverConfig, "server_features"); + // "For forward compatibility reasons, the client will ignore any entry in the list that it + // does not understand, regardless of type." + List serverFeatures = JsonUtil.getList(serverConfig, "server_features"); if (serverFeatures != null) { logger.log(XdsLogLevel.INFO, "Server features: {0}", serverFeatures); ignoreResourceDeletion = serverFeatures.contains(SERVER_FEATURE_IGNORE_RESOURCE_DELETION); diff --git a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java index 30ea76b54f2..feb2525cc9b 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java @@ -650,6 +650,26 @@ public void serverFeatureIgnoreResourceDeletion_xdsV3() throws XdsInitialization assertThat(serverInfo.ignoreResourceDeletion()).isTrue(); } + @Test + public void serverFeatures_ignoresUnknownValues() throws XdsInitializationException { + String rawData = "{\n" + + " \"xds_servers\": [\n" + + " {\n" + + " \"server_uri\": \"" + SERVER_URI + "\",\n" + + " \"channel_creds\": [\n" + + " {\"type\": \"insecure\"}\n" + + " ],\n" + + " \"server_features\": [null, {}, 3, true, \"unexpected\", \"trusted_xds_server\"]\n" + + " }\n" + + " ]\n" + + "}"; + + bootstrapper.setFileReader(createFileReader(BOOTSTRAP_FILE_PATH, rawData)); + BootstrapInfo info = bootstrapper.bootstrap(); + ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); + assertThat(serverInfo.isTrustedXdsServer()).isTrue(); + } + @Test public void notFound() { bootstrapper.bootstrapPathFromEnvVar = null; From 24b769630e13c99ce29d31aa564f5f828593bb0d Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 16 Dec 2024 16:02:49 -0800 Subject: [PATCH 2/2] In test, replace trusted_xds_server with ignore_resource_deletion trusted_xds_server doesn't exist on this branch. --- xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java index feb2525cc9b..b9e4428c59a 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java @@ -659,7 +659,9 @@ public void serverFeatures_ignoresUnknownValues() throws XdsInitializationExcept + " \"channel_creds\": [\n" + " {\"type\": \"insecure\"}\n" + " ],\n" - + " \"server_features\": [null, {}, 3, true, \"unexpected\", \"trusted_xds_server\"]\n" + + " \"server_features\": [\n" + + " null, {}, 3, true, \"unexpected\", \"ignore_resource_deletion\"\n" + + " ]\n" + " }\n" + " ]\n" + "}"; @@ -667,7 +669,7 @@ public void serverFeatures_ignoresUnknownValues() throws XdsInitializationExcept bootstrapper.setFileReader(createFileReader(BOOTSTRAP_FILE_PATH, rawData)); BootstrapInfo info = bootstrapper.bootstrap(); ServerInfo serverInfo = Iterables.getOnlyElement(info.servers()); - assertThat(serverInfo.isTrustedXdsServer()).isTrue(); + assertThat(serverInfo.ignoreResourceDeletion()).isTrue(); } @Test