Merge pull request #1560 from versity/sis/object-retention-err

fix: fixes PutObjectRetention error cases and object lock error code/message.
This commit is contained in:
Ben McClelland
2025-09-25 15:12:14 -07:00
committed by GitHub
14 changed files with 289 additions and 256 deletions
+4 -10
View File
@@ -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()
+22 -13
View File
@@ -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,
+27 -46
View File
@@ -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
},
}