Skip to content

Fix update test resources tagging when tags are empty. Bump max limit #7171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
benbp merged 1 commit into from
Oct 19, 2023
Merged

Fix update test resources tagging when tags are empty. Bump max limit #7171

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion eng/common/TestResources/Update-TestResources.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ param (
[string] $SubscriptionId,

[Parameter()]
[ValidateRange(1, 7*24)]
[ValidateRange(1, 30*24)]
[int] $DeleteAfterHours = 48
)

Expand Down Expand Up @@ -136,6 +136,9 @@ try {

Log "Updating DeleteAfter to '$deleteAfter'"
Write-Warning "Any clean-up scripts running against subscription '$SubscriptionId' may delete resource group '$ResourceGroupName' after $DeleteAfterHours hours."
if (!$resourceGroup.Tags) {
$resourceGroup.Tags = @{}
Copy link
Member

@heaths heaths Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually necessary? Seems more like a bug on the Az PowerShell module. As a workaround, though, I see no harm but if it is a bug, we should make sure it's reported. PowerShell is truthy, so idiomatically so should cmdlets be.

Copy link
Member Author

@benbp benbp Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all json at the API level so I'm not surprised if the underlying value was null that powershell just converted it to $null without initializing empty values?

Copy link
Member

@heaths heaths Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's my assumption as well...but should. If the value is falsy, it should omit the tags in the request or pass an empty object - whatever the control plane requires in that case.

Copy link
Member Author

@benbp benbp Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue here is we try to index into $null a line below, not that we're submitting bad API data.

Copy link
Member

@heaths heaths Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Thanks for the clarification. That said, does sending whatever @{} serializes into cause problems?

Copy link
Member Author

@benbp benbp Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked when I tested it out. The response object is also a hashtable:

$ $g[0].Tags.GetType()

IsPublic IsSerial Name                                     BaseType
-------- -------- ----                                     --------
True     True     Hashtable                                System.Object

}
$resourceGroup.Tags['DeleteAfter'] = $deleteAfter

Log "Updating resource group '$ResourceGroupName'"
Expand Down