Skip to content

Commit 6b73f71

Browse files
authored
PSS: Implement initialisation of new working directory (or use of -reconfigure flag) while using state_store (#37732)
* Minor fixes in diagnostics This can only be done once modules have been parsed and the required providers data is available. There are multiple places where config is parsed, into either Config or Module structs, so this needs to be implemented in multiple places. * Rename test to make it specific to use of backend block in config * Update initBackend to accept whole initArgs collection * Only process --backend-config data, when setting up a `backend`, if that data isn't empty * Simplify how mock provider factories are made in tests * Update mock provider's default logic to track and manage existing workspaces * Add `ProviderSchema` method to `Pluggable` structs. This allows calling code to access the provider schema when using provider configuration data. * Add function for converting a providerreqs.Version to a hashicorp/go-version Version. This is needed for using locks when creating the backend state file. * Implement initial version of init new working directories using `stateStore_C_s`. Default to creating the default workspace if no workspaces exist. * Update test fixtures to match the hashicorp/test mock provider used in PSS tests * Allow tests to obtain locks that include `testingOverrides` providers. The `testingOverrides` field will only be set in tests, so this should not impact end users. * Add tests showing TF can initialize a working directory for the first time (and do the same when forced by -reconfigure flag). Remove replaced tests. * Add -create-default-workspace flag, to be used to disable creating the default workspace by default when -input=false (i.e for use in CI). Refactor creation of default workspace logic. Add tests. * Allow reattached providers to be used during init for PSS * Rename variable to `backendHash` so relation to `backend` is clearer * Allow `(m *Meta) Backend` to return warning diagnostics * Protect against nil testingOverrides in providerFactoriesFromLocks * Add test case seeing what happens if default workspace selected, doesn't exist, but other workspaces do exist. The consequences here are due to using `selectWorkspace` in `stateStore_C_s`, matching what's done in `backend_C_r_s`. * Address code consistency check failure on PR * Refactor use of mock in test that's experiencing EOF error... * Remove test that requires test to supply input for user prompt This test passes when run in isolation but fails when run alongside other tests, even when skipping all other tests using `testStdinPipe`. I don't think the value of this test is great enough to start changing how we test stdin input. * Allow -create-default-workspace to be used regardless of whether input is enabled or disabled * Add TF_SKIP_CREATE_DEFAULT_WORKSPACE environment variable * Responses to feedback, including making testStdinPipe helper log details of errors copying data to stdin. Note: We cannot call t.Fatal from a non-test goroutine. * Use Errorf instead * Allow backend state files to not include version data when a builtin or reattached provider is in use. * Add clarifying comment about re-attached providers when finding the matching entry in required_providers * Report that the default workspace was created to the view * Refactor: use error comparison via `errors.Is` to identify when no workspaces exist. * Move handling of TF_ENABLE_PLUGGABLE_STATE_STORAGE into init's ParseInit func. * Validate that PSS-related flags can only be used when experiments are enabled, enforce coupling of PSS-related flags when in use. * Slight rewording of output message about default workspace * Update test to assert new output about default workspace
1 parent 1047b53 commit 6b73f71

File tree

29 files changed

+1284
-210
lines changed

29 files changed

+1284
-210
lines changed

internal/backend/pluggable/pluggable.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,15 @@ func (p *Pluggable) ConfigSchema() *configschema.Block {
6868
return val.Body
6969
}
7070

71+
// ProviderSchema returns the schema for the provider implementing the state store.
72+
//
73+
// This isn't part of the backend.Backend interface but is needed in calling code.
74+
// When it's used the backend.Backend will need to be cast to a Pluggable.
75+
func (p *Pluggable) ProviderSchema() *configschema.Block {
76+
schemaResp := p.provider.GetProviderSchema()
77+
return schemaResp.Provider.Body
78+
}
79+
7180
// PrepareConfig validates configuration for the state store in
7281
// the state storage provider. The configuration sent from Terraform core
7382
// will not include any values from environment variables; it is the

internal/backend/pluggable/pluggable_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,3 +398,70 @@ func TestPluggable_DeleteWorkspace(t *testing.T) {
398398
t.Fatalf("expected error %q but got: %q", wantError, err)
399399
}
400400
}
401+
402+
func TestPluggable_ProviderSchema(t *testing.T) {
403+
t.Run("Returns the expected provider schema", func(t *testing.T) {
404+
mock := &testing_provider.MockProvider{
405+
GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
406+
Provider: providers.Schema{
407+
Body: &configschema.Block{
408+
Attributes: map[string]*configschema.Attribute{
409+
"custom_attr": {Type: cty.String, Optional: true},
410+
},
411+
},
412+
},
413+
},
414+
}
415+
p, err := NewPluggable(mock, "foobar")
416+
if err != nil {
417+
t.Fatalf("unexpected error: %s", err)
418+
}
419+
420+
// Calling code will need to case to Pluggable after using NewPluggable,
421+
// so we do something similar in this test
422+
var providerSchema *configschema.Block
423+
if pluggable, ok := p.(*Pluggable); ok {
424+
providerSchema = pluggable.ProviderSchema()
425+
}
426+
427+
if !mock.GetProviderSchemaCalled {
428+
t.Fatal("expected ProviderSchema to call the GetProviderSchema RPC")
429+
}
430+
if providerSchema == nil {
431+
t.Fatal("ProviderSchema returned an unexpected nil schema")
432+
}
433+
if val := providerSchema.Attributes["custom_attr"]; val == nil {
434+
t.Fatalf("expected the returned schema to include an attr called %q, but it was missing. Schema contains attrs: %v",
435+
"custom_attr",
436+
slices.Sorted(maps.Keys(providerSchema.Attributes)))
437+
}
438+
})
439+
440+
t.Run("Returns a nil schema when the provider has an empty schema", func(t *testing.T) {
441+
mock := &testing_provider.MockProvider{
442+
GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
443+
Provider: providers.Schema{
444+
// empty schema
445+
},
446+
},
447+
}
448+
p, err := NewPluggable(mock, "foobar")
449+
if err != nil {
450+
t.Fatalf("unexpected error: %s", err)
451+
}
452+
453+
// Calling code will need to case to Pluggable after using NewPluggable,
454+
// so we do something similar in this test
455+
var providerSchema *configschema.Block
456+
if pluggable, ok := p.(*Pluggable); ok {
457+
providerSchema = pluggable.ProviderSchema()
458+
}
459+
460+
if !mock.GetProviderSchemaCalled {
461+
t.Fatal("expected ProviderSchema to call the GetProviderSchema RPC")
462+
}
463+
if providerSchema != nil {
464+
t.Fatalf("expected ProviderSchema to return a nil schema but got: %#v", providerSchema)
465+
}
466+
})
467+
}

