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.

5 files changed, 63 insertions, 85 deletions

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 */);
 }