Skip to content
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.

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

[CI Environment] Do not delete tags that already exist on subscription level #3280

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion modules/resources/tags/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ This module deploys Resources Tags on a subscription or resource group scope.
| :-- | :-- | :-- | :-- |
| `enableDefaultTelemetry` | bool | `True` | Enable telemetry via a Globally Unique Identifier (GUID). |
| `location` | string | `[deployment().location]` | Location deployment metadata. |
| `onlyUpdate` | bool | `False` | Instead of overwriting the existing tags, combine them with the new tags. |
| `onlyUpdate` | bool | `True` | Instead of overwriting the existing tags, combine them with the new tags. |
| `resourceGroupName` | string | `''` | Name of the Resource Group to assign the tags to. If no Resource Group name is provided, and Subscription ID is provided, the module deploys at subscription level, therefore assigns the provided tags to the subscription. |
| `subscriptionId` | string | `[subscription().id]` | Subscription ID of the subscription to assign the tags to. If no Resource Group name is provided, the module deploys at subscription level, therefore assigns the provided tags to the subscription. |
| `tags` | object | `{object}` | Tags for the resource group. If not provided, removes existing tags. |
Expand Down
2 changes: 1 addition & 1 deletion modules/resources/tags/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ targetScope = 'subscription'
param tags object = {}

@description('Optional. Instead of overwriting the existing tags, combine them with the new tags.')
param onlyUpdate bool = false
param onlyUpdate bool = true

@description('Optional. Name of the Resource Group to assign the tags to. If no Resource Group name is provided, and Subscription ID is provided, the module deploys at subscription level, therefore assigns the provided tags to the subscription.')
param resourceGroupName string = ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,31 @@ function Invoke-ResourceRemoval {
}
break
}
'Microsoft.Resources/tags' {
# Get current tags on the subscription
$subscriptionId = $resourceId.Split('/')[2]
$currentTags = $(Get-AzTag -ResourceId /subscriptions/$subscriptionId).Properties

# Get the tags to remove
$tagsToRemove = @('Test', 'TestToo')
Copy link
Contributor

Choose a reason for hiding this comment

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

Tags provided by deployment validation tests are just for testing purposes and shouldn't be hardcoded in the CI environment utilities.

If the entire tags object gets deleted even if onlyUpdate is set to true, then I agree that an issue should be opened to address it. @ntaheij could you please open a bug in the board explaining the actual and indended behavior?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the response. Currently, yes, onlyUpdate does delete all tags. Issue is that, when deploying the tests for the module, currently all tags are being deleted, which means tags for i.e. billing are not available. I will open a bug.

Copy link
Author

Choose a reason for hiding this comment

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

If there is any way to only delete the tags that are created, I'd be interested to use that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @ntaheij! I'm going to put the PR in draft for the moment and reference the PR in the issue

Copy link
Contributor

Choose a reason for hiding this comment

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

Note, I replied in the issue :)

$tagsToKeep = @{}

# Loop over each key and add it to the new tags if it is not in the list of tags to remove
$currentTags.TagsProperty.Keys | ForEach-Object {
$key = $_
if ($tagsToRemove -notcontains $key) {
$value = $currentTags.TagsProperty.$key
$tagsToKeep.Add($key, $value)
}
}

$null = Remove-AzTag -ResourceId /subscriptions/$subscriptionId

if ($tagsToKeep.count -ne 0) {
$null = Update-AzTag -ResourceId /subscriptions/$subscriptionId -Tag $tagsToKeep -Operation Replace
}
break
}
### CODE LOCATION: Add custom removal action here
Default {
$null = Remove-AzResource -ResourceId $resourceId -Force -ErrorAction 'Stop'
Expand Down