rgw/iam: use retry_raced_role_write() for Role apis

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 03c30e3e25ca8ec288a01ed4a12e2b5c4226ef67)

diff --git a/src/rgw/rgw_rest_iam.h b/src/rgw/rgw_rest_iam.h
index ef0e170c38924883051e64cc1a66e53d6e4978da..00f6ff7dfc4aad8fa48fd1e7de1bb42daccdba70 100644 (file)
--- a/src/rgw/rgw_rest_iam.h
+++ b/src/rgw/rgw_rest_iam.h
@@ -8,6 +8,7 @@
 #include "rgw_auth.h"
 #include "rgw_auth_filters.h"
 #include "rgw_rest.h"
+#include "rgw_role.h"
 #include "rgw_sal.h"
 #include "rgw_xml.h"
 
@@ -72,6 +73,24 @@ int retry_raced_group_write(const DoutPrefixProvider* dpp, optional_yield y,
   return r;
 }
 
+/// Perform an atomic read-modify-write operation on the given role metadata.
+/// Racing writes are detected here as ECANCELED errors, where we reload the
+/// updated group metadata and retry the operation.
+template <std::invocable<> F>
+int retry_raced_role_write(const DoutPrefixProvider* dpp, optional_yield y,
+                           rgw::sal::RGWRole* role, const F& f)
+{
+  int r = f();
+  for (int i = 0; i < 10 && r == -ECANCELED; ++i) {
+    role->get_objv_tracker().clear();
+    r = role->get_by_id(dpp, y);
+    if (r >= 0) {
+      r = f();
+    }
+  }
+  return r;
+}
+
 class RGWHandler_REST_IAM : public RGWHandler_REST {
   const rgw::auth::StrategyRegistry& auth_registry;
   bufferlist bl_post_body;

diff --git a/src/rgw/rgw_rest_role.cc b/src/rgw/rgw_rest_role.cc
index 295881ce9b9d24158e4386217802afea1b5bf6b5..79c5c4bbac0007fe5a84df2ef459d5b6549c2c9e 100644 (file)
--- a/src/rgw/rgw_rest_role.cc
+++ b/src/rgw/rgw_rest_role.cc
@@ -471,8 +471,11 @@ void RGWModifyRoleTrustPolicy::execute(optional_yield y)
     }
   }
 
-  role->update_trust_policy(trust_policy);
-  op_ret = role->update(this, y);
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        role->update_trust_policy(trust_policy);
+        return role->update(this, y);
+      });
 
   s->formatter->open_object_section("UpdateAssumeRolePolicyResponse");
   s->formatter->open_object_section("ResponseMetadata");
@@ -594,8 +597,11 @@ void RGWPutRolePolicy::execute(optional_yield y)
     }
   }
 
-  role->set_perm_policy(policy_name, perm_policy);
-  op_ret = role->update(this, y);
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        role->set_perm_policy(policy_name, perm_policy);
+        return role->update(this, y);
+      });
 
   if (op_ret == 0) {
     s->formatter->open_object_section("PutRolePolicyResponse");
@@ -718,15 +724,17 @@ void RGWDeleteRolePolicy::execute(optional_yield y)
     }
   }
 
-  op_ret = role->delete_policy(this, policy_name);
-  if (op_ret == -ENOENT) {
-    op_ret = -ERR_NO_ROLE_FOUND;
-    return;
-  }
-
-  if (op_ret == 0) {
-    op_ret = role->update(this, y);
-  }
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        int r = role->delete_policy(this, policy_name);
+        if (r == -ENOENT) {
+          s->err.message = "The requested PolicyName was not found";
+          return -ERR_NO_SUCH_ENTITY;
+        } else if (r == 0) {
+          r = role->update(this, y);
+        }
+        return r;
+      });
 
   s->formatter->open_object_section("DeleteRolePoliciesResponse");
   s->formatter->open_object_section("ResponseMetadata");
@@ -782,10 +790,14 @@ void RGWTagRole::execute(optional_yield y)
     }
   }
 
