Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle errors in secret store constructors. #135

Merged
merged 1 commit into from
Sep 21, 2018

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Sep 15, 2018

Fixes panic on bad aws configuration.

Before:

~/g/s/g/s/chamber 🤜 chamber list foo                                                         ✘ 130 fix-panic-bad-aws-session ✭ ◼
panic: AssumeRoleTokenProviderNotSetError: assume role with MFA enabled, but AssumeRoleTokenProvider session option not set.

goroutine 1 [running]:
github.com/aws/aws-sdk-go/aws/session.Must(...)
        /Users/joshuacarp/go/src/github.com/aws/aws-sdk-go/aws/session/session.go:281
github.com/segmentio/chamber/store.getSession(0xa, 0x16e7b92, 0x16)
        /Users/joshuacarp/go/src/github.com/segmentio/chamber/store/shared.go:21 +0x284
github.com/segmentio/chamber/store.NewSSMStore(0xa, 0x0)
        /Users/joshuacarp/go/src/github.com/segmentio/chamber/store/ssmstore.go:41 +0x2f
github.com/segmentio/chamber/cmd.getSecretStore(0x7ffeefbff435, 0x3)
        /Users/joshuacarp/go/src/github.com/segmentio/chamber/cmd/root.go:86 +0x8c
github.com/segmentio/chamber/cmd.list(0x1c48a20, 0xc000091710, 0x1, 0x1, 0x0, 0x0)
        /Users/joshuacarp/go/src/github.com/segmentio/chamber/cmd/list.go:36 +0xcc
github.com/spf13/cobra.(*Command).execute(0x1c48a20, 0xc0000916e0, 0x1, 0x1, 0x1c48a20, 0xc0000916e0)
        /Users/joshuacarp/go/src/github.com/spf13/cobra/command.go:762 +0x473
github.com/spf13/cobra.(*Command).ExecuteC(0x1c48ee0, 0xc0001c5f00, 0xc0001c5f88, 0x10071b0)
        /Users/joshuacarp/go/src/github.com/spf13/cobra/command.go:852 +0x2fd
github.com/segmentio/chamber/cmd.Execute(0x16df004, 0x3)
        /Users/joshuacarp/go/src/github.com/segmentio/chamber/cmd/root.go:54 +0x56
main.main()
        /Users/joshuacarp/go/src/github.com/segmentio/chamber/main.go:11 +0x39

After:

~/g/s/g/s/chamber 🤜 chamber list foo                                                         ✘ 2 fix-panic-bad-aws-session ✭ ◼
Error: Failed to get secret store: AssumeRoleTokenProviderNotSetError: assume role with MFA enabled, but AssumeRoleTokenProvider session option not set.

Fixes panic on bad aws configuration.
@jmcarp
Copy link
Contributor Author

jmcarp commented Sep 21, 2018

cc @nickatsegment @systemizer

@@ -71,16 +74,14 @@ func NewS3Store(numRetries int) *S3Store {

bucket, ok := os.LookupEnv(BucketEnvVar)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but I find non-main packages looking at the env to be very distasteful :P

@@ -71,16 +74,14 @@ func NewS3Store(numRetries int) *S3Store {

bucket, ok := os.LookupEnv(BucketEnvVar)
if !ok {
fmt.Fprintf(os.Stderr, "Must set %s for s3 backend\n", BucketEnvVar)
os.Exit(1)
return nil, fmt.Errorf("Must set %s for s3 backend", BucketEnvVar)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 getting rid of an os.Exit in a non-main package

@nickatsegment nickatsegment merged commit 2fd07a7 into segmentio:master Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants