From 5e51749dfbdeb38238c6bb50bbc5451a1b5476a9 Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:29:04 +1200 Subject: [PATCH 1/8] Add Token Provider Chain The provider chain allows us to define all of the places we'd like to look for a token, and then use the first one that returns a valid token to us. It also lets us decide our preference - ie, look here, then look there, then look over there, etc --- go.mod | 2 ++ go.sum | 15 ++++++++++++- token/provider_chain.go | 49 +++++++++++++++++++++++++++++++++++++++++ token/token.go | 17 +++----------- 4 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 token/provider_chain.go diff --git a/go.mod b/go.mod index b7e7b095..b0cd3433 100644 --- a/go.mod +++ b/go.mod @@ -10,8 +10,10 @@ require ( github.com/golang/mock v1.2.0 github.com/golang/protobuf v1.2.0 github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af // indirect + github.com/kr/pretty v0.3.0 // indirect github.com/newrelic/go-agent v3.0.0+incompatible github.com/prometheus/client_golang v0.9.3-0.20190127221311-3c4408c8b829 github.com/prometheus/client_model v0.0.0-20190115171406-56726106282f + github.com/stretchr/testify v1.7.1 google.golang.org/genproto v0.0.0-20190401181712-f467c93bbac2 ) diff --git a/go.sum b/go.sum index caaf38bc..6a9acc9c 100644 --- a/go.sum +++ b/go.sum @@ -25,6 +25,7 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/coreos/go-systemd v0.0.0-20181012123002-c6f51f82210d/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -77,7 +78,14 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= +github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/pty v1.1.3/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= @@ -108,14 +116,17 @@ github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1 h1:/K3IL0Z1quvmJ7X0A1AwNEK7CRkVK3YwfOU/QAL4WGg= github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= +github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= +github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= -github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/tarm/serial v0.0.0-20180830185346-98f6abe2eb07/go.mod h1:kDXzergiv9cbyO7IOYJZWg1U88JhDg3PB6klq9Hg2pA= github.com/urfave/cli/v2 v2.2.0/go.mod h1:SE9GqnLQmjVa0iPEY0f1w3ygNIYcIJ0OKPMoW2caLfQ= go.opencensus.io v0.18.0/go.mod h1:vKdFvxhtzZ9onBp9VKHK8z/sRpBMnKAsufL7wlDrCOA= @@ -199,7 +210,9 @@ google.golang.org/grpc v1.19.0 h1:cfg4PD8YEdSFnm7qLV4++93WcmhH2nIUhMjhdCvl3j8= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= diff --git a/token/provider_chain.go b/token/provider_chain.go new file mode 100644 index 00000000..ab0f2745 --- /dev/null +++ b/token/provider_chain.go @@ -0,0 +1,49 @@ +package token + +import ( + "errors" + "fmt" +) + +type ProviderChain struct { + providers []Provider +} + +func NewProviderChain(providers ...Provider) *ProviderChain { + return &ProviderChain{ + providers: providers, + } +} + +// Append adds a new token provider to the end of the chain, meaning it will be evaluated last +func (p *ProviderChain) Append(provider Provider) { + p.providers = append(p.providers, provider) +} + +// Prepend adds a new token provider to the end of the chain, meaning it will be evaluated first +func (p *ProviderChain) Prepend(provider Provider) { + newProviders := make([]Provider, 0, len(p.providers)+1) + newProviders = append(newProviders, provider) + newProviders = append(newProviders, p.providers...) + + p.providers = newProviders +} + +// Resolve iterates through the providers in the chain, looking for one that's usable, and returning the token that the first +// usable provider returns +func (p *ProviderChain) Resolve() (string, error) { + for _, provider := range p.providers { + token, err := provider.Get() + if err != nil { + if errors.Is(err, errNotUsable) { + // The provider doesn't have the necessary config to fetch a token + continue + } + return "", err + } + + return token, nil + } + + return "", fmt.Errorf("no providers in the chain %v were able to resolve a token", p.providers) +} diff --git a/token/token.go b/token/token.go index 5b26ed8c..8a563569 100644 --- a/token/token.go +++ b/token/token.go @@ -1,24 +1,13 @@ package token import ( - "fmt" - "os" + "errors" ) // Provider represents the behaviour of obtaining a Buildkite token. type Provider interface { Get() (string, error) + String() string } -// Must is a helper function to ensure a Provider object can be successfully instantiated when calling any of the -// constructor functions provided by this package. -// -// This helper is intended to be used at program startup to load the Provider implementation to be used. Such as: -// var provider := token.Must(token.NewSSMProvider()) -func Must(prov Provider, err error) Provider { - if err != nil { - _, _ = fmt.Fprintf(os.Stderr, "failed to initialize Buildkite token provider: %v", err) - os.Exit(1) - } - return prov -} +var errNotUsable = errors.New("provider not usable") From b49338507665c2ec4c9ce61f474ad17f66f0841a Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:30:59 +1200 Subject: [PATCH 2/8] Refactor `memory` token provider into `env_var`, making it usable in provider chain --- token/env_var.go | 29 +++++++++++++++++++++++++++++ token/env_var_test.go | 29 +++++++++++++++++++++++++++++ token/memory.go | 15 --------------- token/memory_test.go | 24 ------------------------ 4 files changed, 58 insertions(+), 39 deletions(-) create mode 100644 token/env_var.go create mode 100644 token/env_var_test.go delete mode 100644 token/memory.go delete mode 100644 token/memory_test.go diff --git a/token/env_var.go b/token/env_var.go new file mode 100644 index 00000000..5cd5ca3d --- /dev/null +++ b/token/env_var.go @@ -0,0 +1,29 @@ +package token + +import ( + "os" +) + +type envVarProvider struct { + // The token value to provide on each Get call. + Token string +} + +const TokenEnvVarKey = "BUILDKITE_AGENT_TOKEN" + +// NewEnvVar constructs a Buildkite API token provider backed by the environment variable BUILDKITE_AGENT_TOKEN +func NewEnvVar() Provider { + return &envVarProvider{Token: os.Getenv(TokenEnvVarKey)} +} + +func (p envVarProvider) Get() (string, error) { + if p.Token == "" { + return "", errNotUsable + } + + return p.Token, nil +} + +func (p envVarProvider) String() string { + return "EnvVarProvider" +} diff --git a/token/env_var_test.go b/token/env_var_test.go new file mode 100644 index 00000000..7a2818e1 --- /dev/null +++ b/token/env_var_test.go @@ -0,0 +1,29 @@ +package token + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +const ( + testToken = "some-token" +) + +func TestEnvVarProvider_Get(t *testing.T) { + t.Setenv(TokenEnvVarKey, testToken) + + provider := NewEnvVar() + token, err := provider.Get() + assert.NoError(t, err) + + assert.Equal(t, testToken, token) +} + +func TestEnvVarProvider_WithNoEnvVar_IsUnusable(t *testing.T) { + provider := NewEnvVar() + _, err := provider.Get() + + assert.Error(t, err) + assert.ErrorIs(t, err, errNotUsable) +} diff --git a/token/memory.go b/token/memory.go deleted file mode 100644 index 7bd34d35..00000000 --- a/token/memory.go +++ /dev/null @@ -1,15 +0,0 @@ -package token - -type inMemoryProvider struct { - // The token value to provide on each Get call. - Token string -} - -// NewInMemory constructs a Buildkite API token provider backed by a in-memory string. -func NewInMemory(token string) (Provider, error) { - return &inMemoryProvider{Token: token}, nil -} - -func (p inMemoryProvider) Get() (string, error) { - return p.Token, nil -} diff --git a/token/memory_test.go b/token/memory_test.go deleted file mode 100644 index d730fb72..00000000 --- a/token/memory_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package token - -import ( - "testing" -) - -const ( - inMemoryTestToken = "some-token" -) - -func TestInMemoryProvider_Get(t *testing.T) { - provider, err := NewInMemory(inMemoryTestToken) - if err != nil { - t.Fatalf("no errors were expected to be returned by NewInMemory but got: %v", err) - } - token, err := provider.Get() - if err != nil { - t.Fatalf("no errors were expected to be returned by InMemoryProvider.Get() but got: %v", err) - } - - if token != inMemoryTestToken { - t.Fatalf("expecting '%s' to be returned by InMemoryProvider.Get() but got: %s", inMemoryTestToken, token) - } -} From 1f5f07de7d2c2103afcd86b899513cb2e38f27a1 Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:31:35 +1200 Subject: [PATCH 3/8] Refactor secretsmanager token provider to be usable in provider chain --- token/secretsmanager.go | 62 +++++------ token/secretsmanager_test.go | 206 ++++++++++++----------------------- 2 files changed, 96 insertions(+), 172 deletions(-) diff --git a/token/secretsmanager.go b/token/secretsmanager.go index 16454901..dc31bffa 100644 --- a/token/secretsmanager.go +++ b/token/secretsmanager.go @@ -4,13 +4,16 @@ import ( "encoding/base64" "encoding/json" "fmt" + "os" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/secretsmanager" ) -// SecretsManagerOpt represents a configuration option for the AWS SecretsManager Buildkite token provider. -type SecretsManagerOpt func(opts *secretsManagerProvider) error +const ( + SecretsManagerKeyEnvVar = "BUILDKITE_AGENT_SECRETS_MANAGER_SECRET_ID" + SecretsManagerJSONKeyEnvVar = "BUILDKITE_AGENT_SECRETS_MANAGER_JSON_KEY" +) // SecretsManagerClient represents the minimal interactions required to retrieve a Buildkite API token from // AWS Secrets Manager. @@ -19,57 +22,47 @@ type SecretsManagerClient interface { } type secretsManagerProvider struct { - Client SecretsManagerClient - SecretID string - JSONKey string + Client SecretsManagerClient } -// WithSecretsManagerJSONSecret instructs SecretsManager Buidlkite token provider that the token is stored within a JSON -// payload. The key parameter specifies the JSON field holding the secret value within the JSON blob. -// -// This configuration option works for both AWS supported secret formats (SecretString and SecretBinary). However, for -// the later case, the binary payload must be a valid JSON document containing the 'key' field. -func WithSecretsManagerJSONSecret(key string) SecretsManagerOpt { - return func(provider *secretsManagerProvider) error { - provider.JSONKey = key - return nil +// NewSecretsManager constructs a Buildkite API token provider backed by AWS Secrets Manager. +func NewSecretsManager(client SecretsManagerClient) Provider { + return &secretsManagerProvider{ + Client: client, } } -// NewSecretsManager constructs a Buildkite API token provider backed by AWS Secrets Manager. -func NewSecretsManager(client SecretsManagerClient, secretID string, opts ...SecretsManagerOpt) (Provider, error) { - provider := &secretsManagerProvider{ - Client: client, - SecretID: secretID, - } +func (p secretsManagerProvider) Get() (string, error) { + secretID := os.Getenv(SecretsManagerKeyEnvVar) - for _, opt := range opts { - err := opt(provider) - if err != nil { - return nil, err - } + if secretID == "" { + return "", errNotUsable } - return provider, nil -} - -func (p secretsManagerProvider) Get() (string, error) { res, err := p.Client.GetSecretValue(&secretsmanager.GetSecretValueInput{ - SecretId: aws.String(p.SecretID), + SecretId: aws.String(secretID), }) if err != nil { - return "", fmt.Errorf("failed to retrieve secret '%s' from SecretsManager: %v", p.SecretID, err) + return "", fmt.Errorf("failed to retrieve secret '%s' from SecretsManager: %v", secretID, err) } secret, err := p.parseResponse(res) if err != nil { - return "", fmt.Errorf("failed to parse SecretsManager's response for '%s': %v", p.SecretID, err) + return "", fmt.Errorf("failed to parse SecretsManager's response for '%s': %v", secretID, err) } return secret, nil } +func (p secretsManagerProvider) IsUsable() bool { + return os.Getenv(SecretsManagerKeyEnvVar) != "" +} + +func (p secretsManagerProvider) String() string { + return "SecretsManagerProvider" +} + func (p secretsManagerProvider) parseResponse(res *secretsmanager.GetSecretValueOutput) (string, error) { var err error var secretBytes []byte @@ -83,8 +76,9 @@ func (p secretsManagerProvider) parseResponse(res *secretsmanager.GetSecretValue } } - if p.JSONKey != "" { - secret, err := extractStringKeyFromJSON(secretBytes, p.JSONKey) + jsonKey := os.Getenv(SecretsManagerJSONKeyEnvVar) + if jsonKey != "" { + secret, err := extractStringKeyFromJSON(secretBytes, jsonKey) if err != nil { return "", err } diff --git a/token/secretsmanager_test.go b/token/secretsmanager_test.go index 5db7183a..6f2d56fe 100644 --- a/token/secretsmanager_test.go +++ b/token/secretsmanager_test.go @@ -2,13 +2,13 @@ package token import ( "encoding/base64" - "fmt" "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/secretsmanager" "github.com/buildkite/buildkite-agent-metrics/token/mock" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" ) //go:generate mockgen -source secretsmanager.go -mock_names SecretsManagerClient=SecretsManagerClient -package mock -destination mock/secretsmanager_client.go @@ -20,217 +20,147 @@ const ( secretsManagerSecretJSONValue = `{"some_json_key" : "super-secret-value"}` ) -func TestSecretsManagerProvider_WithSecretsManagerJSONSecret(t *testing.T) { - provider := secretsManagerProvider{} - - err := WithSecretsManagerJSONSecret(secretsManagerSecretJSONKey)(&provider) - if err != nil { - t.Fatalf("failed to apply WithJSONSecret: %v", err) - } - - if provider.JSONKey != secretsManagerSecretJSONKey { - t.Fatalf( - "expected secretsManagerProvider.JSONKey to be %s but found %s", - secretsManagerSecretJSONKey, provider.JSONKey) - } -} - -func TestSecretsManagerProvider_New_WithErroringOpt(t *testing.T) { - expectedErr := fmt.Errorf("some-error") - - errFunc := func(provider *secretsManagerProvider) error { - return expectedErr - } - - _, err := NewSecretsManager(nil, secretsManagerSecretID, errFunc) +func TestSecretsManagerProvider_Get_WithPlainTextSecret(t *testing.T) { + t.Setenv(SecretsManagerKeyEnvVar, secretsManagerSecretID) - if err == nil { - t.Fatalf("expected error to be '%s' but found 'nil'", expectedErr.Error()) - } + ctrl := gomock.NewController(t) + defer ctrl.Finish() - if err != expectedErr { - t.Fatalf("expected error to be '%s' but found '%s'", expectedErr.Error(), err.Error()) - } -} + client := mock.NewSecretsManagerClient(ctrl) -func TestSecretsManagerProvider_Get_WithPlainTextSecret(t *testing.T) { - req := secretsmanager.GetSecretValueInput{ - SecretId: aws.String(secretsManagerSecretID), - } + req := secretsmanager.GetSecretValueInput{SecretId: aws.String(secretsManagerSecretID)} res := secretsmanager.GetSecretValueOutput{ SecretString: aws.String(secretsManagerSecretValue), SecretBinary: nil, } - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - client := mock.NewSecretsManagerClient(ctrl) client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) - provider, err := NewSecretsManager(client, secretsManagerSecretID) - if err != nil { - t.Fatalf("failed to create SecretsManagerProvider: %v", err) - } + provider := NewSecretsManager(client) token, err := provider.Get() - if err != nil { - t.Error(err) - } + assert.NoError(t, err) - if token != secretsManagerSecretValue { - t.Fatalf("expected token to be '%s' but found '%s'", secretsManagerSecretValue, token) - } + assert.Equal(t, secretsManagerSecretValue, token) } func TestSecretsManagerProvider_Get_WithBinarySecret(t *testing.T) { - req := secretsmanager.GetSecretValueInput{ - SecretId: aws.String(secretsManagerSecretID), - } - res := secretsmanager.GetSecretValueOutput{ - SecretString: nil, - SecretBinary: stringToBase64(secretsManagerSecretValue), - } + t.Setenv(SecretsManagerKeyEnvVar, secretsManagerSecretID) ctrl := gomock.NewController(t) defer ctrl.Finish() client := mock.NewSecretsManagerClient(ctrl) + req := secretsmanager.GetSecretValueInput{SecretId: aws.String(secretsManagerSecretID)} + res := secretsmanager.GetSecretValueOutput{ + SecretString: nil, + SecretBinary: stringToBase64(secretsManagerSecretValue), + } + client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) - provider, err := NewSecretsManager(client, secretsManagerSecretID) - if err != nil { - t.Fatalf("failed to create SecretsManagerProvider: %v", err) - } + provider := NewSecretsManager(client) token, err := provider.Get() - if err != nil { - t.Error(err) - } + assert.NoError(t, err) - if token != secretsManagerSecretValue { - t.Fatalf("expected token to be '%s' but found '%s'", secretsManagerSecretValue, token) - } + assert.Equal(t, secretsManagerSecretValue, token) } func TestSecretsManagerProvider_Get_WithExistingJSONKey(t *testing.T) { - req := secretsmanager.GetSecretValueInput{ - SecretId: aws.String(secretsManagerSecretID), - } - res := secretsmanager.GetSecretValueOutput{ - SecretString: aws.String(secretsManagerSecretJSONValue), - SecretBinary: nil, - } + t.Setenv(SecretsManagerKeyEnvVar, secretsManagerSecretID) + t.Setenv(SecretsManagerJSONKeyEnvVar, secretsManagerSecretJSONKey) ctrl := gomock.NewController(t) defer ctrl.Finish() client := mock.NewSecretsManagerClient(ctrl) - client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) - - provider, err := NewSecretsManager(client, secretsManagerSecretID, - WithSecretsManagerJSONSecret(secretsManagerSecretJSONKey)) - if err != nil { - t.Fatalf("failed to create SecretsManagerProvider: %v", err) + req := secretsmanager.GetSecretValueInput{SecretId: aws.String(secretsManagerSecretID)} + res := secretsmanager.GetSecretValueOutput{ + SecretString: aws.String(secretsManagerSecretJSONValue), + SecretBinary: nil, } + client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) + + provider := NewSecretsManager(client) + token, err := provider.Get() - if err != nil { - t.Error(err) - } + assert.NoError(t, err) - if token != secretsManagerSecretValue { - t.Fatalf("expected token to be '%s' but found '%s'", secretsManagerSecretValue, token) - } + assert.Equal(t, secretsManagerSecretValue, token) } func TestSecretsManagerProvider_Get_WithBinaryJSONSecret(t *testing.T) { - req := secretsmanager.GetSecretValueInput{ - SecretId: aws.String(secretsManagerSecretID), - } - res := secretsmanager.GetSecretValueOutput{ - SecretString: nil, - SecretBinary: stringToBase64(secretsManagerSecretJSONValue), - } + t.Setenv(SecretsManagerKeyEnvVar, secretsManagerSecretID) + t.Setenv(SecretsManagerJSONKeyEnvVar, secretsManagerSecretJSONKey) ctrl := gomock.NewController(t) defer ctrl.Finish() client := mock.NewSecretsManagerClient(ctrl) - client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) - - provider, err := NewSecretsManager(client, secretsManagerSecretID, - WithSecretsManagerJSONSecret(secretsManagerSecretJSONKey)) - if err != nil { - t.Fatalf("failed to create SecretsManagerProvider: %v", err) + req := secretsmanager.GetSecretValueInput{SecretId: aws.String(secretsManagerSecretID)} + res := secretsmanager.GetSecretValueOutput{ + SecretString: nil, + SecretBinary: stringToBase64(secretsManagerSecretJSONValue), } + client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) + + provider := NewSecretsManager(client) + token, err := provider.Get() - if err != nil { - t.Error(err) - } + assert.NoError(t, err) - if token != secretsManagerSecretValue { - t.Fatalf("expected token to be '%s' but found '%s'", secretsManagerSecretValue, token) - } + assert.Equal(t, secretsManagerSecretValue, token) } func TestSecretsManagerProvider_Get_WithNonJSONPayload(t *testing.T) { - req := secretsmanager.GetSecretValueInput{ - SecretId: aws.String(secretsManagerSecretID), - } - res := secretsmanager.GetSecretValueOutput{ - SecretString: aws.String("this is not a JSON payload"), - SecretBinary: nil, - } + t.Setenv(SecretsManagerKeyEnvVar, secretsManagerSecretID) + t.Setenv(SecretsManagerJSONKeyEnvVar, secretsManagerSecretJSONKey) ctrl := gomock.NewController(t) defer ctrl.Finish() client := mock.NewSecretsManagerClient(ctrl) - client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) - - provider, err := NewSecretsManager(client, secretsManagerSecretID, - WithSecretsManagerJSONSecret(secretsManagerSecretJSONKey)) - if err != nil { - t.Fatalf("failed to create SecretsManagerProvider: %v", err) + req := secretsmanager.GetSecretValueInput{SecretId: aws.String(secretsManagerSecretID)} + res := secretsmanager.GetSecretValueOutput{ + SecretString: aws.String("this is not a JSON payload"), + SecretBinary: nil, } - _, err = provider.Get() - if err == nil { - t.Fatalf("expecting error when extracting JSON key from a non-JSON payload") - } + client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) + + provider := NewSecretsManager(client) + + _, err := provider.Get() + assert.Error(t, err) } func TestSecretsManagerProvider_Get_WithNonStringValue(t *testing.T) { - req := secretsmanager.GetSecretValueInput{ - SecretId: aws.String(secretsManagerSecretID), - } - res := secretsmanager.GetSecretValueOutput{ - SecretString: aws.String(`{ "non_string_value": true }`), - SecretBinary: nil, - } + t.Setenv(SecretsManagerKeyEnvVar, secretsManagerSecretID) + t.Setenv(SecretsManagerJSONKeyEnvVar, "non_string_value") ctrl := gomock.NewController(t) defer ctrl.Finish() client := mock.NewSecretsManagerClient(ctrl) - client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) - - provider, err := NewSecretsManager(client, secretsManagerSecretID, - WithSecretsManagerJSONSecret("non_string_value")) - if err != nil { - t.Fatalf("failed to create SecretsManagerProvider: %v", err) + req := secretsmanager.GetSecretValueInput{SecretId: aws.String(secretsManagerSecretID)} + res := secretsmanager.GetSecretValueOutput{ + SecretString: aws.String(`{ "non_string_value": true }`), + SecretBinary: nil, } - _, err = provider.Get() - if err == nil { - t.Fatalf("expecting error when extracting a non-string value from JSON payload") - } + client.EXPECT().GetSecretValue(gomock.Eq(&req)).Return(&res, nil) + + provider := NewSecretsManager(client) + + _, err := provider.Get() + assert.Error(t, err) } func stringToBase64(text string) []byte { From 9fab11010d7614ca4082eba2f39146391b8bf828 Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:31:51 +1200 Subject: [PATCH 4/8] Refactor ssm token provider to be usable in provider chain --- token/ssm.go | 37 +++++++++++++++++-------------------- token/ssm_test.go | 42 +++++++++--------------------------------- 2 files changed, 26 insertions(+), 53 deletions(-) diff --git a/token/ssm.go b/token/ssm.go index 7b357f0e..859ed6b6 100644 --- a/token/ssm.go +++ b/token/ssm.go @@ -2,11 +2,14 @@ package token import ( "fmt" + "os" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ssm" ) +const SSMKeyNameEnvVar = "BUILDKITE_AGENT_TOKEN_SSM_KEY" + // SSMService represents the minimal subset of interactions required to retrieve a Buildkite API token from // AWS Systems Manager parameter store. type SSMClient interface { @@ -15,38 +18,32 @@ type SSMClient interface { type ssmProvider struct { Client SSMClient - Name string } -// SSMProviderOpt represents a configuration option for the AWS SSM Buildkite token provider. -type SSMProviderOpt func(prov *ssmProvider) error - // NewEnvironment constructs a Buildkite API token provider backed by AWS Systems Manager parameter store. -func NewSSM(client SSMClient, name string, opts ...SSMProviderOpt) (Provider, error) { - provider := &ssmProvider{ - Client: client, - Name: name, - } - - for _, opt := range opts { - err := opt(provider) - if err != nil { - return nil, err - } - } - - return provider, nil +func NewSSM(client SSMClient) Provider { + return &ssmProvider{Client: client} } func (p ssmProvider) Get() (string, error) { + keyName := os.Getenv(SSMKeyNameEnvVar) + + if keyName == "" { + return "", errNotUsable + } + output, err := p.Client.GetParameter(&ssm.GetParameterInput{ - Name: aws.String(p.Name), + Name: aws.String(keyName), WithDecryption: aws.Bool(true), }) if err != nil { - return "", fmt.Errorf("failed to retrieve Buildkite token (%s) from AWS SSM: %v", p.Name, err) + return "", fmt.Errorf("failed to retrieve Buildkite token (%s) from AWS SSM: %v", keyName, err) } return *output.Parameter.Value, nil } + +func (p ssmProvider) String() string { + return "SSMProvider" +} diff --git a/token/ssm_test.go b/token/ssm_test.go index 6f57a0e8..d4130354 100644 --- a/token/ssm_test.go +++ b/token/ssm_test.go @@ -1,13 +1,13 @@ package token import ( - "fmt" "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ssm" "github.com/buildkite/buildkite-agent-metrics/token/mock" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" ) //go:generate mockgen -source ssm.go -mock_names SSMClient=SSMClient -package mock -destination mock/ssm_client.go @@ -17,25 +17,14 @@ const ( ssmTestParameterValue = "some-value" ) -func TestSSMProvider_New_WithErroringOpt(t *testing.T) { - expectedErr := fmt.Errorf("some-error") - - errFunc := func(provider *ssmProvider) error { - return expectedErr - } - - _, err := NewSSM(nil, ssmTestParameterName, errFunc) +func TestSSMProvider_Get(t *testing.T) { + t.Setenv(SSMKeyNameEnvVar, ssmTestParameterName) - if err == nil { - t.Fatalf("expected error to be '%s' but found 'nil'", expectedErr.Error()) - } + ctrl := gomock.NewController(t) + defer ctrl.Finish() - if err != expectedErr { - t.Fatalf("expected error to be '%s' but found '%s'", expectedErr.Error(), err.Error()) - } -} + client := mock.NewSSMClient(ctrl) -func TestSSMProvider_Get(t *testing.T) { req := ssm.GetParameterInput{ Name: aws.String(ssmTestParameterName), WithDecryption: aws.Bool(true), @@ -46,24 +35,11 @@ func TestSSMProvider_Get(t *testing.T) { Value: aws.String(ssmTestParameterValue), }, } - - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - client := mock.NewSSMClient(ctrl) client.EXPECT().GetParameter(gomock.Eq(&req)).Return(&res, nil) - provider, err := NewSSM(client, ssmTestParameterName) - if err != nil { - t.Fatalf("failed to create SSMProvider: %v", err) - } - + provider := NewSSM(client) token, err := provider.Get() - if err != nil { - t.Fatalf("failed to call 'Get()' on SSMProvider: %v", err) - } + assert.NoError(t, err) - if token != ssmTestParameterValue { - t.Fatalf("expected token to be '%s' but found '%s'", ssmTestParameterValue, token) - } + assert.Equal(t, ssmTestParameterValue, token) } From 87ffa04173b7ed2a920ccdaa4b5f9b9781f0a7b2 Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:32:06 +1200 Subject: [PATCH 5/8] Add static credentials token provider --- token/static_credentials.go | 20 ++++++++++++++++++++ token/static_credentials_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 token/static_credentials.go create mode 100644 token/static_credentials_test.go diff --git a/token/static_credentials.go b/token/static_credentials.go new file mode 100644 index 00000000..5741cf54 --- /dev/null +++ b/token/static_credentials.go @@ -0,0 +1,20 @@ +package token + +type staticCredentialsProvider struct { + token *string +} + +func NewStaticCredentialsProvider(token *string) Provider { + return staticCredentialsProvider{token: token} +} + +func (p staticCredentialsProvider) Get() (string, error) { + if p.token == nil || *p.token == "" { + return "", errNotUsable + } + return *p.token, nil +} + +func (p staticCredentialsProvider) String() string { + return "StaticCredentialsProvider" +} diff --git a/token/static_credentials_test.go b/token/static_credentials_test.go new file mode 100644 index 00000000..cedb67be --- /dev/null +++ b/token/static_credentials_test.go @@ -0,0 +1,27 @@ +package token + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStaticCredentials_Get(t *testing.T) { + tok := "super secret" + + provider := NewStaticCredentialsProvider(&tok) + + token, err := provider.Get() + + assert.NoError(t, err) + assert.Equal(t, tok, token) +} + +func TestStaticCredentials_Get_WithNilPtr_ReturnsErr(t *testing.T) { + provider := NewStaticCredentialsProvider(nil) + + _, err := provider.Get() + + assert.Error(t, err) + assert.ErrorIs(t, err, errNotUsable) +} From 33711dde44ddfff21f19eb61f6bcf8660ee50c75 Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:35:42 +1200 Subject: [PATCH 6/8] Refactor lambda to use the new token provider chain --- lambda/main.go | 71 +++++--------------------------------------------- 1 file changed, 7 insertions(+), 64 deletions(-) diff --git a/lambda/main.go b/lambda/main.go index 5b19e12a..63cba946 100644 --- a/lambda/main.go +++ b/lambda/main.go @@ -11,7 +11,6 @@ import ( "time" "github.com/aws/aws-lambda-go/lambda" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/secretsmanager" "github.com/aws/aws-sdk-go/service/ssm" @@ -46,8 +45,6 @@ func main() { func Handler(ctx context.Context, evt json.RawMessage) (string, error) { var b backend.Backend - var provider token.Provider - var bkToken string var err error awsRegion := os.Getenv("AWS_REGION") @@ -69,12 +66,14 @@ func Handler(ctx context.Context, evt json.RawMessage) (string, error) { return "", nil } - provider, err = initTokenProvider(awsRegion) - if err != nil { - return "", err - } + sess := session.Must(session.NewSession()) + tokenProviderChain := token.NewProviderChain( + token.NewEnvVar(), + token.NewSecretsManager(secretsmanager.New(sess)), + token.NewSSM(ssm.New(sess)), + ) - bkToken, err = provider.Get() + bkToken, err := tokenProviderChain.Resolve() if err != nil { return "", err } @@ -147,59 +146,3 @@ func Handler(ctx context.Context, evt json.RawMessage) (string, error) { return "", nil } - -func initTokenProvider(awsRegion string) (token.Provider, error) { - mutuallyExclusiveEnvVars := []string{ - BKAgentTokenEnvVar, - BKAgentTokenSSMKeyEnvVar, - BKAgentTokenSecretsManagerSecretIDEnvVar, - } - - if err := checkMutuallyExclusiveEnvVars(mutuallyExclusiveEnvVars...); err != nil { - return nil, err - } - - if bkToken := os.Getenv(BKAgentTokenEnvVar); bkToken != "" { - return token.NewInMemory(bkToken) - } - - if ssmKey := os.Getenv(BKAgentTokenSSMKeyEnvVar); ssmKey != "" { - sess, err := session.NewSession(&aws.Config{Region: aws.String(awsRegion)}) - if err != nil { - return nil, err - } - client := ssm.New(sess) - return token.NewSSM(client, ssmKey) - } - - if secretsManagerSecretID := os.Getenv(BKAgentTokenSecretsManagerSecretIDEnvVar); secretsManagerSecretID != "" { - jsonKey := os.Getenv(BKAgentTokenSecretsManagerJSONKeyEnvVar) - sess, err := session.NewSession(&aws.Config{Region: aws.String(awsRegion)}) - if err != nil { - return nil, err - } - client := secretsmanager.New(sess) - if jsonKey == "" { - return token.NewSecretsManager(client, secretsManagerSecretID) - } else { - return token.NewSecretsManager(client, secretsManagerSecretID, token.WithSecretsManagerJSONSecret(jsonKey)) - } - } - - return nil, fmt.Errorf("failed to initialize Buildkite token provider: one of the [%s] environment variables "+ - "must be provided", strings.Join(mutuallyExclusiveEnvVars, ",")) -} - -func checkMutuallyExclusiveEnvVars(varNames ...string) error { - foundVars := make([]string, 0) - for _, varName := range varNames { - value := os.Getenv(varName) - if value != "" { - foundVars = append(foundVars, value) - } - } - if len(foundVars) > 1 { - return fmt.Errorf("the environment variables [%s] are mutually exclusive", strings.Join(foundVars, ",")) - } - return nil -} From 055987e6967b3cf0a0d81addddd84b2dedd29630 Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:55:04 +1200 Subject: [PATCH 7/8] Refactor main to use new token provider chain --- main.go | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/main.go b/main.go index e5c348de..2811c857 100644 --- a/main.go +++ b/main.go @@ -9,8 +9,12 @@ import ( "strings" "time" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/aws/aws-sdk-go/service/ssm" "github.com/buildkite/buildkite-agent-metrics/backend" "github.com/buildkite/buildkite-agent-metrics/collector" + "github.com/buildkite/buildkite-agent-metrics/token" "github.com/buildkite/buildkite-agent-metrics/version" ) @@ -18,14 +22,14 @@ var bk backend.Backend func main() { var ( - token = flag.String("token", "", "A Buildkite Agent Registration Token") - interval = flag.Duration("interval", 0, "Update metrics every interval, rather than once") - showVersion = flag.Bool("version", false, "Show the version") - quiet = flag.Bool("quiet", false, "Only print errors") - debug = flag.Bool("debug", false, "Show debug output") - debugHttp = flag.Bool("debug-http", false, "Show full http traces") - dryRun = flag.Bool("dry-run", false, "Whether to only print metrics") - endpoint = flag.String("endpoint", "https://agent.buildkite.com/v3", "A custom Buildkite Agent API endpoint") + tokenFromFlag = flag.String("token", "", "A Buildkite Agent Registration Token") + interval = flag.Duration("interval", 0, "Update metrics every interval, rather than once") + showVersion = flag.Bool("version", false, "Show the version") + quiet = flag.Bool("quiet", false, "Only print errors") + debug = flag.Bool("debug", false, "Show debug output") + debugHttp = flag.Bool("debug-http", false, "Show full http traces") + dryRun = flag.Bool("dry-run", false, "Whether to only print metrics") + endpoint = flag.String("endpoint", "https://agent.buildkite.com/v3", "A custom Buildkite Agent API endpoint") // backend config backendOpt = flag.String("backend", "cloudwatch", "Specify the backend to use: cloudwatch, statsd, prometheus, stackdriver") @@ -51,16 +55,23 @@ func main() { os.Exit(0) } - if *token == "" { - if bkToken := os.Getenv("BUILDKITE_AGENT_TOKEN"); bkToken != "" { - *token = bkToken - } else { - fmt.Println("Must provide a token") - os.Exit(1) - } + chain := token.NewProviderChain( + token.NewStaticCredentialsProvider(tokenFromFlag), + token.NewEnvVar(), + ) + + sess, err := session.NewSession() + if err == nil { + chain.Append(token.NewSSM(ssm.New(sess))) + chain.Append(token.NewSecretsManager(secretsmanager.New(sess))) + } + + token, err := chain.Resolve() + if err != nil { + fmt.Printf("Error: %v\n", err) + os.Exit(1) } - var err error switch strings.ToLower(*backendOpt) { case "cloudwatch": region := *clwRegion @@ -112,7 +123,7 @@ func main() { c := collector.Collector{ UserAgent: userAgent, Endpoint: *endpoint, - Token: *token, + Token: token, Queues: []string(queues), Quiet: *quiet, Debug: *debug, From bb2175d8810b8883c1306ba0eef5dd7e5da9e14c Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Wed, 27 Apr 2022 14:36:45 +1200 Subject: [PATCH 8/8] Add stdout metrics backend for local testing --- backend/stdout.go | 16 ++++++++++++++++ lambda/main.go | 2 ++ main.go | 2 ++ 3 files changed, 20 insertions(+) create mode 100644 backend/stdout.go diff --git a/backend/stdout.go b/backend/stdout.go new file mode 100644 index 00000000..d099ca08 --- /dev/null +++ b/backend/stdout.go @@ -0,0 +1,16 @@ +package backend + +import "github.com/buildkite/buildkite-agent-metrics/collector" + +// Stdout is a "metrics collection backend" that simply prints metrics to standard output. +// Useful for testing and local debugging +type Stdout struct{} + +func NewStdoutBackend() *Stdout { + return &Stdout{} +} + +func (s *Stdout) Collect(r *collector.Result) error { + r.Dump() + return nil +} diff --git a/lambda/main.go b/lambda/main.go index 63cba946..2edaf224 100644 --- a/lambda/main.go +++ b/lambda/main.go @@ -111,6 +111,8 @@ func Handler(ctx context.Context, evt json.RawMessage) (string, error) { fmt.Printf("Error starting New Relic client: %v\n", err) os.Exit(1) } + case "stdout": + b = backend.NewStdoutBackend() default: dimensions, err := backend.ParseCloudWatchDimensions(clwDimensions) if err != nil { diff --git a/main.go b/main.go index 2811c857..753615fd 100644 --- a/main.go +++ b/main.go @@ -106,6 +106,8 @@ func main() { fmt.Printf("Error starting New Relic client: %v\n", err) os.Exit(1) } + case "stdout": + bk = backend.NewStdoutBackend() default: fmt.Println("Must provide a supported backend: cloudwatch, statsd, prometheus, stackdriver, newrelic") os.Exit(1)