From 72faa5566ac7e9f0740b7d62ec0e102f6f07cee7 Mon Sep 17 00:00:00 2001 From: Karolis Rusenas Date: Sun, 29 Apr 2018 19:47:33 +0100 Subject: [PATCH 1/2] cache for credentials --- extension/credentialshelper/aws/aws.go | 16 +++- extension/credentialshelper/aws/cache.go | 79 +++++++++++++++++++ extension/credentialshelper/aws/cache_test.go | 59 ++++++++++++++ 3 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 extension/credentialshelper/aws/cache.go create mode 100644 extension/credentialshelper/aws/cache_test.go diff --git a/extension/credentialshelper/aws/aws.go b/extension/credentialshelper/aws/aws.go index cc5f2e42a..20fc353e6 100644 --- a/extension/credentialshelper/aws/aws.go +++ b/extension/credentialshelper/aws/aws.go @@ -6,6 +6,7 @@ import ( "net/url" "os" "strings" + "time" // "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws" @@ -32,6 +33,7 @@ func init() { type CredentialsHelper struct { enabled bool region string + cache *Cache } // New creates a new instance of aws credentials helper @@ -48,6 +50,7 @@ func New() *CredentialsHelper { ch.enabled = true log.Infof("extension.credentialshelper.aws: enabled") ch.region = region + ch.cache = NewCache(2 * time.Hour) } // if os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "" && os.Getenv("AWS_REGION") != "" { @@ -70,6 +73,11 @@ func (h *CredentialsHelper) GetCredentials(image *types.TrackedImage) (*types.Cr return nil, credentialshelper.ErrUnsupportedRegistry } + cached, err := h.cache.Get(registry) + if err == nil { + return cached, nil + } + svc := ecr.New(session.New(), &aws.Config{ Region: aws.String(h.region), }) @@ -116,10 +124,14 @@ func (h *CredentialsHelper) GetCredentials(image *types.TrackedImage) (*types.Cr return nil, fmt.Errorf("failed to decode authentication token: %s, error: %s", *ad.AuthorizationToken, err) } - return &types.Credentials{ + creds := &types.Credentials{ Username: username, Password: password, - }, nil + } + + h.cache.Put(registry, creds) + + return creds, nil } } diff --git a/extension/credentialshelper/aws/cache.go b/extension/credentialshelper/aws/cache.go new file mode 100644 index 000000000..ce181dc74 --- /dev/null +++ b/extension/credentialshelper/aws/cache.go @@ -0,0 +1,79 @@ +package aws + +import ( + "fmt" + "sync" + "time" + + "github.com/keel-hq/keel/types" +) + +type item struct { + credentials *types.Credentials + created time.Time +} + +// Cache - internal cache for aws +type Cache struct { + creds map[string]*item + tick time.Duration + ttl time.Duration + mu *sync.RWMutex +} + +// NewCache - new credentials cache +func NewCache(ttl time.Duration) (c *Cache) { + c = &Cache{ + creds: make(map[string]*item), + mu: &sync.RWMutex{}, + ttl: ttl, + tick: 30 * time.Second, + } + go c.expiryService() + return +} + +func (c *Cache) expiryService() { + ticker := time.NewTicker(c.tick) + defer ticker.Stop() + for { + select { + case <-ticker.C: + c.expire() + } + } +} + +func (c *Cache) expire() { + c.mu.Lock() + t := time.Now() + for k, v := range c.creds { + if t.Sub(v.created) > c.ttl { + delete(c.creds, k) + } + } + c.mu.Unlock() +} + +// Put - saves new creds +func (c *Cache) Put(registry string, creds *types.Credentials) { + c.mu.Lock() + defer c.mu.Unlock() + c.creds[registry] = &item{credentials: creds, created: time.Now()} +} + +// Get - retrieves creds +func (c *Cache) Get(registry string) (*types.Credentials, error) { + c.mu.RLock() + defer c.mu.RUnlock() + + item, ok := c.creds[registry] + if !ok { + return nil, fmt.Errorf("not found") + } + + cr := new(types.Credentials) + *cr = *item.credentials + + return cr, nil +} diff --git a/extension/credentialshelper/aws/cache_test.go b/extension/credentialshelper/aws/cache_test.go new file mode 100644 index 000000000..cae3b175d --- /dev/null +++ b/extension/credentialshelper/aws/cache_test.go @@ -0,0 +1,59 @@ +package aws + +import ( + "sync" + "time" + + "github.com/keel-hq/keel/types" + + "testing" +) + +func TestPutCreds(t *testing.T) { + c := NewCache(time.Second * 5) + + creds := &types.Credentials{ + Username: "user-1", + Password: "pass-1", + } + + c.Put("reg1", creds) + + stored, err := c.Get("reg1") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if stored.Username != "user-1" { + t.Errorf("username mismatch: %s", stored.Username) + } + if stored.Password != "pass-1" { + t.Errorf("password mismatch: %s", stored.Password) + } +} + +func TestExpiry(t *testing.T) { + c := &Cache{ + creds: make(map[string]*item), + mu: &sync.RWMutex{}, + ttl: time.Millisecond * 500, + tick: time.Millisecond * 100, + } + + go c.expiryService() + + creds := &types.Credentials{ + Username: "user-1", + Password: "pass-1", + } + + c.Put("reg1", creds) + + time.Sleep(1100 * time.Millisecond) + + _, err := c.Get("reg1") + if err == nil { + t.Fatalf("expected to get an error about missing record") + } + +} From 3d97b2ed902bcacacb49dcf53920fb18f37c1725 Mon Sep 17 00:00:00 2001 From: Karolis Rusenas Date: Sun, 29 Apr 2018 22:00:31 +0100 Subject: [PATCH 2/2] checking token during init --- extension/credentialshelper/aws/aws.go | 22 +++++++++++++++------ extension/credentialshelper/aws/aws_test.go | 17 ++++++++++++++++ extension/credentialshelper/aws/cache.go | 10 +++++----- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/extension/credentialshelper/aws/aws.go b/extension/credentialshelper/aws/aws.go index 20fc353e6..325efc309 100644 --- a/extension/credentialshelper/aws/aws.go +++ b/extension/credentialshelper/aws/aws.go @@ -20,6 +20,11 @@ import ( log "github.com/sirupsen/logrus" ) +// AWSCredentialsExpiry specifies how long can we keep cached AWS credentials. +// This is required to reduce chance of hiting rate limits, +// more info here: https://docs.aws.amazon.com/AmazonECR/latest/userguide/service_limits.html +const AWSCredentialsExpiry = 2 * time.Hour + func init() { credentialshelper.RegisterCredentialsHelper("aws", New()) } @@ -45,17 +50,18 @@ func New() *CredentialsHelper { Region: aws.String(region), }) - _, err := svc.ListImages(&ecr.ListImagesInput{}) - if err == nil { + _, err := svc.GetAuthorizationToken(&ecr.GetAuthorizationTokenInput{}) + if err != nil { + if os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "" { + log.WithError(err).Error("extension.credentialshelper.aws: environment variables are set but initiliasation failed") + } + } else { ch.enabled = true log.Infof("extension.credentialshelper.aws: enabled") ch.region = region - ch.cache = NewCache(2 * time.Hour) + ch.cache = NewCache(AWSCredentialsExpiry) } - // if os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "" && os.Getenv("AWS_REGION") != "" { - // } - return ch } @@ -67,6 +73,10 @@ func (h *CredentialsHelper) IsEnabled() bool { // GetCredentials - finds credentials func (h *CredentialsHelper) GetCredentials(image *types.TrackedImage) (*types.Credentials, error) { + if !h.enabled { + return nil, fmt.Errorf("not initialised") + } + registry := image.Image.Registry() if !strings.Contains(registry, "amazonaws.com") { diff --git a/extension/credentialshelper/aws/aws_test.go b/extension/credentialshelper/aws/aws_test.go index b6519a435..1a47239cf 100644 --- a/extension/credentialshelper/aws/aws_test.go +++ b/extension/credentialshelper/aws/aws_test.go @@ -45,3 +45,20 @@ func TestAWS(t *testing.T) { t.Errorf("unexpected digest: %s", currentDigest) } } + +func TestCredentialsCaching(t *testing.T) { + + if os.Getenv("AWS_ACCESS_KEY_ID") == "" { + t.Skip() + } + ch := New() + imgRef, _ := image.Parse("528670773427.dkr.ecr.us-east-2.amazonaws.com/webhook-demo:master") + for i := 0; i < 200; i++ { + _, err := ch.GetCredentials(&types.TrackedImage{ + Image: imgRef, + }) + if err != nil { + t.Fatalf("cred helper got error: %s", err) + } + } +} diff --git a/extension/credentialshelper/aws/cache.go b/extension/credentialshelper/aws/cache.go index ce181dc74..960208b36 100644 --- a/extension/credentialshelper/aws/cache.go +++ b/extension/credentialshelper/aws/cache.go @@ -22,15 +22,15 @@ type Cache struct { } // NewCache - new credentials cache -func NewCache(ttl time.Duration) (c *Cache) { - c = &Cache{ +func NewCache(ttl time.Duration) *Cache { + c := &Cache{ creds: make(map[string]*item), - mu: &sync.RWMutex{}, ttl: ttl, tick: 30 * time.Second, + mu: &sync.RWMutex{}, } go c.expiryService() - return + return c } func (c *Cache) expiryService() { @@ -46,13 +46,13 @@ func (c *Cache) expiryService() { func (c *Cache) expire() { c.mu.Lock() + defer c.mu.Unlock() t := time.Now() for k, v := range c.creds { if t.Sub(v.created) > c.ttl { delete(c.creds, k) } } - c.mu.Unlock() } // Put - saves new creds