fix(storage): require gRPC >= 1.85 for DirectPath over Interconnect

google/cloud/storage/doc/environment-variables.dox

@@ -39,12 +39,14 @@ protocol, but currently unused.
 
 `GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS_OVER_INTERCONNECT=true`: attempt DirectPath
 over [Cloud Interconnect][interconnect-link]. This bypasses the usual GCE
-environment detection, making DirectPath usable from on-premise hosts. Set it
-to `true` to enable the feature. Set it to `false` to disable the feature even
-when the corresponding option is enabled in code. Only these exact values are
-recognized, and the comparison is case-sensitive; any other value, such as `1`
-or `TRUE`, is treated as if the variable were not set. This environment
-variable takes precedence over the option.
+environment detection, making DirectPath usable from on-premise hosts. Requires
+gRPC 1.85.0 or later; when compiled against an older version, the client logs a
+warning and falls back to the standard endpoint defaults. Set it to `true` to
+enable the feature. Set it to `false` to disable the feature even when the
+corresponding option is enabled in code. Only these exact values are recognized,
+and the comparison is case-sensitive; any other value, such as `1` or `TRUE`, is
+treated as if the variable were not set. This environment variable takes
+precedence over the option.
 
 @see google::cloud::storage_experimental::DirectPathXdsOverInterconnectOption
 

google/cloud/storage/grpc_plugin.h

@@ -138,6 +138,11 @@ struct GrpcMetricsExcludedLabelsOption {
  * [Cloud Interconnect], where the usual GCE environment detection would
  * otherwise disable it.
  *
+ * This requires gRPC 1.85.0 or later. Older versions accept the `force-xds`
+ * query parameter but ignore it, and would silently connect over regular
+ * DirectPath or CloudPath instead. When compiled against an older version, the
+ * client logs a warning and falls back to the standard endpoint defaults.
+ *
  * The default is `false`. The
  * `GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS_OVER_INTERCONNECT` environment variable
  * overrides this option: set it to `"true"` to enable the feature, or to

google/cloud/storage/internal/grpc/channel_telemetry.cc

@@ -14,6 +14,7 @@
 
 #include "google/cloud/storage/internal/grpc/channel_telemetry.h"
 #include "google/cloud/storage/grpc_plugin.h"
+#include "google/cloud/storage/internal/grpc/default_options.h"
 #include "google/cloud/common_options.h"
 #include "google/cloud/internal/async_connection_ready.h"
 #include "google/cloud/internal/call_context.h"
@@ -23,6 +24,7 @@
 #include "absl/strings/match.h"
 #include "absl/strings/str_split.h"
 #include <grpcpp/channel.h>
+#include <grpcpp/grpcpp.h>
 #include <algorithm>
 #include <chrono>
 #include <cstddef>
@@ -171,10 +173,29 @@ TransportType DetectTransportType(std::string_view endpoint) {
 }
 
 void LogChannelConfiguration(Options const& options) {
+  LogChannelConfiguration(options, GrpcForceXdsIsSupported());
+}
+
+void LogChannelConfiguration(Options const& options,
+                             bool grpc_supports_force_xds) {
   std::string const& endpoint = options.get<EndpointOption>();
   TransportType const transport = DetectTransportType(endpoint);
   bool const requested =
       options.get<storage_experimental::DirectPathXdsOverInterconnectOption>();
+  // Check gRPC version support before the endpoint-override warning: on gRPC <
+  // 1.85 `DefaultOptionsGrpc()` intentionally leaves the default endpoint in
+  // place, and an application-supplied `?force-xds` endpoint is also ignored by
+  // the `google-c2p` resolver without a diagnostic.
+  if (!grpc_supports_force_xds &&
+      (requested || transport == TransportType::kDirectPathInterconnect)) {
+    GCP_LOG(WARNING) << "DirectPath over Interconnect requires gRPC >= 1.85.0, "
+                     << "as older versions of the `google-c2p` resolver ignore "
+                     << "the `force-xds` query parameter, but this application "
+                     << "uses gRPC " << grpc::Version()
+                     << ". transport_type=" << ToString(transport)
+                     << ", endpoint=" << endpoint;
+    return;
+  }
   if (requested && transport != TransportType::kDirectPathInterconnect) {
     GCP_LOG(WARNING) << "DirectPath over Interconnect is enabled, but the "
                      << "effective endpoint does not request it. Setting "

google/cloud/storage/internal/grpc/channel_telemetry.h

@@ -60,13 +60,18 @@ TransportType DetectTransportType(std::string_view endpoint);
  * Logs the effective connectivity configuration.
  *
  * Emits a `WARNING` when the application asked for DirectPath over
- * Interconnect but the effective endpoint does not use it, which happens, for
- * instance, when the application also sets `EndpointOption` or
- * `UniverseDomainOption`. Otherwise emits an `INFO` line describing the
- * transport in use.
+ * Interconnect but the effective endpoint does not use it (for instance, when
+ * `EndpointOption` or `UniverseDomainOption` is also set), or when `force-xds`
+ * is requested on a gRPC version that ignores it. Otherwise emits an `INFO`
+ * line describing the transport in use.
  */
 void LogChannelConfiguration(Options const& options);
 
+/// Overload of `LogChannelConfiguration` that takes an explicit `force-xds`
+/// support flag for testing.
+void LogChannelConfiguration(Options const& options,
+                             bool grpc_supports_force_xds);
+
 /// Logs the outcome of waiting for the first channel to become ready.
 void LogChannelReady(TransportType transport,
                      std::chrono::steady_clock::duration elapsed,

google/cloud/storage/internal/grpc/channel_telemetry_test.cc

@@ -14,6 +14,7 @@
 
 #include "google/cloud/storage/internal/grpc/channel_telemetry.h"
 #include "google/cloud/storage/grpc_plugin.h"
+#include "google/cloud/storage/internal/grpc/default_options.h"
 #include "google/cloud/common_options.h"
 #include "google/cloud/internal/background_threads_impl.h"
 #include "google/cloud/internal/make_status.h"
@@ -141,8 +142,8 @@ TEST(ChannelTelemetry, LogChannelConfigurationInterconnect) {
   LogChannelConfiguration(
       Options{}
           .set<EndpointOption>(kInterconnectEndpoint)
-          .set<storage_experimental::DirectPathXdsOverInterconnectOption>(
-              true));
+          .set<storage_experimental::DirectPathXdsOverInterconnectOption>(true),
+      /*grpc_supports_force_xds=*/true);
   auto const lines = log.ExtractLines();
   EXPECT_THAT(lines,
               Contains(AllOf(HasSubstr("Configured gRPC transport"),
@@ -160,8 +161,8 @@ TEST(ChannelTelemetry, LogChannelConfigurationRequestedButNotApplied) {
   LogChannelConfiguration(
       Options{}
           .set<EndpointOption>(kCloudPathEndpoint)
-          .set<storage_experimental::DirectPathXdsOverInterconnectOption>(
-              true));
+          .set<storage_experimental::DirectPathXdsOverInterconnectOption>(true),
+      /*grpc_supports_force_xds=*/true);
   auto const lines = log.ExtractLines();
   EXPECT_THAT(lines, Contains(AllOf(
                          HasSubstr("DirectPath over Interconnect is enabled"),
@@ -170,6 +171,69 @@ TEST(ChannelTelemetry, LogChannelConfigurationRequestedButNotApplied) {
   EXPECT_THAT(lines, Each(Not(HasSubstr("Configured gRPC transport"))));
 }
 
+/// @test Verify that when gRPC does not support `force-xds`, enabling the
+/// option logs the gRPC version requirement rather than the `EndpointOption`
+/// override warning.
+TEST(ChannelTelemetry, LogChannelConfigurationUnsupportedGrpc) {
+  testing_util::ScopedLog log;
+  LogChannelConfiguration(
+      Options{}
+          .set<EndpointOption>(kCloudPathEndpoint)
+          .set<storage_experimental::DirectPathXdsOverInterconnectOption>(true),
+      /*grpc_supports_force_xds=*/false);
+  std::vector<std::string> const lines = log.ExtractLines();
+  EXPECT_THAT(
+      lines,
+      Contains(AllOf(HasSubstr("DirectPath over Interconnect requires gRPC"),
+                     HasSubstr("1.85.0"), HasSubstr(grpc::Version()),
+                     HasSubstr("transport_type=CloudPath"),
+                     HasSubstr("endpoint=storage.googleapis.com"))));
+  EXPECT_THAT(lines, Each(Not(HasSubstr("Configured gRPC transport"))));
+  EXPECT_THAT(lines, Each(Not(HasSubstr("`EndpointOption`"))));
+}
+
+/// @test Verify that an application-supplied `?force-xds` endpoint also logs a
+/// warning when gRPC does not support `force-xds`.
+TEST(ChannelTelemetry, LogChannelConfigurationUnsupportedGrpcExplicitEndpoint) {
+  testing_util::ScopedLog log;
+  LogChannelConfiguration(Options{}.set<EndpointOption>(kInterconnectEndpoint),
+                          /*grpc_supports_force_xds=*/false);
+  std::vector<std::string> const lines = log.ExtractLines();
+  EXPECT_THAT(lines,
+              Contains(AllOf(HasSubstr("1.85.0"), HasSubstr(grpc::Version()),
+                             HasSubstr("transport_type=DirectPathInterconnect"),
+                             HasSubstr(kInterconnectEndpoint))));
+  EXPECT_THAT(lines, Each(Not(HasSubstr("Configured gRPC transport"))));
+}
+
+/// @test Verify no warning is logged on older gRPC when DirectPath over
+/// Interconnect is neither requested nor present in the endpoint.
+TEST(ChannelTelemetry, LogChannelConfigurationUnsupportedGrpcNotRequested) {
+  testing_util::ScopedLog log;
+  LogChannelConfiguration(Options{}.set<EndpointOption>(kDirectPathEndpoint),
+                          /*grpc_supports_force_xds=*/false);
+  std::vector<std::string> const lines = log.ExtractLines();
+  EXPECT_THAT(lines, Contains(AllOf(HasSubstr("Configured gRPC transport"),
+                                    HasSubstr("transport_type=DirectPath,"))));
+  EXPECT_THAT(lines, Each(Not(HasSubstr("1.85.0"))));
+}
+
+/// @test Verify the 1-argument `LogChannelConfiguration` overload delegates
+/// using `GrpcForceXdsIsSupported()`.
+TEST(ChannelTelemetry, LogChannelConfigurationFollowsGrpcVersion) {
+  testing_util::ScopedLog log;
+  LogChannelConfiguration(
+      Options{}
+          .set<EndpointOption>(kCloudPathEndpoint)
+          .set<storage_experimental::DirectPathXdsOverInterconnectOption>(
+              true));
+  EXPECT_THAT(
+      log.ExtractLines(),
+      Contains(HasSubstr(GrpcForceXdsIsSupported()
+                             ? "DirectPath over Interconnect is enabled"
+                             : "DirectPath over Interconnect requires gRPC")));
+}
+
 /// @test Verify a ready channel logs its transport and elapsed time.
 TEST(ChannelTelemetry, LogChannelReadySuccess) {
   testing_util::ScopedLog log;

google/cloud/storage/internal/grpc/default_options.cc

@@ -26,6 +26,8 @@
 #include <grpcpp/grpcpp.h>
 #include <algorithm>
 #include <thread>
+#include <tuple>
+#include <utility>
 
 namespace google {
 namespace cloud {
@@ -62,6 +64,13 @@ int DefaultGrpcNumChannels(std::string const& endpoint) {
 Options DefaultOptionsGrpc(
     Options options,
     std::shared_ptr<internal::GcpDetector> const& gcp_detector) {
+  return DefaultOptionsGrpc(std::move(options), gcp_detector,
+                            GrpcForceXdsIsSupported());
+}
+
+Options DefaultOptionsGrpc(
+    Options options, std::shared_ptr<internal::GcpDetector> const& gcp_detector,
+    bool grpc_supports_force_xds) {
   using ::google::cloud::internal::GetEnv;
   // Experiments show that gRPC gets better upload throughput when the upload
   // buffer is at least 32MiB.
@@ -122,12 +131,18 @@ Options DefaultOptionsGrpc(
           false);
     }
   }
+  // gRPC < 1.85 ignores the `force-xds` query parameter, which would silently
+  // connect over regular DirectPath or CloudPath while telemetry reports
+  // `DirectPathInterconnect`. Leave the option set so
+  // `LogChannelConfiguration()` can warn that the request was ignored.
+  //     https://github.com/grpc/grpc/pull/42980
   bool const direct_path_interconnect =
+      grpc_supports_force_xds &&
       options.get<storage_experimental::DirectPathXdsOverInterconnectOption>();
 
   // Unless the application configured an endpoint or universe domain, default
   // to direct connectivity: the Interconnect target when that feature is
-  // enabled, otherwise the standard target when running in GCP.
+  // enabled and supported, otherwise the standard target when running in GCP.
   if (!options.has<EndpointOption>() &&
       !options.has<internal::UniverseDomainOption>()) {
     if (direct_path_interconnect) {
@@ -186,6 +201,21 @@ bool GrpcEnableMetricsIsSafe() {
 #endif  // GRPC_CPP_VERSION_MAJOR
 }
 
+bool GrpcForceXdsIsSupported(int major, int minor, int patch) {
+  // The `google-c2p` resolver started honoring `force-xds` in gRPC 1.85.0:
+  //     https://github.com/grpc/grpc/pull/42980
+  return std::make_tuple(major, minor, patch) >= std::make_tuple(1, 85, 0);
+}
+
+bool GrpcForceXdsIsSupported() {
+#ifndef GRPC_CPP_VERSION_MAJOR
+  return false;
+#else
+  return GrpcForceXdsIsSupported(GRPC_CPP_VERSION_MAJOR, GRPC_CPP_VERSION_MINOR,
+                                 GRPC_CPP_VERSION_PATCH);
+#endif  // GRPC_CPP_VERSION_MAJOR
+}
+
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage_internal
 }  // namespace cloud

google/cloud/storage/internal/grpc/default_options.h

@@ -34,9 +34,20 @@ Options DefaultOptionsGrpc(Options = {},
                            std::shared_ptr<internal::GcpDetector> const&
                                gcp_detector = internal::MakeGcpDetector());
 
+/// Overload of `DefaultOptionsGrpc` that takes an explicit `force-xds` support
+/// flag for testing.
+Options DefaultOptionsGrpc(
+    Options options, std::shared_ptr<internal::GcpDetector> const& gcp_detector,
+    bool grpc_supports_force_xds);
+
 bool GrpcEnableMetricsIsSafe(int major, int minor, int patch);
 bool GrpcEnableMetricsIsSafe();
 
+/// Returns true if gRPC `major.minor.patch` honors the `force-xds` query
+/// parameter in `google-c2p` targets (gRPC >= 1.85.0).
+bool GrpcForceXdsIsSupported(int major, int minor, int patch);
+bool GrpcForceXdsIsSupported();
+
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage_internal
 }  // namespace cloud