From 068b04ec6258b2604f55acab3bde3bcd8c023ae4 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Thu, 25 Sep 2025 01:41:41 +0400 Subject: [PATCH] fix: fixes PutObjectRetention error cases and object lock error code/message. Fixes #1559 Fixes #1330 This PR focuses on three main changes: 1. **Fix object lock error codes and descriptions** When an object was WORM-protected and delete/overwrite was disallowed due to object lock configurations, the gateway incorrectly returned the `s3.ErrObjectLocked` error code and description. These have now been corrected. 2. **Update `PutObjectRetention` behavior** Previously, when an object already had a retention mode set, the gateway only allowed modifications if the mode was changed from `GOVERNANCE` to `COMPLIANCE`, and only when the user had the `s3:BypassGovernanceRetention` permission. The logic has been updated: if the existing retention mode is the same as the one being applied, the operation is now allowed regardless of other factors. 3. **Fix error checks in integration tests (AWS SDK regression)** Due to an AWS SDK regression, integration tests were previously limited to checking partial error descriptions. This issue seems to be resolved for some actions (though the ticket is still open: https://github.com/aws/aws-sdk-go-v2/issues/2921). Error checks have been reverted back to full description comparisons where possible. --- auth/object_lock.go | 78 ++++++++- backend/azure/azure.go | 29 +--- backend/backend.go | 4 +- backend/posix/posix.go | 41 +---- backend/s3proxy/s3.go | 2 +- s3api/controllers/backend_moq_test.go | 14 +- s3api/controllers/object-put.go | 35 ++-- s3api/controllers/object-put_test.go | 73 ++++----- s3err/s3err.go | 6 +- tests/integration/group-tests.go | 4 + tests/integration/tests.go | 225 +++++++++++++++----------- tests/integration/utils.go | 24 +-- 12 files changed, 284 insertions(+), 251 deletions(-) diff --git a/auth/object_lock.go b/auth/object_lock.go index cb0d064..8ab6942 100644 --- a/auth/object_lock.go +++ b/auth/object_lock.go @@ -24,6 +24,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/versity/versitygw/backend" + "github.com/versity/versitygw/debuglogger" "github.com/versity/versitygw/s3err" "github.com/versity/versitygw/s3response" ) @@ -92,28 +93,101 @@ func ParseBucketLockConfigurationOutput(input []byte) (*types.ObjectLockConfigur return result, nil } -func ParseObjectLockRetentionInput(input []byte) ([]byte, error) { +func ParseObjectLockRetentionInput(input []byte) (*s3response.PutObjectRetentionInput, error) { var retention s3response.PutObjectRetentionInput if err := xml.Unmarshal(input, &retention); err != nil { + debuglogger.Logf("invalid object lock retention request body: %v", err) return nil, s3err.GetAPIError(s3err.ErrMalformedXML) } if retention.RetainUntilDate.Before(time.Now()) { + debuglogger.Logf("object lock retain until date must be in the future") return nil, s3err.GetAPIError(s3err.ErrPastObjectLockRetainDate) } switch retention.Mode { case types.ObjectLockRetentionModeCompliance: case types.ObjectLockRetentionModeGovernance: default: + debuglogger.Logf("invalid object lock retention mode: %s", retention.Mode) return nil, s3err.GetAPIError(s3err.ErrMalformedXML) } - return json.Marshal(retention) + return &retention, nil +} + +func ParseObjectLockRetentionInputToJSON(input *s3response.PutObjectRetentionInput) ([]byte, error) { + data, err := json.Marshal(input) + if err != nil { + debuglogger.Logf("parse object lock retention to JSON: %v", err) + return nil, fmt.Errorf("parse object lock retention: %w", err) + } + + return data, nil +} + +// IsObjectLockRetentionPutAllowed checks if the object lock retention PUT request +// is allowed against the current state of the object lock +func IsObjectLockRetentionPutAllowed(ctx context.Context, be backend.Backend, bucket, object, versionId, userAccess string, input *s3response.PutObjectRetentionInput, bypass bool) error { + ret, err := be.GetObjectRetention(ctx, bucket, object, versionId) + if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration)) { + // if object lock configuration is not set + // allow the retention modification without any checks + return nil + } + if err != nil { + debuglogger.Logf("failed to get object retention: %v", err) + return err + } + + retention, err := ParseObjectLockRetentionOutput(ret) + if err != nil { + return err + } + + if retention.Mode == input.Mode { + // if retention mode is the same + // the operation is allowed + return nil + } + + if retention.Mode == types.ObjectLockRetentionModeCompliance { + // COMPLIANCE mode is by definition not allowed to modify + debuglogger.Logf("object lock retention change request from 'COMPLIANCE' to 'GOVERNANCE' is not allowed") + return s3err.GetAPIError(s3err.ErrObjectLocked) + } + + if !bypass { + // if x-amz-bypass-governance-retention is not provided + // return error: object is locked + debuglogger.Logf("object lock retention mode change is not allowed and bypass governence is not forced") + return s3err.GetAPIError(s3err.ErrObjectLocked) + } + + // the last case left, when user tries to chenge + // from 'GOVERNANCE' to 'COMPLIANCE' with + // 'x-amz-bypass-governance-retention' header + // first we need to check if user has 's3:BypassGovernanceRetention' + policy, err := be.GetBucketPolicy(ctx, bucket) + if err != nil { + // if it fails to get the policy, return object is locked + debuglogger.Logf("failed to get the bucket policy: %v", err) + return s3err.GetAPIError(s3err.ErrObjectLocked) + } + err = VerifyBucketPolicy(policy, userAccess, bucket, object, BypassGovernanceRetentionAction) + if err != nil { + // if user doesn't have "s3:BypassGovernanceRetention" permission + // return object is locked + debuglogger.Logf("the user is missing 's3:BypassGovernanceRetention' permission") + return s3err.GetAPIError(s3err.ErrObjectLocked) + } + + return nil } func ParseObjectLockRetentionOutput(input []byte) (*types.ObjectLockRetention, error) { var retention types.ObjectLockRetention if err := json.Unmarshal(input, &retention); err != nil { + debuglogger.Logf("parse object lock retention output: %v", err) return nil, fmt.Errorf("parse object lock retention: %w", err) } diff --git a/backend/azure/azure.go b/backend/azure/azure.go index 452f682..5b1514b 100644 --- a/backend/azure/azure.go +++ b/backend/azure/azure.go @@ -364,7 +364,7 @@ func (az *Azure) PutObject(ctx context.Context, po s3response.PutObjectInput) (s if err != nil { return s3response.PutObjectOutput{}, fmt.Errorf("parse object lock retention: %w", err) } - err = az.PutObjectRetention(ctx, *po.Bucket, *po.Key, "", true, retParsed) + err = az.PutObjectRetention(ctx, *po.Bucket, *po.Key, "", retParsed) if err != nil { return s3response.PutObjectOutput{}, err } @@ -977,7 +977,7 @@ func (az *Azure) CopyObject(ctx context.Context, input s3response.CopyObjectInpu if err != nil { return s3response.CopyObjectOutput{}, fmt.Errorf("parse object retention: %w", err) } - err = az.PutObjectRetention(ctx, *input.Bucket, *input.Key, "", true, retParsed) + err = az.PutObjectRetention(ctx, *input.Bucket, *input.Key, "", retParsed) if err != nil { return s3response.CopyObjectOutput{}, azureErrToS3Err(err) } @@ -1678,7 +1678,7 @@ func (az *Azure) GetObjectLockConfiguration(ctx context.Context, bucket string) return cfg, nil } -func (az *Azure) PutObjectRetention(ctx context.Context, bucket, object, versionId string, bypass bool, retention []byte) error { +func (az *Azure) PutObjectRetention(ctx context.Context, bucket, object, versionId string, retention []byte) error { err := az.isBucketObjectLockEnabled(ctx, bucket) if err != nil { return err @@ -1700,28 +1700,7 @@ func (az *Azure) PutObjectRetention(ctx context.Context, bucket, object, version string(keyObjRetention): backend.GetPtrFromString(string(retention)), } } else { - objLockCfg, ok := meta[string(keyObjRetention)] - if !ok { - meta[string(keyObjRetention)] = backend.GetPtrFromString(string(retention)) - } else { - var lockCfg types.ObjectLockRetention - if err := json.Unmarshal([]byte(*objLockCfg), &lockCfg); err != nil { - return fmt.Errorf("unmarshal object lock config: %w", err) - } - - switch lockCfg.Mode { - // Compliance mode can't be overridden - case types.ObjectLockRetentionModeCompliance: - return s3err.GetAPIError(s3err.ErrMethodNotAllowed) - // To override governance mode user should have "s3:BypassGovernanceRetention" permission - case types.ObjectLockRetentionModeGovernance: - if !bypass { - return s3err.GetAPIError(s3err.ErrMethodNotAllowed) - } - } - - meta[string(keyObjRetention)] = backend.GetPtrFromString(string(retention)) - } + meta[string(keyObjRetention)] = backend.GetPtrFromString(string(retention)) } _, err = blobClient.SetMetadata(ctx, meta, nil) diff --git a/backend/backend.go b/backend/backend.go index b0ce7ff..873e798 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -90,7 +90,7 @@ type Backend interface { // object lock operations PutObjectLockConfiguration(_ context.Context, bucket string, config []byte) error GetObjectLockConfiguration(_ context.Context, bucket string) ([]byte, error) - PutObjectRetention(_ context.Context, bucket, object, versionId string, bypass bool, retention []byte) error + PutObjectRetention(_ context.Context, bucket, object, versionId string, retention []byte) error GetObjectRetention(_ context.Context, bucket, object, versionId string) ([]byte, error) PutObjectLegalHold(_ context.Context, bucket, object, versionId string, status bool) error GetObjectLegalHold(_ context.Context, bucket, object, versionId string) (*bool, error) @@ -267,7 +267,7 @@ func (BackendUnsupported) PutObjectLockConfiguration(_ context.Context, bucket s func (BackendUnsupported) GetObjectLockConfiguration(_ context.Context, bucket string) ([]byte, error) { return nil, s3err.GetAPIError(s3err.ErrNotImplemented) } -func (BackendUnsupported) PutObjectRetention(_ context.Context, bucket, object, versionId string, bypass bool, retention []byte) error { +func (BackendUnsupported) PutObjectRetention(_ context.Context, bucket, object, versionId string, retention []byte) error { return s3err.GetAPIError(s3err.ErrNotImplemented) } func (BackendUnsupported) GetObjectRetention(_ context.Context, bucket, object, versionId string) ([]byte, error) { diff --git a/backend/posix/posix.go b/backend/posix/posix.go index ea2ee7e..7e22828 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -1295,7 +1295,7 @@ func (p *Posix) CreateMultipartUpload(ctx context.Context, mpu s3response.Create os.Remove(tmppath) return s3response.InitiateMultipartUploadResult{}, fmt.Errorf("parse object lock retention: %w", err) } - err = p.PutObjectRetention(ctx, bucket, filepath.Join(objdir, uploadID), "", true, retParsed) + err = p.PutObjectRetention(ctx, bucket, filepath.Join(objdir, uploadID), "", retParsed) if err != nil { // cleanup object if returning error os.RemoveAll(filepath.Join(tmppath, uploadID)) @@ -3058,7 +3058,7 @@ func (p *Posix) PutObject(ctx context.Context, po s3response.PutObjectInput) (s3 if err != nil { return s3response.PutObjectOutput{}, fmt.Errorf("parse object lock retention: %w", err) } - err = p.PutObjectRetention(ctx, *po.Bucket, *po.Key, "", true, retParsed) + err = p.PutObjectRetention(ctx, *po.Bucket, *po.Key, "", retParsed) if err != nil { return s3response.PutObjectOutput{}, err } @@ -4948,7 +4948,7 @@ func (p *Posix) GetObjectLegalHold(_ context.Context, bucket, object, versionId return &result, nil } -func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId string, bypass bool, retention []byte) error { +func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId string, retention []byte) error { err := p.doesBucketAndObjectExist(bucket, object) if err != nil { return err @@ -4977,41 +4977,6 @@ func (p *Posix) PutObjectRetention(_ context.Context, bucket, object, versionId } } - objectLockCfg, err := p.meta.RetrieveAttribute(nil, bucket, object, objectRetentionKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { - if versionId != "" { - return s3err.GetAPIError(s3err.ErrInvalidVersionId) - } - return s3err.GetAPIError(s3err.ErrNoSuchKey) - } - if errors.Is(err, meta.ErrNoSuchKey) { - err := p.meta.StoreAttribute(nil, bucket, object, objectRetentionKey, retention) - if err != nil { - return fmt.Errorf("set object lock config: %w", err) - } - - return nil - } - if err != nil { - return fmt.Errorf("get object lock config: %w", err) - } - - var lockCfg types.ObjectLockRetention - if err := json.Unmarshal(objectLockCfg, &lockCfg); err != nil { - return fmt.Errorf("unmarshal object lock config: %w", err) - } - - switch lockCfg.Mode { - // Compliance mode can't be overridden - case types.ObjectLockRetentionModeCompliance: - return s3err.GetAPIError(s3err.ErrMethodNotAllowed) - // To override governance mode user should have "s3:BypassGovernanceRetention" permission - case types.ObjectLockRetentionModeGovernance: - if !bypass { - return s3err.GetAPIError(s3err.ErrMethodNotAllowed) - } - } - err = p.meta.StoreAttribute(nil, bucket, object, objectRetentionKey, retention) if err != nil { return fmt.Errorf("set object lock config: %w", err) diff --git a/backend/s3proxy/s3.go b/backend/s3proxy/s3.go index 071c2d9..87966c0 100644 --- a/backend/s3proxy/s3.go +++ b/backend/s3proxy/s3.go @@ -1558,7 +1558,7 @@ func (s *S3Proxy) GetObjectLockConfiguration(ctx context.Context, bucket string) return nil, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound) } -func (s *S3Proxy) PutObjectRetention(ctx context.Context, bucket, object, versionId string, bypass bool, retention []byte) error { +func (s *S3Proxy) PutObjectRetention(ctx context.Context, bucket, object, versionId string, retention []byte) error { return s3err.GetAPIError(s3err.ErrNotImplemented) } diff --git a/s3api/controllers/backend_moq_test.go b/s3api/controllers/backend_moq_test.go index b30ded2..a0d7710 100644 --- a/s3api/controllers/backend_moq_test.go +++ b/s3api/controllers/backend_moq_test.go @@ -161,7 +161,7 @@ var _ backend.Backend = &BackendMock{} // PutObjectLockConfigurationFunc: func(contextMoqParam context.Context, bucket string, config []byte) error { // panic("mock out the PutObjectLockConfiguration method") // }, -// PutObjectRetentionFunc: func(contextMoqParam context.Context, bucket string, object string, versionId string, bypass bool, retention []byte) error { +// PutObjectRetentionFunc: func(contextMoqParam context.Context, bucket string, object string, versionId string, retention []byte) error { // panic("mock out the PutObjectRetention method") // }, // PutObjectTaggingFunc: func(contextMoqParam context.Context, bucket string, object string, tags map[string]string) error { @@ -331,7 +331,7 @@ type BackendMock struct { PutObjectLockConfigurationFunc func(contextMoqParam context.Context, bucket string, config []byte) error // PutObjectRetentionFunc mocks the PutObjectRetention method. - PutObjectRetentionFunc func(contextMoqParam context.Context, bucket string, object string, versionId string, bypass bool, retention []byte) error + PutObjectRetentionFunc func(contextMoqParam context.Context, bucket string, object string, versionId string, retention []byte) error // PutObjectTaggingFunc mocks the PutObjectTagging method. PutObjectTaggingFunc func(contextMoqParam context.Context, bucket string, object string, tags map[string]string) error @@ -722,8 +722,6 @@ type BackendMock struct { Object string // VersionId is the versionId argument value. VersionId string - // Bypass is the bypass argument value. - Bypass bool // Retention is the retention argument value. Retention []byte } @@ -2554,7 +2552,7 @@ func (mock *BackendMock) PutObjectLockConfigurationCalls() []struct { } // PutObjectRetention calls PutObjectRetentionFunc. -func (mock *BackendMock) PutObjectRetention(contextMoqParam context.Context, bucket string, object string, versionId string, bypass bool, retention []byte) error { +func (mock *BackendMock) PutObjectRetention(contextMoqParam context.Context, bucket string, object string, versionId string, retention []byte) error { if mock.PutObjectRetentionFunc == nil { panic("BackendMock.PutObjectRetentionFunc: method is nil but Backend.PutObjectRetention was just called") } @@ -2563,20 +2561,18 @@ func (mock *BackendMock) PutObjectRetention(contextMoqParam context.Context, buc Bucket string Object string VersionId string - Bypass bool Retention []byte }{ ContextMoqParam: contextMoqParam, Bucket: bucket, Object: object, VersionId: versionId, - Bypass: bypass, Retention: retention, } mock.lockPutObjectRetention.Lock() mock.calls.PutObjectRetention = append(mock.calls.PutObjectRetention, callInfo) mock.lockPutObjectRetention.Unlock() - return mock.PutObjectRetentionFunc(contextMoqParam, bucket, object, versionId, bypass, retention) + return mock.PutObjectRetentionFunc(contextMoqParam, bucket, object, versionId, retention) } // PutObjectRetentionCalls gets all the calls that were made to PutObjectRetention. @@ -2588,7 +2584,6 @@ func (mock *BackendMock) PutObjectRetentionCalls() []struct { Bucket string Object string VersionId string - Bypass bool Retention []byte } { var calls []struct { @@ -2596,7 +2591,6 @@ func (mock *BackendMock) PutObjectRetentionCalls() []struct { Bucket string Object string VersionId string - Bypass bool Retention []byte } mock.lockPutObjectRetention.RLock() diff --git a/s3api/controllers/object-put.go b/s3api/controllers/object-put.go index 7452eb3..af5bd1c 100644 --- a/s3api/controllers/object-put.go +++ b/s3api/controllers/object-put.go @@ -106,20 +106,9 @@ func (c S3ApiController) PutObjectRetention(ctx *fiber.Ctx) (*Response, error) { }, err } - if bypass { - policy, err := c.be.GetBucketPolicy(ctx.Context(), bucket) - if err != nil { - bypass = false - } else { - if err := auth.VerifyBucketPolicy(policy, acct.Access, bucket, key, auth.BypassGovernanceRetentionAction); err != nil { - bypass = false - } - } - } - + // parse the request body bytes into a go struct and validate retention, err := auth.ParseObjectLockRetentionInput(ctx.Body()) if err != nil { - debuglogger.Logf("failed to parse object lock configuration input: %v", err) return &Response{ MetaOpts: &MetaOptions{ BucketOwner: parsedAcl.Owner, @@ -127,7 +116,27 @@ func (c S3ApiController) PutObjectRetention(ctx *fiber.Ctx) (*Response, error) { }, err } - err = c.be.PutObjectRetention(ctx.Context(), bucket, key, versionId, bypass, retention) + // check if the operation is allowed + err = auth.IsObjectLockRetentionPutAllowed(ctx.Context(), c.be, bucket, key, versionId, acct.Access, retention, bypass) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + + // parse the retention to JSON + data, err := auth.ParseObjectLockRetentionInputToJSON(retention) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + + err = c.be.PutObjectRetention(ctx.Context(), bucket, key, versionId, data) return &Response{ MetaOpts: &MetaOptions{ BucketOwner: parsedAcl.Owner, diff --git a/s3api/controllers/object-put_test.go b/s3api/controllers/object-put_test.go index 4af140f..f586818 100644 --- a/s3api/controllers/object-put_test.go +++ b/s3api/controllers/object-put_test.go @@ -186,12 +186,29 @@ func TestS3ApiController_PutObjectRetention(t *testing.T) { err: s3err.GetAPIError(s3err.ErrMalformedXML), }, }, + { + name: "retention put not allowed", + input: testInput{ + locals: defaultLocals, + body: validRetentionBody, + extraMockErr: s3err.GetAPIError(s3err.ErrAccessDenied), + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + err: s3err.GetAPIError(s3err.ErrAccessDenied), + }, + }, { name: "backend returns error", input: testInput{ - locals: defaultLocals, - beErr: s3err.GetAPIError(s3err.ErrNoSuchBucket), - body: validRetentionBody, + locals: defaultLocals, + beErr: s3err.GetAPIError(s3err.ErrNoSuchBucket), + body: validRetentionBody, + extraMockErr: s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration), }, output: testOutput{ response: &Response{ @@ -203,46 +220,11 @@ func TestS3ApiController_PutObjectRetention(t *testing.T) { }, }, { - name: "success bypass GetBucketPolicy fails", + name: "successful response", input: testInput{ locals: defaultLocals, body: validRetentionBody, - extraMockErr: s3err.GetAPIError(s3err.ErrAccessDenied), - headers: map[string]string{ - "X-Amz-Bypass-Governance-Retention": "true", - }, - }, - output: testOutput{ - response: &Response{ - MetaOpts: &MetaOptions{ - BucketOwner: "root", - }, - }, - }, - }, - { - name: "success bypass VerifyBucketPolicy fails", - input: testInput{ - locals: defaultLocals, - body: validRetentionBody, - extraMockResp: []byte("invalid_policy"), - headers: map[string]string{ - "X-Amz-Bypass-Governance-Retention": "true", - }, - }, - output: testOutput{ - response: &Response{ - MetaOpts: &MetaOptions{ - BucketOwner: "root", - }, - }, - }, - }, - { - name: "successful response", - input: testInput{ - locals: defaultLocals, - body: validRetentionBody, + extraMockErr: s3err.GetAPIError(s3err.ErrNoSuchObjectLockConfiguration), }, output: testOutput{ response: &Response{ @@ -256,15 +238,14 @@ func TestS3ApiController_PutObjectRetention(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { be := &BackendMock{ - PutObjectRetentionFunc: func(contextMoqParam context.Context, bucket, object, versionId string, bypass bool, retention []byte) error { + PutObjectRetentionFunc: func(contextMoqParam context.Context, bucket, object, versionId string, retention []byte) error { return tt.input.beErr }, GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) { - if tt.input.extraMockResp == nil { - return nil, tt.input.extraMockErr - } else { - return tt.input.extraMockResp.([]byte), tt.input.extraMockErr - } + return nil, s3err.GetAPIError(s3err.ErrAccessDenied) + }, + GetObjectRetentionFunc: func(contextMoqParam context.Context, bucket, object, versionId string) ([]byte, error) { + return nil, tt.input.extraMockErr }, } diff --git a/s3err/s3err.go b/s3err/s3err.go index 65f1c5b..84b651f 100644 --- a/s3err/s3err.go +++ b/s3err/s3err.go @@ -553,9 +553,9 @@ var errorCodeResponse = map[ErrorCode]APIError{ HTTPStatusCode: http.StatusConflict, }, ErrObjectLocked: { - Code: "InvalidRequest", - Description: "Object is WORM protected and cannot be overwritten.", - HTTPStatusCode: http.StatusBadRequest, + Code: "AccessDenied", + Description: "Access Denied because object protected by object lock.", + HTTPStatusCode: http.StatusForbidden, }, ErrPastObjectLockRetainDate: { Code: "InvalidRequest", diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index a0822fc..0822cbc 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -619,6 +619,8 @@ func TestPutObjectRetention(s *S3Conf) { PutObjectRetention_expired_retain_until_date(s) PutObjectRetention_invalid_mode(s) PutObjectRetention_overwrite_compliance_mode(s) + PutObjectRetention_overwrite_compliance_with_compliance(s) + PutObjectRetention_overwrite_governance_with_governance(s) PutObjectRetention_overwrite_governance_without_bypass_specified(s) PutObjectRetention_overwrite_governance_with_permission(s) PutObjectRetention_success(s) @@ -1440,6 +1442,8 @@ func GetIntTests() IntTests { "PutObjectRetention_expired_retain_until_date": PutObjectRetention_expired_retain_until_date, "PutObjectRetention_invalid_mode": PutObjectRetention_invalid_mode, "PutObjectRetention_overwrite_compliance_mode": PutObjectRetention_overwrite_compliance_mode, + "PutObjectRetention_overwrite_compliance_with_compliance": PutObjectRetention_overwrite_compliance_with_compliance, + "PutObjectRetention_overwrite_governance_with_governance": PutObjectRetention_overwrite_governance_with_governance, "PutObjectRetention_overwrite_governance_without_bypass_specified": PutObjectRetention_overwrite_governance_without_bypass_specified, "PutObjectRetention_overwrite_governance_with_permission": PutObjectRetention_overwrite_governance_with_permission, "PutObjectRetention_success": PutObjectRetention_success, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 8471622..f718a92 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -3017,71 +3017,44 @@ func PutObject_missing_object_lock_retention_config(s *S3Conf) error { func PutObject_with_object_lock(s *S3Conf) error { testName := "PutObject_with_object_lock" - runF(testName) - bucket, obj, lockStatus := getBucketName(), "my-obj", true + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + obj := "my-obj" + retainUntilDate := time.Now().AddDate(1, 0, 0) - client := s.GetClient() - ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - _, err := client.CreateBucket(ctx, &s3.CreateBucketInput{ - Bucket: &bucket, - ObjectLockEnabledForBucket: &lockStatus, - }) - cancel() - if err != nil { - failF("%v: %v", testName, err) - return fmt.Errorf("%v: %w", testName, err) - } + _, err := putObjectWithData(10, &s3.PutObjectInput{ + Bucket: &bucket, + Key: &obj, + ObjectLockLegalHoldStatus: types.ObjectLockLegalHoldStatusOn, + ObjectLockMode: types.ObjectLockModeCompliance, + ObjectLockRetainUntilDate: &retainUntilDate, + }, s3client) + if err != nil { + return err + } - retainDate := time.Now().Add(time.Hour * 48) + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.HeadObject(ctx, &s3.HeadObjectInput{ + Bucket: &bucket, + Key: &obj, + }) + cancel() + if err != nil { + return err + } - ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) - _, err = client.PutObject(ctx, &s3.PutObjectInput{ - Bucket: &bucket, - Key: &obj, - ObjectLockLegalHoldStatus: types.ObjectLockLegalHoldStatusOn, - ObjectLockMode: types.ObjectLockModeCompliance, - ObjectLockRetainUntilDate: &retainDate, - }) + if out.ObjectLockMode != types.ObjectLockModeCompliance { + return fmt.Errorf("expected object lock mode to be %v, instead got %v", types.ObjectLockModeCompliance, out.ObjectLockMode) + } + if out.ObjectLockLegalHoldStatus != types.ObjectLockLegalHoldStatusOn { + return fmt.Errorf("expected object lock legal hold status to be %v, instead got %v", types.ObjectLockLegalHoldStatusOn, out.ObjectLockLegalHoldStatus) + } - cancel() - if err != nil { - failF("%v: %v", testName, err) - return fmt.Errorf("%v: %w", testName, err) - } + if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil { + return err + } - ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) - out, err := client.HeadObject(ctx, &s3.HeadObjectInput{ - Bucket: &bucket, - Key: &obj, - }) - cancel() - if err != nil { - failF("%v: %v", testName, err) - return fmt.Errorf("%v: %w", testName, err) - } - - if out.ObjectLockMode != types.ObjectLockModeCompliance { - failF("%v: expected object lock mode to be %v, instead got %v", testName, types.ObjectLockModeCompliance, out.ObjectLockMode) - return fmt.Errorf("%v: expected object lock mode to be %v, instead got %v", testName, types.ObjectLockModeCompliance, out.ObjectLockMode) - } - if out.ObjectLockLegalHoldStatus != types.ObjectLockLegalHoldStatusOn { - failF("%v: expected object lock mode to be %v, instead got %v", testName, types.ObjectLockLegalHoldStatusOn, out.ObjectLockLegalHoldStatus) - return fmt.Errorf("%v: expected object lock mode to be %v, instead got %v", testName, types.ObjectLockLegalHoldStatusOn, out.ObjectLockLegalHoldStatus) - } - - if err := changeBucketObjectLockStatus(client, bucket, false); err != nil { - failF("%v: %v", testName, err) - return fmt.Errorf("%v: %w", testName, err) - } - - err = teardown(s, bucket) - if err != nil { - failF("%v: %v", testName, err) - return fmt.Errorf("%v: %w", testName, err) - } - - passF(testName) - return nil + return nil + }, withLock()) } func PutObject_invalid_legal_hold(s *S3Conf) error { @@ -16420,7 +16393,103 @@ func PutObjectRetention_overwrite_compliance_mode(s *S3Conf) error { }, }) cancel() - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { + return err + } + + if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil { + return err + } + + return nil + }, withLock()) +} + +func PutObjectRetention_overwrite_compliance_with_compliance(s *S3Conf) error { + testName := "PutObjectRetention_overwrite_compliance_with_compliance" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + date := time.Now().Add(time.Hour * 200) + obj := "my-obj" + _, err := putObjects(s3client, []string{obj}, bucket) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{ + Bucket: &bucket, + Key: &obj, + Retention: &types.ObjectLockRetention{ + Mode: types.ObjectLockRetentionModeCompliance, + RetainUntilDate: &date, + }, + }) + cancel() + if err != nil { + return err + } + + newDate := date.AddDate(2, 0, 0) + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{ + Bucket: &bucket, + Key: &obj, + Retention: &types.ObjectLockRetention{ + Mode: types.ObjectLockRetentionModeCompliance, + RetainUntilDate: &newDate, + }, + }) + cancel() + if err != nil { + return err + } + + if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil { + return err + } + + return nil + }, withLock()) +} + +func PutObjectRetention_overwrite_governance_with_governance(s *S3Conf) error { + testName := "PutObjectRetention_overwrite_governance_with_governance" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + date := time.Now().Add(time.Hour * 200) + obj := "my-obj" + _, err := putObjects(s3client, []string{obj}, bucket) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{ + Bucket: &bucket, + Key: &obj, + Retention: &types.ObjectLockRetention{ + Mode: types.ObjectLockRetentionModeGovernance, + RetainUntilDate: &date, + }, + }) + cancel() + if err != nil { + return err + } + + newDate := date.AddDate(2, 0, 0) + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + _, err = s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{ + Bucket: &bucket, + Key: &obj, + Retention: &types.ObjectLockRetention{ + Mode: types.ObjectLockRetentionModeGovernance, + RetainUntilDate: &newDate, + }, + }) + cancel() + if err != nil { return err } @@ -16466,7 +16535,7 @@ func PutObjectRetention_overwrite_governance_without_bypass_specified(s *S3Conf) }, }) cancel() - if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMethodNotAllowed)); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } @@ -18195,15 +18264,9 @@ func WORMProtection_object_lock_legal_hold_locked(s *S3Conf) error { } _, err = putObjects(s3client, []string{object}, bucket) - if err := checkSdkApiErr(err, "InvalidRequest"); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } - // client sdk regression issue prevents getting full error message, - // change back to below once this is fixed: - // https://github.com/aws/aws-sdk-go-v2/issues/2921 - // if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { - // return err - // } if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil { return err @@ -24068,15 +24131,9 @@ func Versioning_WORM_obj_version_locked_with_legal_hold(s *S3Conf) error { VersionId: version.VersionId, }) cancel() - if err := checkSdkApiErr(err, "InvalidRequest"); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } - // client sdk regression issue prevents getting full error message, - // change back to below once this is fixed: - // https://github.com/aws/aws-sdk-go-v2/issues/2921 - // if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { - // return err - // } if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil { return err @@ -24119,15 +24176,9 @@ func Versioning_WORM_obj_version_locked_with_governance_retention(s *S3Conf) err VersionId: version.VersionId, }) cancel() - if err := checkSdkApiErr(err, "InvalidRequest"); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } - // client sdk regression issue prevents getting full error message, - // change back to below once this is fixed: - // https://github.com/aws/aws-sdk-go-v2/issues/2921 - // if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { - // return err - // } if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil { return err @@ -24170,15 +24221,9 @@ func Versioning_WORM_obj_version_locked_with_compliance_retention(s *S3Conf) err VersionId: version.VersionId, }) cancel() - if err := checkSdkApiErr(err, "InvalidRequest"); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } - // client sdk regression issue prevents getting full error message, - // change back to below once this is fixed: - // https://github.com/aws/aws-sdk-go-v2/issues/2921 - // if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { - // return err - // } if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil { return err diff --git a/tests/integration/utils.go b/tests/integration/utils.go index 6367e84..21d6ede 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1139,15 +1139,9 @@ func checkWORMProtection(client *s3.Client, bucket, object string) error { Key: &object, }) cancel() - if err := checkSdkApiErr(err, "InvalidRequest"); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } - // client sdk regression issue prevents getting full error message, - // change back to below once this is fixed: - // https://github.com/aws/aws-sdk-go-v2/issues/2921 - // if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { - // return err - // } ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) _, err = client.DeleteObject(ctx, &s3.DeleteObjectInput{ @@ -1155,15 +1149,9 @@ func checkWORMProtection(client *s3.Client, bucket, object string) error { Key: &object, }) cancel() - if err := checkSdkApiErr(err, "InvalidRequest"); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } - // client sdk regression issue prevents getting full error message, - // change back to below once this is fixed: - // https://github.com/aws/aws-sdk-go-v2/issues/2921 - // if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { - // return err - // } ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) _, err = client.DeleteObjects(ctx, &s3.DeleteObjectsInput{ @@ -1177,15 +1165,9 @@ func checkWORMProtection(client *s3.Client, bucket, object string) error { }, }) cancel() - if err := checkSdkApiErr(err, "InvalidRequest"); err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { return err } - // client sdk regression issue prevents getting full error message, - // change back to below once this is fixed: - // https://github.com/aws/aws-sdk-go-v2/issues/2921 - // if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil { - // return err - // } return nil }