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 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/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. 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) 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", 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"] 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. 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 +} 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 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/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-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-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/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 4ecb1b52a6..292f81a15e 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -6,12 +6,14 @@ - 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 ### 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). +- [[#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/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/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/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. 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/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..acc0d252ab 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,94 @@ 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, 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; - 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/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index de58b0bcc5..5ce4f5a8d2 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -8,8 +8,12 @@ ### 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()`. + ## 1.5.0-beta.1 (2023-03-07) ### Features Added diff --git a/sdk/identity/azure-identity/CMakeLists.txt b/sdk/identity/azure-identity/CMakeLists.txt index ab2cd1b6b5..25169d9eff 100644 --- a/sdk/identity/azure-identity/CMakeLists.txt +++ b/sdk/identity/azure-identity/CMakeLists.txt @@ -62,6 +62,8 @@ 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/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/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..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,14 +60,7 @@ namespace Azure { namespace Identity { Core::Context const& context) const override; private: - explicit ChainedTokenCredential( - Sources sources, - std::string const& enclosingCredential, - std::vector sourcesFriendlyNames); - - Sources m_sources; - std::vector m_sourcesFriendlyNames; - 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/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/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index 83e3e4b05c..443088770b 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -3,15 +3,15 @@ #include "azure/identity/azure_cli_credential.hpp" +#include "private/identity_log.hpp" #include "private/token_credential_impl.hpp" -#include #include +#include #include #include #include -#include #include #include #include @@ -40,20 +40,19 @@ 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; 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 { -std::string const MsgPrefix = "Identity: AzureCliCredential"; - -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) { @@ -68,34 +67,31 @@ void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& des break; default: - if (!std::isalnum(c, std::locale::classic())) + if (!StringExtensions::IsAlphaNumeric(c)) { throw AuthenticationException( - MsgPrefix + ": Unsafe command line input found in " + description + ": " + input); + 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; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - MsgPrefix - + " created.\n" - "Successful creation does not guarantee further successful token retrieval."); - } + IdentityLog::Write( + IdentityLog::Level::Informational, + GetCredentialName() + + " created.\n" + "Successful creation does not guarantee further successful token retrieval."); } AzureCliCredential::AzureCliCredential(AzureCliCredentialOptions const& options) @@ -161,14 +157,8 @@ AccessToken AzureCliCredential::GetToken( } catch (std::exception const& e) { - auto const errorMsg = MsgPrefix + " didn't get the token: \"" + e.what() + '\"'; - - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write(logLevel, 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 3a63bea9ce..c7b441bac3 100644 --- a/sdk/identity/azure-identity/src/chained_token_credential.cpp +++ b/sdk/identity/azure-identity/src/chained_token_credential.cpp @@ -3,141 +3,99 @@ #include "azure/identity/chained_token_credential.hpp" +#include "private/chained_token_credential_impl.hpp" +#include "private/identity_log.hpp" + #include "azure/core/internal/diagnostics/log.hpp" -#include #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; +using Azure::Identity::_detail::IdentityLog; -namespace { -std::string const IdentityPrefix = "Identity: "; +ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources) + : TokenCredential("ChainedTokenCredential"), + m_impl(std::make_unique(GetCredentialName(), std::move(sources))) +{ } -ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources) - : ChainedTokenCredential(sources, {}, {}) +ChainedTokenCredential::~ChainedTokenCredential() = default; + +AccessToken ChainedTokenCredential::GetToken( + TokenRequestContext const& tokenRequestContext, + Context const& context) const { + return m_impl->GetToken(GetCredentialName(), tokenRequestContext, context); } -ChainedTokenCredential::ChainedTokenCredential( - ChainedTokenCredential::Sources sources, - std::string const& enclosingCredential, - std::vector sourcesFriendlyNames) - : m_sources(std::move(sources)), m_sourcesFriendlyNames(std::move(sourcesFriendlyNames)) +ChainedTokenCredentialImpl::ChainedTokenCredentialImpl( + std::string const& credentialName, + ChainedTokenCredential::Sources&& sources) + : m_sources(std::move(sources)) { - // LCOV_EXCL_START - AZURE_ASSERT(m_sourcesFriendlyNames.empty() || m_sourcesFriendlyNames.size() == m_sources.size()); - // LCOV_EXCL_STOP + auto const logLevel + = m_sources.empty() ? IdentityLog::Level::Warning : IdentityLog::Level::Informational; - auto const logLevel = m_sources.empty() ? Logger::Level::Warning : Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) + if (IdentityLog::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)); - } + IdentityLog::Write(logLevel, credentialName + ": Created" + credSourceDetails); } } -ChainedTokenCredential::~ChainedTokenCredential() = default; - -AccessToken ChainedTokenCredential::GetToken( +AccessToken ChainedTokenCredentialImpl::GetToken( + std::string const& credentialName, TokenRequestContext const& tokenRequestContext, Context const& context) const { - auto const sourcesSize = m_sources.size(); - - if (sourcesSize == 0) - { - const auto logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, m_logPrefix + "Authentication did not succeed: List of sources is empty."); - } - } - else + for (auto const& source : m_sources) { - for (size_t i = 0; i < sourcesSize; ++i) + try { - try - { - auto token = m_sources[i]->GetToken(tokenRequestContext, context); - - { - auto const logLevel = Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - m_logPrefix + "Successfully got token from " + m_sourcesFriendlyNames[i] + '.'); - } - } + auto token = source->GetToken(tokenRequestContext, context); - return token; - } - catch (AuthenticationException const& e) - { - { - auto const logLevel = Logger::Level::Verbose; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - m_logPrefix + "Failed to get token from " + m_sourcesFriendlyNames[i] + ": " - + e.what()); - } - } + IdentityLog::Write( + IdentityLog::Level::Informational, + credentialName + ": Successfully got token from " + source->GetCredentialName() + '.'); - if ((sourcesSize - 1) == i) // On the last credential - { - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - m_logPrefix + "Didn't succeed to get a token from any credential in the chain."); - } - } - } + return token; + } + catch (AuthenticationException const& e) + { + IdentityLog::Write( + IdentityLog::Level::Verbose, + credentialName + ": Failed to get token from " + source->GetCredentialName() + ": " + + e.what()); } } - throw AuthenticationException("Failed to get token from ChainedTokenCredential."); + 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.")); + + throw AuthenticationException("Failed to get token from " + credentialName + '.'); } 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..79bebb9970 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -7,49 +7,43 @@ #include "azure/identity/environment_credential.hpp" #include "azure/identity/managed_identity_credential.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 { -std::string const IdentityPrefix = "Identity: "; -} +using Azure::Identity::_detail::IdentityLog; 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. - auto const logLevel = Logger::Level::Verbose; - if (Log::ShouldWrite(logLevel)) - { - 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, " - "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 " - "is the better fit for the application."); - } + + 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, " + "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); 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}, - "DefaultAzureCredential", // extra args for the ChainedTokenCredential's private constructor. - std::vector{ - "EnvironmentCredential", "AzureCliCredential", "ManagedIdentityCredential"})); + m_impl = std::make_unique<_detail::ChainedTokenCredentialImpl>( + GetCredentialName(), + ChainedTokenCredential::Sources{envCred, azCliCred, managedIdentityCred}); } DefaultAzureCredential::~DefaultAzureCredential() = default; @@ -60,13 +54,13 @@ AccessToken DefaultAzureCredential::GetToken( { try { - return m_credentials->GetToken(tokenRequestContext, context); + return m_impl->GetToken(GetCredentialName(), tokenRequestContext, context); } 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://aka.ms/azsdk/cpp/identity/troubleshooting)."); } } diff --git a/sdk/identity/azure-identity/src/environment_credential.cpp b/sdk/identity/azure-identity/src/environment_credential.cpp index 2d6eb2479c..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,14 +30,14 @@ 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"; - void PrintCredentialCreationLogMessage( + std::string const& logMsgPrefix, std::vector> const& envVarsToParams, char const* credThatGetsCreated); } // namespace EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) + : TokenCredential("EnvironmentCredential") { auto tenantId = Environment::GetVariable(AzureTenantIdEnvVarName); auto clientId = Environment::GetVariable(AzureClientIdEnvVarName); @@ -54,6 +54,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -72,6 +73,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -88,6 +90,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -106,6 +109,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -121,38 +125,33 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!m_credentialImpl) { - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - auto const basicMessage = LogMsgPrefix + " was not initialized with underlying credential"; + IdentityLog::Write( + IdentityLog::Level::Warning, + GetCredentialName() + " was not initialized with underlying credential."); - if (!Log::ShouldWrite(Logger::Level::Verbose)) - { - Log::Write(logLevel, basicMessage + '.'); - } - else + auto const logLevel = IdentityLog::Level::Verbose; + if (IdentityLog::ShouldWrite(logLevel)) + { + auto logMsg = GetCredentialName() + ": 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"; } + + IdentityLog::Write(logLevel, logMsg); } } } @@ -163,17 +162,11 @@ AccessToken EnvironmentCredential::GetToken( { if (!m_credentialImpl) { - auto const AuthUnavailable = LogMsgPrefix + " authentication unavailable. "; + auto const AuthUnavailable = GetCredentialName() + " authentication unavailable. "; - { - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - AuthUnavailable + "See earlier EnvironmentCredential log messages for details."); - } - } + IdentityLog::Write( + IdentityLog::Level::Warning, + AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); throw AuthenticationException( AuthUnavailable + "Environment variables are not fully configured."); @@ -184,18 +177,17 @@ AccessToken EnvironmentCredential::GetToken( namespace { void PrintCredentialCreationLogMessage( + std::string const& logMsgPrefix, std::vector> const& envVarsToParams, char const* credThatGetsCreated) { - if (!Log::ShouldWrite(Logger::Level::Verbose)) - { - if (Log::ShouldWrite(Logger::Level::Informational)) - { - Log::Write( - Logger::Level::Informational, - LogMsgPrefix + " gets created with " + credThatGetsCreated + '.'); - } + IdentityLog::Write( + IdentityLog::Level::Informational, + logMsgPrefix + " gets created with " + credThatGetsCreated + '.'); + auto const logLevel = IdentityLog::Level::Verbose; + if (!IdentityLog::ShouldWrite(logLevel)) + { return; } @@ -222,9 +214,9 @@ void PrintCredentialCreationLogMessage( envVars += And + Tick + envVarsToParams.back().first + Tick; credParams += And + envVarsToParams.back().second; - Log::Write( - Logger::Level::Verbose, - LogMsgPrefix + ": " + envVars + " environment variables are set, so " + credThatGetsCreated + IdentityLog::Write( + logLevel, + 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..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,32 +15,25 @@ 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 { -std::string const IdentityPrefix = "Identity: "; -constexpr auto CredPrefix = "ManagedIdentityCredential"; - 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" - + WithSourceMessage(credSource) + '.'); - } + IdentityLog::Write( + IdentityLog::Level::Verbose, + 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) @@ -52,13 +45,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 + CredPrefix + " will be created" + WithSourceMessage(credSource) + '.'); - } + IdentityLog::Write( + IdentityLog::Level::Informational, + credName + " will be created" + WithSourceMessage(credSource) + '.'); return endpointUrl; } @@ -69,21 +58,17 @@ 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."; - auto const logLevel = Logger::Level::Warning; - if (Log::ShouldWrite(logLevel)) - { - Log::Write(logLevel, IdentityPrefix + errorMessage); - } - + IdentityLog::Write(IdentityLog::Level::Warning, errorMessage); throw AuthenticationException(errorMessage); } 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 +83,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 +151,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 +181,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 +243,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 +256,7 @@ std::unique_ptr AzureArcManagedIdentitySource::Create( if (identityEndpoint.empty() || Environment::GetVariable("IMDS_ENDPOINT").empty()) { - PrintEnvNotSetUpMessage(credSource); + PrintEnvNotSetUpMessage(credName, credSource); return nullptr; } @@ -277,7 +269,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,18 +365,14 @@ 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) { - auto const logLevel = Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) - { - Log::Write( - logLevel, - IdentityPrefix + CredPrefix + " will be created" - + WithSourceMessage("Azure Instance Metadata Service") - + ".\nSuccessful creation does not guarantee further successful token retrieval."); - } + 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/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/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 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..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 @@ -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>; @@ -47,70 +69,73 @@ 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( 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 " "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, - "Identity: DefaultAzureCredential: Created ChainedTokenCredential " - "with the following credentials: " + log[9].second, + "Identity: DefaultAzureCredential: Created with the following credentials: " "EnvironmentCredential, AzureCliCredential, ManagedIdentityCredential."); log.clear(); @@ -127,8 +152,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); } 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..71f4b6c063 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; @@ -37,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; @@ -186,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; @@ -249,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; @@ -326,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; @@ -450,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; 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( 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..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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(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_TRUE(encryptResult.Ciphertext.size() > 0); + EXPECT_GT(encryptResult.Ciphertext.size(), 0u); auto decryptResult = cryptoClient.Decrypt(DecryptParameters::RsaOaepParameters(encryptResult.Ciphertext)) 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 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_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..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 @@ -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); } } @@ -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); } } }