From ef24dfda2ef64d9282020adc5cb351eac0cf2834 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Tue, 14 Mar 2023 16:46:10 -0700 Subject: [PATCH 01/20] Add GetCredentialName() (#4428) * Add GetCredentialName() * Update * Undo accidental change * Clang-format * Call GetCredentialName() instead of using constant; Return in-place constructed name; Explicit tests for GetCredentialName() * PR feedback * constructor parameter + non-virtual GetCredentialName() * Update sdk/core/azure-core/CMakeLists.txt * Update sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp * Update sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp * GCC and Clang warnings * Promote ThrowIfNotSafeCmdLineInput() to private member; avoid copies when calling GetCredentialName() * Spelling * Fix deprecated usage * Fix iteration * Clang-format --------- Co-authored-by: Anton Kolesnyk --- .../azure/core/test/test_proxy_manager.hpp | 2 + sdk/core/azure-core/CHANGELOG.md | 1 + .../azure/core/credentials/credentials.hpp | 25 ++++++- ...earer_token_authentication_policy_test.cpp | 2 +- sdk/identity/azure-identity/CHANGELOG.md | 2 + .../azure/identity/azure_cli_credential.hpp | 2 + .../identity/chained_token_credential.hpp | 6 +- .../src/azure_cli_credential.cpp | 21 +++--- .../src/chained_token_credential.cpp | 68 ++++++++----------- .../src/client_certificate_credential.cpp | 3 +- .../src/client_secret_credential.cpp | 2 +- .../src/default_azure_credential.cpp | 33 +++++---- .../src/environment_credential.cpp | 22 ++++-- .../src/managed_identity_credential.cpp | 12 ++-- .../src/managed_identity_source.cpp | 39 +++++++---- .../src/private/managed_identity_source.hpp | 7 ++ .../test/ut/azure_cli_credential_test.cpp | 6 ++ .../test/ut/chained_token_credential_test.cpp | 28 ++++++-- .../ut/client_certificate_credential_test.cpp | 17 ++++- .../test/ut/client_secret_credential_test.cpp | 10 +++ .../test/ut/default_azure_credential_test.cpp | 22 ++++++ .../test/ut/environment_credential_test.cpp | 16 +++++ .../ut/managed_identity_credential_test.cpp | 15 ++++ .../test/ut/token_credential_impl_test.cpp | 40 ++++++++++- 24 files changed, 292 insertions(+), 109 deletions(-) diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp index 6fae5bc816..f1097c9bb3 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp @@ -31,6 +31,8 @@ namespace Azure { namespace Core { namespace Test { class TestNonExpiringCredential final : public Core::Credentials::TokenCredential { public: + TestNonExpiringCredential() : TokenCredential("TestNonExpiringCredential") {} + Core::Credentials::AccessToken GetToken( Core::Credentials::TokenRequestContext const& tokenRequestContext, Core::Context const& context) const override diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 4ecb1b52a6..cca5d146d9 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -6,6 +6,7 @@ - Added the ability to ignore invalid certificate common name for TLS connections in WinHTTP transport. - Added `DisableTlsCertificateValidation` in `TransportOptions`. +- Added `TokenCredential::GetCredentialName()` to be utilized in diagnostic messages. If you have any custom implementations of `TokenCredential`, it is recommended to pass the name of your credential to `TokenCredential` constructor. The old parameterless constructor is deprecated. ### Breaking Changes diff --git a/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp b/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp index 27a9e62d33..710c192b96 100644 --- a/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp +++ b/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp @@ -60,6 +60,9 @@ namespace Azure { namespace Core { namespace Credentials { * @brief A base type of credential that uses Azure::Core::AccessToken to authenticate requests. */ class TokenCredential { + private: + std::string m_credentialName; + public: /** * @brief Gets an authentication token. @@ -75,6 +78,12 @@ namespace Azure { namespace Core { namespace Credentials { TokenRequestContext const& tokenRequestContext, Context const& context) const = 0; + /** + * @brief Gets the name of the credential. + * + */ + std::string const& GetCredentialName() const { return m_credentialName; } + /** * @brief Destructs `%TokenCredential`. * @@ -82,11 +91,25 @@ namespace Azure { namespace Core { namespace Credentials { virtual ~TokenCredential() = default; protected: + /** + * @brief Constructs an instance of `%TokenCredential`. + * + * @param credentialName Name of the credential for diagnostic messages. + */ + TokenCredential(std::string const& credentialName) + : m_credentialName(credentialName.empty() ? "Custom Credential" : credentialName) + { + } + /** * @brief Constructs a default instance of `%TokenCredential`. * + * @deprecated Use the constructor with parameter. */ - TokenCredential() {} + [[deprecated("Use the constructor with parameter.")]] TokenCredential() + : TokenCredential(std::string{}) + { + } private: /** diff --git a/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp b/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp index 48bacd8fdc..c38c6aa199 100644 --- a/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp @@ -16,7 +16,7 @@ class TestTokenCredential final : public Azure::Core::Credentials::TokenCredenti public: explicit TestTokenCredential( std::shared_ptr accessToken) - : m_accessToken(accessToken) + : TokenCredential("TestTokenCredential"), m_accessToken(accessToken) { } diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index de58b0bcc5..b8143d12df 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -10,6 +10,8 @@ ### Other Changes +- Improved diagnostics to utilize `Azure::Core::Credentials::TokenCredential::GetCredentialName()`. + ## 1.5.0-beta.1 (2023-03-07) ### Features Added diff --git a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp index 946c2293a0..53aec4f32c 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp @@ -58,6 +58,8 @@ namespace Azure { namespace Identity { DateTime::duration cliProcessTimeout, Core::Credentials::TokenCredentialOptions const& options); + void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& description) const; + public: /** * @brief Constructs an Azure CLI Credential. diff --git a/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp index 726735c2ef..042942cb12 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp @@ -62,13 +62,9 @@ namespace Azure { namespace Identity { Core::Context const& context) const override; private: - explicit ChainedTokenCredential( - Sources sources, - std::string const& enclosingCredential, - std::vector sourcesFriendlyNames); + explicit ChainedTokenCredential(Sources sources, std::string const& enclosingCredential); Sources m_sources; - std::vector m_sourcesFriendlyNames; std::string m_logPrefix; }; diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index 83e3e4b05c..f2c631adc1 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -51,9 +51,12 @@ using Azure::Identity::_detail::TokenCache; using Azure::Identity::_detail::TokenCredentialImpl; namespace { -std::string const MsgPrefix = "Identity: AzureCliCredential"; +constexpr auto IdentityPrefix = "Identity: "; +} -void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& description) +void AzureCliCredential::ThrowIfNotSafeCmdLineInput( + std::string const& input, + std::string const& description) const { for (auto const c : input) { @@ -71,20 +74,21 @@ void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& des if (!std::isalnum(c, std::locale::classic())) { throw AuthenticationException( - MsgPrefix + ": Unsafe command line input found in " + description + ": " + input); + IdentityPrefix + GetCredentialName() + ": Unsafe command line input found in " + + description + ": " + input); } } } } -} // namespace - AzureCliCredential::AzureCliCredential( std::string tenantId, DateTime::duration cliProcessTimeout, Core::Credentials::TokenCredentialOptions const& options) - : m_tenantId(std::move(tenantId)), m_cliProcessTimeout(std::move(cliProcessTimeout)) + : TokenCredential("AzureCliCredential"), m_tenantId(std::move(tenantId)), + m_cliProcessTimeout(std::move(cliProcessTimeout)) { static_cast(options); + ThrowIfNotSafeCmdLineInput(m_tenantId, "TenantID"); auto const logLevel = Logger::Level::Informational; @@ -92,7 +96,7 @@ AzureCliCredential::AzureCliCredential( { Log::Write( logLevel, - MsgPrefix + IdentityPrefix + GetCredentialName() + " created.\n" "Successful creation does not guarantee further successful token retrieval."); } @@ -161,7 +165,8 @@ AccessToken AzureCliCredential::GetToken( } catch (std::exception const& e) { - auto const errorMsg = MsgPrefix + " didn't get the token: \"" + e.what() + '\"'; + auto const errorMsg + = IdentityPrefix + GetCredentialName() + " didn't get the token: \"" + e.what() + '\"'; auto const logLevel = Logger::Level::Warning; if (Log::ShouldWrite(logLevel)) diff --git a/sdk/identity/azure-identity/src/chained_token_credential.cpp b/sdk/identity/azure-identity/src/chained_token_credential.cpp index 3a63bea9ce..116270b434 100644 --- a/sdk/identity/azure-identity/src/chained_token_credential.cpp +++ b/sdk/identity/azure-identity/src/chained_token_credential.cpp @@ -4,7 +4,6 @@ #include "azure/identity/chained_token_credential.hpp" #include "azure/core/internal/diagnostics/log.hpp" -#include #include @@ -15,63 +14,53 @@ using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; namespace { -std::string const IdentityPrefix = "Identity: "; -} +constexpr auto IdentityPrefix = "Identity: "; +} // namespace ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources) - : ChainedTokenCredential(sources, {}, {}) + : ChainedTokenCredential(sources, {}) { } ChainedTokenCredential::ChainedTokenCredential( ChainedTokenCredential::Sources sources, - std::string const& enclosingCredential, - std::vector sourcesFriendlyNames) - : m_sources(std::move(sources)), m_sourcesFriendlyNames(std::move(sourcesFriendlyNames)) + std::string const& enclosingCredential) + : TokenCredential("ChainedTokenCredential"), m_sources(std::move(sources)) { - // LCOV_EXCL_START - AZURE_ASSERT(m_sourcesFriendlyNames.empty() || m_sourcesFriendlyNames.size() == m_sources.size()); - // LCOV_EXCL_STOP + m_logPrefix = IdentityPrefix + + (enclosingCredential.empty() ? GetCredentialName() + : (enclosingCredential + " -> " + GetCredentialName())) + + ": "; auto const logLevel = m_sources.empty() ? Logger::Level::Warning : Logger::Level::Informational; if (Log::ShouldWrite(logLevel)) { - std::string credentialsList; - if (!m_sourcesFriendlyNames.empty()) + std::string credSourceDetails = " with EMPTY chain of credentials."; + if (!m_sources.empty()) { - auto const sourceDescriptionsSize = m_sourcesFriendlyNames.size(); - for (size_t i = 0; i < (sourceDescriptionsSize - 1); ++i) + credSourceDetails = " with the following credentials: "; + + auto const sourcesSize = m_sources.size(); + for (size_t i = 0; i < sourcesSize; ++i) { - credentialsList += m_sourcesFriendlyNames[i] + ", "; + if (i != 0) + { + credSourceDetails += ", "; + } + + credSourceDetails += m_sources[i]->GetCredentialName(); } - credentialsList += m_sourcesFriendlyNames.back(); + credSourceDetails += '.'; } Log::Write( logLevel, IdentityPrefix + (enclosingCredential.empty() - ? "ChainedTokenCredential: Created" - : (enclosingCredential + ": Created ChainedTokenCredential")) - + " with " - + (m_sourcesFriendlyNames.empty() - ? (std::to_string(m_sources.size()) + " credentials.") - : (std::string("the following credentials: ") + credentialsList + '.'))); - } - - m_logPrefix = IdentityPrefix - + (enclosingCredential.empty() ? "ChainedTokenCredential" - : (enclosingCredential + " -> ChainedTokenCredential")) - + ": "; - - if (m_sourcesFriendlyNames.empty()) - { - auto const sourcesSize = m_sources.size(); - for (size_t i = 1; i <= sourcesSize; ++i) - { - m_sourcesFriendlyNames.push_back(std::string("credential #") + std::to_string(i)); - } + ? (GetCredentialName() + ": Created") + : (enclosingCredential + ": Created " + GetCredentialName())) + + credSourceDetails); } } @@ -106,7 +95,8 @@ AccessToken ChainedTokenCredential::GetToken( { Log::Write( logLevel, - m_logPrefix + "Successfully got token from " + m_sourcesFriendlyNames[i] + '.'); + m_logPrefix + "Successfully got token from " + m_sources[i]->GetCredentialName() + + '.'); } } @@ -120,7 +110,7 @@ AccessToken ChainedTokenCredential::GetToken( { Log::Write( logLevel, - m_logPrefix + "Failed to get token from " + m_sourcesFriendlyNames[i] + ": " + m_logPrefix + "Failed to get token from " + m_sources[i]->GetCredentialName() + ": " + e.what()); } } @@ -139,5 +129,5 @@ AccessToken ChainedTokenCredential::GetToken( } } - throw AuthenticationException("Failed to get token from ChainedTokenCredential."); + throw AuthenticationException("Failed to get token from " + GetCredentialName() + '.'); } diff --git a/sdk/identity/azure-identity/src/client_certificate_credential.cpp b/sdk/identity/azure-identity/src/client_certificate_credential.cpp index 7f41773fb7..9ffd31fbdb 100644 --- a/sdk/identity/azure-identity/src/client_certificate_credential.cpp +++ b/sdk/identity/azure-identity/src/client_certificate_credential.cpp @@ -80,7 +80,8 @@ ClientCertificateCredential::ClientCertificateCredential( std::string const& clientCertificatePath, std::string const& authorityHost, TokenCredentialOptions const& options) - : m_clientCredentialCore(tenantId, authorityHost), + : TokenCredential("ClientCertificateCredential"), + m_clientCredentialCore(tenantId, authorityHost), m_tokenCredentialImpl(std::make_unique(options)), m_requestBody( std::string( diff --git a/sdk/identity/azure-identity/src/client_secret_credential.cpp b/sdk/identity/azure-identity/src/client_secret_credential.cpp index 42fcedfcb3..c0e8355f61 100644 --- a/sdk/identity/azure-identity/src/client_secret_credential.cpp +++ b/sdk/identity/azure-identity/src/client_secret_credential.cpp @@ -21,7 +21,7 @@ ClientSecretCredential::ClientSecretCredential( std::string const& clientSecret, std::string const& authorityHost, TokenCredentialOptions const& options) - : m_clientCredentialCore(tenantId, authorityHost), + : TokenCredential("ClientSecretCredential"), m_clientCredentialCore(tenantId, authorityHost), m_tokenCredentialImpl(std::make_unique(options)), m_requestBody( std::string("grant_type=client_credentials&client_id=") + Url::Encode(clientId) diff --git a/sdk/identity/azure-identity/src/default_azure_credential.cpp b/sdk/identity/azure-identity/src/default_azure_credential.cpp index 85d0eead91..3a77ebd10e 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -17,10 +17,11 @@ using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; namespace { -std::string const IdentityPrefix = "Identity: "; -} +constexpr auto IdentityPrefix = "Identity: "; +} // namespace DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& options) + : TokenCredential("DefaultAzureCredential") { // Initializing m_credential below and not in the member initializer list to have a specific order // of log messages. @@ -29,13 +30,16 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt { Log::Write( logLevel, - IdentityPrefix - + "Creating DefaultAzureCredential which combines mutiple parameterless credentials " - "into a single one (by using ChainedTokenCredential)." - "\nDefaultAzureCredential is only recommended for the early stages of development, " + std::string(IdentityPrefix) + "Creating " + GetCredentialName() + + " which combines mutiple parameterless credentials " + "into a single one (by using ChainedTokenCredential).\n" + + GetCredentialName() + + " is only recommended for the early stages of development, " "and not for usage in production environment." - "\nOnce the developer focuses on the Credentials and Authentication aspects of their " - "application, DefaultAzureCredential needs to be replaced with the credential that " + "\nOnce the developer focuses on the Credentials and Authentication aspects " + "of their application, " + + GetCredentialName() + + " needs to be replaced with the credential that " "is the better fit for the application."); } @@ -47,9 +51,7 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt // Using the ChainedTokenCredential's private constructor for more detailed log messages. m_credentials.reset(new ChainedTokenCredential( ChainedTokenCredential::Sources{envCred, azCliCred, managedIdentityCred}, - "DefaultAzureCredential", // extra args for the ChainedTokenCredential's private constructor. - std::vector{ - "EnvironmentCredential", "AzureCliCredential", "ManagedIdentityCredential"})); + GetCredentialName())); // extra arg for the ChainedTokenCredential's private constructor. } DefaultAzureCredential::~DefaultAzureCredential() = default; @@ -64,9 +66,10 @@ AccessToken DefaultAzureCredential::GetToken( } catch (AuthenticationException const&) { - throw AuthenticationException("Failed to get token from DefaultAzureCredential." - "\nSee Azure::Core::Diagnostics::Logger for details " - "(https://github.com/Azure/azure-sdk-for-cpp/tree/main/sdk/" - "identity/azure-identity#troubleshooting)."); + throw AuthenticationException( + "Failed to get token from " + GetCredentialName() + + ".\nSee Azure::Core::Diagnostics::Logger for details " + "(https://github.com/Azure/azure-sdk-for-cpp/tree/main/sdk/" + "identity/azure-identity#troubleshooting)."); } } diff --git a/sdk/identity/azure-identity/src/environment_credential.cpp b/sdk/identity/azure-identity/src/environment_credential.cpp index 2d6eb2479c..f6dd764149 100644 --- a/sdk/identity/azure-identity/src/environment_credential.cpp +++ b/sdk/identity/azure-identity/src/environment_credential.cpp @@ -30,15 +30,19 @@ constexpr auto AzureClientSecretEnvVarName = "AZURE_CLIENT_SECRET"; constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST"; constexpr auto AzureClientCertificatePathEnvVarName = "AZURE_CLIENT_CERTIFICATE_PATH"; -std::string const LogMsgPrefix = "Identity: EnvironmentCredential"; +constexpr auto IdentityPrefix = "Identity: "; void PrintCredentialCreationLogMessage( + std::string const& logMsgPrefix, std::vector> const& envVarsToParams, char const* credThatGetsCreated); } // namespace EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) + : TokenCredential("EnvironmentCredential") { + auto const logMsgPrefix = IdentityPrefix + GetCredentialName(); + auto tenantId = Environment::GetVariable(AzureTenantIdEnvVarName); auto clientId = Environment::GetVariable(AzureClientIdEnvVarName); @@ -54,6 +58,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -72,6 +77,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -88,6 +94,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -106,6 +113,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -124,7 +132,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) auto const logLevel = Logger::Level::Warning; if (Log::ShouldWrite(logLevel)) { - auto const basicMessage = LogMsgPrefix + " was not initialized with underlying credential"; + auto const basicMessage = logMsgPrefix + " was not initialized with underlying credential"; if (!Log::ShouldWrite(Logger::Level::Verbose)) { @@ -163,7 +171,8 @@ AccessToken EnvironmentCredential::GetToken( { if (!m_credentialImpl) { - auto const AuthUnavailable = LogMsgPrefix + " authentication unavailable. "; + auto const AuthUnavailable + = IdentityPrefix + GetCredentialName() + " authentication unavailable. "; { auto const logLevel = Logger::Level::Warning; @@ -171,7 +180,7 @@ AccessToken EnvironmentCredential::GetToken( { Log::Write( logLevel, - AuthUnavailable + "See earlier EnvironmentCredential log messages for details."); + AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); } } @@ -184,6 +193,7 @@ AccessToken EnvironmentCredential::GetToken( namespace { void PrintCredentialCreationLogMessage( + std::string const& logMsgPrefix, std::vector> const& envVarsToParams, char const* credThatGetsCreated) { @@ -193,7 +203,7 @@ void PrintCredentialCreationLogMessage( { Log::Write( Logger::Level::Informational, - LogMsgPrefix + " gets created with " + credThatGetsCreated + '.'); + logMsgPrefix + " gets created with " + credThatGetsCreated + '.'); } return; @@ -224,7 +234,7 @@ void PrintCredentialCreationLogMessage( Log::Write( Logger::Level::Verbose, - LogMsgPrefix + ": " + envVars + " environment variables are set, so " + credThatGetsCreated + logMsgPrefix + ": " + envVars + " environment variables are set, so " + credThatGetsCreated + " with corresponding " + credParams + " gets created."); } } // namespace diff --git a/sdk/identity/azure-identity/src/managed_identity_credential.cpp b/sdk/identity/azure-identity/src/managed_identity_credential.cpp index 209dfb8b4f..7b64abcaf1 100644 --- a/sdk/identity/azure-identity/src/managed_identity_credential.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_credential.cpp @@ -8,13 +8,16 @@ using namespace Azure::Identity; namespace { std::unique_ptr<_detail::ManagedIdentitySource> CreateManagedIdentitySource( + std::string const& credentialName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { using namespace Azure::Core::Credentials; using namespace Azure::Identity::_detail; static std::unique_ptr (*managedIdentitySourceCreate[])( - std::string const& clientId, TokenCredentialOptions const& options) + std::string const& credName, + std::string const& clientId, + TokenCredentialOptions const& options) = {AppServiceV2019ManagedIdentitySource::Create, AppServiceV2017ManagedIdentitySource::Create, CloudShellManagedIdentitySource::Create, @@ -25,7 +28,7 @@ std::unique_ptr<_detail::ManagedIdentitySource> CreateManagedIdentitySource( // For that reason, it is not possible to cover that execution branch in tests. for (auto create : managedIdentitySourceCreate) // LCOV_EXCL_LINE { - if (auto source = create(clientId, options)) + if (auto source = create(credentialName, clientId, options)) { return source; } @@ -33,7 +36,7 @@ std::unique_ptr<_detail::ManagedIdentitySource> CreateManagedIdentitySource( // LCOV_EXCL_START throw AuthenticationException( - "ManagedIdentityCredential authentication unavailable. No Managed Identity endpoint found."); + credentialName + " authentication unavailable. No Managed Identity endpoint found."); // LCOV_EXCL_STOP } } // namespace @@ -43,8 +46,9 @@ ManagedIdentityCredential::~ManagedIdentityCredential() = default; ManagedIdentityCredential::ManagedIdentityCredential( std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) - : m_managedIdentitySource(CreateManagedIdentitySource(clientId, options)) + : TokenCredential("ManagedIdentityCredential") { + m_managedIdentitySource = CreateManagedIdentitySource(GetCredentialName(), clientId, options); } ManagedIdentityCredential::ManagedIdentityCredential( diff --git a/sdk/identity/azure-identity/src/managed_identity_source.cpp b/sdk/identity/azure-identity/src/managed_identity_source.cpp index de7e8e6c3c..89dfa0a773 100644 --- a/sdk/identity/azure-identity/src/managed_identity_source.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_source.cpp @@ -19,28 +19,28 @@ using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; namespace { -std::string const IdentityPrefix = "Identity: "; -constexpr auto CredPrefix = "ManagedIdentityCredential"; +constexpr auto IdentityPrefix = "Identity: "; std::string WithSourceMessage(std::string const& credSource) { return " with " + credSource + " source"; } -void PrintEnvNotSetUpMessage(std::string const& credSource) +void PrintEnvNotSetUpMessage(std::string const& credName, std::string const& credSource) { auto const logLevel = Logger::Level::Verbose; if (Log::ShouldWrite(logLevel)) { Log::Write( logLevel, - IdentityPrefix + CredPrefix + ": Environment is not set up for the credential to be created" + IdentityPrefix + credName + ": Environment is not set up for the credential to be created" + WithSourceMessage(credSource) + '.'); } } } // namespace Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( + std::string const& credName, std::string const& url, char const* envVarName, std::string const& credSource) @@ -57,7 +57,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( { Log::Write( logLevel, - IdentityPrefix + CredPrefix + " will be created" + WithSourceMessage(credSource) + '.'); + IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.'); } return endpointUrl; @@ -69,7 +69,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( { } - auto const errorMessage = CredPrefix + WithSourceMessage(credSource) + auto const errorMessage = credName + WithSourceMessage(credSource) + ": Failed to create: The environment variable \'" + envVarName + "\' contains an invalid URL."; @@ -84,6 +84,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( template std::unique_ptr AppServiceManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options, char const* endpointVarName, @@ -98,10 +99,13 @@ std::unique_ptr AppServiceManagedIdentitySource::Create( if (!msiEndpoint.empty() && !msiSecret.empty()) { return std::unique_ptr(new T( - clientId, options, ParseEndpointUrl(msiEndpoint, endpointVarName, credSource), msiSecret)); + clientId, + options, + ParseEndpointUrl(credName, msiEndpoint, endpointVarName, credSource), + msiSecret)); } - PrintEnvNotSetUpMessage(credSource); + PrintEnvNotSetUpMessage(credName, credSource); return nullptr; } @@ -163,22 +167,25 @@ Azure::Core::Credentials::AccessToken AppServiceManagedIdentitySource::GetToken( } std::unique_ptr AppServiceV2017ManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options) { return AppServiceManagedIdentitySource::Create( - clientId, options, "MSI_ENDPOINT", "MSI_SECRET", "2017"); + credName, clientId, options, "MSI_ENDPOINT", "MSI_SECRET", "2017"); } std::unique_ptr AppServiceV2019ManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options) { return AppServiceManagedIdentitySource::Create( - clientId, options, "IDENTITY_ENDPOINT", "IDENTITY_HEADER", "2019"); + credName, clientId, options, "IDENTITY_ENDPOINT", "IDENTITY_HEADER", "2019"); } std::unique_ptr CloudShellManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { @@ -190,10 +197,10 @@ std::unique_ptr CloudShellManagedIdentitySource::Create( if (!msiEndpoint.empty()) { return std::unique_ptr(new CloudShellManagedIdentitySource( - clientId, options, ParseEndpointUrl(msiEndpoint, EndpointVarName, CredSource))); + clientId, options, ParseEndpointUrl(credName, msiEndpoint, EndpointVarName, CredSource))); } - PrintEnvNotSetUpMessage(CredSource); + PrintEnvNotSetUpMessage(credName, CredSource); return nullptr; } @@ -252,6 +259,7 @@ Azure::Core::Credentials::AccessToken CloudShellManagedIdentitySource::GetToken( } std::unique_ptr AzureArcManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { @@ -264,7 +272,7 @@ std::unique_ptr AzureArcManagedIdentitySource::Create( if (identityEndpoint.empty() || Environment::GetVariable("IMDS_ENDPOINT").empty()) { - PrintEnvNotSetUpMessage(credSource); + PrintEnvNotSetUpMessage(credName, credSource); return nullptr; } @@ -277,7 +285,7 @@ std::unique_ptr AzureArcManagedIdentitySource::Create( } return std::unique_ptr(new AzureArcManagedIdentitySource( - options, ParseEndpointUrl(identityEndpoint, EndpointVarName, credSource))); + options, ParseEndpointUrl(credName, identityEndpoint, EndpointVarName, credSource))); } AzureArcManagedIdentitySource::AzureArcManagedIdentitySource( @@ -373,6 +381,7 @@ Azure::Core::Credentials::AccessToken AzureArcManagedIdentitySource::GetToken( } std::unique_ptr ImdsManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { @@ -381,7 +390,7 @@ std::unique_ptr ImdsManagedIdentitySource::Create( { Log::Write( logLevel, - IdentityPrefix + CredPrefix + " will be created" + IdentityPrefix + credName + " will be created" + WithSourceMessage("Azure Instance Metadata Service") + ".\nSuccessful creation does not guarantee further successful token retrieval."); } diff --git a/sdk/identity/azure-identity/src/private/managed_identity_source.hpp b/sdk/identity/azure-identity/src/private/managed_identity_source.hpp index 9972ce904a..146da297bf 100644 --- a/sdk/identity/azure-identity/src/private/managed_identity_source.hpp +++ b/sdk/identity/azure-identity/src/private/managed_identity_source.hpp @@ -30,6 +30,7 @@ namespace Azure { namespace Identity { namespace _detail { _detail::TokenCache m_tokenCache; static Core::Url ParseEndpointUrl( + std::string const& credName, std::string const& url, char const* envVarName, std::string const& credSource); @@ -63,6 +64,7 @@ namespace Azure { namespace Identity { namespace _detail { template static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options, char const* endpointVarName, @@ -97,6 +99,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); }; @@ -123,6 +126,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); }; @@ -139,6 +143,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); @@ -157,6 +162,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); @@ -175,6 +181,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); diff --git a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp index 31b740494e..5e143493c6 100644 --- a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp @@ -158,6 +158,12 @@ TEST(AzureCliCredential, Error) Logger::SetListener(nullptr); } +TEST(AzureCliCredential, GetCredentialName) +{ + AzureCliTestCredential const cred(EmptyOutputCommand); + EXPECT_EQ(cred.GetCredentialName(), "AzureCliCredential"); +} + TEST(AzureCliCredential, EmptyOutput) { AzureCliTestCredential const azCliCred(EmptyOutputCommand); diff --git a/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp index fa8297e24a..3b767572f6 100644 --- a/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp @@ -25,7 +25,7 @@ class TestCredential : public TokenCredential { std::string m_token; public: - TestCredential(std::string token = "") : m_token(token) {} + TestCredential(std::string token = "") : TokenCredential("TestCredential"), m_token(token) {} mutable bool WasInvoked = false; @@ -45,6 +45,12 @@ class TestCredential : public TokenCredential { }; } // namespace +TEST(ChainedTokenCredential, GetCredentialName) +{ + ChainedTokenCredential const cred(ChainedTokenCredential::Sources{}); + EXPECT_EQ(cred.GetCredentialName(), "ChainedTokenCredential"); +} + TEST(ChainedTokenCredential, Success) { auto c1 = std::make_shared("Token1"); @@ -110,7 +116,9 @@ TEST(ChainedTokenCredential, Logging) ChainedTokenCredential cred({}); EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); EXPECT_EQ(log[0].first, Logger::Level::Warning); - EXPECT_EQ(log[0].second, "Identity: ChainedTokenCredential: Created with 0 credentials."); + EXPECT_EQ( + log[0].second, + "Identity: ChainedTokenCredential: Created with EMPTY chain of credentials."); log.clear(); EXPECT_THROW(static_cast(cred.GetToken({}, {})), AuthenticationException); @@ -128,7 +136,10 @@ TEST(ChainedTokenCredential, Logging) ChainedTokenCredential cred({c}); EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); EXPECT_EQ(log[0].first, Logger::Level::Informational); - EXPECT_EQ(log[0].second, "Identity: ChainedTokenCredential: Created with 1 credentials."); + EXPECT_EQ( + log[0].second, + "Identity: ChainedTokenCredential: Created with the following credentials: " + "TestCredential."); log.clear(); EXPECT_FALSE(c->WasInvoked); @@ -141,7 +152,7 @@ TEST(ChainedTokenCredential, Logging) EXPECT_EQ(log[0].first, Logger::Level::Verbose); EXPECT_EQ( log[0].second, - "Identity: ChainedTokenCredential: Failed to get token from credential #1: " + "Identity: ChainedTokenCredential: Failed to get token from TestCredential: " "Test Error"); EXPECT_EQ(log[1].first, Logger::Level::Warning); @@ -158,7 +169,10 @@ TEST(ChainedTokenCredential, Logging) ChainedTokenCredential cred({c1, c2}); EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); EXPECT_EQ(log[0].first, Logger::Level::Informational); - EXPECT_EQ(log[0].second, "Identity: ChainedTokenCredential: Created with 2 credentials."); + EXPECT_EQ( + log[0].second, + "Identity: ChainedTokenCredential: Created with the following credentials: " + "TestCredential, TestCredential."); log.clear(); EXPECT_FALSE(c1->WasInvoked); @@ -175,13 +189,13 @@ TEST(ChainedTokenCredential, Logging) EXPECT_EQ(log[0].first, Logger::Level::Verbose); EXPECT_EQ( log[0].second, - "Identity: ChainedTokenCredential: Failed to get token from credential #1: " + "Identity: ChainedTokenCredential: Failed to get token from TestCredential: " "Test Error"); EXPECT_EQ(log[1].first, Logger::Level::Informational); EXPECT_EQ( log[1].second, - "Identity: ChainedTokenCredential: Successfully got token from credential #2."); + "Identity: ChainedTokenCredential: Successfully got token from TestCredential."); } Logger::SetListener(nullptr); diff --git a/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp b/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp index b7ff3a3b46..f67963ab35 100644 --- a/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp @@ -30,9 +30,20 @@ std::vector SplitString(const std::string& s, char separator); std::string ToString(std::vector const& vec); } // namespace +TEST(ClientCertificateCredential, GetCredentialName) +{ + TempCertFile const tempCertFile; + ClientCertificateCredential const cred( + "01234567-89ab-cdef-fedc-ba8976543210", + "fedcba98-7654-3210-0123-456789abcdef", + TempCertFile::Path); + + EXPECT_EQ(cred.GetCredentialName(), "ClientCertificateCredential"); +} + TEST(ClientCertificateCredential, Regular) { - TempCertFile tempCertFile; + TempCertFile const tempCertFile; auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { @@ -167,7 +178,7 @@ TEST(ClientCertificateCredential, Regular) TEST(ClientCertificateCredential, AzureStack) { - TempCertFile tempCertFile; + TempCertFile const tempCertFile; auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { @@ -294,7 +305,7 @@ TEST(ClientCertificateCredential, AzureStack) TEST(ClientCertificateCredential, Authority) { - TempCertFile tempCertFile; + TempCertFile const tempCertFile; auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { diff --git a/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp b/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp index 7625bc8e69..81d354bf1f 100644 --- a/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp @@ -12,6 +12,16 @@ using Azure::Identity::ClientSecretCredential; using Azure::Identity::ClientSecretCredentialOptions; using Azure::Identity::Test::_detail::CredentialTestHelper; +TEST(ClientSecretCredential, GetCredentialName) +{ + ClientSecretCredential const cred( + "01234567-89ab-cdef-fedc-ba8976543210", + "fedcba98-7654-3210-0123-456789abcdef", + "CLIENTSECRET"); + + EXPECT_EQ(cred.GetCredentialName(), "ClientSecretCredential"); +} + TEST(ClientSecretCredential, Regular) { auto const actual = CredentialTestHelper::SimulateTokenRequest( diff --git a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp index 0b7826f7e0..ba9a4afa5f 100644 --- a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp @@ -17,6 +17,28 @@ using Azure::Identity::Test::_detail::CredentialTestHelper; namespace { } // namespace +TEST(DefaultAzureCredential, GetCredentialName) +{ + CredentialTestHelper::EnvironmentOverride const env({ + {"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"}, + {"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"}, + {"AZURE_CLIENT_SECRET", "CLIENTSECRET"}, + {"AZURE_AUTHORITY_HOST", ""}, + {"AZURE_USERNAME", ""}, + {"AZURE_PASSWORD", ""}, + {"AZURE_CLIENT_CERTIFICATE_PATH", ""}, + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", ""}, + {"IDENTITY_HEADER", "CLIENTSECRET"}, + {"IDENTITY_SERVER_THUMBPRINT", ""}, + }); + + DefaultAzureCredential const cred; + EXPECT_EQ(cred.GetCredentialName(), "DefaultAzureCredential"); +} + TEST(DefaultAzureCredential, LogMessages) { using LogMsgVec = std::vector>; diff --git a/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp b/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp index 5388b21892..61066e7c2b 100644 --- a/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp @@ -13,6 +13,22 @@ using Azure::Core::Http::HttpMethod; using Azure::Identity::EnvironmentCredential; using Azure::Identity::Test::_detail::CredentialTestHelper; +TEST(EnvironmentCredential, GetCredentialName) +{ + CredentialTestHelper::EnvironmentOverride const env({ + {"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"}, + {"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"}, + {"AZURE_CLIENT_SECRET", "CLIENTSECRET"}, + {"AZURE_AUTHORITY_HOST", ""}, + {"AZURE_USERNAME", ""}, + {"AZURE_PASSWORD", ""}, + {"AZURE_CLIENT_CERTIFICATE_PATH", ""}, + }); + + EnvironmentCredential const cred; + EXPECT_EQ(cred.GetCredentialName(), "EnvironmentCredential"); +} + TEST(EnvironmentCredential, RegularClientSecretCredential) { using Azure::Core::Diagnostics::Logger; diff --git a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp index 2923f91346..31deb38bbf 100644 --- a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp @@ -16,6 +16,21 @@ using Azure::Core::Http::HttpStatusCode; using Azure::Identity::ManagedIdentityCredential; using Azure::Identity::Test::_detail::CredentialTestHelper; +TEST(ManagedIdentityCredential, GetCredentialName) +{ + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", ""}, + {"IDENTITY_HEADER", "CLIENTSECRET"}, + {"IDENTITY_SERVER_THUMBPRINT", ""}, + }); + + ManagedIdentityCredential const cred; + EXPECT_EQ(cred.GetCredentialName(), "ManagedIdentityCredential"); +} + TEST(ManagedIdentityCredential, AppServiceV2019) { using Azure::Core::Diagnostics::Logger; diff --git a/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp index 6189fb98ef..567bba73aa 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp @@ -34,15 +34,16 @@ class TokenCredentialImplTester : public TokenCredential { HttpMethod httpMethod, Url url, TokenCredentialOptions const& options) - : m_httpMethod(std::move(httpMethod)), m_url(std::move(url)), - m_tokenCredentialImpl(new TokenCredentialImpl(options)) + : TokenCredential("TokenCredentialImplTester"), m_httpMethod(std::move(httpMethod)), + m_url(std::move(url)), m_tokenCredentialImpl(new TokenCredentialImpl(options)) { } explicit TokenCredentialImplTester( std::function throwingFunction, TokenCredentialOptions const& options) - : m_throwingFunction(std::move(throwingFunction)), + : TokenCredential("TokenCredentialImplTester"), + m_throwingFunction(std::move(throwingFunction)), m_tokenCredentialImpl(new TokenCredentialImpl(options)) { } @@ -63,8 +64,41 @@ class TokenCredentialImplTester : public TokenCredential { }); } }; + +// Disable deprecation warning +#if defined(_MSC_VER) +#pragma warning(push) +#pragma warning(disable : 4996) +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#endif +// This credential is needed to test the default behavior when the customer has custom credential +// they implemented while using earlier versions of the SDK which didn't have a constructor with +// credentialName. +class CustomTokenCredential : public TokenCredential { +public: + AccessToken GetToken(TokenRequestContext const&, Context const&) const override { return {}; } +}; +#if defined(_MSC_VER) +#pragma warning(pop) +#elif defined(__clang__) +#pragma clang diagnostic pop +#elif defined(__GNUC__) +#pragma GCC diagnostic pop +#endif // _MSC_VER + } // namespace +TEST(CustomTokenCredential, GetCredentialName) +{ + CustomTokenCredential cred; + EXPECT_EQ(cred.GetCredentialName(), "Custom Credential"); +} + TEST(TokenCredentialImpl, Normal) { auto const actual = CredentialTestHelper::SimulateTokenRequest( From 6f2e39a5c505326a1246886f0ec34522766397e1 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 15 Mar 2023 11:31:48 -0700 Subject: [PATCH 02/20] Sync eng/common directory with azure-sdk-tools for PR 5702 (#4453) * add healthinsights to Test-SampleMetadata script * spacing * update productSlug to azure-health-insights --------- Co-authored-by: Asaf Levi --- eng/common/scripts/Test-SampleMetadata.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/common/scripts/Test-SampleMetadata.ps1 b/eng/common/scripts/Test-SampleMetadata.ps1 index 71c8972a8a..c20396b0a9 100644 --- a/eng/common/scripts/Test-SampleMetadata.ps1 +++ b/eng/common/scripts/Test-SampleMetadata.ps1 @@ -202,6 +202,7 @@ begin { "azure-genomics", "azure-hdinsight", "azure-hdinsight-rserver", + "azure-health-insights", "azure-hpc-cache", "azure-immersive-reader", "azure-information-protection", From 9f19a28af34824e45e2fdb9d27cad18b0d6183a2 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Wed, 15 Mar 2023 16:18:51 -0700 Subject: [PATCH 03/20] Use aka.ms link to Identity troubleshooting (#4449) * Use aka.ms link to Identity troubleshooting * Update default_azure_credential.cpp * Update default_azure_credential.cpp --------- Co-authored-by: Anton Kolesnyk --- .../azure-identity/samples/default_azure_credential.cpp | 3 +-- sdk/identity/azure-identity/src/default_azure_credential.cpp | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/sdk/identity/azure-identity/samples/default_azure_credential.cpp b/sdk/identity/azure-identity/samples/default_azure_credential.cpp index 238d5c3f39..da2f590905 100644 --- a/sdk/identity/azure-identity/samples/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/samples/default_azure_credential.cpp @@ -14,8 +14,7 @@ int main() // Step 1: Initialize Default Azure Credential. // Default Azure Credential is good for samples and initial development stages only. // It is not recommended used it in a production environment. - // To diagnose, see - // https://github.com/Azure/azure-sdk-for-cpp/tree/main/sdk/identity/azure-identity#troubleshooting + // To diagnose, see https://aka.ms/azsdk/cpp/identity/troubleshooting auto defaultAzureCredential = std::make_shared(); diff --git a/sdk/identity/azure-identity/src/default_azure_credential.cpp b/sdk/identity/azure-identity/src/default_azure_credential.cpp index 3a77ebd10e..45826e0c7a 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -69,7 +69,6 @@ AccessToken DefaultAzureCredential::GetToken( throw AuthenticationException( "Failed to get token from " + GetCredentialName() + ".\nSee Azure::Core::Diagnostics::Logger for details " - "(https://github.com/Azure/azure-sdk-for-cpp/tree/main/sdk/" - "identity/azure-identity#troubleshooting)."); + "(https://aka.ms/azsdk/cpp/identity/troubleshooting)."); } } From acb8e3b9e67dc0aaca190f016990a3449de1bc1c Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Wed, 15 Mar 2023 17:00:01 -0700 Subject: [PATCH 04/20] Undocument ChainedCred usage by DefaultAzCred & remove friend and private ctor (#4447) * Undocument ChainedCred usage by DefaultAzCred & remove friend and private ctor * Clang warning fix --------- Co-authored-by: Anton Kolesnyk --- sdk/identity/azure-identity/CMakeLists.txt | 1 + sdk/identity/azure-identity/README.md | 2 +- .../identity/chained_token_credential.hpp | 13 ++-- .../identity/default_azure_credential.hpp | 10 +-- .../src/chained_token_credential.cpp | 64 ++++++++++--------- .../src/default_azure_credential.cpp | 13 ++-- .../private/chained_token_credential_impl.hpp | 25 ++++++++ .../test/ut/default_azure_credential_test.cpp | 11 ++-- 8 files changed, 80 insertions(+), 59 deletions(-) create mode 100644 sdk/identity/azure-identity/src/private/chained_token_credential_impl.hpp diff --git a/sdk/identity/azure-identity/CMakeLists.txt b/sdk/identity/azure-identity/CMakeLists.txt index ab2cd1b6b5..6d6e4ad7c5 100644 --- a/sdk/identity/azure-identity/CMakeLists.txt +++ b/sdk/identity/azure-identity/CMakeLists.txt @@ -62,6 +62,7 @@ set( set( AZURE_IDENTITY_SOURCE + src/private/chained_token_credential_impl.hpp src/private/managed_identity_source.hpp src/private/package_version.hpp src/private/token_credential_impl.hpp diff --git a/sdk/identity/azure-identity/README.md b/sdk/identity/azure-identity/README.md index 9d581e7547..802c7e3714 100644 --- a/sdk/identity/azure-identity/README.md +++ b/sdk/identity/azure-identity/README.md @@ -58,7 +58,7 @@ The `DefaultAzureCredential` attempts to authenticate via the following mechanis 1. **Azure CLI** - If the developer has authenticated an account via the Azure CLI `az login` command, the `DefaultAzureCredential` will authenticate with that account. 1. **Managed Identity** - If the application is deployed to an Azure host with Managed Identity enabled, the `DefaultAzureCredential` will authenticate with that account. -`DefaultAzureCredential` uses [`ChainedTokenCredential`](#chained-token-credential) that consists of a chain of `EnvironmentCredential`, `AzureCliCredential`, and `ManagedIdentityCredential`. Implementation, including the order in which credentials are applied is documented, but it may change from release to release. +Even though the credentials being used and their order is documented, it may change from release to release. `DefaultAzureCredential` intends to provide a credential that "just works out of the box and without requiring any information", if only the environment is set up sufficiently for the credential to work. Therefore, it could be simple to use, but since it uses a chain of credentials, it could be a bit complicated to diagnose if the environment setup is not sufficient. diff --git a/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp index 042942cb12..a4bd2d1050 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp @@ -15,7 +15,9 @@ #include namespace Azure { namespace Identity { - class DefaultAzureCredential; + namespace _detail { + class ChainedTokenCredentialImpl; + } /** * @brief Chained Token Credential provides a token credential implementation which chains @@ -24,10 +26,6 @@ namespace Azure { namespace Identity { * */ class ChainedTokenCredential final : public Core::Credentials::TokenCredential { - // Friend declaration is needed for DefaultAzureCredential to access ChainedTokenCredential's - // private constructor built to be used specifically by it. - friend class DefaultAzureCredential; - public: /** * @brief A container type to store the ordered chain of credentials. @@ -62,10 +60,7 @@ namespace Azure { namespace Identity { Core::Context const& context) const override; private: - explicit ChainedTokenCredential(Sources sources, std::string const& enclosingCredential); - - Sources m_sources; - std::string m_logPrefix; + std::unique_ptr<_detail::ChainedTokenCredentialImpl> m_impl; }; }} // namespace Azure::Identity diff --git a/sdk/identity/azure-identity/inc/azure/identity/default_azure_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/default_azure_credential.hpp index 6bde29f95e..1f58f185c9 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/default_azure_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/default_azure_credential.hpp @@ -8,13 +8,15 @@ #pragma once +#include #include -#include - #include namespace Azure { namespace Identity { + namespace _detail { + class ChainedTokenCredentialImpl; + } /** * @brief Default Azure Credential combines multiple credentials that depend on the setup @@ -22,7 +24,7 @@ namespace Azure { namespace Identity { * sufficiently for at least one of such credentials to work, `DefaultAzureCredential` will work * as well. * - * @details This credential is using the #ChainedTokenCredential of 3 credentials in the order: + * @details This credential is using several credentials in the following order: * #EnvironmentCredential, #AzureCliCredential, and #ManagedIdentityCredential. Even though the * credentials being used and their order is documented, it may be changed in the future versions * of the SDK, potentially bringing breaking changes in its behavior. @@ -68,7 +70,7 @@ namespace Azure { namespace Identity { Core::Context const& context) const override; private: - std::shared_ptr m_credentials; + std::unique_ptr<_detail::ChainedTokenCredentialImpl> m_impl; }; }} // namespace Azure::Identity diff --git a/sdk/identity/azure-identity/src/chained_token_credential.cpp b/sdk/identity/azure-identity/src/chained_token_credential.cpp index 116270b434..ee4b64cd03 100644 --- a/sdk/identity/azure-identity/src/chained_token_credential.cpp +++ b/sdk/identity/azure-identity/src/chained_token_credential.cpp @@ -2,36 +2,42 @@ // SPDX-License-Identifier: MIT #include "azure/identity/chained_token_credential.hpp" - #include "azure/core/internal/diagnostics/log.hpp" +#include "private/chained_token_credential_impl.hpp" #include using namespace Azure::Identity; +using namespace Azure::Identity::_detail; using namespace Azure::Core::Credentials; using Azure::Core::Context; using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; -namespace { -constexpr auto IdentityPrefix = "Identity: "; -} // namespace - ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources) - : ChainedTokenCredential(sources, {}) + : TokenCredential("ChainedTokenCredential"), + m_impl(std::make_unique(GetCredentialName(), std::move(sources))) { } -ChainedTokenCredential::ChainedTokenCredential( - ChainedTokenCredential::Sources sources, - std::string const& enclosingCredential) - : TokenCredential("ChainedTokenCredential"), m_sources(std::move(sources)) +ChainedTokenCredential::~ChainedTokenCredential() = default; + +AccessToken ChainedTokenCredential::GetToken( + TokenRequestContext const& tokenRequestContext, + Context const& context) const { - m_logPrefix = IdentityPrefix - + (enclosingCredential.empty() ? GetCredentialName() - : (enclosingCredential + " -> " + GetCredentialName())) - + ": "; + return m_impl->GetToken(GetCredentialName(), tokenRequestContext, context); +} + +namespace { +constexpr auto IdentityPrefix = "Identity: "; +} // namespace +ChainedTokenCredentialImpl::ChainedTokenCredentialImpl( + std::string const& credentialName, + ChainedTokenCredential::Sources&& sources) + : m_sources(std::move(sources)) +{ auto const logLevel = m_sources.empty() ? Logger::Level::Warning : Logger::Level::Informational; if (Log::ShouldWrite(logLevel)) { @@ -54,19 +60,12 @@ ChainedTokenCredential::ChainedTokenCredential( credSourceDetails += '.'; } - Log::Write( - logLevel, - IdentityPrefix - + (enclosingCredential.empty() - ? (GetCredentialName() + ": Created") - : (enclosingCredential + ": Created " + GetCredentialName())) - + credSourceDetails); + Log::Write(logLevel, IdentityPrefix + credentialName + ": Created" + credSourceDetails); } } -ChainedTokenCredential::~ChainedTokenCredential() = default; - -AccessToken ChainedTokenCredential::GetToken( +AccessToken ChainedTokenCredentialImpl::GetToken( + std::string const& credentialName, TokenRequestContext const& tokenRequestContext, Context const& context) const { @@ -78,7 +77,9 @@ AccessToken ChainedTokenCredential::GetToken( if (Log::ShouldWrite(logLevel)) { Log::Write( - logLevel, m_logPrefix + "Authentication did not succeed: List of sources is empty."); + logLevel, + IdentityPrefix + credentialName + + ": Authentication did not succeed: List of sources is empty."); } } else @@ -95,8 +96,8 @@ AccessToken ChainedTokenCredential::GetToken( { Log::Write( logLevel, - m_logPrefix + "Successfully got token from " + m_sources[i]->GetCredentialName() - + '.'); + IdentityPrefix + credentialName + ": Successfully got token from " + + m_sources[i]->GetCredentialName() + '.'); } } @@ -110,8 +111,8 @@ AccessToken ChainedTokenCredential::GetToken( { Log::Write( logLevel, - m_logPrefix + "Failed to get token from " + m_sources[i]->GetCredentialName() + ": " - + e.what()); + IdentityPrefix + credentialName + ": Failed to get token from " + + m_sources[i]->GetCredentialName() + ": " + e.what()); } } @@ -122,12 +123,13 @@ AccessToken ChainedTokenCredential::GetToken( { Log::Write( logLevel, - m_logPrefix + "Didn't succeed to get a token from any credential in the chain."); + IdentityPrefix + credentialName + + ": Didn't succeed to get a token from any credential in the chain."); } } } } } - throw AuthenticationException("Failed to get token from " + GetCredentialName() + '.'); + throw AuthenticationException("Failed to get token from " + credentialName + '.'); } diff --git a/sdk/identity/azure-identity/src/default_azure_credential.cpp b/sdk/identity/azure-identity/src/default_azure_credential.cpp index 45826e0c7a..4b4909192f 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -6,6 +6,7 @@ #include "azure/identity/azure_cli_credential.hpp" #include "azure/identity/environment_credential.hpp" #include "azure/identity/managed_identity_credential.hpp" +#include "private/chained_token_credential_impl.hpp" #include "azure/core/internal/diagnostics/log.hpp" @@ -31,8 +32,7 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt Log::Write( logLevel, std::string(IdentityPrefix) + "Creating " + GetCredentialName() - + " which combines mutiple parameterless credentials " - "into a single one (by using ChainedTokenCredential).\n" + + " which combines mutiple parameterless credentials into a single one.\n" + GetCredentialName() + " is only recommended for the early stages of development, " "and not for usage in production environment." @@ -48,10 +48,9 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt auto const azCliCred = std::make_shared(options); auto const managedIdentityCred = std::make_shared(options); - // Using the ChainedTokenCredential's private constructor for more detailed log messages. - m_credentials.reset(new ChainedTokenCredential( - ChainedTokenCredential::Sources{envCred, azCliCred, managedIdentityCred}, - GetCredentialName())); // extra arg for the ChainedTokenCredential's private constructor. + m_impl = std::make_unique<_detail::ChainedTokenCredentialImpl>( + GetCredentialName(), + ChainedTokenCredential::Sources{envCred, azCliCred, managedIdentityCred}); } DefaultAzureCredential::~DefaultAzureCredential() = default; @@ -62,7 +61,7 @@ AccessToken DefaultAzureCredential::GetToken( { try { - return m_credentials->GetToken(tokenRequestContext, context); + return m_impl->GetToken(GetCredentialName(), tokenRequestContext, context); } catch (AuthenticationException const&) { diff --git a/sdk/identity/azure-identity/src/private/chained_token_credential_impl.hpp b/sdk/identity/azure-identity/src/private/chained_token_credential_impl.hpp new file mode 100644 index 0000000000..177d011a9e --- /dev/null +++ b/sdk/identity/azure-identity/src/private/chained_token_credential_impl.hpp @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#pragma once + +#include "azure/identity/chained_token_credential.hpp" + +namespace Azure { namespace Identity { namespace _detail { + + class ChainedTokenCredentialImpl final { + public: + ChainedTokenCredentialImpl( + std::string const& credentialName, + ChainedTokenCredential::Sources&& sources); + + Core::Credentials::AccessToken GetToken( + std::string const& credentialName, + Core::Credentials::TokenRequestContext const& tokenRequestContext, + Core::Context const& context) const; + + private: + ChainedTokenCredential::Sources m_sources; + }; + +}}} // namespace Azure::Identity::_detail diff --git a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp index ba9a4afa5f..d3498d6cc7 100644 --- a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp @@ -74,9 +74,8 @@ TEST(DefaultAzureCredential, LogMessages) EXPECT_EQ(log[0].first, Logger::Level::Verbose); EXPECT_EQ( log[0].second, - "Identity: Creating DefaultAzureCredential which combines mutiple parameterless " - "credentials " - "into a single one (by using ChainedTokenCredential)." + "Identity: Creating DefaultAzureCredential which combines " + "mutiple parameterless credentials into a single one." "\nDefaultAzureCredential is only recommended for the early stages of development, " "and not for usage in production environment." "\nOnce the developer focuses on the Credentials and Authentication aspects of their " @@ -131,8 +130,7 @@ TEST(DefaultAzureCredential, LogMessages) EXPECT_EQ(log[8].first, Logger::Level::Informational); EXPECT_EQ( log[8].second, - "Identity: DefaultAzureCredential: Created ChainedTokenCredential " - "with the following credentials: " + "Identity: DefaultAzureCredential: Created with the following credentials: " "EnvironmentCredential, AzureCliCredential, ManagedIdentityCredential."); log.clear(); @@ -149,8 +147,7 @@ TEST(DefaultAzureCredential, LogMessages) EXPECT_EQ(log[3].first, Logger::Level::Informational); EXPECT_EQ( log[3].second, - "Identity: DefaultAzureCredential -> ChainedTokenCredential: " - "Successfully got token from EnvironmentCredential."); + "Identity: DefaultAzureCredential: Successfully got token from EnvironmentCredential."); Logger::SetListener(nullptr); } From f8ae5d9d8b641aefcdd664c2c3cf204a072d3934 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Wed, 15 Mar 2023 20:21:39 -0700 Subject: [PATCH 05/20] Sync eng/common directory with azure-sdk-tools for PR 5691 (#4450) * typespec renaming * add back scripts for cadl * support .cadl and .tsp * rename * add newline at the end of file --------- Co-authored-by: FAREAST\chunyu --- .../scripts/TypeSpec-Project-Generate.ps1 | 101 ++++++++++++++ eng/common/scripts/TypeSpec-Project-Sync.ps1 | 127 ++++++++++++++++++ 2 files changed, 228 insertions(+) create mode 100644 eng/common/scripts/TypeSpec-Project-Generate.ps1 create mode 100644 eng/common/scripts/TypeSpec-Project-Sync.ps1 diff --git a/eng/common/scripts/TypeSpec-Project-Generate.ps1 b/eng/common/scripts/TypeSpec-Project-Generate.ps1 new file mode 100644 index 0000000000..feba00d37e --- /dev/null +++ b/eng/common/scripts/TypeSpec-Project-Generate.ps1 @@ -0,0 +1,101 @@ +# For details see https://github.com/Azure/azure-sdk-tools/blob/main/doc/common/TypeSpec-Project-Scripts.md + +[CmdletBinding()] +param ( + [Parameter(Position=0)] + [ValidateNotNullOrEmpty()] + [string] $ProjectDirectory, + [Parameter(Position=1)] + [string] $typespecAdditionalOptions ## addtional typespec emitter options, separated by semicolon if more than one, e.g. option1=value1;option2=value2 +) + +$ErrorActionPreference = "Stop" +. $PSScriptRoot/Helpers/PSModule-Helpers.ps1 +. $PSScriptRoot/common.ps1 +Install-ModuleIfNotInstalled "powershell-yaml" "0.4.1" | Import-Module + +function NpmInstallForProject([string]$workingDirectory) { + Push-Location $workingDirectory + try { + $currentDur = Resolve-Path "." + Write-Host "Generating from $currentDur" + + if (Test-Path "package.json") { + Remove-Item -Path "package.json" -Force + } + + if (Test-Path ".npmrc") { + Remove-Item -Path ".npmrc" -Force + } + + if (Test-Path "node_modules") { + Remove-Item -Path "node_modules" -Force -Recurse + } + + if (Test-Path "package-lock.json") { + Remove-Item -Path "package-lock.json" -Force + } + + #default to root/eng/emitter-package.json but you can override by writing + #Get-${Language}-EmitterPackageJsonPath in your Language-Settings.ps1 + $replacementPackageJson = "$PSScriptRoot/../../emitter-package.json" + if (Test-Path "Function:$GetEmitterPackageJsonPathFn") { + $replacementPackageJson = &$GetEmitterPackageJsonPathFn + } + + Write-Host("Copying package.json from $replacementPackageJson") + Copy-Item -Path $replacementPackageJson -Destination "package.json" -Force + npm install --no-lock-file + if ($LASTEXITCODE) { exit $LASTEXITCODE } + } + finally { + Pop-Location + } +} + +$resolvedProjectDirectory = Resolve-Path $ProjectDirectory +$emitterName = &$GetEmitterNameFn +$typespecConfigurationFile = Resolve-Path "$ProjectDirectory/tsp-location.yaml" + +Write-Host "Reading configuration from $typespecConfigurationFile" +$configuration = Get-Content -Path $typespecConfigurationFile -Raw | ConvertFrom-Yaml + +$specSubDirectory = $configuration["directory"] +$innerFolder = Split-Path $specSubDirectory -Leaf + +$tempFolder = "$ProjectDirectory/TempTypeSpecFiles" +$npmWorkingDir = Resolve-Path $tempFolder/$innerFolder +$mainTypeSpecFile = If (Test-Path "$npmWorkingDir/client.*") { Resolve-Path "$npmWorkingDir/client.*" } Else { Resolve-Path "$npmWorkingDir/main.*"} + +try { + Push-Location $npmWorkingDir + NpmInstallForProject $npmWorkingDir + + if ($LASTEXITCODE) { exit $LASTEXITCODE } + + if (Test-Path "Function:$GetEmitterAdditionalOptionsFn") { + $emitterAdditionalOptions = &$GetEmitterAdditionalOptionsFn $resolvedProjectDirectory + if ($emitterAdditionalOptions.Length -gt 0) { + $emitterAdditionalOptions = " $emitterAdditionalOptions" + } + } + $typespecCompileCommand = "npx tsp compile $mainTypeSpecFile --emit $emitterName$emitterAdditionalOptions" + if ($typespecAdditionalOptions) { + $options = $typespecAdditionalOptions.Split(";"); + foreach ($option in $options) { + $typespecCompileCommand += " --option $emitterName.$option" + } + } + Write-Host($typespecCompileCommand) + Invoke-Expression $typespecCompileCommand + + if ($LASTEXITCODE) { exit $LASTEXITCODE } +} +finally { + Pop-Location +} + +$shouldCleanUp = $configuration["cleanup"] ?? $true +if ($shouldCleanUp) { + Remove-Item $tempFolder -Recurse -Force +} diff --git a/eng/common/scripts/TypeSpec-Project-Sync.ps1 b/eng/common/scripts/TypeSpec-Project-Sync.ps1 new file mode 100644 index 0000000000..f8656bc1d6 --- /dev/null +++ b/eng/common/scripts/TypeSpec-Project-Sync.ps1 @@ -0,0 +1,127 @@ +# For details see https://github.com/Azure/azure-sdk-tools/blob/main/doc/common/TypeSpec-Project-Scripts.md + +[CmdletBinding()] +param ( + [Parameter(Position=0)] + [ValidateNotNullOrEmpty()] + [string] $ProjectDirectory +) + +$ErrorActionPreference = "Stop" +. $PSScriptRoot/Helpers/PSModule-Helpers.ps1 +Install-ModuleIfNotInstalled "powershell-yaml" "0.4.1" | Import-Module +$sparseCheckoutFile = ".git/info/sparse-checkout" + +function AddSparseCheckoutPath([string]$subDirectory) { + if (!(Test-Path $sparseCheckoutFile) -or !((Get-Content $sparseCheckoutFile).Contains($subDirectory))) { + Write-Output $subDirectory >> .git/info/sparse-checkout + } +} + +function CopySpecToProjectIfNeeded([string]$specCloneRoot, [string]$mainSpecDir, [string]$dest, [string[]]$specAdditionalSubDirectories) { + $source = "$specCloneRoot/$mainSpecDir" + Copy-Item -Path $source -Destination $dest -Recurse -Force + Write-Host "Copying spec from $source to $dest" + + foreach ($additionalDir in $specAdditionalSubDirectories) { + $source = "$specCloneRoot/$additionalDir" + Write-Host "Copying spec from $source to $dest" + Copy-Item -Path $source -Destination $dest -Recurse -Force + } +} + +function UpdateSparseCheckoutFile([string]$mainSpecDir, [string[]]$specAdditionalSubDirectories) { + AddSparseCheckoutPath $mainSpecDir + foreach ($subDir in $specAdditionalSubDirectories) { + AddSparseCheckoutPath $subDir + } +} + +function GetGitRemoteValue([string]$repo) { + Push-Location $ProjectDirectory + $result = "" + try { + $gitRemotes = (git remote -v) + foreach ($remote in $gitRemotes) { + if ($remote.StartsWith("origin")) { + if ($remote -match 'https://github.com/\S+') { + $result = "https://github.com/$repo.git" + break + } elseif ($remote -match "git@github.com:\S+"){ + $result = "git@github.com:$repo.git" + break + } else { + throw "Unknown git remote format found: $remote" + } + } + } + } + finally { + Pop-Location + } + + return $result +} + +function InitializeSparseGitClone([string]$repo) { + git clone --no-checkout --filter=tree:0 $repo . + if ($LASTEXITCODE) { exit $LASTEXITCODE } + git sparse-checkout init + if ($LASTEXITCODE) { exit $LASTEXITCODE } + Remove-Item $sparseCheckoutFile -Force +} + +function GetSpecCloneDir([string]$projectName) { + Push-Location $ProjectDirectory + try { + $root = git rev-parse --show-toplevel + } + finally { + Pop-Location + } + + $sparseSpecCloneDir = "$root/../sparse-spec/$projectName" + New-Item $sparseSpecCloneDir -Type Directory -Force | Out-Null + $createResult = Resolve-Path $sparseSpecCloneDir + return $createResult +} + +$typespecConfigurationFile = Resolve-Path "$ProjectDirectory/tsp-location.yaml" +Write-Host "Reading configuration from $typespecConfigurationFile" +$configuration = Get-Content -Path $typespecConfigurationFile -Raw | ConvertFrom-Yaml + +$pieces = $typespecConfigurationFile.Path.Replace("\","/").Split("/") +$projectName = $pieces[$pieces.Count - 2] + +$specSubDirectory = $configuration["directory"] + +if ( $configuration["repo"] -and $configuration["commit"]) { + $specCloneDir = GetSpecCloneDir $projectName + $gitRemoteValue = GetGitRemoteValue $configuration["repo"] + + Write-Host "Setting up sparse clone for $projectName at $specCloneDir" + + Push-Location $specCloneDir.Path + try { + if (!(Test-Path ".git")) { + InitializeSparseGitClone $gitRemoteValue + UpdateSparseCheckoutFile $specSubDirectory $configuration["additionalDirectories"] + } + git checkout $configuration["commit"] + if ($LASTEXITCODE) { exit $LASTEXITCODE } + } + finally { + Pop-Location + } +} elseif ( $configuration["spec-root-dir"] ) { + $specCloneDir = $configuration["spec-root-dir"] +} + + +$tempTypeSpecDir = "$ProjectDirectory/TempTypeSpecFiles" +New-Item $tempTypeSpecDir -Type Directory -Force | Out-Null +CopySpecToProjectIfNeeded ` + -specCloneRoot $specCloneDir ` + -mainSpecDir $specSubDirectory ` + -dest $tempTypeSpecDir ` + -specAdditionalSubDirectories $configuration["additionalDirectories"] From 14677e92be8dc2dab97ecfdbcae9bd0d3e1da894 Mon Sep 17 00:00:00 2001 From: George Arama <50641385+gearama@users.noreply.github.com> Date: Thu, 16 Mar 2023 11:03:09 -0700 Subject: [PATCH 06/20] fix cspell for readme.ms in libcurl sterss test (#4441) * fix cspell * capitalize Valgrind and ubuntu * sdada * fix2 --- .vscode/cspell.json | 1 - .../test/libcurl-stress-test/README.md | 16 ++++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index c76dbcf56e..6f3066efc3 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -12,7 +12,6 @@ "*.a", "*.lib", "*.yaml", - "**/libcurl-stress-test/README.md", ".github/CODEOWNERS", ".gitignore", ".vscode/cspell.json", diff --git a/sdk/core/azure-core/test/libcurl-stress-test/README.md b/sdk/core/azure-core/test/libcurl-stress-test/README.md index 300534d165..fe10a6a79b 100644 --- a/sdk/core/azure-core/test/libcurl-stress-test/README.md +++ b/sdk/core/azure-core/test/libcurl-stress-test/README.md @@ -5,24 +5,24 @@ This is work in progress. It's a prototype of how a stress test would look. This The cpp file represents the code for the test, it will generate a number of invalid URLs and then issue CURL send commands. The requests are expected to fail. The point was that it exposes memory leaks in handling the error cases, which we fixed since. ### Dockerfile (https://www.docker.com/) -Represents the build file for the container in which the test runs, it is based on ubuntu 22.04 , from mcr. -The main change from default ubuntu is making sure we have the valgrind tool installed. Valgrind is a heap monitoring tool that helps identify potential stack traces that might leak memory. While not 100% effective is is great at reducing the surface are for investigations. +Represents the build file for the container in which the test runs, it is based on Ubuntu 22.04 , from MCR. +The main change from default Ubuntu is making sure we have the Valgrind tool installed. Valgrind is a heap monitoring tool that helps identify potential stack traces that might leak memory. While not 100% effective is great at reducing the surface are for investigations. ### Helm chart (https://helm.sh/) Chart.yaml together with the bicep file(https://docs.microsoft.com/azure/azure-resource-manager/bicep/overview?tabs=bicep) and the deploy job file , represent the helm chart needed to deploy to the docker image built from the dockerfile to the stress cluster and execute the stress test. -The helm chart creates a pod with a container based on the docker image, and executes the test under valgrind. +The helm chart creates a pod with a container based on the docker image, and executes the test under Valgrind. To deploy the chart you will need to run "azure-sdk-for-cpp\eng\common\scripts\stress-testing> .\deploy-stress-tests.ps1 -Namespace azuresdkforcpp -SearchDirectory E:\src\azure-sdk-for-cpp\sdk\core\azure-core\test -PushImage" -Where namaspace will be created if missing , search directory can be any folder where it will search for charts in it and all it's sub dirs, push image will call it to build the docker image. +Where namespace will be created if missing , search directory can be any folder where it will search for charts in it and all it's sub dirs, push image will call it to build the docker image. -ATM the docker image is build by hand and harcoded in the chart to simplify matters. +ATM the docker image is build by hand and hard-coded in the chart to simplify matters. -To build the image run "docker build -t stresstesttbiruti6oi24k.acr.io/azuresdkforcpp/curlstress:v8 --build-arg targetTest=azure-core-libcurl-stress-test --build-arg build=on ." +To build the image run "docker build -t /azuresdkforcpp/curlstress:v8 --build-arg targetTest=azure-core-libcurl-stress-test --build-arg build=on ." -To push to mcr : "docker push stresstesttbiruti6oi24k.acr.io/azuresdkforcpp/curlstress:v8" -Obviously after logging in to the acr "az acr login -n stresspgs7b6dif73rup6.azurecr.io" +To push to mcr : "docker push /azuresdkforcpp/curlstress:v8" +Obviously after logging in to the acr "az acr login -n " To use another image you will need to go to line 12 in deploy job and update with your new file. From 83f736d8ad97987f461408bc32b4f8e56533e700 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Thu, 16 Mar 2023 12:50:15 -0700 Subject: [PATCH 07/20] Simpler identity logging (#4455) * Simpler identity logging * Even simpler * Remove refactoring artifact * Cosmetic change * foreach --------- Co-authored-by: Anton Kolesnyk --- .../src/azure_cli_credential.cpp | 21 ++--- .../src/chained_token_credential.cpp | 75 +++++------------- .../src/default_azure_credential.cpp | 29 ++++--- .../src/environment_credential.cpp | 73 +++++++----------- .../src/managed_identity_source.cpp | 43 ++++------- .../test/ut/default_azure_credential_test.cpp | 37 +++++---- .../test/ut/environment_credential_test.cpp | 76 +++++++++++++------ 7 files changed, 157 insertions(+), 197 deletions(-) diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index f2c631adc1..f02c32f7e1 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -91,15 +91,11 @@ AzureCliCredential::AzureCliCredential( ThrowIfNotSafeCmdLineInput(m_tenantId, "TenantID"); - auto const logLevel = Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + GetCredentialName() - + " created.\n" - "Successful creation does not guarantee further successful token retrieval."); - } + Log::Write( + Logger::Level::Informational, + IdentityPrefix + GetCredentialName() + + " created.\n" + "Successful creation does not guarantee further successful token retrieval."); } AzureCliCredential::AzureCliCredential(AzureCliCredentialOptions const& options) @@ -168,12 +164,7 @@ AccessToken AzureCliCredential::GetToken( auto const errorMsg = IdentityPrefix + GetCredentialName() + " didn't get the token: \"" + e.what() + '\"'; - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write(logLevel, errorMsg); - } - + Log::Write(Logger::Level::Warning, errorMsg); throw AuthenticationException(errorMsg); } }); diff --git a/sdk/identity/azure-identity/src/chained_token_credential.cpp b/sdk/identity/azure-identity/src/chained_token_credential.cpp index ee4b64cd03..e30774e358 100644 --- a/sdk/identity/azure-identity/src/chained_token_credential.cpp +++ b/sdk/identity/azure-identity/src/chained_token_credential.cpp @@ -69,67 +69,34 @@ AccessToken ChainedTokenCredentialImpl::GetToken( TokenRequestContext const& tokenRequestContext, Context const& context) const { - auto const sourcesSize = m_sources.size(); - - if (sourcesSize == 0) + for (auto const& source : m_sources) { - const auto logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) + try { + auto token = source->GetToken(tokenRequestContext, context); + Log::Write( - logLevel, - IdentityPrefix + credentialName - + ": Authentication did not succeed: List of sources is empty."); + Logger::Level::Informational, + IdentityPrefix + credentialName + ": Successfully got token from " + + source->GetCredentialName() + '.'); + + return token; } - } - else - { - for (size_t i = 0; i < sourcesSize; ++i) + catch (AuthenticationException const& e) { - try - { - auto token = m_sources[i]->GetToken(tokenRequestContext, context); - - { - auto const logLevel = Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + credentialName + ": Successfully got token from " - + m_sources[i]->GetCredentialName() + '.'); - } - } - - return token; - } - catch (AuthenticationException const& e) - { - { - auto const logLevel = Logger::Level::Verbose; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + credentialName + ": Failed to get token from " - + m_sources[i]->GetCredentialName() + ": " + e.what()); - } - } - - if ((sourcesSize - 1) == i) // On the last credential - { - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + credentialName - + ": Didn't succeed to get a token from any credential in the chain."); - } - } - } + Log::Write( + Logger::Level::Verbose, + IdentityPrefix + credentialName + ": Failed to get token from " + + source->GetCredentialName() + ": " + e.what()); } } + Log::Write( + Logger::Level::Warning, + IdentityPrefix + credentialName + + (m_sources.empty() + ? ": Authentication did not succeed: List of sources is empty." + : ": Didn't succeed to get a token from any credential in the chain.")); + throw AuthenticationException("Failed to get token from " + credentialName + '.'); } diff --git a/sdk/identity/azure-identity/src/default_azure_credential.cpp b/sdk/identity/azure-identity/src/default_azure_credential.cpp index 4b4909192f..d50c2a6e73 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -26,22 +26,19 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt { // Initializing m_credential below and not in the member initializer list to have a specific order // of log messages. - auto const logLevel = Logger::Level::Verbose; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - std::string(IdentityPrefix) + "Creating " + GetCredentialName() - + " which combines mutiple parameterless credentials into a single one.\n" - + GetCredentialName() - + " is only recommended for the early stages of development, " - "and not for usage in production environment." - "\nOnce the developer focuses on the Credentials and Authentication aspects " - "of their application, " - + GetCredentialName() - + " needs to be replaced with the credential that " - "is the better fit for the application."); - } + + Log::Write( + Logger::Level::Verbose, + std::string(IdentityPrefix) + "Creating " + GetCredentialName() + + " which combines mutiple parameterless credentials into a single one.\n" + + GetCredentialName() + + " is only recommended for the early stages of development, " + "and not for usage in production environment." + "\nOnce the developer focuses on the Credentials and Authentication aspects " + "of their application, " + + GetCredentialName() + + " needs to be replaced with the credential that " + "is the better fit for the application."); // Creating credentials in order to ensure the order of log messages. auto const envCred = std::make_shared(options); diff --git a/sdk/identity/azure-identity/src/environment_credential.cpp b/sdk/identity/azure-identity/src/environment_credential.cpp index f6dd764149..7d96727da6 100644 --- a/sdk/identity/azure-identity/src/environment_credential.cpp +++ b/sdk/identity/azure-identity/src/environment_credential.cpp @@ -129,38 +129,32 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!m_credentialImpl) { - auto const logLevel = Logger::Level::Warning; + Log::Write( + Logger::Level::Warning, logMsgPrefix + " was not initialized with underlying credential."); + + auto const logLevel = Logger::Level::Verbose; if (Log::ShouldWrite(logLevel)) { - auto const basicMessage = logMsgPrefix + " was not initialized with underlying credential"; - - if (!Log::ShouldWrite(Logger::Level::Verbose)) - { - Log::Write(logLevel, basicMessage + '.'); - } - else + auto logMsg = logMsgPrefix + ": Both '" + AzureTenantIdEnvVarName + "' and '" + + AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName + + "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '" + + AzureAuthorityHostEnvVarName + + "' could be set to override the default authority host. Currently:\n"; + + std::pair envVarStatus[] = { + {AzureTenantIdEnvVarName, !tenantId.empty()}, + {AzureClientIdEnvVarName, !clientId.empty()}, + {AzureClientSecretEnvVarName, !clientSecret.empty()}, + {AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()}, + {AzureAuthorityHostEnvVarName, !authority.empty()}, + }; + for (auto const& status : envVarStatus) { - auto logMsg = basicMessage + ": Both '" + AzureTenantIdEnvVarName + "' and '" - + AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName - + "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '" - + AzureAuthorityHostEnvVarName - + "' could be set to override the default authority host. Currently:\n"; - - std::pair envVarStatus[] = { - {AzureTenantIdEnvVarName, !tenantId.empty()}, - {AzureClientIdEnvVarName, !clientId.empty()}, - {AzureClientSecretEnvVarName, !clientSecret.empty()}, - {AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()}, - {AzureAuthorityHostEnvVarName, !authority.empty()}, - }; - for (auto const& status : envVarStatus) - { - logMsg += std::string(" * '") + status.first + "' " + "is" - + (status.second ? " " : " NOT ") + "set\n"; - } - - Log::Write(logLevel, logMsg); + logMsg += std::string(" * '") + status.first + "' " + "is" + (status.second ? " " : " NOT ") + + "set\n"; } + + Log::Write(logLevel, logMsg); } } } @@ -174,15 +168,9 @@ AccessToken EnvironmentCredential::GetToken( auto const AuthUnavailable = IdentityPrefix + GetCredentialName() + " authentication unavailable. "; - { - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); - } - } + Log::Write( + Logger::Level::Warning, + AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); throw AuthenticationException( AuthUnavailable + "Environment variables are not fully configured."); @@ -197,15 +185,12 @@ void PrintCredentialCreationLogMessage( std::vector> const& envVarsToParams, char const* credThatGetsCreated) { + Log::Write( + Logger::Level::Informational, + logMsgPrefix + " gets created with " + credThatGetsCreated + '.'); + if (!Log::ShouldWrite(Logger::Level::Verbose)) { - if (Log::ShouldWrite(Logger::Level::Informational)) - { - Log::Write( - Logger::Level::Informational, - logMsgPrefix + " gets created with " + credThatGetsCreated + '.'); - } - return; } diff --git a/sdk/identity/azure-identity/src/managed_identity_source.cpp b/sdk/identity/azure-identity/src/managed_identity_source.cpp index 89dfa0a773..daea912fe3 100644 --- a/sdk/identity/azure-identity/src/managed_identity_source.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_source.cpp @@ -28,14 +28,10 @@ std::string WithSourceMessage(std::string const& credSource) void PrintEnvNotSetUpMessage(std::string const& credName, std::string const& credSource) { - auto const logLevel = Logger::Level::Verbose; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + credName + ": Environment is not set up for the credential to be created" - + WithSourceMessage(credSource) + '.'); - } + Log::Write( + Logger::Level::Verbose, + IdentityPrefix + credName + ": Environment is not set up for the credential to be created" + + WithSourceMessage(credSource) + '.'); } } // namespace @@ -52,13 +48,9 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( { auto const endpointUrl = Url(url); - auto const logLevel = Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.'); - } + Log::Write( + Logger::Level::Informational, + IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.'); return endpointUrl; } @@ -73,12 +65,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( + ": Failed to create: The environment variable \'" + envVarName + "\' contains an invalid URL."; - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write(logLevel, IdentityPrefix + errorMessage); - } - + Log::Write(Logger::Level::Warning, IdentityPrefix + errorMessage); throw AuthenticationException(errorMessage); } @@ -385,15 +372,11 @@ std::unique_ptr ImdsManagedIdentitySource::Create( std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { - auto const logLevel = Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + credName + " will be created" - + WithSourceMessage("Azure Instance Metadata Service") - + ".\nSuccessful creation does not guarantee further successful token retrieval."); - } + Log::Write( + Logger::Level::Informational, + IdentityPrefix + credName + " will be created" + + WithSourceMessage("Azure Instance Metadata Service") + + ".\nSuccessful creation does not guarantee further successful token retrieval."); return std::unique_ptr(new ImdsManagedIdentitySource(clientId, options)); } diff --git a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp index d3498d6cc7..7ab26d4b50 100644 --- a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp @@ -69,7 +69,7 @@ TEST(DefaultAzureCredential, LogMessages) auto credential = std::make_unique(options); - EXPECT_EQ(log.size(), LogMsgVec::size_type(9)); + EXPECT_EQ(log.size(), LogMsgVec::size_type(10)); EXPECT_EQ(log[0].first, Logger::Level::Verbose); EXPECT_EQ( @@ -82,54 +82,59 @@ TEST(DefaultAzureCredential, LogMessages) "application, DefaultAzureCredential needs to be replaced with the credential that " "is the better fit for the application."); - EXPECT_EQ(log[1].first, Logger::Level::Verbose); + EXPECT_EQ(log[1].first, Logger::Level::Informational); EXPECT_EQ( log[1].second, + "Identity: EnvironmentCredential gets created with ClientSecretCredential."); + + EXPECT_EQ(log[2].first, Logger::Level::Verbose); + EXPECT_EQ( + log[2].second, "Identity: EnvironmentCredential: 'AZURE_TENANT_ID', 'AZURE_CLIENT_ID', " "'AZURE_CLIENT_SECRET', and 'AZURE_AUTHORITY_HOST' environment variables are set, so " "ClientSecretCredential with corresponding tenantId, clientId, clientSecret, and " "authorityHost gets created."); - EXPECT_EQ(log[2].first, Logger::Level::Informational); + EXPECT_EQ(log[3].first, Logger::Level::Informational); EXPECT_EQ( - log[2].second, + log[3].second, "Identity: AzureCliCredential created." "\nSuccessful creation does not guarantee further successful token retrieval."); - EXPECT_EQ(log[3].first, Logger::Level::Verbose); - EXPECT_EQ( - log[3].second, - "Identity: ManagedIdentityCredential: Environment is not set up for the credential " - "to be created with App Service 2019 source."); - EXPECT_EQ(log[4].first, Logger::Level::Verbose); EXPECT_EQ( log[4].second, "Identity: ManagedIdentityCredential: Environment is not set up for the credential " - "to be created with App Service 2017 source."); + "to be created with App Service 2019 source."); EXPECT_EQ(log[5].first, Logger::Level::Verbose); EXPECT_EQ( log[5].second, "Identity: ManagedIdentityCredential: Environment is not set up for the credential " - "to be created with Cloud Shell source."); + "to be created with App Service 2017 source."); EXPECT_EQ(log[6].first, Logger::Level::Verbose); EXPECT_EQ( log[6].second, "Identity: ManagedIdentityCredential: Environment is not set up for the credential " - "to be created with Azure Arc source."); + "to be created with Cloud Shell source."); - EXPECT_EQ(log[7].first, Logger::Level::Informational); + EXPECT_EQ(log[7].first, Logger::Level::Verbose); EXPECT_EQ( log[7].second, + "Identity: ManagedIdentityCredential: Environment is not set up for the credential " + "to be created with Azure Arc source."); + + EXPECT_EQ(log[8].first, Logger::Level::Informational); + EXPECT_EQ( + log[8].second, "Identity: ManagedIdentityCredential will be created " "with Azure Instance Metadata Service source." "\nSuccessful creation does not guarantee further successful token retrieval."); - EXPECT_EQ(log[8].first, Logger::Level::Informational); + EXPECT_EQ(log[9].first, Logger::Level::Informational); EXPECT_EQ( - log[8].second, + log[9].second, "Identity: DefaultAzureCredential: Created with the following credentials: " "EnvironmentCredential, AzureCliCredential, ManagedIdentityCredential."); diff --git a/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp b/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp index 61066e7c2b..71f4b6c063 100644 --- a/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp @@ -53,14 +53,21 @@ TEST(EnvironmentCredential, RegularClientSecretCredential) auto credential = std::make_unique(options); - EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); - EXPECT_EQ(log[0].first, Logger::Level::Verbose); + EXPECT_EQ(log.size(), LogMsgVec::size_type(2)); + + EXPECT_EQ(log[0].first, Logger::Level::Informational); EXPECT_EQ( log[0].second, + "Identity: EnvironmentCredential gets created with ClientSecretCredential."); + + EXPECT_EQ(log[1].first, Logger::Level::Verbose); + EXPECT_EQ( + log[1].second, "Identity: EnvironmentCredential: 'AZURE_TENANT_ID', 'AZURE_CLIENT_ID', " "'AZURE_CLIENT_SECRET', and 'AZURE_AUTHORITY_HOST' environment variables are set, so " "ClientSecretCredential with corresponding tenantId, clientId, clientSecret, and " "authorityHost gets created."); + log.clear(); return credential; @@ -202,20 +209,26 @@ TEST(EnvironmentCredential, Unavailable) auto credential = std::make_unique(options); - EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); + EXPECT_EQ(log.size(), LogMsgVec::size_type(2)); + EXPECT_EQ(log[0].first, Logger::Level::Warning); EXPECT_EQ( log[0].second, - "Identity: EnvironmentCredential was not initialized with underlying credential: Both " - "'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID', and at least one of 'AZURE_CLIENT_SECRET', " - "'AZURE_CLIENT_CERTIFICATE_PATH' needs to be set. Additionally, " - "'AZURE_AUTHORITY_HOST' could be set to override the default authority host." - " Currently:\n" + "Identity: EnvironmentCredential was not initialized with underlying credential."); + + EXPECT_EQ(log[1].first, Logger::Level::Verbose); + EXPECT_EQ( + log[1].second, + "Identity: EnvironmentCredential: Both 'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID'," + " and at least one of 'AZURE_CLIENT_SECRET', 'AZURE_CLIENT_CERTIFICATE_PATH'" + " needs to be set. Additionally, 'AZURE_AUTHORITY_HOST' could be set" + " to override the default authority host. Currently:\n" " * 'AZURE_TENANT_ID' is NOT set\n" " * 'AZURE_CLIENT_ID' is NOT set\n" " * 'AZURE_CLIENT_SECRET' is NOT set\n" " * 'AZURE_CLIENT_CERTIFICATE_PATH' is NOT set\n" " * 'AZURE_AUTHORITY_HOST' is NOT set\n"); + log.clear(); return credential; @@ -265,13 +278,20 @@ TEST(EnvironmentCredential, ClientSecretDefaultAuthority) auto credential = std::make_unique(options); - EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); - EXPECT_EQ(log[0].first, Logger::Level::Verbose); + EXPECT_EQ(log.size(), LogMsgVec::size_type(2)); + + EXPECT_EQ(log[0].first, Logger::Level::Informational); EXPECT_EQ( log[0].second, + "Identity: EnvironmentCredential gets created with ClientSecretCredential."); + + EXPECT_EQ(log[1].first, Logger::Level::Verbose); + EXPECT_EQ( + log[1].second, "Identity: EnvironmentCredential: 'AZURE_TENANT_ID', 'AZURE_CLIENT_ID', and " "'AZURE_CLIENT_SECRET' environment variables are set, so ClientSecretCredential with " "corresponding tenantId, clientId, and clientSecret gets created."); + log.clear(); return credential; @@ -342,20 +362,26 @@ TEST(EnvironmentCredential, ClientSecretNoTenantId) auto credential = std::make_unique(options); - EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); + EXPECT_EQ(log.size(), LogMsgVec::size_type(2)); + EXPECT_EQ(log[0].first, Logger::Level::Warning); EXPECT_EQ( log[0].second, - "Identity: EnvironmentCredential was not initialized with underlying credential: Both " - "'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID', and at least one of 'AZURE_CLIENT_SECRET', " - "'AZURE_CLIENT_CERTIFICATE_PATH' needs to be set. Additionally, " - "'AZURE_AUTHORITY_HOST' could be set to override the default authority host." - " Currently:\n" + "Identity: EnvironmentCredential was not initialized with underlying credential."); + + EXPECT_EQ(log[1].first, Logger::Level::Verbose); + EXPECT_EQ( + log[1].second, + "Identity: EnvironmentCredential: Both 'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID'," + " and at least one of 'AZURE_CLIENT_SECRET', 'AZURE_CLIENT_CERTIFICATE_PATH'" + " needs to be set. Additionally, 'AZURE_AUTHORITY_HOST' could be set" + " to override the default authority host. Currently:\n" " * 'AZURE_TENANT_ID' is NOT set\n" " * 'AZURE_CLIENT_ID' is set\n" " * 'AZURE_CLIENT_SECRET' is set\n" " * 'AZURE_CLIENT_CERTIFICATE_PATH' is NOT set\n" " * 'AZURE_AUTHORITY_HOST' is set\n"); + log.clear(); return credential; @@ -466,20 +492,26 @@ TEST(EnvironmentCredential, ClientSecretNoClientSecret) auto credential = std::make_unique(options); - EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); + EXPECT_EQ(log.size(), LogMsgVec::size_type(2)); + EXPECT_EQ(log[0].first, Logger::Level::Warning); EXPECT_EQ( log[0].second, - "Identity: EnvironmentCredential was not initialized with underlying credential: Both " - "'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID', and at least one of 'AZURE_CLIENT_SECRET', " - "'AZURE_CLIENT_CERTIFICATE_PATH' needs to be set. Additionally, " - "'AZURE_AUTHORITY_HOST' could be set to override the default authority host." - " Currently:\n" + "Identity: EnvironmentCredential was not initialized with underlying credential."); + + EXPECT_EQ(log[1].first, Logger::Level::Verbose); + EXPECT_EQ( + log[1].second, + "Identity: EnvironmentCredential: Both 'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID'," + " and at least one of 'AZURE_CLIENT_SECRET', 'AZURE_CLIENT_CERTIFICATE_PATH'" + " needs to be set. Additionally, 'AZURE_AUTHORITY_HOST' could be set" + " to override the default authority host. Currently:\n" " * 'AZURE_TENANT_ID' is set\n" " * 'AZURE_CLIENT_ID' is set\n" " * 'AZURE_CLIENT_SECRET' is NOT set\n" " * 'AZURE_CLIENT_CERTIFICATE_PATH' is NOT set\n" " * 'AZURE_AUTHORITY_HOST' is set\n"); + log.clear(); return credential; From d5e4a5c48b87c5213bcc787986eb23a74060a974 Mon Sep 17 00:00:00 2001 From: Ronnie Geraghty Date: Thu, 16 Mar 2023 16:16:57 -0700 Subject: [PATCH 08/20] 3rd Party Libs in Samples (#4408) Adding guidance on the proper usage of 3rd Party Libraries in our samples. --- CONTRIBUTING.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ef6bbec83d..3470fd648c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -257,3 +257,23 @@ If the coverage data has been previously generated (for example, if you manually ### Visual Studio 2019 You can also build the project by simply opening the repo directory in Visual Studio. Visual Studio will detect the `CMake` file and will configure itself to generate, build and run tests. + +## Samples + +### Third-party dependencies + +Third party libraries should only be included in samples when necessary to demonstrate usage of an Azure SDK package; they should not be suggested or endorsed as alternatives to the Azure SDK. + +When code samples take dependencies, readers should be able to use the material without significant license burden or research on terms. This goal requires restricting dependencies to certain types of open source or commercial licenses. + +Samples may take the following categories of dependencies: + +- **Open-source** : Open source offerings that use an [Open Source Initiative (OSI) approved license](https://opensource.org/licenses). Any component whose license isn't OSI-approved is considered a commercial offering. Prefer OSS projects that are members of any of the [OSS foundations that Microsoft is part of](https://opensource.microsoft.com/ecosystem/). Prefer permissive licenses for libraries, like [MIT](https://opensource.org/licenses/MIT) and [Apache 2](https://opensource.org/licenses/Apache-2.0). Copy-left licenses like [GPL](https://opensource.org/licenses/gpl-license) are acceptable for tools, and OSs. [Kubernetes](https://github.com/kubernetes/kubernetes), [Linux](https://github.com/torvalds/linux), and [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) are examples of this license type. Links to open source components should be to where the source is hosted, including any applicable license, such as a GitHub repository (or similar). + +- **Commercial**: Commercial offerings that enable readers to learn from our content without unnecessary extra costs. Typically, the offering has some form of a community edition, or a free trial sufficient for its use in content. A commercial license may be a form of dual-license, or tiered license. Links to commercial components should be to the commercial site for the software, even if the source software is hosted publicly on GitHub (or similar). + +- **Dual licensed**: Commercial offerings that enable readers to choose either license based on their needs. For example, if the offering has an OSS and commercial license, readers can choose between them. [MySql](https://github.com/mysql/mysql-server) is an example of this license type. + +- **Tiered licensed**: Offerings that enable readers to use the license tier that corresponds to their characteristics. For example, tiers may be available for students, hobbyists, or companies with defined revenue thresholds. For offerings with tiered licenses, strive to limit our use in tutorials to the features available in the lowest tier. This policy enables the widest audience for the article. [Docker](https://www.docker.com/), [IdentityServer](https://duendesoftware.com/products/identityserver), [ImageSharp](https://sixlabors.com/products/imagesharp/), and [Visual Studio](https://visualstudio.com) are examples of this license type. + +In general, we prefer taking dependencies on licensed components in the order of the listed categories. In cases where the category may not be well known, we'll document the category so that readers understand the choice that they're making by using that dependency. From ade505d087dbc81c28f0ac61fcd82d439c89fbbd Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Thu, 16 Mar 2023 17:20:32 -0700 Subject: [PATCH 09/20] Update changelog with issue link (#4458) Co-authored-by: Anton Kolesnyk --- sdk/core/azure-core/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index cca5d146d9..fa8e241cda 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -12,7 +12,7 @@ ### Bugs Fixed -- Fixed a bug where `Host` request header is not set for non-default port (80, 443). +- [[#4213]](https://github.com/Azure/azure-sdk-for-cpp/issues/4213) Fixed a bug where `Host` request header is not set for non-default port (80, 443). ### Other Changes From 43632ebce82cd1d61d28b37b4b05fe662ef105ac Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Fri, 17 Mar 2023 03:55:14 -0700 Subject: [PATCH 10/20] Tests: replace most `EXPECT_TRUE(a OP b)` with `EXPECT_OP(a, b)` (#4457) * Tests: replace most `EXPECT_TRUE(a OP b)` with `EXPECT_OP(a, b)` * Undo unnecessary change --------- Co-authored-by: Anton Kolesnyk --- .../test/ut/token_test.cpp | 2 +- .../test/ut/service_support_test.cpp | 6 +- sdk/core/azure-core/test/ut/context_test.cpp | 36 +++++------ sdk/core/azure-core/test/ut/nullable_test.cpp | 40 ++++++------ .../azure-core/test/ut/operation_test.cpp | 2 +- sdk/core/azure-core/test/ut/string_test.cpp | 63 +++++++++---------- .../test/ut/transport_adapter_base_test.cpp | 6 +- sdk/core/azure-core/test/ut/uuid_test.cpp | 4 +- .../ut/key_cryptographic_client_test_live.cpp | 24 +++---- .../test/ut/share_client_test.cpp | 24 +++---- .../test/ut/share_file_client_test.cpp | 10 +-- 11 files changed, 108 insertions(+), 109 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/test/ut/token_test.cpp b/sdk/attestation/azure-security-attestation/test/ut/token_test.cpp index 64bcc65891..7725792062 100644 --- a/sdk/attestation/azure-security-attestation/test/ut/token_test.cpp +++ b/sdk/attestation/azure-security-attestation/test/ut/token_test.cpp @@ -431,7 +431,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace Test { auto serializedObject = TestObjectSerializer::Serialize(testObject); auto targetObject = TestObjectSerializer::Deserialize(json::parse(serializedObject)); - EXPECT_TRUE(testObject == targetObject); + EXPECT_EQ(testObject, targetObject); } } diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index 65fec1a75e..10684d7a1b 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -194,14 +194,14 @@ class OpenTelemetryServiceTests : public Azure::Core::Test::TestBase { // Make sure that every expected attribute is somewhere in the actual attributes. for (auto const& expectedAttribute : expectedAttributes.items()) { - EXPECT_TRUE( + EXPECT_NE( std::find_if( attributes.begin(), attributes.end(), [&](std::pair& item) { return item.first == expectedAttribute.key(); - }) - != attributes.end()); + }), + attributes.end()); } for (auto const& foundAttribute : attributes) diff --git a/sdk/core/azure-core/test/ut/context_test.cpp b/sdk/core/azure-core/test/ut/context_test.cpp index 0c91449481..ab61ff903b 100644 --- a/sdk/core/azure-core/test/ut/context_test.cpp +++ b/sdk/core/azure-core/test/ut/context_test.cpp @@ -33,7 +33,7 @@ TEST(Context, BasicBool) auto c2 = context.WithValue(key, true); bool value{}; EXPECT_TRUE(c2.TryGetValue(key, value)); - EXPECT_TRUE(value == true); + EXPECT_TRUE(value); Context::Key const anotherKey; auto c3 = c2.WithValue(anotherKey, std::make_shared(true)); @@ -56,7 +56,7 @@ TEST(Context, BasicInt) auto c2 = context.WithValue(key, 123); int value; EXPECT_TRUE(c2.TryGetValue(key, value)); - EXPECT_TRUE(value == 123); + EXPECT_EQ(value, 123); } TEST(Context, BasicStdString) @@ -70,7 +70,7 @@ TEST(Context, BasicStdString) auto c2 = context.WithValue(key, s); std::string value; EXPECT_TRUE(c2.TryGetValue(key, value)); - EXPECT_TRUE(value == s); + EXPECT_EQ(value, s); } TEST(Context, BasicChar) @@ -85,8 +85,8 @@ TEST(Context, BasicChar) auto c2 = context.WithValue(key, str); const char* value; EXPECT_TRUE(c2.TryGetValue(key, value)); - EXPECT_TRUE(value == s); - EXPECT_TRUE(value == str); + EXPECT_EQ(value, s); + EXPECT_EQ(value, str); } TEST(Context, IsCancelled) @@ -206,13 +206,13 @@ TEST(Context, Chain) const char* valueT8; EXPECT_TRUE(finalContext.TryGetValue(keyFinal, valueT8)); - EXPECT_TRUE(valueT2 == 123); - EXPECT_TRUE(valueT3 == 456); - EXPECT_TRUE(valueT4 == 789); - EXPECT_TRUE(valueT5 == std::string("5")); - EXPECT_TRUE(valueT6 == std::string("6")); - EXPECT_TRUE(valueT7 == std::string("7")); - EXPECT_TRUE(valueT8 == std::string("Final")); + EXPECT_EQ(valueT2, 123); + EXPECT_EQ(valueT3, 456); + EXPECT_EQ(valueT4, 789); + EXPECT_EQ(valueT5, std::string("5")); + EXPECT_EQ(valueT6, std::string("6")); + EXPECT_EQ(valueT7, std::string("7")); + EXPECT_EQ(valueT8, std::string("Final")); } TEST(Context, MatchingKeys) @@ -229,8 +229,8 @@ TEST(Context, MatchingKeys) int valueT3; EXPECT_TRUE(c3.TryGetValue(key, valueT3)); - EXPECT_TRUE(valueT2 == 123); - EXPECT_TRUE(valueT3 == 456); + EXPECT_EQ(valueT2, 123); + EXPECT_EQ(valueT3, 456); } struct SomeStructForContext final @@ -492,10 +492,10 @@ TEST(Context, KeyTypePairPrecondition) #endif #endif - EXPECT_TRUE(strValue == "previous value"); + EXPECT_EQ(strValue, "previous value"); EXPECT_TRUE(c2.TryGetValue(key, intValue)); - EXPECT_TRUE(intValue == 123); + EXPECT_EQ(intValue, 123); #if GTEST_HAS_DEATH_TEST // Type-safe assert requires RTTI build @@ -509,8 +509,8 @@ TEST(Context, KeyTypePairPrecondition) #endif #endif - EXPECT_TRUE(intValue == 123); + EXPECT_EQ(intValue, 123); EXPECT_TRUE(c3.TryGetValue(key, strValue)); - EXPECT_TRUE(strValue == s); + EXPECT_EQ(strValue, s); } diff --git a/sdk/core/azure-core/test/ut/nullable_test.cpp b/sdk/core/azure-core/test/ut/nullable_test.cpp index 6306af33ee..adee4830d3 100644 --- a/sdk/core/azure-core/test/ut/nullable_test.cpp +++ b/sdk/core/azure-core/test/ut/nullable_test.cpp @@ -12,15 +12,15 @@ TEST(Nullable, Basic) { Nullable testString{"hello world"}; EXPECT_TRUE(testString.HasValue()); - EXPECT_TRUE(testString.Value() == "hello world"); + EXPECT_EQ(testString.Value(), "hello world"); Nullable testInt{54321}; EXPECT_TRUE(testInt.HasValue()); - EXPECT_TRUE(testInt.Value() == 54321); + EXPECT_EQ(testInt.Value(), 54321); Nullable testDouble{10.0}; EXPECT_TRUE(testDouble.HasValue()); - EXPECT_TRUE(testDouble.Value() == 10.0); + EXPECT_EQ(testDouble.Value(), 10.0); } TEST(Nullable, Empty) @@ -55,18 +55,18 @@ TEST(Nullable, Assignment) Nullable instance{"hello world"}; auto instance2 = instance; EXPECT_TRUE(instance2.HasValue()); - EXPECT_TRUE(instance2.Value() == "hello world"); + EXPECT_EQ(instance2.Value(), "hello world"); auto instance3 = std::move(instance); EXPECT_TRUE(instance3.HasValue()); - EXPECT_TRUE(instance3.Value() == "hello world"); + EXPECT_EQ(instance3.Value(), "hello world"); EXPECT_TRUE(instance.HasValue()); // This is not a guarantee that the string will be empty // It is an implementation detail that the contents are moved // Should a future compiler change this assumption this test will need updates - EXPECT_TRUE(instance.Value() == ""); + EXPECT_EQ(instance.Value(), ""); EXPECT_TRUE(instance.HasValue()); } @@ -76,22 +76,22 @@ TEST(Nullable, ValueAssignment) EXPECT_FALSE(intVal.HasValue()); intVal = 7; EXPECT_TRUE(intVal.HasValue()); - EXPECT_TRUE(intVal.Value() == 7); + EXPECT_EQ(intVal.Value(), 7); Nullable doubleVal; EXPECT_FALSE(doubleVal.HasValue()); doubleVal = 10.12345; EXPECT_TRUE(doubleVal.HasValue()); - EXPECT_TRUE(doubleVal.Value() == 10.12345); + EXPECT_EQ(doubleVal.Value(), 10.12345); Nullable strVal; EXPECT_FALSE(strVal.HasValue()); strVal = std::string("Hello World"); EXPECT_TRUE(strVal.HasValue()); - EXPECT_TRUE(strVal.Value() == "Hello World"); + EXPECT_EQ(strVal.Value(), "Hello World"); strVal = "New String"; - EXPECT_TRUE(strVal.Value() == "New String"); + EXPECT_EQ(strVal.Value(), "New String"); strVal.Reset(); EXPECT_FALSE(strVal.HasValue()); @@ -111,13 +111,13 @@ TEST(Nullable, Swap) val3.Swap(val4); EXPECT_TRUE(val3); EXPECT_TRUE(val4); - EXPECT_TRUE(val3.Value() == 678910); - EXPECT_TRUE(val4.Value() == 12345); + EXPECT_EQ(val3.Value(), 678910); + EXPECT_EQ(val4.Value(), 12345); val1.Swap(val3); EXPECT_TRUE(val1); EXPECT_FALSE(val3); - EXPECT_TRUE(val1.Value() == 678910); + EXPECT_EQ(val1.Value(), 678910); EXPECT_FALSE(val3.HasValue()); } @@ -134,19 +134,19 @@ TEST(Nullable, CopyConstruction) Nullable val4(val3); EXPECT_TRUE(val3); EXPECT_TRUE(val4); - EXPECT_TRUE(val3.Value() == 12345); - EXPECT_TRUE(val4.Value() == 12345); + EXPECT_EQ(val3.Value(), 12345); + EXPECT_EQ(val4.Value(), 12345); // Literal Nullable val5 = 54321; EXPECT_TRUE(val5); - EXPECT_TRUE(val5.Value() == 54321); + EXPECT_EQ(val5.Value(), 54321); // Value const int i = 1; Nullable val6(i); EXPECT_TRUE(val6); - EXPECT_TRUE(val6.Value() == 1); + EXPECT_EQ(val6.Value(), 1); } TEST(Nullable, Disengage) @@ -162,12 +162,12 @@ TEST(Nullable, ValueOr) Nullable val2; EXPECT_TRUE(val1); - EXPECT_TRUE(val1.ValueOr(678910) == 12345); + EXPECT_EQ(val1.ValueOr(678910), 12345); // Ensure the value was unmodified in ValueOr - EXPECT_TRUE(val1.Value() == 12345); + EXPECT_EQ(val1.Value(), 12345); EXPECT_FALSE(val2); - EXPECT_TRUE(val2.ValueOr(678910) == 678910); + EXPECT_EQ(val2.ValueOr(678910), 678910); // Ensure val2 is still disengaged after call to ValueOr EXPECT_FALSE(val2); } diff --git a/sdk/core/azure-core/test/ut/operation_test.cpp b/sdk/core/azure-core/test/ut/operation_test.cpp index 40667587e3..f76755d939 100644 --- a/sdk/core/azure-core/test/ut/operation_test.cpp +++ b/sdk/core/azure-core/test/ut/operation_test.cpp @@ -51,7 +51,7 @@ TEST(Operation, PollUntilDone) auto end = std::chrono::high_resolution_clock::now(); std::chrono::duration elapsed = end - start; // StringOperation test code is implemented to poll 2 times - EXPECT_TRUE(elapsed >= 1s); + EXPECT_GE(elapsed, 1s); EXPECT_TRUE(operation.IsDone()); EXPECT_TRUE(operation.HasValue()); diff --git a/sdk/core/azure-core/test/ut/string_test.cpp b/sdk/core/azure-core/test/ut/string_test.cpp index c3e710cd9c..704e5c8aca 100644 --- a/sdk/core/azure-core/test/ut/string_test.cpp +++ b/sdk/core/azure-core/test/ut/string_test.cpp @@ -16,6 +16,7 @@ TEST(String, invariantCompare) EXPECT_TRUE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("AA", "aa")); EXPECT_TRUE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("aA", "aa")); EXPECT_TRUE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("ABC", "abc")); + EXPECT_FALSE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("", "a")); EXPECT_FALSE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("a", "")); EXPECT_FALSE(StringExtensions::LocaleInvariantCaseInsensitiveEqual("A", "aA")); @@ -28,7 +29,7 @@ TEST(String, toLowerC) for (unsigned i = 0; i <= 255; ++i) { auto const c = static_cast(static_cast(i)); - EXPECT_TRUE(StringExtensions::ToLower(c) == std::tolower(c, std::locale::classic())); + EXPECT_EQ(StringExtensions::ToLower(c), std::tolower(c, std::locale::classic())); } } @@ -38,46 +39,44 @@ TEST(String, toUpperC) for (unsigned i = 0; i <= 255; ++i) { auto const c = static_cast(static_cast(i)); - EXPECT_TRUE(StringExtensions::ToUpper(c) == std::toupper(c, std::locale::classic())); + EXPECT_EQ(StringExtensions::ToUpper(c), std::toupper(c, std::locale::classic())); } } TEST(String, toLower) { using Azure::Core::_internal::StringExtensions; - EXPECT_TRUE(StringExtensions::ToLower("") == ""); - EXPECT_TRUE(StringExtensions::ToLower("a") == "a"); - EXPECT_TRUE(StringExtensions::ToLower("A") == "a"); - 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"); + EXPECT_EQ(StringExtensions::ToLower(""), ""); + EXPECT_EQ(StringExtensions::ToLower("a"), "a"); + EXPECT_EQ(StringExtensions::ToLower("A"), "a"); + EXPECT_EQ(StringExtensions::ToLower("AA"), "aa"); + EXPECT_EQ(StringExtensions::ToLower("aA"), "aa"); + EXPECT_EQ(StringExtensions::ToLower("ABC"), "abc"); + EXPECT_EQ(StringExtensions::ToLower("abcdefghijklmnopqrstuvwxyz"), "abcdefghijklmnopqrstuvwxyz"); + EXPECT_EQ(StringExtensions::ToLower("ABCDEFGHIJKLMNOPQRSTUVWXYZ"), "abcdefghijklmnopqrstuvwxyz"); + EXPECT_EQ(StringExtensions::ToLower("ABC-1-,!@#$%^&*()_+=ABC"), "abc-1-,!@#$%^&*()_+=abc"); + + EXPECT_NE(StringExtensions::ToLower(""), "a"); + EXPECT_NE(StringExtensions::ToLower("a"), ""); + EXPECT_NE(StringExtensions::ToLower("a"), "aA"); + EXPECT_NE(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"); + EXPECT_EQ(StringExtensions::ToUpper(""), ""); + EXPECT_EQ(StringExtensions::ToUpper("a"), "A"); + EXPECT_EQ(StringExtensions::ToUpper("A"), "A"); + EXPECT_EQ(StringExtensions::ToUpper("AA"), "AA"); + EXPECT_EQ(StringExtensions::ToUpper("aA"), "AA"); + EXPECT_EQ(StringExtensions::ToUpper("ABCDEFGHIJKLMNOPQRSTUVWXYZ"), "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + EXPECT_EQ(StringExtensions::ToUpper("ABC"), "ABC"); + EXPECT_EQ(StringExtensions::ToUpper("ABCDEFGHIJKLMNOPQRSTUVWXYZ"), "ABCDEFGHIJKLMNOPQRSTUVWXYZ"); + EXPECT_EQ(StringExtensions::ToUpper("ABC-1-,!@#$%^&*()_+=ABC"), "ABC-1-,!@#$%^&*()_+=ABC"); + + EXPECT_NE(StringExtensions::ToUpper(""), "A"); + EXPECT_NE(StringExtensions::ToUpper("a"), ""); + EXPECT_NE(StringExtensions::ToUpper("a"), "aA"); + EXPECT_NE(StringExtensions::ToUpper("abc"), "abcd"); } diff --git a/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp b/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp index 8f34dfe6b7..98b7149082 100644 --- a/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp +++ b/sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp @@ -84,7 +84,7 @@ namespace Azure { namespace Core { namespace Test { // Check content-length header to be greater than 0 int64_t contentLengthHeader = std::stoull(response->GetHeaders().at("content-length")); - EXPECT_TRUE(contentLengthHeader > 0); + EXPECT_GT(contentLengthHeader, 0); } TEST_P(TransportAdapter, put) @@ -221,7 +221,7 @@ namespace Azure { namespace Core { namespace Test { // Check content-length header to be greater than 0 int64_t contentLengthHeader = std::stoull(response->GetHeaders().at("content-length")); - EXPECT_TRUE(contentLengthHeader > 0); + EXPECT_GT(contentLengthHeader, 0); } TEST_P(TransportAdapter, putWithStream) @@ -301,7 +301,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Response responseT(expectedType, std::move(response)); auto& r = responseT.RawResponse; - EXPECT_TRUE(r->GetStatusCode() == Azure::Core::Http::HttpStatusCode::Ok); + EXPECT_EQ(r->GetStatusCode(), Azure::Core::Http::HttpStatusCode::Ok); auto expectedResponseBodySize = std::stoull(r->GetHeaders().at("content-length")); CheckBodyFromBuffer(*r, expectedResponseBodySize); diff --git a/sdk/core/azure-core/test/ut/uuid_test.cpp b/sdk/core/azure-core/test/ut/uuid_test.cpp index d1da8dfcf6..1b522087f3 100644 --- a/sdk/core/azure-core/test/ut/uuid_test.cpp +++ b/sdk/core/azure-core/test/ut/uuid_test.cpp @@ -11,7 +11,7 @@ using namespace Azure::Core; TEST(Uuid, Basic) { auto uuid = Uuid::CreateUuid(); - EXPECT_TRUE(uuid.ToString().length() == 36); + EXPECT_EQ(uuid.ToString().length(), 36); } TEST(Uuid, Randomness) @@ -25,7 +25,7 @@ TEST(Uuid, Randomness) // ret.second == false means the insert failed. EXPECT_TRUE(ret.second); } - EXPECT_TRUE(uuids.size() == size); + EXPECT_EQ(uuids.size(), size); } TEST(Uuid, separatorPosition) diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp index c20df1c2e6..06429eb336 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp @@ -37,7 +37,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteEncrypt) = cryptoClient->Encrypt(EncryptParameters::RsaOaepParameters(plaintext)).Value; EXPECT_EQ(encryptResult.Algorithm.ToString(), EncryptionAlgorithm::RsaOaep.ToString()); EXPECT_EQ(encryptResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(encryptResult.Ciphertext.size() > 0); + EXPECT_GT(encryptResult.Ciphertext.size(), 0); auto decryptResult = cryptoClient->Decrypt(DecryptParameters::RsaOaepParameters(encryptResult.Ciphertext)) @@ -68,7 +68,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteWrap) auto wrapResult = cryptoClient->WrapKey(KeyWrapAlgorithm::RsaOaep256, plaintext).Value; EXPECT_EQ(wrapResult.Algorithm.ToString(), KeyWrapAlgorithm::RsaOaep256.ToString()); EXPECT_EQ(wrapResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(wrapResult.EncryptedKey.size() > 0); + EXPECT_GT(wrapResult.EncryptedKey.size(), 0); auto unwrapResult = cryptoClient->UnwrapKey(wrapResult.Algorithm, wrapResult.EncryptedKey).Value; @@ -102,7 +102,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -121,7 +121,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -152,7 +152,7 @@ TEST_F(KeyVaultKeyClient, RemoteSignVerifyES256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, ecKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -176,7 +176,7 @@ TEST_F(KeyVaultKeyClient, RemoteSignVerifyES256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, ecKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -210,7 +210,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA384) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -229,7 +229,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA384) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -260,7 +260,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyDataRSA256) auto signResult = cryptoClient->SignData(signatureAlgorithm, data).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->VerifyData(signResult.Algorithm, data, signResult.Signature).Value; @@ -275,7 +275,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyDataRSA256) auto signResult = cryptoClient->SignData(signatureAlgorithm, data).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(signResult.Signature.size() > 0); + EXPECT_GT(signResult.Signature.size(), 0); auto verifyResult = cryptoClient->VerifyData(signResult.Algorithm, data, signResult.Signature).Value; @@ -305,7 +305,7 @@ TEST_P(KeyVaultKeyClientWithParam, GetCryptoFromKeyRemoteEncrypt) = cryptoClient.Encrypt(EncryptParameters::RsaOaepParameters(plaintext)).Value; EXPECT_EQ(encryptResult.Algorithm.ToString(), EncryptionAlgorithm::RsaOaep.ToString()); EXPECT_EQ(encryptResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(encryptResult.Ciphertext.size() > 0); + EXPECT_GT(encryptResult.Ciphertext.size(), 0); auto decryptResult = cryptoClient.Decrypt(DecryptParameters::RsaOaepParameters(encryptResult.Ciphertext)) @@ -336,7 +336,7 @@ TEST_P(KeyVaultKeyClientWithParam, GetCryptoFromKeyVersionRemoteEncrypt) = cryptoClient.Encrypt(EncryptParameters::RsaOaepParameters(plaintext)).Value; EXPECT_EQ(encryptResult.Algorithm.ToString(), EncryptionAlgorithm::RsaOaep.ToString()); EXPECT_EQ(encryptResult.KeyId, rsaKey.Id()); - EXPECT_TRUE(encryptResult.Ciphertext.size() > 0); + EXPECT_GT(encryptResult.Ciphertext.size(), 0); auto decryptResult = cryptoClient.Decrypt(DecryptParameters::RsaOaepParameters(encryptResult.Ciphertext)) diff --git a/sdk/storage/azure-storage-files-shares/test/ut/share_client_test.cpp b/sdk/storage/azure-storage-files-shares/test/ut/share_client_test.cpp index 63475b85b3..0375f6595e 100644 --- a/sdk/storage/azure-storage-files-shares/test/ut/share_client_test.cpp +++ b/sdk/storage/azure-storage-files-shares/test/ut/share_client_test.cpp @@ -301,12 +301,12 @@ namespace Azure { namespace Storage { namespace Test { Files::Shares::ShareLeaseClient leaseClient(*m_shareClient, leaseId1); auto aLease = leaseClient.Acquire(leaseDuration).Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(aLease.LeaseId, leaseId1); lastModified = m_shareClient->GetProperties().Value.LastModified; aLease = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(aLease.LeaseId, leaseId1); auto properties = m_shareClient->GetProperties().Value; @@ -316,7 +316,7 @@ namespace Azure { namespace Storage { namespace Test { lastModified = m_shareClient->GetProperties().Value.LastModified; auto rLease = leaseClient.Renew().Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(rLease.LeaseId, leaseId1); lastModified = m_shareClient->GetProperties().Value.LastModified; @@ -324,14 +324,14 @@ namespace Azure { namespace Storage { namespace Test { EXPECT_NE(leaseId1, leaseId2); auto cLease = leaseClient.Change(leaseId2).Value; EXPECT_TRUE(cLease.ETag.HasValue()); - EXPECT_TRUE(cLease.LastModified >= lastModified); + EXPECT_GE(cLease.LastModified, lastModified); EXPECT_EQ(cLease.LeaseId, leaseId2); EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); lastModified = m_shareClient->GetProperties().Value.LastModified; auto relLease = leaseClient.Release().Value; EXPECT_TRUE(relLease.ETag.HasValue()); - EXPECT_TRUE(relLease.LastModified >= lastModified); + EXPECT_GE(relLease.LastModified, lastModified); } { @@ -343,7 +343,7 @@ namespace Azure { namespace Storage { namespace Test { Files::Shares::Models::LeaseDurationType::Infinite, properties.LeaseDuration.Value()); auto brokenLease = leaseClient.Break().Value; EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(brokenLease.LastModified >= properties.LastModified); + EXPECT_GE(brokenLease.LastModified, properties.LastModified); } } @@ -358,14 +358,14 @@ namespace Azure { namespace Storage { namespace Test { Files::Shares::ShareLeaseClient shareSnapshotLeaseClient(shareSnapshot, leaseId1); auto aLease = shareSnapshotLeaseClient.Acquire(leaseDuration).Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(aLease.LeaseId, leaseId1); lastModified = shareSnapshot.GetProperties().Value.LastModified; aLease = shareSnapshotLeaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration) .Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(aLease.LeaseId, leaseId1); auto properties = shareSnapshot.GetProperties().Value; @@ -375,7 +375,7 @@ namespace Azure { namespace Storage { namespace Test { lastModified = shareSnapshot.GetProperties().Value.LastModified; auto rLease = shareSnapshotLeaseClient.Renew().Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(rLease.LeaseId, leaseId1); lastModified = shareSnapshot.GetProperties().Value.LastModified; @@ -383,14 +383,14 @@ namespace Azure { namespace Storage { namespace Test { EXPECT_NE(leaseId1, leaseId2); auto cLease = shareSnapshotLeaseClient.Change(leaseId2).Value; EXPECT_TRUE(cLease.ETag.HasValue()); - EXPECT_TRUE(cLease.LastModified >= lastModified); + EXPECT_GE(cLease.LastModified, lastModified); EXPECT_EQ(cLease.LeaseId, leaseId2); EXPECT_EQ(shareSnapshotLeaseClient.GetLeaseId(), leaseId2); lastModified = shareSnapshot.GetProperties().Value.LastModified; auto relLease = shareSnapshotLeaseClient.Release().Value; EXPECT_TRUE(relLease.ETag.HasValue()); - EXPECT_TRUE(relLease.LastModified >= lastModified); + EXPECT_GE(relLease.LastModified, lastModified); } { @@ -403,7 +403,7 @@ namespace Azure { namespace Storage { namespace Test { Files::Shares::Models::LeaseDurationType::Infinite, properties.LeaseDuration.Value()); auto brokenLease = shareSnapshotLeaseClient.Break().Value; EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(brokenLease.LastModified >= properties.LastModified); + EXPECT_GE(brokenLease.LastModified, properties.LastModified); shareSnapshotLeaseClient.Release(); } 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 26d7f50849..b16ac7a961 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 @@ -330,12 +330,12 @@ namespace Azure { namespace Storage { namespace Test { auto aLease = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(aLease.LeaseId, leaseId1); lastModified = m_fileClient->GetProperties().Value.LastModified; aLease = leaseClient.Acquire(Files::Shares::ShareLeaseClient::InfiniteLeaseDuration).Value; EXPECT_TRUE(aLease.ETag.HasValue()); - EXPECT_TRUE(aLease.LastModified >= lastModified); + EXPECT_GE(aLease.LastModified, lastModified); EXPECT_EQ(aLease.LeaseId, leaseId1); auto properties = m_fileClient->GetProperties().Value; @@ -347,14 +347,14 @@ namespace Azure { namespace Storage { namespace Test { lastModified = m_fileClient->GetProperties().Value.LastModified; auto cLease = leaseClient.Change(leaseId2).Value; EXPECT_TRUE(cLease.ETag.HasValue()); - EXPECT_TRUE(cLease.LastModified >= lastModified); + EXPECT_GE(cLease.LastModified, lastModified); EXPECT_EQ(cLease.LeaseId, leaseId2); EXPECT_EQ(leaseClient.GetLeaseId(), leaseId2); lastModified = m_fileClient->GetProperties().Value.LastModified; auto fileInfo = leaseClient.Release().Value; EXPECT_TRUE(fileInfo.ETag.HasValue()); - EXPECT_TRUE(fileInfo.LastModified >= lastModified); + EXPECT_GE(fileInfo.LastModified, lastModified); } { @@ -365,7 +365,7 @@ namespace Azure { namespace Storage { namespace Test { auto lastModified = m_fileClient->GetProperties().Value.LastModified; auto brokenLease = leaseClient.Break().Value; EXPECT_TRUE(brokenLease.ETag.HasValue()); - EXPECT_TRUE(brokenLease.LastModified >= lastModified); + EXPECT_GE(brokenLease.LastModified, lastModified); } } From a08730142cf212c7b192ef85cdebd490a9ccc981 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Fri, 17 Mar 2023 14:00:00 -0700 Subject: [PATCH 11/20] Revert "[check-spelling] Temporarily pin Node 18 to 18.13.0 (#5537)" (#4456) This reverts commit 8a02e02adfc0d213509fce2764132afa74bd4ba4. Co-authored-by: Mike Harder --- eng/common/pipelines/templates/steps/check-spelling.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eng/common/pipelines/templates/steps/check-spelling.yml b/eng/common/pipelines/templates/steps/check-spelling.yml index 0c489f0893..a25fd94441 100644 --- a/eng/common/pipelines/templates/steps/check-spelling.yml +++ b/eng/common/pipelines/templates/steps/check-spelling.yml @@ -18,8 +18,8 @@ steps: - task: NodeTool@0 condition: and(succeededOrFailed(), ne(variables['Skip.SpellCheck'],'true')) inputs: - versionSpec: 18.13.0 - displayName: Use Node.js 18.13.0 + versionSpec: 18.x + displayName: Use Node.js 18.x - task: PowerShell@2 displayName: Check spelling (cspell) From ba086576be4995a14a6ca3d17aba74374a6ad1ee Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Mon, 20 Mar 2023 13:06:11 -0700 Subject: [PATCH 12/20] Fix potentially high CPU usage on Windows (#4448) * Fix potentially high CPU usage on Windows * Undo unnecessary formatting * Undo unnecessary changelog * Undo unnecessary formatting * Undo unnecessary formatting * Uninclude locale * Add issue link to changelog * EXPECT_TRUE(a == b) => EXPECT_EQ(a, b) * Update second changelog with link as well --------- Co-authored-by: Anton Kolesnyk --- .../policy-certificates/cryptohelpers.hpp | 2 +- sdk/core/azure-core/CHANGELOG.md | 1 + .../inc/azure/core/internal/strings.hpp | 16 ++++++ sdk/core/azure-core/src/datetime.cpp | 8 +-- sdk/core/azure-core/src/http/curl/curl.cpp | 4 +- sdk/core/azure-core/src/http/http.cpp | 4 +- sdk/core/azure-core/src/http/url.cpp | 13 +++-- sdk/core/azure-core/src/http/user_agent.cpp | 14 ++++-- sdk/core/azure-core/test/ut/string_test.cpp | 50 +++++++++++++++++++ sdk/identity/azure-identity/CHANGELOG.md | 2 + .../src/azure_cli_credential.cpp | 5 +- 11 files changed, 96 insertions(+), 23 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/samples/policy-certificates/cryptohelpers.hpp b/sdk/attestation/azure-security-attestation/samples/policy-certificates/cryptohelpers.hpp index 7dc9ce49e5..edf87acf48 100644 --- a/sdk/attestation/azure-security-attestation/samples/policy-certificates/cryptohelpers.hpp +++ b/sdk/attestation/azure-security-attestation/samples/policy-certificates/cryptohelpers.hpp @@ -18,7 +18,7 @@ #include #include /** - * @brief THe Cryptography class provides a set of basic cryptographic primatives required + * @brief The Cryptography class provides a set of basic cryptographic primatives required * by the attestation samples. */ diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index fa8e241cda..292f81a15e 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -13,6 +13,7 @@ ### Bugs Fixed - [[#4213]](https://github.com/Azure/azure-sdk-for-cpp/issues/4213) Fixed a bug where `Host` request header is not set for non-default port (80, 443). +- [[#4443]](https://github.com/Azure/azure-sdk-for-cpp/issues/4443) Fixed potentially high CPU usage on Windows. ### Other Changes 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 d7e47ad912..1f6525f355 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/strings.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/strings.hpp @@ -30,6 +30,22 @@ namespace Azure { namespace Core { namespace _internal { return (c < 'A' || c > 'Z') ? c : c + ('a' - 'A'); } + static constexpr bool IsDigit(char c) noexcept { return c >= '0' && c <= '9'; } + + static constexpr bool IsHexDigit(char c) noexcept + { + return IsDigit(c) || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f'); + } + + static constexpr bool IsAlphaNumeric(char c) noexcept + { + return IsDigit(c) || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'); + } + + static constexpr bool IsSpace(char c) noexcept { return c == ' ' || (c >= '\t' && c <= '\r'); } + + static constexpr bool IsPrintable(char c) noexcept { return c >= ' ' && c <= '~'; } + struct CaseInsensitiveComparator final { bool operator()(std::string const& lhs, std::string const& rhs) const diff --git a/sdk/core/azure-core/src/datetime.cpp b/sdk/core/azure-core/src/datetime.cpp index 0d653b1b04..7b25ae3e13 100644 --- a/sdk/core/azure-core/src/datetime.cpp +++ b/sdk/core/azure-core/src/datetime.cpp @@ -2,13 +2,13 @@ // SPDX-License-Identifier: MIT #include "azure/core/datetime.hpp" +#include "azure/core/internal/strings.hpp" #include "azure/core/platform.hpp" #include #include #include #include -#include #include #include @@ -320,7 +320,7 @@ T ParseNumber( for (; i < MaxChars; ++i) { auto const ch = str[*cursor + i]; - if (std::isdigit(ch, std::locale::classic())) + if (Core::_internal::StringExtensions::IsDigit(ch)) { value = (value * 10) + (static_cast(static_cast(ch)) - '0'); continue; @@ -655,7 +655,7 @@ DateTime DateTime::Parse(std::string const& dateTime, DateFormat format) if (charsRead == 7 && (DateTimeLength - cursor) > 0) { auto const ch = dateTime[cursor]; - if (std::isdigit(ch, std::locale::classic())) + if (Core::_internal::StringExtensions::IsDigit(ch)) { auto const num = static_cast(static_cast(ch) - '0'); if (num > 4) @@ -677,7 +677,7 @@ DateTime DateTime::Parse(std::string const& dateTime, DateFormat format) for (auto i = DateTimeLength - cursor; i > 0; --i) { - if (std::isdigit(dateTime[cursor], std::locale::classic())) + if (Core::_internal::StringExtensions::IsDigit(dateTime[cursor])) { ++minDateTimeLength; ++cursor; diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index 01edbcc107..80309f8936 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -30,6 +30,7 @@ #include "azure/core/http/policies/policy.hpp" #include "azure/core/http/transport.hpp" #include "azure/core/internal/diagnostics/log.hpp" +#include "azure/core/internal/strings.hpp" // Private include #include "curl_connection_pool_private.hpp" @@ -70,7 +71,6 @@ #include #include #include -#include #include #include #include @@ -1340,7 +1340,7 @@ void DumpCurlInfoToLog(std::string const& text, uint8_t* ptr, size_t size) // Log the contents of the buffer as text, if it's printable, print the character, otherwise // print '.' auto const ch = static_cast(ptr[i + c]); - if (std::isprint(ch, std::locale::classic())) + if (Azure::Core::_internal::StringExtensions::IsPrintable(ch)) { ss << ch; } diff --git a/sdk/core/azure-core/src/http/http.cpp b/sdk/core/azure-core/src/http/http.cpp index c76691b042..0cd34fc89f 100644 --- a/sdk/core/azure-core/src/http/http.cpp +++ b/sdk/core/azure-core/src/http/http.cpp @@ -3,10 +3,10 @@ #include "azure/core/http/http.hpp" #include "azure/core/http/policies/policy.hpp" +#include "azure/core/internal/strings.hpp" #include "azure/core/url.hpp" #include -#include #include #include @@ -32,7 +32,7 @@ bool IsInvalidHeaderNameChar(char c) static std::unordered_set const HeaderNameExtraValidChars = {' ', '!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~'}; - return !std::isalnum(c, std::locale::classic()) + return !Azure::Core::_internal::StringExtensions::IsAlphaNumeric(c) && HeaderNameExtraValidChars.find(c) == HeaderNameExtraValidChars.end(); } } // namespace diff --git a/sdk/core/azure-core/src/http/url.cpp b/sdk/core/azure-core/src/http/url.cpp index 6304ca55b7..9ef2faa46e 100644 --- a/sdk/core/azure-core/src/http/url.cpp +++ b/sdk/core/azure-core/src/http/url.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include @@ -22,7 +21,7 @@ Url::Url(std::string const& url) if (schemePos != std::string::npos) { - m_scheme = Azure::Core::_internal::StringExtensions::ToLower(url.substr(0, schemePos)); + m_scheme = _internal::StringExtensions::ToLower(url.substr(0, schemePos)); urlIter += schemePos + SchemeEnd.length(); } } @@ -43,8 +42,8 @@ Url::Url(std::string const& url) if (*urlIter == ':') { ++urlIter; - auto const portIter = std::find_if_not( - urlIter, url.end(), [](auto c) { return std::isdigit(c, std::locale::classic()); }); + auto const portIter + = std::find_if_not(urlIter, url.end(), _internal::StringExtensions::IsDigit); auto const portNumber = std::stoi(std::string(urlIter, portIter)); @@ -105,8 +104,8 @@ std::string Url::Decode(std::string const& value) { case '%': if ((valueSize - i) < 3 // need at least 3 characters: "%XY" - || !std::isxdigit(value[i + 1], std::locale::classic()) - || !std::isxdigit(value[i + 2], std::locale::classic())) + || !_internal::StringExtensions::IsHexDigit(value[i + 1]) + || !_internal::StringExtensions::IsHexDigit(value[i + 2])) { throw std::runtime_error("failed when decoding URL component"); } @@ -133,7 +132,7 @@ bool ShouldEncode(char c) { static std::unordered_set const ExtraNonEncodableChars = {'-', '.', '_', '~'}; - return !std::isalnum(c, std::locale::classic()) + return !_internal::StringExtensions::IsAlphaNumeric(c) && ExtraNonEncodableChars.find(c) == ExtraNonEncodableChars.end(); } } // namespace diff --git a/sdk/core/azure-core/src/http/user_agent.cpp b/sdk/core/azure-core/src/http/user_agent.cpp index 03ae3e8e76..3e5ae5128b 100644 --- a/sdk/core/azure-core/src/http/user_agent.cpp +++ b/sdk/core/azure-core/src/http/user_agent.cpp @@ -11,9 +11,9 @@ #include "azure/core/context.hpp" #include "azure/core/http/policies/policy.hpp" +#include "azure/core/internal/strings.hpp" #include "azure/core/internal/tracing/service_tracing.hpp" #include "azure/core/platform.hpp" -#include #include #if defined(AZ_PLATFORM_WINDOWS) @@ -133,10 +133,14 @@ std::string GetOSVersion() std::string TrimString(std::string s) { - auto const isNotSpace = [](char c) { return !std::isspace(c, std::locale::classic()); }; - - s.erase(s.begin(), std::find_if(s.begin(), s.end(), isNotSpace)); - s.erase(std::find_if(s.rbegin(), s.rend(), isNotSpace).base(), s.end()); + s.erase( + s.begin(), + std::find_if_not(s.begin(), s.end(), Azure::Core::_internal::StringExtensions::IsSpace)); + + s.erase( + std::find_if_not(s.rbegin(), s.rend(), Azure::Core::_internal::StringExtensions::IsSpace) + .base(), + s.end()); return s; } diff --git a/sdk/core/azure-core/test/ut/string_test.cpp b/sdk/core/azure-core/test/ut/string_test.cpp index 704e5c8aca..acc0d252ab 100644 --- a/sdk/core/azure-core/test/ut/string_test.cpp +++ b/sdk/core/azure-core/test/ut/string_test.cpp @@ -43,6 +43,56 @@ TEST(String, toUpperC) } } +TEST(String, isDigit) +{ + using Azure::Core::_internal::StringExtensions; + for (unsigned i = 0; i <= 255; ++i) + { + auto const c = static_cast(static_cast(i)); + EXPECT_EQ(StringExtensions::IsDigit(c), std::isdigit(c, std::locale::classic())); + } +} + +TEST(String, isHexDigit) +{ + using Azure::Core::_internal::StringExtensions; + for (unsigned i = 0; i <= 255; ++i) + { + auto const c = static_cast(static_cast(i)); + EXPECT_EQ(StringExtensions::IsHexDigit(c), std::isxdigit(c, std::locale::classic())); + } +} + +TEST(String, isAlphaNumeric) +{ + using Azure::Core::_internal::StringExtensions; + for (unsigned i = 0; i <= 255; ++i) + { + auto const c = static_cast(static_cast(i)); + EXPECT_EQ(StringExtensions::IsAlphaNumeric(c), std::isalnum(c, std::locale::classic())); + } +} + +TEST(String, isSpace) +{ + using Azure::Core::_internal::StringExtensions; + for (unsigned i = 0; i <= 255; ++i) + { + auto const c = static_cast(static_cast(i)); + EXPECT_EQ(StringExtensions::IsSpace(c), std::isspace(c, std::locale::classic())); + } +} + +TEST(String, isPrintable) +{ + using Azure::Core::_internal::StringExtensions; + for (unsigned i = 0; i <= 255; ++i) + { + auto const c = static_cast(static_cast(i)); + EXPECT_EQ(StringExtensions::IsPrintable(c), std::isprint(c, std::locale::classic())); + } +} + TEST(String, toLower) { using Azure::Core::_internal::StringExtensions; diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index b8143d12df..5ce4f5a8d2 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +- [[#4443]](https://github.com/Azure/azure-sdk-for-cpp/issues/4443) Fixed potentially high CPU usage on Windows. + ### Other Changes - Improved diagnostics to utilize `Azure::Core::Credentials::TokenCredential::GetCredentialName()`. diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index f02c32f7e1..ceb73e535d 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -7,11 +7,11 @@ #include #include +#include #include #include #include -#include #include #include #include @@ -40,6 +40,7 @@ using Azure::Identity::AzureCliCredential; using Azure::DateTime; using Azure::Core::Context; using Azure::Core::_internal::Environment; +using Azure::Core::_internal::StringExtensions; using Azure::Core::Credentials::AccessToken; using Azure::Core::Credentials::AuthenticationException; using Azure::Core::Credentials::TokenCredentialOptions; @@ -71,7 +72,7 @@ void AzureCliCredential::ThrowIfNotSafeCmdLineInput( break; default: - if (!std::isalnum(c, std::locale::classic())) + if (!StringExtensions::IsAlphaNumeric(c)) { throw AuthenticationException( IdentityPrefix + GetCredentialName() + ": Unsafe command line input found in " From c457e91373406de2eb3f3f2cf2b16bad2b684ed0 Mon Sep 17 00:00:00 2001 From: Rick Winter Date: Tue, 21 Mar 2023 13:57:07 -0700 Subject: [PATCH 13/20] Ensure the comparison is unsigned to unsigned (#4464) * Ensure the comparison is unsigned to unsigned * Remove cast --- .../ut/key_cryptographic_client_test_live.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp index 06429eb336..9d67decca3 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_cryptographic_client_test_live.cpp @@ -37,7 +37,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteEncrypt) = cryptoClient->Encrypt(EncryptParameters::RsaOaepParameters(plaintext)).Value; EXPECT_EQ(encryptResult.Algorithm.ToString(), EncryptionAlgorithm::RsaOaep.ToString()); EXPECT_EQ(encryptResult.KeyId, rsaKey.Id()); - EXPECT_GT(encryptResult.Ciphertext.size(), 0); + EXPECT_GT(encryptResult.Ciphertext.size(), 0u); auto decryptResult = cryptoClient->Decrypt(DecryptParameters::RsaOaepParameters(encryptResult.Ciphertext)) @@ -68,7 +68,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteWrap) auto wrapResult = cryptoClient->WrapKey(KeyWrapAlgorithm::RsaOaep256, plaintext).Value; EXPECT_EQ(wrapResult.Algorithm.ToString(), KeyWrapAlgorithm::RsaOaep256.ToString()); EXPECT_EQ(wrapResult.KeyId, rsaKey.Id()); - EXPECT_GT(wrapResult.EncryptedKey.size(), 0); + EXPECT_GT(wrapResult.EncryptedKey.size(), 0u); auto unwrapResult = cryptoClient->UnwrapKey(wrapResult.Algorithm, wrapResult.EncryptedKey).Value; @@ -102,7 +102,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -121,7 +121,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -152,7 +152,7 @@ TEST_F(KeyVaultKeyClient, RemoteSignVerifyES256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, ecKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -176,7 +176,7 @@ TEST_F(KeyVaultKeyClient, RemoteSignVerifyES256) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, ecKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -210,7 +210,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA384) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -229,7 +229,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyRSA384) auto signResult = cryptoClient->Sign(signatureAlgorithm, digest).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->Verify(signResult.Algorithm, digest, signResult.Signature).Value; @@ -260,7 +260,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyDataRSA256) auto signResult = cryptoClient->SignData(signatureAlgorithm, data).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->VerifyData(signResult.Algorithm, data, signResult.Signature).Value; @@ -275,7 +275,7 @@ TEST_P(KeyVaultKeyClientWithParam, RemoteSignVerifyDataRSA256) auto signResult = cryptoClient->SignData(signatureAlgorithm, data).Value; EXPECT_EQ(signResult.Algorithm.ToString(), signatureAlgorithm.ToString()); EXPECT_EQ(signResult.KeyId, rsaKey.Id()); - EXPECT_GT(signResult.Signature.size(), 0); + EXPECT_GT(signResult.Signature.size(), 0u); auto verifyResult = cryptoClient->VerifyData(signResult.Algorithm, data, signResult.Signature).Value; @@ -305,7 +305,7 @@ TEST_P(KeyVaultKeyClientWithParam, GetCryptoFromKeyRemoteEncrypt) = cryptoClient.Encrypt(EncryptParameters::RsaOaepParameters(plaintext)).Value; EXPECT_EQ(encryptResult.Algorithm.ToString(), EncryptionAlgorithm::RsaOaep.ToString()); EXPECT_EQ(encryptResult.KeyId, rsaKey.Id()); - EXPECT_GT(encryptResult.Ciphertext.size(), 0); + EXPECT_GT(encryptResult.Ciphertext.size(), 0u); auto decryptResult = cryptoClient.Decrypt(DecryptParameters::RsaOaepParameters(encryptResult.Ciphertext)) @@ -336,7 +336,7 @@ TEST_P(KeyVaultKeyClientWithParam, GetCryptoFromKeyVersionRemoteEncrypt) = cryptoClient.Encrypt(EncryptParameters::RsaOaepParameters(plaintext)).Value; EXPECT_EQ(encryptResult.Algorithm.ToString(), EncryptionAlgorithm::RsaOaep.ToString()); EXPECT_EQ(encryptResult.KeyId, rsaKey.Id()); - EXPECT_GT(encryptResult.Ciphertext.size(), 0); + EXPECT_GT(encryptResult.Ciphertext.size(), 0u); auto decryptResult = cryptoClient.Decrypt(DecryptParameters::RsaOaepParameters(encryptResult.Ciphertext)) From bd98ee06ec610a6f42c93b8bd59e0c68d7969fb4 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Tue, 21 Mar 2023 16:14:28 -0700 Subject: [PATCH 14/20] Sync eng/common directory with azure-sdk-tools for PR 5742 (#4465) * add some default output to see about minimizing any occurrence of the task failing for no reason. perhaps having some output will allow devops to have an easier time with the invocation * update message * Update eng/common/scripts/trust-proxy-certificate.ps1 Co-authored-by: Wes Haggard --- eng/common/scripts/trust-proxy-certificate.ps1 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/eng/common/scripts/trust-proxy-certificate.ps1 b/eng/common/scripts/trust-proxy-certificate.ps1 index 144d304cfd..3587ff3eea 100644 --- a/eng/common/scripts/trust-proxy-certificate.ps1 +++ b/eng/common/scripts/trust-proxy-certificate.ps1 @@ -3,4 +3,8 @@ if ($TestProxyTrustCertFn -and (Test-Path "Function:$TestProxyTrustCertFn")) { &$TestProxyTrustCertFn +} +else +{ + Write-Host "No implementation of Import-Dev-Cert- provided in eng/scripts/Language-Settings.ps1." } \ No newline at end of file From a91ff6345e2788e3d4ad0f4ae8895f2fa5a34762 Mon Sep 17 00:00:00 2001 From: Rick Winter Date: Wed, 22 Mar 2023 13:09:42 -0700 Subject: [PATCH 15/20] Update CODEOWNERS (#4461) Fixup an invalid user --- .github/CODEOWNERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index bc52d464a2..ac73a157a3 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -23,10 +23,10 @@ ########### # PRLabel: %Attestation -/sdk/attestation/ @LarryOsterman @gkostal @anilba06 @kroshkina-ms @ahmadmsft @rickwinter @ahsonkhan @antkmsft @vhvb1989 @gearama +/sdk/attestation/ @LarryOsterman @gkostal @anilba06 @ahmadmsft @rickwinter @ahsonkhan @antkmsft @vhvb1989 @gearama # PRLabel: %KeyVault -/sdk/keyvault/ @vhvb1989 @gearama @antkmsft @rickwinter +/sdk/keyvault/ @gearama @antkmsft @rickwinter @LarryOsterman # PRLabel: %Storage /sdk/storage/ @vinjiang @Jinming-Hu @EmmaZhu @antkmsft @vhvb1989 @gearama @LarryOsterman @microzchang From 2a39a3422b1ffaceea8ccf73dcc3e807ebf814b8 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Wed, 22 Mar 2023 14:46:32 -0700 Subject: [PATCH 16/20] Organize applying Identity log prefix (#4459) * Organize applying Identity log prefix * logLevel * Cosmetic changes --------- Co-authored-by: Anton Kolesnyk --- sdk/identity/azure-identity/CMakeLists.txt | 1 + .../src/azure_cli_credential.cpp | 25 ++++------ .../src/chained_token_credential.cpp | 41 ++++++++-------- .../src/default_azure_credential.cpp | 16 +++--- .../src/environment_credential.cpp | 49 +++++++++---------- .../src/managed_identity_source.cpp | 30 +++++------- .../src/private/identity_log.hpp | 29 +++++++++++ 7 files changed, 101 insertions(+), 90 deletions(-) create mode 100644 sdk/identity/azure-identity/src/private/identity_log.hpp diff --git a/sdk/identity/azure-identity/CMakeLists.txt b/sdk/identity/azure-identity/CMakeLists.txt index 6d6e4ad7c5..25169d9eff 100644 --- a/sdk/identity/azure-identity/CMakeLists.txt +++ b/sdk/identity/azure-identity/CMakeLists.txt @@ -63,6 +63,7 @@ set( set( AZURE_IDENTITY_SOURCE src/private/chained_token_credential_impl.hpp + src/private/identity_log.hpp src/private/managed_identity_source.hpp src/private/package_version.hpp src/private/token_credential_impl.hpp diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index ceb73e535d..443088770b 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -3,9 +3,9 @@ #include "azure/identity/azure_cli_credential.hpp" +#include "private/identity_log.hpp" #include "private/token_credential_impl.hpp" -#include #include #include #include @@ -45,16 +45,11 @@ using Azure::Core::Credentials::AccessToken; using Azure::Core::Credentials::AuthenticationException; using Azure::Core::Credentials::TokenCredentialOptions; using Azure::Core::Credentials::TokenRequestContext; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; using Azure::Identity::AzureCliCredentialOptions; +using Azure::Identity::_detail::IdentityLog; using Azure::Identity::_detail::TokenCache; using Azure::Identity::_detail::TokenCredentialImpl; -namespace { -constexpr auto IdentityPrefix = "Identity: "; -} - void AzureCliCredential::ThrowIfNotSafeCmdLineInput( std::string const& input, std::string const& description) const @@ -75,8 +70,8 @@ void AzureCliCredential::ThrowIfNotSafeCmdLineInput( if (!StringExtensions::IsAlphaNumeric(c)) { throw AuthenticationException( - IdentityPrefix + GetCredentialName() + ": Unsafe command line input found in " - + description + ": " + input); + GetCredentialName() + ": Unsafe command line input found in " + description + ": " + + input); } } } @@ -92,9 +87,9 @@ AzureCliCredential::AzureCliCredential( ThrowIfNotSafeCmdLineInput(m_tenantId, "TenantID"); - Log::Write( - Logger::Level::Informational, - IdentityPrefix + GetCredentialName() + IdentityLog::Write( + IdentityLog::Level::Informational, + GetCredentialName() + " created.\n" "Successful creation does not guarantee further successful token retrieval."); } @@ -162,10 +157,8 @@ AccessToken AzureCliCredential::GetToken( } catch (std::exception const& e) { - auto const errorMsg - = IdentityPrefix + GetCredentialName() + " didn't get the token: \"" + e.what() + '\"'; - - Log::Write(Logger::Level::Warning, errorMsg); + auto const errorMsg = GetCredentialName() + " didn't get the token: \"" + e.what() + '\"'; + IdentityLog::Write(IdentityLog::Level::Warning, errorMsg); throw AuthenticationException(errorMsg); } }); diff --git a/sdk/identity/azure-identity/src/chained_token_credential.cpp b/sdk/identity/azure-identity/src/chained_token_credential.cpp index e30774e358..c7b441bac3 100644 --- a/sdk/identity/azure-identity/src/chained_token_credential.cpp +++ b/sdk/identity/azure-identity/src/chained_token_credential.cpp @@ -2,8 +2,11 @@ // SPDX-License-Identifier: MIT #include "azure/identity/chained_token_credential.hpp" -#include "azure/core/internal/diagnostics/log.hpp" + #include "private/chained_token_credential_impl.hpp" +#include "private/identity_log.hpp" + +#include "azure/core/internal/diagnostics/log.hpp" #include @@ -11,8 +14,7 @@ using namespace Azure::Identity; using namespace Azure::Identity::_detail; using namespace Azure::Core::Credentials; using Azure::Core::Context; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; +using Azure::Identity::_detail::IdentityLog; ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources) : TokenCredential("ChainedTokenCredential"), @@ -29,17 +31,15 @@ AccessToken ChainedTokenCredential::GetToken( return m_impl->GetToken(GetCredentialName(), tokenRequestContext, context); } -namespace { -constexpr auto IdentityPrefix = "Identity: "; -} // namespace - ChainedTokenCredentialImpl::ChainedTokenCredentialImpl( std::string const& credentialName, ChainedTokenCredential::Sources&& sources) : m_sources(std::move(sources)) { - auto const logLevel = m_sources.empty() ? Logger::Level::Warning : Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) + auto const logLevel + = m_sources.empty() ? IdentityLog::Level::Warning : IdentityLog::Level::Informational; + + if (IdentityLog::ShouldWrite(logLevel)) { std::string credSourceDetails = " with EMPTY chain of credentials."; if (!m_sources.empty()) @@ -60,7 +60,7 @@ ChainedTokenCredentialImpl::ChainedTokenCredentialImpl( credSourceDetails += '.'; } - Log::Write(logLevel, IdentityPrefix + credentialName + ": Created" + credSourceDetails); + IdentityLog::Write(logLevel, credentialName + ": Created" + credSourceDetails); } } @@ -75,25 +75,24 @@ AccessToken ChainedTokenCredentialImpl::GetToken( { auto token = source->GetToken(tokenRequestContext, context); - Log::Write( - Logger::Level::Informational, - IdentityPrefix + credentialName + ": Successfully got token from " - + source->GetCredentialName() + '.'); + IdentityLog::Write( + IdentityLog::Level::Informational, + credentialName + ": Successfully got token from " + source->GetCredentialName() + '.'); return token; } catch (AuthenticationException const& e) { - Log::Write( - Logger::Level::Verbose, - IdentityPrefix + credentialName + ": Failed to get token from " - + source->GetCredentialName() + ": " + e.what()); + IdentityLog::Write( + IdentityLog::Level::Verbose, + credentialName + ": Failed to get token from " + source->GetCredentialName() + ": " + + e.what()); } } - Log::Write( - Logger::Level::Warning, - IdentityPrefix + credentialName + IdentityLog::Write( + IdentityLog::Level::Warning, + credentialName + (m_sources.empty() ? ": Authentication did not succeed: List of sources is empty." : ": Didn't succeed to get a token from any credential in the chain.")); diff --git a/sdk/identity/azure-identity/src/default_azure_credential.cpp b/sdk/identity/azure-identity/src/default_azure_credential.cpp index d50c2a6e73..79bebb9970 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -6,20 +6,16 @@ #include "azure/identity/azure_cli_credential.hpp" #include "azure/identity/environment_credential.hpp" #include "azure/identity/managed_identity_credential.hpp" -#include "private/chained_token_credential_impl.hpp" -#include "azure/core/internal/diagnostics/log.hpp" +#include "private/chained_token_credential_impl.hpp" +#include "private/identity_log.hpp" using namespace Azure::Identity; using namespace Azure::Core::Credentials; using Azure::Core::Context; using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; - -namespace { -constexpr auto IdentityPrefix = "Identity: "; -} // namespace +using Azure::Identity::_detail::IdentityLog; DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& options) : TokenCredential("DefaultAzureCredential") @@ -27,9 +23,9 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt // Initializing m_credential below and not in the member initializer list to have a specific order // of log messages. - Log::Write( - Logger::Level::Verbose, - std::string(IdentityPrefix) + "Creating " + GetCredentialName() + IdentityLog::Write( + IdentityLog::Level::Verbose, + "Creating " + GetCredentialName() + " which combines mutiple parameterless credentials into a single one.\n" + GetCredentialName() + " is only recommended for the early stages of development, " diff --git a/sdk/identity/azure-identity/src/environment_credential.cpp b/sdk/identity/azure-identity/src/environment_credential.cpp index 7d96727da6..80dc948ff3 100644 --- a/sdk/identity/azure-identity/src/environment_credential.cpp +++ b/sdk/identity/azure-identity/src/environment_credential.cpp @@ -5,8 +5,9 @@ #include "azure/identity/client_certificate_credential.hpp" #include "azure/identity/client_secret_credential.hpp" +#include "private/identity_log.hpp" + #include -#include #include #include @@ -20,8 +21,7 @@ using Azure::Core::Credentials::AccessToken; using Azure::Core::Credentials::AuthenticationException; using Azure::Core::Credentials::TokenCredentialOptions; using Azure::Core::Credentials::TokenRequestContext; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; +using Azure::Identity::_detail::IdentityLog; namespace { constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID"; @@ -30,8 +30,6 @@ constexpr auto AzureClientSecretEnvVarName = "AZURE_CLIENT_SECRET"; constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST"; constexpr auto AzureClientCertificatePathEnvVarName = "AZURE_CLIENT_CERTIFICATE_PATH"; -constexpr auto IdentityPrefix = "Identity: "; - void PrintCredentialCreationLogMessage( std::string const& logMsgPrefix, std::vector> const& envVarsToParams, @@ -41,8 +39,6 @@ void PrintCredentialCreationLogMessage( EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) : TokenCredential("EnvironmentCredential") { - auto const logMsgPrefix = IdentityPrefix + GetCredentialName(); - auto tenantId = Environment::GetVariable(AzureTenantIdEnvVarName); auto clientId = Environment::GetVariable(AzureClientIdEnvVarName); @@ -58,7 +54,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -77,7 +73,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -94,7 +90,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -113,7 +109,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -129,13 +125,14 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!m_credentialImpl) { - Log::Write( - Logger::Level::Warning, logMsgPrefix + " was not initialized with underlying credential."); + IdentityLog::Write( + IdentityLog::Level::Warning, + GetCredentialName() + " was not initialized with underlying credential."); - auto const logLevel = Logger::Level::Verbose; - if (Log::ShouldWrite(logLevel)) + auto const logLevel = IdentityLog::Level::Verbose; + if (IdentityLog::ShouldWrite(logLevel)) { - auto logMsg = logMsgPrefix + ": Both '" + AzureTenantIdEnvVarName + "' and '" + auto logMsg = GetCredentialName() + ": Both '" + AzureTenantIdEnvVarName + "' and '" + AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName + "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '" + AzureAuthorityHostEnvVarName @@ -154,7 +151,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) + "set\n"; } - Log::Write(logLevel, logMsg); + IdentityLog::Write(logLevel, logMsg); } } } @@ -165,11 +162,10 @@ AccessToken EnvironmentCredential::GetToken( { if (!m_credentialImpl) { - auto const AuthUnavailable - = IdentityPrefix + GetCredentialName() + " authentication unavailable. "; + auto const AuthUnavailable = GetCredentialName() + " authentication unavailable. "; - Log::Write( - Logger::Level::Warning, + IdentityLog::Write( + IdentityLog::Level::Warning, AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); throw AuthenticationException( @@ -185,11 +181,12 @@ void PrintCredentialCreationLogMessage( std::vector> const& envVarsToParams, char const* credThatGetsCreated) { - Log::Write( - Logger::Level::Informational, + IdentityLog::Write( + IdentityLog::Level::Informational, logMsgPrefix + " gets created with " + credThatGetsCreated + '.'); - if (!Log::ShouldWrite(Logger::Level::Verbose)) + auto const logLevel = IdentityLog::Level::Verbose; + if (!IdentityLog::ShouldWrite(logLevel)) { return; } @@ -217,8 +214,8 @@ void PrintCredentialCreationLogMessage( envVars += And + Tick + envVarsToParams.back().first + Tick; credParams += And + envVarsToParams.back().second; - Log::Write( - Logger::Level::Verbose, + IdentityLog::Write( + logLevel, logMsgPrefix + ": " + envVars + " environment variables are set, so " + credThatGetsCreated + " with corresponding " + credParams + " gets created."); } diff --git a/sdk/identity/azure-identity/src/managed_identity_source.cpp b/sdk/identity/azure-identity/src/managed_identity_source.cpp index daea912fe3..e3001b9c6c 100644 --- a/sdk/identity/azure-identity/src/managed_identity_source.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_source.cpp @@ -3,9 +3,9 @@ #include "private/managed_identity_source.hpp" -#include +#include "private/identity_log.hpp" -#include +#include #include #include @@ -15,12 +15,9 @@ using namespace Azure::Identity::_detail; using Azure::Core::_internal::Environment; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; +using Azure::Identity::_detail::IdentityLog; namespace { -constexpr auto IdentityPrefix = "Identity: "; - std::string WithSourceMessage(std::string const& credSource) { return " with " + credSource + " source"; @@ -28,9 +25,9 @@ std::string WithSourceMessage(std::string const& credSource) void PrintEnvNotSetUpMessage(std::string const& credName, std::string const& credSource) { - Log::Write( - Logger::Level::Verbose, - IdentityPrefix + credName + ": Environment is not set up for the credential to be created" + IdentityLog::Write( + IdentityLog::Level::Verbose, + credName + ": Environment is not set up for the credential to be created" + WithSourceMessage(credSource) + '.'); } } // namespace @@ -48,9 +45,9 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( { auto const endpointUrl = Url(url); - Log::Write( - Logger::Level::Informational, - IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.'); + IdentityLog::Write( + IdentityLog::Level::Informational, + credName + " will be created" + WithSourceMessage(credSource) + '.'); return endpointUrl; } @@ -65,7 +62,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( + ": Failed to create: The environment variable \'" + envVarName + "\' contains an invalid URL."; - Log::Write(Logger::Level::Warning, IdentityPrefix + errorMessage); + IdentityLog::Write(IdentityLog::Level::Warning, errorMessage); throw AuthenticationException(errorMessage); } @@ -372,10 +369,9 @@ std::unique_ptr ImdsManagedIdentitySource::Create( std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { - Log::Write( - Logger::Level::Informational, - IdentityPrefix + credName + " will be created" - + WithSourceMessage("Azure Instance Metadata Service") + IdentityLog::Write( + IdentityLog::Level::Informational, + credName + " will be created" + WithSourceMessage("Azure Instance Metadata Service") + ".\nSuccessful creation does not guarantee further successful token retrieval."); return std::unique_ptr(new ImdsManagedIdentitySource(clientId, options)); diff --git a/sdk/identity/azure-identity/src/private/identity_log.hpp b/sdk/identity/azure-identity/src/private/identity_log.hpp new file mode 100644 index 0000000000..ee33661dec --- /dev/null +++ b/sdk/identity/azure-identity/src/private/identity_log.hpp @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#pragma once + +#include + +namespace Azure { namespace Identity { namespace _detail { + + class IdentityLog final { + public: + using Level = Core::Diagnostics::Logger::Level; + + static void Write(Level level, std::string const& message) + { + Core::Diagnostics::_internal::Log::Write(level, "Identity: " + message); + } + + static bool ShouldWrite(Level level) + { + return Core::Diagnostics::_internal::Log::ShouldWrite(level); + } + + private: + IdentityLog() = delete; + ~IdentityLog() = delete; + }; + +}}} // namespace Azure::Identity::_detail From e9f900f7d27c95b1b1c279991606079939bec8fc Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Fri, 24 Mar 2023 11:12:14 -0700 Subject: [PATCH 17/20] Explicitly set PSNativeCommandArgumentPassing to Legacy for git push script (#4481) https://learn.microsoft.com/en-us/powershell/scripting/learn/experimental-features?view=powershell-7.3#psnativecommandargumentpassing Do to that breaking change in PS 7.3 we need to opt into the legacy arg parsing. Co-authored-by: Wes Haggard --- eng/common/scripts/git-branch-push.ps1 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/eng/common/scripts/git-branch-push.ps1 b/eng/common/scripts/git-branch-push.ps1 index 3e012a1f28..edd792ffd8 100644 --- a/eng/common/scripts/git-branch-push.ps1 +++ b/eng/common/scripts/git-branch-push.ps1 @@ -37,6 +37,10 @@ param( [boolean] $AmendCommit = $false ) +# Explicit set arg parsing to Legacy mode because some of the git calls in this script depend on empty strings being empty and not passing a "" git. +# more info https://learn.microsoft.com/en-us/powershell/scripting/learn/experimental-features?view=powershell-7.3#psnativecommandargumentpassing +$PSNativeCommandArgumentPassing = "Legacy" + # This is necessary because of the git command output writing to stderr. # Without explicitly setting the ErrorActionPreference to continue the script # would fail the first time git wrote command output. From ef5aec8b4c8633bd5d41333191bc3200ad0a4fdb Mon Sep 17 00:00:00 2001 From: JinmingHu Date: Sat, 25 Mar 2023 08:47:55 +0800 Subject: [PATCH 18/20] Fix unmatched parenthesis in doc (#4482) --- sdk/storage/MigrationGuide.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/storage/MigrationGuide.md b/sdk/storage/MigrationGuide.md index 8e48b93c12..a57420e13b 100644 --- a/sdk/storage/MigrationGuide.md +++ b/sdk/storage/MigrationGuide.md @@ -81,7 +81,7 @@ cloud_blob_client blob_client(storage_uri(blob_url), storage_credentials(sas_tok ``` ```C++ -cloud_blob_client blob_client(storage_uri(blob_url_with_sas); +cloud_blob_client blob_client(storage_uri(blob_url_with_sas)); ``` v12 From 42017bf69e71ee9dc82ba9c3e61ccc76581bb778 Mon Sep 17 00:00:00 2001 From: JinmingHu Date: Mon, 27 Mar 2023 09:04:44 +0800 Subject: [PATCH 19/20] fix concurrent upload failures (#4484) --- .../test/ut/block_blob_client_test.cpp | 19 +++++-------------- .../test/ut/datalake_file_client_test.cpp | 19 +++++-------------- .../test/ut/share_file_client_test.cpp | 19 +++++-------------- 3 files changed, 15 insertions(+), 42 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 4ee6a5cfae..38588144ee 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 @@ -1737,23 +1737,14 @@ namespace Azure { namespace Storage { namespace Test { for (int c : {1, 2, 4}) { - std::vector> futures; - // random range for (int i = 0; i < 16; ++i) { + // random range int64_t fileSize = RandomInt(1, 1_MB); - futures.emplace_back( - std::async(std::launch::async, testUploadFromBuffer, c, fileSize, 4_KB, 47_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromFile, c, fileSize, 2_KB, 185_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromBuffer, c, fileSize, 0, 117_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromFile, c, fileSize, 0, 259_KB)); - } - for (auto& f : futures) - { - f.get(); + testUploadFromBuffer(c, fileSize, 4_KB, 47_KB); + testUploadFromFile(c, fileSize, 2_KB, 185_KB); + testUploadFromBuffer(c, fileSize, 0, 117_KB); + testUploadFromFile(c, fileSize, 0, 259_KB); } } } 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 eaeb1d9c3c..268f43d509 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 @@ -868,23 +868,14 @@ namespace Azure { namespace Storage { namespace Test { for (int c : {1, 2, 4}) { - std::vector> futures; - // random range for (int i = 0; i < 16; ++i) { + // random range int64_t fileSize = RandomInt(1, 1_MB); - futures.emplace_back( - std::async(std::launch::async, testUploadFromBuffer, c, fileSize, 4_KB, 56_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromFile, c, fileSize, 2_KB, 172_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromBuffer, c, fileSize, 0, 109_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromFile, c, fileSize, 0, 256_KB)); - } - for (auto& f : futures) - { - f.get(); + testUploadFromBuffer(c, fileSize, 4_KB, 56_KB); + testUploadFromFile(c, fileSize, 2_KB, 172_KB); + testUploadFromBuffer(c, fileSize, 0, 109_KB); + testUploadFromFile(c, fileSize, 0, 256_KB); } } } 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 b16ac7a961..8f7f98d81e 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 @@ -430,23 +430,14 @@ namespace Azure { namespace Storage { namespace Test { for (int c : {1, 2, 4}) { - std::vector> futures; - // random range for (int i = 0; i < 16; ++i) { + // random range int64_t fileSize = RandomInt(1, 1_MB); - futures.emplace_back( - std::async(std::launch::async, testUploadFromBuffer, c, fileSize, 4_KB, 40_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromFile, c, fileSize, 2_KB, 162_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromBuffer, c, fileSize, 0, 127_KB)); - futures.emplace_back( - std::async(std::launch::async, testUploadFromFile, c, fileSize, 0, 253_KB)); - } - for (auto& f : futures) - { - f.get(); + testUploadFromBuffer(c, fileSize, 4_KB, 40_KB); + testUploadFromFile(c, fileSize, 2_KB, 162_KB); + testUploadFromBuffer(c, fileSize, 0, 127_KB); + testUploadFromFile(c, fileSize, 0, 253_KB); } } } From 97c7630c2d31f52c79d5a44a0b4d6f8af973ee76 Mon Sep 17 00:00:00 2001 From: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Date: Tue, 28 Mar 2023 09:37:59 -0700 Subject: [PATCH 20/20] Sync eng/common directory with azure-sdk-tools for PR 5726 (#4488) * rerun flag * rerun failed stress test * naming & commenting * update * function and var renaming for better readability * readability & exit on error --------- Co-authored-by: Albert Cheng --- .../stress-testing/deploy-stress-tests.ps1 | 2 + .../stress-test-deployment-lib.ps1 | 97 ++++++++++++++++--- 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/eng/common/scripts/stress-testing/deploy-stress-tests.ps1 b/eng/common/scripts/stress-testing/deploy-stress-tests.ps1 index bbf1d1d425..bc028f26aa 100644 --- a/eng/common/scripts/stress-testing/deploy-stress-tests.ps1 +++ b/eng/common/scripts/stress-testing/deploy-stress-tests.ps1 @@ -25,6 +25,8 @@ param( # Renders chart templates locally without deployment [Parameter(Mandatory=$False)][switch]$Template, + [Parameter(Mandatory=$False)][switch]$RetryFailedTests, + # Matrix generation parameters [Parameter(Mandatory=$False)][string]$MatrixFileName, [Parameter(Mandatory=$False)][string]$MatrixSelection, diff --git a/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 b/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 index bafbf77a94..a7d45ae66f 100644 --- a/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 +++ b/eng/common/scripts/stress-testing/stress-test-deployment-lib.ps1 @@ -98,6 +98,7 @@ function DeployStressTests( })] [System.IO.FileInfo]$LocalAddonsPath, [Parameter(Mandatory=$False)][switch]$Template, + [Parameter(Mandatory=$False)][switch]$RetryFailedTests, [Parameter(Mandatory=$False)][string]$MatrixFileName, [Parameter(Mandatory=$False)][string]$MatrixSelection = "sparse", [Parameter(Mandatory=$False)][string]$MatrixDisplayNameFilter, @@ -215,11 +216,16 @@ function DeployStressPackage( if ($LASTEXITCODE) {exit $LASTEXITCODE} $dockerBuildConfigs = @() - - $genValFile = Join-Path $pkg.Directory "generatedValues.yaml" - $genVal = Get-Content $genValFile -Raw | ConvertFrom-Yaml -Ordered - if (Test-Path $genValFile) { - $scenarios = $genVal.Scenarios + + $generatedHelmValuesFilePath = Join-Path $pkg.Directory "generatedValues.yaml" + $generatedHelmValues = Get-Content $generatedHelmValuesFilePath -Raw | ConvertFrom-Yaml -Ordered + $releaseName = $pkg.ReleaseName + if ($RetryFailedTests) { + $releaseName, $generatedHelmValues = generateRetryTestsHelmValues $pkg $releaseName $generatedHelmValues + } + + if (Test-Path $generatedHelmValuesFilePath) { + $scenarios = $generatedHelmValues.Scenarios foreach ($scenario in $scenarios) { if ("image" -in $scenario.keys) { $dockerFilePath = Join-Path $pkg.Directory $scenario.image @@ -286,7 +292,7 @@ function DeployStressPackage( } } } - $genVal.scenarios = @( foreach ($scenario in $genVal.scenarios) { + $generatedHelmValues.scenarios = @( foreach ($scenario in $generatedHelmValues.scenarios) { $dockerPath = if ("image" -notin $scenario) { $dockerFilePath } else { @@ -298,15 +304,15 @@ function DeployStressPackage( $scenario } ) - $genVal | ConvertTo-Yaml | Out-File -FilePath $genValFile + $generatedHelmValues | ConvertTo-Yaml | Out-File -FilePath $generatedHelmValuesFilePath } - Write-Host "Installing or upgrading stress test $($pkg.ReleaseName) from $($pkg.Directory)" + Write-Host "Installing or upgrading stress test $releaseName from $($pkg.Directory)" $generatedConfigPath = Join-Path $pkg.Directory generatedValues.yaml $subCommand = $Template ? "template" : "upgrade" $installFlag = $Template ? "" : "--install" - $helmCommandArg = "helm", $subCommand, $pkg.ReleaseName, $pkg.Directory, "-n", $pkg.Namespace, $installFlag, "--set", "stress-test-addons.env=$environment", "--values", $generatedConfigPath + $helmCommandArg = "helm", $subCommand, $releaseName, $pkg.Directory, "-n", $pkg.Namespace, $installFlag, "--set", "stress-test-addons.env=$environment", "--values", $generatedConfigPath $result = (Run @helmCommandArg) 2>&1 | Write-Host @@ -322,7 +328,7 @@ function DeployStressPackage( # Issues like 'UPGRADE FAILED: another operation (install/upgrade/rollback) is in progress' # can be the result of cancelled `upgrade` operations (e.g. ctrl-c). # See https://github.com/helm/helm/issues/4558 - Write-Warning "The issue may be fixable by first running 'helm rollback -n $($pkg.Namespace) $($pkg.ReleaseName)'" + Write-Warning "The issue may be fixable by first running 'helm rollback -n $($pkg.Namespace) $releaseName'" return } } @@ -333,7 +339,7 @@ function DeployStressPackage( if(!$Template) { $helmReleaseConfig = RunOrExitOnFailure kubectl get secrets ` -n $pkg.Namespace ` - -l "status=deployed,name=$($pkg.ReleaseName)" ` + -l "status=deployed,name=$releaseName" ` -o jsonpath='{.items[0].metadata.name}' Run kubectl label secret -n $pkg.Namespace --overwrite $helmReleaseConfig deployId=$deployId } @@ -375,3 +381,72 @@ function CheckDependencies() } } + +function generateRetryTestsHelmValues ($pkg, $releaseName, $generatedHelmValues) { + $podOutput = RunOrExitOnFailure kubectl get pods -n $pkg.namespace -o json + $pods = $podOutput | ConvertFrom-Json + + # Get all jobs within this helm release + + $helmStatusOutput = RunOrExitOnFailure helm status -n $pkg.Namespace $pkg.ReleaseName --show-resources + # -----Example output----- + # NAME: + # LAST DEPLOYED: Mon Jan 01 12:12:12 2020 + # NAMESPACE: + # STATUS: deployed + # REVISION: 10 + # RESOURCES: + # ==> v1alpha1/Schedule + # NAME AGE + # 5h5m + # 5h5m + + # ==> v1/SecretProviderClass + # 7d4h + + # ==> v1/Job + # NAME COMPLETIONS DURATION AGE + # 0/1 5h5m 5h5m + # 0/1 5h5m 5h5m + $discoveredJob = $False + $jobs = @() + foreach ($line in $helmStatusOutput) { + if ($discoveredJob -and $line -match "==>") {break} + if ($discoveredJob) { + $jobs += ($line -split '\s+')[0] | Where-Object {($_ -ne "NAME") -and ($_)} + } + if ($line -match "==> v1/Job") { + $discoveredJob = $True + } + } + + $failedJobsScenario = @() + $revision = 0 + foreach ($job in $jobs) { + $jobRevision = [int]$job.split('-')[-1] + if ($jobRevision -gt $revision) { + $revision = $jobRevision + } + + $jobOutput = RunOrExitOnFailure kubectl describe jobs -n $pkg.Namespace $job + $podPhase = $jobOutput | Select-String "0 Failed" + if ([System.String]::IsNullOrEmpty($podPhase)) { + $failedJobsScenario += $job.split("-$($pkg.ReleaseName)")[0] + } + } + + $releaseName = "$($pkg.ReleaseName)-$revision-retry" + + $retryTestsHelmVal = @{"scenarios"=@()} + foreach ($failedScenario in $failedJobsScenario) { + $failedScenarioObject = $generatedHelmValues.scenarios | Where {$_.Scenario -eq $failedScenario} + $retryTestsHelmVal.scenarios += $failedScenarioObject + } + + if (!$retryTestsHelmVal.scenarios.length) { + Write-Host "There are no failed pods to retry." + return + } + $generatedHelmValues = $retryTestsHelmVal + return $releaseName, $generatedHelmValues +}