Skip to content

Commit

Permalink
Merge pull request #889 from versity/fix/legal-hold-error-type
Browse files Browse the repository at this point in the history 
fix: Adds bucket object lock status check in GetObjectLegalHold and G…
  • Loading branch information
@benmcclelland
benmcclelland authored Oct 15, 2024
2 parents 79ece46 + 16e8134 commit 366186e
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 84 deletions.
66 changes: 36 additions & 30 deletions backend/azure/azure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1300,22 +1300,9 @@ func (az *Azure) GetObjectLockConfiguration(ctx context.Context, bucket string)
}

func (az *Azure) PutObjectRetention(ctx context.Context, bucket, object, versionId string, bypass bool, retention []byte) error {
cfg, err := az.getContainerMetaData(ctx, bucket, string(keyBucketLock))
err := az.isBucketObjectLockEnabled(ctx, bucket)
if err != nil {
return azureErrToS3Err(err)
}

if len(cfg) == 0 {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
}

var bucketLockConfig auth.BucketLockConfig
if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil {
return fmt.Errorf("parse bucket lock config: %w", err)
}

if !bucketLockConfig.Enabled {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
return err
}

blobClient, err := az.getBlobClient(bucket, object)
Expand Down Expand Up @@ -1376,6 +1363,11 @@ func (az *Azure) GetObjectRetention(ctx context.Context, bucket, object, version
return nil, azureErrToS3Err(err)
}

err = az.isBucketObjectLockEnabled(ctx, bucket)
if err != nil {
return nil, err
}

retentionPtr, ok := props.Metadata[string(keyObjRetention)]
if !ok {
return nil, s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration)
Expand All @@ -1385,22 +1377,9 @@ func (az *Azure) GetObjectRetention(ctx context.Context, bucket, object, version
}

func (az *Azure) PutObjectLegalHold(ctx context.Context, bucket, object, versionId string, status bool) error {
cfg, err := az.getContainerMetaData(ctx, bucket, string(keyBucketLock))
err := az.isBucketObjectLockEnabled(ctx, bucket)
if err != nil {
return azureErrToS3Err(err)
}

if len(cfg) == 0 {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
}

var bucketLockConfig auth.BucketLockConfig
if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil {
return fmt.Errorf("parse bucket lock config: %w", err)
}

if !bucketLockConfig.Enabled {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
return err
}

blobClient, err := az.getBlobClient(bucket, object)
Expand Down Expand Up @@ -1447,6 +1426,11 @@ func (az *Azure) GetObjectLegalHold(ctx context.Context, bucket, object, version
return nil, azureErrToS3Err(err)
}

err = az.isBucketObjectLockEnabled(ctx, bucket)
if err != nil {
return nil, err
}

retentionPtr, ok := props.Metadata[string(keyObjLegalHold)]
if !ok {
return nil, s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration)
Expand Down Expand Up @@ -1486,6 +1470,28 @@ func (az *Azure) ListBucketsAndOwners(ctx context.Context) (buckets []s3response
return buckets, nil
}

func (az *Azure) isBucketObjectLockEnabled(ctx context.Context, bucket string) error {
cfg, err := az.getContainerMetaData(ctx, bucket, string(keyBucketLock))
if err != nil {
return azureErrToS3Err(err)
}

if len(cfg) == 0 {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
}

var bucketLockConfig auth.BucketLockConfig
if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil {
return fmt.Errorf("parse bucket lock config: %w", err)
}

if !bucketLockConfig.Enabled {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
}

return nil
}

func (az *Azure) getContainerURL(cntr string) string {
return fmt.Sprintf("%v/%v", strings.TrimRight(az.serviceURL, "/"), cntr)
}
Expand Down
110 changes: 62 additions & 48 deletions backend/posix/posix.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,26 @@ func (p *Posix) versioningEnabled() bool {
return p.versioningDir != ""
}

func (p *Posix) doesBucketAndObjectExist(bucket, object string) error {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchBucket)
}
if err != nil {
return fmt.Errorf("stat bucket: %w", err)
}

_, err = os.Stat(filepath.Join(bucket, object))
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchKey)
}
if err != nil {
return fmt.Errorf("stat object: %w", err)
}

return nil
}

func (p *Posix) ListBuckets(_ context.Context, owner string, isAdmin bool) (s3response.ListAllMyBucketsResult, error) {
entries, err := os.ReadDir(".")
if err != nil {
Expand Down Expand Up @@ -3626,6 +3646,30 @@ func (p *Posix) DeleteBucketPolicy(ctx context.Context, bucket string) error {
return p.PutBucketPolicy(ctx, bucket, nil)
}

func (p *Posix) isBucketObjectLockEnabled(bucket string) error {
cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchBucket)
}
if errors.Is(err, meta.ErrNoSuchKey) {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
}
if err != nil {
return fmt.Errorf("get object lock config: %w", err)
}

var bucketLockConfig auth.BucketLockConfig
if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil {
return fmt.Errorf("parse bucket lock config: %w", err)
}

if !bucketLockConfig.Enabled {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
}

return nil
}

func (p *Posix) PutObjectLockConfiguration(ctx context.Context, bucket string, config []byte) error {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
Expand Down Expand Up @@ -3681,29 +3725,13 @@ func (p *Posix) GetObjectLockConfiguration(_ context.Context, bucket string) ([]
}

func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId string, status bool) error {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchBucket)
}
err := p.doesBucketAndObjectExist(bucket, object)
if err != nil {
return fmt.Errorf("stat bucket: %w", err)
}

cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey)
if errors.Is(err, meta.ErrNoSuchKey) {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
return err
}
err = p.isBucketObjectLockEnabled(bucket)
if err != nil {
return fmt.Errorf("get object lock config: %w", err)
}

