Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rulesets): improve handling of id and ref unknown values #4748

Closed

Conversation

jacobbednarz
Copy link
Member

@jacobbednarz jacobbednarz commented Dec 10, 2024

Updates the schema definition to apply
stringplanmodifier.UseStateForUnknown on the values as once they are set, they should not be changing and we can safely rely on the values.

This is the last piece of the rulesets drift issue.

Copy link
Contributor

Oops! It looks like no changelog entry is attached to this PR. Please include a release note as described in https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/contributing/changelog-process.md.

Example:

```release-note:TYPE
Release note
```

If you do not require a release note to be included and you have permission, please add the workflow/skip-changelog-entry label. Otherwise, a maintainer will add the label or ask you for one when they review the PR.

Updates the schema definition to apply
`stringplanmodifier.UseStateForUnknown` on the values as once they are
set, they should not be changing and we can safely rely on the values.
@jacobbednarz jacobbednarz force-pushed the mark-id-and-ref-as-use-state-for-unknown branch from a3ebd35 to 1eaa51b Compare December 11, 2024 01:12
@zakcutner
Copy link
Contributor

We actually shouldn't mark either of the id or ref fields as UseStateForUnknown. This is because they will change across updates to the ruleset, so adding this will cause the provider to emit an incorrect plan. This is because the API cannot know which rules in the new version of the ruleset map to which rules in the previous version (i.e., rules might have been reordered, rules might have been added or deleted, etc.).

The way around this is to give each rule an explicit ref (by default, the ref is set to the same as the id). This way, the API knows which rule ids to keep the same in the new version, by looking at which refs have remained the same. The custom plan modifier that was added in #4697 will show the correct diff in this case, by matching rule refs in the state and the plan against each other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants