From 86d03df7797f4103127132b69565dcaf0e6478e3 Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Wed, 16 Oct 2019 09:37:03 +0100 Subject: [PATCH] backend/ssm: minor cleanups (#76) Avoid the opaque-looking generated mock code and apply some other minor cleanups. --- .travis.yml | 2 +- backend/ssm/ssm.go | 44 ++++---- backend/ssm/ssm_test.go | 230 ++++++++++++++++++---------------------- go.mod | 5 +- go.sum | 6 ++ 5 files changed, 136 insertions(+), 151 deletions(-) diff --git a/.travis.yml b/.travis.yml index 61e4f1f..539c0c8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,4 +30,4 @@ matrix: before_script: - docker run -d -p 2379:2379 quay.io/coreos/etcd /usr/local/bin/etcd -advertise-client-urls http://0.0.0.0:2379 -listen-client-urls http://0.0.0.0:2379 - docker run -d -p 8500:8500 --name consul consul - - docker run -d -p 8200:8200 --cap-add=IPC_LOCK -e 'VAULT_DEV_ROOT_TOKEN_ID=root' -e 'VAULT_DEV_LISTEN_ADDRESS=0.0.0.0:8200' vault:0.9.6 \ No newline at end of file + - docker run -d -p 8200:8200 --cap-add=IPC_LOCK -e 'VAULT_DEV_ROOT_TOKEN_ID=root' -e 'VAULT_DEV_LISTEN_ADDRESS=0.0.0.0:8200' vault:0.9.6 diff --git a/backend/ssm/ssm.go b/backend/ssm/ssm.go index c2d1ae4..4ad7de8 100644 --- a/backend/ssm/ssm.go +++ b/backend/ssm/ssm.go @@ -6,20 +6,24 @@ import ( "github.com/aws/aws-sdk-go/service/ssm" "github.com/aws/aws-sdk-go/service/ssm/ssmiface" + "github.com/heetch/confita/backend" ) -type Backend struct { +type ssmBackend struct { client ssmiface.SSMAPI ssmPath string cache map[string][]byte } -func NewBackend(ssm ssmiface.SSMAPI, path string) *Backend { - return &Backend{client: ssm, ssmPath: path} +// NewBackend returns a backend instance that uses the given SSMAPI implementation +// to retrieve keys from the parameter store at the given path. +func NewBackend(ssm ssmiface.SSMAPI, path string) backend.Backend { + return &ssmBackend{client: ssm, ssmPath: path} } -func (b *Backend) Get(ctx context.Context, key string) ([]byte, error) { +// Get implements backend.Backend.Get by fetching the key from SSM params. +func (b *ssmBackend) Get(ctx context.Context, key string) ([]byte, error) { if b.cache == nil { err := b.fetchParams(ctx) if err != nil { @@ -30,11 +34,12 @@ func (b *Backend) Get(ctx context.Context, key string) ([]byte, error) { return b.fromCache(ctx, key) } -func (b *Backend) Name() string { +// Name implements backend.Backend.Name. +func (b *ssmBackend) Name() string { return "ssm" } -func (b *Backend) fetchParams(ctx context.Context) error { +func (b *ssmBackend) fetchParams(ctx context.Context) error { b.cache = make(map[string][]byte) ssmInput := &ssm.GetParametersByPathInput{ @@ -43,40 +48,33 @@ func (b *Backend) fetchParams(ctx context.Context) error { WithDecryption: newBool(true), MaxResults: newInt64(10), } - for { res, err := b.client.GetParametersByPathWithContext(ctx, ssmInput) if err != nil { return err } - for _, p := range res.Parameters { - if p.Name != nil && p.Value != nil { - path := strings.Split(*p.Name, "/") - key := path[len(path)-1] - if key != "" { - b.cache[key] = []byte(*p.Value) - } + if p.Name == nil || p.Value == nil { + continue + } + path := strings.Split(*p.Name, "/") + if key := path[len(path)-1]; key != "" { + b.cache[key] = []byte(*p.Value) } } - if res.NextToken == nil { break } - ssmInput.NextToken = res.NextToken } - return nil } -func (b *Backend) fromCache(ctx context.Context, key string) ([]byte, error) { - v, ok := b.cache[key] - if !ok { - return nil, backend.ErrNotFound +func (b *ssmBackend) fromCache(ctx context.Context, key string) ([]byte, error) { + if v, ok := b.cache[key]; ok { + return v, nil } - - return v, nil + return nil, backend.ErrNotFound } func newBool(b bool) *bool { diff --git a/backend/ssm/ssm_test.go b/backend/ssm/ssm_test.go index 7bd09cf..261e217 100644 --- a/backend/ssm/ssm_test.go +++ b/backend/ssm/ssm_test.go @@ -5,162 +5,111 @@ import ( "fmt" "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/service/ssm" "github.com/aws/aws-sdk-go/service/ssm/ssmiface" "github.com/heetch/confita/backend" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -type mockSSM struct { - mock.Mock - ssmiface.SSMAPI -} - -func (_m *mockSSM) GetParametersByPathWithContext(_a0 context.Context, _a1 *ssm.GetParametersByPathInput, _a2 ...request.Option) (*ssm.GetParametersByPathOutput, error) { - _va := make([]interface{}, len(_a2)) - for _i := range _a2 { - _va[_i] = _a2[_i] - } - var _ca []interface{} - _ca = append(_ca, _a0, _a1) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 *ssm.GetParametersByPathOutput - if rf, ok := ret.Get(0).(func(context.Context, *ssm.GetParametersByPathInput, ...request.Option) *ssm.GetParametersByPathOutput); ok { - r0 = rf(_a0, _a1, _a2...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*ssm.GetParametersByPathOutput) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, *ssm.GetParametersByPathInput, ...request.Option) error); ok { - r1 = rf(_a0, _a1, _a2...) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - func TestAWSError(t *testing.T) { - client := new(mockSSM) - ssmOpts := getSSMOpts("/borked/") - ctx := context.Background() - expected := fmt.Errorf("aws down") - client.On("GetParametersByPathWithContext", ctx, ssmOpts).Return( - nil, expected) - + client := newFakeSSM(t, "/borked/", []getParamsRequest{{ + resultErr: fmt.Errorf("aws down"), + }}) b := NewBackend(client, "/borked/") - _, actual := b.Get(context.Background(), "some_key") - require.Equal(t, expected, actual) + _, err := b.Get(context.Background(), "some_key") + require.Error(t, err) + require.Contains(t, err.Error(), "aws down") } func TestNilNameAndValue(t *testing.T) { - client := new(mockSSM) - ssmOpts := getSSMOpts("/sup/") - ctx := context.Background() - - client.On("GetParametersByPathWithContext", ctx, ssmOpts).Return(&ssm.GetParametersByPathOutput{ - Parameters: []*ssm.Parameter{ - { + // nil names and values in the response are ignored. + client := newFakeSSM(t, "/borked/", []getParamsRequest{{ + result: &ssm.GetParametersByPathOutput{ + Parameters: []*ssm.Parameter{{ Name: nil, + Value: newString("ignorevalue"), + }, { + Name: newString("ignorename"), Value: nil, - }, - { - Name: ptrString("/sup/key"), - Value: nil, - }, + }, { + Name: newString("ignoreboth"), + Value: newString("ignoreboth"), + }, { + Name: newString("/sup/key"), + Value: newString("hello"), + }}, }, - }, nil) - - b := NewBackend(client, "/sup/") - - _, actual := b.Get(context.Background(), "key") - require.Equal(t, backend.ErrNotFound, actual) + }}) + b := NewBackend(client, "/borked/") + val, err := b.Get(context.Background(), "key") + require.NoError(t, err) + require.Equal(t, "hello", string(val)) } func TestEmptyKey(t *testing.T) { - client := new(mockSSM) - ssmOpts := getSSMOpts("/sup/") - ctx := context.Background() - - client.On("GetParametersByPathWithContext", ctx, ssmOpts).Return(&ssm.GetParametersByPathOutput{ - Parameters: []*ssm.Parameter{ - { - Name: ptrString("/sup/"), - Value: ptrString("a value"), - }, + client := newFakeSSM(t, "/borked/", []getParamsRequest{{ + result: &ssm.GetParametersByPathOutput{ + Parameters: []*ssm.Parameter{{ + Name: newString("/sup/key"), + Value: newString("hello"), + }}, }, - }, nil) - - b := NewBackend(client, "/sup/") - - _, actual := b.Get(context.Background(), "") - require.Equal(t, backend.ErrNotFound, actual) + }}) + b := NewBackend(client, "/borked/") + val, err := b.Get(context.Background(), "") + require.Equal(t, backend.ErrNotFound, err) + require.Equal(t, "", string(val)) } func TestKeyNotFound(t *testing.T) { - client := new(mockSSM) - ssmOpts := getSSMOpts("/whatevs/") - ctx := context.Background() - client.On("GetParametersByPathWithContext", ctx, ssmOpts).Return( - &ssm.GetParametersByPathOutput{}, nil) - + client := newFakeSSM(t, "/whatevs/", []getParamsRequest{{ + result: &ssm.GetParametersByPathOutput{}, + }}) b := NewBackend(client, "/whatevs/") - _, actual := b.Get(context.Background(), "some_key") - require.Equal(t, backend.ErrNotFound, actual) -} - -func ptrString(str string) *string { - return &str + val, err := b.Get(context.Background(), "some_key") + require.Equal(t, backend.ErrNotFound, err) + require.Equal(t, "", string(val)) } func TestKeysFound(t *testing.T) { - client := new(mockSSM) - ctx := context.Background() - ssmOpts := getSSMOpts("/yo/whatup/") - client.On("GetParametersByPathWithContext", ctx, ssmOpts).Return( - &ssm.GetParametersByPathOutput{ + client := newFakeSSM(t, "/yo/whatup/", []getParamsRequest{{ + result: &ssm.GetParametersByPathOutput{ Parameters: []*ssm.Parameter{ - {Name: ptrString("/yo/whatup/a_key"), Value: ptrString("wow")}, - {Name: ptrString("/yo/whatup/some_key"), Value: ptrString("wondrous")}, + {Name: newString("/yo/whatup/a_key"), Value: newString("wow")}, + {Name: newString("/yo/whatup/some_key"), Value: newString("wondrous")}, }, - }, nil) - + }, + }}) b := NewBackend(client, "/yo/whatup/") - actual, err := b.Get(context.Background(), "a_key") + val, err := b.Get(context.Background(), "a_key") require.Nil(t, err) - require.Equal(t, "wow", string(actual)) - actual, err = b.Get(context.Background(), "some_key") + require.Equal(t, "wow", string(val)) + val, err = b.Get(context.Background(), "some_key") require.Nil(t, err) - require.Equal(t, "wondrous", string(actual)) + require.Equal(t, "wondrous", string(val)) } func TestSSMPagedCall(t *testing.T) { - client := new(mockSSM) - ctx := context.Background() - firstOpts := getSSMOpts("/a/path/") - client.On("GetParametersByPathWithContext", ctx, firstOpts).Return( - &ssm.GetParametersByPathOutput{ - Parameters: []*ssm.Parameter{}, - NextToken: ptrString("/a/path/your_key"), - }, nil) - - secondOpts := getSSMOpts("/a/path/") - secondOpts.NextToken = ptrString("/a/path/your_key") - client.On("GetParametersByPathWithContext", ctx, secondOpts).Return( - &ssm.GetParametersByPathOutput{ + client := newFakeSSM(t, "/a/path/", []getParamsRequest{{ + result: &ssm.GetParametersByPathOutput{ + Parameters: []*ssm.Parameter{ + {Name: newString("/yo/whatup/a_key"), Value: newString("wow")}, + {Name: newString("/yo/whatup/some_key"), Value: newString("wondrous")}, + }, + NextToken: newString("/a/path/your_key"), + }, + }, { + expectToken: newString("/a/path/your_key"), + result: &ssm.GetParametersByPathOutput{ Parameters: []*ssm.Parameter{ - {Name: ptrString("/a/path/your_key"), Value: ptrString("shazam")}, - {Name: ptrString("/a/path/another_key"), Value: ptrString("kazam")}, + {Name: newString("/a/path/your_key"), Value: newString("shazam")}, + {Name: newString("/a/path/another_key"), Value: newString("kazam")}, }, NextToken: nil, - }, nil) + }, + }}) b := NewBackend(client, "/a/path/") actual, err := b.Get(context.Background(), "your_key") @@ -171,11 +120,42 @@ func TestSSMPagedCall(t *testing.T) { require.Equal(t, "kazam", string(actual)) } -func getSSMOpts(path string) *ssm.GetParametersByPathInput { - return &ssm.GetParametersByPathInput{ - Path: &path, - Recursive: newBool(true), - WithDecryption: newBool(true), - MaxResults: newInt64(10), +type getParamsRequest struct { + expectToken *string + result *ssm.GetParametersByPathOutput + resultErr error +} + +type fakeSSM struct { + ssmiface.SSMAPI + t *testing.T + // We always expect the backend to use the same path. + expectPath string + // The sequence of calls we're expecting. + calls []getParamsRequest + // The index of the next expected call. + call int +} + +func newFakeSSM(t *testing.T, path string, calls []getParamsRequest) *fakeSSM { + return &fakeSSM{ + t: t, + expectPath: path, + calls: calls, } } + +func (f *fakeSSM) GetParametersByPathWithContext(ctx aws.Context, arg *ssm.GetParametersByPathInput, opts ...request.Option) (*ssm.GetParametersByPathOutput, error) { + if f.call >= len(f.calls) { + f.t.Errorf("too many calls to SSM (expected max of %d)", len(f.calls)) + } + call := f.calls[f.call] + f.call++ + require.Equal(f.t, newString(f.expectPath), arg.Path) + require.Equal(f.t, call.expectToken, arg.NextToken) + return call.result, call.resultErr +} + +func newString(str string) *string { + return &str +} diff --git a/go.mod b/go.mod index 0f8da47..8c29fc6 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/duosecurity/duo_api_golang v0.0.0-20190308151101-6c680f768e74 // indirect github.com/dustin/go-humanize v1.0.0 // indirect github.com/fatih/structs v1.0.0 // indirect + github.com/frankban/quicktest v1.5.0 github.com/go-sql-driver/mysql v1.4.1 // indirect github.com/gocql/gocql v0.0.0-20190423091413-b99afaf3b163 // indirect github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect @@ -67,7 +68,7 @@ require ( github.com/ryanuber/go-glob v0.0.0-20160226084822-572520ed46db // indirect github.com/sethgrid/pester v0.0.0-20180227223404-ed9870dad317 // indirect github.com/soheilhy/cmux v0.1.4 // indirect - github.com/stretchr/testify v1.3.0 + github.com/stretchr/testify v1.4.0 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect github.com/ugorji/go v1.1.4 // indirect github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 // indirect @@ -80,7 +81,7 @@ require ( gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce // indirect gopkg.in/ory-am/dockertest.v3 v3.3.4 // indirect gopkg.in/yaml.v2 v2.2.2 - gotest.tools v2.2.0+incompatible // indirect + gotest.tools v2.2.0+incompatible ) go 1.12 diff --git a/go.sum b/go.sum index 9894951..7bf60b1 100644 --- a/go.sum +++ b/go.sum @@ -79,6 +79,8 @@ github.com/eapache/queue v1.1.0/go.mod h1:6eCeP0CKFpHLu8blIFXhExK/dRa7WDZfr6jVFP github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/structs v1.0.0 h1:BrX964Rv5uQ3wwS+KRUAJCBBw5PQmgJfJ6v4yly5QwU= github.com/fatih/structs v1.0.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= +github.com/frankban/quicktest v1.5.0 h1:Tb4jWdSpdjKzTUicPnY61PZxKbDoGa7ABbrReT3gQVY= +github.com/frankban/quicktest v1.5.0/go.mod h1:jaStnuzAqU1AJdCO0l53JDCJrVDKcS03DbaAcR7Ks/o= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= @@ -109,6 +111,8 @@ github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= +github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg= +github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-github v17.0.0+incompatible h1:N0LgJ1j65A7kfXrZnUDaYCs/Sf4rEjNlfyDHW9dolSY= github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ= github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk= @@ -297,6 +301,8 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 h1:LnC5Kc/wtumK+WB441p7ynQJzVuNRJiqddSIE3IlSEQ= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=