var bucketLockConfig auth.BucketLockConfig
if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil {
return fmt.Errorf("parse bucket lock config: %w", err)
}

if !bucketLockConfig.Enabled {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
return err
}

var statusData []byte
Expand Down Expand Up @@ -3747,12 +3775,13 @@ func (p *Posix) PutObjectLegalHold(_ context.Context, bucket, object, versionId
}

func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId string) (*bool, error) {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)
err := p.doesBucketAndObjectExist(bucket, object)
if err != nil {
return nil, err
}
err = p.isBucketObjectLockEnabled(bucket)
if err != nil {
return nil, fmt.Errorf("stat bucket: %w", err)
return nil, err
}

if versionId != "" {
Expand Down Expand Up @@ -3794,29 +3823,13 @@ func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId
}

func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId string, bypass bool, retention []byte) error {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchBucket)
}
err := p.doesBucketAndObjectExist(bucket, object)
if err != nil {
return fmt.Errorf("stat bucket: %w", err)
}

cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey)
if errors.Is(err, meta.ErrNoSuchKey) {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
return err
}
err = p.isBucketObjectLockEnabled(bucket)
if err != nil {
return fmt.Errorf("get object lock config: %w", err)
}

var bucketLockConfig auth.BucketLockConfig
if err := json.Unmarshal(cfg, &bucketLockConfig); err != nil {
return fmt.Errorf("parse bucket lock config: %w", err)
}

if !bucketLockConfig.Enabled {
return s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)
return err
}

if versionId != "" {
Expand Down Expand Up @@ -3882,12 +3895,13 @@ func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId
}

func (p *Posix) GetObjectRetention(_ context.Context, bucket, object, versionId string) ([]byte, error) {
_, err := os.Stat(bucket)
if errors.Is(err, fs.ErrNotExist) {
return nil, s3err.GetAPIError(s3err.ErrNoSuchBucket)
err := p.doesBucketAndObjectExist(bucket, object)
if err != nil {
return nil, err
}
err = p.isBucketObjectLockEnabled(bucket)
if err != nil {
return nil, fmt.Errorf("stat bucket: %w", err)
return nil, err
}

if versionId != "" {
Expand Down
9 changes: 5 additions & 4 deletions s3err/s3err.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,12 +437,12 @@ var errorCodeResponse = map[ErrorCode]APIError{
},
ErrNoSuchObjectLockConfiguration: {
Code: "NoSuchObjectLockConfiguration",
Description: "The specified object does not have an ObjectLock configuration.",
Description: "The specified object does not have a ObjectLock configuration.",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidBucketObjectLockConfiguration: {
Code: "InvalidRequest",
Description: "Bucket is missing ObjectLockConfiguration.",
Description: "Bucket is missing Object Lock Configuration.",
HTTPStatusCode: http.StatusBadRequest,
},
ErrObjectLockConfigurationNotAllowed: {
Expand Down Expand Up @@ -521,8 +521,9 @@ var errorCodeResponse = map[ErrorCode]APIError{
HTTPStatusCode: http.StatusNotFound,
},
ErrInvalidMetadataDirective: {
Code: "InvalidArgument",
Description: "Unknown metadata directive.",
Code: "InvalidArgument",
Description: "Unknown metadata directive.",
HTTPStatusCode: http.StatusBadRequest,
},
ErrInvalidVersionId: {
Code: "InvalidArgument",
Expand Down
4 changes: 4 additions & 0 deletions tests/integration/group-tests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ func TestPutObjectRetention(s *S3Conf) {
func TestGetObjectRetention(s *S3Conf) {
GetObjectRetention_non_existing_bucket(s)
GetObjectRetention_non_existing_object(s)
GetObjectRetention_disabled_lock(s)
GetObjectRetention_unset_config(s)
GetObjectRetention_success(s)
}
Expand All @@ -424,6 +425,7 @@ func TestPutObjectLegalHold(s *S3Conf) {
func TestGetObjectLegalHold(s *S3Conf) {
GetObjectLegalHold_non_existing_bucket(s)
GetObjectLegalHold_non_existing_object(s)
GetObjectLegalHold_disabled_lock(s)
GetObjectLegalHold_unset_config(s)
GetObjectLegalHold_success(s)
}
Expand Down Expand Up @@ -877,6 +879,7 @@ func GetIntTests() IntTests {
"PutObjectRetention_success": PutObjectRetention_success,
"GetObjectRetention_non_existing_bucket": GetObjectRetention_non_existing_bucket,
"GetObjectRetention_non_existing_object": GetObjectRetention_non_existing_object,
"GetObjectRetention_disabled_lock": GetObjectRetention_disabled_lock,
"GetObjectRetention_unset_config": GetObjectRetention_unset_config,
"GetObjectRetention_success": GetObjectRetention_success,
"PutObjectLegalHold_non_existing_bucket": PutObjectLegalHold_non_existing_bucket,
Expand All @@ -888,6 +891,7 @@ func GetIntTests() IntTests {
"PutObjectLegalHold_success": PutObjectLegalHold_success,
"GetObjectLegalHold_non_existing_bucket": GetObjectLegalHold_non_existing_bucket,
"GetObjectLegalHold_non_existing_object": GetObjectLegalHold_non_existing_object,
"GetObjectLegalHold_disabled_lock": GetObjectLegalHold_disabled_lock,
"GetObjectLegalHold_unset_config": GetObjectLegalHold_unset_config,
"GetObjectLegalHold_success": GetObjectLegalHold_success,
"WORMProtection_bucket_object_lock_configuration_compliance_mode": WORMProtection_bucket_object_lock_configuration_compliance_mode,
Expand Down
Loading

0 comments on commit 366186e

Please sign in to comment.