New Resource: azurerm_data_protection_backup_policy_data_lake_storage#31179
Conversation
| ForceNew: true, | ||
| ValidateFunc: validation.StringMatch( | ||
| regexp.MustCompile("^[-a-zA-Z0-9]{3,150}$"), | ||
| "DataProtection BackupPolicy name must be 3 - 150 characters long, contain only letters, numbers and hyphens.", |
There was a problem hiding this comment.
| "DataProtection BackupPolicy name must be 3 - 150 characters long, contain only letters, numbers and hyphens.", | |
| "`name` must be 3 - 150 characters long, contain only letters, numbers and hyphens(-).", |
| MaxItems: 1, | ||
| Elem: &pluginsdk.Resource{ | ||
| Schema: map[string]*pluginsdk.Schema{ | ||
| "life_cycle": { |
There was a problem hiding this comment.
since life_cycle is the only nested-property of default_retention_rule, can it be flattened?
There was a problem hiding this comment.
All existing similar resources under https://github.com/hashicorp/terraform-provider-azurerm/tree/main/internal/services/dataprotection already use this structure for that property, so keeping it consistent with the style of other resources would be better, making it easier for users to use and understand.
There was a problem hiding this comment.
Discussed with HC: please flatten this nested prop because it only contains 1 inner prop. See guide: Flattening nested properties in schema-design-considerations.md. It's ok to deviate from the other backup_policy resources.
Please also drop the lifecycle term from the name since it does not appear anywhere in portal.
This block should look like:
default_retention_rule {
duration = "P7D"
}
Note that we cannot flatten "duration" because we want to reserve possibility to add "data_store_type" in the future.
| var model BackupPolicyDataLakeStorageModel | ||
| if err := metadata.Decode(&model); err != nil { | ||
| return fmt.Errorf("decoding: %+v", err) | ||
| } | ||
|
|
||
| client := metadata.Client.DataProtection.BackupPolicyClient | ||
| subscriptionId := metadata.Client.Account.SubscriptionId |
There was a problem hiding this comment.
| var model BackupPolicyDataLakeStorageModel | |
| if err := metadata.Decode(&model); err != nil { | |
| return fmt.Errorf("decoding: %+v", err) | |
| } | |
| client := metadata.Client.DataProtection.BackupPolicyClient | |
| subscriptionId := metadata.Client.Account.SubscriptionId | |
| client := metadata.Client.DataProtection.BackupPolicyClient | |
| subscriptionId := metadata.Client.Account.SubscriptionId | |
| var model BackupPolicyDataLakeStorageModel | |
| if err := metadata.Decode(&model); err != nil { | |
| return fmt.Errorf("decoding: %+v", err) | |
| } |
|
|
||
| * `vault_id` - (Required) The ID of the Backup Vault where the Azure Backup Policy Data Lake Storage should exist. Changing this forces a new resource to be created. | ||
|
|
||
| * `backup_repeating_time_intervals` - (Required) Specifies a list of repeating time interval. It supports weekly back. It should follow `ISO 8601` repeating time interval format. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
| * `backup_repeating_time_intervals` - (Required) Specifies a list of repeating time interval. It supports weekly back. It should follow `ISO 8601` repeating time interval format. Changing this forces a new resource to be created. | |
| * `backup_repeating_time_intervals` - (Required) Specifies a list of repeating time interval. It supports weekly backup. It should follow `ISO 8601` repeating time interval format. Changing this forces a new resource to be created. |
|
|
||
| ## Import | ||
|
|
||
| Azure Backup Policy Data Lake Storage's can be imported using the `resource id`, e.g. |
There was a problem hiding this comment.
| Azure Backup Policy Data Lake Storage's can be imported using the `resource id`, e.g. | |
| Azure Backup Policy Data Lake Storages can be imported using the `resource id`, e.g. |
| } | ||
| resp, err := client.DataProtection.BackupPolicyClient.Get(ctx, *id) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("reading %s: %+v", *id, err) |
There was a problem hiding this comment.
| return nil, fmt.Errorf("reading %s: %+v", *id, err) | |
| return nil, fmt.Errorf("retrieving %s: %+v", *id, err) |
| if err != nil { | ||
| return nil, fmt.Errorf("reading %s: %+v", *id, err) | ||
| } | ||
| return utils.Bool(resp.Model != nil), nil |
There was a problem hiding this comment.
| return utils.Bool(resp.Model != nil), nil | |
| return pointer.To(resp.Model != nil), nil |
| provider "azurerm" { | ||
| features {} | ||
| } |
There was a problem hiding this comment.
could you move the provider definition into each testcase?
|
|
||
| // API has bug, which appears API returns before the resource is fully deleted. Tracked by this issue: https://github.com/Azure/azure-rest-api-specs/issues/38944 | ||
| log.Printf("[DEBUG] Waiting for %s to be fully deleted..", *id) | ||
| stateConf := &pluginsdk.StateChangeConf{ |
There was a problem hiding this comment.
pluginsdk.StateChangeConf is deprecated. Could you use poller instead? (ref)
|
@ms-zhenhua , thanks for the comments. I updated PR. Please take another look. Below is the latest test result.
|
liuwuliuyun
left a comment
There was a problem hiding this comment.
Left some comments for small changes. Thanks
| } | ||
|
|
||
| func (r DataProtectionBackupPolicyDataLakeStorageResource) Arguments() map[string]*pluginsdk.Schema { | ||
| arguments := map[string]*pluginsdk.Schema{ |
There was a problem hiding this comment.
Consider reordering the fields in the Arguments() function to follow the standard pattern: Required fields first (alphabetically but name first), then Optional fields (alphabetically).
|
@liuwuliuyun , thanks for the comments. I updated PR. Please take another look. |
|
|
||
| * `weeks_of_month` - (Optional) Possible values are `First`, `Second`, `Third`, `Fourth` and `Last`. Changing this forces a new resource to be created. | ||
|
|
||
| -> **Note:** When not using `absolute_criteria`, you must use exactly one of `days_of_month` or `days_of_week`. Regarding the remaining two properties, `weeks_of_month` and `months_of_year`, you may use either, both, or neither. If you would like to set multiple intervals, you may do so by using multiple `retention_rule` blocks. |
There was a problem hiding this comment.
days_of_month argument does not exist
|
|
||
| * `retention_rule` - (Optional) One or more `retention_rule` blocks as defined below. The priority of each rule is determined by its order in the list, where the first rule has the highest priority. Changing this forces a new resource to be created. | ||
|
|
||
| * `time_zone` - (Optional) Specifies the Time Zone which should be used by the backup schedule. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Can we provide a link to list of valid values? A link to the internal/services/dataprotection/validate/backup_policy_data_lake_storage_time_zone.go file it will be very helpful to our users.
There was a problem hiding this comment.
I did not find a proper link to add here, and not thinking add a link to go file is good choice.. So added the long list there as possible values.
| Status: pollers.PollingStatusSucceeded, | ||
| } | ||
| pollingInProgress = pollers.PollResult{ | ||
| HttpResponse: nil, |
There was a problem hiding this comment.
AZBP006: redundant nil assignment to pointer field "HttpResponse" - omit the field
Got this error from azurerm-linter. There are 6 other violations. Please run the linter on your local and fix / add explanation if you disagree with the linters.
There was a problem hiding this comment.
❯ azurerm-linter --base main --remote hashi
2026/03/23 03:59:27 Using local git diff mode
2026/03/23 03:59:27 Current branch: dataprotectionbackuppolicyadlsstorage
2026/03/23 03:59:27 Merge-base with hashi/main: 9a03001
2026/03/23 03:59:27 ✓ Found 7 changed files with 968 changed lines
2026/03/23 03:59:27 Changed lines filter: tracking 7 files with 968 changed lines
2026/03/23 03:59:27 Auto-detected 1 changed packages:
2026/03/23 03:59:27 ./internal/services/dataprotection/...
2026/03/23 03:59:27 Loading packages...
2026/03/23 03:59:37 Running analysis...
2026/03/23 03:59:39 ✓ Analysis completed successfully with no issues found
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
weeks_of_month, scheduled_backup_times, months_of_year, days_of_week and absolute_criteria are all optional. Is it valid to create a retention_rule with just name and duration?
Maybe this needs an AtLeastOneOf?
There was a problem hiding this comment.
AtLeastOneOf does not support well for elements in a list.. added a validation logic in the create() function
There was a problem hiding this comment.
Ah right, this always trips me. I remember you've tested the API error is descriptive enough. I think this is fine. Thanks for adding the validation.
|
|
||
| * `weeks_of_month` - (Optional) Possible values are `First`, `Second`, `Third`, `Fourth` and `Last`. Changing this forces a new resource to be created. | ||
|
|
||
| -> **Note:** When not using `absolute_criteria`, you must use exactly one of `weeks_of_month` or `days_of_week`. Regarding the remaining two properties, `weeks_of_month` and `months_of_year`, you may use either, both, or neither. If you would like to set multiple intervals, you may do so by using multiple `retention_rule` blocks. |
There was a problem hiding this comment.
Prefer passive grammar, avoid using 'you'
| -> **Note:** When not using `absolute_criteria`, you must use exactly one of `weeks_of_month` or `days_of_week`. Regarding the remaining two properties, `weeks_of_month` and `months_of_year`, you may use either, both, or neither. If you would like to set multiple intervals, you may do so by using multiple `retention_rule` blocks. | |
| -> **Note:** When not using `absolute_criteria`, exactly one of `weeks_of_month` or `days_of_week` must be used. `weeks_of_month` and `months_of_year` are optional, both can be supplied together. Multiple intervals may be set using multiple `retention_rule` blocks. |
| ValidateFunc: azValidate.ISO8601Duration, | ||
| }, | ||
|
|
||
| "data_protection_backup_vault_id": commonschema.ResourceIDReferenceRequiredForceNew(pointer.To(basebackuppolicyresources.BackupVaultId{})), |
There was a problem hiding this comment.
This is a parent id, it has to be ordered after name. Please also reorder the markdown doc.
Refer to guide-new-resource.md:169
| backup_schedule = ["R/2021-05-23T02:30:00+00:00/P1W"] | ||
| default_retention_duration = "P4M" |
There was a problem hiding this comment.
Keep these tied to the original resource
| backup_schedule = ["R/2021-05-23T02:30:00+00:00/P1W"] | |
| default_retention_duration = "P4M" | |
| backup_schedule = azurerm_data_protection_backup_policy_data_lake_storage.test.backup_schedule | |
| default_retention_duration = azurerm_data_protection_backup_policy_data_lake_storage.test.default_retention_duration |
There was a problem hiding this comment.
This comment is unresolved
gerrytan
left a comment
There was a problem hiding this comment.
Thank you for addressing all the feedbacks @ziyeqf , I have triggered another acctest run: https://hashicorp.teamcity.com/viewQueued.html?itemId=636263
Assuming the result is good, this PR is good, nice work! 👍
| backup_schedule = ["R/2021-05-23T02:30:00+00:00/P1W"] | ||
| default_retention_duration = "P4M" |
There was a problem hiding this comment.
This comment is unresolved
| * `absolute_criteria` - (Optional) Possible values are `AllBackup`, `FirstOfDay`, `FirstOfWeek`, `FirstOfMonth` and `FirstOfYear`. These values mean the first successful backup of the day/week/month/year. Changing this forces a new resource to be created. | ||
|
|
||
| * `days_of_week` - (Optional) Possible values are `Monday`, `Tuesday`, `Wednesday`, `Thursday`, `Friday`, `Saturday` and `Sunday`. Changing this forces a new resource to be created. | ||
|
|
||
| * `weeks_of_month` - (Optional) Possible values are `First`, `Second`, `Third`, `Fourth` and `Last`. Changing this forces a new resource to be created. | ||
|
|
||
| * `months_of_year` - (Optional) Possible values are `January`, `February`, `March`, `April`, `May`, `June`, `July`, `August`, `September`, `October`, `November` and `December`. Changing this forces a new resource to be created. |
There was a problem hiding this comment.
Missing some form of a description for these properties. E.g. The days of the week on which to retain a backup (I think that's what this does?)
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks for making those final changes @ziyeqf - LGTM ✅



Community Note
Description
This PR is to support Azure Data Lake Storage (ADLS) Policy under Backup Vault mentioned in #31156
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_data_protection_backup_policy_data_lake_storageThis is a (please select all that apply):
Related Issue(s)
Fixes # 0000
AI Assistance Disclosure
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.