rgw/lc: report x-amz-expiration only for the current version

The x-amz-expiration response header describes the current-version
Expiration lifecycle action for an object. s3_expiration_header() instead
selected between the Expiration and the NoncurrentVersionExpiration rule
based on whether the object key carried an instance id:

  const LCExpiration& rule_expiration =
    (obj_key.instance.empty()) ? expiration : noncur_expiration;

This was wrong in two ways. OLH resolution writes the current version's
instance id into the key before the header is computed, so even a plain
GET/HEAD of the current version (no versionId) was treated as a
non-current request and reported a NoncurrentVersionExpiration rule. And
x-amz-expiration is not meant to surface NoncurrentVersionExpiration at
all. A versioned bucket whose only rule was NoncurrentVersionExpiration
therefore returned a bogus expiry-date and rule-id on the current object,
for both a plain request and a versionId request.

Compute the header from the current-version Expiration rule only, and
return no header when the request targets a specific version. The header
helper now passes the client request key (s->object_key) rather than the
OLH-resolved key, so a plain GET/HEAD of the current version is no longer
misclassified as a versioned request.

Add a unit test covering a noncurrent-only policy (no header for either a
plain or a versioned request) and a combined policy (the current
Expiration rule for the current version, no header for a versionId
request).

Tracker: https://tracker.ceph.com/issues/77564
Reported-by: Saravanan Palanisamy
Signed-off-by: Lumir Sliva <61183145+lumir-sliva@users.noreply.github.com>

diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc
index fdfbbc5abf25defff22e057acce7d7df187024a6..b9383cd7a49358c6f8b7b5b255a383d69d31bfd9 100644 (file)
--- a/src/rgw/rgw_lc.cc
+++ b/src/rgw/rgw_lc.cc
@@ -3239,6 +3239,14 @@ std::string s3_expiration_header(
   RGWLifecycleConfiguration config(cct);
   std::string hdr{""};
 
+  /* The x-amz-expiration header reports the current-version Expiration
+   * lifecycle action for the object. It must never be derived from a
+   * NoncurrentVersionExpiration rule, and it does not apply to a request that
+   * targets a specific version: a non-empty instance here means the caller
+   * asked for a particular version by versionId, so no header is returned. */
+  if (!obj_key.instance.empty())
+    return hdr;
+
   const auto& aiter = bucket_attrs.find(RGW_ATTR_LC);
   if (aiter == bucket_attrs.end())
     return hdr;
@@ -3273,16 +3281,12 @@ std::string s3_expiration_header(
     auto& filter = rule.get_filter();
     auto& prefix = filter.has_prefix() ? filter.get_prefix(): rule.get_prefix();
     auto& expiration = rule.get_expiration();
-    auto& noncur_expiration = rule.get_noncur_expiration();
 
     ldpp_dout(dpp, 10) << "rule: " << ri.first
                       << " prefix: " << prefix
                       << " expiration: "
                       << " date: " << expiration.get_date()
                       << " days: " << expiration.get_days()
-                      << " noncur_expiration: "
-                      << " date: " << noncur_expiration.get_date()
-                      << " days: " << noncur_expiration.get_days()
                       << dendl;
 
     /* skip if rule !enabled
@@ -3324,10 +3328,9 @@ std::string s3_expiration_header(
              continue;
     }
 
-    // compute a uniform expiration date
+    // compute a uniform expiration date (current-version Expiration only)
     boost::optional<ceph::real_time> rule_expiration_date;
-    const LCExpiration& rule_expiration =
-      (obj_key.instance.empty()) ? expiration : noncur_expiration;
+    const LCExpiration& rule_expiration = expiration;
 
     if (rule_expiration.has_date()) {
       rule_expiration_date =

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index e0d1fc47e792fe1c09554b67c003f1155d7ce3d9..3e0b0ec3c45a2570d99913390b17ab2fd164ff8a 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -122,8 +122,14 @@ static inline std::string get_s3_expiration_header(
   req_state* s,
   const ceph::real_time& mtime)
 {
+  /* Use the client-supplied request key (s->object_key), not the
+   * OLH-resolved key. OLH resolution fills in the current version's instance
+   * id even when the client did not request a specific version, which would
+   * otherwise make every versioned-bucket response look like a versionId
+   * request. s3_expiration_header() keys its current-version decision off an
+   * empty instance. */
   return rgw::lc::s3_expiration_header(
-    s, s->object->get_key(), s->tagset, mtime, s->bucket_attrs);
+    s, s->object_key, s->tagset, mtime, s->bucket_attrs);
 }
 
 static inline bool get_s3_multipart_abort_header(

diff --git a/src/test/rgw/test_rgw_lc.cc b/src/test/rgw/test_rgw_lc.cc
index ab5297709afc8acd2b93ad02215c505a2dad7615..ced9bebcee37d36ce32976f9967645cdab79d2f1 100644 (file)
--- a/src/test/rgw/test_rgw_lc.cc
+++ b/src/test/rgw/test_rgw_lc.cc
@@ -4,6 +4,9 @@
 #include "rgw_xml.h"
 #include "rgw_lc.h"
 #include "rgw_lc_s3.h"
+#include "rgw_common.h"
+#include "common/ceph_context.h"
+#include "common/dout.h"
 #include <gtest/gtest.h>
 #include <string>
 #include <vector>
@@ -419,3 +422,78 @@ TEST_F(LCWorkTimeTests, ScheduleNextStartTime)
 
    run_schedule_next_start_time_test(test_values_to_expectations);
 }
