add support for enabling delete protection #268
Conversation
DaanHoogland
requested review from
Pearl1594,
Copilot and
vishesh92
and removed request for
Pearl1594 and
vishesh92
There was a problem hiding this comment.
Pull request overview
This pull request adds delete protection support for CloudStack instances (virtual machines) and volumes (disks) by introducing a new deleteprotection boolean field to both the cloudstack_instance and cloudstack_disk resources. The feature integrates with CloudStack's API updateVirtualMachine and updateVolume endpoints to enable protection against accidental deletion.
Key Changes:
- Added
deleteprotectionoptional boolean field to instance and disk resource schemas - Implemented delete protection setting via CloudStack update APIs after resource creation
- Updated documentation for both resources with usage notes about limitations when resources are managed by other services
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
cloudstack/resource_cloudstack_instance.go |
Added deleteprotection schema field and implementation to set/update protection via UpdateVirtualMachine API |
cloudstack/resource_cloudstack_disk.go |
Added deleteprotection schema field and implementation to set/update protection via UpdateVolume API |
website/docs/r/instance.html.markdown |
Documented deleteprotection parameter with usage notes and reformatted userdata_id/userdata_details entries |
website/docs/r/disk.html.markdown |
Documented deleteprotection parameter with usage notes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Set deleteprotection using UpdateVolume | ||
| if v, ok := d.GetOk("deleteprotection"); ok { | ||
| // p_update := cs.Volume.NewUpdateVolumeParams() | ||
| // p_update.SetDeleteprotection(v.(bool)) |
There was a problem hiding this comment.
Commented-out code should be removed. If this code is not needed, it should be deleted rather than left as a comment.
| // p_update.SetDeleteprotection(v.(bool)) |
| * `reattach_on_change` - (Optional) Determines whether or not to detach the disk volume | ||
| from the virtual machine on disk offering or size change. | ||
|
|
||
| * `deleteprotection` - (Optional) Set delete protection for the volume. If true, The volume will be protected from deletion. |
There was a problem hiding this comment.
Capitalization inconsistency: "The" should be lowercase as it continues from "If true,".
| * `deleteprotection` - (Optional) Set delete protection for the volume. If true, The volume will be protected from deletion. | |
| * `deleteprotection` - (Optional) Set delete protection for the volume. If true, the volume will be protected from deletion. |
|
|
||
| "deleteprotection": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, |
There was a problem hiding this comment.
The deleteprotection field should be marked as Computed: true in addition to Optional: true. This allows Terraform to properly track the value when it's read back from the API, ensuring state synchronization and preventing unnecessary diffs.
Similar to the attach field (lines 47-51), update the schema to:
"deleteprotection": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
},| Optional: true, | |
| Optional: true, | |
| Computed: true, |
|
|
||
| "deleteprotection": { | ||
| Type: schema.TypeBool, | ||
| Optional: true, |
There was a problem hiding this comment.
The deleteprotection field should be marked as Computed: true in addition to Optional: true. This allows Terraform to properly track the value when it's read back from the API, ensuring state synchronization and preventing unnecessary diffs.
Update the schema to:
"deleteprotection": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
},| Optional: true, | |
| Optional: true, | |
| Computed: true, |
| // Set deleteprotection using UpdateVirtualMachine | ||
| if v, ok := d.GetOk("deleteprotection"); ok { | ||
| // p_update := cs.VirtualMachine.NewUpdateVirtualMachineParams(r.Id) | ||
| // p_update.SetDeleteprotection(v.(bool)) |
There was a problem hiding this comment.
Commented-out code should be removed. If this code is not needed, it should be deleted rather than left as a comment.
| // p_update.SetDeleteprotection(v.(bool)) |
1 participant
Description
Add support for enabling the delete protection for instances (virtual machines) and volumes (disks).
Reason
This feature allows users to enable the delete protection for certain resources to prevent accidental deletion.
References
Changes
Added deleteprotection option to
cloudstack_instanceandcloudstack_diskresource and updated the corresponding documentation pages.cloudstack/resource_cloudstack_instance.gocloudstack/resource_cloudstack_disk.gowebsite/docs/r/instance.html.markdownwebsite/docs/r/disk.html.markdown