RGW Zipper - clean up setting bucket on s->object

s->object is always created wihtout a bucket, and the bucket is added
later.  The proper place for this is in rgw_build_bucket_policies(), in
the permissions handling callpaths.  Remove all the other places where
bucket is set, ensuring that s->object has a stable bucket pointer.

Signed-off-by: Daniel Gryniewicz <dang@redhat.com>

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 66ec854a33b1aced3f8f8ecc0b9fb5759970886e..99c3d36d236436436e340c6fdc5653e2ff04ed44 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -664,8 +664,6 @@ int rgw_build_object_policies(const DoutPrefixProvider *dpp, rgw::sal::Store* st
     }
     s->object_acl = std::make_unique<RGWAccessControlPolicy>(s->cct);
 
-    s->object->set_bucket(s->bucket.get());
-      
     s->object->set_atomic(s->obj_ctx);
     if (prefetch_data) {
       s->object->set_prefetch_data(s->obj_ctx);
@@ -3638,9 +3636,6 @@ int RGWPutObj::verify_permission(optional_yield y)
       rgw_add_to_iam_environment(s->env, s3_kms_attr, kms_header->second);
     }
 
-    /* Object needs a bucket from this point */
-    s->object->set_bucket(s->bucket.get());
-
     // Add bucket tags for authorization
     auto [has_s3_existing_tag, has_s3_resource_tag] = rgw_check_policy_condition(this, s, false);
     if (has_s3_resource_tag)
@@ -3853,9 +3848,6 @@ void RGWPutObj::execute(optional_yield y)
     return;
   }
 
-  /* Object needs a bucket from this point */
-  s->object->set_bucket(s->bucket.get());
-
   op_ret = get_system_versioning_params(s, &olh_epoch, &version_id);
   if (op_ret < 0) {
     ldpp_dout(this, 20) << "get_system_versioning_params() returned ret="
@@ -4904,8 +4896,6 @@ void RGWDeleteObj::execute(optional_yield y)
     op_ret = -ERR_NO_SUCH_BUCKET;
     return;
   }
-  s->object->set_bucket(s->bucket.get());
-
 
   if (!rgw::sal::Object::empty(s->object.get())) {
     uint64_t obj_size = 0;
@@ -5111,6 +5101,7 @@ int RGWCopyObj::verify_permission(optional_yield y)
     return op_ret;
   }
 
+  /* This is the only place the bucket is set on src_object */
   s->src_object->set_bucket(src_bucket.get());
   /* get buckets info (source and dest) */
   if (s->local_source &&  source_zone.empty()) {
@@ -5225,8 +5216,7 @@ int RGWCopyObj::verify_permission(optional_yield y)
     }
   }
 
-  dest_object = store->get_object(rgw_obj_key(dest_obj_name));
-  dest_object->set_bucket(dest_bucket.get());
+  dest_object = dest_bucket->get_object(rgw_obj_key(dest_obj_name));
   dest_object->set_atomic(s->obj_ctx);
 
   /* check dest bucket permissions */
@@ -6353,7 +6343,6 @@ void RGWCompleteMultipart::execute(optional_yield y)
 bool RGWCompleteMultipart::check_previously_completed(const RGWMultiCompleteUpload* parts)
 {
   // re-calculate the etag from the parts and compare to the existing object
-  s->object->set_bucket(s->bucket.get());
   int ret = s->object->get_obj_attrs(s->obj_ctx, s->yield, this);
   if (ret < 0) {
     ldpp_dout(this, 0) << __func__ << "() ERROR: get_obj_attrs() returned ret=" << ret << dendl;
@@ -7731,8 +7720,6 @@ void RGWGetObjLayout::pre_exec()
 
 void RGWGetObjLayout::execute(optional_yield y)
 {
-  /* Make sure bucket is correct */
-  s->object->set_bucket(s->bucket.get());
 }
 
 

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index e4191f0c9888afd7c73125b6b5ee59528ec9461c..73f3ed5729a3f0bab8dc01b19c5d06a54115a38b 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -4654,6 +4654,8 @@ int RGWHandler_REST_S3::init_from_header(rgw::sal::Store* store,
       encoded_obj_str = req.substr(pos+1);
     }
 
+    /* dang: s->bucket is never set here, since it's created with permissions.
+     * These calls will always create an object with no bucket. */
     if (!encoded_obj_str.empty()) {
       if (s->bucket) {
        s->object = s->bucket->get_object(rgw_obj_key(encoded_obj_str, s->info.args.get("versionId")));