Skip to content

fix: update PlanModifyList#347

Open
jatcod3r wants to merge 4 commits into
mainfrom
fix/template-unknown-id
Open

fix: update PlanModifyList#347
jatcod3r wants to merge 4 commits into
mainfrom
fix/template-unknown-id

Conversation

@jatcod3r
Copy link
Copy Markdown

tf_vars breaks due to sensitive attribute being stripped on plan/update. Also breaks when the name of a version is unknown, breaking cases where the name of a version is inherited by another resource, like "random_uuid".

@jatcod3r jatcod3r self-assigned this May 12, 2026
@jatcod3r jatcod3r changed the title fix: update PlanModifyList to only touch id, name, and directory_hash fix: update PlanModifyList May 12, 2026
@jatcod3r
Copy link
Copy Markdown
Author

@codex review

@jatcod3r jatcod3r linked an issue May 13, 2026 that may be closed by this pull request
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7045b299a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1549 to 1550
if configVersions[i].Name.IsNull() {
planVersions[i].Name = types.StringValue(prevList[0].Name)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid reusing prior ID while leaving name unknown

When configVersions[i].Name is non-null but unknown (for example, a recreated upstream random_uuid), this branch reuses a prior version ID but intentionally leaves planVersions[i].Name unknown. In Update(), known IDs go through the metadata-update path, where !curVersion.Name.Equal(newState.Versions[idx].Name) is true and ValueString() on the unknown name becomes an empty string, causing an invalid rename request (or a wrong blank name update). This regression is introduced by changing the backfill condition here and can break applies for unknown-but-configured names.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

@jatcod3r jatcod3r May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When tf_vars change, the third loop resets the ID to unknown, so during Apply it goes through the "create new version" path where the name gets populated from the server response, so an empty name is expected. The unknown name never hits ValueString() in a rename request, or at least it shouldn't.

@jatcod3r jatcod3r marked this pull request as ready for review May 13, 2026 17:29
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.

coderd_template "Attributes Set" input types issue

2 participants