Missing validation for incorrectly quoted moved addresses

[rbtcollins]

Terraform Version

1.5.3

Terraform Configuration Files

I can't tell whats relevant, and our configuration is uhm, sizeable. Also internal.

Debug Output

.

Expected Behavior

Terraform should not have crashed.

Actual Behavior

running "/atlantis/bin/terraform1.5.3 init -input=false -upgrade" in "/atlantis/repos/cognitedata/terraform/8272/default/cdf/greenfield": exit status 11

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

Terraform crashed! This is always indicative of a bug within Terraform.
Please report the crash with Terraform[1] so that we can fix this.

When reporting bugs, please include your terraform version, the stack trace
shown below, and any additional information which may help replicate the issue.

[1]: https://github.com/hashicorp/terraform/issues

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

runtime error: invalid memory address or nil pointer dereference
goroutine 1 [running]:
runtime/debug.Stack()
	/opt/hostedtoolcache/go/1.20.0/x64/src/runtime/debug/stack.go:24 +0x65
runtime/debug.PrintStack()
	/opt/hostedtoolcache/go/1.20.0/x64/src/runtime/debug/stack.go:16 +0x19
github.com/hashicorp/terraform/internal/logging.PanicHandler()
	/home/runner/work/terraform/terraform/internal/logging/panic.go:58 +0x153
panic({0x242c520, 0x41dc420})
	/opt/hostedtoolcache/go/1.20.0/x64/src/runtime/panic.go:890 +0x263
github.com/hashicorp/terraform/internal/addrs.(*MoveEndpoint).String(...)
	/home/runner/work/terraform/terraform/internal/addrs/move_endpoint.go:54
github.com/hashicorp/terraform/internal/configs.(*Module).appendFile(0xc000b01d40, 0xc000164c00)
	/home/runner/work/terraform/terraform/internal/configs/module.go:443 +0x3dd8
github.com/hashicorp/terraform/internal/configs.NewModule({0xc00013e3e0, 0x4, 0x0?}, {0x0, 0x0, 0x122f616?})
	/home/runner/work/terraform/terraform/internal/configs/module.go:154 +0x475
github.com/hashicorp/terraform/internal/configs.(*Parser).LoadConfigDir(0xc000154000?, {0x2db13b8, 0x1})
	/home/runner/work/terraform/terraform/internal/configs/parser_config_dir.go:45 +0x265
github.com/hashicorp/terraform/internal/command.(*Meta).loadSingleModule(0xc000064234?, {0xc000064234?, 0xc0001541a4?})
	/home/runner/work/terraform/terraform/internal/command/meta_config.go:71 +0x9b
github.com/hashicorp/terraform/internal/command.(*InitCommand).Run(0xc000154000, {0xc0000500a0, 0x2, 0x2})
	/home/runner/work/terraform/terraform/internal/command/init.go:161 +0xe8b
github.com/mitchellh/cli.(*CLI).Run(0xc00013c280)
	/home/runner/go/pkg/mod/github.com/mitchellh/[email protected]/cli.go:262 +0x5f8
main.realMain()
	/home/runner/work/terraform/terraform/main.go:318 +0x1729
main.main()
	/home/runner/work/terraform/terraform/main.go:61 +0x19

Steps to Reproduce

terraform1.5.3 init -input=false -upgrade

Additional Context

No response

References

No response

[jbardin]

Hi @rbtcollins,

Thanks for filing the issue! Can you verify that the crash still happens in a current release version, to make sure it hasn't already been patched? I'm not sure what would trigger this yet, but my first guess us that there is a moved block with an invalid to or from address that somehow slips through.

Thanks!

[rbtcollins]

running "/atlantis/bin/terraform1.6.1 init -input=false -upgrade" in "/atlantis/repos/....": exit status 11

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

Terraform crashed! This is always indicative of a bug within Terraform.
Please report the crash with Terraform[1] so that we can fix this.

When reporting bugs, please include your terraform version, the stack trace
shown below, and any additional information which may help replicate the issue.

[1]: https://github.com/hashicorp/terraform/issues

!!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

runtime error: invalid memory address or nil pointer dereference
goroutine 1 [running]:
runtime/debug.Stack()
	/opt/hostedtoolcache/go/1.21.1/x64/src/runtime/debug/stack.go:24 +0x5e
runtime/debug.PrintStack()
	/opt/hostedtoolcache/go/1.21.1/x64/src/runtime/debug/stack.go:16 +0x13
github.com/hashicorp/terraform/internal/logging.PanicHandler()
	/home/runner/work/terraform/terraform/internal/logging/panic.go:58 +0x13b
panic({0x2cd7d40?, 0x50ab790?})
	/opt/hostedtoolcache/go/1.21.1/x64/src/runtime/panic.go:920 +0x270
