From 503215ae51440be4b7c190a079574d0ffc5c0b33 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 18 Feb 2022 17:58:18 -0800 Subject: [PATCH 01/29] Changed one line --- sdk/attestation/azure-security-attestation/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/attestation/azure-security-attestation/README.md b/sdk/attestation/azure-security-attestation/README.md index b211eb9a8a..aefdb34132 100644 --- a/sdk/attestation/azure-security-attestation/README.md +++ b/sdk/attestation/azure-security-attestation/README.md @@ -397,3 +397,4 @@ Azure SDK for C++ is licensed under the [MIT](https://github.com/Azure/azure-sdk [cloud_shell_bash]: https://shell.azure.com/bash ![Impressions](https://azure-sdk-impressions.azurewebsites.net/api/impressions/azure-sdk-for-cpp%2Fsdk%2Fattestation%2Fazure-security-attestation%2FREADME.png) + From 639af2d91d96f8fe9e78250611e51a53af476bc5 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 22 Feb 2022 12:39:12 -0800 Subject: [PATCH 02/29] Set WinHTTP_OPTION_CLIENT_CERT_CONTEXT to enable connections to attestation service --- .../src/http/winhttp/win_http_transport.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp index 50fa628815..c35186004d 100644 --- a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp +++ b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp @@ -330,6 +330,19 @@ void WinHttpTransport::CreateRequestHandle(std::unique_ptr<_detail::HandleManage // ERROR_NOT_ENOUGH_MEMORY GetErrorAndThrow("Error while getting a request handle."); } + + // If the service requests TLS client certificates, we want to let the WinHTTP APIs know that it's ok + // to initiate the request without a client certificate. + // + // Note: If/When TLS client certificate support is added to the pipeline, this line may need to be revisited. + if (!WinHttpSetOption( + handleManager->m_requestHandle, + WINHTTP_OPTION_CLIENT_CERT_CONTEXT, + WINHTTP_NO_CLIENT_CERT_CONTEXT, + 0)) + { + GetErrorAndThrow("Error while setting client cert context to ignore.."); + } } // For PUT/POST requests, send additional data using WinHttpWriteData. From b5486358086e89fc8c3af28fefd9c9c2790b1c3f Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 22 Feb 2022 12:40:04 -0800 Subject: [PATCH 03/29] Map service specific CLIENT_ID, CLIENT_SECRET, and TENANT_ID to AZURE_ equivalent on live tests - parallels what is done for Python tests --- .../private/attestation_client_private.hpp | 2 +- .../test/ut/administration_test.cpp | 2 +- .../test/ut/attestation_test.cpp | 2 +- .../inc/azure/core/test/test_base.hpp | 47 +++++++++++++++++-- .../test/ut/token_credential_test.cpp | 2 +- .../test/ut/certificate_client_base_test.hpp | 2 +- .../test/ut/key_client_base_test.hpp | 2 +- .../test/ut/secret_client_base_test.hpp | 2 +- .../test/ut/test_base.hpp | 2 +- 9 files changed, 51 insertions(+), 12 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index b3ae8c5161..a33e4258dd 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -425,6 +425,6 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail ValidateTokenIssuer(validationOptions); } - operator Models::AttestationToken&&() { return std::move(m_token); } + operator Models::AttestationToken &&() { return std::move(m_token); } }; }}}} // namespace Azure::Security::Attestation::_detail diff --git a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp index aebb8883db..d62a0c3487 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp @@ -25,7 +25,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { // Create virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase("ATTESTATION", AZURE_TEST_RECORDING_DIR); std::string const mode(std::get<0>(GetParam())); if (mode == "Shared") diff --git a/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp index 1ab8473056..6f9bcceb67 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp @@ -22,7 +22,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { // Create virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase("ATTESTATION", AZURE_TEST_RECORDING_DIR); std::string mode(GetParam()); if (mode == "Shared") diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index ec916d032f..8f114a88c6 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -17,6 +17,7 @@ #include "azure/core/test/network_models.hpp" #include "azure/core/test/test_context_manager.hpp" +#include #include #include #include @@ -45,6 +46,7 @@ namespace Azure { namespace Core { namespace Test { * */ bool m_wasSkipped = false; + std::string m_serviceName{}; void PrepareOptions(Azure::Core::_internal::ClientOptions& options) { @@ -244,10 +246,26 @@ namespace Azure { namespace Core { namespace Test { } // Util for tests getting env vars - std::string GetEnv(const std::string& name) + std::string GetEnv(std::string const& name) { - const auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str()); - +#if !defined(NDEBUG) + // The azure CI pipeline uppercases all EnvVar values from ci.yml files. + // That means that any mixed case strings will not be found when run from the CI + // pipeline. + // + // Check to ensure that the filename is upper case + { + std::string ucName = name; + std::transform(ucName.begin(), ucName.end(), ucName.begin(), [](char const& ch) { + return static_cast(std::toupper(ch)); + }); + if (ucName != name) + { + throw std::runtime_error("All Azure SDK environment variables must be all upper case."); + } + } +#endif + auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str()); if (ret.empty()) { throw std::runtime_error("Missing required environment variable: " + name); @@ -262,8 +280,11 @@ namespace Azure { namespace Core { namespace Test { * @brief Run before each test. * */ - void SetUpTestBase(std::string const& baseRecordingPath) + void SetUpTestBase(std::string const& serviceName, std::string const& baseRecordingPath) { + + m_serviceName = serviceName; + // Init interceptor from PlayBackRecorder std::string recordingPath(baseRecordingPath); recordingPath.append("/recordings"); @@ -276,6 +297,24 @@ namespace Azure { namespace Core { namespace Test { Sanitize(testNameInfo->test_suite_name()), Sanitize(testNameInfo->name())); m_testContext.RecordingPath = recordingPath; m_interceptor = std::make_unique(m_testContext); + + if (!m_testContext.IsPlaybackMode()) + { + auto SetBuiltinEnvironment + = [](std::string const& serviceName, std::string const& targetVariable) { + std::string targetValue = Azure::Core::_internal::Environment::GetVariable( + (serviceName + targetVariable).c_str()); + if (!targetValue.empty()) + { + Azure::Core::_internal::Environment::SetVariable( + ("AZURE" + targetVariable).c_str(), targetValue.c_str()); + } + }; + ; + SetBuiltinEnvironment(serviceName, "_TENANT_ID"); + SetBuiltinEnvironment(serviceName, "_CLIENT_ID"); + SetBuiltinEnvironment(serviceName, "_CLIENT_SECRET"); + } } /** diff --git a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp index 1ad894727d..9f8ac866cc 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp @@ -45,7 +45,7 @@ namespace Azure { namespace Identity { namespace Test { // Runs before every test. virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase("IDENTITY", AZURE_TEST_RECORDING_DIR); } }; }}} // namespace Azure::Identity::Test diff --git a/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp b/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp index dde88453e2..b40f5f00b6 100644 --- a/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp +++ b/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp @@ -65,7 +65,7 @@ namespace Azure { // Runs before every test. virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase("KEYVAULT", AZURE_TEST_RECORDING_DIR); m_keyVaultUrl = GetEnv("AZURE_KEYVAULT_URL"); // Options and credential for the client diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp index d7553a6f2f..245bfa71e8 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp @@ -54,7 +54,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys { nam // Create virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase("KEYVAULT", AZURE_TEST_RECORDING_DIR); m_keyVaultUrl = GetEnv("AZURE_KEYVAULT_URL"); m_keyVaultHsmUrl = GetEnv("AZURE_KEYVAULT_HSM_URL"); diff --git a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp index a37bf6ad3b..e933a52039 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp +++ b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp @@ -41,7 +41,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets { // Create void InitializeClient() { - Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase("KEYVAULT", AZURE_TEST_RECORDING_DIR); m_keyVaultUrl = GetEnv("AZURE_KEYVAULT_URL"); // Options and credential for the client diff --git a/sdk/storage/azure-storage-common/test/ut/test_base.hpp b/sdk/storage/azure-storage-common/test/ut/test_base.hpp index d75ba434af..fa71854e1b 100644 --- a/sdk/storage/azure-storage-common/test/ut/test_base.hpp +++ b/sdk/storage/azure-storage-common/test/ut/test_base.hpp @@ -132,7 +132,7 @@ namespace Azure { namespace Storage { virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase("STORAGE", AZURE_TEST_RECORDING_DIR); } public: From f82904f3405cbb632f055c2f36c1b3912601ce88 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 22 Feb 2022 12:54:05 -0800 Subject: [PATCH 04/29] clang-format --- .../azure-core/src/http/winhttp/win_http_transport.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp index c35186004d..0dce686037 100644 --- a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp +++ b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp @@ -331,10 +331,11 @@ void WinHttpTransport::CreateRequestHandle(std::unique_ptr<_detail::HandleManage GetErrorAndThrow("Error while getting a request handle."); } - // If the service requests TLS client certificates, we want to let the WinHTTP APIs know that it's ok - // to initiate the request without a client certificate. + // If the service requests TLS client certificates, we want to let the WinHTTP APIs know that it's + // ok to initiate the request without a client certificate. // - // Note: If/When TLS client certificate support is added to the pipeline, this line may need to be revisited. + // Note: If/When TLS client certificate support is added to the pipeline, this line may need to be + // revisited. if (!WinHttpSetOption( handleManager->m_requestHandle, WINHTTP_OPTION_CLIENT_CERT_CONTEXT, From fc1a481c921f0b31a2135a89657e6292a64e1f2f Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 22 Feb 2022 13:08:25 -0800 Subject: [PATCH 05/29] clang-format again --- .../src/private/attestation_client_private.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index a33e4258dd..b5a535ba22 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -425,6 +425,9 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail ValidateTokenIssuer(validationOptions); } - operator Models::AttestationToken &&() { return std::move(m_token); } + /** + * @brief Convert the internal attestation token to a public AttestationToken object. + */ + operator Models::AttestationToken&() { return m_token; } }; }}}} // namespace Azure::Security::Attestation::_detail From 9c3966ae3563355c78b4d6ef10fcf996a5585f8c Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 22 Feb 2022 14:09:40 -0800 Subject: [PATCH 06/29] Only set TLS options if we request TLS --- .../src/http/winhttp/win_http_transport.cpp | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp index 0dce686037..578529bebb 100644 --- a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp +++ b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp @@ -303,6 +303,9 @@ void WinHttpTransport::CreateRequestHandle(std::unique_ptr<_detail::HandleManage { const std::string& path = handleManager->m_request.GetUrl().GetRelativeUrl(); HttpMethod requestMethod = handleManager->m_request.GetMethod(); + bool requestSecureHttp( + !Azure::Core::_internal::StringExtensions::LocaleInvariantCaseInsensitiveEqual( + handleManager->m_request.GetUrl().GetScheme(), HttpScheme)); // Create an HTTP request handle. handleManager->m_requestHandle = WinHttpOpenRequest( @@ -314,10 +317,7 @@ void WinHttpTransport::CreateRequestHandle(std::unique_ptr<_detail::HandleManage NULL, // Use HTTP/1.1 WINHTTP_NO_REFERER, WINHTTP_DEFAULT_ACCEPT_TYPES, // No media types are accepted by the client - Azure::Core::_internal::StringExtensions::LocaleInvariantCaseInsensitiveEqual( - handleManager->m_request.GetUrl().GetScheme(), HttpScheme) - ? 0 - : WINHTTP_FLAG_SECURE); // Uses secure transaction semantics (SSL/TLS) + requestSecureHttp ? WINHTTP_FLAG_SECURE : 0); // Uses secure transaction semantics (SSL/TLS) if (!handleManager->m_requestHandle) { @@ -331,18 +331,21 @@ void WinHttpTransport::CreateRequestHandle(std::unique_ptr<_detail::HandleManage GetErrorAndThrow("Error while getting a request handle."); } - // If the service requests TLS client certificates, we want to let the WinHTTP APIs know that it's - // ok to initiate the request without a client certificate. - // - // Note: If/When TLS client certificate support is added to the pipeline, this line may need to be - // revisited. - if (!WinHttpSetOption( - handleManager->m_requestHandle, - WINHTTP_OPTION_CLIENT_CERT_CONTEXT, - WINHTTP_NO_CLIENT_CERT_CONTEXT, - 0)) + if (requestSecureHttp) { - GetErrorAndThrow("Error while setting client cert context to ignore.."); + // If the service requests TLS client certificates, we want to let the WinHTTP APIs know that + // it's ok to initiate the request without a client certificate. + // + // Note: If/When TLS client certificate support is added to the pipeline, this line may need to + // be revisited. + if (!WinHttpSetOption( + handleManager->m_requestHandle, + WINHTTP_OPTION_CLIENT_CERT_CONTEXT, + WINHTTP_NO_CLIENT_CERT_CONTEXT, + 0)) + { + GetErrorAndThrow("Error while setting client cert context to ignore.."); + } } } From 48e5ce5eaf47dbfd7cdfd79c5849973225afa6ab Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 22 Feb 2022 16:02:04 -0800 Subject: [PATCH 07/29] Fixed uninitialized variable problem in storage --- .../test/ut/block_blob_client_test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp b/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp index 3f58a6595e..d0250e7ca1 100644 --- a/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp +++ b/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp @@ -701,7 +701,7 @@ namespace Azure { namespace Storage { namespace Test { TEST_P(DownloadBlockBlob, downloadToBuffer) { - auto const p = GetParam(); + BlobConcurrentDownloadParameter const& p(GetParam()); auto const testName(GetTestName(true)); auto client = GetBlockBlobClient(testName); UploadBlockBlob(8_MB); @@ -773,7 +773,7 @@ namespace Azure { namespace Storage { namespace Test { TEST_P(DownloadBlockBlob, downloadToFile) { - auto const p = GetParam(); + BlobConcurrentDownloadParameter const& p(GetParam()); auto const testName(GetTestName(true)); auto client = GetBlockBlobClient(testName); UploadBlockBlob(8_MB); @@ -1191,7 +1191,7 @@ namespace Azure { namespace Storage { namespace Test { auto const testName(GetTestName()); auto blockBlobClient = GetBlockBlobClient(testName); SetOptions(); - auto const p = GetParam(); + BlobConcurrentDownloadParameter const& p(GetParam()); auto const blobSize = p.Size; std::vector blobContent(static_cast(8_MB), 'x'); @@ -1227,7 +1227,7 @@ namespace Azure { namespace Storage { namespace Test { auto const testName(GetTestName()); auto blockBlobClient = GetBlockBlobClient(testName); SetOptions(); - auto const p = GetParam(); + BlobConcurrentDownloadParameter const& p(GetParam()); auto const blobSize = p.Size; std::vector blobContent(static_cast(8_MB), 'x'); From 4982f1207462fd90e6989344e3f1f68855284d02 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 22 Feb 2022 16:12:23 -0800 Subject: [PATCH 08/29] Compilation issues --- .../azure-storage-blobs/test/ut/block_blob_client_test.cpp | 4 ++-- .../test/ut/datalake_file_client_test.cpp | 4 ++-- .../test/ut/share_file_client_test.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp b/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp index d0250e7ca1..89d5ccce45 100644 --- a/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp +++ b/sdk/storage/azure-storage-blobs/test/ut/block_blob_client_test.cpp @@ -1191,7 +1191,7 @@ namespace Azure { namespace Storage { namespace Test { auto const testName(GetTestName()); auto blockBlobClient = GetBlockBlobClient(testName); SetOptions(); - BlobConcurrentDownloadParameter const& p(GetParam()); + UploadBlockBlob::ParamType const& p(GetParam()); auto const blobSize = p.Size; std::vector blobContent(static_cast(8_MB), 'x'); @@ -1227,7 +1227,7 @@ namespace Azure { namespace Storage { namespace Test { auto const testName(GetTestName()); auto blockBlobClient = GetBlockBlobClient(testName); SetOptions(); - BlobConcurrentDownloadParameter const& p(GetParam()); + UploadBlockBlob::ParamType const& p(GetParam()); auto const blobSize = p.Size; std::vector blobContent(static_cast(8_MB), 'x'); diff --git a/sdk/storage/azure-storage-files-datalake/test/ut/datalake_file_client_test.cpp b/sdk/storage/azure-storage-files-datalake/test/ut/datalake_file_client_test.cpp index b79db998af..edf1723852 100644 --- a/sdk/storage/azure-storage-files-datalake/test/ut/datalake_file_client_test.cpp +++ b/sdk/storage/azure-storage-files-datalake/test/ut/datalake_file_client_test.cpp @@ -429,7 +429,7 @@ namespace Azure { namespace Storage { namespace Test { TEST_P(UploadFile, fromBuffer) { - auto const p = GetParam(); + UploadFile::ParamType const& p(GetParam()); std::vector fileContent(static_cast(8_MB), 'x'); auto fileClient = m_fileSystemClient->GetFileClient(GetTestNameLowerCase()); @@ -461,7 +461,7 @@ namespace Azure { namespace Storage { namespace Test { TEST_P(UploadFile, fromFile) { - auto const p = GetParam(); + UploadFile::ParamType const& p(GetParam()); std::vector fileContent(static_cast(8_MB), 'x'); auto fileClient = m_fileSystemClient->GetFileClient(GetTestNameLowerCase()); diff --git a/sdk/storage/azure-storage-files-shares/test/ut/share_file_client_test.cpp b/sdk/storage/azure-storage-files-shares/test/ut/share_file_client_test.cpp index 7f45637c2e..fed1ae16f0 100644 --- a/sdk/storage/azure-storage-files-shares/test/ut/share_file_client_test.cpp +++ b/sdk/storage/azure-storage-files-shares/test/ut/share_file_client_test.cpp @@ -395,7 +395,7 @@ namespace Azure { namespace Storage { namespace Test { TEST_P(UploadShare, fromBuffer) { - auto const p = GetParam(); + UploadShare::ParamType const& p(GetParam()); auto fileClient = m_fileShareDirectoryClient->GetFileClient(m_testName); std::vector fileContent(static_cast(p.FileSize), 'x'); @@ -418,7 +418,7 @@ namespace Azure { namespace Storage { namespace Test { TEST_P(UploadShare, fromFile) { - auto const p = GetParam(); + UploadShare::ParamType const& p(GetParam()); auto fileClient = m_fileShareDirectoryClient->GetFileClient(m_testName); std::vector fileContent = std::vector(static_cast(p.FileSize), 'x'); From eb8005760bbcf2f62031374589ed703497babc56 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 11:50:07 -0800 Subject: [PATCH 09/29] Pull request feedback --- .../private/attestation_client_private.hpp | 2 +- .../inc/azure/core/test/test_base.hpp | 11 ++--- .../inc/azure/core/internal/strings.hpp | 6 ++- sdk/core/azure-core/src/strings.cpp | 41 +++++++++++++++--- sdk/core/azure-core/test/ut/CMakeLists.txt | 2 +- sdk/core/azure-core/test/ut/string_test.cpp | 42 +++++++++++++++++++ 6 files changed, 87 insertions(+), 17 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index b5a535ba22..91ecec460b 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -428,6 +428,6 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken&() { return m_token; } + operator Models::AttestationToken &() { return m_token; } }; }}}} // namespace Azure::Security::Attestation::_detail diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 8f114a88c6..16f280a1ac 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -251,15 +251,10 @@ namespace Azure { namespace Core { namespace Test { #if !defined(NDEBUG) // The azure CI pipeline uppercases all EnvVar values from ci.yml files. // That means that any mixed case strings will not be found when run from the CI - // pipeline. - // - // Check to ensure that the filename is upper case + // pipeline. Check to make sure that the developer only passed in an upper case environment + // variable. { - std::string ucName = name; - std::transform(ucName.begin(), ucName.end(), ucName.begin(), [](char const& ch) { - return static_cast(std::toupper(ch)); - }); - if (ucName != name) + if (name != Azure::Core::_internal::StringExtensions::ToUpper(name)) { throw std::runtime_error("All Azure SDK environment variables must be all upper case."); } diff --git a/sdk/core/azure-core/inc/azure/core/internal/strings.hpp b/sdk/core/azure-core/inc/azure/core/internal/strings.hpp index d6e68d2e17..0858c230e4 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/strings.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/strings.hpp @@ -33,8 +33,10 @@ namespace Azure { namespace Core { namespace _internal { static bool LocaleInvariantCaseInsensitiveEqual( const std::string& lhs, const std::string& rhs) noexcept; - static std::string const ToLower(const std::string& src) noexcept; - static unsigned char ToLower(const unsigned char src) noexcept; + static std::string const ToLower(std::string const& src) noexcept; + static unsigned char ToLower(unsigned char const src) noexcept; + static std::string const ToUpper(std::string const& src) noexcept; + static unsigned char ToUpper(unsigned char const src) noexcept; }; }}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/src/strings.cpp b/sdk/core/azure-core/src/strings.cpp index 4047bc5a95..702ebd1b68 100644 --- a/sdk/core/azure-core/src/strings.cpp +++ b/sdk/core/azure-core/src/strings.cpp @@ -65,11 +65,29 @@ const unsigned char LocaleInvariantLowercaseTable[256] = { 0xE0, 0xE1, 0xE2, 0xE3, 0xE4, 0xE5, 0xE6, 0xE7, 0xE8, 0xE9, 0xEA, 0xEB, 0xEC, 0xED, 0xEE, 0xEF, 0xF0, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, }; +const unsigned char LocaleInvariantUppercaseTable[256] = { + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, + 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x3A, 0x3B, 0x3C, 0x3D, 0x3E, 0x3F, + 0x40, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x4E, 0x4F, + 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5A, 0x5B, 0x5C, 0x5D, 0x5E, 0x5F, + 0x60, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x4E, 0x4F, + 0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, 0x59, 0x5A, 0x7B, 0x7C, 0x7D, 0x7E, 0x7F, + 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8A, 0x8B, 0x8C, 0x8D, 0x8E, 0x8F, + 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, 0x98, 0x99, 0x9A, 0x9B, 0x9C, 0x9D, 0x9E, 0x9F, + 0xA0, 0xA1, 0xA2, 0xA3, 0xA4, 0xA5, 0xA6, 0xA7, 0xA8, 0xA9, 0xAA, 0xAB, 0xAC, 0xAD, 0xAE, 0xAF, + 0xB0, 0xB1, 0xB2, 0xB3, 0xB4, 0xB5, 0xB6, 0xB7, 0xB8, 0xB9, 0xBA, 0xBB, 0xBC, 0xBD, 0xBE, 0xBF, + 0xC0, 0xC1, 0xC2, 0xC3, 0xC4, 0xC5, 0xC6, 0xC7, 0xC8, 0xC9, 0xCA, 0xCB, 0xCC, 0xCD, 0xCE, 0xCF, + 0xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5, 0xD6, 0xD7, 0xD8, 0xD9, 0xDA, 0xDB, 0xDC, 0xDD, 0xDE, 0xDF, + 0xE0, 0xE1, 0xE2, 0xE3, 0xE4, 0xE5, 0xE6, 0xE7, 0xE8, 0xE9, 0xEA, 0xEB, 0xEC, 0xED, 0xEE, 0xEF, + 0xF0, 0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, +}; } // unnamed namespace namespace Azure { namespace Core { namespace _internal { - unsigned char StringExtensions::ToLower(const unsigned char symbol) noexcept + unsigned char StringExtensions::ToLower(unsigned char const symbol) noexcept { return LocaleInvariantLowercaseTable[symbol]; } @@ -77,10 +95,23 @@ namespace Azure { namespace Core { namespace _internal { std::string const StringExtensions::ToLower(const std::string& src) noexcept { auto result = std::string(src); - for (auto i = result.begin(); i < result.end(); i++) - { - *i = ToLower(static_cast(*i)); - } + std::transform(result.begin(), result.end(), result.begin(), [](char const ch) { + return StringExtensions::ToLower(ch); + }); + return result; + } + + unsigned char StringExtensions::ToUpper(unsigned char const symbol) noexcept + { + return LocaleInvariantUppercaseTable[symbol]; + } + + std::string const StringExtensions::ToUpper(const std::string& src) noexcept + { + auto result = std::string(src); + std::transform(result.begin(), result.end(), result.begin(), [](char const ch) { + return StringExtensions::ToUpper(ch); + }); return result; } diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index 95a5ef8ed8..27dcaed5a6 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -77,7 +77,7 @@ add_executable ( url_test.cpp uuid_test.cpp exception_test.cpp -) + ) if (MSVC) # Disable warnings: diff --git a/sdk/core/azure-core/test/ut/string_test.cpp b/sdk/core/azure-core/test/ut/string_test.cpp index dc0c9c4a48..91d277ff74 100644 --- a/sdk/core/azure-core/test/ut/string_test.cpp +++ b/sdk/core/azure-core/test/ut/string_test.cpp @@ -20,6 +20,24 @@ TEST(String, invariantCompare) EXPECT_FALSE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("ABC", "abcd")); } +TEST(String, toLowerC) +{ + using Azure::Core::_internal::StringExtensions; + for (unsigned char ch = 0; ch < 255; ch += 1) + { + EXPECT_TRUE(StringExtensions::ToLower(ch) == std::tolower(ch)); + } +} + +TEST(String, toUpperC) +{ + using Azure::Core::_internal::StringExtensions; + for (unsigned char ch = 0; ch < 255; ch += 1) + { + EXPECT_TRUE(StringExtensions::ToUpper(ch) == std::toupper(ch)); + } +} + TEST(String, toLower) { using Azure::Core::_internal::StringExtensions; @@ -29,9 +47,33 @@ TEST(String, toLower) EXPECT_TRUE(StringExtensions::ToLower("AA") == "aa"); EXPECT_TRUE(StringExtensions::ToLower("aA") == "aa"); EXPECT_TRUE(StringExtensions::ToLower("ABC") == "abc"); + EXPECT_TRUE( + StringExtensions::ToLower("abcdefghijklmnopqrstuvwxyz") == "abcdefghijklmnopqrstuvwxyz"); + EXPECT_TRUE( + StringExtensions::ToLower("ABCDEFGHIJKLMNOPQRSTUVWXYZ") == "abcdefghijklmnopqrstuvwxyz"); EXPECT_TRUE(StringExtensions::ToLower("ABC-1-,!@#$%^&*()_+=ABC") == "abc-1-,!@#$%^&*()_+=abc"); EXPECT_FALSE(StringExtensions::ToLower("") == "a"); EXPECT_FALSE(StringExtensions::ToLower("a") == ""); EXPECT_FALSE(StringExtensions::ToLower("a") == "aA"); EXPECT_FALSE(StringExtensions::ToLower("abc") == "abcd"); } + +TEST(String, toUpper) +{ + using Azure::Core::_internal::StringExtensions; + EXPECT_TRUE(StringExtensions::ToUpper("") == ""); + EXPECT_TRUE(StringExtensions::ToUpper("a") == "A"); + EXPECT_TRUE(StringExtensions::ToUpper("A") == "A"); + EXPECT_TRUE(StringExtensions::ToUpper("AA") == "AA"); + EXPECT_TRUE(StringExtensions::ToUpper("aA") == "AA"); + EXPECT_TRUE( + StringExtensions::ToUpper("ABCDEFGHIJKLMNOPQRSTUVWXYZ") == "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + EXPECT_TRUE(StringExtensions::ToUpper("ABC") == "ABC"); + EXPECT_TRUE( + StringExtensions::ToUpper("ABCDEFGHIJKLMNOPQRSTUVWXYZ") == "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + EXPECT_TRUE(StringExtensions::ToUpper("ABC-1-,!@#$%^&*()_+=ABC") == "ABC-1-,!@#$%^&*()_+=ABC"); + EXPECT_FALSE(StringExtensions::ToUpper("") == "A"); + EXPECT_FALSE(StringExtensions::ToUpper("a") == ""); + EXPECT_FALSE(StringExtensions::ToUpper("a") == "aA"); + EXPECT_FALSE(StringExtensions::ToUpper("abc") == "abcd"); +} From cc7f426ef7bd9958a6513d01d9647ced3f57eb54 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 12:05:15 -0800 Subject: [PATCH 10/29] Renamed serviceName parameter to match the parameter to new-testresources.ps1 --- .../inc/azure/core/test/test_base.hpp | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 16f280a1ac..7477bfcf31 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -274,11 +274,22 @@ namespace Azure { namespace Core { namespace Test { /** * @brief Run before each test. * + * @param serviceDirectory - the Service Directory as provided to the new-testresources.ps1 + * script. + * @param baseRecordingPath - the base recording path to be used for this test. Normally this is + * `AZURE_TEST_RECORDING_DIR`. + * + * For example: + * + * \code{.cpp} + * Azure::Core::Test::TestBase::SetUpTestBase("STORAGE", AZURE_TEST_RECORDING_DIR); + * \endcode + * */ - void SetUpTestBase(std::string const& serviceName, std::string const& baseRecordingPath) + void SetUpTestBase(std::string const& serviceDirectory, std::string const& baseRecordingPath) { - m_serviceName = serviceName; + m_serviceName = serviceDirectory; // Init interceptor from PlayBackRecorder std::string recordingPath(baseRecordingPath); @@ -306,9 +317,9 @@ namespace Azure { namespace Core { namespace Test { } }; ; - SetBuiltinEnvironment(serviceName, "_TENANT_ID"); - SetBuiltinEnvironment(serviceName, "_CLIENT_ID"); - SetBuiltinEnvironment(serviceName, "_CLIENT_SECRET"); + SetBuiltinEnvironment(serviceDirectory, "_TENANT_ID"); + SetBuiltinEnvironment(serviceDirectory, "_CLIENT_ID"); + SetBuiltinEnvironment(serviceDirectory, "_CLIENT_SECRET"); } } From 304f92e364c07efe66ff9334877ff7334d80d2fe Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 14:06:16 -0800 Subject: [PATCH 11/29] Don't set environment variables if they're already set --- .../inc/azure/core/test/test_base.hpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 7477bfcf31..8f7338575a 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -308,12 +308,16 @@ namespace Azure { namespace Core { namespace Test { { auto SetBuiltinEnvironment = [](std::string const& serviceName, std::string const& targetVariable) { - std::string targetValue = Azure::Core::_internal::Environment::GetVariable( - (serviceName + targetVariable).c_str()); - if (!targetValue.empty()) + std::string azureVariable = "AZURE" + targetVariable; + if (Azure::Core::_internal::Environment::GetVariable(azureVariable.c_str()).empty()) { - Azure::Core::_internal::Environment::SetVariable( - ("AZURE" + targetVariable).c_str(), targetValue.c_str()); + std::string targetValue = Azure::Core::_internal::Environment::GetVariable( + (serviceName + targetVariable).c_str()); + if (!targetValue.empty()) + { + Azure::Core::_internal::Environment::SetVariable( + azureVariable.c_str(), targetValue.c_str()); + } } }; ; From d51d09c58462181948142faf9bcfcb1a41f41ebc Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 15:21:42 -0800 Subject: [PATCH 12/29] clang-format --- .../src/private/attestation_client_private.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index 91ecec460b..b5a535ba22 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -428,6 +428,6 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken &() { return m_token; } + operator Models::AttestationToken&() { return m_token; } }; }}}} // namespace Azure::Security::Attestation::_detail From 9fe931ee79a6a1d812653b2342cf0303f384e296 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 15:42:19 -0800 Subject: [PATCH 13/29] Set serviceDirectory from environment variables --- .../test/ut/administration_test.cpp | 2 +- .../test/ut/attestation_test.cpp | 2 +- sdk/attestation/ci.yml | 2 ++ .../inc/azure/core/test/test_base.hpp | 20 ++++++++++--------- .../test/ut/token_credential_test.cpp | 2 +- sdk/identity/ci.yml | 2 ++ .../test/ut/certificate_client_base_test.hpp | 2 +- .../test/ut/key_client_base_test.hpp | 2 +- .../test/ut/secret_client_base_test.hpp | 2 +- sdk/keyvault/ci.yml | 2 ++ .../test/ut/test_base.hpp | 2 +- sdk/storage/ci.yml | 2 ++ sdk/template/ci.yml | 6 ++++++ 13 files changed, 32 insertions(+), 16 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp index d62a0c3487..aebb8883db 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp @@ -25,7 +25,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { // Create virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase("ATTESTATION", AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); std::string const mode(std::get<0>(GetParam())); if (mode == "Shared") diff --git a/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp index 6f9bcceb67..1ab8473056 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/attestation_test.cpp @@ -22,7 +22,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { // Create virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase("ATTESTATION", AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); std::string mode(GetParam()); if (mode == "Shared") diff --git a/sdk/attestation/ci.yml b/sdk/attestation/ci.yml index 18a1fb3bbe..663c06d1e4 100644 --- a/sdk/attestation/ci.yml +++ b/sdk/attestation/ci.yml @@ -52,3 +52,5 @@ stages: # NOTE: The LOCATION_SHORT_NAME *must* match the region in which the tests were created. - Name: LOCATION_SHORT_NAME Value: "wus" + - Name: AZURE_SERVICE_DIRECTORY + Value: ATTESTATION diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 8f7338575a..2bed09a975 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -274,8 +274,6 @@ namespace Azure { namespace Core { namespace Test { /** * @brief Run before each test. * - * @param serviceDirectory - the Service Directory as provided to the new-testresources.ps1 - * script. * @param baseRecordingPath - the base recording path to be used for this test. Normally this is * `AZURE_TEST_RECORDING_DIR`. * @@ -286,11 +284,9 @@ namespace Azure { namespace Core { namespace Test { * \endcode * */ - void SetUpTestBase(std::string const& serviceDirectory, std::string const& baseRecordingPath) + void SetUpTestBase(std::string const& baseRecordingPath) { - m_serviceName = serviceDirectory; - // Init interceptor from PlayBackRecorder std::string recordingPath(baseRecordingPath); recordingPath.append("/recordings"); @@ -307,10 +303,16 @@ namespace Azure { namespace Core { namespace Test { if (!m_testContext.IsPlaybackMode()) { auto SetBuiltinEnvironment - = [](std::string const& serviceName, std::string const& targetVariable) { + = [](std::string const& targetVariable) { std::string azureVariable = "AZURE" + targetVariable; if (Azure::Core::_internal::Environment::GetVariable(azureVariable.c_str()).empty()) { + std::string serviceName = Azure::Core::_internal::Environment::GetVariable("AZURE_SERVICE_DIRECTORY"); + if (serviceName.empty()) + { + throw std::runtime_error("Could not find a value for AZURE_SERVICE_DIRECTORY. Check ci.yml " + "to confirm that it has been configured."); + } std::string targetValue = Azure::Core::_internal::Environment::GetVariable( (serviceName + targetVariable).c_str()); if (!targetValue.empty()) @@ -321,9 +323,9 @@ namespace Azure { namespace Core { namespace Test { } }; ; - SetBuiltinEnvironment(serviceDirectory, "_TENANT_ID"); - SetBuiltinEnvironment(serviceDirectory, "_CLIENT_ID"); - SetBuiltinEnvironment(serviceDirectory, "_CLIENT_SECRET"); + SetBuiltinEnvironment("_TENANT_ID"); + SetBuiltinEnvironment("_CLIENT_ID"); + SetBuiltinEnvironment("_CLIENT_SECRET"); } } diff --git a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp index 9f8ac866cc..1ad894727d 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp @@ -45,7 +45,7 @@ namespace Azure { namespace Identity { namespace Test { // Runs before every test. virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase("IDENTITY", AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); } }; }}} // namespace Azure::Identity::Test diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index e9285dab88..a0da0721fa 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -48,6 +48,8 @@ stages: Value: "non-real-client" - Name: AZURE_CLIENT_SECRET Value: "non-real-secret" + - Name: AZURE_SERVICE_DIRECTORY + Value: IDENTITY CMakeTestOptions: - Name: Default Value: '' diff --git a/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp b/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp index b40f5f00b6..dde88453e2 100644 --- a/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp +++ b/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_base_test.hpp @@ -65,7 +65,7 @@ namespace Azure { // Runs before every test. virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase("KEYVAULT", AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); m_keyVaultUrl = GetEnv("AZURE_KEYVAULT_URL"); // Options and credential for the client diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp index 245bfa71e8..d7553a6f2f 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_base_test.hpp @@ -54,7 +54,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys { nam // Create virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase("KEYVAULT", AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); m_keyVaultUrl = GetEnv("AZURE_KEYVAULT_URL"); m_keyVaultHsmUrl = GetEnv("AZURE_KEYVAULT_HSM_URL"); diff --git a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp index e933a52039..a37bf6ad3b 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp +++ b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_base_test.hpp @@ -41,7 +41,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets { // Create void InitializeClient() { - Azure::Core::Test::TestBase::SetUpTestBase("KEYVAULT", AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); m_keyVaultUrl = GetEnv("AZURE_KEYVAULT_URL"); // Options and credential for the client diff --git a/sdk/keyvault/ci.yml b/sdk/keyvault/ci.yml index 82c9f8e6db..12b3e90804 100644 --- a/sdk/keyvault/ci.yml +++ b/sdk/keyvault/ci.yml @@ -66,6 +66,8 @@ stages: Value: "non-real-client" - Name: AZURE_CLIENT_SECRET Value: "non-real-secret" + - Name: AZURE_SERVICE_DIRECTORY + Value: KEYVAULT CMakeTestOptions: - Name: Default Value: '' diff --git a/sdk/storage/azure-storage-common/test/ut/test_base.hpp b/sdk/storage/azure-storage-common/test/ut/test_base.hpp index fa71854e1b..d75ba434af 100644 --- a/sdk/storage/azure-storage-common/test/ut/test_base.hpp +++ b/sdk/storage/azure-storage-common/test/ut/test_base.hpp @@ -132,7 +132,7 @@ namespace Azure { namespace Storage { virtual void SetUp() override { - Azure::Core::Test::TestBase::SetUpTestBase("STORAGE", AZURE_TEST_RECORDING_DIR); + Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); } public: diff --git a/sdk/storage/ci.yml b/sdk/storage/ci.yml index 4238300c08..667edcf536 100644 --- a/sdk/storage/ci.yml +++ b/sdk/storage/ci.yml @@ -60,3 +60,5 @@ stages: Value: "DefaultEndpointsProtocol=https;AccountName=notReal;AccountKey=3333333333333333333333333333333333333333333333333333333333333333333333333333333333333333;EndpointSuffix=core.windows.net" - Name: PREMIUM_FILE_CONNECTION_STRING Value: "DefaultEndpointsProtocol=https;AccountName=notReal;AccountKey=3333333333333333333333333333333333333333333333333333333333333333333333333333333333333333;EndpointSuffix=core.windows.net" + - Name: AZURE_SERVICE_DIRECTORY + Value: STORAGE diff --git a/sdk/template/ci.yml b/sdk/template/ci.yml index fece1b1b9e..2614db7a4d 100644 --- a/sdk/template/ci.yml +++ b/sdk/template/ci.yml @@ -31,3 +31,9 @@ stages: - Name: azure-template Path: azure-template VcpkgPortName: azure-template-cpp + TestEnv: + # NOTE: The CI pipeline uppercases all "Name" values in the TestEnv, so if you + # add a mixed case name, be careful to ensure it is read using an upper case + # name. + - Name: AZURE_SERVICE_DIRECTORY + Value: TEMPLATE From 565f60cf3ddcaa5b528c51ba704bc46aae7f1536 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 16:10:52 -0800 Subject: [PATCH 14/29] @#$@# clang-format --- .../inc/azure/core/test/test_base.hpp | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 2bed09a975..6a566418d5 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -302,27 +302,27 @@ namespace Azure { namespace Core { namespace Test { if (!m_testContext.IsPlaybackMode()) { - auto SetBuiltinEnvironment - = [](std::string const& targetVariable) { - std::string azureVariable = "AZURE" + targetVariable; - if (Azure::Core::_internal::Environment::GetVariable(azureVariable.c_str()).empty()) - { - std::string serviceName = Azure::Core::_internal::Environment::GetVariable("AZURE_SERVICE_DIRECTORY"); - if (serviceName.empty()) - { - throw std::runtime_error("Could not find a value for AZURE_SERVICE_DIRECTORY. Check ci.yml " - "to confirm that it has been configured."); - } - std::string targetValue = Azure::Core::_internal::Environment::GetVariable( - (serviceName + targetVariable).c_str()); - if (!targetValue.empty()) - { - Azure::Core::_internal::Environment::SetVariable( - azureVariable.c_str(), targetValue.c_str()); - } - } - }; - ; + auto SetBuiltinEnvironment = [](std::string const& targetVariable) { + std::string azureVariable = "AZURE" + targetVariable; + if (Azure::Core::_internal::Environment::GetVariable(azureVariable.c_str()).empty()) + { + std::string serviceName + = Azure::Core::_internal::Environment::GetVariable("AZURE_SERVICE_DIRECTORY"); + if (serviceName.empty()) + { + throw std::runtime_error( + "Could not find a value for AZURE_SERVICE_DIRECTORY. Check ci.yml " + "to confirm that it has been configured."); + } + std::string targetValue = Azure::Core::_internal::Environment::GetVariable( + (serviceName + targetVariable).c_str()); + if (!targetValue.empty()) + { + Azure::Core::_internal::Environment::SetVariable( + azureVariable.c_str(), targetValue.c_str()); + } + } + }; SetBuiltinEnvironment("_TENANT_ID"); SetBuiltinEnvironment("_CLIENT_ID"); SetBuiltinEnvironment("_CLIENT_SECRET"); From fb5a4b3b89d1bdd5a5c32f85436ed2cd887c8b2c Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 16:33:20 -0800 Subject: [PATCH 15/29] backed out inadvertant change to cmakelists.txt in azure-core\test --- sdk/core/azure-core/test/ut/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index 27dcaed5a6..95a5ef8ed8 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -77,7 +77,7 @@ add_executable ( url_test.cpp uuid_test.cpp exception_test.cpp - ) +) if (MSVC) # Disable warnings: From 07c1f22b85936df1cd4193066e2ab5fbc2b5c2d0 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 16:40:37 -0800 Subject: [PATCH 16/29] Further updates to reduce code churn --- .../azure-core-test/inc/azure/core/test/test_base.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 6a566418d5..484f5a973a 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -17,7 +17,6 @@ #include "azure/core/test/network_models.hpp" #include "azure/core/test/test_context_manager.hpp" -#include #include #include #include @@ -46,7 +45,6 @@ namespace Azure { namespace Core { namespace Test { * */ bool m_wasSkipped = false; - std::string m_serviceName{}; void PrepareOptions(Azure::Core::_internal::ClientOptions& options) { @@ -260,7 +258,7 @@ namespace Azure { namespace Core { namespace Test { } } #endif - auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str()); + const auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str()); if (ret.empty()) { throw std::runtime_error("Missing required environment variable: " + name); @@ -280,9 +278,13 @@ namespace Azure { namespace Core { namespace Test { * For example: * * \code{.cpp} - * Azure::Core::Test::TestBase::SetUpTestBase("STORAGE", AZURE_TEST_RECORDING_DIR); + * Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); * \endcode * + * @note If AZURE_TENANT_ID, AZURE_CLIENT_ID, or AZURE_CLIENT_SECRET are not available in the + * environment, the AZURE_SERVICE_DIRECTORY environment variable is used to set those values + * with the values emitted by the New-TestResources.ps1 script. + * */ void SetUpTestBase(std::string const& baseRecordingPath) { From 303dd5edfe9e4a63186deec58918489dd099ee87 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 18:46:59 -0800 Subject: [PATCH 17/29] Added AZURE_SERVICE_DIRECTORY to CI test invocation --- eng/pipelines/templates/jobs/archetype-sdk-tests.yml | 3 +++ sdk/attestation/ci.yml | 2 -- sdk/identity/ci.yml | 2 -- sdk/keyvault/ci.yml | 2 -- sdk/storage/ci.yml | 2 -- sdk/template/ci.yml | 6 ------ 6 files changed, 3 insertions(+), 14 deletions(-) diff --git a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml index 1cb5993a81..ebfc5e6bb3 100644 --- a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml @@ -218,6 +218,9 @@ jobs: # This enables to run tests and samples at the same time as different matrix configuration. # Then unit-tests runs, samples should not run. condition: and(succeeded(), ne(variables['RunSamples'], '1')) + env: + AZURE_SERVICE_DIRECTORY: {{ parameters.ServiceDirectory }} + - task: PublishTestResults@2 inputs: diff --git a/sdk/attestation/ci.yml b/sdk/attestation/ci.yml index 663c06d1e4..18a1fb3bbe 100644 --- a/sdk/attestation/ci.yml +++ b/sdk/attestation/ci.yml @@ -52,5 +52,3 @@ stages: # NOTE: The LOCATION_SHORT_NAME *must* match the region in which the tests were created. - Name: LOCATION_SHORT_NAME Value: "wus" - - Name: AZURE_SERVICE_DIRECTORY - Value: ATTESTATION diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index a0da0721fa..e9285dab88 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -48,8 +48,6 @@ stages: Value: "non-real-client" - Name: AZURE_CLIENT_SECRET Value: "non-real-secret" - - Name: AZURE_SERVICE_DIRECTORY - Value: IDENTITY CMakeTestOptions: - Name: Default Value: '' diff --git a/sdk/keyvault/ci.yml b/sdk/keyvault/ci.yml index 12b3e90804..82c9f8e6db 100644 --- a/sdk/keyvault/ci.yml +++ b/sdk/keyvault/ci.yml @@ -66,8 +66,6 @@ stages: Value: "non-real-client" - Name: AZURE_CLIENT_SECRET Value: "non-real-secret" - - Name: AZURE_SERVICE_DIRECTORY - Value: KEYVAULT CMakeTestOptions: - Name: Default Value: '' diff --git a/sdk/storage/ci.yml b/sdk/storage/ci.yml index 667edcf536..4238300c08 100644 --- a/sdk/storage/ci.yml +++ b/sdk/storage/ci.yml @@ -60,5 +60,3 @@ stages: Value: "DefaultEndpointsProtocol=https;AccountName=notReal;AccountKey=3333333333333333333333333333333333333333333333333333333333333333333333333333333333333333;EndpointSuffix=core.windows.net" - Name: PREMIUM_FILE_CONNECTION_STRING Value: "DefaultEndpointsProtocol=https;AccountName=notReal;AccountKey=3333333333333333333333333333333333333333333333333333333333333333333333333333333333333333;EndpointSuffix=core.windows.net" - - Name: AZURE_SERVICE_DIRECTORY - Value: STORAGE diff --git a/sdk/template/ci.yml b/sdk/template/ci.yml index 2614db7a4d..fece1b1b9e 100644 --- a/sdk/template/ci.yml +++ b/sdk/template/ci.yml @@ -31,9 +31,3 @@ stages: - Name: azure-template Path: azure-template VcpkgPortName: azure-template-cpp - TestEnv: - # NOTE: The CI pipeline uppercases all "Name" values in the TestEnv, so if you - # add a mixed case name, be careful to ensure it is read using an upper case - # name. - - Name: AZURE_SERVICE_DIRECTORY - Value: TEMPLATE From 119e2b3b93892f193d62c58e8d9280c2b1c1008e Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 22:06:10 -0800 Subject: [PATCH 18/29] Fixed missing $ --- eng/pipelines/templates/jobs/archetype-sdk-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml index ebfc5e6bb3..3ef5065af8 100644 --- a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml @@ -219,7 +219,7 @@ jobs: # Then unit-tests runs, samples should not run. condition: and(succeeded(), ne(variables['RunSamples'], '1')) env: - AZURE_SERVICE_DIRECTORY: {{ parameters.ServiceDirectory }} + AZURE_SERVICE_DIRECTORY: ${{ parameters.ServiceDirectory }} - task: PublishTestResults@2 From 62916f0c64eab2faaa0b5bea4848474e4f29a021 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 23 Feb 2022 22:26:33 -0800 Subject: [PATCH 19/29] Dump environment variables during tests --- .../test/ut/administration_test.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp index aebb8883db..f87471e347 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp @@ -5,6 +5,7 @@ #include "azure/identity/client_secret_credential.hpp" #include #include +#include #include #include #include @@ -53,6 +54,15 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { options.TokenValidationOptions.ValidateNotBeforeTime = false; options.TokenValidationOptions.ValidateExpirationTime = false; } + char** env = environ; + Azure::Core::Test::TestBase::TestLog("Dumping Environment variables"); + while (*env != nullptr) + { + std::stringstream ss; + ss << "Env Variable " << *env << std::endl; + Azure::Core::Test::TestBase::TestLog(ss.str()); + env += 1; + } std::shared_ptr credential = std::make_shared( GetEnv("AZURE_TENANT_ID"), GetEnv("AZURE_CLIENT_ID"), GetEnv("AZURE_CLIENT_SECRET")); From 31dbac05155be950b98eedd1961fd278e00963e7 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 09:25:14 -0800 Subject: [PATCH 20/29] Upper case serviceDirectory; clarified exception message to make it more diagnostic --- .../test/ut/administration_test.cpp | 9 --------- .../azure-core-test/inc/azure/core/test/test_base.hpp | 8 ++++++-- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp index f87471e347..4874faea70 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/administration_test.cpp @@ -54,15 +54,6 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { options.TokenValidationOptions.ValidateNotBeforeTime = false; options.TokenValidationOptions.ValidateExpirationTime = false; } - char** env = environ; - Azure::Core::Test::TestBase::TestLog("Dumping Environment variables"); - while (*env != nullptr) - { - std::stringstream ss; - ss << "Env Variable " << *env << std::endl; - Azure::Core::Test::TestBase::TestLog(ss.str()); - env += 1; - } std::shared_ptr credential = std::make_shared( GetEnv("AZURE_TENANT_ID"), GetEnv("AZURE_CLIENT_ID"), GetEnv("AZURE_CLIENT_SECRET")); diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 484f5a973a..dd61395c79 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -313,9 +313,13 @@ namespace Azure { namespace Core { namespace Test { if (serviceName.empty()) { throw std::runtime_error( - "Could not find a value for AZURE_SERVICE_DIRECTORY. Check ci.yml " - "to confirm that it has been configured."); + "Could not find a value for AZURE_" + targetVariable + + " and AZURE_SERVICE_DIRECTORY was not defined. Define either AZURE_" + + targetVariable + " or AZURE_SERVICE_DIRECTORY to resolve."); } + // Upper case the serviceName environment variable because all ci.yml environment + // variables are upper cased. + serviceName = Azure::Core::_internal::StringExtensions::ToUpper(serviceName); std::string targetValue = Azure::Core::_internal::Environment::GetVariable( (serviceName + targetVariable).c_str()); if (!targetValue.empty()) From 4689bb2d78b58aa4729426b36a2ef545fd5cd2a1 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 09:55:44 -0800 Subject: [PATCH 21/29] Moved AZURE_SERVICE_DIRECTORY definition from ctest step to variables --- eng/pipelines/templates/jobs/archetype-sdk-tests.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml index 3ef5065af8..5e6e3fcb21 100644 --- a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml @@ -149,6 +149,7 @@ jobs: CmakeArgs: "" AZURE_TEST_MODE: "LIVE" AZURE_LOG_LEVEL: "verbose" + AZURE_SERVICE_DIRECTORY: ${{ parameters.ServiceDirectory }} steps: - checkout: self @@ -218,8 +219,6 @@ jobs: # This enables to run tests and samples at the same time as different matrix configuration. # Then unit-tests runs, samples should not run. condition: and(succeeded(), ne(variables['RunSamples'], '1')) - env: - AZURE_SERVICE_DIRECTORY: ${{ parameters.ServiceDirectory }} - task: PublishTestResults@2 From 8c79c6e9b03d6ba0cb248a5e452c89ffbb2f3fdc Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 10:47:04 -0800 Subject: [PATCH 22/29] Fixed thumbprint generation to use correct buffer - fixes rare crash in CI pipeline --- .../src/private/crypto/openssl/opensslcert.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/crypto/openssl/opensslcert.hpp b/sdk/attestation/azure-security-attestation/src/private/crypto/openssl/opensslcert.hpp index 5b678d1d94..5e41033b26 100644 --- a/sdk/attestation/azure-security-attestation/src/private/crypto/openssl/opensslcert.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/crypto/openssl/opensslcert.hpp @@ -140,7 +140,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail { throw OpenSSLException("i2d_X509"); } - if (EVP_DigestUpdate(hash.get(), buf, thumbprintBuffer.size()) != 1) + if (EVP_DigestUpdate(hash.get(), thumbprintBuffer.data(), thumbprintBuffer.size()) != 1) { throw OpenSSLException("EVP_DigestUpdate"); } From 03eedca84e576cf7afcede018d852dbef970cfa1 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 10:49:21 -0800 Subject: [PATCH 23/29] Explain purpose of AZURE_SERVICE_DIRECTORY environment variable in archetype-sdk-tests.yml --- eng/pipelines/templates/jobs/archetype-sdk-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml index 5e6e3fcb21..6f845729ae 100644 --- a/eng/pipelines/templates/jobs/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/jobs/archetype-sdk-tests.yml @@ -149,6 +149,7 @@ jobs: CmakeArgs: "" AZURE_TEST_MODE: "LIVE" AZURE_LOG_LEVEL: "verbose" + # Surface the ServiceDirectory parameter as an environment variable so tests can take advantage of it. AZURE_SERVICE_DIRECTORY: ${{ parameters.ServiceDirectory }} steps: From 731fb3c6af80f96470c8877d33d549350574e510 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 11:58:00 -0800 Subject: [PATCH 24/29] Small readme update --- .../azure-security-attestation/README.md | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/README.md b/sdk/attestation/azure-security-attestation/README.md index aefdb34132..87f3229181 100644 --- a/sdk/attestation/azure-security-attestation/README.md +++ b/sdk/attestation/azure-security-attestation/README.md @@ -207,16 +207,38 @@ clients to add, remove or enumerate the policy management certificates. The `AttestationClientBuilder` class is used to create instances of the attestation client: ```cpp readme-sample-create-synchronous-client + std::string endpoint = std::getenv("ATTESTATION_AAD_URL"); + AttestationClientOptions options; + return std::make_unique(m_endpoint, options); ``` +If the attestation APIs require authentication, use the following: + +```cpp readme-sample-create-synchronous-client +std::string endpoint = std::getenv("ATTESTATION_AAD_URL"); +AttestationClientOptions options; + std::shared_ptr credential + = std::make_shared( + GetEnv("AZURE_TENANT_ID"), GetEnv("AZURE_CLIENT_ID"), GetEnv("AZURE_CLIENT_SECRET")); +return std::make_unique(m_endpoint, credential, options); +``` + +The same pattern is used to create an `Azure::Security::Attestation::AttestationAdministrationClient`. + #### Retrieve Token Certificates -Use `listAttestationSigners` to retrieve the set of certificates, which can be used to validate the token returned from the attestation service. +Use `GetAttestationSigningCertificates` to retrieve the set of certificates, which can be used to validate the token returned from the attestation service. Normally, this information is not required as the attestation SDK will perform the validation as a part of the interaction with the attestation service, however the APIs are provided for completeness and to facilitate customer's independently validating attestation results. ```cpp readme-sample-getSigningCertificates +auto attestationSigners = attestationClient->GetAttestationSigningCertificates(); +// Enumerate the signers. +for (const auto& signer : attestationSigners.Value.Signers) +{ +} + ``` #### Attest an SGX Enclave @@ -230,22 +252,22 @@ Use the `AttestSgxEnclave` method to attest an SGX enclave. All administrative clients are authenticated. -```cpp readme-sample-create-admin-client -AttestationAdministrationClientBuilder attestationBuilder = new AttestationAdministrationClientBuilder(); -// Note that the "policy" calls require authentication. -AttestationAdministrationClient client = attestationBuilder - .endpoint(endpoint) - .credential(new DefaultAzureCredentialBuilder().build()) - .buildClient(); +```cpp readme-sample-create-synchronous-client +std::string endpoint = std::getenv("ATTESTATION_AAD_URL"); +AttestationClientOptions options; + std::shared_ptr credential + = std::make_shared( + GetEnv("AZURE_TENANT_ID"), GetEnv("AZURE_CLIENT_ID"), GetEnv("AZURE_CLIENT_SECRET")); +auto adminClient = std::make_unique(m_endpoint, credential, options); ``` #### Retrieve current attestation policy for OpenEnclave Use the `GetAttestationPolicy` API to retrieve the current attestation policy for a given TEE. -```java readme-sample-getCurrentPolicy -String currentPolicy = client.getAttestationPolicy(AttestationType.OPEN_ENCLAVE); -System.out.printf("Current policy for OpenEnclave is: %s\n", currentPolicy); +```cpp readme-sample-getCurrentPolicy +auto currentPolicy = adminClient->GetAttestationPolicy(AttestationType.OPEN_ENCLAVE); +std::cout << "Current policy for OpenEnclave is " << currentPolicy.Value.Body << std::endl; ``` #### Set unsigned attestation policy (AAD clients only) From 514f25d9f5a554d2b575d2c5e16209f38c6bf0d8 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 11:58:24 -0800 Subject: [PATCH 25/29] Move AZURE_SERVICE_DIRECTORY check to getenv --- .../inc/azure/core/test/test_base.hpp | 58 ++++++++----------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index dd61395c79..6a300037a2 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -258,9 +258,30 @@ namespace Azure { namespace Core { namespace Test { } } #endif - const auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str()); + auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str()); if (ret.empty()) { + if (!m_testContext.IsPlaybackMode() && name.find("AZURE_") == 0) + { + std::string serviceDirectory + = Azure::Core::_internal::Environment::GetVariable("AZURE_SERVICE_DIRECTORY"); + if (serviceDirectory.empty()) + { + throw std::runtime_error( + "Could not find a value for " + name + + " and AZURE_SERVICE_DIRECTORY was not defined. Define either " + + name + " or AZURE_SERVICE_DIRECTORY to resolve."); + } + // Upper case the serviceName environment variable because all ci.yml environment + // variables are upper cased. + std::string serviceDirectoryEnvVar = Azure::Core::_internal::StringExtensions::ToUpper(serviceDirectory); + serviceDirectoryEnvVar += name.substr(sizeof("AZURE") - 1); + ret = Azure::Core::_internal::Environment::GetVariable(serviceDirectoryEnvVar.c_str()); + if (!ret.empty()) + { + return ret; + } + } throw std::runtime_error("Missing required environment variable: " + name); } @@ -278,7 +299,7 @@ namespace Azure { namespace Core { namespace Test { * For example: * * \code{.cpp} - * Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); + * Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); * \endcode * * @note If AZURE_TENANT_ID, AZURE_CLIENT_ID, or AZURE_CLIENT_SECRET are not available in the @@ -288,7 +309,6 @@ namespace Azure { namespace Core { namespace Test { */ void SetUpTestBase(std::string const& baseRecordingPath) { - // Init interceptor from PlayBackRecorder std::string recordingPath(baseRecordingPath); recordingPath.append("/recordings"); @@ -301,38 +321,6 @@ namespace Azure { namespace Core { namespace Test { Sanitize(testNameInfo->test_suite_name()), Sanitize(testNameInfo->name())); m_testContext.RecordingPath = recordingPath; m_interceptor = std::make_unique(m_testContext); - - if (!m_testContext.IsPlaybackMode()) - { - auto SetBuiltinEnvironment = [](std::string const& targetVariable) { - std::string azureVariable = "AZURE" + targetVariable; - if (Azure::Core::_internal::Environment::GetVariable(azureVariable.c_str()).empty()) - { - std::string serviceName - = Azure::Core::_internal::Environment::GetVariable("AZURE_SERVICE_DIRECTORY"); - if (serviceName.empty()) - { - throw std::runtime_error( - "Could not find a value for AZURE_" + targetVariable - + " and AZURE_SERVICE_DIRECTORY was not defined. Define either AZURE_" - + targetVariable + " or AZURE_SERVICE_DIRECTORY to resolve."); - } - // Upper case the serviceName environment variable because all ci.yml environment - // variables are upper cased. - serviceName = Azure::Core::_internal::StringExtensions::ToUpper(serviceName); - std::string targetValue = Azure::Core::_internal::Environment::GetVariable( - (serviceName + targetVariable).c_str()); - if (!targetValue.empty()) - { - Azure::Core::_internal::Environment::SetVariable( - azureVariable.c_str(), targetValue.c_str()); - } - } - }; - SetBuiltinEnvironment("_TENANT_ID"); - SetBuiltinEnvironment("_CLIENT_ID"); - SetBuiltinEnvironment("_CLIENT_SECRET"); - } } /** From 2eb1782da99843718bf267fd27533bbb9ceb86d0 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 12:09:27 -0800 Subject: [PATCH 26/29] clang_format --- sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 6a300037a2..45b2e121c9 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -269,12 +269,13 @@ namespace Azure { namespace Core { namespace Test { { throw std::runtime_error( "Could not find a value for " + name - + " and AZURE_SERVICE_DIRECTORY was not defined. Define either " - + name + " or AZURE_SERVICE_DIRECTORY to resolve."); + + " and AZURE_SERVICE_DIRECTORY was not defined. Define either " + name + + " or AZURE_SERVICE_DIRECTORY to resolve."); } // Upper case the serviceName environment variable because all ci.yml environment // variables are upper cased. - std::string serviceDirectoryEnvVar = Azure::Core::_internal::StringExtensions::ToUpper(serviceDirectory); + std::string serviceDirectoryEnvVar + = Azure::Core::_internal::StringExtensions::ToUpper(serviceDirectory); serviceDirectoryEnvVar += name.substr(sizeof("AZURE") - 1); ret = Azure::Core::_internal::Environment::GetVariable(serviceDirectoryEnvVar.c_str()); if (!ret.empty()) From 7f8c322eea0446f4f45b8107ea2c926d30384e09 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 12:19:40 -0800 Subject: [PATCH 27/29] Comment cleanup --- .../inc/azure/core/test/test_base.hpp | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 45b2e121c9..8f71eaee74 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -243,7 +243,22 @@ namespace Azure { namespace Core { namespace Test { "Test Log from: [ " + m_testContext.GetTestPlaybackRecordingName() + " ] - " + message); } - // Util for tests getting env vars + /** + * @brief Utility function used by tests to retrieve env vars + * + * @param name Environment variable name to retrieve. + * + * @return The value of the environment variable retrieved. + * + * @note If AZURE_TENANT_ID, AZURE_CLIENT_ID, or AZURE_CLIENT_SECRET are not available in the + * environment, the AZURE_SERVICE_DIRECTORY environment variable is used to set those values + * with the values emitted by the New-TestResources.ps1 script. + * + * @note The Azure CI pipeline upper cases all environment variables defined in the pipeline. + * Since some operating systems have case sensitive environment variables, on debug builds, this + * function ensures that the environment variable being retrieved is all upper case. + * + */ std::string GetEnv(std::string const& name) { #if !defined(NDEBUG) @@ -303,10 +318,6 @@ namespace Azure { namespace Core { namespace Test { * Azure::Core::Test::TestBase::SetUpTestBase(AZURE_TEST_RECORDING_DIR); * \endcode * - * @note If AZURE_TENANT_ID, AZURE_CLIENT_ID, or AZURE_CLIENT_SECRET are not available in the - * environment, the AZURE_SERVICE_DIRECTORY environment variable is used to set those values - * with the values emitted by the New-TestResources.ps1 script. - * */ void SetUpTestBase(std::string const& baseRecordingPath) { From b889020e27fe838572d6f396408763d20461842e Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 14:13:30 -0800 Subject: [PATCH 28/29] Declare requestSecureHttp as const Co-authored-by: Victor Vazquez --- sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp index 578529bebb..8c26b0ca71 100644 --- a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp +++ b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp @@ -303,7 +303,7 @@ void WinHttpTransport::CreateRequestHandle(std::unique_ptr<_detail::HandleManage { const std::string& path = handleManager->m_request.GetUrl().GetRelativeUrl(); HttpMethod requestMethod = handleManager->m_request.GetMethod(); - bool requestSecureHttp( + bool const requestSecureHttp( !Azure::Core::_internal::StringExtensions::LocaleInvariantCaseInsensitiveEqual( handleManager->m_request.GetUrl().GetScheme(), HttpScheme)); From 73490470a73cf38ce1a8d7a22acac349c75fab61 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 24 Feb 2022 14:20:46 -0800 Subject: [PATCH 29/29] Pull request feedback --- sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp index 8f71eaee74..3889ec3597 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_base.hpp @@ -276,7 +276,8 @@ namespace Azure { namespace Core { namespace Test { auto ret = Azure::Core::_internal::Environment::GetVariable(name.c_str()); if (ret.empty()) { - if (!m_testContext.IsPlaybackMode() && name.find("AZURE_") == 0) + static const char azurePrefix[] = "AZURE_"; + if (!m_testContext.IsPlaybackMode() && name.find(azurePrefix) == 0) { std::string serviceDirectory = Azure::Core::_internal::Environment::GetVariable("AZURE_SERVICE_DIRECTORY"); @@ -291,7 +292,7 @@ namespace Azure { namespace Core { namespace Test { // variables are upper cased. std::string serviceDirectoryEnvVar = Azure::Core::_internal::StringExtensions::ToUpper(serviceDirectory); - serviceDirectoryEnvVar += name.substr(sizeof("AZURE") - 1); + serviceDirectoryEnvVar += name.substr(sizeof(azurePrefix) - 2); ret = Azure::Core::_internal::Environment::GetVariable(serviceDirectoryEnvVar.c_str()); if (!ret.empty()) {