PSS: Let the `init` command recognise when there are no changes in configuration. #37777

SarahFrench · 2025-10-15T13:53:34Z

This PR allows the experimental PSS version of init logic recognise when there have been no changes since the last init, so init will perform a no-op (in regards to state storage, at least).

I've added a test showing that init is a no-op in those circumstances : TestInit_stateStore_configUnchanged

I've also added tests showing that when backend state and config don't match we hit a not configured yet error diagnostic - see TestInit_stateStore_configChanges. To do this I've created sets of test fixtures where the backend state file and config are not in agreement. A special case of this, upgraded provider versions, is in TestInit_stateStore_providerUpgrade.

Changes covered:

state_store config changed - internal/command/testdata/state-store-changed/store-config
provider config changed - internal/command/testdata/state-store-changed/provider-config
state_store type changed - internal/command/testdata/state-store-changed/state-store-type
provider used for PSS changed - internal/command/testdata/state-store-changed/provider-used
provider upgraded - internal/command/testdata/state-store-changed/provider-upgraded

Target Release

N/A

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.

…separate method

…ing backend state (i.e. no changes)

… return

…iltin and re-attached providers. This makes comparison easier when determining if config has changed since last init.

…d state_store config changes being tested

…a change, but handling this scenario isn't implemented yet

Previously dummy values were fine, but as tests using hashes to identify changes these values need to be accurate!

…der version as described in the backend state file fixture for the test.

…as a change, but handling this scenario isn't implemented yet

…ation in the same provider is detected as a change, but handling this scenario isn't implemented yet

…as a change, but handling this scenario isn't implemented yet

…ut handling this scenario isn't implemented yet

Just to avoid any confusion if copy-pasting happens in future.

…ull, and replace dummy hash values with correct values.

…ce would already exist in this scenario

…no-op for backend state

…& update test to not expect a value for region attr in provider config

internal/command/meta_backend.go

We will allow downgrades to succeed as long as the schema version number is unchanged

radeksimko · 2025-10-17T09:33:36Z

internal/command/testdata/state-store-changed/provider-used/.terraform/terraform.tfstate

+            "hash": 2116468040
+        },
+        "hash": 2116468040


Just a random observation - not a blocker - how is it that these two hashes match exactly? I thought that each hash represents a different part of the configuration, so should differ?

Yeah it's odd, but I double checked and the values are correct.

Using a debugger I found an explanation for why they match. To get hashes of the state_store and provider configs we use the (cty.Value) Hash method (used here in Core code). The (cty.Value) Hash method iterates through values to create a slice of bytes which are used to create the final hash number. The process of building those bytes for the hashes in this use case hits this code, which ignore the names of keys and only use values.

The end result is that, because both the state store and provider have 1 attr with the value "foobar", the bytes used to make the hash in both cases is:

"<"test_store";<"foobar";>;>"

Ah I'm half correct - the tuple is constructed in our calling code so we can make the hashes be distinct:

terraform/internal/configs/state_store.go

Lines 205 to 208 in cbda324

pToHash := cty.TupleVal([]cty.Value{

cty.StringVal(b.Type),

pVal,

})

~~I think we should address this, but in a separate PR~~

Edit: I've since changed this opinion, after we agreed that we should swap to a single hash for the whole state_store block in this discussion.

All commits following this thread, barring removing unused code, are related to this thread.

internal/command/testdata/state-store-changed/state-store-type/.terraform/terraform.tfstate

… impacted by: state store config, provider config, state store type, provider source

…rovider block no longer has its own hash

…e provider block!

SarahFrench · 2025-10-17T14:48:06Z

Ah there are still issues...go vet failed me

…ted tests and test fixtures

…0 to indicate they're not set accurately.

SarahFrench · 2025-10-17T17:08:26Z

internal/configs/state_store.go

