Skip to content

OCPBUGS-53454: ccoctl: only add owned tag to azure resources on create #876

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
merged 1 commit into from
Jun 30, 2025
Merged

OCPBUGS-53454: ccoctl: only add owned tag to azure resources on create #876

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion pkg/cmd/provisioning/azure/create_managed_identities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ func createManagedIdentities(client *azureclients.AzureClientWrapper, credReqDir

// Ensure the installation resource group exists
if !dryRun {
err := ensureResourceGroup(client, installationResourceGroupName, region, resourceTags)
err := ensureResourceGroup(client, name, installationResourceGroupName, region, resourceTags)
if err != nil {
return errors.Wrap(err, "failed to ensure resource group")
}
Expand Down
34 changes: 22 additions & 12 deletions pkg/cmd/provisioning/azure/create_oidc_issuer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ type: Opaque`

// ensureResourceGroup ensures that a resource group with resourceGroupName exists within the provided region and subscription.
// Resource group tags will be updated to include provided resourceTags if found to be missing from existing resource group tags.
func ensureResourceGroup(client *azureclients.AzureClientWrapper, resourceGroupName, region string, resourceTags map[string]string) error {
func ensureResourceGroup(client *azureclients.AzureClientWrapper, name, resourceGroupName, region string, resourceTags map[string]string) error {
// Check if resource group already exists
needToCreateResourceGroup := false
var rawResponse *http.Response
Expand Down Expand Up @@ -110,7 +110,15 @@ func ensureResourceGroup(client *azureclients.AzureClientWrapper, resourceGroupN
region)
}

mergedResourceTags, needToUpdateResourceGroup := mergeResourceTags(resourceTags, getResourceGroupResp.Tags)
existingTags := map[string]*string{}
if needToCreateResourceGroup {
// Add CCO's "owned" tag to resource tags map
resourceTags[fmt.Sprintf("%s_%s", ownedAzureResourceTagKeyPrefix, name)] = ownedAzureResourceTagValue
} else {
existingTags = getResourceGroupResp.Tags
}

mergedResourceTags, needToUpdateResourceGroup := mergeResourceTags(resourceTags, existingTags)

// Found and validated existing resource group, return
if !needToCreateResourceGroup && !needToUpdateResourceGroup {
Expand Down Expand Up @@ -161,7 +169,7 @@ func mergeResourceTags(one map[string]string, two map[string]*string) (map[strin

// ensureStorageAccount ensures that a storage account with storageAccountName exists within the provided resource group.
// Storage account tags will be updated to include provided resourceTags if found to be missing from existing storage account tags.
func ensureStorageAccount(client *azureclients.AzureClientWrapper, storageAccountName, resourceGroupName, region string, resourceTags map[string]string) error {
func ensureStorageAccount(client *azureclients.AzureClientWrapper, name, storageAccountName, resourceGroupName, region string, resourceTags map[string]string) error {
var existingStorageAccount *armstorage.Account
listAccounts := client.StorageAccountClient.NewListByResourceGroupPager(resourceGroupName, &armstorage.AccountsClientListByResourceGroupOptions{})
for listAccounts.More() {
Expand All @@ -182,11 +190,16 @@ func ensureStorageAccount(client *azureclients.AzureClientWrapper, storageAccoun
}

needToCreateStorageAccount := existingStorageAccount == nil
mergedResourceTags := map[string]*string{}
if !needToCreateStorageAccount {
mergedResourceTags = existingStorageAccount.Tags

existingTags := map[string]*string{}
if needToCreateStorageAccount {
// Add CCO's "owned" tag to resource tags map
resourceTags[fmt.Sprintf("%s_%s", ownedAzureResourceTagKeyPrefix, name)] = ownedAzureResourceTagValue
} else {
existingTags = existingStorageAccount.Tags
}
mergedResourceTags, needToUpdateStorageAccount := mergeResourceTags(resourceTags, mergedResourceTags)

mergedResourceTags, needToUpdateStorageAccount := mergeResourceTags(resourceTags, existingTags)

if !needToCreateStorageAccount && !needToUpdateStorageAccount {
log.Printf("Found existing storage account %s", *existingStorageAccount.ID)
Expand Down Expand Up @@ -408,9 +421,6 @@ func createPodIdentityWebhookConfigSecret(tenantID, outputDir string) error {
// * storage account
// * blob container which hosts OIDC documents
func createOIDCIssuer(client *azureclients.AzureClientWrapper, name, region, oidcResourceGroupName, storageAccountName, blobContainerName, subscriptionID, tenantID, publicKeyPath, outputDir string, resourceTags map[string]string, dryRun bool) (string, error) {
// Add CCO's "owned" tag to resource tags map
resourceTags[fmt.Sprintf("%s_%s", ownedAzureResourceTagKeyPrefix, name)] = ownedAzureResourceTagValue

storageAccountKey := ""
if !dryRun {
// Ensure that the public key file can be read at the publicKeyPath before continuing
Expand All @@ -420,13 +430,13 @@ func createOIDCIssuer(client *azureclients.AzureClientWrapper, name, region, oid
}

// Ensure the resource group exists
err = ensureResourceGroup(client, oidcResourceGroupName, region, resourceTags)
err = ensureResourceGroup(client, name, oidcResourceGroupName, region, resourceTags)
if err != nil {
return "", errors.Wrap(err, "failed to ensure resource group")
}

// Ensure storage account exists
err = ensureStorageAccount(client, storageAccountName, oidcResourceGroupName, region, resourceTags)
err = ensureStorageAccount(client, name, storageAccountName, oidcResourceGroupName, region, resourceTags)
if err != nil {
return "", errors.Wrap(err, "failed to ensure storage account")
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/provisioning/azure/create_oidc_issuer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func TestEnsureResourceGroup(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
mockCtrl := gomock.NewController(t)
mockAzureClientWrapper := test.mockAzureClientWrapper(mockCtrl)
err := ensureResourceGroup(mockAzureClientWrapper, testOIDCResourceGroupName, testRegionName, testUserTags)
err := ensureResourceGroup(mockAzureClientWrapper, testInfraName, testOIDCResourceGroupName, testRegionName, testUserTags)
if test.expectError {
require.Error(t, err, "expected error")
} else {
Expand Down Expand Up @@ -279,7 +279,7 @@ func TestEnsureStorageAccount(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
mockCtrl := gomock.NewController(t)
mockAzureClientWrapper := test.mockAzureClientWrapper(mockCtrl)
err := ensureStorageAccount(mockAzureClientWrapper, testStorageAccountName, testOIDCResourceGroupName, testRegionName, testUserTags)
err := ensureStorageAccount(mockAzureClientWrapper, testInfraName, testStorageAccountName, testOIDCResourceGroupName, testRegionName, testUserTags)
if test.expectError {
require.Error(t, err, "expected error")
} else {
Expand Down