Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cleanup: const-correctness for unified credentials #12073

Merged
merged 1 commit into from
Jul 12, 2023
Merged

cleanup: const-correctness for unified credentials #12073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion google/cloud/credentials.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class Credentials {

private:
friend class internal::CredentialsVisitor;
virtual void dispatch(internal::CredentialsVisitor& visitor) = 0;
virtual void dispatch(internal::CredentialsVisitor& visitor) const = 0;
};

/**
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/internal/credentials_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Options PopulateAuthOptions(Options options) {
std::move(options));
}

void CredentialsVisitor::dispatch(Credentials& credentials,
void CredentialsVisitor::dispatch(Credentials const& credentials,
CredentialsVisitor& visitor) {
credentials.dispatch(visitor);
}
Expand Down
29 changes: 15 additions & 14 deletions google/cloud/internal/credentials_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,15 @@ class ExternalAccountConfig;
class CredentialsVisitor {
public:
virtual ~CredentialsVisitor() = default;
virtual void visit(InsecureCredentialsConfig&) = 0;
virtual void visit(GoogleDefaultCredentialsConfig&) = 0;
virtual void visit(AccessTokenConfig&) = 0;
virtual void visit(ImpersonateServiceAccountConfig&) = 0;
virtual void visit(ServiceAccountConfig&) = 0;
virtual void visit(ExternalAccountConfig&) = 0;

static void dispatch(Credentials& credentials, CredentialsVisitor& visitor);
virtual void visit(InsecureCredentialsConfig const&) = 0;
virtual void visit(GoogleDefaultCredentialsConfig const&) = 0;
virtual void visit(AccessTokenConfig const&) = 0;
virtual void visit(ImpersonateServiceAccountConfig const&) = 0;
virtual void visit(ServiceAccountConfig const&) = 0;
virtual void visit(ExternalAccountConfig const&) = 0;

static void dispatch(Credentials const& credentials,
CredentialsVisitor& visitor);
};

class InsecureCredentialsConfig : public Credentials {
Expand All @@ -60,7 +61,7 @@ class InsecureCredentialsConfig : public Credentials {
Options const& options() const { return options_; }

private:
void dispatch(CredentialsVisitor& v) override { v.visit(*this); }
void dispatch(CredentialsVisitor& v) const override { v.visit(*this); }

Options options_;
};
Expand All @@ -73,7 +74,7 @@ class GoogleDefaultCredentialsConfig : public Credentials {
Options const& options() const { return options_; }

private:
void dispatch(CredentialsVisitor& v) override { v.visit(*this); }
void dispatch(CredentialsVisitor& v) const override { v.visit(*this); }

Options options_;
};
Expand All @@ -89,7 +90,7 @@ class AccessTokenConfig : public Credentials {
Options const& options() const { return options_; }

private:
void dispatch(CredentialsVisitor& v) override { v.visit(*this); }
void dispatch(CredentialsVisitor& v) const override { v.visit(*this); }

AccessToken access_token_;
Options options_;
Expand All @@ -113,7 +114,7 @@ class ImpersonateServiceAccountConfig : public Credentials {
Options const& options() const { return options_; }

private:
void dispatch(CredentialsVisitor& v) override { v.visit(*this); }
void dispatch(CredentialsVisitor& v) const override { v.visit(*this); }

std::shared_ptr<Credentials> base_credentials_;
std::string target_service_account_;
Expand All @@ -128,7 +129,7 @@ class ServiceAccountConfig : public Credentials {
Options const& options() const { return options_; }

private:
void dispatch(CredentialsVisitor& v) override { v.visit(*this); }
void dispatch(CredentialsVisitor& v) const override { v.visit(*this); }

std::string json_object_;
Options options_;
Expand All @@ -142,7 +143,7 @@ class ExternalAccountConfig : public Credentials {
Options const& options() const { return options_; }

private:
void dispatch(CredentialsVisitor& v) override { v.visit(*this); }
void dispatch(CredentialsVisitor& v) const override { v.visit(*this); }

std::string json_object_;
Options options_;
Expand Down
14 changes: 7 additions & 7 deletions google/cloud/internal/credentials_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,29 @@ using ::testing::IsNull;
struct Visitor : public CredentialsVisitor {
std::string name;
AccessToken access_token;
ImpersonateServiceAccountConfig* impersonate = nullptr;
ImpersonateServiceAccountConfig const* impersonate = nullptr;
std::string json_object;
Options options;

void visit(InsecureCredentialsConfig&) override {
void visit(InsecureCredentialsConfig const&) override {
name = "InsecureCredentialsConfig";
}
void visit(GoogleDefaultCredentialsConfig&) override {
void visit(GoogleDefaultCredentialsConfig const&) override {
name = "GoogleDefaultCredentialsConfig";
}
void visit(AccessTokenConfig& cfg) override {
void visit(AccessTokenConfig const& cfg) override {
name = "AccessTokenConfig";
access_token = cfg.access_token();
}
void visit(ImpersonateServiceAccountConfig& cfg) override {
void visit(ImpersonateServiceAccountConfig const& cfg) override {
name = "ImpersonateServiceAccountConfig";
impersonate = &cfg;
}
void visit(ServiceAccountConfig& cfg) override {
void visit(ServiceAccountConfig const& cfg) override {
name = "ServiceAccountConfig";
json_object = cfg.json_object();
}
void visit(ExternalAccountConfig& cfg) override {
void visit(ExternalAccountConfig const& cfg) override {
name = "ExternalAccountConfig";
json_object = cfg.json_object();
options = cfg.options();
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/internal/curl_rest_client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ CurlRestClient::CurlRestClient(std::string endpoint_address,
google::cloud::internal::ApiClientHeader()),
options_(std::move(options)) {
if (options_.has<UnifiedCredentialsOption>()) {
credentials_ = MapCredentials(options_.get<UnifiedCredentialsOption>());
credentials_ = MapCredentials(*options_.get<UnifiedCredentialsOption>());
}
}

Expand Down
2 changes: 1 addition & 1 deletion google/cloud/internal/grpc_impersonate_service_account.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ using ::google::iam::credentials::v1::GenerateAccessTokenResponse;
AsyncAccessTokenSource MakeSource(ImpersonateServiceAccountConfig const& config,
CompletionQueue cq, Options const& options) {
auto stub = MakeMinimalIamCredentialsStub(
CreateAuthenticationStrategy(config.base_credentials(), std::move(cq),
CreateAuthenticationStrategy(*config.base_credentials(), std::move(cq),
options),
MakeMinimalIamCredentialsOptions(options));

Expand Down
2 changes: 1 addition & 1 deletion google/cloud/internal/oauth2_impersonate_service_account_credentials.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ ImpersonateServiceAccountCredentials::ImpersonateServiceAccountCredentials(
HttpClientFactory client_factory)
: ImpersonateServiceAccountCredentials(
config, MakeMinimalIamCredentialsRestStub(
rest_internal::MapCredentials(config.base_credentials()),
rest_internal::MapCredentials(*config.base_credentials()),
config.options(), std::move(client_factory))) {}

ImpersonateServiceAccountCredentials::ImpersonateServiceAccountCredentials(
Expand Down
19 changes: 9 additions & 10 deletions google/cloud/internal/unified_grpc_credentials.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,15 @@ std::shared_ptr<GrpcAuthenticationStrategy> CreateAuthenticationStrategy(
google::cloud::CompletionQueue cq, Options const& options) {
if (options.has<google::cloud::UnifiedCredentialsOption>()) {
return google::cloud::internal::CreateAuthenticationStrategy(
options.get<google::cloud::UnifiedCredentialsOption>(), std::move(cq),
*options.get<google::cloud::UnifiedCredentialsOption>(), std::move(cq),
options);
}
return google::cloud::internal::CreateAuthenticationStrategy(
options.get<google::cloud::GrpcCredentialOption>());
}

std::shared_ptr<GrpcAuthenticationStrategy> CreateAuthenticationStrategy(
std::shared_ptr<Credentials> const& credentials, CompletionQueue cq,
Options options) {
Credentials const& credentials, CompletionQueue cq, Options options) {
struct Visitor : public CredentialsVisitor {
CompletionQueue cq;
Options options;
Expand All @@ -69,35 +68,35 @@ std::shared_ptr<GrpcAuthenticationStrategy> CreateAuthenticationStrategy(
Visitor(CompletionQueue c, Options o)
: cq(std::move(c)), options(std::move(o)) {}

void visit(InsecureCredentialsConfig&) override {
void visit(InsecureCredentialsConfig const&) override {
result = std::make_unique<GrpcChannelCredentialsAuthentication>(
grpc::InsecureChannelCredentials());
}
void visit(GoogleDefaultCredentialsConfig&) override {
void visit(GoogleDefaultCredentialsConfig const&) override {
result = std::make_unique<GrpcChannelCredentialsAuthentication>(
grpc::GoogleDefaultCredentials());
}
void visit(AccessTokenConfig& cfg) override {
void visit(AccessTokenConfig const& cfg) override {
result = std::make_unique<GrpcAccessTokenAuthentication>(
cfg.access_token(), std::move(options));
}
void visit(ImpersonateServiceAccountConfig& cfg) override {
void visit(ImpersonateServiceAccountConfig const& cfg) override {
result = GrpcImpersonateServiceAccount::Create(std::move(cq), cfg,
std::move(options));
}
void visit(ServiceAccountConfig& cfg) override {
void visit(ServiceAccountConfig const& cfg) override {
result = std::make_unique<GrpcServiceAccountAuthentication>(
cfg.json_object(), std::move(options));
}
void visit(ExternalAccountConfig& cfg) override {
void visit(ExternalAccountConfig const& cfg) override {
result = std::make_unique<GrpcChannelCredentialsAuthentication>(
grpc::CompositeChannelCredentials(
grpc::SslCredentials(grpc::SslCredentialsOptions()),
GrpcExternalAccountCredentials(cfg)));
}
} visitor(std::move(cq), std::move(options));

CredentialsVisitor::dispatch(*credentials, visitor);
CredentialsVisitor::dispatch(credentials, visitor);
return std::move(visitor.result);
}

Expand Down
3 changes: 1 addition & 2 deletions google/cloud/internal/unified_grpc_credentials.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ std::shared_ptr<GrpcAuthenticationStrategy> CreateAuthenticationStrategy(
google::cloud::CompletionQueue cq, Options const& options);

std::shared_ptr<GrpcAuthenticationStrategy> CreateAuthenticationStrategy(
std::shared_ptr<Credentials> const& credentials, CompletionQueue cq,
Options options = {});
Credentials const& credentials, CompletionQueue cq, Options options = {});

std::shared_ptr<GrpcAuthenticationStrategy> CreateAuthenticationStrategy(
std::shared_ptr<grpc::ChannelCredentials> const& credentials);
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/internal/unified_grpc_credentials_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ TEST(UnifiedGrpcCredentialsTest, WithGrpcCredentials) {

TEST(UnifiedGrpcCredentialsTest, WithInsecureCredentials) {
CompletionQueue cq;
auto result = CreateAuthenticationStrategy(MakeInsecureCredentials(), cq);
auto result = CreateAuthenticationStrategy(*MakeInsecureCredentials(), cq);
ASSERT_NE(nullptr, result.get());
grpc::ClientContext context;
auto status = result->ConfigureContext(context);
Expand All @@ -83,7 +83,7 @@ TEST(UnifiedGrpcCredentialsTest, WithDefaultCredentials) {

CompletionQueue cq;
auto result =
CreateAuthenticationStrategy(MakeGoogleDefaultCredentials(), cq);
CreateAuthenticationStrategy(*MakeGoogleDefaultCredentials(), cq);
ASSERT_NE(nullptr, result.get());
grpc::ClientContext context;
auto status = result->ConfigureContext(context);
Expand All @@ -96,7 +96,7 @@ TEST(UnifiedGrpcCredentialsTest, WithAccessTokenCredentials) {
std::chrono::system_clock::now() + std::chrono::hours(1);
CompletionQueue cq;
auto result = CreateAuthenticationStrategy(
MakeAccessTokenCredentials("test-token", expiration), cq);
*MakeAccessTokenCredentials("test-token", expiration), cq);
ASSERT_NE(nullptr, result.get());
grpc::ClientContext context;
auto status = result->ConfigureContext(context);
Expand Down
20 changes: 10 additions & 10 deletions google/cloud/internal/unified_rest_credentials.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ CreateServiceAccountCredentialsFromJsonContents(
} // namespace

std::shared_ptr<oauth2_internal::Credentials> MapCredentials(
std::shared_ptr<google::cloud::Credentials> const& credentials) {
return MapCredentials(std::move(credentials), [](Options const& options) {
google::cloud::Credentials const& credentials) {
return MapCredentials(credentials, [](Options const& options) {
return MakeDefaultRestClient("", options);
});
}

std::shared_ptr<oauth2_internal::Credentials> MapCredentials(
std::shared_ptr<google::cloud::Credentials> const& credentials,
google::cloud::Credentials const& credentials,
oauth2_internal::HttpClientFactory client_factory) {
class Visitor : public CredentialsVisitor {
public:
Expand All @@ -82,11 +82,11 @@ std::shared_ptr<oauth2_internal::Credentials> MapCredentials(

std::shared_ptr<oauth2_internal::Credentials> result;

void visit(InsecureCredentialsConfig&) override {
void visit(InsecureCredentialsConfig const&) override {
result = std::make_shared<oauth2_internal::AnonymousCredentials>();
}

void visit(GoogleDefaultCredentialsConfig& cfg) override {
void visit(GoogleDefaultCredentialsConfig const& cfg) override {
auto credentials =
google::cloud::oauth2_internal::GoogleDefaultCredentials(
cfg.options(), std::move(client_factory_));
Expand All @@ -97,26 +97,26 @@ std::shared_ptr<oauth2_internal::Credentials> MapCredentials(
result = MakeErrorCredentials(std::move(credentials).status());
}

void visit(AccessTokenConfig& cfg) override {
void visit(AccessTokenConfig const& cfg) override {
result = std::make_shared<oauth2_internal::AccessTokenCredentials>(
cfg.access_token());
}

void visit(ImpersonateServiceAccountConfig& cfg) override {
void visit(ImpersonateServiceAccountConfig const& cfg) override {
result = std::make_shared<
oauth2_internal::ImpersonateServiceAccountCredentials>(
cfg, std::move(client_factory_));
result = Decorate(std::move(result), cfg.options());
}

void visit(ServiceAccountConfig& cfg) override {
void visit(ServiceAccountConfig const& cfg) override {
result = Decorate(
CreateServiceAccountCredentialsFromJsonContents(
cfg.json_object(), cfg.options(), std::move(client_factory_)),
cfg.options());
}

void visit(ExternalAccountConfig& cfg) override {
void visit(ExternalAccountConfig const& cfg) override {
auto const ec = internal::ErrorContext();
auto info = oauth2_internal::ParseExternalAccountConfiguration(
cfg.json_object(), ec);
Expand All @@ -135,7 +135,7 @@ std::shared_ptr<oauth2_internal::Credentials> MapCredentials(
};

Visitor visitor(std::move(client_factory));
CredentialsVisitor::dispatch(*credentials, visitor);
CredentialsVisitor::dispatch(credentials, visitor);
return std::move(visitor.result);
}

Expand Down
4 changes: 2 additions & 2 deletions google/cloud/internal/unified_rest_credentials.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
* `google::cloud::oauth2_internal::Credentials` type.
*/
std::shared_ptr<oauth2_internal::Credentials> MapCredentials(
std::shared_ptr<google::cloud::Credentials> const& credentials);
google::cloud::Credentials const& credentials);

/**
* @copydoc MapCredentials(std::shared_ptr<google::cloud::Credentials> const&)
*
* This is used in tests, where the HTTP client needs to be mocked.
*/
std::shared_ptr<oauth2_internal::Credentials> MapCredentials(
std::shared_ptr<google::cloud::Credentials> const& credentials,
google::cloud::Credentials const& credentials,
oauth2_internal::HttpClientFactory client_factory);

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
Loading