From 48ef4bb95c9033106f67631a30ac6778a668807d Mon Sep 17 00:00:00 2001 From: John Collinson <13622412+johncollinson2001@users.noreply.github.com> Date: Thu, 3 Oct 2024 22:01:12 +0100 Subject: [PATCH 1/2] Added validation to backup policy retention period. --- README.md | 5 +- infrastructure/variables.tf | 33 ++++++- .../backup_modules_blob_storage.tftest.hcl | 83 +++++++++++++++- .../backup_modules_managed_disk.tftest.hcl | 98 ++++++++++++++++++- 4 files changed, 208 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 3354b7f..6bdca45 100644 --- a/README.md +++ b/README.md @@ -147,14 +147,15 @@ To deploy the module an Azure identity (typically an app registration with clien | `vault_name` | The name of the backup vault. The value supplied will be automatically prefixed with `rg-nhsbackup-`. If more than one az-backup module is created, this value must be unique across them. | Yes | n/a | | `vault_location` | The location of the resource group that is created to contain the vault. | No | `uksouth` | | `vault_redundancy` | The redundancy of the vault, e.g. `GeoRedundant`. [See the following link for the possible values](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/data_protection_backup_vault#redundancy) | No | `LocallyRedundant` | +| `use_extended_retention` | If set to on, then retention periods can be configured up to 365 days (otherwise they are limited to 7 days). | No | `false` | | `blob_storage_backups` | A map of blob storage backups that should be created. For each backup the following values should be provided: `storage_account_id`, `backup_name` and `retention_period`. When no value is provided then no backups are created. | No | n/a | | `blob_storage_backups.storage_account_id` | The id of the storage account that should be backed up. | Yes | n/a | | `blob_storage_backups.backup_name` | The name of the backup, which must be unique across blob storage backups. | Yes | n/a | -| `blob_storage_backups.retention_period` | How long the backed up data will be retained for, which should be in `ISO 8601` duration format. [See the following link for the possible values](https://en.wikipedia.org/wiki/ISO_8601#Durations). | Yes | n/a | +| `blob_storage_backups.retention_period` | How long the backed up data will be retained for, which should be in `ISO 8601` duration format. This must be specified in days, and can be up to 7 days unless `use_extended_retention` is on, in which case it can be up to 365 days. [See the following link for more information about the format](https://en.wikipedia.org/wiki/ISO_8601#Durations). | Yes | n/a | | `managed_disk_backups` | A map of managed disk backups that should be created. For each backup the following values should be provided: `managed_disk_id`, `backup_name` and `retention_period`. When no value is provided then no backups are created. | No | n/a | | `managed_disk_backups.managed_disk_id` | The id of the managed disk that should be backed up. | Yes | n/a | | `managed_disk_backups.backup_name` | The name of the backup, which must be unique across managed disk backups. | Yes | n/a | -| `managed_disk_backups.retention_period` | How long the backed up data will be retained for, which should be in `ISO 8601` duration format. [See the following link for the possible values](https://en.wikipedia.org/wiki/ISO_8601#Durations). | Yes | n/a | +| `managed_disk_backups.retention_period` | How long the backed up data will be retained for, which should be in `ISO 8601` duration format. This must be specified in days, and can be up to 7 days unless `use_extended_retention` is on, in which case it can be up to 365 days. [See the following link for more information about the format](https://en.wikipedia.org/wiki/ISO_8601#Durations). | Yes | n/a | | `managed_disk_backups.backup_intervals` | A list of intervals at which backups should be taken, which should be in `ISO 8601` duration format. [See the following link for the possible values](https://en.wikipedia.org/wiki/ISO_8601#Time_intervals). | Yes | n/a | ## Developer Guide diff --git a/infrastructure/variables.tf b/infrastructure/variables.tf index 73ed877..4de8d92 100644 --- a/infrastructure/variables.tf +++ b/infrastructure/variables.tf @@ -12,13 +12,35 @@ variable "vault_redundancy" { default = "LocallyRedundant" } +variable "use_extended_retention" { + type = bool + default = false +} + +locals { + valid_retention_periods = ( + var.use_extended_retention + ? [for days in range(1, 366) : "P${days}D"] + : [for days in range(1, 8) : "P${days}D"] + ) +} + variable "blob_storage_backups" { type = map(object({ backup_name = string retention_period = string storage_account_id = string })) + default = {} + + validation { + condition = alltrue([ + for k, v in var.blob_storage_backups : + contains(local.valid_retention_periods, v.retention_period) + ]) + error_message = "Invalid retention period. Valid periods are up to 7 days. If extended retention is enabled, valid periods are any duration less than 365 days (e.g., P30D, P60D, etc.)." + } } variable "managed_disk_backups" { @@ -32,5 +54,14 @@ variable "managed_disk_backups" { name = string }) })) + default = {} -} \ No newline at end of file + + validation { + condition = alltrue([ + for k, v in var.managed_disk_backups : + contains(local.valid_retention_periods, v.retention_period) + ]) + error_message = "Invalid retention period. Valid periods are up to 7 days. If extended retention is enabled, valid periods are any duration less than 365 days (e.g., P30D, P60D, etc.)." + } +} diff --git a/tests/integration-tests/backup_modules_blob_storage.tftest.hcl b/tests/integration-tests/backup_modules_blob_storage.tftest.hcl index 4f3f87f..7de0b6d 100644 --- a/tests/integration-tests/backup_modules_blob_storage.tftest.hcl +++ b/tests/integration-tests/backup_modules_blob_storage.tftest.hcl @@ -21,12 +21,12 @@ run "create_blob_storage_backup" { blob_storage_backups = { backup1 = { backup_name = "storage1" - retention_period = "P7D" + retention_period = "P1D" storage_account_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Storage/storageAccounts/sastorage1" } backup2 = { backup_name = "storage2" - retention_period = "P30D" + retention_period = "P7D" storage_account_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Storage/storageAccounts/sastorage2" } } @@ -53,7 +53,7 @@ run "create_blob_storage_backup" { } assert { - condition = module.blob_storage_backup["backup1"].backup_policy.operational_default_retention_duration == "P7D" + condition = module.blob_storage_backup["backup1"].backup_policy.operational_default_retention_duration == "P1D" error_message = "Blob storage backup policy retention period not as expected." } @@ -103,7 +103,7 @@ run "create_blob_storage_backup" { } assert { - condition = module.blob_storage_backup["backup2"].backup_policy.operational_default_retention_duration == "P30D" + condition = module.blob_storage_backup["backup2"].backup_policy.operational_default_retention_duration == "P7D" error_message = "Blob storage backup policy retention period not as expected." } @@ -136,4 +136,79 @@ run "create_blob_storage_backup" { condition = module.blob_storage_backup["backup2"].backup_instance.backup_policy_id == module.blob_storage_backup["backup2"].backup_policy.id error_message = "Blob storage backup instance backup policy id not as expected." } +} + +run "validate_blob_storage_backup_retention" { + command = plan + + module { + source = "../../infrastructure" + } + + variables { + vault_name = run.setup_tests.vault_name + vault_location = "uksouth" + blob_storage_backups = { + backup1 = { + backup_name = "storage1" + retention_period = "P30D" + storage_account_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Storage/storageAccounts/sastorage1" + } + } + } + + expect_failures = [ + var.blob_storage_backups, + ] +} + +run "validate_blob_storage_backup_retention_with_extended_retention_valid" { + command = plan + + module { + source = "../../infrastructure" + } + + variables { + vault_name = run.setup_tests.vault_name + vault_location = "uksouth" + use_extended_retention = true + blob_storage_backups = { + backup1 = { + backup_name = "storage1" + retention_period = "P30D" + storage_account_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Storage/storageAccounts/sastorage1" + } + } + } + + assert { + condition = length(module.blob_storage_backup) == 1 + error_message = "Number of backup modules not as expected." + } +} + +run "validate_blob_storage_backup_retention_with_extended_retention_invalid" { + command = plan + + module { + source = "../../infrastructure" + } + + variables { + vault_name = run.setup_tests.vault_name + vault_location = "uksouth" + use_extended_retention = true + blob_storage_backups = { + backup1 = { + backup_name = "storage1" + retention_period = "P366D" + storage_account_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Storage/storageAccounts/sastorage1" + } + } + } + + expect_failures = [ + var.blob_storage_backups, + ] } \ No newline at end of file diff --git a/tests/integration-tests/backup_modules_managed_disk.tftest.hcl b/tests/integration-tests/backup_modules_managed_disk.tftest.hcl index 08fdb6f..520ddb8 100644 --- a/tests/integration-tests/backup_modules_managed_disk.tftest.hcl +++ b/tests/integration-tests/backup_modules_managed_disk.tftest.hcl @@ -21,7 +21,7 @@ run "create_managed_disk_backup" { managed_disk_backups = { backup1 = { backup_name = "disk1" - retention_period = "P7D" + retention_period = "P1D" backup_intervals = ["R/2024-01-01T00:00:00+00:00/P1D"] managed_disk_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Compute/disks/disk-1" managed_disk_resource_group = { @@ -31,7 +31,7 @@ run "create_managed_disk_backup" { } backup2 = { backup_name = "disk2" - retention_period = "P30D" + retention_period = "P7D" backup_intervals = ["R/2024-01-01T00:00:00+00:00/P2D"] managed_disk_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Compute/disks/disk-2" managed_disk_resource_group = { @@ -63,7 +63,7 @@ run "create_managed_disk_backup" { } assert { - condition = module.managed_disk_backup["backup1"].backup_policy.default_retention_duration == "P7D" + condition = module.managed_disk_backup["backup1"].backup_policy.default_retention_duration == "P1D" error_message = "Managed disk backup policy retention period not as expected." } @@ -123,7 +123,7 @@ run "create_managed_disk_backup" { } assert { - condition = module.managed_disk_backup["backup2"].backup_policy.default_retention_duration == "P30D" + condition = module.managed_disk_backup["backup2"].backup_policy.default_retention_duration == "P7D" error_message = "Managed disk backup policy retention period not as expected." } @@ -166,4 +166,94 @@ run "create_managed_disk_backup" { condition = module.managed_disk_backup["backup2"].backup_instance.backup_policy_id == module.managed_disk_backup["backup2"].backup_policy.id error_message = "Managed disk backup instance backup policy id not as expected." } +} + +run "validate_managed_disk_backup_retention" { + command = plan + + module { + source = "../../infrastructure" + } + + variables { + vault_name = run.setup_tests.vault_name + vault_location = "uksouth" + managed_disk_backups = { + backup1 = { + backup_name = "disk1" + retention_period = "P30D" + backup_intervals = ["R/2024-01-01T00:00:00+00:00/P1D"] + managed_disk_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Compute/disks/disk-1" + managed_disk_resource_group = { + id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group1" + name = "example-resource-group1" + } + } + } + } + + expect_failures = [ + var.managed_disk_backups, + ] +} + +run "validate_managed_disk_backup_retention_with_extended_retention_valid" { + command = plan + + module { + source = "../../infrastructure" + } + + variables { + vault_name = run.setup_tests.vault_name + vault_location = "uksouth" + use_extended_retention = true + managed_disk_backups = { + backup1 = { + backup_name = "disk1" + retention_period = "P30D" + backup_intervals = ["R/2024-01-01T00:00:00+00:00/P1D"] + managed_disk_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Compute/disks/disk-1" + managed_disk_resource_group = { + id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group1" + name = "example-resource-group1" + } + } + } + } + + assert { + condition = length(module.managed_disk_backup) == 1 + error_message = "Number of backup modules not as expected." + } +} + +run "validate_managed_disk_backup_retention_with_extended_retention_invalid" { + command = plan + + module { + source = "../../infrastructure" + } + + variables { + vault_name = run.setup_tests.vault_name + vault_location = "uksouth" + use_extended_retention = true + managed_disk_backups = { + backup1 = { + backup_name = "disk1" + retention_period = "P366D" + backup_intervals = ["R/2024-01-01T00:00:00+00:00/P1D"] + managed_disk_id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.Compute/disks/disk-1" + managed_disk_resource_group = { + id = "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group1" + name = "example-resource-group1" + } + } + } + } + + expect_failures = [ + var.managed_disk_backups, + ] } \ No newline at end of file From 6ff8f678fecaf62571696204083a70082f1f71c6 Mon Sep 17 00:00:00 2001 From: John Collinson <13622412+johncollinson2001@users.noreply.github.com> Date: Fri, 4 Oct 2024 12:48:47 +0100 Subject: [PATCH 2/2] Adjusted retention periods in end to end tests so they pass TF validation. --- tests/end-to-end-tests/blob_storage_backup_test.go | 4 ++-- tests/end-to-end-tests/managed_disk_backup_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/end-to-end-tests/blob_storage_backup_test.go b/tests/end-to-end-tests/blob_storage_backup_test.go index 14ce3c7..6968bf9 100644 --- a/tests/end-to-end-tests/blob_storage_backup_test.go +++ b/tests/end-to-end-tests/blob_storage_backup_test.go @@ -66,12 +66,12 @@ func TestBlobStorageBackup(t *testing.T) { blobStorageBackups := map[string]map[string]interface{}{ "backup1": { "backup_name": "blob1", - "retention_period": "P7D", + "retention_period": "P1D", "storage_account_id": *externalResources.StorageAccountOne.ID, }, "backup2": { "backup_name": "blob2", - "retention_period": "P30D", + "retention_period": "P7D", "storage_account_id": *externalResources.StorageAccountTwo.ID, }, } diff --git a/tests/end-to-end-tests/managed_disk_backup_test.go b/tests/end-to-end-tests/managed_disk_backup_test.go index 73bfc28..04289aa 100644 --- a/tests/end-to-end-tests/managed_disk_backup_test.go +++ b/tests/end-to-end-tests/managed_disk_backup_test.go @@ -66,7 +66,7 @@ func TestManagedDiskBackup(t *testing.T) { managedDiskBackups := map[string]map[string]interface{}{ "backup1": { "backup_name": "disk1", - "retention_period": "P7D", + "retention_period": "P1D", "backup_intervals": []string{"R/2024-01-01T00:00:00+00:00/P1D"}, "managed_disk_id": *externalResources.ManagedDiskOne.ID, "managed_disk_resource_group": map[string]interface{}{ @@ -76,7 +76,7 @@ func TestManagedDiskBackup(t *testing.T) { }, "backup2": { "backup_name": "disk2", - "retention_period": "P30D", + "retention_period": "P7D", "backup_intervals": []string{"R/2024-01-01T00:00:00+00:00/P2D"}, "managed_disk_id": *externalResources.ManagedDiskTwo.ID, "managed_disk_resource_group": map[string]interface{}{