+	toHash := cty.TupleVal([]cty.Value{
+		cty.StringVal(b.Type), // state store type
+		ssVal,                 // state store config
+
+		cty.StringVal(b.ProviderAddr.String()),            // provider source
+		cty.StringVal(stateStoreProviderVersion.String()), // provider version
+		cty.StringVal(b.Provider.Name),                    // provider name - this is directly parsed from the config, whereas provider source is added separately later after config is parsed.
+		pVal,                                              // provider config
+	})


Behold the beautiful new hash contents ✨

That's great in terms of comments and clarity 👍🏻 🌟

I am wondering why this is a tuple and why it contains just the values and not the attribute names but I think that is somewhat irrelevant to this PR. Just thinking out loud as I read through the code. 😅

Yeah this is something that I've copied from the original implementation of hashing for backends and haven't dug into that deeply. I can imagine the reason being that attribute names would be the same each time and not help with producing a unique hash per backend/state_store, and attribute names were less useful for hashing the 2 values from backends, too.

…hat's had a successful prior init using PSS

radeksimko · 2025-10-20T14:08:16Z

internal/command/testdata/state-store-to-backend/.terraform/terraform.tfstate

+            }
        },
-        "hash": 12345
+        "hash": 0


Why do we persist zero hash and other values if the state store is being removed instead of removing the entire state_store object attribute from here?

Is that for some sort of consistency with how backends behave today?

I made the hash here zero just to make it really clear that the test fixture doesn't have a valid value for the hash, in case that affects anything in future.

The internal/command/testdata/state-store-to-backend test fixture is intended to be used testing what happens when a user swaps from using a state_store to using a backend. In that scenario the user would have previously run a successful init command, which produces this backend state file. In the intended test scenario, migrating to using a backend, the user would update the config to describe the new backend and then run an init command. This folder represents that in-between state where the config and the backend state file do not match, as the user hasn't run an init command since editing the config.

Does that make sense?

I made the hash here zero just to make it really clear that the test fixture doesn't have a valid value for the hash, in case that affects anything in future.

This was in response to having a painful experience needing to update test fixtures to have accurate hash values 😅

SarahFrench added 19 commits October 15, 2025 11:19

Pull determining of PSS provider's version from current locks into a …

dff4555

…separate method

Add code for identifying when config and provider version match exist…

bd41b67

…ing backend state (i.e. no changes)

Update test - locks are now needed before it hits expected error diag…

141478a

… return

Add test showing successful init when no config changes are detected.

5c77b8d

Update getStateStorageProviderVersion to return nil versions for bu…

bda07d4

…iltin and re-attached providers. This makes comparison easier when determining if config has changed since last init.

Add test coverage for getStateStorageProviderVersion

c4c5b1b

Move testing fixtures around, preparing for different types of change…

3aeb44f

…d state_store config changes being tested

Add test showing that changing the state_store config is detected as …

5f43a22

…a change, but handling this scenario isn't implemented yet

Update hashes in test fixture backend state file to be accurate

5ede062

Previously dummy values were fine, but as tests using hashes to identify changes these values need to be accurate!

Update existing test cases so that Terraform uses the same test provi…

690ce23

…der version as described in the backend state file fixture for the test.

Add test showing that changing the PSS provider's config is detected …

0d6e33b

…as a change, but handling this scenario isn't implemented yet

Add test showing that swapping to a different state storage implement…

7899855

…ation in the same provider is detected as a change, but handling this scenario isn't implemented yet

Add test showing that changing the provider used for PSS is detected …

e8579b9

…as a change, but handling this scenario isn't implemented yet

Add test showing that upgrading a provider is detected as a change, b…

d1146c8

…ut handling this scenario isn't implemented yet

Update test to use v1.2.3 for consistency with other tests

eb205a7

Just to avoid any confusion if copy-pasting happens in future.

More corrections to existing test fixtures - unset config should be n…

3e5495e

…ull, and replace dummy hash values with correct values.

Fix test for using -reconfigure with state_store; the default workspa…

b485cd8

…ce would already exist in this scenario

