diff --git a/errors/errors.go b/errors/errors.go index fcab3604..faf56d12 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -76,4 +76,5 @@ var ( ErrLimitIsNegative = errors.New("pageturner: limit has negative value") ErrOffsetIsNegative = errors.New("pageturner: offset has negative value") ErrSortCriteriaNotSupported = errors.New("pageturner: the sort criteria is not supported") + ErrTimeout = errors.New("operation timeout") ) diff --git a/pkg/meta/repodb/dynamodb-wrapper/dynamo_internal_test.go b/pkg/meta/repodb/dynamodb-wrapper/dynamo_internal_test.go new file mode 100644 index 00000000..4f4cc44b --- /dev/null +++ b/pkg/meta/repodb/dynamodb-wrapper/dynamo_internal_test.go @@ -0,0 +1,102 @@ +package dynamo + +import ( + "context" + "os" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/service/dynamodb" + guuid "github.com/gofrs/uuid" + "github.com/rs/zerolog" + . "github.com/smartystreets/goconvey/convey" + + "zotregistry.io/zot/pkg/log" //nolint:go-staticcheck + "zotregistry.io/zot/pkg/meta/repodb/version" +) + +func TestWrapperErrors(t *testing.T) { + const ( + endpoint = "http://localhost:4566" + region = "us-east-2" + ) + + uuid, err := guuid.NewV4() + if err != nil { + panic(err) + } + + repoMetaTablename := "RepoMetadataTable" + uuid.String() + manifestDataTablename := "ManifestDataTable" + uuid.String() + versionTablename := "Version" + uuid.String() + + Convey("Create table errors", t, func() { + badEndpoint := endpoint + "1" + + customResolver := aws.EndpointResolverWithOptionsFunc( + func(service, region string, options ...interface{}) (aws.Endpoint, error) { + return aws.Endpoint{ + PartitionID: "aws", + URL: badEndpoint, + SigningRegion: region, + }, nil + }, + ) + + cfg, err := config.LoadDefaultConfig(context.Background(), config.WithRegion(region), + config.WithEndpointResolverWithOptions(customResolver)) + So(err, ShouldBeNil) + + dynamoWrapper := DBWrapper{ + Client: dynamodb.NewFromConfig(cfg), + RepoMetaTablename: repoMetaTablename, + ManifestDataTablename: manifestDataTablename, + VersionTablename: versionTablename, + Patches: version.GetDynamoDBPatches(), + Log: log.Logger{Logger: zerolog.New(os.Stdout)}, + } + + // The table creation should fail as the endpoint is not configured correctly + err = dynamoWrapper.createRepoMetaTable() + So(err, ShouldNotBeNil) + + err = dynamoWrapper.createManifestDataTable() + So(err, ShouldNotBeNil) + + err = dynamoWrapper.createVersionTable() + So(err, ShouldNotBeNil) + }) + + Convey("Delete table errors", t, func() { + customResolver := aws.EndpointResolverWithOptionsFunc( + func(service, region string, options ...interface{}) (aws.Endpoint, error) { + return aws.Endpoint{ + PartitionID: "aws", + URL: endpoint, + SigningRegion: region, + }, nil + }, + ) + + cfg, err := config.LoadDefaultConfig(context.Background(), config.WithRegion(region), + config.WithEndpointResolverWithOptions(customResolver)) + So(err, ShouldBeNil) + + dynamoWrapper := DBWrapper{ + Client: dynamodb.NewFromConfig(cfg), + RepoMetaTablename: repoMetaTablename, + ManifestDataTablename: manifestDataTablename, + VersionTablename: versionTablename, + Patches: version.GetDynamoDBPatches(), + Log: log.Logger{Logger: zerolog.New(os.Stdout)}, + } + + // The tables were not created so delete calls fail, but dynamoWrapper should not error + err = dynamoWrapper.deleteRepoMetaTable() + So(err, ShouldBeNil) + + err = dynamoWrapper.deleteManifestDataTable() + So(err, ShouldBeNil) + }) +} diff --git a/pkg/meta/repodb/dynamodb-wrapper/dynamo_test.go b/pkg/meta/repodb/dynamodb-wrapper/dynamo_test.go index 2f7fe636..451f441a 100644 --- a/pkg/meta/repodb/dynamodb-wrapper/dynamo_test.go +++ b/pkg/meta/repodb/dynamodb-wrapper/dynamo_test.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue" "github.com/aws/aws-sdk-go-v2/service/dynamodb" "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" + guuid "github.com/gofrs/uuid" "github.com/rs/zerolog" . "github.com/smartystreets/goconvey/convey" @@ -27,13 +28,22 @@ func TestIterator(t *testing.T) { region = "us-east-2" ) + uuid, err := guuid.NewV4() + if err != nil { + panic(err) + } + + repoMetaTablename := "RepoMetadataTable" + uuid.String() + manifestDataTablename := "ManifestDataTable" + uuid.String() + versionTablename := "Version" + uuid.String() + Convey("TestIterator", t, func() { dynamoWrapper, err := dynamo.NewDynamoDBWrapper(dynamoParams.DBDriverParameters{ Endpoint: endpoint, Region: region, - RepoMetaTablename: "RepoMetadataTable", - ManifestDataTablename: "ManifestDataTable", - VersionTablename: "Version", + RepoMetaTablename: repoMetaTablename, + ManifestDataTablename: manifestDataTablename, + VersionTablename: versionTablename, }) So(err, ShouldBeNil) @@ -51,7 +61,7 @@ func TestIterator(t *testing.T) { repoMetaAttributeIterator := iterator.NewBaseDynamoAttributesIterator( dynamoWrapper.Client, - "RepoMetadataTable", + repoMetaTablename, "RepoMetadata", 1, log.Logger{Logger: zerolog.New(os.Stdout)}, @@ -109,15 +119,24 @@ func TestWrapperErrors(t *testing.T) { region = "us-east-2" ) + uuid, err := guuid.NewV4() + if err != nil { + panic(err) + } + + repoMetaTablename := "RepoMetadataTable" + uuid.String() + manifestDataTablename := "ManifestDataTable" + uuid.String() + versionTablename := "Version" + uuid.String() + ctx := context.Background() Convey("Errors", t, func() { dynamoWrapper, err := dynamo.NewDynamoDBWrapper(dynamoParams.DBDriverParameters{ //nolint:contextcheck Endpoint: endpoint, Region: region, - RepoMetaTablename: "RepoMetadataTable", - ManifestDataTablename: "ManifestDataTable", - VersionTablename: "Version", + RepoMetaTablename: repoMetaTablename, + ManifestDataTablename: manifestDataTablename, + VersionTablename: versionTablename, }) So(err, ShouldBeNil) @@ -139,7 +158,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetManifestData unmarshal error", func() { - err := setBadManifestData(dynamoWrapper.Client, "dig") + err := setBadManifestData(dynamoWrapper.Client, manifestDataTablename, "dig") So(err, ShouldBeNil) _, err = dynamoWrapper.GetManifestData("dig") @@ -147,7 +166,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("SetManifestMeta GetRepoMeta error", func() { - err := setBadRepoMeta(dynamoWrapper.Client, "repo1") + err := setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo1") So(err, ShouldBeNil) err = dynamoWrapper.SetManifestMeta("repo1", "dig", repodb.ManifestMetadata{}) @@ -174,7 +193,7 @@ func TestWrapperErrors(t *testing.T) { err := dynamoWrapper.SetManifestData("dig", repodb.ManifestData{}) So(err, ShouldBeNil) - err = setBadRepoMeta(dynamoWrapper.Client, "repo") + err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") So(err, ShouldBeNil) _, err = dynamoWrapper.GetManifestMeta("repo", "dig") @@ -200,7 +219,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("DeleteRepoTag unmarshal error", func() { - err = setBadRepoMeta(dynamoWrapper.Client, "repo") + err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") So(err, ShouldBeNil) err = dynamoWrapper.DeleteRepoTag("repo", "tag") @@ -216,7 +235,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetRepoMeta unmarshal error", func() { - err = setBadRepoMeta(dynamoWrapper.Client, "repo") + err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") So(err, ShouldBeNil) _, err = dynamoWrapper.GetRepoMeta("repo") @@ -276,7 +295,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("DeleteSignature sigDigest.SignatureManifestDigest != sigMeta.SignatureDigest true", func() { - err := setRepoMeta(dynamoWrapper.Client, repodb.RepoMetadata{ + err := setRepoMeta(dynamoWrapper.Client, repoMetaTablename, repodb.RepoMetadata{ Name: "repo", Signatures: map[string]repodb.ManifestSignatures{ "tag1": { @@ -297,7 +316,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("GetMultipleRepoMeta unmarshal error", func() { - err = setBadRepoMeta(dynamoWrapper.Client, "repo") //nolint:contextcheck + err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") //nolint:contextcheck So(err, ShouldBeNil) _, err = dynamoWrapper.GetMultipleRepoMeta(ctx, func(repoMeta repodb.RepoMetadata) bool { return true }, @@ -307,7 +326,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("SearchRepos repoMeta unmarshal error", func() { - err = setBadRepoMeta(dynamoWrapper.Client, "repo") //nolint:contextcheck + err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") //nolint:contextcheck So(err, ShouldBeNil) _, _, err = dynamoWrapper.SearchRepos(ctx, "", repodb.Filter{}, repodb.PageInput{}) @@ -340,7 +359,7 @@ func TestWrapperErrors(t *testing.T) { }) Convey("SearchTags repoMeta unmarshal error", func() { - err = setBadRepoMeta(dynamoWrapper.Client, "repo") //nolint:contextcheck + err = setBadRepoMeta(dynamoWrapper.Client, repoMetaTablename, "repo") //nolint:contextcheck So(err, ShouldBeNil) _, _, err = dynamoWrapper.SearchTags(ctx, "repo:", repodb.Filter{}, repodb.PageInput{}) @@ -377,7 +396,7 @@ func TestWrapperErrors(t *testing.T) { }) } -func setBadManifestData(client *dynamodb.Client, digest string) error { +func setBadManifestData(client *dynamodb.Client, manifestDataTableName, digest string) error { mdAttributeValue, err := attributevalue.Marshal("string") if err != nil { return err @@ -395,14 +414,14 @@ func setBadManifestData(client *dynamodb.Client, digest string) error { Value: digest, }, }, - TableName: aws.String("ManifestDataTable"), + TableName: aws.String(manifestDataTableName), UpdateExpression: aws.String("SET #MD = :ManifestData"), }) return err } -func setBadRepoMeta(client *dynamodb.Client, repoName string) error { +func setBadRepoMeta(client *dynamodb.Client, repoMetadataTableName, repoName string) error { repoAttributeValue, err := attributevalue.Marshal("string") if err != nil { return err @@ -420,14 +439,14 @@ func setBadRepoMeta(client *dynamodb.Client, repoName string) error { Value: repoName, }, }, - TableName: aws.String("RepoMetadataTable"), + TableName: aws.String(repoMetadataTableName), UpdateExpression: aws.String("SET #RM = :RepoMetadata"), }) return err } -func setRepoMeta(client *dynamodb.Client, repoMeta repodb.RepoMetadata) error { +func setRepoMeta(client *dynamodb.Client, repoMetadataTableName string, repoMeta repodb.RepoMetadata) error { repoAttributeValue, err := attributevalue.Marshal(repoMeta) if err != nil { return err @@ -445,7 +464,7 @@ func setRepoMeta(client *dynamodb.Client, repoMeta repodb.RepoMetadata) error { Value: repoMeta.Name, }, }, - TableName: aws.String("RepoMetadataTable"), + TableName: aws.String(repoMetadataTableName), UpdateExpression: aws.String("SET #RM = :RepoMetadata"), }) diff --git a/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go b/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go index f529c4eb..2f8fa0a3 100644 --- a/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go +++ b/pkg/meta/repodb/dynamodb-wrapper/dynamo_wrapper.go @@ -833,11 +833,11 @@ func (dwr DBWrapper) createRepoMetaTable() error { BillingMode: types.BillingModePayPerRequest, }) - if err != nil && strings.Contains(err.Error(), "Table already exists") { - return nil + if err != nil && !strings.Contains(err.Error(), "Table already exists") { + return err } - return err + return dwr.waitTableToBeCreated(dwr.RepoMetaTablename) } func (dwr DBWrapper) deleteRepoMetaTable() error { @@ -845,7 +845,11 @@ func (dwr DBWrapper) deleteRepoMetaTable() error { TableName: aws.String(dwr.RepoMetaTablename), }) - return err + if temp := new(types.ResourceNotFoundException); errors.As(err, &temp) { + return nil + } + + return dwr.waitTableToBeDeleted(dwr.RepoMetaTablename) } func (dwr DBWrapper) ResetRepoMetaTable() error { @@ -857,6 +861,26 @@ func (dwr DBWrapper) ResetRepoMetaTable() error { return dwr.createRepoMetaTable() } +func (dwr DBWrapper) waitTableToBeCreated(tableName string) error { + const maxWaitTime = 20 * time.Second + + waiter := dynamodb.NewTableExistsWaiter(dwr.Client) + + return waiter.Wait(context.Background(), &dynamodb.DescribeTableInput{ + TableName: &tableName, + }, maxWaitTime) +} + +func (dwr DBWrapper) waitTableToBeDeleted(tableName string) error { + const maxWaitTime = 20 * time.Second + + waiter := dynamodb.NewTableNotExistsWaiter(dwr.Client) + + return waiter.Wait(context.Background(), &dynamodb.DescribeTableInput{ + TableName: &tableName, + }, maxWaitTime) +} + func (dwr DBWrapper) createManifestDataTable() error { _, err := dwr.Client.CreateTable(context.Background(), &dynamodb.CreateTableInput{ TableName: aws.String(dwr.ManifestDataTablename), @@ -875,11 +899,11 @@ func (dwr DBWrapper) createManifestDataTable() error { BillingMode: types.BillingModePayPerRequest, }) - if err != nil && strings.Contains(err.Error(), "Table already exists") { - return nil + if err != nil && !strings.Contains(err.Error(), "Table already exists") { + return err } - return err + return dwr.waitTableToBeCreated(dwr.ManifestDataTablename) } func (dwr *DBWrapper) createVersionTable() error { @@ -899,9 +923,17 @@ func (dwr *DBWrapper) createVersionTable() error { }, BillingMode: types.BillingModePayPerRequest, }) + if err != nil { + if strings.Contains(err.Error(), "Table already exists") { + return nil + } - if err != nil && strings.Contains(err.Error(), "Table already exists") { - return nil + return err + } + + err = dwr.waitTableToBeCreated(dwr.VersionTablename) + if err != nil { + return err } if err == nil { @@ -931,7 +963,7 @@ func (dwr *DBWrapper) createVersionTable() error { } } - return err + return nil } func (dwr *DBWrapper) getDBVersion() (string, error) { @@ -964,7 +996,11 @@ func (dwr DBWrapper) deleteManifestDataTable() error { TableName: aws.String(dwr.ManifestDataTablename), }) - return err + if temp := new(types.ResourceNotFoundException); errors.As(err, &temp) { + return nil + } + + return dwr.waitTableToBeDeleted(dwr.ManifestDataTablename) } func (dwr DBWrapper) ResetManifestDataTable() error { diff --git a/pkg/meta/repodb/repodb_test.go b/pkg/meta/repodb/repodb_test.go index 461e69f5..0c3e337d 100644 --- a/pkg/meta/repodb/repodb_test.go +++ b/pkg/meta/repodb/repodb_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + guuid "github.com/gofrs/uuid" "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/specs-go" ispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -65,12 +66,21 @@ func TestBoltDBWrapper(t *testing.T) { func TestDynamoDBWrapper(t *testing.T) { skipIt(t) + uuid, err := guuid.NewV4() + if err != nil { + panic(err) + } + + repoMetaTablename := "RepoMetadataTable" + uuid.String() + manifestDataTablename := "ManifestDataTable" + uuid.String() + versionTablename := "Version" + uuid.String() + Convey("DynamoDB Wrapper", t, func() { dynamoDBDriverParams := dynamoParams.DBDriverParameters{ Endpoint: os.Getenv("DYNAMODBMOCK_ENDPOINT"), - RepoMetaTablename: "RepoMetadataTable", - ManifestDataTablename: "ManifestDataTable", - VersionTablename: "Version", + RepoMetaTablename: repoMetaTablename, + ManifestDataTablename: manifestDataTablename, + VersionTablename: versionTablename, Region: "us-east-2", }