internal/command/arguments/init.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package arguments
55

66
import (
7+
"os"
78
"time"
89

910
"github.com/hashicorp/terraform/internal/tfdiags"
@@ -78,12 +79,16 @@ type Init struct {
7879
// TODO(SarahFrench/radeksimko): Remove this once the feature is no longer
7980
// experimental
8081
EnablePssExperiment bool
82+
83+
// CreateDefaultWorkspace indicates whether the default workspace should be created by
84+
// Terraform when initializing a state store for the first time.
85+
CreateDefaultWorkspace bool
8186
}
8287

8388
// ParseInit processes CLI arguments, returning an Init value and errors.
8489
// If errors are encountered, an Init value is still returned representing
8590
// the best effort interpretation of the arguments.
86-
func ParseInit(args []string) (*Init, tfdiags.Diagnostics) {
91+
func ParseInit(args []string, experimentsEnabled bool) (*Init, tfdiags.Diagnostics) {
8792
var diags tfdiags.Diagnostics
8893
init := &Init{
8994
Vars: &Vars{},
@@ -111,6 +116,7 @@ func ParseInit(args []string) (*Init, tfdiags.Diagnostics) {
111116
cmdFlags.BoolVar(&init.Json, "json", false, "json")
112117
cmdFlags.Var(&init.BackendConfig, "backend-config", "")
113118
cmdFlags.Var(&init.PluginPath, "plugin-dir", "plugin directory")
119+
cmdFlags.BoolVar(&init.CreateDefaultWorkspace, "create-default-workspace", true, "when -input=false, use this flag to block creation of the default workspace")
114120

115121
// Used for enabling experimental code that's invoked before configuration is parsed.
116122
cmdFlags.BoolVar(&init.EnablePssExperiment, "enable-pluggable-state-storage-experiment", false, "Enable the pluggable state storage experiment")
@@ -123,6 +129,46 @@ func ParseInit(args []string) (*Init, tfdiags.Diagnostics) {
123129
))
124130
}
125131

132+
if v := os.Getenv("TF_ENABLE_PLUGGABLE_STATE_STORAGE"); v != "" {
133+
init.EnablePssExperiment = true
134+
}
135+
136+
if v := os.Getenv("TF_SKIP_CREATE_DEFAULT_WORKSPACE"); v != "" {
137+
// If TF_SKIP_CREATE_DEFAULT_WORKSPACE is set it will override
138+
// a -create-default-workspace=true flag that's set explicitly,
139+
// as that's indistinguishable from the default value being used.
140+
init.CreateDefaultWorkspace = false
141+
}
142+
143+
if !experimentsEnabled {
144+
// If experiments aren't enabled then these flags should not be used.
145+
if init.EnablePssExperiment {
146+
diags = diags.Append(tfdiags.Sourceless(
147+
tfdiags.Error,
148+
"Cannot use -enable-pluggable-state-storage-experiment flag without experiments enabled",
149+
"Terraform cannot use the-enable-pluggable-state-storage-experiment flag (or TF_ENABLE_PLUGGABLE_STATE_STORAGE environment variable) unless experiments are enabled.",
150+
))
151+
}
152+
if !init.CreateDefaultWorkspace {
153+
// Can only be set to false by using the flag
154+
// and we cannot identify if -create-default-workspace=true is set explicitly.
155+
diags = diags.Append(tfdiags.Sourceless(
156+
tfdiags.Error,
157+
"Cannot use -create-default-workspace flag without experiments enabled",
158+
"Terraform cannot use the -create-default-workspace flag (or TF_SKIP_CREATE_DEFAULT_WORKSPACE environment variable) unless experiments are enabled.",
159+
))
160+
}
161+
} else {
162+
// Errors using flags despite experiments being enabled.
163+
if !init.CreateDefaultWorkspace && !init.EnablePssExperiment {
164+
diags = diags.Append(tfdiags.Sourceless(
165+
tfdiags.Error,
166+
"Cannot use -create-default-workspace=false flag unless the pluggable state storage experiment is enabled",
167+
"Terraform cannot use the -create-default-workspace=false flag (or TF_SKIP_CREATE_DEFAULT_WORKSPACE environment variable) unless you also supply the -enable-pluggable-state-storage-experiment flag (or set the TF_ENABLE_PLUGGABLE_STATE_STORAGE environment variable).",
168+
))
169+
}
170+
}
171+
126172
if init.MigrateState && init.Json {
127173
diags = diags.Append(tfdiags.Sourceless(
128174
tfdiags.Error,

internal/command/arguments/init_test.go

Lines changed: 85 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,11 @@ func TestParseInit_basicValid(t *testing.T) {
4040
FlagName: "-backend-config",
4141
Items: &flagNameValue,
4242
},
43-
Vars: &Vars{},
44-
InputEnabled: true,
45-
CompactWarnings: false,
46-
TargetFlags: nil,
43+
Vars: &Vars{},
44+
InputEnabled: true,
45+
CompactWarnings: false,
46+
TargetFlags: nil,
47+
CreateDefaultWorkspace: true,
4748
},
4849
},
4950
"setting multiple options": {
@@ -72,11 +73,12 @@ func TestParseInit_basicValid(t *testing.T) {
7273
FlagName: "-backend-config",
7374
Items: &flagNameValue,
7475
},
75-
Vars: &Vars{},
76-
InputEnabled: true,
77-
Args: []string{},
78-
CompactWarnings: true,
79-
TargetFlags: nil,
76+
Vars: &Vars{},
77+
InputEnabled: true,
78+
Args: []string{},
79+
CompactWarnings: true,
80+
TargetFlags: nil,
81+
CreateDefaultWorkspace: true,
8082
},
8183
},
8284
"with cloud option": {
@@ -101,11 +103,12 @@ func TestParseInit_basicValid(t *testing.T) {
101103
FlagName: "-backend-config",
102104
Items: &[]FlagNameValue{{Name: "-backend-config", Value: "backend.config"}},
103105
},
104-
Vars: &Vars{},
105-
InputEnabled: false,
106-
Args: []string{},
107-
CompactWarnings: false,
108-
TargetFlags: []string{"foo_bar.baz"},
106+
Vars: &Vars{},
107+
InputEnabled: false,
108+
Args: []string{},
109+
CompactWarnings: false,
110+
TargetFlags: []string{"foo_bar.baz"},
111+
CreateDefaultWorkspace: true,
109112
},
110113
},
111114
}
@@ -114,7 +117,8 @@ func TestParseInit_basicValid(t *testing.T) {
114117

115118
for name, tc := range testCases {
116119
t.Run(name, func(t *testing.T) {
117-
got, diags := ParseInit(tc.args)
120+
experimentsEnabled := false
121+
got, diags := ParseInit(tc.args, experimentsEnabled)
118122
if len(diags) > 0 {
119123
t.Fatalf("unexpected diags: %v", diags)
120124
}
@@ -156,7 +160,8 @@ func TestParseInit_invalid(t *testing.T) {
156160

157161
for name, tc := range testCases {
158162
t.Run(name, func(t *testing.T) {
159-
got, diags := ParseInit(tc.args)
163+
experimentsEnabled := false
164+
got, diags := ParseInit(tc.args, experimentsEnabled)
160165
if len(diags) == 0 {
161166
t.Fatal("expected diags but got none")
162167
}
@@ -170,6 +175,68 @@ func TestParseInit_invalid(t *testing.T) {
170175
}
171176
}
172177

178+
func TestParseInit_experimentalFlags(t *testing.T) {
179+
testCases := map[string]struct {
180+
args []string
181+
envs map[string]string
182+
wantErr string
183+
experimentsEnabled bool
184+
}{
185+
"error: -enable-pluggable-state-storage-experiment and experiments are disabled": {
186+
args: []string{"-enable-pluggable-state-storage-experiment"},
187+
experimentsEnabled: false,
188+
wantErr: "Cannot use -enable-pluggable-state-storage-experiment flag without experiments enabled",
189+
},
190+
"error: TF_ENABLE_PLUGGABLE_STATE_STORAGE is set and experiments are disabled": {
191+
envs: map[string]string{
192+
"TF_ENABLE_PLUGGABLE_STATE_STORAGE": "1",
193+
},
194+
experimentsEnabled: false,
195+
wantErr: "Cannot use -enable-pluggable-state-storage-experiment flag without experiments enabled",
196+
},
197+
"error: -create-default-workspace=false and experiments are disabled": {
198+
args: []string{"-create-default-workspace=false"},
199+
experimentsEnabled: false,
200+
wantErr: "Cannot use -create-default-workspace flag without experiments enabled",
201+
},
202+
"error: TF_SKIP_CREATE_DEFAULT_WORKSPACE is set and experiments are disabled": {
203+
envs: map[string]string{
204+
"TF_SKIP_CREATE_DEFAULT_WORKSPACE": "1",
205+
},
206+
experimentsEnabled: false,
207+
wantErr: "Cannot use -create-default-workspace flag without experiments enabled",
208+
},
209+
"error: -create-default-workspace=false used without -enable-pluggable-state-storage-experiment, while experiments are enabled": {
210+
args: []string{"-create-default-workspace=false"},
211+
experimentsEnabled: true,
212+
wantErr: "Cannot use -create-default-workspace=false flag unless the pluggable state storage experiment is enabled",
213+
},
214+
"error: TF_SKIP_CREATE_DEFAULT_WORKSPACE used without -enable-pluggable-state-storage-experiment, while experiments are enabled": {
215+
envs: map[string]string{
216+
"TF_SKIP_CREATE_DEFAULT_WORKSPACE": "1",
217+
},
218+
experimentsEnabled: true,
219+
wantErr: "Cannot use -create-default-workspace=false flag unless the pluggable state storage experiment is enabled",
220+
},
221+
}
222+
223+
for name, tc := range testCases {
224+
t.Run(name, func(t *testing.T) {
225+
for k, v := range tc.envs {
226+
t.Setenv(k, v)
227+
}
228+
229+
_, diags := ParseInit(tc.args, tc.experimentsEnabled)
230+
if len(diags) == 0 {
231+
t.Fatal("expected diags but got none")
232+
}
233+
if got, want := diags.Err().Error(), tc.wantErr; !strings.Contains(got, want) {
234+
t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want)
235+
}
236+
})
237+
}
238+
}
239+
173240
func TestParseInit_vars(t *testing.T) {
174241
testCases := map[string]struct {
175242
args []string
@@ -207,7 +274,8 @@ func TestParseInit_vars(t *testing.T) {
207274

208275
for name, tc := range testCases {
209276
t.Run(name, func(t *testing.T) {
210-
got, diags := ParseInit(tc.args)
277+
experimentsEnabled := false
278+
got, diags := ParseInit(tc.args, experimentsEnabled)
211279
if len(diags) > 0 {
212280
t.Fatalf("unexpected diags: %v", diags)
213281
}

internal/command/command_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,10 @@ func testStdinPipe(t *testing.T, src io.Reader) func() {
657657
// Copy the data from the reader to the pipe
658658
go func() {
659659
defer w.Close()
660-
io.Copy(w, src)
660+
_, err := io.Copy(w, src)
661+
if err != nil {
662+
t.Errorf("error when copying data from testStdinPipe reader argument to stdin: %s", err)
663+
}
661664
}()
662665

663666
return func() {

0 commit comments

Comments
 (0)