feat(ruleset): add dry-run validation to plan - #7291
Conversation
There was a problem hiding this comment.
Let's add two more cases:
- Dependency during creation, and then update with validation error.
- No dependency during creation, and then update with dependency and validation error.
There was a problem hiding this comment.
Let's run each test case for an account-level and zone-level ruleset
|
|
||
| // planIsKnownForDryRun reports whether enough of the plan is known to build a | ||
| // request that the API can validate | ||
| func planIsKnownForDryRun(plan *RulesetModel) bool { |
There was a problem hiding this comment.
I think we want to check if anything in the plan (recursively) is unknown.
There was a problem hiding this comment.
I think it might be simpler to test account and zone rulesets in the same test (I believe you can have multiple resources per test). Will also make it easier to ensure that the account and zone tests stay in sync
There was a problem hiding this comment.
You're removing the description in this step, is that intended?
| } | ||
|
|
||
| // Check if enough of the plan is known to build a request for the dry-run | ||
| if !req.Config.Raw.IsFullyKnown() { |
There was a problem hiding this comment.
You're checking req.Config here, but you later use plan (derived from req.Plan) instead
|
|
||
| var err error | ||
| switch { | ||
| // The plan is absent only when the resource is being deleted |
There was a problem hiding this comment.
I think it would be nicer to check explicitly that it's a delete event, instead of that the plan is empty. Terraform actually has a lot of different events beyond simple CRUD (e.g., imports), so I think there could be another event (now or in the future) where the plan is also empty.
| return | ||
| } | ||
|
|
||
| // A plan that leaves the computed ID unknown is creating the ruleset |
There was a problem hiding this comment.
Same here, would be nice to check for create events explicitly, rather than inferring from the ID being unknown
2 participants
Changes being requested
Configuration errors in Cloudflare Rulesets are currently reported during
terraform apply, because the provider does not call the API until it writes. This means a plan can look completely healthy and then fail partway through an apply. This change adds dry-run validation forterraform plan, allowing customers to validate their setups beforehand and catch errors early.We validate the planned configuration against the API during terraform plan by issuing the same request the apply would issue, with
?dry_run=true. The Rulesets API performs full validation and returns the same errors it would for a real write, but does not persist anything.Acceptance test run results
Steps to run acceptance tests
Test output
Additional context & links