upbound / up

The @upbound CLI
Apache License 2.0
49 stars 42 forks source link

`up ctx` panics if trying to set the same context #545

Open RedbackThomson opened 1 month ago

RedbackThomson commented 1 month ago

What happened?

@avalanche123 found that up ctx panics under certain conditions:

goroutine 1 [running]:
runtime/debug.Stack()
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/runtime/debug/stack.go:24 +0x64
runtime/debug.PrintStack()
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/runtime/debug/stack.go:16 +0x1c
github.com/charmbracelet/bubbletea.(*Program).Run.func1()
        /Users/bulat/go/pkg/mod/github.com/charmbracelet/bubbletea@v0.25.0/tea.go:478 +0x88
panic({0x10631d300?, 0x108515cb0?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/runtime/panic.go:770 +0x124
github.com/upbound/up/cmd/up/ctx.(*fileWriter).Write(0x14000bc1f40, 0x140013b0540)
        /Users/bulat/Projects/upbound/up/cmd/up/ctx/writer.go:81 +0x98
github.com/upbound/up/cmd/up/ctx.(*Space).Accept(0x14001336ba0, 0x1400066fdb0?, {0x10686c448, 0x14000bc1f40})
        /Users/bulat/Projects/upbound/up/cmd/up/ctx/actions.go:40 +0x11c
github.com/upbound/up/cmd/up/ctx.(*Space).Items.func2({0x21, {0x0, 0x0, 0x0, 0x0, 0x1, 0x0, {0x1053a4ba6, 0x4}, {0x1053a7ee3, ...}, ...}, ...})
        /Users/bulat/Projects/upbound/up/cmd/up/ctx/navigation.go:253 +0x54
github.com/upbound/up/cmd/up/ctx.model.Update({0x21, {0x0, 0x0, 0x0, 0x0, 0x1, 0x0, {0x1053a4ba6, 0x4}, {0x1053a7ee3, ...}, ...}, ...}, ...)
        /Users/bulat/Projects/upbound/up/cmd/up/ctx/list.go:231 +0x1320
github.com/charmbracelet/bubbletea.(*Program).eventLoop(0x1400136d790, {0x106888ae8?, 0x14001370008?}, 0x140006ecd20)
        /Users/bulat/go/pkg/mod/github.com/charmbracelet/bubbletea@v0.25.0/tea.go:411 +0x4d8
github.com/charmbracelet/bubbletea.(*Program).Run(0x1400136d790)
        /Users/bulat/go/pkg/mod/github.com/charmbracelet/bubbletea@v0.25.0/tea.go:543 +0x6c8
github.com/upbound/up/cmd/up/ctx.(*Cmd).RunInteractive(0x140011021d0, {0x10689ae30, 0x10875eb48}, 0x1400075ba00, 0x14000f82b40?, {0x10687c110, 0x10875eb48})
        /Users/bulat/Projects/upbound/up/cmd/up/ctx/cmd.go:346 +0x3b4
github.com/upbound/up/cmd/up/ctx.(*Cmd).Run(0x140011021d0, {0x10689ae30, 0x10875eb48}, 0x1400075ba00, 0x14000f82b40)
        /Users/bulat/Projects/upbound/up/cmd/up/ctx/cmd.go:119 +0x13c
reflect.Value.call({0x1064e7da0?, 0x140011021d0?, 0x14000d1b908?}, {0x1053a4b1a, 0x4}, {0x14000f20fc0, 0x3, 0x102e6f458?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/reflect/value.go:596 +0x970
reflect.Value.Call({0x1064e7da0?, 0x140011021d0?, 0x14000d1ba08?}, {0x14000f20fc0?, 0x10684e680?, 0x102c797f0?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/reflect/value.go:380 +0x94
github.com/alecthomas/kong.callFunction({0x1064e7da0?, 0x140011021d0?, 0x18?}, 0x14000fd3710)
        /Users/bulat/go/pkg/mod/github.com/alecthomas/kong@v0.8.0/callbacks.go:98 +0x390
github.com/alecthomas/kong.callMethod({0x1053a24d4, 0x3}, {0x1065f58a0?, 0x140011021d0?, 0x3?}, {0x1064e7da0?, 0x140011021d0?, 0x14000b5b720?}, 0x106442380?)
        /Users/bulat/go/pkg/mod/github.com/alecthomas/kong@v0.8.0/callbacks.go:132 +0x54
github.com/alecthomas/kong.(*Context).RunNode(0x1400075ba00, 0x14000d2e000, {0x0, 0x0, 0x1068ca560?})
        /Users/bulat/go/pkg/mod/github.com/alecthomas/kong@v0.8.0/context.go:762 +0x64c
github.com/alecthomas/kong.(*Context).Run(0x14000f1b7a0?, {0x0?, 0x10875eb48?, 0x10605de00?})
        /Users/bulat/go/pkg/mod/github.com/alecthomas/kong@v0.8.0/context.go:787 +0x138
main.main()
        /Users/bulat/Projects/upbound/up/cmd/up/main.go:176 +0x3f0
up: error: ctx.Cmd.Run(): unexpected model type: <nil>
exit status 1

It appears that when comparing the new context to the existing current context, it will return nil for the new config pointer. Since we don't do a nil check before updating the config, this causes the panic.

Ideally we'd check to see if the context matches and do a safe no-op, since it doesn't make sense to update anything.

How can we reproduce it?

What environment did it happen in?