fix: add retry logic to recreate existing trustpolicy secret (#1776)

Signed-off-by: Andreea-Lupu <andreealupu1470@yahoo.com>

pkg/extensions/imagetrust/cosign.go

@@ -15,7 +15,6 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
 	"github.com/aws/aws-sdk-go-v2/service/secretsmanager/types"
-	"github.com/aws/aws-secretsmanager-caching-go/secretcache"
 	godigest "github.com/opencontainers/go-digest"
 	"github.com/sigstore/cosign/v2/pkg/cosign/pkcs11key"
 	sigs "github.com/sigstore/cosign/v2/pkg/signature"
@@ -33,8 +32,8 @@ type PublicKeyLocalStorage struct {
 }
 
 type PublicKeyAWSStorage struct {
-	secretsManagerClient *secretsmanager.Client
+	secretsManagerClient SecretsManagerClient
-	secretsManagerCache  *secretcache.Cache
+	secretsManagerCache  SecretsManagerCache
 }
 
 type publicKeyStorage interface {
@@ -64,7 +63,7 @@ func NewPublicKeyLocalStorage(rootDir string) (*PublicKeyLocalStorage, error) {
 }
 
 func NewPublicKeyAWSStorage(
-	secretsManagerClient *secretsmanager.Client, secretsManagerCache *secretcache.Cache,
+	secretsManagerClient SecretsManagerClient, secretsManagerCache SecretsManagerCache,
 ) *PublicKeyAWSStorage {
 	return &PublicKeyAWSStorage{
 		secretsManagerClient: secretsManagerClient,

pkg/extensions/imagetrust/image_trust.go

@@ -36,6 +36,19 @@ type ImageTrustStore struct {
 	NotationStorage certificateStorage
 }
 
+type SecretsManagerClient interface {
+	CreateSecret(ctx context.Context, params *secretsmanager.CreateSecretInput,
+		optFns ...func(*secretsmanager.Options)) (*secretsmanager.CreateSecretOutput, error)
+	DeleteSecret(ctx context.Context, params *secretsmanager.DeleteSecretInput,
+		optFns ...func(*secretsmanager.Options)) (*secretsmanager.DeleteSecretOutput, error)
+	ListSecrets(ctx context.Context, params *secretsmanager.ListSecretsInput,
+		optFns ...func(*secretsmanager.Options)) (*secretsmanager.ListSecretsOutput, error)
+}
+
+type SecretsManagerCache interface {
+	GetSecretString(secretID string) (string, error)
+}
+
 func NewLocalImageTrustStore(rootDir string) (*ImageTrustStore, error) {
 	publicKeyStorage, err := NewPublicKeyLocalStorage(rootDir)
 	if err != nil {

pkg/extensions/imagetrust/image_trust_test.go

@@ -17,10 +17,12 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
+	"github.com/aws/aws-sdk-go-v2/service/secretsmanager/types"
 	guuid "github.com/gofrs/uuid"
 	"github.com/notaryproject/notation-go"
 	notreg "github.com/notaryproject/notation-go/registry"
 	"github.com/notaryproject/notation-go/verifier/trustpolicy"
+	"github.com/opencontainers/go-digest"
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sigstore/cosign/v2/cmd/cosign/cli/generate"
 	"github.com/sigstore/cosign/v2/cmd/cosign/cli/options"
@@ -36,9 +38,13 @@ import (
 	extconf "zotregistry.io/zot/pkg/extensions/config"
 	"zotregistry.io/zot/pkg/extensions/imagetrust"
 	"zotregistry.io/zot/pkg/test"
+	"zotregistry.io/zot/pkg/test/mocks"
 )
 
-var errExpiryError = errors.New("expiry err")
+var (
+	errExpiryError     = errors.New("expiry err")
+	errUnexpectedError = errors.New("unexpected err")
+)
 
 func TestInitCosignAndNotationDirs(t *testing.T) {
 	Convey("InitCosignDir error", t, func() {
@@ -666,11 +672,85 @@ func TestLocalTrustStore(t *testing.T) {
 func TestAWSTrustStore(t *testing.T) {
 	skipIt(t)
 
+	trustpolicy := "trustpolicy"
+
 	Convey("NewAWSImageTrustStore error", t, func() {
 		_, err := imagetrust.NewAWSImageTrustStore("us-east-2", "wrong;endpoint")
 		So(err, ShouldNotBeNil)
 	})
 
+	Convey("InitTrustpolicy retry", t, func() {
+		smanager, err := imagetrust.GetSecretsManagerClient("us-east-2", os.Getenv("DYNAMODBMOCK_ENDPOINT"))
+		So(err, ShouldBeNil)
+
+		smCache := imagetrust.GetSecretsManagerRetrieval("us-east-2", os.Getenv("DYNAMODBMOCK_ENDPOINT"))
+
+		description := "notation trustpolicy file"
+		content := "trustpolicy content"
+
+		_, err = smanager.CreateSecret(context.Background(),
+			&secretsmanager.CreateSecretInput{
+				Name:         &trustpolicy,
+				Description:  &description,
+				SecretString: &content,
+			})
+		So(err, ShouldBeNil)
+
+		secretsManagerMock := mocks.SecretsManagerMock{
+			DeleteSecretFn: func(ctx context.Context, params *secretsmanager.DeleteSecretInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.DeleteSecretOutput, error) {
+				return &secretsmanager.DeleteSecretOutput{}, nil
+			},
+			CreateSecretFn: func(ctx context.Context, params *secretsmanager.CreateSecretInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.CreateSecretOutput, error) {
+				return smanager.CreateSecret(ctx, params, optFns...)
+			},
+		}
+
+		_, err = imagetrust.NewCertificateAWSStorage(secretsManagerMock, smCache)
+		So(err, ShouldNotBeNil)
+
+		secretsManagerMock = mocks.SecretsManagerMock{
+			DeleteSecretFn: func(ctx context.Context, params *secretsmanager.DeleteSecretInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.DeleteSecretOutput, error) {
+				return &secretsmanager.DeleteSecretOutput{}, errUnexpectedError
+			},
+			CreateSecretFn: func(ctx context.Context, params *secretsmanager.CreateSecretInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.CreateSecretOutput, error) {
+				return smanager.CreateSecret(ctx, params, optFns...)
+			},
+		}
+
+		_, err = imagetrust.NewCertificateAWSStorage(secretsManagerMock, smCache)
+		So(err, ShouldNotBeNil)
+
+		secretsManagerMock = mocks.SecretsManagerMock{
+			DeleteSecretFn: func(ctx context.Context, params *secretsmanager.DeleteSecretInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.DeleteSecretOutput, error) {
+				go func() {
+					time.Sleep(3 * time.Second)
+
+					smanager.DeleteSecret(ctx, params, optFns...) //nolint:errcheck
+				}()
+
+				return &secretsmanager.DeleteSecretOutput{}, nil
+			},
+			CreateSecretFn: func(ctx context.Context, params *secretsmanager.CreateSecretInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.CreateSecretOutput, error) {
+				return smanager.CreateSecret(ctx, params, optFns...)
+			},
+		}
+
+		_, err = imagetrust.NewCertificateAWSStorage(secretsManagerMock, smCache)
+		So(err, ShouldBeNil)
+	})
+
 	Convey("GetCertificates errors", t, func() {
 		smanager, err := imagetrust.GetSecretsManagerClient("us-east-2", os.Getenv("DYNAMODBMOCK_ENDPOINT"))
 		So(err, ShouldBeNil)
@@ -716,6 +796,40 @@ func TestAWSTrustStore(t *testing.T) {
 
 		_, err = notationStorage.GetCertificates(context.Background(), "ca", "newtest")
 		So(err, ShouldNotBeNil)
+
+		secretsManagerMock := mocks.SecretsManagerMock{
+			ListSecretsFn: func(ctx context.Context, params *secretsmanager.ListSecretsInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.ListSecretsOutput, error) {
+				return &secretsmanager.ListSecretsOutput{}, errUnexpectedError
+			},
+		}
+
+		notationStorage, err = imagetrust.NewCertificateAWSStorage(secretsManagerMock, smCache)
+		So(err, ShouldBeNil)
+
+		_, err = notationStorage.GetCertificates(context.Background(), "ca", "newtest")
+		So(err, ShouldNotBeNil)
+
+		secretsManagerMock = mocks.SecretsManagerMock{
+			ListSecretsFn: func(ctx context.Context, params *secretsmanager.ListSecretsInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.ListSecretsOutput, error) {
+				return &secretsmanager.ListSecretsOutput{SecretList: []types.SecretListEntry{{Name: &name}}}, nil
+			},
+		}
+
+		secretsManagerCacheMock := mocks.SecretsManagerCacheMock{
+			GetSecretStringFn: func(secretID string) (string, error) {
+				return "", errUnexpectedError
+			},
+		}
+
+		notationStorage, err = imagetrust.NewCertificateAWSStorage(secretsManagerMock, secretsManagerCacheMock)
+		So(err, ShouldBeNil)
+
+		_, err = notationStorage.GetCertificates(context.Background(), "ca", "newtest")
+		So(err, ShouldNotBeNil)
 	})
 
 	Convey("GetPublicKeyVerifier errors", t, func() {
@@ -760,6 +874,36 @@ func TestAWSTrustStore(t *testing.T) {
 		So(err, ShouldNotBeNil)
 	})
 
+	Convey("GetPublicKeys error", t, func() {
+		secretsManagerMock := mocks.SecretsManagerMock{
+			ListSecretsFn: func(ctx context.Context, params *secretsmanager.ListSecretsInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.ListSecretsOutput, error) {
+				return &secretsmanager.ListSecretsOutput{}, errUnexpectedError
+			},
+		}
+
+		cosignStorage := imagetrust.NewPublicKeyAWSStorage(secretsManagerMock, nil)
+
+		_, err := cosignStorage.GetPublicKeys()
+		So(err, ShouldNotBeNil)
+	})
+
+	Convey("StorePublicKeys error", t, func() {
+		secretsManagerMock := mocks.SecretsManagerMock{
+			CreateSecretFn: func(ctx context.Context, params *secretsmanager.CreateSecretInput,
+				optFns ...func(*secretsmanager.Options),
+			) (*secretsmanager.CreateSecretOutput, error) {
+				return &secretsmanager.CreateSecretOutput{}, errUnexpectedError
+			},
+		}
+
+		cosignStorage := imagetrust.NewPublicKeyAWSStorage(secretsManagerMock, nil)
+
+		err := cosignStorage.StorePublicKey(digest.FromString("dig"), []byte("content"))
+		So(err, ShouldNotBeNil)
+	})
+
 	Convey("VerifySignature - trustpolicy.json does not exist", t, func() {
 		repo := "repo"
 		image := test.CreateRandomImage()
@@ -777,12 +921,11 @@ func TestAWSTrustStore(t *testing.T) {
 		notationStorage, err := imagetrust.NewCertificateAWSStorage(smanager, smCache)
 		So(err, ShouldBeNil)
 
-		secretName := "trustpolicy"
 		force := true
 
 		_, err = smanager.DeleteSecret(context.Background(),
 			&secretsmanager.DeleteSecretInput{
-				SecretId:                   &secretName,
+				SecretId:                   &trustpolicy,
 				ForceDeleteWithoutRecovery: &force,
 			})
 		So(err, ShouldBeNil)
@@ -817,12 +960,11 @@ func TestAWSTrustStore(t *testing.T) {
 			NotationStorage: notationStorage,
 		}
 
-		secretName := "trustpolicy"
 		force := true
 
 		_, err = smanager.DeleteSecret(context.Background(),
 			&secretsmanager.DeleteSecretInput{
-				SecretId:                   &secretName,
+				SecretId:                   &trustpolicy,
 				ForceDeleteWithoutRecovery: &force,
 			})
 		So(err, ShouldBeNil)
@@ -832,7 +974,7 @@ func TestAWSTrustStore(t *testing.T) {
 
 		_, err = smanager.CreateSecret(context.Background(),
 			&secretsmanager.CreateSecretInput{
-				Name:         &secretName,
+				Name:         &trustpolicy,
 				Description:  &description,
 				SecretString: &secret,
 			})
@@ -856,7 +998,7 @@ func TestAWSTrustStore(t *testing.T) {
 
 		_, err = smanager.DeleteSecret(context.Background(),
 			&secretsmanager.DeleteSecretInput{
-				SecretId:                   &secretName,
+				SecretId:                   &trustpolicy,
 				ForceDeleteWithoutRecovery: &force,
 			})
 		So(err, ShouldBeNil)
@@ -865,7 +1007,7 @@ func TestAWSTrustStore(t *testing.T) {
 
 		_, err = smanager.CreateSecret(context.Background(),
 			&secretsmanager.CreateSecretInput{
-				Name:         &secretName,
+				Name:         &trustpolicy,
 				Description:  &description,
 				SecretString: &newSecret,
 			})
@@ -889,7 +1031,7 @@ func TestAWSTrustStore(t *testing.T) {
 
 		_, err = smanager.DeleteSecret(context.Background(),
 			&secretsmanager.DeleteSecretInput{
-				SecretId:                   &secretName,
+				SecretId:                   &trustpolicy,
 				ForceDeleteWithoutRecovery: &force,
 			})
 		So(err, ShouldBeNil)
@@ -898,7 +1040,7 @@ func TestAWSTrustStore(t *testing.T) {
 
 		_, err = smanager.CreateSecret(context.Background(),
 			&secretsmanager.CreateSecretInput{
-				Name:         &secretName,
+				Name:         &trustpolicy,
 				Description:  &description,
 				SecretString: &newSecret,
 			})

pkg/extensions/imagetrust/notation.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
 	"github.com/aws/aws-sdk-go-v2/service/secretsmanager/types"
-	"github.com/aws/aws-secretsmanager-caching-go/secretcache"
 	_ "github.com/notaryproject/notation-core-go/signature/jws"
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/dir"
@@ -45,8 +44,8 @@ type CertificateLocalStorage struct {
 }
 
 type CertificateAWSStorage struct {
-	secretsManagerClient *secretsmanager.Client
+	secretsManagerClient SecretsManagerClient
-	secretsManagerCache  *secretcache.Cache
+	secretsManagerCache  SecretsManagerCache
 }
 
 type certificateStorage interface {
@@ -99,7 +98,7 @@ func NewCertificateLocalStorage(rootDir string) (*CertificateLocalStorage, error
 }
 
 func NewCertificateAWSStorage(
-	secretsManagerClient *secretsmanager.Client, secretsManagerCache *secretcache.Cache,
+	secretsManagerClient SecretsManagerClient, secretsManagerCache SecretsManagerCache,
 ) (*CertificateAWSStorage, error) {
 	certStorage := &CertificateAWSStorage{
 		secretsManagerClient: secretsManagerClient,
@@ -185,7 +184,7 @@ func (cloud *CertificateAWSStorage) InitTrustpolicy(trustpolicy []byte) error {
 			return err
 		}
 
-		_, err = cloud.secretsManagerClient.CreateSecret(context.Background(), secretInputParam)
+		err = cloud.recreateSecret(secretInputParam)
 
 		return err
 	}
@@ -193,6 +192,25 @@ func (cloud *CertificateAWSStorage) InitTrustpolicy(trustpolicy []byte) error {
 	return err
 }
 
+func (cloud *CertificateAWSStorage) recreateSecret(secretInputParam *secretsmanager.CreateSecretInput) error {
+	maxAttempts := 5
+	retrySec := 2
+
+	var err error
+
+	for i := 0; i < maxAttempts; i++ {
+		_, err = cloud.secretsManagerClient.CreateSecret(context.Background(), secretInputParam)
+
+		if err == nil {
+			return nil
+		}
+
+		time.Sleep(time.Duration(retrySec) * time.Second)
+	}
+
+	return err
+}
+
 func (local *CertificateLocalStorage) GetNotationDirPath() (string, error) {
 	if local.notationDir != "" {
 		return local.notationDir, nil

pkg/test/mocks/secrets_manager_mock.go

@@ -0,0 +1,58 @@
+package mocks
+
+import (
+	"context"
+
+	"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
+)
+
+type SecretsManagerMock struct {
+	CreateSecretFn func(ctx context.Context, params *secretsmanager.CreateSecretInput,
+		optFns ...func(*secretsmanager.Options)) (*secretsmanager.CreateSecretOutput, error)
+	DeleteSecretFn func(ctx context.Context, params *secretsmanager.DeleteSecretInput,
+		optFns ...func(*secretsmanager.Options)) (*secretsmanager.DeleteSecretOutput, error)
+	ListSecretsFn func(ctx context.Context, params *secretsmanager.ListSecretsInput,
+		optFns ...func(*secretsmanager.Options)) (*secretsmanager.ListSecretsOutput, error)
+}
+
+func (secretsManagerMock SecretsManagerMock) CreateSecret(ctx context.Context, params *secretsmanager.CreateSecretInput,
+	optFns ...func(*secretsmanager.Options),
+) (*secretsmanager.CreateSecretOutput, error) {
+	if secretsManagerMock.CreateSecretFn != nil {
+		return secretsManagerMock.CreateSecretFn(ctx, params, optFns...)
+	}
+
+	return &secretsmanager.CreateSecretOutput{}, nil
+}
+
+func (secretsManagerMock SecretsManagerMock) DeleteSecret(ctx context.Context, params *secretsmanager.DeleteSecretInput,
+	optFns ...func(*secretsmanager.Options),
+) (*secretsmanager.DeleteSecretOutput, error) {
+	if secretsManagerMock.DeleteSecretFn != nil {
+		return secretsManagerMock.DeleteSecretFn(ctx, params, optFns...)
+	}
+
+	return &secretsmanager.DeleteSecretOutput{}, nil
+}
+
+func (secretsManagerMock SecretsManagerMock) ListSecrets(ctx context.Context, params *secretsmanager.ListSecretsInput,
+	optFns ...func(*secretsmanager.Options),
+) (*secretsmanager.ListSecretsOutput, error) {
+	if secretsManagerMock.ListSecretsFn != nil {
+		return secretsManagerMock.ListSecretsFn(ctx, params, optFns...)
+	}
+
+	return &secretsmanager.ListSecretsOutput{}, nil
+}
+
+type SecretsManagerCacheMock struct {
+	GetSecretStringFn func(string) (string, error)
+}
+
+func (secretsManagerCacheMock SecretsManagerCacheMock) GetSecretString(secretID string) (string, error) {
+	if secretsManagerCacheMock.GetSecretStringFn != nil {
+		return secretsManagerCacheMock.GetSecretStringFn(secretID)
+	}
+
+	return "", nil
+}