From ea9807ce1aa4016e4a2c46db6e7763c85e9ca80e Mon Sep 17 00:00:00 2001 From: Evgenii Baidakov Date: Mon, 30 Oct 2023 16:56:34 +0400 Subject: [PATCH 1/2] api: Return correct error on lock configuration check Closes #869. Signed-off-by: Evgenii Baidakov --- api/handler/locking.go | 10 +++++----- api/handler/locking_test.go | 5 +++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/api/handler/locking.go b/api/handler/locking.go index 94718d63..4be530bf 100644 --- a/api/handler/locking.go +++ b/api/handler/locking.go @@ -278,7 +278,7 @@ func (h *handler) GetObjectRetentionHandler(w http.ResponseWriter, r *http.Reque func checkLockConfiguration(conf *data.ObjectLockConfiguration) error { if conf.ObjectLockEnabled != "" && conf.ObjectLockEnabled != enabledValue { - return fmt.Errorf("invalid ObjectLockEnabled value: %s", conf.ObjectLockEnabled) + return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid ObjectLockEnabled value: %s", conf.ObjectLockEnabled)) } if conf.Rule == nil || conf.Rule.DefaultRetention == nil { @@ -287,15 +287,15 @@ func checkLockConfiguration(conf *data.ObjectLockConfiguration) error { retention := conf.Rule.DefaultRetention if retention.Mode != governanceMode && retention.Mode != complianceMode { - return fmt.Errorf("invalid Mode value: %s", retention.Mode) + return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid Mode value: %s", retention.Mode)) } - if retention.Days == 0 && retention.Years == 0 { - return fmt.Errorf("you must specify Days or Years") + if retention.Days <= 0 && retention.Years <= 0 { + return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("you must specify Days or Years")) } if retention.Days != 0 && retention.Years != 0 { - return fmt.Errorf("you cannot specify Days and Years at the same time") + return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("you cannot specify Days and Years at the same time")) } return nil diff --git a/api/handler/locking_test.go b/api/handler/locking_test.go index 2f22c5ff..7878cbee 100644 --- a/api/handler/locking_test.go +++ b/api/handler/locking_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/xml" + "fmt" "net/http" "net/http/httptest" "strconv" @@ -282,9 +283,9 @@ func TestPutBucketLockConfigurationHandler(t *testing.T) { expectedError: s3errors.GetAPIError(s3errors.ErrObjectLockConfigurationNotAllowed), }, { - name: "invalid configuration", + name: "invalid ObjectLockEnabled", bucket: bktLockEnabled, - expectedError: s3errors.GetAPIError(s3errors.ErrInternalError), + expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid ObjectLockEnabled value: %s", "dummy")), configuration: &data.ObjectLockConfiguration{ObjectLockEnabled: "dummy"}, }, { From a6a90941c7ad178be45772054ba134f5b222a98f Mon Sep 17 00:00:00 2001 From: Evgenii Baidakov Date: Mon, 20 Nov 2023 11:36:58 +0400 Subject: [PATCH 2/2] api: Extend tests for the object lock retention Signed-off-by: Evgenii Baidakov --- api/handler/locking.go | 10 ++++++++-- api/handler/locking_test.go | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/api/handler/locking.go b/api/handler/locking.go index 4be530bf..1d8c867d 100644 --- a/api/handler/locking.go +++ b/api/handler/locking.go @@ -3,6 +3,7 @@ package handler import ( "context" "encoding/xml" + "errors" "fmt" "net/http" "strconv" @@ -25,6 +26,11 @@ const ( legalHoldOff = "OFF" ) +var ( + errEmptyDaysErrors = errors.New("you must specify Days or Years") + errNonEmptyDaysErrors = errors.New("you cannot specify Days and Years at the same time") +) + func (h *handler) PutBucketObjectLockConfigHandler(w http.ResponseWriter, r *http.Request) { reqInfo := api.GetReqInfo(r.Context()) @@ -291,11 +297,11 @@ func checkLockConfiguration(conf *data.ObjectLockConfiguration) error { } if retention.Days <= 0 && retention.Years <= 0 { - return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("you must specify Days or Years")) + return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errEmptyDaysErrors) } if retention.Days != 0 && retention.Years != 0 { - return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("you cannot specify Days and Years at the same time")) + return s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errNonEmptyDaysErrors) } return nil diff --git a/api/handler/locking_test.go b/api/handler/locking_test.go index 7878cbee..89fd3ee1 100644 --- a/api/handler/locking_test.go +++ b/api/handler/locking_test.go @@ -288,6 +288,24 @@ func TestPutBucketLockConfigurationHandler(t *testing.T) { expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid ObjectLockEnabled value: %s", "dummy")), configuration: &data.ObjectLockConfiguration{ObjectLockEnabled: "dummy"}, }, + { + name: "invalid retention mode", + bucket: bktLockEnabled, + expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, fmt.Errorf("invalid Mode value: %s", "dummy")), + configuration: &data.ObjectLockConfiguration{Rule: &data.ObjectLockRule{DefaultRetention: &data.DefaultRetention{Mode: "dummy"}}}, + }, + { + name: "empty retention days and years", + bucket: bktLockEnabled, + expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errEmptyDaysErrors), + configuration: &data.ObjectLockConfiguration{Rule: &data.ObjectLockRule{DefaultRetention: &data.DefaultRetention{Mode: complianceMode}}}, + }, + { + name: "non empty retention days and years", + bucket: bktLockEnabled, + expectedError: s3errors.GetAPIErrorWithError(s3errors.ErrMalformedXML, errNonEmptyDaysErrors), + configuration: &data.ObjectLockConfiguration{Rule: &data.ObjectLockRule{DefaultRetention: &data.DefaultRetention{Mode: complianceMode, Days: 1, Years: 1}}}, + }, { name: "basic", bucket: bktLockEnabled,