fix(circleci_project): in-place updates no longer cascade replacement; fix pr_only_branch_overrides quoting#106
Open
james-crowley wants to merge 2 commits into
Conversation
…dates
The circleci_project resource declares id, slug, organization_name,
organization_slug, vcs_info_url, vcs_info_provider, vcs_info_default_branch
and the seven Optional+Computed boolean settings as Computed without any
plan modifiers. By terraform-plugin-framework default, every Computed
attribute is marked Unknown during plan when any other field on the
resource changes, so the provider can recompute it after apply.
In practice id and slug are stable across in-place updates -- only a
RequiresReplace forces a new value. With them marked Unknown in plan,
dependent resources whose RequiresReplace fields reference the project
get cascaded into replacement:
- circleci_context_restriction.value = circleci_project.X.id
- circleci_project_environment_variable.project_slug
= circleci_project.X.slug
- circleci_webhook.scope_id = circleci_project.X.id
A single auto_cancel_builds=false flip on the project plans as
"4 to add, 6 to change, 4 to destroy" and destroys all project-scoped
secrets and the project webhook on apply. Customers running terraform
apply -auto-approve in CI lose secrets without warning.
Add UseStateForUnknown plan modifiers so the framework keeps the prior
state values for these Computed attributes during in-place updates. The
same pattern is used elsewhere in this provider (e.g.
runner_resource_class_resource.go), so this aligns the project resource
with the existing convention.
After this change the same auto_cancel_builds flip plans as
"0 to add, 1 to change, 0 to destroy".
The Create and Update paths for circleci_project iterate
plan.PROnlyBranchOverrides.Elements() and call .String() on each
element. attr.Value.String() returns the Terraform-syntax
representation, which for a string includes the surrounding double
quotes -- "main" becomes the literal seven-character string `"main"`.
The quoted strings are then sent to the CircleCI API and stored
verbatim. On the next refresh the API returns the quoted form, the
framework compares it to the planned value, and apply fails with:
Error: Provider produced inconsistent result after apply
.pr_only_branch_overrides[0]: was cty.StringVal("main"),
but now cty.StringVal("\"main\"").
Replace .String() with the standard
elem.(types.String).ValueString() pattern (with a defensive type
assertion to surface unexpected element types via diagnostics rather
than a panic). This is the same idiom used for individual string
attributes elsewhere in the provider (plan.X.ValueString()), now
applied to list-element extraction.
Fixes CircleCI-Public#59.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Two bugs in
circleci_projectthat combine to make in-place updates unsafe.1. Cascading replacement on benign edits
Changing any field on a
circleci_project(e.g.auto_cancel_builds = false) plans as a destructive cascade:Any resource referencing
circleci_project.X.idor.slug(context restrictions, project env vars, webhooks scoped to the project) gets destroyed and recreated. Project env vars carry secrets, so this silently drops customer secrets on apply.Cause:
Computedattributes includingidandslughad noUseStateForUnknownplan modifier, so the framework marked themUnknownduring plan whenever any other field changed. Dependents whoseRequiresReplacefields reference the project then had to replace.Fix: add
UseStateForUnknownto allComputedattributes. Same pattern is already used inrunner_resource_class_resource.goin this repo.2.
pr_only_branch_overrideselements get double-quoted (Fixes #59)…fails apply with:
Cause:
CreateandUpdateextract list elements viabranch.String(), which returns the Terraform-syntax representation (literally"main"including the quotes). The quoted strings were sent to the API and stored.Fix: use
elem.(types.String).ValueString()instead — the same pattern used for individual string attributes in this provider.#61 proposed the same extraction fix bundled with a
ListAttribute→SetAttributeschema change. This PR keeps the existing list type to avoid breaking existing HCL; the type-shape decision can be a separate follow-up.Why both in one PR
Fix 1 alone exposes Fix 2: removing the cascade means
Updateactually runs (it didn't before, because every change destroyed and recreated), which immediately trips the quoting bug. Shipping Fix 1 without Fix 2 just moves the failure from "destructive plan" to "apply error".Testing
Tested locally against a CircleCI standalone organization with a project, two project env vars, a context restriction, and a webhook.
Before: toggling
auto_cancel_buildsplans as4 add / 6 change / 4 destroy. Forcing past that hits thepr_only_branch_overrides400.After: same toggle plans as
0 add / 1 change / 0 destroy, applies clean, re-plan reportsNo changes. Settingpr_only_branch_overrides = ["main", "develop"]and applying stores the values verbatim with no embedded quotes.Migration note
Existing projects whose API state has the quoted form stored (
["\"main\""]) will continue to read that way until cleaned up. Settingpr_only_branch_overridesexplicitly in HCL and applying once overwrites the API state via the correctedUpdatepath.Closes #59.