rgw/sfs: Don't return version id for unversioned buckets

Avoid to return the version id when getting the object from the database
 when it wasn't explicitly required.

A few tags s3tests were failing because, for example:
```c++
int RGWDeleteObjTags::verify_permission(optional_yield y)
{
  if (!rgw::sal::Object::empty(s->object.get())) {
    auto iam_action = s->object->get_instance().empty() ?
      rgw::IAM::s3DeleteObjectTagging:
      rgw::IAM::s3DeleteObjectVersionTagging;
```
If `object->get_instance()` is not empty it changes the action to be
looked for to `DeleteObjectVersionTagging`.

The s3test is running in an unversioned bucket and setting the
`DeleteObjectTagging` action to be allowed, but it was later looking for
a different one.

Fixes: https://github.com/aquarist-labs/s3gw/issues/675
Fixes: https://github.com/aquarist-labs/s3gw/issues/695
Signed-off-by: Xavi Garcia <[email protected]>

qa/rgw/store/sfs/tests/fixtures/s3-tests.txt

@@ -298,7 +298,7 @@ test_100_continue
 # test_cors_origin_response
 test_cors_origin_wildcard
 test_cors_header_option
-# test_set_bucket_tagging
+test_set_bucket_tagging
 test_atomic_read_1mb
 test_atomic_read_4mb
 test_atomic_read_8mb
@@ -356,9 +356,9 @@ test_lifecycle_set_invalid_date
 test_lifecycle_expiration_date
 test_lifecycle_expiration_days0
 test_lifecycle_expiration_header_put
-# test_lifecycle_expiration_header_head
-# test_lifecycle_expiration_header_tags_head
-# test_lifecycle_expiration_header_and_tags_head
+test_lifecycle_expiration_header_head
+test_lifecycle_expiration_header_tags_head
+test_lifecycle_expiration_header_and_tags_head
 test_lifecycle_set_noncurrent
 test_lifecycle_noncur_expiration
 test_lifecycle_set_deletemarker
@@ -414,26 +414,26 @@ test_bucketv2_policy_different_tenant
 test_bucket_policy_another_bucket
 test_bucketv2_policy_another_bucket
 # test_bucket_policy_set_condition_operator_end_with_IfExists
-# test_get_obj_tagging
-# test_get_obj_head_tagging
-# test_put_max_tags
+test_get_obj_tagging
+test_get_obj_head_tagging
+test_put_max_tags
 test_put_excess_tags
-# test_put_max_kvsize_tags
+test_put_max_kvsize_tags
 test_put_excess_key_tags
 test_put_excess_val_tags
-# test_put_modify_tags
-# test_put_delete_tags
-# test_post_object_tags_anonymous_request
+test_put_modify_tags
+test_put_delete_tags
+test_post_object_tags_anonymous_request
 test_post_object_tags_authenticated_request
-# test_put_obj_with_tags
-# test_get_tags_acl_public
-# test_put_tags_acl_public
-# test_delete_tags_obj_public
+test_put_obj_with_tags
+test_get_tags_acl_public
+test_put_tags_acl_public
+test_delete_tags_obj_public
 # test_versioning_bucket_atomic_upload_return_version_id (https://github.com/aquarist-labs/s3gw/issues/534)
 # test_versioning_bucket_multipart_upload_return_version_id (https://github.com/aquarist-labs/s3gw/issues/534)
-# test_bucket_policy_get_obj_existing_tag
-# test_bucket_policy_get_obj_tagging_existing_tag
-# test_bucket_policy_put_obj_tagging_existing_tag
+test_bucket_policy_get_obj_existing_tag
+test_bucket_policy_get_obj_tagging_existing_tag
+test_bucket_policy_put_obj_tagging_existing_tag
 # test_bucket_policy_put_obj_copy_source
 # test_bucket_policy_put_obj_copy_source_meta
 # test_bucket_policy_put_obj_acl
@@ -447,7 +447,7 @@ test_post_object_tags_authenticated_request
 # test_bucket_policy_put_obj_kms_noenc
 # test_bucket_policy_put_obj_kms_s3
 # test_bucket_policy_put_obj_request_obj_tag
-# test_bucket_policy_get_obj_acl_existing_tag
+test_bucket_policy_get_obj_acl_existing_tag
 # test_object_lock_put_obj_lock
 test_object_lock_put_obj_lock_invalid_bucket
 # test_object_lock_put_obj_lock_with_days_and_years

qa/rgw/store/sfs/tests/fixtures/s3tr_excuses.csv

@@ -7,22 +7,14 @@ test_bucket_list_delimiter_not_skip_special;https://github.com/aquarist-labs/s3g
 test_bucket_list_delimiter_prefix_underscore;https://github.com/aquarist-labs/s3gw/issues/691;BUG
 test_bucket_listv2_delimiter_prefix_underscore;https://github.com/aquarist-labs/s3gw/issues/691;BUG
 test_bucket_policy_different_tenant;https://github.com/ceph/s3-tests/issues;Broken test, marked fails_on_rgw
