diff options
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() |