Skip to content

Commit

Permalink
configs: Fix module loader nil pointer panic
Browse files Browse the repository at this point in the history
In configurations which have already been initialized, updating the
source of a non-root module call to an invalid value could cause a nil
pointer panic. This commit fixes the bug and adds test coverage.
  • Loading branch information
alisdair committed May 17, 2022
1 parent 1fd140f commit d1e35a3
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 3 deletions.
7 changes: 5 additions & 2 deletions internal/configs/configload/loader_load.go
Expand Up @@ -60,8 +60,11 @@ func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module,

var diags hcl.Diagnostics

// Check for inconsistencies between manifest and config
if req.SourceAddr.String() != record.SourceAddr {
// Check for inconsistencies between manifest and config.

// We ignore a nil SourceAddr here, which represents a failure during
// configuration parsing, and will be reported in a diagnostic elsewhere.
if req.SourceAddr != nil && req.SourceAddr.String() != record.SourceAddr {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Module source has changed",
Expand Down
21 changes: 21 additions & 0 deletions internal/configs/configload/loader_snapshot_test.go
@@ -1,6 +1,7 @@
package configload

import (
"os"
"path/filepath"
"reflect"
"testing"
Expand Down Expand Up @@ -71,6 +72,26 @@ module "child_b" {

}

func TestLoadConfigWithSnapshot_invalidSource(t *testing.T) {
fixtureDir := filepath.Clean("testdata/already-installed-now-invalid")

old, _ := os.Getwd()
os.Chdir(fixtureDir)
defer os.Chdir(old)

loader, err := NewLoader(&Config{
ModulesDir: ".terraform/modules",
})
if err != nil {
t.Fatalf("unexpected error from NewLoader: %s", err)
}

_, _, diags := loader.LoadConfigWithSnapshot(".")
if !diags.HasErrors() {
t.Error("LoadConfigWithSnapshot succeeded; want errors")
}
}

func TestSnapshotRoundtrip(t *testing.T) {
fixtureDir := filepath.Clean("testdata/already-installed")
loader, err := NewLoader(&Config{
Expand Down
2 changes: 1 addition & 1 deletion internal/configs/configload/loader_test.go
Expand Up @@ -14,7 +14,7 @@ func assertNoDiagnostics(t *testing.T, diags hcl.Diagnostics) bool {

func assertDiagnosticCount(t *testing.T, diags hcl.Diagnostics, want int) bool {
t.Helper()
if len(diags) != 0 {
if len(diags) != want {
t.Errorf("wrong number of diagnostics %d; want %d", len(diags), want)
for _, diag := range diags {
t.Logf("- %s", diag)
Expand Down
@@ -0,0 +1 @@
{"Modules":[{"Key":"","Source":"","Dir":"."},{"Key":"foo","Source":"./foo","Dir":"foo"},{"Key":"foo.bar","Source":"./bar","Dir":"foo/bar"}]}
@@ -0,0 +1,3 @@
output "hello" {
value = "Hello from foo/bar"
}
@@ -0,0 +1,3 @@
module "bar" {
source = "${path.module}/bar"
}
@@ -0,0 +1,3 @@
module "foo" {
source = "./foo"
}

0 comments on commit d1e35a3

Please sign in to comment.