impl: prevent logging options from being overwritten (#15090)

Author: scotthart

google/cloud/internal/credentials_impl.cc

@@ -42,8 +42,8 @@ Options PopulateAuthOptions(Options options) {
                        .set<AccessTokenLifetimeOption>(kDefaultTokenLifetime));
   // Then apply any overrides.
   return MergeOptions(
-      Options{}.set<LoggingComponentsOption>(DefaultTracingComponents()),
-      std::move(options));
+      std::move(options),
+      Options{}.set<LoggingComponentsOption>(DefaultTracingComponents()));
 }
 
 void CredentialsVisitor::dispatch(Credentials const& credentials,

google/cloud/internal/credentials_impl_test.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/internal/credentials_impl.h"
+#include "google/cloud/internal/populate_common_options.h"
 #include "google/cloud/testing_util/credentials.h"
 #include <gmock/gmock.h>
 
@@ -24,6 +25,7 @@ namespace {
 
 using ::google::cloud::testing_util::TestCredentialsVisitor;
 using ::testing::ElementsAre;
+using ::testing::Eq;
 using ::testing::IsEmpty;
 using ::testing::IsNull;
 
@@ -122,6 +124,42 @@ TEST(Credentials, ApiKeyCredentials) {
   EXPECT_EQ("api-key", visitor.api_key);
 }
 
+TEST(PopulateAuthOptions, EmptyOptions) {
+  auto result_options = PopulateAuthOptions(Options{});
+
+  EXPECT_THAT(result_options.get<ScopesOption>(),
+              ElementsAre("https://www.googleapis.com/auth/cloud-platform"));
+  EXPECT_THAT(result_options.get<AccessTokenLifetimeOption>(),
+              Eq(std::chrono::hours(1)));
+  EXPECT_THAT(result_options.get<LoggingComponentsOption>(),
+              Eq(DefaultTracingComponents()));
+}
+
+TEST(PopulateAuthOptions, ExistingNonIntersectingOptions) {
+  auto options = Options{}.set<EndpointOption>("my-endpoint");
+  auto result_options = PopulateAuthOptions(options);
+
+  EXPECT_THAT(result_options.get<EndpointOption>(), Eq("my-endpoint"));
+  EXPECT_THAT(result_options.get<ScopesOption>(),
+              ElementsAre("https://www.googleapis.com/auth/cloud-platform"));
+  EXPECT_THAT(result_options.get<AccessTokenLifetimeOption>(),
+              Eq(std::chrono::hours(1)));
+  EXPECT_THAT(result_options.get<LoggingComponentsOption>(),
+              Eq(DefaultTracingComponents()));
+}
+
+TEST(PopulateAuthOptions, ExistingIntersectingOptions) {
+  auto options = Options{}
+                     .set<ScopesOption>({"my-scope"})
+                     .set<LoggingComponentsOption>({"my-logging-component"});
+  auto result_options = PopulateAuthOptions(options);
+  EXPECT_THAT(result_options.get<ScopesOption>(), ElementsAre("my-scope"));
+  EXPECT_THAT(result_options.get<AccessTokenLifetimeOption>(),
+              Eq(std::chrono::hours(1)));
+  EXPECT_THAT(result_options.get<LoggingComponentsOption>(),
+              ElementsAre("my-logging-component"));
+}
+
 }  // namespace
 }  // namespace internal
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

google/cloud/internal/populate_common_options.cc

@@ -94,12 +94,17 @@ TracingOptions DefaultTracingOptions() {
   return TracingOptions{}.SetOptions(*tracing_options);
 }
 
+// TODO(#15089): Determine if this function needs to preserve more (or all) of
+// the options passed in.
 Options MakeAuthOptions(Options const& options) {
   Options opts;
   if (options.has<OpenTelemetryTracingOption>()) {
     opts.set<OpenTelemetryTracingOption>(
         options.get<OpenTelemetryTracingOption>());
   }
+  if (options.has<LoggingComponentsOption>()) {
+    opts.set<LoggingComponentsOption>(options.get<LoggingComponentsOption>());
+  }
   return opts;
 }
 

google/cloud/internal/populate_common_options_test.cc

@@ -299,6 +299,23 @@ TEST(MakeAuthOptions, WithTracing) {
   EXPECT_TRUE(auth_options.get<OpenTelemetryTracingOption>());
 }
 
+TEST(MakeAuthOptions, WithoutLoggingComponents) {
+  auto options = Options{}.set<EndpointOption>("endpoint_option");
+  auto auth_options = MakeAuthOptions(options);
+  EXPECT_FALSE(auth_options.has<EndpointOption>());
+  EXPECT_FALSE(auth_options.has<LoggingComponentsOption>());
+}
+
+TEST(MakeAuthOptions, WithLoggingComponents) {
+  auto options = Options{}
+                     .set<EndpointOption>("endpoint_option")
+                     .set<LoggingComponentsOption>({"logging_component"});
+  auto auth_options = MakeAuthOptions(options);
+  EXPECT_FALSE(auth_options.has<EndpointOption>());
+  EXPECT_THAT(auth_options.get<LoggingComponentsOption>(),
+              ElementsAre("logging_component"));
+}
+
 }  // namespace
 }  // namespace internal
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END