Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

Do not allow configure to remove a definition when a instance depends on it #69

Open
vieiralucas opened this issue Sep 9, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@vieiralucas
Copy link
Member

vieiralucas commented Sep 9, 2023

Reproducing the bug

configure-remove-definition-bug.mov
  1. Configure the local example:
layerform configure --file examples/local/layerform.json
  1. Spawn an instance of bar
layerform spawn bar bar_instance
  1. Update examples/local/layerform.json removing layer bar:
{
  "layers": [
    {
      "name": "foo",
      "files": ["foo.tf"]
    },
    {
      "name"  : "baz",
      "files": ["baz.tf"],
      "dependencies": ["foo"]
    }
  ]
}
  1. Run configure again
layerform configure --file examples/local/layerform.json

Well, we just broke layerform, just trying to run layeform list instances will result in a panic, trying to kill bar_instance will also fail because it will not find the definition.

> layerform list instances                                
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x40 pc=0x1054f05c0]

goroutine 1 [running]:
github.com/ergomake/layerform/cmd/cli.computeDepth(0x1058a89a0?, 0x140001c5ae8?, 0x0)
        /Users/lucas/code/ergomake/layerform/cmd/cli/list_instances.go:111 +0x20
github.com/ergomake/layerform/cmd/cli.sortInstancesByDepth.func1(0x140001c58e8?, 0x1)
        /Users/lucas/code/ergomake/layerform/cmd/cli/list_instances.go:125 +0x68
sort.insertionSort_func({0x140001c59a0?, 0x14000141ec0?}, 0x0, 0x3)
        /opt/homebrew/Cellar/go/1.20.5/libexec/src/sort/zsortfunc.go:12 +0xc0
sort.stable_func({0x140001c59a0?, 0x14000141ec0?}, 0x3)
        /opt/homebrew/Cellar/go/1.20.5/libexec/src/sort/zsortfunc.go:343 +0x6c
sort.SliceStable({0x105856e40?, 0x1400012ced0?}, 0x4?)
        /opt/homebrew/Cellar/go/1.20.5/libexec/src/sort/slice.go:38 +0x104
github.com/ergomake/layerform/cmd/cli.sortInstancesByDepth({0x14000141e60, 0x3, 0x4}, 0x140001c5ae8)
        /Users/lucas/code/ergomake/layerform/cmd/cli/list_instances.go:122 +0x78
github.com/ergomake/layerform/cmd/cli.glob..func4(0x105e85c60?, {0x1054f29c3?, 0x0?, 0x0?})
        /Users/lucas/code/ergomake/layerform/cmd/cli/list_instances.go:82 +0x450
github.com/spf13/cobra.(*Command).execute(0x105e85c60, {0x105ec51b8, 0x0, 0x0})
        /Users/lucas/go/pkg/mod/github.com/spf13/[email protected]/command.go:944 +0x5b0
github.com/spf13/cobra.(*Command).ExecuteC(0x105e86220)
        /Users/lucas/go/pkg/mod/github.com/spf13/[email protected]/command.go:1068 +0x35c
github.com/spf13/cobra.(*Command).Execute(...)
        /Users/lucas/go/pkg/mod/github.com/spf13/[email protected]/command.go:992
github.com/ergomake/layerform/cmd/cli.Execute()
        /Users/lucas/code/ergomake/layerform/cmd/cli/root.go:30 +0xfc
main.main()
        /Users/lucas/code/ergomake/layerform/main.go:13 +0x144

Proposed solution

The second configure command should just fail in this situation, telling the user that it can't update configuration because it is trying to remove a definition that has an instance currently running.

@vieiralucas vieiralucas added the bug Something isn't working label Sep 9, 2023
@rafiramadhana
Copy link
Contributor

@vieiralucas Can I help with this?

I think we can compare the data in c.instancesBackend.ListInstances and layerfile.Layers to ensure no running instances are removed from the configuration.

Somewhere around this line:

layerfile, err := layerfile.FromFile(fpath)
if err != nil {
	loadSpinner.Error()
	sm.Stop()
	return errors.Wrap(err, "fail to read layerform layers definitions from file")
}

https://github.com/ergomake/layerform/blob/main/pkg/command/configure.go#L47

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants