rgw/logging: fix partitioned key format

Fixes: https://tracker.ceph.com/issues/71537
Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
(cherry picked from commit 500534382e708e055fe248ca567639fe66e58450)

diff --git a/doc/radosgw/bucket_logging.rst b/doc/radosgw/bucket_logging.rst
index bffc9f0374df7e6420c84f073f2306627d467f7e..58da7caa8e141daaf12acfad4479d8e043d98db3 100644 (file)
--- a/doc/radosgw/bucket_logging.rst
+++ b/doc/radosgw/bucket_logging.rst
@@ -153,13 +153,13 @@ has the following format:
 
 ::
 
-  <prefix><bucket owner>/<source region>/[tenant:]<bucket name>/<year>/<month>/<day>/<year-month-day-hour-minute-second>-<16 bytes unique-id>
+  <prefix><source bucket owner>/<zone group>/[tenant:]<source bucket name>/<year>/<month>/<day>/<year-month-day-hour-minute-second>-<16 bytes unique-id>
 
 For example:
 
 ::
 
-  fish/testid//all-log/2024/08/06/2024-08-06-10-11-18-0000000000000002
+  fish/testid/default/fish-bucket/2024/08/06/2024-08-06-10-11-18-0000000000000002
 
 Log Records
 ~~~~~~~~~~~

diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc
index 3a474948e0b6a5acd7919e66826524f2eed2aa3e..488379700ba73d99e3e888dfe26b61f327d5e8fd 100644 (file)
--- a/src/rgw/radosgw-admin/radosgw-admin.cc
+++ b/src/rgw/radosgw-admin/radosgw-admin.cc
@@ -7809,7 +7809,8 @@ int main(int argc, const char **argv)
       return -ret;
     }
     const auto old_obj = obj_name;
-    ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), null_yield, true, &objv_tracker);
+    const auto region = driver->get_zone()->get_zonegroup().get_api_name();
+    ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), region, bucket, null_yield, true, &objv_tracker);
     if (ret < 0) {
       cerr << "ERROR: failed to flush pending logging object '" << old_obj
         << "' to target bucket '" << configuration.target_bucket << "'" << std::endl;

diff --git a/src/rgw/rgw_bucket_logging.cc b/src/rgw/rgw_bucket_logging.cc
index aa39a509248d211fc4d2105439cecade70d17678..94ba1187d443d99a8fbf45645b7a2eb4cb94a85b 100644 (file)
--- a/src/rgw/rgw_bucket_logging.cc
+++ b/src/rgw/rgw_bucket_logging.cc
@@ -242,6 +242,8 @@ int new_logging_object(const configuration& conf,
     const std::unique_ptr<rgw::sal::Bucket>& target_bucket,
     std::string& obj_name,
     const DoutPrefixProvider *dpp,
+    const std::string& region,
+    const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
     optional_yield y,
     std::optional<std::string> old_name,
     RGWObjVersionTracker* objv_tracker) {
@@ -253,6 +255,7 @@ int new_logging_object(const configuration& conf,
 
   switch (conf.obj_key_format) {
     case KeyFormat::Simple:
+      // [DestinationPrefix][YYYY]-[MM]-[DD]-[hh]-[mm]-[ss]-[UniqueString]
       obj_name = fmt::format("{}{:%Y-%m-%d-%H-%M-%S}-{}",
         conf.target_prefix,
         t,
@@ -260,13 +263,13 @@ int new_logging_object(const configuration& conf,
       break;
     case KeyFormat::Partitioned:
       {
-        // TODO: use date_source
-        const auto source_region = ""; // TODO
+        // TODO: support both EventTime and DeliveryTime
+        // [DestinationPrefix][SourceAccountId]/[SourceRegion]/[SourceBucket]/[YYYY]/[MM]/[DD]/[YYYY]-[MM]-[DD]-[hh]-[mm]-[ss]-[UniqueString]
         obj_name = fmt::format("{}{}/{}/{}/{:%Y/%m/%d}/{:%Y-%m-%d-%H-%M-%S}-{}",
           conf.target_prefix,
-          to_string(target_bucket->get_owner()),
-          source_region,
-          full_bucket_name(target_bucket),
+          to_string(source_bucket->get_owner()),
+          region,
+          full_bucket_name(source_bucket),
           t,
           t,
           unique);
@@ -341,6 +344,8 @@ int rollover_logging_object(const configuration& conf,
     const std::unique_ptr<rgw::sal::Bucket>& target_bucket,
     std::string& obj_name,
     const DoutPrefixProvider *dpp,
+    const std::string& region,
+    const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
     optional_yield y,
     bool must_commit,
     RGWObjVersionTracker* objv_tracker) {
@@ -353,7 +358,7 @@ int rollover_logging_object(const configuration& conf,
     return -EINVAL;
   }
   const auto old_obj = obj_name;
-  const int ret = new_logging_object(conf, target_bucket, obj_name, dpp, y, old_obj, objv_tracker);
+  const int ret = new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker);
   if (ret == -ECANCELED) {
     ldpp_dout(dpp, 20) << "INFO: rollover already performed for object '" << old_obj <<  "' to logging bucket '" <<
       target_bucket->get_key() << "'. ret = " << ret << dendl;
@@ -462,6 +467,7 @@ int log_record(rgw::sal::Driver* driver,
     return ret;
   }
 
+  const auto region = driver->get_zone()->get_zonegroup().get_api_name();
   std::string obj_name;
   RGWObjVersionTracker objv_tracker;
   ret = target_bucket->get_logging_object_name(obj_name, conf.target_prefix, y, dpp, &objv_tracker);
@@ -470,7 +476,7 @@ int log_record(rgw::sal::Driver* driver,
     if (ceph::coarse_real_time::clock::now() > time_to_commit) {
       ldpp_dout(dpp, 20) << "INFO: logging object '" << obj_name << "' exceeded its time, will be committed to bucket '" <<
         target_bucket_id << "'" << dendl;
-      if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, y, false, &objv_tracker); ret < 0) {
+      if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, false, &objv_tracker); ret < 0) {
         return ret;
       }
     } else {
@@ -478,7 +484,7 @@ int log_record(rgw::sal::Driver* driver,
     }
   } else if (ret == -ENOENT) {
     // try to create the temporary log object for the first time
-    ret = new_logging_object(conf, target_bucket, obj_name, dpp, y, std::nullopt, nullptr);
+    ret = new_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, std::nullopt, nullptr);
     if (ret == 0) {
       ldpp_dout(dpp, 20) << "INFO: first time logging for bucket '" << target_bucket_id << "' and prefix '" <<
         conf.target_prefix << "'" << dendl;
@@ -509,9 +515,13 @@ int log_record(rgw::sal::Driver* driver,
     fqdn.append(".").append(s->info.domain);
   }
 
+  std::string aws_version("-");
+  std::string auth_type("-");
+  rgw::auth::s3::get_aws_version_and_auth_type(s, aws_version, auth_type);
   std::string bucket_owner;
   std::string bucket_name;
-  if (log_source_bucket) {
+  if (log_source_bucket && conf.logging_type == LoggingType::Standard) {
+    // log source bucket for COPY operations only in standard mode
     if (!s->src_object || !s->src_object->get_bucket()) {
       ldpp_dout(dpp, 1) << "ERROR: source object or bucket is missing when logging source bucket" << dendl;
       return -EINVAL;
@@ -523,9 +533,6 @@ int log_record(rgw::sal::Driver* driver,
     bucket_name = full_bucket_name(s->bucket);
   }
 
-  std::string aws_version("-");
-  std::string auth_type("-");
-  rgw::auth::s3::get_aws_version_and_auth_type(s, aws_version, auth_type);
 
   switch (conf.logging_type) {
     case LoggingType::Standard:
@@ -562,8 +569,8 @@ int log_record(rgw::sal::Driver* driver,
       break;
     case LoggingType::Journal:
       record = fmt::format("{} {} [{:%d/%b/%Y:%H:%M:%S %z}] {} {} {} {} {}",
-        dash_if_empty(to_string(s->bucket->get_owner())),
-        dash_if_empty(full_bucket_name(s->bucket)),
+        dash_if_empty(bucket_owner),
+        dash_if_empty(bucket_name),
         t,
         op_name,
         dash_if_empty_or_null(obj, obj->get_name()),
@@ -615,7 +622,7 @@ int log_record(rgw::sal::Driver* driver,
   if (ret == -EFBIG) {
     ldpp_dout(dpp, 5) << "WARNING: logging object '" << obj_name << "' is full, will be committed to bucket '" <<
       target_bucket->get_key() << "'" << dendl;
-    if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, y, true, nullptr); ret < 0 ) {
+    if (ret = rollover_logging_object(conf, target_bucket, obj_name, dpp, region, s->bucket, y, true, nullptr); ret < 0 ) {
       return ret;
     }
     if (ret = target_bucket->write_logging_object(obj_name,

diff --git a/src/rgw/rgw_bucket_logging.h b/src/rgw/rgw_bucket_logging.h
index 25eb3261600a4debb628e0374b63941bda671e03..0b7a9c75423be7ba9b1e7b0ce53e57aadeda0c98 100644 (file)
--- a/src/rgw/rgw_bucket_logging.h
+++ b/src/rgw/rgw_bucket_logging.h
@@ -176,6 +176,8 @@ int rollover_logging_object(const configuration& conf,
     const std::unique_ptr<rgw::sal::Bucket>& bucket,
     std::string& obj_name,
     const DoutPrefixProvider *dpp,
+    const std::string& region,
+    const std::unique_ptr<rgw::sal::Bucket>& source_bucket,
     optional_yield y,
     bool must_commit,
     RGWObjVersionTracker* objv_tracker);

diff --git a/src/rgw/rgw_rest_bucket_logging.cc b/src/rgw/rgw_rest_bucket_logging.cc
index 534cc15b1cad273910622828481e4ff4a0b90c02..4564d5accff5ef2447058fa7f9e3cfa049b94fc5 100644 (file)
--- a/src/rgw/rgw_rest_bucket_logging.cc
+++ b/src/rgw/rgw_rest_bucket_logging.cc
@@ -340,22 +340,22 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
   // and usd in execute()
   rgw::bucketlogging::configuration configuration;
   std::unique_ptr<rgw::sal::Bucket> target_bucket;
+  std::unique_ptr<rgw::sal::Bucket> source_bucket;
 
   int init_processing(optional_yield y) override {
     if (const auto ret = verify_bucket_logging_params(this, s); ret < 0) {
       return ret;
     }
 
-    std::unique_ptr<rgw::sal::Bucket> src_bucket;
     {
       const rgw_bucket src_bucket_id{s->bucket_tenant, s->bucket_name};
       if (const auto ret = driver->load_bucket(this, src_bucket_id,
-                                 &src_bucket, y); ret < 0) {
+                                 &source_bucket, y); ret < 0) {
         ldpp_dout(this, 1) << "ERROR: failed to get bucket '" << src_bucket_id << "', ret = " << ret << dendl;
         return ret;
       }
     }
-    return rgw::bucketlogging::get_target_and_conf_from_source(this, driver, src_bucket.get(), s->bucket_tenant, configuration, target_bucket, y);
+    return rgw::bucketlogging::get_target_and_conf_from_source(this, driver, source_bucket.get(), s->bucket_tenant, configuration, target_bucket, y);
   }
 
   int verify_permission(optional_yield y) override {
@@ -385,7 +385,8 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
       return;
     }
     const auto old_obj = obj_name;
-    op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, null_yield, true, &objv_tracker);
+    const auto region = driver->get_zone()->get_zonegroup().get_api_name();
+    op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, null_yield, true, &objv_tracker);
     if (op_ret < 0) {
       ldpp_dout(this, 1) << "ERROR: failed to flush pending logging object '" << old_obj
                << "' to target bucket '" << target_bucket_id << "'" << dendl;