Merge pull request #19014 from yashykt/defsccvferror

Add tests for setting a channel to lame on an invalid default service config

src/core/ext/filters/client_channel/client_channel.cc

@@ -1069,8 +1069,6 @@ ChannelData::ChannelData(grpc_channel_element_args* args, grpc_error** error)
   // Get default service config
   const char* service_config_json = grpc_channel_arg_get_string(
       grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVICE_CONFIG));
-  // TODO(yashkt): Make sure we set the channel in TRANSIENT_FAILURE on an
-  // invalid default service config
   if (service_config_json != nullptr) {
     *error = GRPC_ERROR_NONE;
     default_service_config_ = ServiceConfig::Create(service_config_json, error);

test/cpp/end2end/service_config_end2end_test.cc

@@ -237,6 +237,14 @@ class ServiceConfigEnd2endTest : public ::testing::Test {
     return ::grpc::CreateCustomChannel("fake:///", creds_, args);
   }
 
+  std::shared_ptr<Channel> BuildChannelWithInvalidDefaultServiceConfig() {
+    ChannelArguments args;
+    args.SetServiceConfigJSON(InvalidDefaultServiceConfig());
+    args.SetPointer(GRPC_ARG_FAKE_RESOLVER_RESPONSE_GENERATOR,
+                    response_generator_.get());
+    return ::grpc::CreateCustomChannel("fake:///", creds_, args);
+  }
+
   bool SendRpc(
       const std::unique_ptr<grpc::testing::EchoTestService::Stub>& stub,
       EchoResponse* response = nullptr, int timeout_ms = 1000,
@@ -402,7 +410,7 @@ class ServiceConfigEnd2endTest : public ::testing::Test {
   }
 
   const char* InvalidDefaultServiceConfig() {
-    return "{\"version\": \"invalid_default\"}";
+    return "{\"version\": \"invalid_default\"";
   }
 
   const grpc::string server_host_;
@@ -549,6 +557,51 @@ TEST_F(ServiceConfigEnd2endTest,
   CheckRpcSendFailure(stub);
 }
 
+TEST_F(ServiceConfigEnd2endTest, InvalidDefaultServiceConfigTest) {
+  StartServers(1);
+  auto channel = BuildChannelWithInvalidDefaultServiceConfig();
+  auto stub = BuildStub(channel);
+  // An invalid default service config results in a lame channel which fails all
+  // RPCs
+  CheckRpcSendFailure(stub);
+}
+
+TEST_F(ServiceConfigEnd2endTest,
+       InvalidDefaultServiceConfigTestWithValidServiceConfig) {
+  StartServers(1);
+  auto channel = BuildChannelWithInvalidDefaultServiceConfig();
+  auto stub = BuildStub(channel);
+  CheckRpcSendFailure(stub);
+  // An invalid default service config results in a lame channel which fails all
+  // RPCs
+  SetNextResolutionValidServiceConfig(GetServersPorts());
+  CheckRpcSendFailure(stub);
+}
+
+TEST_F(ServiceConfigEnd2endTest,
+       InvalidDefaultServiceConfigTestWithInvalidServiceConfig) {
+  StartServers(1);
+  auto channel = BuildChannelWithInvalidDefaultServiceConfig();
+  auto stub = BuildStub(channel);
+  CheckRpcSendFailure(stub);
+  // An invalid default service config results in a lame channel which fails all
+  // RPCs
+  SetNextResolutionInvalidServiceConfig(GetServersPorts());
+  CheckRpcSendFailure(stub);
+}
+
+TEST_F(ServiceConfigEnd2endTest,
+       InvalidDefaultServiceConfigTestWithNoServiceConfig) {
+  StartServers(1);
+  auto channel = BuildChannelWithInvalidDefaultServiceConfig();
+  auto stub = BuildStub(channel);
+  CheckRpcSendFailure(stub);
+  // An invalid default service config results in a lame channel which fails all
+  // RPCs
+  SetNextResolutionNoServiceConfig(GetServersPorts());
+  CheckRpcSendFailure(stub);
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace grpc