treeverse / lakeFS

lakeFS - Data version control for your data lake | Git for data
https://docs.lakefs.io
Apache License 2.0
4.46k stars 359 forks source link

lakectl repo list segfaults on expired playground account #3696

Closed arielshaqed closed 2 years ago

arielshaqed commented 2 years ago

I (mistakenly...) attempted to access an ancient playground account:

❯ go run ./cmd/lakectl -c ~/.lakectl.demo.yaml repo list
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x163a0a6]

goroutine 1 [running]:
github.com/treeverse/lakefs/cmd/lakectl/cmd.glob..func67(0x2ca44e0, {0x1b39fde, 0x2, 0x2})
        /home/ariels/dev/lakeFS/cmd/lakectl/cmd/repo.go:35 +0x1c6
github.com/spf13/cobra.(*Command).execute(0x2ca44e0, {0xc0004aa9c0, 0x2, 0x2})
        /home/ariels/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:860 +0x5f8
github.com/spf13/cobra.(*Command).ExecuteC(0x2caa3e0)
        /home/ariels/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:974 +0x3bc
github.com/spf13/cobra.(*Command).Execute(...)
        /home/ariels/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:902
github.com/treeverse/lakefs/cmd/lakectl/cmd.Execute()
        /home/ariels/dev/lakeFS/cmd/lakectl/cmd/root.go:137 +0x25
main.main()
        /home/ariels/dev/lakeFS/cmd/lakectl/main.go:6 +0x17
exit status 2

This demo account (fancy-lion) serves a redirect, probably because it's expired:

❯ http get https://fancy-lion.lakefs-demo.io/api/v1
HTTP/1.1 307 Temporary Redirect
Connection: keep-alive
Content-Length: 66
Content-Type: text/html; charset=utf-8
Date: Wed, 20 Jul 2022 05:38:08 GMT
Location: https://demo.lakefs.io/expired

<a href="https://demo.lakefs.io/expired">Temporary Redirect</a>.

lakectl should handle redirects and other unexpected errors correctly.

My guess

lakectl uses the Swagger-generated client. A 3xx response is not possible according to this client. And it assumes some handling from the generated API client that does not actually occur.

itaidavid commented 2 years ago

lakectl need to handle such error gracefully Other than that, we might want to investigate if the error itself is problematic, but lakectl should not segfault