Skip to content

Resource Group - ManagedBy NULL/Empty String issue #4351

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

Closed
coolhome wants to merge 1 commit into from
Closed

Resource Group - ManagedBy NULL/Empty String issue #4351

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
1 change: 0 additions & 1 deletion modules/resources/resource-group/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ The ID of the resource that manages this resource group.

- Required: No
- Type: string
- Default: `''`

### Parameter: `roleAssignments`

Expand Down
25 changes: 16 additions & 9 deletions modules/resources/resource-group/main.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ param roleAssignments roleAssignmentType
param tags object?

@description('Optional. The ID of the resource that manages this resource group.')
param managedBy string = ''
param managedBy string?

@description('Optional. Enable telemetry via a Globally Unique Identifier (GUID).')
param enableDefaultTelemetry bool = true
Expand Down Expand Up @@ -51,25 +51,32 @@ resource defaultTelemetry 'Microsoft.Resources/deployments@2021-04-01' = if (ena
}
}

resource resourceGroup 'Microsoft.Resources/resourceGroups@2021-04-01' = {
resource resourceGroup 'Microsoft.Resources/resourceGroups@2021-04-01' = if (managedBy == null) {
location: location
name: name
tags: tags
managedBy: managedBy
properties: {}
}

resource managedResourceGroup 'Microsoft.Resources/resourceGroups@2021-04-01' = if (managedBy != null) {
location: location
name: name
tags: tags
managedBy: length(managedBy ?? '') > 0 ? managedBy : ''
Copy link
Copy Markdown
Contributor

@AlexanderSehr AlexanderSehr Dec 10, 2023

Choose a reason for hiding this comment

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

Suggested change
managedBy: length(managedBy ?? '') > 0 ? managedBy : ''
managedBy: managedBy ?? ''

If I'm not mistaken, this should do the trick to. Even better would be only mangedBy: mangedBy, IF null is accepted by the RP.

Copy link
Copy Markdown
Contributor Author

@coolhome coolhome Dec 12, 2023

Choose a reason for hiding this comment

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

That is correct, but the RP does not accept null. If its json property exists it has to be a string, which makes this problem hard. This is why I've had challenges figuring out who to report this to CARML/AVM, Bicep, or Azure Support for RP.

I think the RP should accept null and require a string length greater than 0. Regardless it's a breaking change if someone used this module to create the resource group because it has to be deleted to remove managedBy AFAIK.

Copy link
Copy Markdown
Contributor

@AlexanderSehr AlexanderSehr Dec 13, 2023

Choose a reason for hiding this comment

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

In the corresponding AVM module the owner now opted to not expose the ability to write to this property. This won't impact the RP but at least it wouldn't cause any other issues for users of the module.

properties: {}
}

module resourceGroup_lock 'modules/nested_lock.bicep' = if (!empty(lock ?? {}) && lock.?kind != 'None') {
name: '${uniqueString(deployment().name, location)}-RG-Lock'
params: {
lock: lock
name: resourceGroup.name
name: managedBy == null ? resourceGroup.name : managedResourceGroup.name
}
scope: resourceGroup
scope: managedBy == null ? resourceGroup : managedResourceGroup
}

resource resourceGroup_roleAssignments 'Microsoft.Authorization/roleAssignments@2022-04-01' = [for (roleAssignment, index) in (roleAssignments ?? []): {
name: guid(resourceGroup.id, roleAssignment.principalId, roleAssignment.roleDefinitionIdOrName)
name: guid(managedBy == null ? resourceGroup.id : managedResourceGroup.id, roleAssignment.principalId, roleAssignment.roleDefinitionIdOrName)
properties: {
roleDefinitionId: contains(builtInRoleNames, roleAssignment.roleDefinitionIdOrName) ? builtInRoleNames[roleAssignment.roleDefinitionIdOrName] : contains(roleAssignment.roleDefinitionIdOrName, '/providers/Microsoft.Authorization/roleDefinitions/') ? roleAssignment.roleDefinitionIdOrName : subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleAssignment.roleDefinitionIdOrName)
principalId: roleAssignment.principalId
Expand All @@ -82,13 +89,13 @@ resource resourceGroup_roleAssignments 'Microsoft.Authorization/roleAssignments@
}]

@description('The name of the resource group.')
output name string = resourceGroup.name
output name string = managedBy == null ? resourceGroup.name : managedResourceGroup.name

@description('The resource ID of the resource group.')
output resourceId string = resourceGroup.id
output resourceId string = managedBy == null ? resourceGroup.id : managedResourceGroup.id

@description('The location the resource was deployed into.')
output location string = resourceGroup.location
output location string = managedBy == null ? resourceGroup.location : managedResourceGroup.location

// =============== //
// Definitions //
Expand Down
32 changes: 21 additions & 11 deletions modules/resources/resource-group/main.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"_generator": {
"name": "bicep",
"version": "0.23.1.45101",
"templateHash": "4157027857802113569"
"templateHash": "14745510264593051323"
},
"name": "Resource Groups",
"description": "This module deploys a Resource Group.",
Expand Down Expand Up @@ -140,7 +140,7 @@
},
"managedBy": {
"type": "string",
"defaultValue": "",
"nullable": true,
"metadata": {
"description": "Optional. The ID of the resource that manages this resource group."
}
Expand Down Expand Up @@ -184,12 +184,22 @@
}
},
"resourceGroup": {
"condition": "[equals(parameters('managedBy'), null())]",
"type": "Microsoft.Resources/resourceGroups",
"apiVersion": "2021-04-01",
"name": "[parameters('name')]",
"location": "[parameters('location')]",
"tags": "[parameters('tags')]",
"managedBy": "[parameters('managedBy')]",
"properties": {}
},
"managedResourceGroup": {
"condition": "[not(equals(parameters('managedBy'), null()))]",
"type": "Microsoft.Resources/resourceGroups",
"apiVersion": "2021-04-01",
"name": "[parameters('name')]",
"location": "[parameters('location')]",
"tags": "[parameters('tags')]",
"managedBy": "[if(greater(length(coalesce(parameters('managedBy'), '')), 0), parameters('managedBy'), '')]",
"properties": {}
},
"resourceGroup_roleAssignments": {
Expand All @@ -199,7 +209,7 @@
},
"type": "Microsoft.Authorization/roleAssignments",
"apiVersion": "2022-04-01",
"name": "[guid(subscriptionResourceId('Microsoft.Resources/resourceGroups', parameters('name')), coalesce(parameters('roleAssignments'), createArray())[copyIndex()].principalId, coalesce(parameters('roleAssignments'), createArray())[copyIndex()].roleDefinitionIdOrName)]",
"name": "[guid(if(equals(parameters('managedBy'), null()), subscriptionResourceId('Microsoft.Resources/resourceGroups', parameters('name')), subscriptionResourceId('Microsoft.Resources/resourceGroups', parameters('name'))), coalesce(parameters('roleAssignments'), createArray())[copyIndex()].principalId, coalesce(parameters('roleAssignments'), createArray())[copyIndex()].roleDefinitionIdOrName)]",
"properties": {
"roleDefinitionId": "[if(contains(variables('builtInRoleNames'), coalesce(parameters('roleAssignments'), createArray())[copyIndex()].roleDefinitionIdOrName), variables('builtInRoleNames')[coalesce(parameters('roleAssignments'), createArray())[copyIndex()].roleDefinitionIdOrName], if(contains(coalesce(parameters('roleAssignments'), createArray())[copyIndex()].roleDefinitionIdOrName, '/providers/Microsoft.Authorization/roleDefinitions/'), coalesce(parameters('roleAssignments'), createArray())[copyIndex()].roleDefinitionIdOrName, subscriptionResourceId('Microsoft.Authorization/roleDefinitions', coalesce(parameters('roleAssignments'), createArray())[copyIndex()].roleDefinitionIdOrName)))]",
"principalId": "[coalesce(parameters('roleAssignments'), createArray())[copyIndex()].principalId]",
Expand All @@ -210,6 +220,7 @@
"delegatedManagedIdentityResourceId": "[tryGet(coalesce(parameters('roleAssignments'), createArray())[copyIndex()], 'delegatedManagedIdentityResourceId')]"
},
"dependsOn": [
"managedResourceGroup",
"resourceGroup"
]
},
Expand All @@ -218,7 +229,7 @@
"type": "Microsoft.Resources/deployments",
"apiVersion": "2022-09-01",
"name": "[format('{0}-RG-Lock', uniqueString(deployment().name, parameters('location')))]",
"resourceGroup": "[parameters('name')]",
"location": "[deployment().location]",
"properties": {
"expressionEvaluationOptions": {
"scope": "inner"
Expand All @@ -228,9 +239,7 @@
"lock": {
"value": "[parameters('lock')]"
},
"name": {
"value": "[parameters('name')]"
}
"name": "[if(equals(parameters('managedBy'), null()), createObject('value', parameters('name')), createObject('value', parameters('name')))]"
},
"template": {
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
Expand Down Expand Up @@ -299,6 +308,7 @@
}
},
"dependsOn": [
"managedResourceGroup",
"resourceGroup"
]
}
Expand All @@ -309,21 +319,21 @@
"metadata": {
"description": "The name of the resource group."
},
"value": "[parameters('name')]"
"value": "[if(equals(parameters('managedBy'), null()), parameters('name'), parameters('name'))]"
},
"resourceId": {
"type": "string",
"metadata": {
"description": "The resource ID of the resource group."
},
"value": "[subscriptionResourceId('Microsoft.Resources/resourceGroups', parameters('name'))]"
"value": "[if(equals(parameters('managedBy'), null()), subscriptionResourceId('Microsoft.Resources/resourceGroups', parameters('name')), subscriptionResourceId('Microsoft.Resources/resourceGroups', parameters('name')))]"
},
"location": {
"type": "string",
"metadata": {
"description": "The location the resource was deployed into."
},
"value": "[reference('resourceGroup', '2021-04-01', 'full').location]"
"value": "[if(equals(parameters('managedBy'), null()), reference('resourceGroup', '2021-04-01', 'full').location, reference('managedResourceGroup', '2021-04-01', 'full').location)]"
}
}
}