-  op_ret = role->set_tags(this, tags);
-  if (op_ret == 0) {
-    op_ret = role->update(this, y);
-  }
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        int r = role->set_tags(this, tags);
+        if (r == 0) {
+          r = role->update(this, y);
+        }
+        return r;
+      });
 
   if (op_ret == 0) {
     s->formatter->open_object_section("TagRoleResponse");
@@ -882,8 +894,11 @@ void RGWUntagRole::execute(optional_yield y)
     }
   }
 
-  role->erase_tags(untag);
-  op_ret = role->update(this, y);
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        role->erase_tags(untag);
+        return role->update(this, y);
+      });
 
   if (op_ret == 0) {
     s->formatter->open_object_section("UntagRoleResponse");
@@ -939,16 +954,18 @@ void RGWUpdateRole::execute(optional_yield y)
     }
   }
 
-  if (description) {
-    role->get_info().description = std::move(*description);
-  }
-  role->update_max_session_duration(max_session_duration);
-  if (!role->validate_max_session_duration(this)) {
-    op_ret = -EINVAL;
-    return;
-  }
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        if (description) {
+          role->get_info().description = std::move(*description);
+        }
+        role->update_max_session_duration(max_session_duration);
+        if (!role->validate_max_session_duration(this)) {
+          return -EINVAL;
+        }
 
-  op_ret = role->update(this, y);
+        return role->update(this, y);
+      });
 
   s->formatter->open_object_section("UpdateRoleResponse");
   s->formatter->open_object_section("UpdateRoleResult");
@@ -1056,12 +1073,15 @@ void RGWAttachRolePolicy_IAM::execute(optional_yield y)
     return;
   }
 
-  // insert the policy arn. if it's already there, just return success
-  auto &policies = role->get_info().managed_policies;
-  const bool inserted = policies.arns.insert(policy_arn).second;
-  if (inserted) {
-    op_ret = role->update(this, y);
-  }
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        // insert the policy arn. if it's already there, just return success
+        auto &policies = role->get_info().managed_policies;
+        if (!policies.arns.insert(policy_arn).second) {
+          return 0;
+        }
+        return role->update(this, y);
+      });
 
   if (op_ret == 0) {
     s->formatter->open_object_section_in_ns("AttachRolePolicyResponse", RGW_REST_IAM_XMLNS);
@@ -1136,15 +1156,17 @@ void RGWDetachRolePolicy_IAM::execute(optional_yield y)
     }
   }
 
-  auto &policies = role->get_info().managed_policies;
-  auto p = policies.arns.find(policy_arn);
-  if (p == policies.arns.end()) {
-    op_ret = -ERR_NO_SUCH_ENTITY;
-    s->err.message = "The requested PolicyArn is not attached to the role";
-    return;
-  }
-  policies.arns.erase(p);
-  op_ret = role->update(this, y);
+  op_ret = retry_raced_role_write(this, y, role.get(),
+      [this, y] {
+        auto &policies = role->get_info().managed_policies;
+        auto p = policies.arns.find(policy_arn);
+        if (p == policies.arns.end()) {
+          s->err.message = "The requested PolicyArn is not attached to the role";
+          return -ERR_NO_SUCH_ENTITY;
+        }
+        policies.arns.erase(p);
+        return role->update(this, y);
+      });
 
   if (op_ret == 0) {
     s->formatter->open_object_section_in_ns("DetachRolePolicyResponse", RGW_REST_IAM_XMLNS);

diff --git a/src/rgw/rgw_role.h b/src/rgw/rgw_role.h
index 9f49351d6e643949f8cc58aa44a73518ceee12ea..585f723973573f6e3261c18dc37690057c033e28 100644 (file)
--- a/src/rgw/rgw_role.h
+++ b/src/rgw/rgw_role.h
@@ -135,6 +135,7 @@ public:
   const std::string& get_create_date() const { return info.creation_date; }
   const std::string& get_assume_role_policy() const { return info.trust_policy;}
   const uint64_t& get_max_session_duration() const { return info.max_session_duration; }
+  RGWObjVersionTracker& get_objv_tracker() { return info.objv_tracker; }
   const RGWObjVersionTracker& get_objv_tracker() const { return info.objv_tracker; }
   const real_time& get_mtime() const { return info.mtime; }
   std::map<std::string, bufferlist>& get_attrs() { return info.attrs; }