AWS Static Secrets: Requeue credential for rotation if initial attempt fails (#23673)

builtin/logical/aws/rotation.go

@@ -5,6 +5,7 @@ package aws
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -37,13 +38,13 @@ func (b *backend) rotateExpiredStaticCreds(ctx context.Context, req *logical.Req
 }
 
 // rotateCredential pops an element from the priority queue, and if it is expired, rotate and re-push.
-// If a cred was rotated, it returns true, otherwise false.
-func (b *backend) rotateCredential(ctx context.Context, storage logical.Storage) (rotated bool, err error) {
+// If a cred was ready for rotation, return true, otherwise return false.
+func (b *backend) rotateCredential(ctx context.Context, storage logical.Storage) (wasReady bool, err error) {
 	// If queue is empty or first item does not need a rotation (priority is next rotation timestamp) there is nothing to do
 	item, err := b.credRotationQueue.Pop()
 	if err != nil {
 		// the queue is just empty, which is fine.
-		if err == queue.ErrEmpty {
+		if errors.Is(err, queue.ErrEmpty) {
 			return false, nil
 		}
 		return false, fmt.Errorf("failed to pop from queue for role %q: %w", item.Key, err)
@@ -62,14 +63,21 @@ func (b *backend) rotateCredential(ctx context.Context, storage logical.Storage)
 
 	err = b.createCredential(ctx, storage, cfg, true)
 	if err != nil {
-		return false, err
+		// put it back in the queue with a backoff
+		item.Priority = time.Now().Add(10 * time.Second).Unix()
+		innerErr := b.credRotationQueue.Push(item)
+		if innerErr != nil {
+			return true, fmt.Errorf("failed to add item into the rotation queue for role %q(%w), while attempting to recover from failure to create credential: %w", cfg.Name, innerErr, err)
+		}
+		// there was one that "should have" rotated, so we want to keep looking further down the queue
+		return true, err
 	}
 
 	// set new priority and re-queue
 	item.Priority = time.Now().Add(cfg.RotationPeriod).Unix()
 	err = b.credRotationQueue.Push(item)
 	if err != nil {
-		return false, fmt.Errorf("failed to add item into the rotation queue for role %q: %w", cfg.Name, err)
+		return true, fmt.Errorf("failed to add item into the rotation queue for role %q: %w", cfg.Name, err)
 	}
 
 	return true, nil

builtin/logical/aws/rotation_test.go

@@ -349,3 +349,92 @@ func TestCreateCredential(t *testing.T) {
 		})
 	}
 }
+
+// TestRequeueOnError verifies that in the case of an error, the entry will still be in the queue for later rotation
+func TestRequeueOnError(t *testing.T) {
+	bgCTX := context.Background()
+
+	cred := staticRoleEntry{
+		Name:           "test",
+		Username:       "jane-doe",
+		RotationPeriod: 30 * time.Minute,
+	}
+
+	ak := "long-access-key-id"
+	oldSecret := "abcdefghijklmnopqrstuvwxyz"
+
+	config := logical.TestBackendConfig()
+	config.StorageView = &logical.InmemStorage{}
+
+	b := Backend(config)
+
+	// go through the process of adding a key
+	miam, err := awsutil.NewMockIAM(
+		awsutil.WithListAccessKeysOutput(&iam.ListAccessKeysOutput{
+			AccessKeyMetadata: []*iam.AccessKeyMetadata{
+				{},
+			},
+		}),
+		// initial key to store
+		awsutil.WithCreateAccessKeyOutput(&iam.CreateAccessKeyOutput{
+			AccessKey: &iam.AccessKey{
+				AccessKeyId:     aws.String(ak),
+				SecretAccessKey: aws.String(oldSecret),
+			},
+		}),
+		awsutil.WithGetUserOutput(&iam.GetUserOutput{
+			User: &iam.User{
+				UserId:   aws.String(cred.ID),
+				UserName: aws.String(cred.Username),
+			},
+		}),
+	)(nil)
+	if err != nil {
+		t.Fail()
+	}
+
+	b.iamClient = miam
+
+	err = b.createCredential(bgCTX, config.StorageView, cred, true)
+	if err != nil {
+		t.Fatalf("couldn't insert credential: %s", err)
+	}
+
+	// put the cred in the queue but age it out
+	item := &queue.Item{
+		Key:      cred.Name,
+		Value:    cred,
+		Priority: time.Now().Add(-10 * time.Minute).Unix(),
+	}
+	err = b.credRotationQueue.Push(item)
+	if err != nil {
+		t.Fatalf("couldn't push item onto queue: %s", err)
+	}
+
+	// update the mock iam with the next requests
+	miam, err = awsutil.NewMockIAM(
+		awsutil.WithGetUserError(errors.New("oh no")),
+	)(nil)
+	if err != nil {
+		t.Fatalf("couldn't initialize the mock iam: %s", err)
+	}
+	b.iamClient = miam
+
+	// now rotate, but it will fail
+	r, e := b.rotateCredential(bgCTX, config.StorageView)
+	if !r {
+		t.Fatalf("rotate credential should return true in this case, but it didn't")
+	}
+	if e == nil {
+		t.Fatalf("we expected an error when rotating a credential, but didn't get one")
+	}
+	// the queue should be updated though
+	i, e := b.credRotationQueue.PopByKey(cred.Name)
+	if err != nil {
+		t.Fatalf("queue error: %s", e)
+	}
+	delta := time.Now().Add(10*time.Second).Unix() - i.Priority
+	if delta < -5 || delta > 5 {
+		t.Fatalf("priority should be within 5 seconds of our backoff interval")
+	}
+}

changelog/23673.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+secrets/aws: fix requeueing of rotation entry in cases where rotation fails
+```