-test_bucket_policy_get_obj_acl_existing_tag;https://github.com/aquarist-labs/s3gw/issues/698;BUG
-test_bucket_policy_get_obj_existing_tag;https://github.com/aquarist-labs/s3gw/issues/698;BUG
-test_bucket_policy_get_obj_tagging_existing_tag;https://github.com/aquarist-labs/s3gw/issues/698;BUG
 test_bucket_policy_put_obj_copy_source;https://github.com/aquarist-labs/s3gw/issues/698;BUG
 test_bucket_policy_put_obj_copy_source_meta;https://github.com/aquarist-labs/s3gw/issues/698;BUG
 test_bucket_policy_put_obj_request_obj_tag;https://github.com/aquarist-labs/s3gw/issues/698;BUG
 test_bucket_policy_put_obj_s3_noenc;https://github.com/aquarist-labs/s3gw/issues/686;No external keystore support in s3tr
 test_object_copy_canned_acl;https://github.com/aquarist-labs/s3gw/issues/686;No external keystore support in s3tr
-test_bucket_policy_put_obj_tagging_existing_tag;https://github.com/aquarist-labs/s3gw/issues/698;BUG
 test_bucket_policy_set_condition_operator_end_with_IfExists;https://github.com/ceph/s3-tests/issues;Broken test, marked fails_on_rgw
 test_bucket_recreate_new_acl;https://github.com/aquarist-labs/s3gw/issues/617;BUG
 test_bucket_recreate_overwrite_acl;https://github.com/aquarist-labs/s3gw/issues/617;BUG
-test_delete_tags_obj_public;https://github.com/aquarist-labs/s3gw/issues/675;BUG
-test_get_tags_acl_public;https://github.com/aquarist-labs/s3gw/issues/675;BUG
-test_lifecycle_expiration_header_head;https://github.com/aquarist-labs/s3gw/issues/695;BUG
-test_lifecycle_expiration_header_tags_head;https://github.com/aquarist-labs/s3gw/issues/695;BUG
 test_logging_toggle;https://tracker.ceph.com/issues/984;Not supported by RGW
 test_object_copy_replacing_metadata;https://github.com/aquarist-labs/s3gw/issues/525;BUG
 test_object_copy_to_itself_with_metadata;https://github.com/aquarist-labs/s3gw/issues/525;BUG
@@ -35,7 +27,6 @@ test_put_object_ifmatch_failed;https://github.com/aquarist-labs/s3gw/issues/674;
 test_put_object_ifmatch_nonexisted_failed;https://github.com/aquarist-labs/s3gw/issues/674;Unsupported RGW Extension
 test_put_object_ifnonmatch_failed;https://github.com/aquarist-labs/s3gw/issues/674;Unsupported RGW Extension
 test_put_object_ifnonmatch_overwrite_existed_failed;https://github.com/aquarist-labs/s3gw/issues/674;Unsupported RGW Extension
-test_put_tags_acl_public;https://github.com/aquarist-labs/s3gw/issues/675;BUG
 test_sse_s3_default_method_head;https://github.com/aquarist-labs/s3gw/issues/686;No external keystore support in s3tr
 test_sse_s3_default_multipart_upload;https://github.com/aquarist-labs/s3gw/issues/686;No external keystore support in s3tr
 test_sse_s3_default_post_object_authenticated_request;https://github.com/aquarist-labs/s3gw/issues/686;No external keystore support in s3tr

qa/rgw/store/sfs/tests/test-sfs-multipart.py

@@ -168,8 +168,6 @@ def verify_multipart_response(self, response, nb_parts, objsize) -> None:
         self.assertGreaterEqual(now.year, response["LastModified"].year)
         self.assertTrue("ContentType" in response)
         self.assertEqual("binary/octet-stream", response["ContentType"])
-        self.assertTrue("VersionId" in response)
-        self.assertNotEqual("", response["VersionId"])
         self.assertTrue("ContentLength" in response)
         self.assertEqual(objsize, response["ContentLength"])
 

src/rgw/driver/sfs/types.cc

@@ -149,6 +149,12 @@ Object* Object::try_fetch_from_database(
     return nullptr;
   }
 
+  // don't return the version_id if versioning is not enabled
+  // and the user didn't ask for a specific version.
+  // Returning the version_id when no required breaks s3tests related to tags
+  if (!versioning_enabled && version_id.empty()) {
+    version->version_id.clear();
+  }
   auto result =
       new Object(rgw_obj_key(name, version->version_id), version->object_id);
   result->deleted = (version->version_type == VersionType::DELETE_MARKER);