diff options
author | Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com> | 2024-07-02 16:47:13 +0200 |
---|---|---|
committer | Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com> | 2024-07-04 16:05:08 +0200 |
commit | c2f7ead468d5e65c57e7ecb49d7fbba4254c46b7 (patch) | |
tree | b96ce2e63d9d6a2d486cb4133c290187156b97ac | |
parent | 0d60cd9ba4a5c18b01b6ef996434953071f0576e (diff) | |
download | justbuild-c2f7ead468d5e65c57e7ecb49d7fbba4254c46b7.tar.gz |
Replace the Auth and Auth::TLS singletons
Use a builder pattern for creation and validation, in a manner that
allows also other authentication methods to be added in the future
besides the current TLS/SSL.
The main Auth instances are built early and then passed by not_null
const pointers, to avoid passing temporaries, replacing the previous
Auth::TLS instances passed by simple nullable const pointers. Where
needed, these passed Auth instances are also stored, by const ref.
Tests also build Auth instances as needed, either with the default
'no certification' or from the test environment arguments.
58 files changed, 601 insertions, 548 deletions
diff --git a/src/buildtool/auth/TARGETS b/src/buildtool/auth/TARGETS index 4d501a91..a66aae44 100644 --- a/src/buildtool/auth/TARGETS +++ b/src/buildtool/auth/TARGETS @@ -6,6 +6,7 @@ [ ["@", "gsl", "", "gsl"] , ["src/buildtool/logging", "log_level"] , ["src/buildtool/logging", "logging"] + , ["src/utils/cpp", "expected"] ] , "stage": ["src", "buildtool", "auth"] } diff --git a/src/buildtool/auth/authentication.hpp b/src/buildtool/auth/authentication.hpp index ae45561a..cd4ff86d 100644 --- a/src/buildtool/auth/authentication.hpp +++ b/src/buildtool/auth/authentication.hpp @@ -14,6 +14,7 @@ #ifndef INCLUDED_SRC_BUILDTOOL_AUTH_AUTHENTICATION_HPP #define INCLUDED_SRC_BUILDTOOL_AUTH_AUTHENTICATION_HPP + #include <cstdint> #include <filesystem> #include <fstream> @@ -25,135 +26,189 @@ #include "gsl/gsl" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" -enum class AuthMethod : std::uint8_t { kNONE, kTLS }; +#include "src/utils/cpp/expected.hpp" + +struct Auth final { + struct TLS final { + class Builder; + + // CA certificate bundle + std::string const ca_cert = {}; + // Client-side signed certificate + std::string const client_cert = {}; + // Client-side private key + std::string const client_key = {}; + // Server-side signed certificate + std::string const server_cert = {}; + // Server-side private key + std::string const server_key = {}; + }; -class Auth { + std::variant<std::monostate, TLS> method = {}; +}; + +class Auth::TLS::Builder final { public: - [[nodiscard]] static auto Instance() noexcept -> Auth& { - static Auth instance{}; - return instance; + auto SetCACertificate( + std::optional<std::filesystem::path> cert_file) noexcept -> Builder& { + ca_cert_file_ = std::move(cert_file); + return *this; } - void SetAuthMethod(AuthMethod x) { auth_ = x; } - [[nodiscard]] auto GetAuthMethod() const noexcept -> AuthMethod { - return auth_; + auto SetClientCertificate( + std::optional<std::filesystem::path> cert_file) noexcept -> Builder& { + client_cert_file_ = std::move(cert_file); + return *this; } - class TLS { - public: - [[nodiscard]] static auto Instance() noexcept -> TLS& { - static TLS instance{}; - return instance; - } - - [[nodiscard]] auto CACert() const noexcept -> const std::string& { - return ca_cert_; - } - - [[nodiscard]] auto ClientCert() const noexcept -> const std::string& { - return client_cert_; - } - - [[nodiscard]] auto ClientKey() const noexcept -> const std::string& { - return client_key_; - } - - [[nodiscard]] auto ServerCert() const noexcept -> const std::string& { - return server_cert_; - } + auto SetClientKey(std::optional<std::filesystem::path> key_file) noexcept + -> Builder& { + client_key_file_ = std::move(key_file); + return *this; + } - [[nodiscard]] auto ServerKey() const noexcept -> const std::string& { - return server_key_; - } + auto SetServerCertificate( + std::optional<std::filesystem::path> cert_file) noexcept -> Builder& { + server_cert_file_ = std::move(cert_file); + return *this; + } - [[nodiscard]] auto SetCACertificate( - std::filesystem::path const& cert_file) noexcept -> bool { - return set(cert_file, &ca_cert_); - } + auto SetServerKey(std::optional<std::filesystem::path> key_file) noexcept + -> Builder& { + server_key_file_ = std::move(key_file); + return *this; + } - [[nodiscard]] auto SetClientCertificate( - std::filesystem::path const& cert_file) noexcept -> bool { - return set(cert_file, &client_cert_); + /// \brief Finalize building, validate the entries, and create an Auth with + /// TLS as method. Validation ensures that either both tls_client_cert or + /// tls_client_key are set, or none of the two. + /// \return Auth on success, error string on failure, nullopt if no TLS + /// configuration fields were set. + [[nodiscard]] auto Build() noexcept + -> std::optional<expected<Auth, std::string>> { + // To not duplicate default arguments of Auth::TLS in builder, + // create a default config and copy default arguments from there. + Auth::TLS const default_auth_tls; + bool tls_args_exist = false; + + // Set and validate the CA certification. + // If provided, the CA certificate bundle should of course not be empty. + auto ca_cert = default_auth_tls.ca_cert; + if (ca_cert_file_.has_value()) { + if (auto content = read(*ca_cert_file_)) { + if (content->empty()) { + return unexpected( + std::string("Please provide tls-ca-cert")); + } + ca_cert = *std::move(content); + tls_args_exist = true; + } + else { + return unexpected( + fmt::format("Could not read '{}' CA certificate.", + ca_cert_file_->string())); + } } - [[nodiscard]] auto SetClientKey( - std::filesystem::path const& key_file) noexcept -> bool { - return set(key_file, &client_key_); + // Set and validate the client-side certification. + // To enable mTLS, both tls_client_{certificate,key} must be supplied. + auto client_cert = default_auth_tls.client_cert; + if (client_cert_file_.has_value()) { + if (auto content = read(*client_cert_file_)) { + client_cert = *std::move(content); + tls_args_exist = true; + } + else { + return unexpected( + fmt::format("Could not read '{}' client certificate.", + client_cert_file_->string())); + } } - - [[nodiscard]] auto SetServerCertificate( - std::filesystem::path const& cert_file) noexcept -> bool { - return set(cert_file, &server_cert_); + auto client_key = default_auth_tls.client_key; + if (client_key_file_.has_value()) { + if (auto content = read(*client_key_file_)) { + client_key = *std::move(content); + tls_args_exist = true; + } + else { + return unexpected(fmt::format("Could not read '{}' client key.", + client_key_file_->string())); + } } - - [[nodiscard]] auto SetServerKey( - std::filesystem::path const& key_file) noexcept -> bool { - return set(key_file, &server_key_); + if (client_cert.empty() != client_key.empty()) { + std::string error = client_cert.empty() + ? "Please also provide tls-client-cert" + : "Please also provide tls-client-key"; + return unexpected(std::move(error)); } - // must be called after the parsing of cmd line arguments - // we ensure that either both tls_client_cert or tls_client_key are set - // or none of the two. - [[nodiscard]] auto Validate() const noexcept -> bool { - if (CACert().empty()) { - Logger::Log(LogLevel::Error, "Please provide tls-ca-cert"); - return false; - } - // to enable mTLS, both tls_client_{certificate,key} must be - // supplied - if (ClientCert().empty() && not(ClientKey().empty())) { - Logger::Log(LogLevel::Error, - "Please also provide tls-client-cert"); - return false; + // Set and validate the server-side certification. + // To enable mTLS, both tls_server_{certificate,key} must be supplied. + auto server_cert = default_auth_tls.server_cert; + if (server_cert_file_.has_value()) { + if (auto content = read(*server_cert_file_)) { + server_cert = *std::move(content); + tls_args_exist = true; } - if (not(ClientCert().empty()) && ClientKey().empty()) { - Logger::Log(LogLevel::Error, - "Please also provide tls-client-key"); - return false; + else { + return unexpected( + fmt::format("Could not read '{}' server certificate.", + server_cert_file_->string())); } - - // to enable mTLS, both tls_server_{certificate,key} must be - // supplied - if (ServerCert().empty() && not(ServerKey().empty())) { - Logger::Log(LogLevel::Error, - "Please also provide tls-server-cert"); - return false; + } + auto server_key = default_auth_tls.server_key; + if (server_key_file_.has_value()) { + if (auto content = read(*server_key_file_)) { + server_key = *std::move(content); + tls_args_exist = true; } - if (not(ServerCert().empty()) && ServerKey().empty()) { - Logger::Log(LogLevel::Error, - "Please also provide tls-server-key"); - return false; + else { + return unexpected(fmt::format("Could not read '{}' server key.", + server_key_file_->string())); } - return true; + } + if (server_cert.empty() != server_key.empty()) { + std::string error = server_cert.empty() + ? "Please also provide tls-server-cert" + : "Please also provide tls-server-key"; + return unexpected(std::move(error)); } - private: - std::string ca_cert_; - std::string client_cert_; - std::string client_key_; - std::string server_cert_; - std::string server_key_; - // auxiliary function to set the content of the members of this class - [[nodiscard]] static auto set( - std::filesystem::path const& x, - gsl::not_null<std::string*> const& member) noexcept -> bool { - Auth::Instance().SetAuthMethod(AuthMethod::kTLS); - try { - // if the file does not exist, it will throw an exception - auto file = std::filesystem::canonical(x); - std::ifstream cert{file}; - std::string tmp((std::istreambuf_iterator<char>(cert)), - std::istreambuf_iterator<char>()); - *member = std::move(tmp); - } catch (std::exception const& e) { - Logger::Log(LogLevel::Error, e.what()); - return false; - } - return true; + // If no TLS arguments were ever set, there is nothing to build. + if (not tls_args_exist) { + return std::nullopt; } - }; + + // Return an authentication configuration with mTLS enabled. + return Auth{.method = Auth::TLS{.ca_cert = std::move(ca_cert), + .client_cert = std::move(client_cert), + .client_key = std::move(client_key), + .server_cert = std::move(server_cert), + .server_key = std::move(server_key)}}; + } private: - AuthMethod auth_{AuthMethod::kNONE}; + std::optional<std::filesystem::path> ca_cert_file_; + std::optional<std::filesystem::path> client_cert_file_; + std::optional<std::filesystem::path> client_key_file_; + std::optional<std::filesystem::path> server_cert_file_; + std::optional<std::filesystem::path> server_key_file_; + + /// \brief Auxiliary function to read the content of certification files. + [[nodiscard]] static auto read(std::filesystem::path const& x) noexcept + -> std::optional<std::string> { + try { + // if the file does not exist, it will throw an exception + auto file = std::filesystem::canonical(x); + std::ifstream cert{file}; + std::string tmp((std::istreambuf_iterator<char>(cert)), + std::istreambuf_iterator<char>()); + return tmp; + } catch (std::exception const& e) { + Logger::Log(LogLevel::Error, e.what()); + } + return std::nullopt; + } }; -#endif + +#endif // INCLUDED_SRC_BUILDTOOL_AUTH_AUTHENTICATION_HPP diff --git a/src/buildtool/common/remote/TARGETS b/src/buildtool/common/remote/TARGETS index 06604234..a5480e2b 100644 --- a/src/buildtool/common/remote/TARGETS +++ b/src/buildtool/common/remote/TARGETS @@ -5,6 +5,7 @@ , "deps": [ ["@", "fmt", "", "fmt"] , ["@", "grpc", "", "grpc++"] + , ["@", "gsl", "", "gsl"] , ["src/buildtool/auth", "auth"] , ["src/buildtool/common", "common"] , "port" diff --git a/src/buildtool/common/remote/client_common.hpp b/src/buildtool/common/remote/client_common.hpp index 4fd9e9a7..58c19458 100644 --- a/src/buildtool/common/remote/client_common.hpp +++ b/src/buildtool/common/remote/client_common.hpp @@ -21,9 +21,11 @@ #include <optional> #include <sstream> #include <string> +#include <variant> #include "fmt/core.h" #include "grpcpp/grpcpp.h" +#include "gsl/gsl" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/bazel_types.hpp" #include "src/buildtool/common/remote/port.hpp" @@ -33,16 +35,18 @@ [[maybe_unused]] [[nodiscard]] static inline auto CreateChannelWithCredentials( std::string const& server, Port port, - Auth::TLS const* auth) noexcept { + gsl::not_null<Auth const*> const& auth) noexcept { std::shared_ptr<grpc::ChannelCredentials> creds; std::string address = server + ':' + std::to_string(port); - if (auth != nullptr) { + if (const auto* tls_auth = std::get_if<Auth::TLS>(&auth->method); + tls_auth != nullptr) { auto tls_opts = grpc::SslCredentialsOptions{ - auth->CACert(), auth->ClientKey(), auth->ClientCert()}; + tls_auth->ca_cert, tls_auth->client_key, tls_auth->client_cert}; creds = grpc::SslCredentials(tls_opts); } else { + // currently only TLS/SSL is supported creds = grpc::InsecureChannelCredentials(); } return grpc::CreateChannel(address, creds); diff --git a/src/buildtool/execution_api/common/api_bundle.cpp b/src/buildtool/execution_api/common/api_bundle.cpp index 17e3d3af..41637eb4 100644 --- a/src/buildtool/execution_api/common/api_bundle.cpp +++ b/src/buildtool/execution_api/common/api_bundle.cpp @@ -19,10 +19,10 @@ #include "src/buildtool/execution_api/remote/bazel/bazel_api.hpp" ApiBundle::ApiBundle(RepositoryConfig const* repo_config, - Auth::TLS const* authentication, + gsl::not_null<Auth const*> const& authentication, std::optional<ServerAddress> const& remote_address) : local{std::make_shared<LocalApi>(repo_config)}, // needed by remote - auth{authentication}, // needed by remote + auth{*authentication}, // needed by remote remote{CreateRemote(remote_address)} {} auto ApiBundle::CreateRemote(std::optional<ServerAddress> const& address) const @@ -31,7 +31,7 @@ auto ApiBundle::CreateRemote(std::optional<ServerAddress> const& address) const ExecutionConfiguration config; config.skip_cache_lookup = false; return std::make_shared<BazelApi>( - "remote-execution", address->host, address->port, auth, config); + "remote-execution", address->host, address->port, &auth, config); } return local; } diff --git a/src/buildtool/execution_api/common/api_bundle.hpp b/src/buildtool/execution_api/common/api_bundle.hpp index 72fdb687..0148b6af 100644 --- a/src/buildtool/execution_api/common/api_bundle.hpp +++ b/src/buildtool/execution_api/common/api_bundle.hpp @@ -29,14 +29,14 @@ /// exactly the same instance that local api is (&*remote == & *local). struct ApiBundle final { explicit ApiBundle(RepositoryConfig const* repo_config, - Auth::TLS const* authentication, + gsl::not_null<Auth const*> const& authentication, std::optional<ServerAddress> const& remote_address); [[nodiscard]] auto CreateRemote(std::optional<ServerAddress> const& address) const -> gsl::not_null<IExecutionApi::Ptr>; gsl::not_null<IExecutionApi::Ptr> const local; // needed by remote - Auth::TLS const* auth; // needed by remote + Auth const& auth; // needed by remote gsl::not_null<IExecutionApi::Ptr> const remote; }; diff --git a/src/buildtool/execution_api/execution_service/server_implementation.cpp b/src/buildtool/execution_api/execution_service/server_implementation.cpp index 4a9f23cf..676bd0a7 100644 --- a/src/buildtool/execution_api/execution_service/server_implementation.cpp +++ b/src/buildtool/execution_api/execution_service/server_implementation.cpp @@ -70,13 +70,15 @@ auto ServerImpl::Run(ApiBundle const& apis) -> bool { .RegisterService(&cap) .RegisterService(&op); + // check authentication credentials; currently only TLS/SSL is supported std::shared_ptr<grpc::ServerCredentials> creds; - if (apis.auth != nullptr) { + if (const auto* tls_auth = std::get_if<Auth::TLS>(&apis.auth.method); + tls_auth != nullptr) { auto tls_opts = grpc::SslServerCredentialsOptions{}; - tls_opts.pem_root_certs = apis.auth->CACert(); + tls_opts.pem_root_certs = tls_auth->ca_cert; grpc::SslServerCredentialsOptions::PemKeyCertPair keycert = { - apis.auth->ServerKey(), apis.auth->ServerCert()}; + tls_auth->server_key, tls_auth->server_cert}; tls_opts.pem_key_cert_pairs.emplace_back(keycert); diff --git a/src/buildtool/execution_api/remote/bazel/bazel_ac_client.cpp b/src/buildtool/execution_api/remote/bazel/bazel_ac_client.cpp index 65853702..4f93b62d 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_ac_client.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_ac_client.cpp @@ -14,7 +14,6 @@ #include "src/buildtool/execution_api/remote/bazel/bazel_ac_client.hpp" -#include "gsl/gsl" #include "src/buildtool/common/bazel_types.hpp" #include "src/buildtool/common/remote/client_common.hpp" #include "src/buildtool/common/remote/retry.hpp" @@ -22,7 +21,7 @@ BazelAcClient::BazelAcClient(std::string const& server, Port port, - Auth::TLS const* auth) noexcept { + gsl::not_null<Auth const*> const& auth) noexcept { stub_ = bazel_re::ActionCache::NewStub( CreateChannelWithCredentials(server, port, auth)); } diff --git a/src/buildtool/execution_api/remote/bazel/bazel_ac_client.hpp b/src/buildtool/execution_api/remote/bazel/bazel_ac_client.hpp index 4ac24bbc..36e83649 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_ac_client.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_ac_client.hpp @@ -21,6 +21,7 @@ #include <vector> #include "build/bazel/remote/execution/v2/remote_execution.grpc.pb.h" +#include "gsl/gsl" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/bazel_types.hpp" #include "src/buildtool/common/remote/port.hpp" @@ -34,7 +35,7 @@ class BazelAcClient { public: explicit BazelAcClient(std::string const& server, Port port, - Auth::TLS const* auth) noexcept; + gsl::not_null<Auth const*> const& auth) noexcept; [[nodiscard]] auto GetActionResult( std::string const& instance_name, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp index f463ab3c..1ce65259 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp @@ -190,7 +190,7 @@ namespace { BazelApi::BazelApi(std::string const& instance_name, std::string const& host, Port port, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, ExecutionConfiguration const& exec_config) noexcept { network_ = std::make_shared<BazelNetwork>( instance_name, host, port, auth, exec_config); diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp index e87e6159..c9771ed7 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp @@ -43,7 +43,7 @@ class BazelApi final : public IExecutionApi { BazelApi(std::string const& instance_name, std::string const& host, Port port, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, ExecutionConfiguration const& exec_config) noexcept; BazelApi(BazelApi const&) = delete; BazelApi(BazelApi&& other) noexcept; diff --git a/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp b/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp index 3850ccff..af23e9b1 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_cas_client.cpp @@ -175,7 +175,7 @@ namespace { BazelCasClient::BazelCasClient(std::string const& server, Port port, - Auth::TLS const* auth) noexcept + gsl::not_null<Auth const*> const& auth) noexcept : stream_{std::make_unique<ByteStreamClient>(server, port, auth)} { stub_ = bazel_re::ContentAddressableStorage::NewStub( CreateChannelWithCredentials(server, port, auth)); diff --git a/src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp b/src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp index d7aa5c05..90d9eb75 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp @@ -40,7 +40,7 @@ class BazelCasClient { public: explicit BazelCasClient(std::string const& server, Port port, - Auth::TLS const* auth) noexcept; + gsl::not_null<Auth const*> const& auth) noexcept; /// \brief Find missing blobs /// \param[in] instance_name Name of the CAS instance diff --git a/src/buildtool/execution_api/remote/bazel/bazel_execution_client.cpp b/src/buildtool/execution_api/remote/bazel/bazel_execution_client.cpp index f4ad250c..51ee1869 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_execution_client.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_execution_client.cpp @@ -17,7 +17,6 @@ #include <utility> // std::move #include "grpcpp/grpcpp.h" -#include "gsl/gsl" #include "src/buildtool/common/remote/client_common.hpp" #include "src/buildtool/common/remote/retry.hpp" #include "src/buildtool/logging/log_level.hpp" @@ -56,9 +55,10 @@ auto DebugString(grpc::Status const& status) -> std::string { } // namespace -BazelExecutionClient::BazelExecutionClient(std::string const& server, - Port port, - Auth::TLS const* auth) noexcept { +BazelExecutionClient::BazelExecutionClient( + std::string const& server, + Port port, + gsl::not_null<Auth const*> const& auth) noexcept { stub_ = bazel_re::Execution::NewStub( CreateChannelWithCredentials(server, port, auth)); } diff --git a/src/buildtool/execution_api/remote/bazel/bazel_execution_client.hpp b/src/buildtool/execution_api/remote/bazel/bazel_execution_client.hpp index 74676d45..aa505121 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_execution_client.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_execution_client.hpp @@ -22,6 +22,7 @@ #include "build/bazel/remote/execution/v2/remote_execution.grpc.pb.h" #include "google/longrunning/operations.pb.h" +#include "gsl/gsl" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/bazel_types.hpp" #include "src/buildtool/common/remote/port.hpp" @@ -55,9 +56,10 @@ class BazelExecutionClient { } }; - explicit BazelExecutionClient(std::string const& server, - Port port, - Auth::TLS const* auth) noexcept; + explicit BazelExecutionClient( + std::string const& server, + Port port, + gsl::not_null<Auth const*> const& auth) noexcept; [[nodiscard]] auto Execute(std::string const& instance_name, bazel_re::Digest const& action_digest, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp index 4d5509c9..6094888d 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp @@ -24,7 +24,7 @@ BazelNetwork::BazelNetwork(std::string instance_name, std::string const& host, Port port, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, ExecutionConfiguration const& exec_config) noexcept : instance_name_{std::move(instance_name)}, exec_config_{exec_config}, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp index 4da302c9..ca0b0e2a 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp @@ -22,6 +22,7 @@ #include <utility> #include <vector> +#include "gsl/gsl" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/bazel_types.hpp" #include "src/buildtool/common/remote/port.hpp" @@ -39,7 +40,7 @@ class BazelNetwork { explicit BazelNetwork(std::string instance_name, std::string const& host, Port port, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, ExecutionConfiguration const& exec_config) noexcept; /// \brief Check if digest exists in CAS diff --git a/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp b/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp index 2879a90f..88abe6fd 100644 --- a/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp +++ b/src/buildtool/execution_api/remote/bazel/bytestream_client.hpp @@ -83,7 +83,7 @@ class ByteStreamClient { explicit ByteStreamClient(std::string const& server, Port port, - Auth::TLS const* auth) noexcept { + gsl::not_null<Auth const*> const& auth) noexcept { stub_ = google::bytestream::ByteStream::NewStub( CreateChannelWithCredentials(server, port, auth)); } diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp index 89a384b8..8b94a016 100644 --- a/src/buildtool/execution_engine/executor/executor.hpp +++ b/src/buildtool/execution_engine/executor/executor.hpp @@ -61,7 +61,7 @@ class ExecutorImpl { std::map<std::string, std::string> const& properties, std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> const& dispatch_list, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, std::chrono::milliseconds const& timeout, IExecutionAction::CacheFlag cache_flag, gsl::not_null<Statistics*> const& stats, @@ -668,7 +668,7 @@ class ExecutorImpl { const std::map<std::string, std::string>& properties, const std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>>& dispatch_list, - const Auth::TLS* auth) -> std::unique_ptr<BazelApi> { + const gsl::not_null<Auth const*>& auth) -> std::unique_ptr<BazelApi> { for (auto const& [pred, endpoint] : dispatch_list) { bool match = true; for (auto const& [k, v] : pred) { @@ -708,7 +708,7 @@ class Executor { std::map<std::string, std::string> properties, std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, Logger const* logger = nullptr, // log in caller logger, if given @@ -718,7 +718,7 @@ class Executor { remote_api_{*remote_api}, properties_{std::move(properties)}, dispatch_list_{std::move(dispatch_list)}, - auth_{auth}, + auth_{*auth}, stats_{stats}, progress_{progress}, logger_{logger}, @@ -741,7 +741,7 @@ class Executor { Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, - auth_, + &auth_, Impl::ScaleTime(timeout_, action->TimeoutScale()), action->NoCache() ? CF::DoNotCacheOutput : CF::CacheOutput, stats_, @@ -760,7 +760,7 @@ class Executor { remote_api_, Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, - auth_, + &auth_, Impl::ScaleTime(timeout_, action->TimeoutScale()), action->NoCache() ? CF::DoNotCacheOutput : CF::CacheOutput, stats_, @@ -798,7 +798,7 @@ class Executor { std::map<std::string, std::string> properties_; std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list_; - Auth::TLS const* auth_; + Auth const& auth_; gsl::not_null<Statistics*> stats_; gsl::not_null<Progress*> progress_; Logger const* logger_; @@ -824,7 +824,7 @@ class Rebuilder { std::map<std::string, std::string> properties, std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, std::chrono::milliseconds timeout = IExecutionAction::kDefaultTimeout) @@ -834,7 +834,7 @@ class Rebuilder { api_cached_{*api_cached}, properties_{std::move(properties)}, dispatch_list_{std::move(dispatch_list)}, - auth_{auth}, + auth_{*auth}, stats_{stats}, progress_{progress}, timeout_{timeout} {} @@ -850,7 +850,7 @@ class Rebuilder { remote_api_, Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, - auth_, + &auth_, Impl::ScaleTime(timeout_, action->TimeoutScale()), CF::PretendCached, stats_, @@ -867,7 +867,7 @@ class Rebuilder { api_cached_, Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, - auth_, + &auth_, Impl::ScaleTime(timeout_, action->TimeoutScale()), CF::FromCacheOnly, stats_, @@ -916,7 +916,7 @@ class Rebuilder { std::map<std::string, std::string> properties_; std::vector<std::pair<std::map<std::string, std::string>, ServerAddress>> dispatch_list_; - Auth::TLS const* auth_; + Auth const& auth_; gsl::not_null<Statistics*> stats_; gsl::not_null<Progress*> progress_; std::chrono::milliseconds timeout_; diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp index 40ca1ec3..5cc59d9b 100644 --- a/src/buildtool/graph_traverser/graph_traverser.hpp +++ b/src/buildtool/graph_traverser/graph_traverser.hpp @@ -364,7 +364,7 @@ class GraphTraverser { &*apis_.remote, platform_properties_, dispatch_list_, - apis_.auth, + &apis_.auth, stats_, progress_, logger_, @@ -398,7 +398,7 @@ class GraphTraverser { &*api_cached, platform_properties_, dispatch_list_, - apis_.auth, + &apis_.auth, stats_, progress_, clargs_.build.timeout}; diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp index cd073847..0e72890a 100644 --- a/src/buildtool/main/main.cpp +++ b/src/buildtool/main/main.cpp @@ -184,66 +184,32 @@ void SetupExecutionConfig(EndpointArguments const& eargs, return std::nullopt; } -void SetupAuthConfig(CommonAuthArguments const& authargs, - ClientAuthArguments const& client_authargs, - ServerAuthArguments const& server_authargs) { - auto use_tls = false; - if (authargs.tls_ca_cert) { - use_tls = true; - if (not Auth::TLS::Instance().SetCACertificate(*authargs.tls_ca_cert)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' certificate.", - authargs.tls_ca_cert->string()); - std::exit(kExitFailure); - } - } - if (client_authargs.tls_client_cert) { - use_tls = true; - if (not Auth::TLS::Instance().SetClientCertificate( - *client_authargs.tls_client_cert)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' certificate.", - client_authargs.tls_client_cert->string()); - std::exit(kExitFailure); - } - } - if (client_authargs.tls_client_key) { - use_tls = true; - if (not Auth::TLS::Instance().SetClientKey( - *client_authargs.tls_client_key)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' key.", - client_authargs.tls_client_key->string()); - std::exit(kExitFailure); +[[nodiscard]] auto CreateAuthConfig( + CommonAuthArguments const& authargs, + ClientAuthArguments const& client_authargs, + ServerAuthArguments const& server_authargs) noexcept + -> std::optional<Auth> { + Auth::TLS::Builder tls_builder; + tls_builder.SetCACertificate(authargs.tls_ca_cert) + .SetClientCertificate(client_authargs.tls_client_cert) + .SetClientKey(client_authargs.tls_client_key) + .SetServerCertificate(server_authargs.tls_server_cert) + .SetServerKey(server_authargs.tls_server_key); + + // create auth config (including validation) + auto result = tls_builder.Build(); + if (result) { + if (*result) { + // correctly configured TLS/SSL certification + return *std::move(*result); } + Logger::Log(LogLevel::Error, result->error()); + return std::nullopt; } - if (server_authargs.tls_server_cert) { - use_tls = true; - if (not Auth::TLS::Instance().SetServerCertificate( - *server_authargs.tls_server_cert)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' certificate.", - server_authargs.tls_server_cert->string()); - std::exit(kExitFailure); - } - } - if (server_authargs.tls_server_key) { - use_tls = true; - if (not Auth::TLS::Instance().SetServerKey( - *server_authargs.tls_server_key)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' key.", - server_authargs.tls_server_key->string()); - std::exit(kExitFailure); - } - } - - if (use_tls) { - if (not Auth::TLS::Instance().Validate()) { - std::exit(kExitFailure); - } - } + // no TLS/SSL configuration was given, and we currently support no other + // certification method, so return an empty config (no certification) + return Auth{}; } void SetupExecutionServiceConfig(ServiceArguments const& args) { @@ -812,10 +778,10 @@ auto main(int argc, char* argv[]) -> int { return kExitFailure; } - SetupAuthConfig(arguments.auth, arguments.cauth, arguments.sauth); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); + auto auth_config = + CreateAuthConfig(arguments.auth, arguments.cauth, arguments.sauth); + if (not auth_config) { + return kExitFailure; } if (arguments.cmd == SubCommand::kGc) { @@ -829,7 +795,7 @@ auto main(int argc, char* argv[]) -> int { if (arguments.cmd == SubCommand::kExecute) { SetupExecutionServiceConfig(arguments.service); ApiBundle const exec_apis{/*repo_config=*/nullptr, - auth ? &*auth : nullptr, + &*auth_config, RemoteExecutionConfig::RemoteAddress()}; if (!ServerImpl::Instance().Run(exec_apis)) { return kExitFailure; @@ -846,7 +812,7 @@ auto main(int argc, char* argv[]) -> int { if (serve_server) { ApiBundle const serve_apis{ /*repo_config=*/nullptr, - auth ? &*auth : nullptr, + &*auth_config, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &serve_apis); bool with_execute = not RemoteExecutionConfig::RemoteAddress(); @@ -899,7 +865,7 @@ auto main(int argc, char* argv[]) -> int { std::exit(kExitFailure); } ApiBundle const main_apis{&repo_config, - auth ? &*auth : nullptr, + &*auth_config, RemoteExecutionConfig::RemoteAddress()}; GraphTraverser const traverser{ {jobs, diff --git a/src/buildtool/serve_api/remote/TARGETS b/src/buildtool/serve_api/remote/TARGETS index e6f03424..4af6387f 100644 --- a/src/buildtool/serve_api/remote/TARGETS +++ b/src/buildtool/serve_api/remote/TARGETS @@ -16,7 +16,8 @@ , "hdrs": ["source_tree_client.hpp"] , "srcs": ["source_tree_client.cpp"] , "deps": - [ ["src/buildtool/auth", "auth"] + [ ["@", "gsl", "", "gsl"] + , ["src/buildtool/auth", "auth"] , ["src/buildtool/common/remote", "port"] , ["src/buildtool/file_system", "git_types"] , ["src/buildtool/file_system/symlinks_map", "pragma_special"] @@ -80,7 +81,8 @@ , "hdrs": ["configuration_client.hpp"] , "srcs": ["configuration_client.cpp"] , "deps": - [ ["src/buildtool/auth", "auth"] + [ ["@", "gsl", "", "gsl"] + , ["src/buildtool/auth", "auth"] , ["src/buildtool/common/remote", "port"] , ["src/buildtool/logging", "logging"] , ["src/buildtool/common/remote", "client_common"] diff --git a/src/buildtool/serve_api/remote/configuration_client.hpp b/src/buildtool/serve_api/remote/configuration_client.hpp index b7785315..3d0eb7ff 100644 --- a/src/buildtool/serve_api/remote/configuration_client.hpp +++ b/src/buildtool/serve_api/remote/configuration_client.hpp @@ -21,6 +21,7 @@ #include <utility> #include <vector> +#include "gsl/gsl" #include "justbuild/just_serve/just_serve.grpc.pb.h" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/remote/client_common.hpp" @@ -32,8 +33,9 @@ /// src/buildtool/serve_api/serve_service/just_serve.proto class ConfigurationClient { public: - explicit ConfigurationClient(ServerAddress address, - Auth::TLS const* auth) noexcept + explicit ConfigurationClient( + ServerAddress address, + gsl::not_null<Auth const*> const& auth) noexcept : client_serve_address_{std::move(address)}, stub_{justbuild::just_serve::Configuration::NewStub( CreateChannelWithCredentials(client_serve_address_.host, diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp index 9e4a29bf..3374e6aa 100644 --- a/src/buildtool/serve_api/remote/serve_api.hpp +++ b/src/buildtool/serve_api/remote/serve_api.hpp @@ -42,9 +42,9 @@ class ServeApi final { public: explicit ServeApi(ServerAddress const& address, gsl::not_null<ApiBundle const*> const& apis) noexcept - : stc_{address, apis->auth}, + : stc_{address, &apis->auth}, tc_{address, apis}, - cc_{address, apis->auth} {} + cc_{address, &apis->auth} {} ~ServeApi() noexcept = default; ServeApi(ServeApi const&) = delete; diff --git a/src/buildtool/serve_api/remote/source_tree_client.cpp b/src/buildtool/serve_api/remote/source_tree_client.cpp index 3070a44f..7a922671 100644 --- a/src/buildtool/serve_api/remote/source_tree_client.cpp +++ b/src/buildtool/serve_api/remote/source_tree_client.cpp @@ -59,8 +59,9 @@ auto PragmaSpecialToSymlinksResolve( } // namespace -SourceTreeClient::SourceTreeClient(ServerAddress const& address, - Auth::TLS const* auth) noexcept { +SourceTreeClient::SourceTreeClient( + ServerAddress const& address, + gsl::not_null<Auth const*> const& auth) noexcept { stub_ = justbuild::just_serve::SourceTree::NewStub( CreateChannelWithCredentials(address.host, address.port, auth)); } diff --git a/src/buildtool/serve_api/remote/source_tree_client.hpp b/src/buildtool/serve_api/remote/source_tree_client.hpp index 17eb0c57..e5001029 100644 --- a/src/buildtool/serve_api/remote/source_tree_client.hpp +++ b/src/buildtool/serve_api/remote/source_tree_client.hpp @@ -19,6 +19,7 @@ #include <string> #include <unordered_map> +#include "gsl/gsl" #include "justbuild/just_serve/just_serve.grpc.pb.h" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/remote/port.hpp" @@ -33,7 +34,7 @@ class SourceTreeClient { public: explicit SourceTreeClient(ServerAddress const& address, - Auth::TLS const* auth) noexcept; + gsl::not_null<Auth const*> const& auth) noexcept; // An error + data union type using result_t = expected<std::string, GitLookupError>; diff --git a/src/buildtool/serve_api/remote/target_client.cpp b/src/buildtool/serve_api/remote/target_client.cpp index d19ac26f..526121ab 100644 --- a/src/buildtool/serve_api/remote/target_client.cpp +++ b/src/buildtool/serve_api/remote/target_client.cpp @@ -31,7 +31,7 @@ TargetClient::TargetClient(ServerAddress const& address, gsl::not_null<ApiBundle const*> const& apis) noexcept : apis_{*apis} { stub_ = justbuild::just_serve::Target::NewStub( - CreateChannelWithCredentials(address.host, address.port, apis->auth)); + CreateChannelWithCredentials(address.host, address.port, &apis->auth)); } auto TargetClient::ServeTarget(const TargetCacheKey& key, diff --git a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp index 1d81ebb0..906f331c 100644 --- a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp +++ b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp @@ -18,6 +18,7 @@ #include <iostream> #include <memory> +#include <variant> #ifdef __unix__ #include <sys/types.h> @@ -132,13 +133,15 @@ auto ServeServerImpl::Run(RemoteServeConfig const& serve_config, .RegisterService(&op); } + // check authentication credentials; currently only TLS/SSL is supported std::shared_ptr<grpc::ServerCredentials> creds; - if (apis.auth != nullptr) { + if (const auto* tls_auth = std::get_if<Auth::TLS>(&apis.auth.method); + tls_auth != nullptr) { auto tls_opts = grpc::SslServerCredentialsOptions{}; - tls_opts.pem_root_certs = apis.auth->CACert(); + tls_opts.pem_root_certs = tls_auth->ca_cert; grpc::SslServerCredentialsOptions::PemKeyCertPair keycert = { - apis.auth->ServerKey(), apis.auth->ServerCert()}; + tls_auth->server_key, tls_auth->server_cert}; tls_opts.pem_key_cert_pairs.emplace_back(keycert); diff --git a/src/buildtool/serve_api/serve_service/target.cpp b/src/buildtool/serve_api/serve_service/target.cpp index fc09ee8c..44632590 100644 --- a/src/buildtool/serve_api/serve_service/target.cpp +++ b/src/buildtool/serve_api/serve_service/target.cpp @@ -499,7 +499,7 @@ auto TargetService::ServeTarget( // Use a new ApiBundle that knows about local repository config for // traversing. - ApiBundle const local_apis{&repository_config, apis_.auth, address}; + ApiBundle const local_apis{&repository_config, &apis_.auth, address}; GraphTraverser const traverser{ std::move(traverser_args), &repository_config, diff --git a/src/other_tools/just_mr/TARGETS b/src/other_tools/just_mr/TARGETS index b8afc631..a2878802 100644 --- a/src/other_tools/just_mr/TARGETS +++ b/src/other_tools/just_mr/TARGETS @@ -82,7 +82,8 @@ , "hdrs": ["setup_utils.hpp"] , "srcs": ["setup_utils.cpp"] , "deps": - [ ["src/buildtool/build_engine/expression", "expression_ptr_interface"] + [ ["src/buildtool/auth", "auth"] + , ["src/buildtool/build_engine/expression", "expression_ptr_interface"] , ["src/buildtool/build_engine/expression", "expression"] , ["src/buildtool/serve_api/remote", "config"] , "cli" @@ -94,7 +95,6 @@ , ["src/buildtool/logging", "log_level"] , ["src/buildtool/logging", "logging"] , "exit_codes" - , ["src/buildtool/auth", "auth"] ] } , "fetch": diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp index 11ae3a01..07a03f3d 100644 --- a/src/other_tools/just_mr/fetch.cpp +++ b/src/other_tools/just_mr/fetch.cpp @@ -398,14 +398,13 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config, common_args.remote_serve_address); // setup authentication - JustMR::Utils::SetupAuthConfig(auth_args); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); + auto auth_config = JustMR::Utils::CreateAuthConfig(auth_args); + if (not auth_config) { + return kExitConfigError; } ApiBundle const apis{/*repo_config=*/nullptr, - auth ? &*auth : nullptr, + &*auth_config, RemoteExecutionConfig::RemoteAddress()}; bool const has_remote_api = diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp index 70c74eb0..6866bc26 100644 --- a/src/other_tools/just_mr/setup.cpp +++ b/src/other_tools/just_mr/setup.cpp @@ -117,14 +117,13 @@ auto MultiRepoSetup(std::shared_ptr<Configuration> const& config, common_args.remote_serve_address); // setup authentication - JustMR::Utils::SetupAuthConfig(auth_args); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); + auto auth_config = JustMR::Utils::CreateAuthConfig(auth_args); + if (not auth_config) { + return std::nullopt; } ApiBundle const apis{/*repo_config=*/nullptr, - auth ? &*auth : nullptr, + &*auth_config, RemoteExecutionConfig::RemoteAddress()}; bool const has_remote_api = diff --git a/src/other_tools/just_mr/setup_utils.cpp b/src/other_tools/just_mr/setup_utils.cpp index 7ed61897..577fb1bf 100644 --- a/src/other_tools/just_mr/setup_utils.cpp +++ b/src/other_tools/just_mr/setup_utils.cpp @@ -19,7 +19,6 @@ #include <variant> #include "nlohmann/json.hpp" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/build_engine/expression/expression.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/logging/log_level.hpp" @@ -193,42 +192,28 @@ auto ReadConfiguration( } } -void SetupAuthConfig(MultiRepoRemoteAuthArguments const& authargs) noexcept { - bool use_tls{false}; - if (authargs.tls_ca_cert) { - use_tls = true; - if (not Auth::TLS::Instance().SetCACertificate(*authargs.tls_ca_cert)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' certificate.", - authargs.tls_ca_cert->string()); - std::exit(kExitConfigError); - } - } - if (authargs.tls_client_cert) { - use_tls = true; - if (not Auth::TLS::Instance().SetClientCertificate( - *authargs.tls_client_cert)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' certificate.", - authargs.tls_client_cert->string()); - std::exit(kExitConfigError); - } - } - if (authargs.tls_client_key) { - use_tls = true; - if (not Auth::TLS::Instance().SetClientKey(*authargs.tls_client_key)) { - Logger::Log(LogLevel::Error, - "Could not read '{}' key.", - authargs.tls_client_key->string()); - std::exit(kExitConfigError); - } - } +auto CreateAuthConfig(MultiRepoRemoteAuthArguments const& authargs) noexcept + -> std::optional<Auth> { - if (use_tls) { - if (not Auth::TLS::Instance().Validate()) { - std::exit(kExitConfigError); + Auth::TLS::Builder tls_builder; + tls_builder.SetCACertificate(authargs.tls_ca_cert) + .SetClientCertificate(authargs.tls_client_cert) + .SetClientKey(authargs.tls_client_key); + + // create auth config (including validation) + auto result = tls_builder.Build(); + if (result) { + if (*result) { + // correctly configured TLS/SSL certification + return *std::move(*result); } + Logger::Log(LogLevel::Error, result->error()); + return std::nullopt; } + + // no TLS/SSL configuration was given, and we currently support no other + // certification method, so return an empty config (no certification) + return Auth{}; } void SetupRemoteConfig( diff --git a/src/other_tools/just_mr/setup_utils.hpp b/src/other_tools/just_mr/setup_utils.hpp index 2ae08d41..3043855c 100644 --- a/src/other_tools/just_mr/setup_utils.hpp +++ b/src/other_tools/just_mr/setup_utils.hpp @@ -21,6 +21,7 @@ #include <string> #include <vector> +#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/build_engine/expression/configuration.hpp" #include "src/buildtool/build_engine/expression/expression_ptr.hpp" #include "src/buildtool/serve_api/remote/config.hpp" @@ -60,7 +61,9 @@ void DefaultReachableRepositories( std::optional<std::filesystem::path> const& absent_file_opt) noexcept -> std::shared_ptr<Configuration>; -void SetupAuthConfig(MultiRepoRemoteAuthArguments const& authargs) noexcept; +[[nodiscard]] auto CreateAuthConfig( + MultiRepoRemoteAuthArguments const& authargs) noexcept + -> std::optional<Auth>; void SetupRemoteConfig( std::optional<std::string> const& remote_exec_addr, diff --git a/test/buildtool/build_engine/target_map/TARGETS b/test/buildtool/build_engine/target_map/TARGETS index ac369d22..c73a0fa2 100644 --- a/test/buildtool/build_engine/target_map/TARGETS +++ b/test/buildtool/build_engine/target_map/TARGETS @@ -24,6 +24,7 @@ , "private-deps": [ ["@", "catch2", "", "catch2"] , ["", "catch-main"] + , ["@", "src", "src/buildtool/auth", "auth"] , ["@", "src", "src/buildtool/build_engine/base_maps", "directory_map"] , ["@", "src", "src/buildtool/build_engine/base_maps", "rule_map"] , ["@", "src", "src/buildtool/build_engine/base_maps", "source_map"] diff --git a/test/buildtool/build_engine/target_map/target_map.test.cpp b/test/buildtool/build_engine/target_map/target_map.test.cpp index 7bb8ab3e..5a721ab6 100644 --- a/test/buildtool/build_engine/target_map/target_map.test.cpp +++ b/test/buildtool/build_engine/target_map/target_map.test.cpp @@ -17,6 +17,7 @@ #include <utility> // std::move #include "catch2/catch_test_macros.hpp" +#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/build_engine/base_maps/directory_map.hpp" #include "src/buildtool/build_engine/base_maps/entity_name.hpp" #include "src/buildtool/build_engine/base_maps/expression_map.hpp" @@ -99,9 +100,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "simple targets", "[target_map]") { auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), @@ -544,9 +545,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), @@ -634,9 +635,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), @@ -734,9 +735,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "built-in rules", "[target_map]") { auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), @@ -944,9 +945,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "target reference", "[target_map]") { auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), @@ -1087,9 +1088,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "trees", "[target_map]") { auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), @@ -1196,9 +1197,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), @@ -1362,9 +1363,9 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "wrong arguments", "[target_map]") { auto serve_config = TestServeConfig::ReadServeConfigFromEnvironment(); REQUIRE(serve_config); - ApiBundle const apis{/*repo_config=*/nullptr, - /*authentication=*/nullptr, - RemoteExecutionConfig::RemoteAddress()}; + Auth auth{}; + ApiBundle const apis{ + /*repo_config=*/nullptr, &auth, RemoteExecutionConfig::RemoteAddress()}; auto serve = ServeApi::Create(*serve_config, &apis); AnalyseContext ctx{.repo_config = &repo_config, .target_cache = &Storage::Instance().TargetCache(), diff --git a/test/buildtool/execution_api/bazel/TARGETS b/test/buildtool/execution_api/bazel/TARGETS index 17f02068..d3e42c6d 100644 --- a/test/buildtool/execution_api/bazel/TARGETS +++ b/test/buildtool/execution_api/bazel/TARGETS @@ -6,7 +6,7 @@ [ ["@", "catch2", "", "catch2"] , ["@", "gsl", "", "gsl"] , ["utils", "catch-main-remote-execution"] - , ["@", "src", "src/buildtool/auth", "auth"] + , ["utils", "test_auth_config"] , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/execution_api/bazel_msg", "bazel_msg"] , ["@", "src", "src/buildtool/execution_api/remote", "bazel_network"] @@ -23,7 +23,7 @@ [ ["@", "catch2", "", "catch2"] , ["utils", "catch-main-remote-execution"] , ["utils", "execution_bazel"] - , ["@", "src", "src/buildtool/auth", "auth"] + , ["utils", "test_auth_config"] , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/execution_api/remote", "bazel_network"] , ["@", "src", "src/buildtool/execution_api/remote", "config"] @@ -39,7 +39,7 @@ [ ["@", "catch2", "", "catch2"] , ["utils", "catch-main-remote-execution"] , ["utils", "execution_bazel"] - , ["@", "src", "src/buildtool/auth", "auth"] + , ["utils", "test_auth_config"] , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/execution_api/bazel_msg", "bazel_msg"] , ["@", "src", "src/buildtool/execution_api/remote", "bazel_network"] @@ -56,7 +56,7 @@ [ ["@", "catch2", "", "catch2"] , ["utils", "catch-main-remote-execution"] , ["utils", "execution_bazel"] - , ["@", "src", "src/buildtool/auth", "auth"] + , ["utils", "test_auth_config"] , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/compatibility", "compatibility"] , ["@", "src", "src/buildtool/execution_api/bazel_msg", "bazel_msg"] @@ -92,7 +92,7 @@ , "private-deps": [ ["@", "catch2", "", "catch2"] , ["utils", "catch-main-remote-execution"] - , ["@", "src", "src/buildtool/auth", "auth"] + , ["utils", "test_auth_config"] , ["@", "src", "src/buildtool/execution_api/remote", "bazel"] , ["buildtool/execution_api/common", "api_test"] ] diff --git a/test/buildtool/execution_api/bazel/bazel_api.test.cpp b/test/buildtool/execution_api/bazel/bazel_api.test.cpp index af457066..073bf08d 100644 --- a/test/buildtool/execution_api/bazel/bazel_api.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_api.test.cpp @@ -13,29 +13,22 @@ // limitations under the License. #include <cstdlib> -#include <optional> #include <string> #include "catch2/catch_test_macros.hpp" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/execution_api/remote/bazel/bazel_api.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "test/buildtool/execution_api/common/api_test.hpp" -#include "test/utils/test_env.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" namespace { auto const kApiFactory = []() { static auto const& server = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - return IExecutionApi::Ptr{new BazelApi{"remote-execution", - server->host, - server->port, - auth ? &*auth : nullptr, - {}}}; + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + return IExecutionApi::Ptr{new BazelApi{ + "remote-execution", server->host, server->port, &*auth_config, {}}}; }; } // namespace diff --git a/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp index 069421c9..85a23590 100644 --- a/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp @@ -13,19 +13,18 @@ // limitations under the License. #include <functional> // std::equal_to -#include <optional> #include <string> #include <vector> #include "catch2/catch_test_macros.hpp" #include "gsl/gsl" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/artifact_digest.hpp" #include "src/buildtool/execution_api/bazel_msg/bazel_blob_container.hpp" #include "src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp" #include "src/buildtool/execution_api/remote/bazel/bazel_execution_client.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/file_system/object_type.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" TEST_CASE("Bazel internals: CAS Client", "[execution_api]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); @@ -33,13 +32,11 @@ TEST_CASE("Bazel internals: CAS Client", "[execution_api]") { std::string instance_name{"remote-execution"}; std::string content("test"); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); // Create CAS client - BazelCasClient cas_client(info->host, info->port, auth ? &*auth : nullptr); + BazelCasClient cas_client(info->host, info->port, &*auth_config); SECTION("Valid digest and blob") { // digest of "test" diff --git a/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp index 50869542..fe9da14f 100755 --- a/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp @@ -12,17 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include <optional> #include <string> #include "catch2/catch_test_macros.hpp" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/artifact_digest.hpp" #include "src/buildtool/execution_api/remote/bazel/bazel_execution_client.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/file_system/object_type.hpp" #include "test/utils/remote_execution/bazel_action_creator.hpp" -#include "test/utils/test_env.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" TEST_CASE("Bazel internals: Execution Client", "[execution_api]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); @@ -32,13 +30,11 @@ TEST_CASE("Bazel internals: Execution Client", "[execution_api]") { auto test_digest = static_cast<bazel_re::Digest>( ArtifactDigest::Create<ObjectType::File>(content)); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); BazelExecutionClient execution_client( - info->host, info->port, auth ? &*auth : nullptr); + info->host, info->port, &*auth_config); ExecutionConfiguration config; config.skip_cache_lookup = false; @@ -106,13 +102,11 @@ TEST_CASE("Bazel internals: Execution Client using env variables", auto test_digest = static_cast<bazel_re::Digest>( ArtifactDigest::Create<ObjectType::File>(content)); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); BazelExecutionClient execution_client( - info->host, info->port, auth ? &*auth : nullptr); + info->host, info->port, &*auth_config); ExecutionConfiguration config; config.skip_cache_lookup = false; diff --git a/test/buildtool/execution_api/bazel/bazel_network.test.cpp b/test/buildtool/execution_api/bazel/bazel_network.test.cpp index 75821454..54d99305 100644 --- a/test/buildtool/execution_api/bazel/bazel_network.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_network.test.cpp @@ -26,18 +26,17 @@ #include "src/buildtool/execution_api/remote/bazel/bazel_network.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/file_system/object_type.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" constexpr std::size_t kLargeSize = GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH + 1; TEST_CASE("Bazel network: write/read blobs", "[execution_api]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); std::string instance_name{"remote-execution"}; - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - auto network = BazelNetwork{ - instance_name, info->host, info->port, auth ? &*auth : nullptr, {}}; + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + auto network = + BazelNetwork{instance_name, info->host, info->port, &*auth_config, {}}; std::string content_foo("foo"); std::string content_bar("bar"); @@ -82,12 +81,10 @@ TEST_CASE("Bazel network: read blobs with unknown size", "[execution_api]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); std::string instance_name{"remote-execution"}; - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - auto network = BazelNetwork{ - instance_name, info->host, info->port, auth ? &*auth : nullptr, {}}; + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + auto network = + BazelNetwork{instance_name, info->host, info->port, &*auth_config, {}}; std::string content_foo("foo"); std::string content_bar(kLargeSize, 'x'); // single larger blob diff --git a/test/buildtool/execution_api/bazel/bytestream_client.test.cpp b/test/buildtool/execution_api/bazel/bytestream_client.test.cpp index b1c49947..9903039b 100644 --- a/test/buildtool/execution_api/bazel/bytestream_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bytestream_client.test.cpp @@ -24,17 +24,15 @@ #include "src/buildtool/execution_api/remote/bazel/bytestream_client.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/file_system/object_type.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" constexpr std::size_t kLargeSize = GRPC_DEFAULT_MAX_RECV_MESSAGE_LENGTH + 1; TEST_CASE("ByteStream Client: Transfer single blob", "[execution_api]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - auto stream = - ByteStreamClient{info->host, info->port, auth ? &*auth : nullptr}; + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + auto stream = ByteStreamClient{info->host, info->port, &*auth_config}; auto uuid = CreateUUIDVersion4(*CreateProcessUniqueId()); SECTION("Upload small blob") { @@ -112,12 +110,9 @@ TEST_CASE("ByteStream Client: Transfer single blob", "[execution_api]") { TEST_CASE("ByteStream Client: Transfer multiple blobs", "[execution_api]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - auto stream = - ByteStreamClient{info->host, info->port, auth ? &*auth : nullptr}; + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + auto stream = ByteStreamClient{info->host, info->port, &*auth_config}; auto uuid = CreateUUIDVersion4(*CreateProcessUniqueId()); SECTION("Upload small blobs") { diff --git a/test/buildtool/execution_engine/executor/TARGETS b/test/buildtool/execution_engine/executor/TARGETS index 0cca077d..df0f9df6 100644 --- a/test/buildtool/execution_engine/executor/TARGETS +++ b/test/buildtool/execution_engine/executor/TARGETS @@ -13,7 +13,6 @@ , ["@", "src", "src/buildtool/execution_engine/executor", "executor"] , ["@", "src", "src/buildtool/file_system", "file_system_manager"] , ["@", "src", "src/buildtool/progress_reporting", "progress"] - , ["utils", "test_env"] , ["@", "catch2", "", "catch2"] , ["@", "gsl", "", "gsl"] ] @@ -24,7 +23,8 @@ , "name": ["executor"] , "srcs": ["executor.test.cpp"] , "private-deps": - [ ["@", "src", "src/buildtool/common", "artifact_factory"] + [ ["@", "src", "src/buildtool/auth", "auth"] + , ["@", "src", "src/buildtool/common", "artifact_factory"] , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/common", "config"] , ["@", "src", "src/buildtool/execution_api/common", "common"] @@ -44,7 +44,6 @@ , "data": ["test_data"] , "private-deps": [ "executor_api_tests" - , ["@", "src", "src/buildtool/auth", "auth"] , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/common", "config"] , ["@", "src", "src/buildtool/execution_api/local", "local"] @@ -53,6 +52,7 @@ , ["@", "src", "src/buildtool/progress_reporting", "progress"] , ["utils", "catch-main-remote-execution"] , ["utils", "local_hermeticity"] + , ["utils", "test_auth_config"] , ["@", "catch2", "", "catch2"] ] , "stage": ["test", "buildtool", "execution_engine", "executor"] @@ -64,7 +64,6 @@ , "data": ["test_data"] , "private-deps": [ "executor_api_tests" - , ["@", "src", "src/buildtool/auth", "auth"] , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/common", "config"] , ["@", "src", "src/buildtool/execution_api/remote", "bazel"] @@ -72,6 +71,7 @@ , ["@", "src", "src/buildtool/execution_engine/executor", "executor"] , ["@", "src", "src/buildtool/progress_reporting", "progress"] , ["utils", "catch-main-remote-execution"] + , ["utils", "test_auth_config"] , ["@", "catch2", "", "catch2"] ] , "stage": ["test", "buildtool", "execution_engine", "executor"] diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp index a55c398d..dd9f59db 100644 --- a/test/buildtool/execution_engine/executor/executor.test.cpp +++ b/test/buildtool/execution_engine/executor/executor.test.cpp @@ -23,6 +23,7 @@ #include "catch2/catch_test_macros.hpp" #include "gsl/gsl" +#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/artifact_factory.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" @@ -280,12 +281,13 @@ TEST_CASE("Executor: Process artifact", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -299,12 +301,13 @@ TEST_CASE("Executor: Process artifact", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -318,12 +321,13 @@ TEST_CASE("Executor: Process artifact", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -360,12 +364,13 @@ TEST_CASE("Executor: Process action", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -382,12 +387,13 @@ TEST_CASE("Executor: Process action", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -404,12 +410,13 @@ TEST_CASE("Executor: Process action", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -429,12 +436,13 @@ TEST_CASE("Executor: Process action", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -451,12 +459,13 @@ TEST_CASE("Executor: Process action", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; @@ -476,12 +485,13 @@ TEST_CASE("Executor: Process action", "[executor]") { auto api = TestApi::Ptr{new TestApi{config}}; Statistics stats{}; Progress progress{}; + Auth auth{}; Executor runner{&repo_config, api.get(), api.get(), /*properties=*/{}, /*dispatch_list=*/{}, - /*auth=*/nullptr, + &auth, &stats, &progress}; diff --git a/test/buildtool/execution_engine/executor/executor_api.test.hpp b/test/buildtool/execution_engine/executor/executor_api.test.hpp index db80e861..e499efae 100644 --- a/test/buildtool/execution_engine/executor/executor_api.test.hpp +++ b/test/buildtool/execution_engine/executor/executor_api.test.hpp @@ -24,7 +24,6 @@ #include "catch2/catch_test_macros.hpp" #include "gsl/gsl" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/artifact.hpp" #include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/artifact_factory.hpp" @@ -36,7 +35,6 @@ #include "src/buildtool/execution_engine/executor/executor.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/progress_reporting/progress.hpp" -#include "test/utils/test_env.hpp" using ApiFactory = std::function<IExecutionApi::Ptr()>; @@ -94,7 +92,7 @@ static inline void RunHelloWorldCompilation( gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool is_hermetic = true, int expected_queued = 0, int expected_cached = 0) { @@ -168,7 +166,7 @@ static inline void RunGreeterCompilation( gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, std::string const& greetcpp, bool is_hermetic = true, int expected_queued = 0, @@ -303,7 +301,7 @@ static inline void RunGreeterCompilation( gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool is_hermetic = true) { SetupConfig(repo_config); // expecting 1 action queued, 0 results from cache @@ -324,7 +322,7 @@ static inline void RunGreeterCompilation( gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool is_hermetic = true) { SetupConfig(repo_config); // expecting 3 action queued, 0 results from cache @@ -371,7 +369,7 @@ static inline void TestUploadAndDownloadTrees( gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool /*is_hermetic*/ = true, int /*expected_queued*/ = 0, int /*expected_cached*/ = 0) { @@ -525,7 +523,7 @@ static inline void TestRetrieveOutputDirectories( gsl::not_null<Statistics*> const& stats, gsl::not_null<Progress*> const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool /*is_hermetic*/ = true, int /*expected_queued*/ = 0, int /*expected_cached*/ = 0) { diff --git a/test/buildtool/execution_engine/executor/executor_api_local.test.cpp b/test/buildtool/execution_engine/executor/executor_api_local.test.cpp index b6ffe2d7..0572dc7b 100755 --- a/test/buildtool/execution_engine/executor/executor_api_local.test.cpp +++ b/test/buildtool/execution_engine/executor/executor_api_local.test.cpp @@ -13,10 +13,8 @@ // limitations under the License. #include <memory> -#include <optional> #include "catch2/catch_test_macros.hpp" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" #include "src/buildtool/execution_api/local/local_api.hpp" @@ -25,6 +23,7 @@ #include "src/buildtool/progress_reporting/progress.hpp" #include "test/buildtool/execution_engine/executor/executor_api.test.hpp" #include "test/utils/hermeticity/local.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" TEST_CASE_METHOD(HermeticLocalTestFixture, "Executor<LocalApi>: Upload blob", @@ -40,18 +39,14 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; - - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestHelloWorldCompilation( &repo_config, &stats, &progress, [&] { return std::make_unique<LocalApi>(&repo_config); }, - auth ? &*auth : nullptr); + &*auth_config); } TEST_CASE_METHOD(HermeticLocalTestFixture, @@ -60,18 +55,14 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; - - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestGreeterCompilation( &repo_config, &stats, &progress, [&] { return std::make_unique<LocalApi>(&repo_config); }, - auth ? &*auth : nullptr); + &*auth_config); } TEST_CASE_METHOD(HermeticLocalTestFixture, @@ -80,18 +71,14 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; - - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestUploadAndDownloadTrees( &repo_config, &stats, &progress, [&] { return std::make_unique<LocalApi>(&repo_config); }, - auth ? &*auth : nullptr); + &*auth_config); } TEST_CASE_METHOD(HermeticLocalTestFixture, @@ -100,16 +87,12 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; - - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestRetrieveOutputDirectories( &repo_config, &stats, &progress, [&] { return std::make_unique<LocalApi>(&repo_config); }, - auth ? &*auth : nullptr); + &*auth_config); } diff --git a/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp b/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp index 83554e64..d2f41bf1 100755 --- a/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp +++ b/test/buildtool/execution_engine/executor/executor_api_remote_bazel.test.cpp @@ -15,7 +15,6 @@ #include <optional> #include "catch2/catch_test_macros.hpp" -#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" #include "src/buildtool/execution_api/remote/bazel/bazel_api.hpp" @@ -23,22 +22,18 @@ #include "src/buildtool/execution_engine/executor/executor.hpp" #include "src/buildtool/progress_reporting/progress.hpp" #include "test/buildtool/execution_engine/executor/executor_api.test.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" TEST_CASE("Executor<BazelApi>: Upload blob", "[executor]") { RepositoryConfig repo_config{}; ExecutionConfiguration config; auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestBlobUpload(&repo_config, [&] { - return BazelApi::Ptr{new BazelApi{"remote-execution", - info->host, - info->port, - auth ? &*auth : nullptr, - config}}; + return BazelApi::Ptr{new BazelApi{ + "remote-execution", info->host, info->port, &*auth_config, config}}; }); } @@ -51,10 +46,8 @@ TEST_CASE("Executor<BazelApi>: Compile hello world", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestHelloWorldCompilation( &repo_config, @@ -64,10 +57,10 @@ TEST_CASE("Executor<BazelApi>: Compile hello world", "[executor]") { return BazelApi::Ptr{new BazelApi{"remote-execution", info->host, info->port, - auth ? &*auth : nullptr, + &*auth_config, config}}; }, - auth ? &*auth : nullptr, + &*auth_config, false /* not hermetic */); } @@ -80,10 +73,8 @@ TEST_CASE("Executor<BazelApi>: Compile greeter", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestGreeterCompilation( &repo_config, @@ -93,10 +84,10 @@ TEST_CASE("Executor<BazelApi>: Compile greeter", "[executor]") { return BazelApi::Ptr{new BazelApi{"remote-execution", info->host, info->port, - auth ? &*auth : nullptr, + &*auth_config, config}}; }, - auth ? &*auth : nullptr, + &*auth_config, false /* not hermetic */); } @@ -109,10 +100,8 @@ TEST_CASE("Executor<BazelApi>: Upload and download trees", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestUploadAndDownloadTrees( &repo_config, @@ -122,10 +111,10 @@ TEST_CASE("Executor<BazelApi>: Upload and download trees", "[executor]") { return BazelApi::Ptr{new BazelApi{"remote-execution", info->host, info->port, - auth ? &*auth : nullptr, + &*auth_config, config}}; }, - auth ? &*auth : nullptr, + &*auth_config, false /* not hermetic */); } @@ -138,10 +127,8 @@ TEST_CASE("Executor<BazelApi>: Retrieve output directories", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); TestRetrieveOutputDirectories( &repo_config, @@ -151,9 +138,9 @@ TEST_CASE("Executor<BazelApi>: Retrieve output directories", "[executor]") { return BazelApi::Ptr{new BazelApi{"remote-execution", info->host, info->port, - auth ? &*auth : nullptr, + &*auth_config, config}}; }, - auth ? &*auth : nullptr, + &*auth_config, false /* not hermetic */); } diff --git a/test/buildtool/graph_traverser/TARGETS b/test/buildtool/graph_traverser/TARGETS index c67ded03..1d11b04a 100644 --- a/test/buildtool/graph_traverser/TARGETS +++ b/test/buildtool/graph_traverser/TARGETS @@ -15,7 +15,6 @@ , ["@", "src", "src/buildtool/logging", "logging"] , ["@", "src", "src/buildtool/progress_reporting", "progress"] , ["@", "src", "src/buildtool/execution_api/common", "api_bundle"] - , ["utils", "test_env"] ] , "stage": ["test", "buildtool", "graph_traverser"] } @@ -29,6 +28,7 @@ , ["@", "catch2", "", "catch2"] , ["", "catch-main"] , ["utils", "local_hermeticity"] + , ["utils", "test_auth_config"] ] , "stage": ["test", "buildtool", "graph_traverser"] } @@ -41,7 +41,7 @@ [ "graph_traverser_tests" , ["@", "catch2", "", "catch2"] , ["utils", "catch-main-remote-execution"] - , ["@", "src", "src/buildtool/auth", "auth"] + , ["utils", "test_auth_config"] ] , "stage": ["test", "buildtool", "graph_traverser"] } diff --git a/test/buildtool/graph_traverser/graph_traverser.test.hpp b/test/buildtool/graph_traverser/graph_traverser.test.hpp index 7d23dcd5..f1be7eec 100644 --- a/test/buildtool/graph_traverser/graph_traverser.test.hpp +++ b/test/buildtool/graph_traverser/graph_traverser.test.hpp @@ -26,6 +26,7 @@ #include <vector> #include "catch2/catch_test_macros.hpp" +#include "gsl/gsl" #include "nlohmann/json.hpp" #include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/common/statistics.hpp" @@ -38,7 +39,6 @@ #include "src/buildtool/logging/logger.hpp" #include "src/buildtool/progress_reporting/progress.hpp" #include "src/utils/cpp/json.hpp" -#include "test/utils/test_env.hpp" // NOLINTNEXTLINE(google-build-namespaces) namespace { @@ -152,7 +152,7 @@ inline void SetLauncher() { } // namespace [[maybe_unused]] static void TestHelloWorldCopyMessage( - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool is_hermetic = true) { TestProject p("hello_world_copy_message"); @@ -217,8 +217,9 @@ inline void SetLauncher() { } } -[[maybe_unused]] static void TestCopyLocalFile(Auth::TLS const* auth, - bool is_hermetic = true) { +[[maybe_unused]] static void TestCopyLocalFile( + gsl::not_null<Auth const*> const& auth, + bool is_hermetic = true) { TestProject p("copy_local_file"); SetLauncher(); @@ -249,7 +250,7 @@ inline void SetLauncher() { } [[maybe_unused]] static void TestSequencePrinterBuildLibraryOnly( - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool is_hermetic = true) { TestProject p("sequence_printer_build_library_only"); @@ -301,7 +302,7 @@ inline void SetLauncher() { } [[maybe_unused]] static void TestHelloWorldWithKnownSource( - Auth::TLS const* auth, + gsl::not_null<Auth const*> const& auth, bool is_hermetic = true) { TestProject full_hello_world("hello_world_copy_message"); @@ -360,7 +361,7 @@ inline void SetLauncher() { } } -static void TestBlobsUploadedAndUsed(Auth::TLS const* auth, +static void TestBlobsUploadedAndUsed(gsl::not_null<Auth const*> const& auth, bool is_hermetic = true) { TestProject p("use_uploaded_blobs"); auto const clargs = p.CmdLineArgs(); @@ -399,8 +400,9 @@ static void TestBlobsUploadedAndUsed(Auth::TLS const* auth, } } -static void TestEnvironmentVariablesSetAndUsed(Auth::TLS const* auth, - bool is_hermetic = true) { +static void TestEnvironmentVariablesSetAndUsed( + gsl::not_null<Auth const*> const& auth, + bool is_hermetic = true) { TestProject p("use_env_variables"); auto const clargs = p.CmdLineArgs(); @@ -438,7 +440,8 @@ static void TestEnvironmentVariablesSetAndUsed(Auth::TLS const* auth, } } -static void TestTreesUsed(Auth::TLS const* auth, bool is_hermetic = true) { +static void TestTreesUsed(gsl::not_null<Auth const*> const& auth, + bool is_hermetic = true) { TestProject p("use_trees"); auto const clargs = p.CmdLineArgs(); @@ -476,7 +479,7 @@ static void TestTreesUsed(Auth::TLS const* auth, bool is_hermetic = true) { } } -static void TestNestedTreesUsed(Auth::TLS const* auth, +static void TestNestedTreesUsed(gsl::not_null<Auth const*> const& auth, bool is_hermetic = true) { TestProject p("use_nested_trees"); auto const clargs = p.CmdLineArgs(); @@ -515,7 +518,7 @@ static void TestNestedTreesUsed(Auth::TLS const* auth, } } -static void TestFlakyHelloWorldDetected(Auth::TLS const* auth, +static void TestFlakyHelloWorldDetected(gsl::not_null<Auth const*> const& auth, bool /*is_hermetic*/ = true) { TestProject p("flaky_hello_world"); diff --git a/test/buildtool/graph_traverser/graph_traverser_local.test.cpp b/test/buildtool/graph_traverser/graph_traverser_local.test.cpp index 64d7189b..88990dc8 100644 --- a/test/buildtool/graph_traverser/graph_traverser_local.test.cpp +++ b/test/buildtool/graph_traverser/graph_traverser_local.test.cpp @@ -13,59 +13,70 @@ // limitations under the License. #include "catch2/catch_test_macros.hpp" +#include "src/buildtool/auth/authentication.hpp" #include "test/buildtool/graph_traverser/graph_traverser.test.hpp" #include "test/utils/hermeticity/local.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Output created when entry point is local artifact", "[graph_traverser]") { - TestCopyLocalFile(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestCopyLocalFile(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Output created and contents are correct", "[graph_traverser]") { - TestHelloWorldCopyMessage(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestHelloWorldCopyMessage(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Actions are not re-run", "[graph_traverser]") { - TestSequencePrinterBuildLibraryOnly(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestSequencePrinterBuildLibraryOnly(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: KNOWN artifact", "[graph_traverser]") { - TestHelloWorldWithKnownSource(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestHelloWorldWithKnownSource(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Blobs uploaded and correctly used", "[graph_traverser]") { - TestBlobsUploadedAndUsed(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestBlobsUploadedAndUsed(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Environment variables are set and used", "[graph_traverser]") { - TestEnvironmentVariablesSetAndUsed(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestEnvironmentVariablesSetAndUsed(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Trees correctly used", "[graph_traverser]") { - TestTreesUsed(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestTreesUsed(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Nested trees correctly used", "[graph_traverser]") { - TestNestedTreesUsed(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestNestedTreesUsed(&auth); } TEST_CASE_METHOD(HermeticLocalTestFixture, "Local: Detect flaky actions", "[graph_traverser]") { - TestFlakyHelloWorldDetected(/*auth=*/nullptr); + Auth auth{}; /*no TLS needed*/ + TestFlakyHelloWorldDetected(&auth); } diff --git a/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp b/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp index e33d5d27..e24a8d4e 100644 --- a/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp +++ b/test/buildtool/graph_traverser/graph_traverser_remote.test.cpp @@ -12,88 +12,73 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include <optional> - #include "catch2/catch_test_macros.hpp" -#include "src/buildtool/auth/authentication.hpp" #include "test/buildtool/graph_traverser/graph_traverser.test.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" TEST_CASE("Remote: Output created and contents are correct", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestHelloWorldCopyMessage(auth ? &*auth : nullptr, - false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestHelloWorldCopyMessage(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: Output created when entry point is local artifact", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestCopyLocalFile(auth ? &*auth : nullptr, false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestCopyLocalFile(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: Actions are not re-run", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestSequencePrinterBuildLibraryOnly(auth ? &*auth : nullptr, + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestSequencePrinterBuildLibraryOnly(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: KNOWN artifact", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestHelloWorldWithKnownSource(auth ? &*auth : nullptr, - false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestHelloWorldWithKnownSource(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: Blobs uploaded and correctly used", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestBlobsUploadedAndUsed(auth ? &*auth : nullptr, false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestBlobsUploadedAndUsed(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: Environment variables are set and used", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestEnvironmentVariablesSetAndUsed(auth ? &*auth : nullptr, - false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestEnvironmentVariablesSetAndUsed(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: Trees correctly used", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestTreesUsed(auth ? &*auth : nullptr, false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestTreesUsed(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: Nested trees correctly used", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestNestedTreesUsed(auth ? &*auth : nullptr, false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestNestedTreesUsed(&*auth_config, false /* not hermetic */); } TEST_CASE("Remote: Detect flaky actions", "[graph_traverser]") { - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); - } - TestFlakyHelloWorldDetected(auth ? &*auth : nullptr, - false /* not hermetic */); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + REQUIRE(auth_config); + + TestFlakyHelloWorldDetected(&*auth_config, false /* not hermetic */); } diff --git a/test/buildtool/serve_api/TARGETS b/test/buildtool/serve_api/TARGETS index 190f1445..279692ad 100644 --- a/test/buildtool/serve_api/TARGETS +++ b/test/buildtool/serve_api/TARGETS @@ -6,6 +6,7 @@ , "private-deps": [ ["@", "catch2", "", "catch2"] , ["utils", "catch-main-serve"] + , ["@", "src", "src/buildtool/auth", "auth"] , ["@", "src", "src/buildtool/serve_api/remote", "source_tree_client"] , ["utils", "test_serve_config"] , ["@", "src", "src/buildtool/serve_api/remote", "config"] diff --git a/test/buildtool/serve_api/source_tree_client.test.cpp b/test/buildtool/serve_api/source_tree_client.test.cpp index fb506dc2..970d2aca 100644 --- a/test/buildtool/serve_api/source_tree_client.test.cpp +++ b/test/buildtool/serve_api/source_tree_client.test.cpp @@ -16,6 +16,7 @@ #include <variant> #include "catch2/catch_test_macros.hpp" +#include "src/buildtool/auth/authentication.hpp" #include "src/buildtool/serve_api/remote/config.hpp" #include "src/buildtool/serve_api/remote/source_tree_client.hpp" #include "test/utils/serve_service/test_serve_config.hpp" @@ -36,7 +37,8 @@ TEST_CASE("Serve service client: tree-of-commit request", "[serve_api]") { REQUIRE(config->remote_address); // Create TLC client - SourceTreeClient st_client(*config->remote_address, nullptr); + Auth auth{}; + SourceTreeClient st_client(*config->remote_address, &auth); SECTION("Commit in bare checkout") { auto root_id = st_client.ServeCommitTree(kRootCommit, ".", false); diff --git a/test/utils/TARGETS b/test/utils/TARGETS index c0f85b49..e5ef694a 100644 --- a/test/utils/TARGETS +++ b/test/utils/TARGETS @@ -13,6 +13,8 @@ [ ["@", "gsl", "", "gsl"] , ["@", "src", "src/buildtool/execution_api/remote", "bazel_network"] , ["@", "src", "src/buildtool/common", "bazel_types"] + , "test_env" + , "test_auth_config" ] , "stage": ["test", "utils"] } @@ -76,6 +78,7 @@ , ["@", "src", "src/buildtool/compatibility", "compatibility"] , "log_config" , "test_env" + , "test_auth_config" ] , "stage": ["test", "utils"] } @@ -91,6 +94,18 @@ ] , "stage": ["test", "utils"] } +, "test_auth_config": + { "type": ["@", "rules", "CC", "library"] + , "name": ["test_auth_config"] + , "hdrs": ["remote_execution/test_auth_config.hpp"] + , "deps": + [ ["@", "src", "src/buildtool/serve_api/remote", "config"] + , ["@", "src", "src/buildtool/logging", "log_level"] + , ["@", "src", "src/buildtool/logging", "logging"] + , "test_env" + ] + , "stage": ["test", "utils"] + } , "catch-main-serve": { "type": ["@", "rules", "CC", "library"] , "name": ["catch-main-serve"] diff --git a/test/utils/remote_execution/bazel_action_creator.hpp b/test/utils/remote_execution/bazel_action_creator.hpp index b35b2325..38468535 100644 --- a/test/utils/remote_execution/bazel_action_creator.hpp +++ b/test/utils/remote_execution/bazel_action_creator.hpp @@ -28,6 +28,7 @@ #include "src/buildtool/crypto/hash_function.hpp" #include "src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp" #include "src/buildtool/execution_api/remote/config.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" [[nodiscard]] static inline auto CreateAction( std::string const& instance_name, @@ -82,12 +83,12 @@ auto action_id = ArtifactDigest::Create<ObjectType::File>(action_data); blobs.emplace_back(action_id, action_data, /*is_exec=*/false); - std::optional<Auth::TLS> auth = {}; - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - auth = Auth::TLS::Instance(); + auto auth_config = TestAuthConfig::ReadAuthConfigFromEnvironment(); + if (not auth_config) { + return nullptr; } - BazelCasClient cas_client(info->host, info->port, auth ? &*auth : nullptr); + BazelCasClient cas_client(info->host, info->port, &*auth_config); std::vector<gsl::not_null<BazelBlob const*>> blob_ptrs; blob_ptrs.reserve(blobs.size()); diff --git a/test/utils/remote_execution/main-remote-execution.cpp b/test/utils/remote_execution/main-remote-execution.cpp index fa47d585..7b5cba00 100644 --- a/test/utils/remote_execution/main-remote-execution.cpp +++ b/test/utils/remote_execution/main-remote-execution.cpp @@ -28,6 +28,7 @@ #include "src/buildtool/logging/logger.hpp" #include "src/buildtool/storage/storage.hpp" #include "test/utils/logging/log_config.hpp" +#include "test/utils/remote_execution/test_auth_config.hpp" #include "test/utils/test_env.hpp" namespace { @@ -42,9 +43,12 @@ void wait_for_grpc_to_shutdown() { /// \returns true If remote execution was successfully configured. [[nodiscard]] auto ConfigureRemoteExecution() -> bool { ReadCompatibilityFromEnv(); - if (not ReadTLSAuthArgsFromEnv()) { - return false; + + // Ensure authentication config is available + if (not TestAuthConfig::ReadAuthConfigFromEnvironment()) { + std::exit(EXIT_FAILURE); } + HashFunction::SetHashType(Compatibility::IsCompatible() ? HashFunction::JustHash::Compatible : HashFunction::JustHash::Native); diff --git a/test/utils/remote_execution/test_auth_config.hpp b/test/utils/remote_execution/test_auth_config.hpp new file mode 100644 index 00000000..fd157f22 --- /dev/null +++ b/test/utils/remote_execution/test_auth_config.hpp @@ -0,0 +1,50 @@ +// Copyright 2024 Huawei Cloud Computing Technology Co., Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef INCLUDED_SRC_TEST_UTILS_REMOTE_EXECUTION_TEST_AUTH_CONFIG_HPP +#define INCLUDED_SRC_TEST_UTILS_REMOTE_EXECUTION_TEST_AUTH_CONFIG_HPP + +#include <optional> +#include <string> +#include <variant> + +#include "src/buildtool/auth/authentication.hpp" +#include "src/buildtool/logging/log_level.hpp" +#include "src/buildtool/logging/logger.hpp" +#include "test/utils/test_env.hpp" + +class TestAuthConfig final { + public: + [[nodiscard]] static auto ReadAuthConfigFromEnvironment() noexcept + -> std::optional<Auth> { + Auth::TLS::Builder tls_builder; + auto config = tls_builder.SetCACertificate(ReadTLSAuthCACertFromEnv()) + .SetClientCertificate(ReadTLSAuthClientCertFromEnv()) + .SetClientKey(ReadTLSAuthClientKeyFromEnv()) + .Build(); + + if (config) { + if (*config) { + // correctly configured TLS/SSL certification + return *std::move(*config); + } + // given TLS certificates are invalid + Logger::Log(LogLevel::Error, config->error()); + return std::nullopt; + } + return Auth{}; // no TLS certificates provided + } +}; + +#endif // INCLUDED_SRC_TEST_UTILS_REMOTE_EXECUTION_TEST_AUTH_CONFIG_HPP diff --git a/test/utils/test_env.hpp b/test/utils/test_env.hpp index d9c60fb8..30eba26c 100644 --- a/test/utils/test_env.hpp +++ b/test/utils/test_env.hpp @@ -16,6 +16,7 @@ #define INCLUDED_SRC_TEST_UTILS_TEST_ENV_HPP #include <cstdlib> +#include <filesystem> #include <map> #include <optional> #include <sstream> @@ -62,31 +63,28 @@ static inline void ReadCompatibilityFromEnv() { : std::make_optional(std::string{serve_address}); } -[[nodiscard]] static inline auto ReadTLSAuthArgsFromEnv() -> bool { +[[nodiscard]] static inline auto ReadTLSAuthCACertFromEnv() + -> std::optional<std::filesystem::path> { auto* ca_cert = std::getenv("TLS_CA_CERT"); + return ca_cert == nullptr + ? std::nullopt + : std::make_optional(std::filesystem::path(ca_cert)); +} + +[[nodiscard]] static inline auto ReadTLSAuthClientCertFromEnv() + -> std::optional<std::filesystem::path> { auto* client_cert = std::getenv("TLS_CLIENT_CERT"); + return client_cert == nullptr + ? std::nullopt + : std::make_optional(std::filesystem::path(client_cert)); +} + +[[nodiscard]] static inline auto ReadTLSAuthClientKeyFromEnv() + -> std::optional<std::filesystem::path> { auto* client_key = std::getenv("TLS_CLIENT_KEY"); - if (ca_cert != nullptr) { - if (not Auth::TLS::Instance().SetCACertificate(ca_cert)) { - return false; - } - } - if (client_cert != nullptr) { - if (not Auth::TLS::Instance().SetClientCertificate(client_cert)) { - return false; - } - } - if (client_key != nullptr) { - if (not Auth::TLS::Instance().SetClientKey(client_key)) { - return false; - } - } - if (Auth::Instance().GetAuthMethod() == AuthMethod::kTLS) { - if (not Auth::TLS::Instance().Validate()) { - return false; - } - } - return true; + return client_key == nullptr + ? std::nullopt + : std::make_optional(std::filesystem::path(client_key)); } [[nodiscard]] static inline auto ReadRemoteServeReposFromEnv() |