github.com/hashicorp/terraform/internal/addrs.(*MoveEndpoint).String(...)
	/home/runner/work/terraform/terraform/internal/addrs/move_endpoint.go:54
github.com/hashicorp/terraform/internal/configs.(*Module).appendFile(0xc0005771e0, 0xc000574180)
	/home/runner/work/terraform/terraform/internal/configs/module.go:456 +0x3d75
github.com/hashicorp/terraform/internal/configs.NewModule({0xc0002e1f00, 0x5, 0x1?}, {0x0, 0x0, 0x1?})
	/home/runner/work/terraform/terraform/internal/configs/module.go:167 +0x4d5
github.com/hashicorp/terraform/internal/configs.NewModuleWithTests(...)
	/home/runner/work/terraform/terraform/internal/configs/module.go:100
github.com/hashicorp/terraform/internal/configs.(*Parser).LoadConfigDirWithTests(0xc000996e00?, {0x38bf420, 0x1}, {0x317b3ae?, 0x41?})
	/home/runner/work/terraform/terraform/internal/configs/parser_config_dir.go:73 +0x3a5
github.com/hashicorp/terraform/internal/command.(*Meta).loadSingleModuleWithTests(0xc000062234?, {0xc000062234?, 0xc0009a9c20?}, {0x317b3ae, 0x5})
	/home/runner/work/terraform/terraform/internal/command/meta_config.go:108 +0xac
github.com/hashicorp/terraform/internal/command.(*InitCommand).Run(0xc000996e00, {0xc0000500a0, 0x2, 0x2})
	/home/runner/work/terraform/terraform/internal/command/init.go:176 +0x125a
github.com/mitchellh/cli.(*CLI).Run(0xc0002fc3c0)
	/home/runner/go/pkg/mod/github.com/mitchellh/[email protected]/cli.go:262 +0x5b8
main.realMain()
	/home/runner/work/terraform/terraform/main.go:339 +0x1eab
main.main()
	/home/runner/work/terraform/terraform/main.go:64 +0x13

[rbtcollins]

Yes, I have a move block. I'm refactoring a configuration from:

root module +- childone +- childtwo

to root module

• unify
  • childone with count=0/1 conditional
  • childtwo

in order to remove highly duplicated code in our fleet - we have many root modules that are 99% identical code (providers, marshalling outputs of childone to childtwo).

moved {
  from = "module.childone"
  to   = "module.unify.module.childone[0]"
}
moved {
  from = "module.childtwo"
  to   = "module.unify.module.childtwo"
}

[rbtcollins]

Just finding my way at the moment,

=> 421:         m.Moved = append(m.Moved, file.Moved...)
   422:
   423:         for _, i := range file.Import {
   424:                 for _, mi := range m.Import {
   425:                         if i.To.Equal(mi.To) {
   426:                                 diags = append(diags, &hcl.Diagnostic{
(dlv) p file.Moved
[]*github.com/hashicorp/terraform/internal/configs.Moved len: 2, cap: 2, [
        *{
                From: *github.com/hashicorp/terraform/internal/addrs.MoveEndpoint nil,
                To: *github.com/hashicorp/terraform/internal/addrs.MoveEndpoint nil,
                DeclRange: (*"github.com/hashicorp/hcl/v2.Range")(0xc0000c3230),},
        *{
                From: *github.com/hashicorp/terraform/internal/addrs.MoveEndpoint nil,
                To: *github.com/hashicorp/terraform/internal/addrs.MoveEndpoint nil,
                DeclRange: (*"github.com/hashicorp/hcl/v2.Range")(0xc0000c3280),},
]

This seems very wrong - it is as far as I can tell the right file - it has the module invocation in it; the code looks like that in my previous comment, but has endpoint values of nil

[rbtcollins]

The problem was the quotation marks around the resource addresses. Removing them prevented the crash. I can't offer a patch - nothing ideological, just the overheads of a CLA for a one-off don't add up; I would suggest that both the parser extracting addresses and the appendFile method that is consuming the slice of Moved's should be validating the datastructure's basic properies. E.g. pre conditions or other such assertions (since golang doesn't permit encoding this in the type system).

[jbardin]

No worries @rbtcollins, thanks for adding the configuration example!

[kmoe]

The panic occurs when there is an import block and a moved block without valid to and from addresses in the same module.

Repro:

moved {
  to = null_resource.foo
  from = "null_resource.bar"
}

import {}

Minimal repro:

moved {}
import {}

The cross-validation of the import block and moved block args happens whether or not there were errors parsing the args, hence panic.

[kiweezi]

Hey @jbardin sorry for getting that mixed up! I'll remove the comment :).