fix: implement Read/Update/Delete for stub resources#285
fix: implement Read/Update/Delete for stub resources#285kristofer-atlas wants to merge 3 commits intoapache:mainfrom
Conversation
- Add Read, Update, Delete for account resource - Add Read, Update for disk_offering resource - Add Read, Delete for domain resource - Add Read for user resource
Add comprehensive and networking test configurations for validating the CloudStack Terraform Provider.
There was a problem hiding this comment.
Pull request overview
This PR implements previously stubbed CRUD functionality for several CloudStack Terraform provider resources to reduce state drift (notably for account/domain associations), and adds new Terraform configurations under tests/ for manual provider verification.
Changes:
- Implement
Read/Updateforcloudstack_account,cloudstack_domain,cloudstack_disk_offering, andcloudstack_user(plusDeletefor disk offering). - Adjust
cloudstack_accountschema to force recreation on immutable user/account fields. - Add new test configurations under
tests/comprehensiveandtests/networking(plus documentation) to exercise provider behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/networking/variables.tf | Adds variables for the networking egress-rule test configuration. |
| tests/networking/terraform.tfvars.example | Example tfvars for the networking test suite. |
| tests/networking/main.tf | Networking test configuration demonstrating security group ingress + egress rules. |
| tests/comprehensive/versions.tf | Defines provider requirements for the comprehensive test suite. |
| tests/comprehensive/variables.tf | Adds variables for the comprehensive test suite inputs. |
| tests/comprehensive/terraform.tfvars.example | Example tfvars for the comprehensive test suite. |
| tests/comprehensive/main.tf | Comprehensive manual test configuration covering many resources. |
| tests/comprehensive/README.md | Documents the comprehensive manual test suite. |
| tests/README.md | Top-level documentation for running the manual test suites. |
| cloudstack/resource_cloudstack_user.go | Implements user Update and Read logic. |
| cloudstack/resource_cloudstack_domain.go | Implements domain Read and Update logic. |
| cloudstack/resource_cloudstack_disk_offering.go | Implements disk offering Read/Update/Delete and adds needed imports. |
| cloudstack/resource_cloudstack_account.go | Adds ForceNew to immutable fields and implements account Read/Update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@kristofer-atlas We already have some tests here: https://github.com/apache/cloudstack-terraform-provider/blob/main/cloudstack/resource_cloudstack_disk_test.go How are the new tests helping us here? |
bcc0576 to
17fcf46
Compare
- Domain: add Computed+ForceNew to domain_id and parent_domain_id to fix perpetual diffs causing 19 acceptance test failures - Account: remove unnecessary ForceNew from email/first_name/last_name/ password; implement user-level updates via updateUser API; add Sensitive to password field - Disk offering: mark disk_size as ForceNew since updateDiskOffering does not support size changes - User: mark account and username as ForceNew to prevent silent drift; add Sensitive to password; wrap Update error with resource context - Remove tests/ directory (manual TF configs, not Go acceptance tests) which also fixes Apache RAT check failures from missing headers
|
@vishesh92, the intent of the tests was to be a sort of end-to-end test. Removed for now. Hadn't noticed the tests you point to. Wondering, though, whether we should have some sort of end-to-end tests similar tor what I had... In some cases I have had issues with create -> destroy -> recreate, where the destroy was incomplete, or some assumptions about state and lingering resources. |
Implements missing Read, Update, and Delete funcitons for stub resources in the provider.
Affected resources:
Previously these resources had empty stub implementations that would cause state drift and prevent proper resource management.
Added test configurations under tests/ covering comprehensive provider features and networking with egress rules.
All changes pass make build and make test.
Partially addresses #133 (account domain association now supported through Read function).