Repository navigation
fix(hubs): keep stored ingestion and final retention on redeploy #2288
+113
−7
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| <# | ||
| Regression coverage for the settings.json retention guard (#2206): | ||
| Copy-FileToAzureBlob.ps1 unconditionally overwrote retention.ingestion.months / retention.final.months | ||
| with whatever the deploymentScript's Bicep parameters passed in. Bicep always resolves a value for an | ||
| optional parameter (defaulting to 13 if the caller didn't specify one), so the script cannot tell an | ||
| explicit redeploy value from a silently-defaulted one. A redeploy that omitted a previously-customized | ||
| retention value therefore silently reset it to the toolkit default, and the next purge pipeline run | ||
| aged out historical data older than the new (lower) cutoff -- oldest data first. | ||
|
|
||
| The fix: a value already stored in settings.json is kept on redeploy. The requested value only applies | ||
| when nothing is stored yet (new hub, or settings.json from before retention existed). | ||
|
|
||
| These tests run the real script with stubbed Az cmdlets and assert on the JSON it uploads. | ||
| #> | ||
|
|
||
| Describe 'HubsRetentionGuard' { | ||
|
|
||
| BeforeAll { | ||
| $repoRoot = (Resolve-Path "$PSScriptRoot/../../../..").Path | ||
| $scriptPath = Join-Path $repoRoot 'src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Core/Copy-FileToAzureBlob.ps1' | ||
|
|
||
| function Invoke-CopyScript | ||
| { | ||
| param([string] $ExistingJson, [int] $Ingestion, [int] $Final) | ||
|
|
||
| $workDir = Join-Path ([IO.Path]::GetTempPath()) ([guid]::NewGuid().ToString()) | ||
| New-Item -ItemType Directory -Path $workDir | Out-Null | ||
| $existingJson = $ExistingJson | ||
|
|
||
| function New-AzStorageContext { param($StorageAccountName, [switch] $UseConnectedAccount) 'ctx' } | ||
| function Get-AzStorageBlobContent | ||
| { | ||
| param($Context, $Container, $Blob, $Destination, [switch] $Force) | ||
| if ($null -eq $existingJson) { return $null } | ||
| Set-Content -Path $Destination -Value $existingJson | ||
| return 'blob' | ||
| } | ||
| function Set-AzStorageBlobContent { param($Context, $Container, $File, [switch] $Force) } | ||
|
|
||
| $env:storageAccountName = 'sa' | ||
| $env:containerName = 'config' | ||
| $env:scopes = '' | ||
| $env:ftkVersion = '0.0.0' | ||
| $env:msexportRetentionInDays = '0' | ||
| $env:rawRetentionInDays = '0' | ||
| $env:ingestionRetentionInMonths = "$Ingestion" | ||
| $env:finalRetentionInMonths = "$Final" | ||
|
Comment on lines
+43
to
+50
Collaborator
There was a problem hiding this comment. Non-blocking: these process-level environment variables stay set after the tests finish, so they can affect other tests in the same Pester session ( |
||
|
|
||
| Push-Location $workDir | ||
| try | ||
| { | ||
| & $scriptPath *> $null | ||
| # The script writes the final settings.json to the working directory before uploading it | ||
| return (Get-Content -Path (Join-Path $workDir settings.json) -Raw | ConvertFrom-Json) | ||
| } | ||
| finally { Pop-Location; Remove-Item $workDir -Recurse -Force -ErrorAction SilentlyContinue } | ||
| } | ||
| } | ||
|
|
||
| Context 'Existing settings.json' { | ||
|
|
||
| It 'Should keep a stored value that is higher than the requested one' { | ||
| $r = Invoke-CopyScript -ExistingJson '{"version":"","scopes":[],"retention":{"msexports":{"days":0},"ingestion":{"months":36},"raw":{"days":0},"final":{"months":60}}}' -Ingestion 13 -Final 13 | ||
| $r.retention.ingestion.months | Should -Be 36 -Because 'a redeploy without an explicit value must not reset customized retention (#2206)' | ||
| $r.retention.final.months | Should -Be 60 | ||
| } | ||
|
|
||
| It 'Should keep a stored value that is lower than the requested one' { | ||
| $r = Invoke-CopyScript -ExistingJson '{"version":"","scopes":[],"retention":{"msexports":{"days":0},"ingestion":{"months":6},"raw":{"days":0},"final":{"months":6}}}' -Ingestion 13 -Final 13 | ||
| $r.retention.ingestion.months | Should -Be 6 -Because 'a stored value is never changed on redeploy' | ||
| $r.retention.final.months | Should -Be 6 | ||
| } | ||
|
|
||
| It 'Should seed retention from the request when settings.json predates retention' { | ||
| $r = Invoke-CopyScript -ExistingJson '{"version":"","scopes":[]}' -Ingestion 24 -Final 30 | ||
| $r.retention.ingestion.months | Should -Be 24 | ||
| $r.retention.final.months | Should -Be 30 | ||
| } | ||
| } | ||
|
|
||
| Context 'First deploy' { | ||
|
|
||
| It 'Should honor the requested retention when no settings.json exists' { | ||
| $r = Invoke-CopyScript -ExistingJson $null -Ingestion 6 -Final 8 | ||
| $r.retention.ingestion.months | Should -Be 6 -Because 'there is no stored value to protect on a brand-new hub' | ||
| $r.retention.final.months | Should -Be 8 | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking: this note is good, but the parameter reference still describes these parameters as if they always apply. In
docs-mslearn/toolkit/hubs/template.md(lines 95 and 97),ingestionRetentionInMonthsanddataExplorerFinalRetentionInMonthsonly say "Number of months of data to retain …". After this PR, both values apply only when nothing is stored yet. Someone who redeploys from the portal and picks 36 months to increase retention is now ignored without any message. Consider adding a sentence to both rows, for example: "Applied only when no value is stored in config/settings.json. To change it later, edit that file."