diff --git a/pkg/extensions/imagetrust/cosign.go b/pkg/extensions/imagetrust/cosign.go index 7764965d..41b9935c 100644 --- a/pkg/extensions/imagetrust/cosign.go +++ b/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 - secretsManagerCache *secretcache.Cache + secretsManagerClient SecretsManagerClient + 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, diff --git a/pkg/extensions/imagetrust/image_trust.go b/pkg/extensions/imagetrust/image_trust.go index b64b17d4..8db7775a 100644 --- a/pkg/extensions/imagetrust/image_trust.go +++ b/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 { diff --git a/pkg/extensions/imagetrust/image_trust_test.go b/pkg/extensions/imagetrust/image_trust_test.go index 9b31f385..78e1cee9 100644 --- a/pkg/extensions/imagetrust/image_trust_test.go +++ b/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, }) diff --git a/pkg/extensions/imagetrust/notation.go b/pkg/extensions/imagetrust/notation.go index c3d0d821..c2fd0e28 100644 --- a/pkg/extensions/imagetrust/notation.go +++ b/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 - secretsManagerCache *secretcache.Cache + secretsManagerClient SecretsManagerClient + 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 diff --git a/pkg/test/mocks/secrets_manager_mock.go b/pkg/test/mocks/secrets_manager_mock.go new file mode 100644 index 00000000..83f3e596 --- /dev/null +++ b/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 +}