From fb937deabf5845996d06a5d7edd7c1b5146346c8 Mon Sep 17 00:00:00 2001 From: ddelange <14880945+ddelange@users.noreply.github.com> Date: Fri, 1 Apr 2022 11:51:36 +0200 Subject: [PATCH 1/2] Support all S3 instant retrieval storage classes Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com> --- registry/storage/driver/s3-aws/s3.go | 39 ++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go index 238c5d80..d8680513 100644 --- a/registry/storage/driver/s3-aws/s3.go +++ b/registry/storage/driver/s3-aws/s3.go @@ -308,16 +308,45 @@ func FromParameters(parameters map[string]interface{}) (*Driver, error) { if storageClassParam != nil { storageClassString, ok := storageClassParam.(string) if !ok { - return nil, fmt.Errorf("the storageclass parameter must be one of %v, %v invalid", - []string{s3.StorageClassStandard, s3.StorageClassReducedRedundancy}, storageClassParam) + return nil, fmt.Errorf( + "the storageclass parameter must be one of %v, %v invalid", + []string{ + noStorageClass, + s3.StorageClassStandard, + s3.StorageClassReducedRedundancy, + s3.StorageClassStandardIa, + s3.StorageClassOnezoneIa, + s3.StorageClassIntelligentTiering, + s3.StorageClassOutposts, + s3.StorageClassGlacierIr, + }, + storageClassParam, + ) } // All valid storage class parameters are UPPERCASE, so be a bit more flexible here storageClassString = strings.ToUpper(storageClassString) if storageClassString != noStorageClass && storageClassString != s3.StorageClassStandard && - storageClassString != s3.StorageClassReducedRedundancy { - return nil, fmt.Errorf("the storageclass parameter must be one of %v, %v invalid", - []string{noStorageClass, s3.StorageClassStandard, s3.StorageClassReducedRedundancy}, storageClassParam) + storageClassString != s3.StorageClassReducedRedundancy && + storageClassString != s3.StorageClassStandardIa && + storageClassString != s3.StorageClassOnezoneIa && + storageClassString != s3.StorageClassIntelligentTiering && + storageClassString != s3.StorageClassOutposts && + storageClassString != s3.StorageClassGlacierIr { + return nil, fmt.Errorf( + "the storageclass parameter must be one of %v, %v invalid", + []string{ + noStorageClass, + s3.StorageClassStandard, + s3.StorageClassReducedRedundancy, + s3.StorageClassStandardIa, + s3.StorageClassOnezoneIa, + s3.StorageClassIntelligentTiering, + s3.StorageClassOutposts, + s3.StorageClassGlacierIr, + }, + storageClassParam, + ) } storageClass = storageClassString } From 966fae54639c0268949ee9a329b6951b92b4921f Mon Sep 17 00:00:00 2001 From: ddelange <14880945+ddelange@users.noreply.github.com> Date: Mon, 4 Apr 2022 09:55:51 +0200 Subject: [PATCH 2/2] Add tests for all supported storage classes Signed-off-by: ddelange <14880945+ddelange@users.noreply.github.com> --- registry/storage/driver/s3-aws/s3.go | 34 ++++---- registry/storage/driver/s3-aws/s3_test.go | 95 ++++++++++------------- 2 files changed, 54 insertions(+), 75 deletions(-) diff --git a/registry/storage/driver/s3-aws/s3.go b/registry/storage/driver/s3-aws/s3.go index d8680513..6b8d1771 100644 --- a/registry/storage/driver/s3-aws/s3.go +++ b/registry/storage/driver/s3-aws/s3.go @@ -75,6 +75,18 @@ const listMax = 1000 // noStorageClass defines the value to be used if storage class is not supported by the S3 endpoint const noStorageClass = "NONE" +// s3StorageClasses lists all compatible (instant retrieval) S3 storage classes +var s3StorageClasses = []string{ + noStorageClass, + s3.StorageClassStandard, + s3.StorageClassReducedRedundancy, + s3.StorageClassStandardIa, + s3.StorageClassOnezoneIa, + s3.StorageClassIntelligentTiering, + s3.StorageClassOutposts, + s3.StorageClassGlacierIr, +} + // validRegions maps known s3 region identifiers to region descriptors var validRegions = map[string]struct{}{} @@ -310,16 +322,7 @@ func FromParameters(parameters map[string]interface{}) (*Driver, error) { if !ok { return nil, fmt.Errorf( "the storageclass parameter must be one of %v, %v invalid", - []string{ - noStorageClass, - s3.StorageClassStandard, - s3.StorageClassReducedRedundancy, - s3.StorageClassStandardIa, - s3.StorageClassOnezoneIa, - s3.StorageClassIntelligentTiering, - s3.StorageClassOutposts, - s3.StorageClassGlacierIr, - }, + s3StorageClasses, storageClassParam, ) } @@ -335,16 +338,7 @@ func FromParameters(parameters map[string]interface{}) (*Driver, error) { storageClassString != s3.StorageClassGlacierIr { return nil, fmt.Errorf( "the storageclass parameter must be one of %v, %v invalid", - []string{ - noStorageClass, - s3.StorageClassStandard, - s3.StorageClassReducedRedundancy, - s3.StorageClassStandardIa, - s3.StorageClassOnezoneIa, - s3.StorageClassIntelligentTiering, - s3.StorageClassOutposts, - s3.StorageClassGlacierIr, - }, + s3StorageClasses, storageClassParam, ) } diff --git a/registry/storage/driver/s3-aws/s3_test.go b/registry/storage/driver/s3-aws/s3_test.go index 31662b17..c74a2179 100644 --- a/registry/storage/driver/s3-aws/s3_test.go +++ b/registry/storage/driver/s3-aws/s3_test.go @@ -228,66 +228,51 @@ func TestStorageClass(t *testing.T) { } defer os.Remove(rootDir) - standardDriver, err := s3DriverConstructor(rootDir, s3.StorageClassStandard) - if err != nil { - t.Fatalf("unexpected error creating driver with standard storage: %v", err) - } - - rrDriver, err := s3DriverConstructor(rootDir, s3.StorageClassReducedRedundancy) - if err != nil { - t.Fatalf("unexpected error creating driver with reduced redundancy storage: %v", err) - } - - if _, err = s3DriverConstructor(rootDir, noStorageClass); err != nil { - t.Fatalf("unexpected error creating driver without storage class: %v", err) - } - - standardFilename := "/test-standard" - rrFilename := "/test-rr" contents := []byte("contents") ctx := context.Background() + for _, storageClass := range s3StorageClasses { + filename := "/test-" + storageClass + s3Driver, err := s3DriverConstructor(rootDir, storageClass) + if err != nil { + t.Fatalf("unexpected error creating driver with storage class %v: %v", storageClass, err) + } - err = standardDriver.PutContent(ctx, standardFilename, contents) - if err != nil { - t.Fatalf("unexpected error creating content: %v", err) - } - defer standardDriver.Delete(ctx, standardFilename) + err = s3Driver.PutContent(ctx, filename, contents) + if err != nil { + t.Fatalf("unexpected error creating content with storage class %v: %v", storageClass, err) + } + defer s3Driver.Delete(ctx, filename) - err = rrDriver.PutContent(ctx, rrFilename, contents) - if err != nil { - t.Fatalf("unexpected error creating content: %v", err) + driverUnwrapped := s3Driver.Base.StorageDriver.(*driver) + resp, err := driverUnwrapped.S3.GetObject(&s3.GetObjectInput{ + Bucket: aws.String(driverUnwrapped.Bucket), + Key: aws.String(driverUnwrapped.s3Path(filename)), + }) + if err != nil { + t.Fatalf("unexpected error retrieving file with storage class %v: %v", storageClass, err) + } + defer resp.Body.Close() + // Amazon only populates this header value for non-standard storage classes + if storageClass == s3.StorageClassStandard && resp.StorageClass != nil { + t.Fatalf( + "unexpected response storage class for file with storage class %v: %v", + storageClass, + *resp.StorageClass, + ) + } else if storageClass != s3.StorageClassStandard && resp.StorageClass == nil { + t.Fatalf( + "unexpected response storage class for file with storage class %v: %v", + storageClass, + s3.StorageClassStandard, + ) + } else if storageClass != s3.StorageClassStandard && storageClass != *resp.StorageClass { + t.Fatalf( + "unexpected response storage class for file with storage class %v: %v", + storageClass, + *resp.StorageClass, + ) + } } - defer rrDriver.Delete(ctx, rrFilename) - - standardDriverUnwrapped := standardDriver.Base.StorageDriver.(*driver) - resp, err := standardDriverUnwrapped.S3.GetObject(&s3.GetObjectInput{ - Bucket: aws.String(standardDriverUnwrapped.Bucket), - Key: aws.String(standardDriverUnwrapped.s3Path(standardFilename)), - }) - if err != nil { - t.Fatalf("unexpected error retrieving standard storage file: %v", err) - } - defer resp.Body.Close() - // Amazon only populates this header value for non-standard storage classes - if resp.StorageClass != nil { - t.Fatalf("unexpected storage class for standard file: %v", resp.StorageClass) - } - - rrDriverUnwrapped := rrDriver.Base.StorageDriver.(*driver) - resp, err = rrDriverUnwrapped.S3.GetObject(&s3.GetObjectInput{ - Bucket: aws.String(rrDriverUnwrapped.Bucket), - Key: aws.String(rrDriverUnwrapped.s3Path(rrFilename)), - }) - if err != nil { - t.Fatalf("unexpected error retrieving reduced-redundancy storage file: %v", err) - } - defer resp.Body.Close() - if resp.StorageClass == nil { - t.Fatalf("unexpected storage class for reduced-redundancy file: %v", s3.StorageClassStandard) - } else if *resp.StorageClass != s3.StorageClassReducedRedundancy { - t.Fatalf("unexpected storage class for reduced-redundancy file: %v", *resp.StorageClass) - } - } func TestDelete(t *testing.T) {