Update TestInit_stateStore_configUnchanged to assert that init was a …

0b3e759

…no-op for backend state

Remove unused fixture

62394f8

SarahFrench added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Oct 15, 2025

SarahFrench added 2 commits October 15, 2025 15:40

Remove test that's replaced by new tests in command/init_test.go

3ffe9a7

Replace old references to deleted "state-store-changed" test fixture …

9d6464d

…& update test to not expect a value for region attr in provider config

SarahFrench marked this pull request as ready for review October 15, 2025 17:08

SarahFrench requested a review from a team as a code owner October 15, 2025 17:08

SarahFrench commented Oct 15, 2025

View reviewed changes

internal/command/meta_backend.go Outdated Show resolved Hide resolved

SarahFrench added 3 commits October 16, 2025 11:12

Make test fixture coupling a little more understandable

74fd9b2

Refactor detection of no need to migrate into a function

9db12fd

Add TODO about more involved provider version change tests

2e7c182

We will allow downgrades to succeed as long as the schema version number is unchanged

radeksimko previously approved these changes Oct 17, 2025

View reviewed changes

Update (configs.StateStore)Hash method to return a single hash that's…

a68dd2b

… impacted by: state store config, provider config, state store type, provider source

SarahFrench added 4 commits October 17, 2025 14:29

Update calling code and test helper code to reflect that the nested p…

07c4ce2

…rovider block no longer has its own hash

Remove test; there is now a single hash that SHOULD be affected by th…

4cb4773

…e provider block!

Also use provider name, from config, in hash

25e4dc2

Update tests to reflect changes in how hashes are made

5b11631

SarahFrench dismissed radeksimko’s stale review via 5b11631 October 17, 2025 13:30

SarahFrench added 2 commits October 17, 2025 14:31

Remove unused stateStoreConfigNeedsMigration function

9d1c54a

Remove duplicate isProviderReattached function.

63ce4e7

SarahFrench requested a review from radeksimko October 17, 2025 13:36

SarahFrench added 5 commits October 17, 2025 15:50

Fixes to affected tests

f2cbb3b

Allow provider version to impact the state storage hash, update impac…

647bca7

…ted tests and test fixtures

Update tests that now require locks data to be present in test setup

ce766f5

Update comment for accuracy

e585e61

Fixes to other test fixtures - remove excess hash field, set hash to …

848b369

…0 to indicate they're not set accurately.

SarahFrench commented Oct 17, 2025

View reviewed changes

SarahFrench added 2 commits October 20, 2025 13:18

Make upgrade test actually use upgrade code path

82f939a

Add lock files to test fixture directories that represent a project t…

24be93c

…hat's had a successful prior init using PSS

radeksimko approved these changes Oct 20, 2025

View reviewed changes

SarahFrench merged commit efb2a1c into main Oct 20, 2025
7 checks passed

SarahFrench deleted the pss/init-base-case-no-config-changes branch October 20, 2025 15:51

SarahFrench mentioned this pull request Oct 20, 2025

PSS: Update inaccurate comment about hashing state_store data #37789

Merged

3 tasks

	pToHash := cty.TupleVal([]cty.Value{
	cty.StringVal(b.Type),
	pVal,
	})

Uh oh!

PSS: Let the init command recognise when there are no changes in configuration. #37777

PSS: Let the init command recognise when there are no changes in configuration. #37777

Uh oh!

Conversation

SarahFrench commented Oct 15, 2025

Target Release

Rollback Plan

Changes to Security Controls

CHANGELOG entry

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SarahFrench Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SarahFrench Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SarahFrench Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SarahFrench commented Oct 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PSS: Let the `init` command recognise when there are no changes in configuration. #37777

PSS: Let the `init` command recognise when there are no changes in configuration. #37777

SarahFrench Oct 17, 2025 •

edited

Loading

SarahFrench Oct 17, 2025 •

edited

Loading

SarahFrench Oct 17, 2025 •

edited

Loading