Skip to content

Commit

Permalink
fix: Added explicit databricks post-removal (#2345)
Browse files Browse the repository at this point in the history
## Description

- Even though the documentation claims otherwise, managed RGs are not
consistently removed by Databricks
- Fixed the rg naming to align across tests & module (and WAF for that
matter)
- This PR adds custom logic to address this in case the native removal
did not work.
> Note: It's functionality depends on the explicit prefix & suffic set
by the module for the managed-rgs. If they were to change, the test
would need to change


![image](https://github.com/Azure/bicep-registry-modules/assets/5365358/0ae127da-4f28-4198-bd40-f04781800968)

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|
[![avm.res.databricks.workspace](https://github.com/Azure/bicep-registry-modules/actions/workflows/avm.res.databricks.workspace.yml/badge.svg?branch=users%2Falsehr%2FdatabricksRemoval&event=workflow_dispatch)](https://github.com/Azure/bicep-registry-modules/actions/workflows/avm.res.databricks.workspace.yml)
|

## Type of Change

<!-- Use the checkboxes [x] on the options that are relevant. -->

- [x] Update to CI Environment or utilities (Non-module affecting
changes)
- [ ] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [ ] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation
  • Loading branch information
AlexanderSehr authored Jun 18, 2024
1 parent 91517b7 commit 39af1b3
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 53 deletions.
92 changes: 43 additions & 49 deletions avm/res/databricks/workspace/main.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -113,52 +113,47 @@ var builtInRoleNames = {
)
}

resource avmTelemetry 'Microsoft.Resources/deployments@2023-07-01' =
if (enableTelemetry) {
name: '46d3xbcp.res.databricks-workspace.${replace('-..--..-', '.', '-')}.${substring(uniqueString(deployment().name, location), 0, 4)}'
properties: {
mode: 'Incremental'
template: {
'$schema': 'https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#'
contentVersion: '1.0.0.0'
resources: []
outputs: {
telemetry: {
type: 'String'
value: 'For more information, see https://aka.ms/avm/TelemetryInfo'
}
resource avmTelemetry 'Microsoft.Resources/deployments@2023-07-01' = if (enableTelemetry) {
name: '46d3xbcp.res.databricks-workspace.${replace('-..--..-', '.', '-')}.${substring(uniqueString(deployment().name, location), 0, 4)}'
properties: {
mode: 'Incremental'
template: {
'$schema': 'https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#'
contentVersion: '1.0.0.0'
resources: []
outputs: {
telemetry: {
type: 'String'
value: 'For more information, see https://aka.ms/avm/TelemetryInfo'
}
}
}
}
}

resource cMKKeyVault 'Microsoft.KeyVault/vaults@2023-02-01' existing =
if (!empty(customerManagedKey.?keyVaultResourceId)) {
name: last(split((customerManagedKey.?keyVaultResourceId ?? 'dummyVault'), '/'))
scope: resourceGroup(
split((customerManagedKey.?keyVaultResourceId ?? '//'), '/')[2],
split((customerManagedKey.?keyVaultResourceId ?? '////'), '/')[4]
)
resource cMKKeyVault 'Microsoft.KeyVault/vaults@2023-02-01' existing = if (!empty(customerManagedKey.?keyVaultResourceId)) {
name: last(split((customerManagedKey.?keyVaultResourceId ?? 'dummyVault'), '/'))
scope: resourceGroup(
split((customerManagedKey.?keyVaultResourceId ?? '//'), '/')[2],
split((customerManagedKey.?keyVaultResourceId ?? '////'), '/')[4]
)

resource cMKKey 'keys@2023-02-01' existing =
if (!empty(customerManagedKey.?keyVaultResourceId) && !empty(customerManagedKey.?keyName)) {
name: customerManagedKey.?keyName ?? 'dummyKey'
}
resource cMKKey 'keys@2023-02-01' existing = if (!empty(customerManagedKey.?keyVaultResourceId) && !empty(customerManagedKey.?keyName)) {
name: customerManagedKey.?keyName ?? 'dummyKey'
}
}

resource cMKManagedDiskKeyVault 'Microsoft.KeyVault/vaults@2023-02-01' existing =
if (!empty(customerManagedKeyManagedDisk.?keyVaultResourceId)) {
name: last(split((customerManagedKeyManagedDisk.?keyVaultResourceId ?? 'dummyVault'), '/'))
scope: resourceGroup(
split((customerManagedKeyManagedDisk.?keyVaultResourceId ?? '//'), '/')[2],
split((customerManagedKeyManagedDisk.?keyVaultResourceId ?? '////'), '/')[4]
)
resource cMKManagedDiskKeyVault 'Microsoft.KeyVault/vaults@2023-02-01' existing = if (!empty(customerManagedKeyManagedDisk.?keyVaultResourceId)) {
name: last(split((customerManagedKeyManagedDisk.?keyVaultResourceId ?? 'dummyVault'), '/'))
scope: resourceGroup(
split((customerManagedKeyManagedDisk.?keyVaultResourceId ?? '//'), '/')[2],
split((customerManagedKeyManagedDisk.?keyVaultResourceId ?? '////'), '/')[4]
)

resource cMKKey 'keys@2023-02-01' existing =
if (!empty(customerManagedKeyManagedDisk.?keyVaultResourceId) && !empty(customerManagedKeyManagedDisk.?keyName)) {
name: customerManagedKeyManagedDisk.?keyName ?? 'dummyKey'
}
resource cMKKey 'keys@2023-02-01' existing = if (!empty(customerManagedKeyManagedDisk.?keyVaultResourceId) && !empty(customerManagedKeyManagedDisk.?keyName)) {
name: customerManagedKeyManagedDisk.?keyName ?? 'dummyKey'
}
}

resource workspace 'Microsoft.Databricks/workspaces@2023-02-01' = {
name: name
Expand All @@ -170,7 +165,7 @@ resource workspace 'Microsoft.Databricks/workspaces@2023-02-01' = {
properties: {
managedResourceGroupId: !empty(managedResourceGroupResourceId)
? managedResourceGroupResourceId
: '${subscription().id}/resourceGroups/${name}-rg'
: '${subscription().id}/resourceGroups/rg-${name}-managed'
parameters: union(
// Always added parameters
{
Expand Down Expand Up @@ -271,7 +266,7 @@ resource workspace 'Microsoft.Databricks/workspaces@2023-02-01' = {
keyVaultUri: cMKKeyVault.properties.vaultUri
keyName: customerManagedKey!.keyName
keyVersion: !empty(customerManagedKey.?keyVersion ?? '')
? customerManagedKey!.keyVersion
? customerManagedKey!.keyVersion!
: last(split(cMKKeyVault::cMKKey.properties.keyUriWithVersion, '/'))
}
}
Expand All @@ -283,7 +278,7 @@ resource workspace 'Microsoft.Databricks/workspaces@2023-02-01' = {
keyVaultUri: cMKManagedDiskKeyVault.properties.vaultUri
keyName: customerManagedKeyManagedDisk!.keyName
keyVersion: !empty(customerManagedKeyManagedDisk.?keyVersion ?? '')
? customerManagedKeyManagedDisk!.keyVersion
? customerManagedKeyManagedDisk!.keyVersion!
: last(split(cMKManagedDiskKeyVault::cMKKey.properties.keyUriWithVersion, '/'))
}
rotationToLatestKeyVersionEnabled: customerManagedKeyManagedDisk.?rotationToLatestKeyVersionEnabled ?? true
Expand All @@ -295,17 +290,16 @@ resource workspace 'Microsoft.Databricks/workspaces@2023-02-01' = {
}
}

resource workspace_lock 'Microsoft.Authorization/locks@2020-05-01' =
if (!empty(lock ?? {}) && lock.?kind != 'None') {
name: lock.?name ?? 'lock-${name}'
properties: {
level: lock.?kind ?? ''
notes: lock.?kind == 'CanNotDelete'
? 'Cannot delete resource or child resources.'
: 'Cannot delete or modify the resource or child resources.'
}
scope: workspace
resource workspace_lock 'Microsoft.Authorization/locks@2020-05-01' = if (!empty(lock ?? {}) && lock.?kind != 'None') {
name: lock.?name ?? 'lock-${name}'
properties: {
level: lock.?kind ?? ''
notes: lock.?kind == 'CanNotDelete'
? 'Cannot delete resource or child resources.'
: 'Cannot delete or modify the resource or child resources.'
}
scope: workspace
}

// Note: Diagnostic Settings are only supported by the premium tier
resource workspace_diagnosticSettings 'Microsoft.Insights/diagnosticSettings@2021-05-01-preview' = [
Expand Down
6 changes: 3 additions & 3 deletions avm/res/databricks/workspace/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"metadata": {
"_generator": {
"name": "bicep",
"version": "0.26.170.59819",
"templateHash": "7425369674344103053"
"version": "0.28.1.47646",
"templateHash": "4409228583583673969"
},
"name": "Azure Databricks Workspaces",
"description": "This module deploys an Azure Databricks Workspace.",
Expand Down Expand Up @@ -739,7 +739,7 @@
"name": "[parameters('skuName')]"
},
"properties": {
"managedResourceGroupId": "[if(not(empty(parameters('managedResourceGroupResourceId'))), parameters('managedResourceGroupResourceId'), format('{0}/resourceGroups/{1}-rg', subscription().id, parameters('name')))]",
"managedResourceGroupId": "[if(not(empty(parameters('managedResourceGroupResourceId'))), parameters('managedResourceGroupResourceId'), format('{0}/resourceGroups/rg-{1}', subscription().id, parameters('name')))]",
"parameters": "[union(createObject('enableNoPublicIp', createObject('value', parameters('disablePublicIp')), 'prepareEncryption', createObject('value', parameters('prepareEncryption')), 'vnetAddressPrefix', createObject('value', parameters('vnetAddressPrefix')), 'requireInfrastructureEncryption', createObject('value', parameters('requireInfrastructureEncryption'))), if(not(empty(parameters('customVirtualNetworkResourceId'))), createObject('customVirtualNetworkId', createObject('value', parameters('customVirtualNetworkResourceId'))), createObject()), if(not(empty(parameters('amlWorkspaceResourceId'))), createObject('amlWorkspaceId', createObject('value', parameters('amlWorkspaceResourceId'))), createObject()), if(not(empty(parameters('customPrivateSubnetName'))), createObject('customPrivateSubnetName', createObject('value', parameters('customPrivateSubnetName'))), createObject()), if(not(empty(parameters('customPublicSubnetName'))), createObject('customPublicSubnetName', createObject('value', parameters('customPublicSubnetName'))), createObject()), if(not(empty(parameters('loadBalancerBackendPoolName'))), createObject('loadBalancerBackendPoolName', createObject('value', parameters('loadBalancerBackendPoolName'))), createObject()), if(not(empty(parameters('loadBalancerResourceId'))), createObject('loadBalancerId', createObject('value', parameters('loadBalancerResourceId'))), createObject()), if(not(empty(parameters('natGatewayName'))), createObject('natGatewayName', createObject('value', parameters('natGatewayName'))), createObject()), if(not(empty(parameters('publicIpName'))), createObject('publicIpName', createObject('value', parameters('publicIpName'))), createObject()), if(not(empty(parameters('storageAccountName'))), createObject('storageAccountName', createObject('value', parameters('storageAccountName'))), createObject()), if(not(empty(parameters('storageAccountSkuName'))), createObject('storageAccountSkuName', createObject('value', parameters('storageAccountSkuName'))), createObject()))]",
"publicNetworkAccess": "[parameters('publicNetworkAccess')]",
"requiredNsgRules": "[parameters('requiredNsgRules')]",
Expand Down
1 change: 1 addition & 0 deletions avm/res/databricks/workspace/tests/e2e/max/main.test.bicep
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ module testDeployment '../../../main.bicep' = [
service: 'browser_authentication'
}
]
// Please do not change the name of the managed resource group as the CI's removal logic relies on it
managedResourceGroupResourceId: '${subscription().id}/resourceGroups/rg-${resourceGroupName}-managed'
requireInfrastructureEncryption: true
vnetAddressPrefix: '10.100'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ module testDeployment '../../../main.bicep' = [
}
}
]
// Please do not change the name of the managed resource group as the CI's removal logic relies on it
managedResourceGroupResourceId: '${subscription().id}/resourceGroups/rg-${resourceGroupName}-managed'
requireInfrastructureEncryption: true
vnetAddressPrefix: '10.100'
Expand Down
2 changes: 1 addition & 1 deletion avm/res/databricks/workspace/version.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "https://aka.ms/bicep-registry-module-version-file-schema#",
"version": "0.4",
"version": "0.5",
"pathFilters": [
"./main.json"
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,29 @@ function Invoke-ResourcePostRemoval {
$null = Set-AzRecoveryServicesVaultProperty -VaultId $vaultId -SoftDeleteFeatureState $softDeleteStatus.TrimEnd('d')
break
}
'Microsoft.Databricks/workspaces' {
$resourceGroupName = $ResourceId.Split('/')[4]
$resourceName = Split-Path $ResourceId -Leaf
# If the `managedResourceGroupResourceId` parameter was set during deployment, we should look for that resource group and remove it as it is not automatically removed
# NOTE: This requires that the provided value uses the prefix `rg-` and suffix '-managed'
$managedResourceGroupName = "rg-$resourceGroupName-managed"
if (Get-AzResourceGroup -Name $managedResourceGroupName -ErrorAction 'SilentlyContinue') {
Write-Verbose ('[*] Removing managed resource group [{0}] of workspace [{1}]' -f $managedResourceGroupName, $resourceName) -Verbose
if ($PSCmdlet.ShouldProcess(('Managed resource group [{0}]' -f $managedResourceGroupName), 'Remove')) {
$null = Remove-AzResourceGroup -Name $managedResourceGroupName -Force
}
}
# If the `managedResourceGroupResourceId` parameter was NOT set during deployment, we should look for a resource group with the default name and remove it as it is not automatically removed
# NOTE: This requires that the default value uses the prefix 'rg-'
$defaultManagedResourceGroupName = "rg-$resourceName-managed"
if (Get-AzResourceGroup -Name $defaultManagedResourceGroupName -ErrorAction 'SilentlyContinue') {
Write-Verbose ('[*] Removing managed resource group [{0}] of workspace [{1}]' -f $defaultManagedResourceGroupName, $resourceName) -Verbose
if ($PSCmdlet.ShouldProcess(('Managed resource group [{0}]' -f $defaultManagedResourceGroupName), 'Remove')) {
$null = Remove-AzResourceGroup -Name $defaultManagedResourceGroupName -Force
}
}
break
}
### CODE LOCATION: Add custom post-removal operation here
}
}

0 comments on commit 39af1b3

Please sign in to comment.