Support for Data Retention Policies #1385
Conversation
| Description: "Name of the organization. If omitted, organization must be defined in the provider config.", | ||
| Optional: true, | ||
| Computed: true, | ||
| PlanModifiers: []planmodifier.String{ |
There was a problem hiding this comment.
Would it be possible to add a validation that either organization or workspace is specified?
Edit: Or maybe both? I see that both is possible now...I feel like it would be acceptable to say just one or the other should be specified, to reduce ambiguity. But I dont feel strongly
There was a problem hiding this comment.
I left in it because I saw that seemed to be the pattern for some other resources, but I think you're right that it makes a lot more sense this way 👍
| // configuration value updates, the Update logic can be left empty and ensure all | ||
| // configurable schema attributes implement the resource.RequiresReplace() | ||
| // attribute plan modifier. | ||
| resp.Diagnostics.AddError("Update not supported", "The update operation is not supported on this resource. This is a bug in the provider.") |
There was a problem hiding this comment.
I can hit this error if I update a DRP from having a delete_after block to a dont_delete block, or vice versa. Can we force replacement on removal/addition of any blocks?
There was a problem hiding this comment.
Dang, I thought having the requirereplace modifier on the number would be enough 🤔 good catch 👍
| stringplanmodifier.RequiresReplace(), | ||
| }, | ||
| }, | ||
| "workspace_id": schema.StringAttribute{ |
There was a problem hiding this comment.
I am able to generate errors by creating a config with multiple policies specified for the same workspace.
resource "tfe_data_retention_policy" "workspace1_policy" {
workspace_id = tfe_workspace.workspace1.id
delete_older_than {
days = 3
}
}
resource "tfe_data_retention_policy" "workspace1_policy2" {
workspace_id = tfe_workspace.workspace1.id
delete_older_than {
days = 6
}
}
If I start with one policy and then add the other, the policy is replaced with the new value (e.g. it goes from 3 days to 6 days). Then every subsequent apply detects drift, and tries to update the policy back to the other value (3 -> 6 -> 3 -> 6 -> .....)
If I try to modify both policies at once, I get an error as it attempts to delete the same policy twice.
There was a problem hiding this comment.
If I delete one of the configs, it deletes the policy on TFE (as expected). Then further applies crash with the error
│ The plugin.(*GRPCProvider).ReadResource request was cancelled.
╵
Stack trace from the terraform-provider-tfe plugin:
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x8 pc=0x1027f0434]
There was a problem hiding this comment.
Good catch 👍 I knew having multiple resources "overriding each other" on a given workspace or organization would be problematic, but we've ignored that problem in the past. That being said, I didn't expect the removal to cause a nasty error like that. I pushed a "fix" that at least allows for users to remove policies attached to the same workspace/organization.
| }, | ||
| }, | ||
| }, | ||
| Blocks: map[string]schema.Block{ |
There was a problem hiding this comment.
Interesting! This is different than how I imagined the implementation. If we can fix the issue with errors when changing this block then I am open to this, but in case it is helps I was expecting that we would have two different resource types, a tfe_data_retention_policy_dont_delelte and a tfe_data_retention_policy_delete_older.
The two policies tracks a bit closer to how the DRPs are represented in the API. Although the different types may be returned from the same endpoint, the type of their JSONAPI representation is actually different.
There was a problem hiding this comment.
I prefer that too, OR even adding a config block in the workspace/organization itself to avoid the problem of having more than 1 DRP attached to the same resource. BUT I moved away from those ideas for two reasons:
- Manuel and the CLI team had already agreed on a design and I didn't wanna stir the pot.
- Far less "importantly", but I kinda like the idea of reducing the number TFE-only resources that we have.
Description
Add support for Data Retention Policies, only available in Terraform Enterprise
Output from acceptance tests