Skip to content

Refactor code to use backend config state's SetConfig method, protect against nil receiver #37254

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

Merged
merged 2 commits into from
Jun 27, 2025

Conversation

SarahFrench
Copy link
Member

@SarahFrench SarahFrench commented Jun 19, 2025

For context, the SetConfig method on BackendConfigState is only used in the context of tests currently. Whenever a BackendConfigState is assembled the config is set directly by assigning a value to ConfigRaw:

// Store the metadata in our saved state location
s := sMgr.State()
if s == nil {
s = workdir.NewBackendStateFile()
}
s.Backend = &workdir.BackendConfigState{
Type: c.Type,
ConfigRaw: json.RawMessage(configJSON),
Hash: uint64(cHash),
}

That configJSON value is constructed in the code immediately preceding the above snippet:

configJSON, err := ctyjson.Marshal(configVal, b.ConfigSchema().ImpliedType())
if err != nil {
diags = diags.Append(fmt.Errorf("Can't serialize backend configuration as JSON: %s", err))
return nil, diags
}

This PR refactors that code to instead use the SetConfig method. This method includes the logic for marshalling cty data to JSON, so replaces the equivalent code above that gets the implied type of a schema and marshals the cty value to json.

Also, this PR adds a nil receiver check to the method.

Target Release

1.13.x

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@SarahFrench SarahFrench added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Jun 19, 2025
@SarahFrench SarahFrench marked this pull request as ready for review June 23, 2025 16:35
@SarahFrench SarahFrench requested a review from a team as a code owner June 23, 2025 16:35
@SarahFrench SarahFrench merged commit d3b02a8 into main Jun 27, 2025
18 of 20 checks passed
@SarahFrench SarahFrench deleted the sarah/setconfig-protect-nil-receiver branch June 27, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog-needed Add this to your PR if the change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants