From ff2d753062b9a9d40dafbdb02839c2aff0701dfc Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 29 Mar 2017 12:50:20 -0400 Subject: [PATCH 1/4] add Rehash to terraform.BackendState This method mirrors that of config.Backend, so we can compare the configration of a backend read from a config vs that of a backend read from a state. This will prevent init from reinitializing when using `-backend-config` options that match the existing state. --- command/init_test.go | 58 +++++++++++++++++++ command/meta_backend.go | 18 +++++- .../test-fixtures/init-backend-empty/main.tf | 4 ++ config/config_terraform.go | 2 +- terraform/state.go | 27 +++++++++ 5 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 command/test-fixtures/init-backend-empty/main.tf diff --git a/command/init_test.go b/command/init_test.go index dee54495d..ede29cfc3 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -406,6 +406,64 @@ func TestInit_copyBackendDst(t *testing.T) { } } +func TestInit_backendReinitWithExtra(t *testing.T) { + td := tempDir(t) + copy.CopyDir(testFixturePath("init-backend-empty"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + m := testMetaBackend(t, nil) + opts := &BackendOpts{ + ConfigExtra: map[string]interface{}{"path": "hello"}, + Init: true, + } + + b, err := m.backendConfig(opts) + if err != nil { + t.Fatal(err) + } + + ui := new(cli.MockUi) + c := &InitCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(testProvider()), + Ui: ui, + }, + } + + args := []string{"-backend-config", "path=hello"} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + // Read our saved backend config and verify we have our settings + state := testStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + if v := state.Backend.Config["path"]; v != "hello" { + t.Fatalf("bad: %#v", v) + } + + if state.Backend.Hash != b.Hash { + t.Fatal("mismatched state and config backend hashes") + } + + if state.Backend.Rehash() != b.Rehash() { + t.Fatal("mismatched state and config re-hashes") + } + + // init again and make sure nothing changes + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + state = testStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + if v := state.Backend.Config["path"]; v != "hello" { + t.Fatalf("bad: %#v", v) + } + + if state.Backend.Hash != b.Hash { + t.Fatal("mismatched state and config backend hashes") + } +} + /* func TestInit_remoteState(t *testing.T) { tmp, cwd := testCwd(t) diff --git a/command/meta_backend.go b/command/meta_backend.go index b30659dc0..8cf639044 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -415,8 +415,16 @@ func (m *Meta) backendFromConfig(opts *BackendOpts) (backend.Backend, error) { case c != nil && s.Remote.Empty() && !s.Backend.Empty(): // If our configuration is the same, then we're just initializing // a previously configured remote backend. - if !s.Backend.Empty() && s.Backend.Hash == cHash { - return m.backend_C_r_S_unchanged(c, sMgr) + if !s.Backend.Empty() { + hash := s.Backend.Hash + // on init we need an updated hash containing any extra options + // that were added after merging. + if opts.Init { + hash = s.Backend.Rehash() + } + if hash == cHash { + return m.backend_C_r_S_unchanged(c, sMgr) + } } if !opts.Init { @@ -451,7 +459,11 @@ func (m *Meta) backendFromConfig(opts *BackendOpts) (backend.Backend, error) { case c != nil && !s.Remote.Empty() && !s.Backend.Empty(): // If the hashes are the same, we have a legacy remote state with // an unchanged stored backend state. - if s.Backend.Hash == cHash { + hash := s.Backend.Hash + if opts.Init { + hash = s.Backend.Rehash() + } + if hash == cHash { if !opts.Init { initReason := fmt.Sprintf( "Legacy remote state found with configured backend %q", diff --git a/command/test-fixtures/init-backend-empty/main.tf b/command/test-fixtures/init-backend-empty/main.tf new file mode 100644 index 000000000..7f62e0e19 --- /dev/null +++ b/command/test-fixtures/init-backend-empty/main.tf @@ -0,0 +1,4 @@ +terraform { + backend "local" { + } +} diff --git a/config/config_terraform.go b/config/config_terraform.go index a547cc798..8535c9648 100644 --- a/config/config_terraform.go +++ b/config/config_terraform.go @@ -72,7 +72,7 @@ type Backend struct { Hash uint64 } -// Hash returns a unique content hash for this backend's configuration +// Rehash returns a unique content hash for this backend's configuration // as a uint64 value. func (b *Backend) Rehash() uint64 { // If we have no backend, the value is zero diff --git a/terraform/state.go b/terraform/state.go index 4e5aa713f..d5adefca1 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/config" "github.com/mitchellh/copystructure" + "github.com/mitchellh/hashstructure" "github.com/satori/go.uuid" ) @@ -801,6 +802,32 @@ func (s *BackendState) Empty() bool { return s == nil || s.Type == "" } +// Rehash returns a unique content hash for this backend's configuration +// as a uint64 value. +// The Hash stored in the backend state needs to match the config itself, but +// we need to compare the backend config after it has been combined with all +// options. +// This function must match the implementation used by config.Backend. +func (s *BackendState) Rehash() uint64 { + if s == nil { + return 0 + } + + // Use hashstructure to hash only our type with the config. + code, err := hashstructure.Hash(map[string]interface{}{ + "type": s.Type, + "config": s.Config, + }, nil) + + // This should never happen since we have just some basic primitives + // so panic if there is an error. + if err != nil { + panic(err) + } + + return code +} + // RemoteState is used to track the information about a remote // state store that we push/pull state to. type RemoteState struct { From c891ab50b7bd1c2f282a97d324a6ef4ea63a0cd3 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 29 Mar 2017 15:51:24 -0400 Subject: [PATCH 2/4] detect when backend.Hash needs update It's possible to not change the backend config, but require updating the stored backend state by moving init options from the config file to the `-backend-config` flag. If the config is the same, but the hash doesn't match, update the stored state. --- command/init_test.go | 44 +++++++++++++++++++++++++++++++++++++++++ command/meta_backend.go | 10 ++++++++++ 2 files changed, 54 insertions(+) diff --git a/command/init_test.go b/command/init_test.go index ede29cfc3..a0a8c32a0 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -464,6 +464,50 @@ func TestInit_backendReinitWithExtra(t *testing.T) { } } +// move option from config to -backend-config args +func TestInit_backendReinitConfigToExtra(t *testing.T) { + 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{ + ContextOpts: testCtxConfig(testProvider()), + Ui: ui, + }, + } + + if code := c.Run([]string{"-input=false"}); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + // Read our saved backend config and verify we have our settings + state := testStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + if v := state.Backend.Config["path"]; v != "foo" { + t.Fatalf("bad: %#v", v) + } + + backendHash := state.Backend.Hash + + // init again but remove the path option from the config + cfg := "terraform {\n backend \"local\" {}\n}\n" + if err := ioutil.WriteFile("main.tf", []byte(cfg), 0644); err != nil { + t.Fatal(err) + } + + args := []string{"-input=false", "-backend-config=path=foo"} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + state = testStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + + if state.Backend.Hash == backendHash { + t.Fatal("state.Backend.Hash was not updated") + } +} + /* func TestInit_remoteState(t *testing.T) { tmp, cwd := testCwd(t) diff --git a/command/meta_backend.go b/command/meta_backend.go index 8cf639044..c50214116 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -1158,6 +1158,16 @@ func (m *Meta) backend_C_r_S_unchanged( c *config.Backend, sMgr state.State) (backend.Backend, error) { s := sMgr.State() + // it's possible for a backend to be unchanged, and the config itself to + // have changed by moving a paramter from the config to `-backend-config` + // In this case we only need to update the Hash. + if c != nil && s.Backend.Hash != c.Hash { + s.Backend.Hash = c.Hash + if err := sMgr.WriteState(s); err != nil { + return nil, fmt.Errorf(errBackendWriteSaved, err) + } + } + // Create the config. We do this from the backend state since this // has the complete configuration data whereas the config itself // may require input. From 7d23e1ef2090d1190ff514fdb0515111eded9cb5 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 29 Mar 2017 17:50:55 -0400 Subject: [PATCH 3/4] add equivalent tests to meta_backend_test --- command/meta_backend_test.go | 104 +++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/command/meta_backend_test.go b/command/meta_backend_test.go index 8c1fe2d66..143759f98 100644 --- a/command/meta_backend_test.go +++ b/command/meta_backend_test.go @@ -3217,6 +3217,110 @@ func TestMetaBackend_planLegacy(t *testing.T) { } } +// init a backend using -backend-config options multiple times +func TestMetaBackend_configureWithExtra(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + copy.CopyDir(testFixturePath("init-backend-empty"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + extras := map[string]interface{}{"path": "hello"} + m := testMetaBackend(t, nil) + opts := &BackendOpts{ + ConfigExtra: extras, + Init: true, + } + + backendCfg, err := m.backendConfig(opts) + if err != nil { + t.Fatal(err) + } + + // init the backend + _, err = m.Backend(&BackendOpts{ + ConfigExtra: extras, + Init: true, + }) + if err != nil { + t.Fatalf("bad: %s", err) + } + + // Check the state + s := testStateRead(t, filepath.Join(DefaultDataDir, backendlocal.DefaultStateFilename)) + if s.Backend.Hash != backendCfg.Hash { + t.Fatal("mismatched state and config backend hashes") + } + if s.Backend.Rehash() == s.Backend.Hash { + t.Fatal("saved hash should not match actual hash") + } + if s.Backend.Rehash() != backendCfg.Rehash() { + t.Fatal("mismatched state and config re-hashes") + } + + // init the backend again with the same options + m = testMetaBackend(t, nil) + _, err = m.Backend(&BackendOpts{ + ConfigExtra: extras, + Init: true, + }) + if err != nil { + t.Fatalf("bad: %s", err) + } + + // Check the state + s = testStateRead(t, filepath.Join(DefaultDataDir, backendlocal.DefaultStateFilename)) + if s.Backend.Hash != backendCfg.Hash { + t.Fatal("mismatched state and config backend hashes") + } +} + +// move options from config to -backend-config +func TestMetaBackend_configToExtra(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)() + + // init the backend + m := testMetaBackend(t, nil) + _, err := m.Backend(&BackendOpts{ + Init: true, + }) + if err != nil { + t.Fatalf("bad: %s", err) + } + + // Check the state + s := testStateRead(t, filepath.Join(DefaultDataDir, backendlocal.DefaultStateFilename)) + backendHash := s.Backend.Hash + + // init again but remove the path option from the config + cfg := "terraform {\n backend \"local\" {}\n}\n" + if err := ioutil.WriteFile("main.tf", []byte(cfg), 0644); err != nil { + t.Fatal(err) + } + + // init the backend again with the options + extras := map[string]interface{}{"path": "hello"} + m = testMetaBackend(t, nil) + m.forceInitCopy = true + _, err = m.Backend(&BackendOpts{ + ConfigExtra: extras, + Init: true, + }) + if err != nil { + t.Fatalf("bad: %s", err) + } + + s = testStateRead(t, filepath.Join(DefaultDataDir, backendlocal.DefaultStateFilename)) + + if s.Backend.Hash == backendHash { + t.Fatal("state.Backend.Hash was not updated") + } +} + func testMetaBackend(t *testing.T, args []string) *Meta { var m Meta m.Ui = new(cli.MockUi) From c55a5082f5117709d7fddf090fdd80ebc4e9f185 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 29 Mar 2017 18:01:03 -0400 Subject: [PATCH 4/4] delegate BackendState.Rehash to config.Backend --- terraform/state.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/terraform/state.go b/terraform/state.go index d5adefca1..905ec3dcb 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -20,7 +20,6 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/config" "github.com/mitchellh/copystructure" - "github.com/mitchellh/hashstructure" "github.com/satori/go.uuid" ) @@ -813,19 +812,14 @@ func (s *BackendState) Rehash() uint64 { return 0 } - // Use hashstructure to hash only our type with the config. - code, err := hashstructure.Hash(map[string]interface{}{ - "type": s.Type, - "config": s.Config, - }, nil) - - // This should never happen since we have just some basic primitives - // so panic if there is an error. - if err != nil { - panic(err) + cfg := config.Backend{ + Type: s.Type, + RawConfig: &config.RawConfig{ + Raw: s.Config, + }, } - return code + return cfg.Rehash() } // RemoteState is used to track the information about a remote