+
+// Build bucket attrs holding an encoded lifecycle configuration.
+static std::map<std::string, bufferlist>
+make_lc_attrs(const RGWLifecycleConfiguration& cfg)
+{
+  std::map<std::string, bufferlist> attrs;
+  bufferlist bl;
+  cfg.encode(bl);
+  attrs[RGW_ATTR_LC] = std::move(bl);
+  return attrs;
+}
+
+static LCRule make_rule(const std::string& id, const std::string* exp_days,
+                        const std::string* noncur_days)
+{
+  LCRule rule;
+  rule.set_id(id);
+  rule.set_status("Enabled");
+  if (exp_days) {
+    LCExpiration exp;
+    exp.set_days(*exp_days);
+    rule.set_expiration(exp);
+  }
+  if (noncur_days) {
+    LCExpiration ncexp;
+    ncexp.set_days(*noncur_days);
+    rule.set_noncur_expiration(ncexp);
+  }
+  return rule;
+}
+
+// x-amz-expiration must reflect only the current-version Expiration action, and
+// only for the current version (an empty instance). It must never be derived
+// from NoncurrentVersionExpiration, nor returned for an explicit versionId
+// request. Reproducer for https://tracker.ceph.com/issues/77564.
+TEST(ExpHdr, S3ExpirationHeaderCurrentVersionOnly)
+{
+  boost::intrusive_ptr<CephContext> cct{
+    new CephContext(CEPH_ENTITY_TYPE_ANY), false};
+  NoDoutPrefix dpp(cct.get(), ceph_subsys_rgw);
+  RGWObjTags notags;
+  auto mtime = ceph::real_clock::now();
+
+  const std::string days30{"30"}, days60{"60"};
+  const rgw_obj_key current_key{"obj"};
+  const rgw_obj_key versioned_key{"obj", "versionid-0123456789"};
+
+  // (1) only NoncurrentVersionExpiration: never emit the header (the bug).
+  {
+    RGWLifecycleConfiguration cfg(cct.get());
+    cfg.add_rule(make_rule("noncur-only", nullptr, &days60));
+    auto attrs = make_lc_attrs(cfg);
+
+    EXPECT_TRUE(rgw::lc::s3_expiration_header(
+                  &dpp, current_key, notags, mtime, attrs).empty());
+    EXPECT_TRUE(rgw::lc::s3_expiration_header(
+                  &dpp, versioned_key, notags, mtime, attrs).empty());
+  }
+
+  // (2) Expiration + NoncurrentVersionExpiration: emit for the current version
+  // from the Expiration rule only; nothing for an explicit versionId request.
+  {
+    RGWLifecycleConfiguration cfg(cct.get());
+    cfg.add_rule(make_rule("cur-and-noncur", &days30, &days60));
+    auto attrs = make_lc_attrs(cfg);
+
+    auto cur = rgw::lc::s3_expiration_header(
+                 &dpp, current_key, notags, mtime, attrs);
+    EXPECT_FALSE(cur.empty());
+    EXPECT_NE(cur.find("rule-id=\"cur-and-noncur\""), std::string::npos);
+
+    EXPECT_TRUE(rgw::lc::s3_expiration_header(
+                  &dpp, versioned_key, notags, mtime, attrs).empty());
+  }
+}