From c2f7ead468d5e65c57e7ecb49d7fbba4254c46b7 Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Tue, 2 Jul 2024 16:47:13 +0200 Subject: Replace the Auth and Auth::TLS singletons Use a builder pattern for creation and validation, in a manner that allows also other authentication methods to be added in the future besides the current TLS/SSL. The main Auth instances are built early and then passed by not_null const pointers, to avoid passing temporaries, replacing the previous Auth::TLS instances passed by simple nullable const pointers. Where needed, these passed Auth instances are also stored, by const ref. Tests also build Auth instances as needed, either with the default 'no certification' or from the test environment arguments. --- test/buildtool/build_engine/target_map/TARGETS | 1 + .../build_engine/target_map/target_map.test.cpp | 49 ++++++------ test/buildtool/execution_api/bazel/TARGETS | 10 +-- .../execution_api/bazel/bazel_api.test.cpp | 17 ++--- .../execution_api/bazel/bazel_cas_client.test.cpp | 11 +-- .../bazel/bazel_execution_client.test.cpp | 20 ++--- .../execution_api/bazel/bazel_network.test.cpp | 21 +++-- .../execution_api/bazel/bytestream_client.test.cpp | 19 ++--- test/buildtool/execution_engine/executor/TARGETS | 8 +- .../execution_engine/executor/executor.test.cpp | 28 ++++--- .../executor/executor_api.test.hpp | 14 ++-- .../executor/executor_api_local.test.cpp | 43 ++++------- .../executor/executor_api_remote_bazel.test.cpp | 55 +++++-------- test/buildtool/graph_traverser/TARGETS | 4 +- .../graph_traverser/graph_traverser.test.hpp | 27 ++++--- .../graph_traverser/graph_traverser_local.test.cpp | 29 ++++--- .../graph_traverser_remote.test.cpp | 89 +++++++++------------- test/buildtool/serve_api/TARGETS | 1 + .../serve_api/source_tree_client.test.cpp | 4 +- test/utils/TARGETS | 15 ++++ .../remote_execution/bazel_action_creator.hpp | 9 ++- .../remote_execution/main-remote-execution.cpp | 8 +- test/utils/remote_execution/test_auth_config.hpp | 50 ++++++++++++ test/utils/test_env.hpp | 42 +++++----- 24 files changed, 300 insertions(+), 274 deletions(-) create mode 100644 test/utils/remote_execution/test_auth_config.hpp (limited to 'test') 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 // 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 -#include #include #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 = {}; - 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 // std::equal_to -#include #include #include #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 = {}; - 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 #include #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( ArtifactDigest::Create(content)); - std::optional 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( ArtifactDigest::Create(content)); - std::optional 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 = {}; - 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 = {}; - 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 = {}; - 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 = {}; - 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; @@ -94,7 +92,7 @@ static inline void RunHelloWorldCompilation( gsl::not_null const& stats, gsl::not_null const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null 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 const& stats, gsl::not_null const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null 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 const& stats, gsl::not_null const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null 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 const& stats, gsl::not_null const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null 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 const& stats, gsl::not_null const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null 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 const& stats, gsl::not_null const& progress, ApiFactory const& factory, - Auth::TLS const* auth, + gsl::not_null 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 -#include #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: Upload blob", @@ -40,18 +39,14 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; - - std::optional 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(&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 = {}; - 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(&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 = {}; - 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(&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 = {}; - 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(&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 #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: Upload blob", "[executor]") { RepositoryConfig repo_config{}; ExecutionConfiguration config; auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional 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: Compile hello world", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional 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: 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: Compile greeter", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional 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: 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: Upload and download trees", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional 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: 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: Retrieve output directories", "[executor]") { auto const& info = RemoteExecutionConfig::RemoteAddress(); - std::optional 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: 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 #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 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 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 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 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 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 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 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 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 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 - #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 = {}; - 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 = {}; - 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 = {}; - 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 = {}; - 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 = {}; - 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 = {}; - 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 = {}; - 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 = {}; - 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 = {}; - 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 #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(action_data); blobs.emplace_back(action_id, action_data, /*is_exec=*/false); - std::optional 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> 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 +#include +#include + +#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::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 +#include #include #include #include @@ -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 { 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 { 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 { 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() -- cgit v1.2.3