From cefc927e486efdb57d6ca967dd1f1f72fca399fc Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 23 May 2019 18:21:52 -0400 Subject: [PATCH 1/4] failing test for backend re-init --- command/init_test.go | 46 +++++++++++++++++++++++++++++++++ command/meta_backend.go | 2 ++ command/meta_backend_migrate.go | 1 + 3 files changed, 49 insertions(+) diff --git a/command/init_test.go b/command/init_test.go index 7aadb0f0f..62bd484fb 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -1,6 +1,7 @@ package command import ( + "encoding/json" "fmt" "io/ioutil" "log" @@ -410,6 +411,51 @@ func TestInit_backendConfigKV(t *testing.T) { } } +func TestInit_backendConfigKVReInit(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + copy.CopyDir(testFixturePath("init-backend-config-kv"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + + args := []string{"-backend-config", "path=test"} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + ui = new(cli.MockUi) + c = &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + + // a second init should require no changes, nor should it change the backend. + args = []string{"-input=false"} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + // make sure the backend is configured how we expect + configState := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + cfg := map[string]interface{}{} + if err := json.Unmarshal(configState.Backend.ConfigRaw, &cfg); err != nil { + t.Fatal(err) + } + if cfg["path"] != "test" { + t.Fatalf(`expected backend path="test", got path="%v"`, cfg["path"]) + } +} + func TestInit_targetSubdir(t *testing.T) { // Create a temporary working directory that is empty td := tempDir(t) diff --git a/command/meta_backend.go b/command/meta_backend.go index bfaa9b44e..c649c3336 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -823,6 +823,8 @@ func (m *Meta) backend_C_r_S_changed(c *configs.Backend, cHash int, sMgr *state. return nil, diags } + fmt.Println("HERE") + // Perform the migration err := m.backendMigrateState(&backendMigrateOpts{ OneType: s.Backend.Type, diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index 9227844a1..8d020f90a 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -98,6 +98,7 @@ func (m *Meta) backendMigrateState(opts *backendMigrateOpts) error { // Multi-state to multi-state. We merge the states together (migrating // each from the source to the destination one by one). case !oneSingle && !twoSingle: + fmt.Printf("STATES: %q\n", oneStates) // If the source only has one state and it is the default, // treat it as if it doesn't support multi-state. if len(oneStates) == 1 && oneStates[0] == backend.DefaultStateName { From ee9a61836920c53f031b03762ed59cbeb2d34ce4 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 24 May 2019 11:31:04 -0400 Subject: [PATCH 2/4] don't migrate backend during init without override If the backend config hashes match during init, and there are no new backend override options, then we assume the existing config is OK. Since init should be idempotent, we should be able to run init with no options or config changes, and not effect the backends at all. --- command/init_test.go | 2 +- command/meta_backend.go | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 62bd484fb..6e5985944 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -671,7 +671,7 @@ func TestInit_inputFalse(t *testing.T) { } // A missing input=false should abort rather than loop infinitely - args = []string{"-backend-config=path=bar"} + args = []string{"-backend-config=path=baz"} if code := c.Run(args); code == 0 { t.Fatal("init should have failed", ui.OutputWriter) } diff --git a/command/meta_backend.go b/command/meta_backend.go index c649c3336..738a0b94f 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -463,12 +463,11 @@ func (m *Meta) backendFromConfig(opts *BackendOpts) (backend.Backend, tfdiags.Di // Potentially changing a backend configuration case c != nil && !s.Backend.Empty(): - // If we're not initializing, then it's sufficient for the configuration - // hashes to match, since that suggests that the static backend - // settings in the configuration files are unchanged. (The only - // record we have of CLI overrides is in the settings cache in this - // case, so we have no other source to compare with. - if !opts.Init && uint64(cHash) == s.Backend.Hash { + // We are not going to migrate if were not initializing and the hashes + // match indicating that the stored config is valid. If we are + // initializing, then we also assume the the backend config is OK if + // the hashes match, as long as we're not providing any new overrides. + if (uint64(cHash) == s.Backend.Hash) && (!opts.Init || opts.ConfigOverride == nil) { log.Printf("[TRACE] Meta.Backend: using already-initialized, unchanged %q backend configuration", c.Type) return m.backend_C_r_S_unchanged(c, cHash, sMgr) } @@ -823,8 +822,6 @@ func (m *Meta) backend_C_r_S_changed(c *configs.Backend, cHash int, sMgr *state. return nil, diags } - fmt.Println("HERE") - // Perform the migration err := m.backendMigrateState(&backendMigrateOpts{ OneType: s.Backend.Type, From c017149b31fd82c40ccac65c2b1dfb814555818c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 24 May 2019 14:51:18 -0400 Subject: [PATCH 3/4] don't store prepared backend config The backend gets to "prepare" the configuration before Configure is called, in order to validate the values and insert defaults. We don't want to store this value in the "config state", because it will often not match the raw config after it is prepared, forcing unecessary backend migrations during init. Since PrepareConfig is always called before Configure, we can store the config value directly, and assume that it will be prepared in the same manner each time. --- command/init_test.go | 46 +++++++++++++++++++++++++++++++++ command/meta_backend.go | 12 +++------ command/meta_backend_migrate.go | 1 - 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 6e5985944..bda5fe06e 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -456,6 +456,52 @@ func TestInit_backendConfigKVReInit(t *testing.T) { } } +func TestInit_backendConfigKVReInitWithConfigDiff(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + copy.CopyDir(testFixturePath("init-backend"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + ui := new(cli.MockUi) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + + args := []string{"-input=false"} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + ui = new(cli.MockUi) + c = &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + + // a second init with identical config should require no changes, nor + // should it change the backend. + args = []string{"-input=false", "-backend-config", "path=foo"} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + // make sure the backend is configured how we expect + configState := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + cfg := map[string]interface{}{} + if err := json.Unmarshal(configState.Backend.ConfigRaw, &cfg); err != nil { + t.Fatal(err) + } + if cfg["path"] != "foo" { + t.Fatalf(`expected backend path="foo", got path="%v"`, cfg["foo"]) + } +} + func TestInit_targetSubdir(t *testing.T) { // Create a temporary working directory that is empty td := tempDir(t) diff --git a/command/meta_backend.go b/command/meta_backend.go index 738a0b94f..205361cba 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -185,9 +185,8 @@ func (m *Meta) BackendForPlan(settings plans.Backend) (backend.Enhanced, tfdiags if validateDiags.HasErrors() { return nil, diags } - configVal = newVal - configureDiags := b.Configure(configVal) + configureDiags := b.Configure(newVal) diags = diags.Append(configureDiags) // If the backend supports CLI initialization, do it. @@ -922,9 +921,8 @@ func (m *Meta) backend_C_r_S_unchanged(c *configs.Backend, cHash int, sMgr *stat if validDiags.HasErrors() { return nil, diags } - configVal = newVal - configDiags := b.Configure(configVal) + configDiags := b.Configure(newVal) diags = diags.Append(configDiags) if configDiags.HasErrors() { return nil, diags @@ -1051,9 +1049,8 @@ func (m *Meta) backendInitFromConfig(c *configs.Backend) (backend.Backend, cty.V if validateDiags.HasErrors() { return nil, cty.NilVal, diags } - configVal = newVal - configureDiags := b.Configure(configVal) + configureDiags := b.Configure(newVal) diags = diags.Append(configureDiags.InConfigBody(c.Config)) return b, configVal, diags @@ -1082,9 +1079,8 @@ func (m *Meta) backendInitFromSaved(s *terraform.BackendState) (backend.Backend, if validateDiags.HasErrors() { return nil, diags } - configVal = newVal - configureDiags := b.Configure(configVal) + configureDiags := b.Configure(newVal) diags = diags.Append(configureDiags) return b, diags diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index 8d020f90a..9227844a1 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -98,7 +98,6 @@ func (m *Meta) backendMigrateState(opts *backendMigrateOpts) error { // Multi-state to multi-state. We merge the states together (migrating // each from the source to the destination one by one). case !oneSingle && !twoSingle: - fmt.Printf("STATES: %q\n", oneStates) // If the source only has one state and it is the default, // treat it as if it doesn't support multi-state. if len(oneStates) == 1 && oneStates[0] == backend.DefaultStateName { From 06dfc4abd8f132fb4909b0dd2c8f0c63bd422d6a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 29 May 2019 12:58:04 -0500 Subject: [PATCH 4/4] allow setting -backend-config='' to unset override There is currently no way to unset -backend-config during init, since not setting that option assumes the user will use the saved config. Allow setting `-backend-config=""` to specify no overrides. --- command/init.go | 8 +++++++- command/init_test.go | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/command/init.go b/command/init.go index b8c8faab5..c4e9554df 100644 --- a/command/init.go +++ b/command/init.go @@ -299,7 +299,7 @@ func (c *InitCommand) Run(args []string) int { if back == nil { // If we didn't initialize a backend then we'll try to at least - // instantiate one. This might fail if it wasn't already initalized + // instantiate one. This might fail if it wasn't already initialized // by a previous run, so we must still expect that "back" may be nil // in code that follows. var backDiags tfdiags.Diagnostics @@ -675,6 +675,12 @@ func (c *InitCommand) backendConfigOverrideBody(flags rawFlags, schema *configsc synthVals = make(map[string]cty.Value) } + if len(items) == 1 && items[0].Value == "" { + // Explicitly remove all -backend-config options. + // We do this by setting an empty but non-nil ConfigOverrides. + return configs.SynthBody("-backend-config=''", synthVals), diags + } + for _, item := range items { eq := strings.Index(item.Value, "=") diff --git a/command/init_test.go b/command/init_test.go index bda5fe06e..dfae2e02a 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -454,6 +454,22 @@ func TestInit_backendConfigKVReInit(t *testing.T) { if cfg["path"] != "test" { t.Fatalf(`expected backend path="test", got path="%v"`, cfg["path"]) } + + // override the -backend-config options by settings + args = []string{"-input=false", "-backend-config", ""} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + // make sure the backend is configured how we expect + configState = testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + cfg = map[string]interface{}{} + if err := json.Unmarshal(configState.Backend.ConfigRaw, &cfg); err != nil { + t.Fatal(err) + } + if cfg["path"] != nil { + t.Fatalf(`expected backend path="", got path="%v"`, cfg["path"]) + } } func TestInit_backendConfigKVReInitWithConfigDiff(t *testing.T) {