Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 78a9c3b

Browse filesBrowse files
scotthartdevbww
scotthart
authored and
devbww
committed
impl: prevent logging options from being overwritten (googleapis#15090)
1 parent 4eaba95 commit 78a9c3b
Copy full SHA for 78a9c3b

File tree

Expand file treeCollapse file tree

4 files changed

+62
-2
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+62
-2
lines changed

‎google/cloud/internal/credentials_impl.cc

Copy file name to clipboardExpand all lines: google/cloud/internal/credentials_impl.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ Options PopulateAuthOptions(Options options) {
4242
.set<AccessTokenLifetimeOption>(kDefaultTokenLifetime));
4343
// Then apply any overrides.
4444
return MergeOptions(
45-
Options{}.set<LoggingComponentsOption>(DefaultTracingComponents()),
46-
std::move(options));
45+
std::move(options),
46+
Options{}.set<LoggingComponentsOption>(DefaultTracingComponents()));
4747
}
4848

4949
void CredentialsVisitor::dispatch(Credentials const& credentials,

‎google/cloud/internal/credentials_impl_test.cc

Copy file name to clipboardExpand all lines: google/cloud/internal/credentials_impl_test.cc
+38Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/internal/credentials_impl.h"
16+
#include "google/cloud/internal/populate_common_options.h"
1617
#include "google/cloud/testing_util/credentials.h"
1718
#include <gmock/gmock.h>
1819

@@ -24,6 +25,7 @@ namespace {
2425

2526
using ::google::cloud::testing_util::TestCredentialsVisitor;
2627
using ::testing::ElementsAre;
28+
using ::testing::Eq;
2729
using ::testing::IsEmpty;
2830
using ::testing::IsNull;
2931

@@ -122,6 +124,42 @@ TEST(Credentials, ApiKeyCredentials) {
122124
EXPECT_EQ("api-key", visitor.api_key);
123125
}
124126

127+
TEST(PopulateAuthOptions, EmptyOptions) {
128+
auto result_options = PopulateAuthOptions(Options{});
129+
130+
EXPECT_THAT(result_options.get<ScopesOption>(),
131+
ElementsAre("https://www.googleapis.com/auth/cloud-platform"));
132+
EXPECT_THAT(result_options.get<AccessTokenLifetimeOption>(),
133+
Eq(std::chrono::hours(1)));
134+
EXPECT_THAT(result_options.get<LoggingComponentsOption>(),
135+
Eq(DefaultTracingComponents()));
136+
}
137+
138+
TEST(PopulateAuthOptions, ExistingNonIntersectingOptions) {
139+
auto options = Options{}.set<EndpointOption>("my-endpoint");
140+
auto result_options = PopulateAuthOptions(options);
141+
142+
EXPECT_THAT(result_options.get<EndpointOption>(), Eq("my-endpoint"));
143+
EXPECT_THAT(result_options.get<ScopesOption>(),
144+
ElementsAre("https://www.googleapis.com/auth/cloud-platform"));
145+
EXPECT_THAT(result_options.get<AccessTokenLifetimeOption>(),
146+
Eq(std::chrono::hours(1)));
147+
EXPECT_THAT(result_options.get<LoggingComponentsOption>(),
148+
Eq(DefaultTracingComponents()));
149+
}
150+
151+
TEST(PopulateAuthOptions, ExistingIntersectingOptions) {
152+
auto options = Options{}
153+
.set<ScopesOption>({"my-scope"})
154+
.set<LoggingComponentsOption>({"my-logging-component"});
155+
auto result_options = PopulateAuthOptions(options);
156+
EXPECT_THAT(result_options.get<ScopesOption>(), ElementsAre("my-scope"));
157+
EXPECT_THAT(result_options.get<AccessTokenLifetimeOption>(),
158+
Eq(std::chrono::hours(1)));
159+
EXPECT_THAT(result_options.get<LoggingComponentsOption>(),
160+
ElementsAre("my-logging-component"));
161+
}
162+
125163
} // namespace
126164
} // namespace internal
127165
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

‎google/cloud/internal/populate_common_options.cc

Copy file name to clipboardExpand all lines: google/cloud/internal/populate_common_options.cc
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,17 @@ TracingOptions DefaultTracingOptions() {
9494
return TracingOptions{}.SetOptions(*tracing_options);
9595
}
9696

97+
// TODO(#15089): Determine if this function needs to preserve more (or all) of
98+
// the options passed in.
9799
Options MakeAuthOptions(Options const& options) {
98100
Options opts;
99101
if (options.has<OpenTelemetryTracingOption>()) {
100102
opts.set<OpenTelemetryTracingOption>(
101103
options.get<OpenTelemetryTracingOption>());
102104
}
105+
if (options.has<LoggingComponentsOption>()) {
106+
opts.set<LoggingComponentsOption>(options.get<LoggingComponentsOption>());
107+
}
103108
return opts;
104109
}
105110

‎google/cloud/internal/populate_common_options_test.cc

Copy file name to clipboardExpand all lines: google/cloud/internal/populate_common_options_test.cc
+17Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,23 @@ TEST(MakeAuthOptions, WithTracing) {
299299
EXPECT_TRUE(auth_options.get<OpenTelemetryTracingOption>());
300300
}
301301

302+
TEST(MakeAuthOptions, WithoutLoggingComponents) {
303+
auto options = Options{}.set<EndpointOption>("endpoint_option");
304+
auto auth_options = MakeAuthOptions(options);
305+
EXPECT_FALSE(auth_options.has<EndpointOption>());
306+
EXPECT_FALSE(auth_options.has<LoggingComponentsOption>());
307+
}
308+
309+
TEST(MakeAuthOptions, WithLoggingComponents) {
310+
auto options = Options{}
311+
.set<EndpointOption>("endpoint_option")
312+
.set<LoggingComponentsOption>({"logging_component"});
313+
auto auth_options = MakeAuthOptions(options);
314+
EXPECT_FALSE(auth_options.has<EndpointOption>());
315+
EXPECT_THAT(auth_options.get<LoggingComponentsOption>(),
316+
ElementsAre("logging_component"));
317+
}
318+
302319
} // namespace
303320
} // namespace internal
304321
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.