Skip to content

Commit

Permalink
Fix account pagination and its api functional test
Browse files Browse the repository at this point in the history
  • Loading branch information
jayanandagit committed Nov 17, 2020
1 parent 2deed97 commit 7f7ce8e
Show file tree
Hide file tree
Showing 11 changed files with 185 additions and 22 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## v0.33.1
- Fix populate reset queue when dynamodb returns paginated result
- Add account status to last evaluated key when querying account table using global secondary index
- Fix gosec issue related to G601 (CWE-118): Implicit memory aliasing in for loop

## v0.33.0
- Upgrade aws-nuke to v1.3.0

Expand Down
7 changes: 6 additions & 1 deletion cmd/lambda/accounts/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestGetAccounts(t *testing.T) {
retAccounts *account.Accounts
retErr error
nextID *string
status *account.Status
}{
{
name: "get all accounts",
Expand All @@ -53,7 +54,8 @@ func TestGetAccounts(t *testing.T) {
},
},
nextID: ptrString("234567890123"),
expLink: "<https://example.com/unit/accounts?limit=1&nextId=234567890123>; rel=\"next\"",
status: account.StatusNotReady.StatusPtr(),
expLink: "<https://example.com/unit/accounts?limit=1&nextId=234567890123&status=NotReady>; rel=\"next\"",
retErr: nil,
},
{
Expand Down Expand Up @@ -81,6 +83,7 @@ func TestGetAccounts(t *testing.T) {

values := url.Values{}
err := schema.NewEncoder().Encode(tt.query, values)

assert.Nil(t, err)

r.URL.RawQuery = values.Encode()
Expand All @@ -94,6 +97,8 @@ func TestGetAccounts(t *testing.T) {
if (input.ID != nil && tt.query.ID != nil && *input.ID == *tt.query.ID) || input.ID == tt.query.ID {
if tt.nextID != nil {
input.NextID = tt.nextID
input.Status = tt.status

input.Limit = ptr64(1)
}
return true
Expand Down
3 changes: 2 additions & 1 deletion cmd/lambda/fan_out_update_lease_status/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ func handler(cloudWatchEvent events.CloudWatchEvent) error {
err = services.LeaseService().ListPages(query,
func(leases *lease.Leases) bool {
for _, ls := range *leases {
leaseJSON, err := json.Marshal(&ls)
l := ls
leaseJSON, err := json.Marshal(&l)
// save any errors to handle later
if err != nil {
errs = append(errs, err)
Expand Down
3 changes: 2 additions & 1 deletion cmd/lambda/populate_reset_queue/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ func Handler(cloudWatchEvent events.CloudWatchEvent) error {
func(accts *account.Accounts) bool {

for _, acct := range *accts {
a := acct
// Send Message
err := api.AccountReset(&acct)
err := api.AccountReset(&a)
if err != nil {
errs = append(errs, err)
}
Expand Down
14 changes: 10 additions & 4 deletions cmd/lambda/populate_reset_queue/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func TestPopulateResetQeue(t *testing.T) {
listAccounts *account.Accounts
listErr error
alertErr error
nextID *string
nextId *string
status *account.Status
}{
{
name: "should send accounts to reset queue",
Expand All @@ -54,7 +55,8 @@ func TestPopulateResetQeue(t *testing.T) {
PrincipalRoleArn: arn.New("aws", "iam", "", "123456789012", "role/AdminRole"),
},
},
nextID: ptrString("123456789013"),
nextId: ptrString("123456789013"),
status: account.StatusNotReady.StatusPtr(),
listErr: nil,
},
{
Expand Down Expand Up @@ -93,18 +95,22 @@ func TestPopulateResetQeue(t *testing.T) {
svcBldr := &config.ServiceBuilder{Config: cfgBldr}

mocksRwd := &mocks.ReaderWriterDeleter{}

mocksRwd.On("List", mock.MatchedBy(func(input *account.Account) bool {
if input.Status.String() == "NotReady" {
if input.NextID == nil {
input.NextID = tt.nextID
input.NextID = tt.nextId
input.Status = tt.status

return true
}
}
return false
})).Return(tt.listAccounts, tt.listErr)

mocksRwd.On("List", mock.MatchedBy(func(input *account.Account) bool {
if input.Status.String() == "NotReady" {
if input.NextID == tt.nextID {
if input.NextID == tt.nextId {
input.NextID = nil
return true
}
Expand Down
7 changes: 2 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@ go 1.15

require (
github.com/360EntSecGroup-Skylar/excelize v1.4.1
github.com/AlekSi/gocov-xml v0.0.0-20190121064608-3a14fb1c4737 // indirect
github.com/Bowery/prompt v0.0.0-20190419144237-972d0ceb96f5 // indirect
github.com/Joker/jade v1.0.0 // indirect
github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef // indirect
github.com/avast/retry-go v2.6.0+incompatible
github.com/aws/aws-lambda-go v1.19.1
github.com/aws/aws-sdk-go v1.34.20
github.com/awslabs/aws-lambda-go-api-proxy v0.8.0
github.com/axw/gocov v1.0.0 // indirect
github.com/caarlos0/env v3.5.0+incompatible
github.com/dchest/safefile v0.0.0-20151022103144-855e8d98f185 // indirect
github.com/fatih/color v1.9.0 // indirect
github.com/flosch/pongo2 v0.0.0-20190707114632-bbf5a6c351f4 // indirect
github.com/go-ozzo/ozzo-validation v3.6.0+incompatible
github.com/google/shlex v0.0.0-20181106134648-c34317bd91bf // indirect
Expand All @@ -33,16 +34,12 @@ require (
github.com/mitchellh/mapstructure v1.3.3
github.com/oleiade/reflections v1.0.0 // indirect
github.com/pkg/errors v0.9.1
github.com/pquerna/otp v1.2.0 // indirect
github.com/rebuy-de/aws-nuke v1.4.0
github.com/securego/gosec v0.0.0-20200316084457-7da9f46445fd // indirect
github.com/sirupsen/logrus v1.6.0 // indirect
github.com/spf13/cobra v1.0.0 // indirect
github.com/stretchr/objx v0.3.0 // indirect
github.com/stretchr/testify v1.6.1
golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a
golang.org/x/net v0.0.0-20200904194848-62affa334b73 // indirect
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 // indirect
golang.org/x/sys v0.0.0-20200909081042-eff7692f9009 // indirect
google.golang.org/appengine v1.6.6 // indirect
gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiy
dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
github.com/360EntSecGroup-Skylar/excelize v1.4.1 h1:l55mJb6rkkaUzOpSsgEeKYtS6/0gHwBYyfo5Jcjv/Ks=
github.com/360EntSecGroup-Skylar/excelize v1.4.1/go.mod h1:vnax29X2usfl7HHkBrX5EvSCJcmH3dT9luvxzu8iGAE=
github.com/AlekSi/gocov-xml v0.0.0-20190121064608-3a14fb1c4737 h1:JZHBkt0GhM+ARQykshqpI49yaWCHQbJonH3XpDTwMZQ=
github.com/AlekSi/gocov-xml v0.0.0-20190121064608-3a14fb1c4737/go.mod h1:w1KSuh2JgIL3nyRiZijboSUwbbxOrTzWwyWVFUHtXBQ=
github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8=
github.com/Azure/azure-sdk-for-go v35.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
github.com/Azure/azure-sdk-for-go v38.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
Expand Down Expand Up @@ -87,6 +89,8 @@ github.com/awslabs/aws-lambda-go-api-proxy v0.5.0 h1:mmzE5dJ2yt23lmWr6QNtCCAA3H0
github.com/awslabs/aws-lambda-go-api-proxy v0.5.0/go.mod h1:9ZpbR64sd0A73+ylC1tP63Kyz2VhijeDw1O8naJqehA=
github.com/awslabs/aws-lambda-go-api-proxy v0.8.0 h1:XUx+5PMwtZEIWc7oyMduXUfAhumHFU/xbSPwB2csYx0=
github.com/awslabs/aws-lambda-go-api-proxy v0.8.0/go.mod h1:V3jj7BZnRY8y2QTKSABIwBc+dTjPkX7vLxz61Id7vsQ=
github.com/axw/gocov v1.0.0 h1:YsqYR66hUmilVr23tu8USgnJIJvnwh3n7j5zRn7x4LU=
github.com/axw/gocov v1.0.0/go.mod h1:LvQpEYiwwIb2nYkXY2fDWhg9/AsYqkhmrCshjlUJECE=
github.com/aymerick/raymond v2.0.2+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=
github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
Expand Down Expand Up @@ -640,6 +644,7 @@ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down Expand Up @@ -709,6 +714,7 @@ golang.org/x/tools v0.0.0-20190506145303-2d16b83fe98c/go.mod h1:RgjU9mgBXZiqYHBn
golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190617190820-da514acc4774/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190706070813-72ffa07ba3db/go.mod h1:jcCCGcm9btYwXyDqrUWc6MKQKKGJCWEQ3AfLSRIbEuI=
Expand Down
5 changes: 5 additions & 0 deletions pkg/account/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ type Accounts []Account
// Status is an account status type
type Status string

type LastEvaluatedKey struct {
ID dynamodb.AttributeValue
AccountStatus dynamodb.AttributeValue
}

const (
// StatusNone status
StatusNone Status = "None"
Expand Down
30 changes: 26 additions & 4 deletions pkg/data/accounts.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package data

import (
"encoding/json"

"github.com/Optum/dce/pkg/account"
"github.com/Optum/dce/pkg/errors"
"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -43,12 +45,16 @@ func (a *Account) queryAccounts(query *account.Account, keyName string, index st
}

queryInput.SetLimit(*query.Limit)
if query.NextID != nil {

if query.NextID != nil && query.Status != nil {
// Should be more dynamic
queryInput.SetExclusiveStartKey(map[string]*dynamodb.AttributeValue{
"Id": &dynamodb.AttributeValue{
S: query.NextID,
},
"AccountStatus": &dynamodb.AttributeValue{
S: query.Status.StringPtr(),
},
})
}

Expand Down Expand Up @@ -89,6 +95,7 @@ func (a *Account) scanAccounts(query *account.Account) (*queryScanOutput, error)
}

scanInput.SetLimit(*query.Limit)

if query.NextID != nil {
// Should be more dynamic
scanInput.SetExclusiveStartKey(map[string]*dynamodb.AttributeValue{
Expand Down Expand Up @@ -129,9 +136,24 @@ func (a *Account) List(query *account.Account) (*account.Accounts, error) {
return nil, err
}

query.NextID = nil
for _, v := range outputs.lastEvaluatedKey {
query.NextID = v.S
if outputs.lastEvaluatedKey != nil {
jsondata, err := json.Marshal(outputs.lastEvaluatedKey)

if err != nil {
return nil, errors.NewInternalServer("failed marshaling of last evaluated key", err)
}

lastEvaluatedKey := account.LastEvaluatedKey{}

// set last evaluated key to next id for next query/scan
if err := json.Unmarshal(jsondata, &lastEvaluatedKey); err != nil {
return nil, errors.NewInternalServer("failed unmarshaling of last evaluated key to next ID", err)
}

query.NextID = lastEvaluatedKey.ID.S
} else {
// clear next id and account status if there is no more page
query.NextID = nil
}

accounts := &account.Accounts{}
Expand Down
Loading

0 comments on commit 7f7ce8e

Please sign in to comment.