diff --git a/state/cache_test.go b/state/cache_test.go index 0edfd85643..7252637e00 100644 --- a/state/cache_test.go +++ b/state/cache_test.go @@ -1,6 +1,7 @@ package state import ( + "fmt" "os" "reflect" "testing" @@ -70,36 +71,42 @@ func TestCacheState_RefreshState(t *testing.T) { &terraform.ModuleState{ Path: terraform.RootModulePath, Resources: map[string]*terraform.ResourceState{ - "foo.foo": &terraform.ResourceState{}, + "foo.foo": &terraform.ResourceState{ + Primary: &terraform.InstanceState{ + ID: "ID", + }, + }, }, }, }, expected: CacheRefreshLocalNewer, }, } { - cache := testLocalState(t) - durable := testLocalState(t) - defer os.Remove(cache.Path) - defer os.Remove(durable.Path) + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + cache := testLocalState(t) + durable := testLocalState(t) + defer os.Remove(cache.Path) + defer os.Remove(durable.Path) - cs := &CacheState{ - Cache: cache, - Durable: durable, - } + cs := &CacheState{ + Cache: cache, + Durable: durable, + } - state := cache.State() - state.Modules = test.cacheModules - if err := cs.WriteState(state); err != nil { - t.Fatalf("err: %s", err) - } + state := cache.State() + state.Modules = test.cacheModules + if err := cs.WriteState(state); err != nil { + t.Fatalf("err: %s", err) + } - if err := cs.RefreshState(); err != nil { - t.Fatalf("err: %s", err) - } + if err := cs.RefreshState(); err != nil { + t.Fatalf("err: %s", err) + } - if cs.RefreshResult() != test.expected { - t.Fatalf("bad %d: %v", i, cs.RefreshResult()) - } + if cs.RefreshResult() != test.expected { + t.Fatalf("bad %d: %v", i, cs.RefreshResult()) + } + }) } } diff --git a/terraform/state.go b/terraform/state.go index 30023fb297..90b8d4dbc0 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -644,6 +644,10 @@ func (s *State) init() { } s.ensureHasLineage() + // We can't trust that state read from a file doesn't have nil/empty + // modules + s.prune() + for _, mod := range s.Modules { mod.init() } @@ -651,6 +655,7 @@ func (s *State) init() { if s.Remote != nil { s.Remote.init() } + } func (s *State) EnsureHasLineage() { @@ -695,6 +700,18 @@ func (s *State) prune() { if s == nil { return } + + // Filter out empty modules. + // A module is always assumed to have a path, and it's length isn't always + // bounds checked later on. Modules may be "emptied" during destroy, but we + // never want to store those in the state. + for i := 0; i < len(s.Modules); i++ { + if s.Modules[i] == nil || len(s.Modules[i].Path) == 0 { + s.Modules = append(s.Modules[:i], s.Modules[i+1:]...) + i-- + } + } + for _, mod := range s.Modules { mod.prune() } @@ -1074,11 +1091,12 @@ func (m *ModuleState) prune() { defer m.Unlock() for k, v := range m.Resources { - v.prune() - - if (v.Primary == nil || v.Primary.ID == "") && len(v.Deposed) == 0 { + if v == nil || (v.Primary == nil || v.Primary.ID == "") && len(v.Deposed) == 0 { delete(m.Resources, k) + continue } + + v.prune() } for k, v := range m.Outputs { @@ -1420,19 +1438,6 @@ func (s *ResourceState) init() { if s.Deposed == nil { s.Deposed = make([]*InstanceState, 0) } - - // clean out any possible nil values read in from the state file - end := len(s.Deposed) - 1 - for i := 0; i <= end; i++ { - if s.Deposed[i] == nil { - s.Deposed[i], s.Deposed[end] = s.Deposed[end], s.Deposed[i] - end-- - i-- - } else { - s.Deposed[i].init() - } - } - s.Deposed = s.Deposed[:end+1] } func (s *ResourceState) deepcopy() *ResourceState { diff --git a/terraform/state_test.go b/terraform/state_test.go index de84b8431f..f66c2b3242 100644 --- a/terraform/state_test.go +++ b/terraform/state_test.go @@ -1674,3 +1674,37 @@ func TestParseResourceStateKey(t *testing.T) { } } } + +func TestStateModuleOrphans_empty(t *testing.T) { + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: RootModulePath, + }, + &ModuleState{ + Path: []string{RootModuleName, "foo", "bar"}, + }, + &ModuleState{ + Path: []string{}, + }, + nil, + }, + } + + state.init() + + // just calling this to check for panic + state.ModuleOrphans(RootModulePath, nil) + + for _, mod := range state.Modules { + if mod == nil { + t.Fatal("found nil module") + } + if mod.Path == nil { + t.Fatal("found nil module path") + } + if len(mod.Path) == 0 { + t.Fatal("found empty module path") + } + } +}