src/rgw: Empty configuration support

This PR solves: https://tracker.ceph.com/issues/53040

So, if we pass on empty configuration it should ideally
delete all the notifications.

Signed-off-by: Kalpesh Pandya <kapandya@redhat.com>

diff --git a/src/rgw/rgw_pubsub.cc b/src/rgw/rgw_pubsub.cc
index 6963ba8fda53cd5b25df920048f97dc5c6e407bc..67b9fa41446c08537e2449a24a6b74d9a0bc75df 100644 (file)
--- a/src/rgw/rgw_pubsub.cc
+++ b/src/rgw/rgw_pubsub.cc
@@ -237,9 +237,6 @@ void rgw_pubsub_s3_notification::dump_xml(Formatter *f) const {
 
 bool rgw_pubsub_s3_notifications::decode_xml(XMLObj *obj) {
   do_decode_xml_obj(list, "TopicConfiguration", obj);
-  if (list.empty()) {
-    throw RGWXMLDecoder::err("at least one 'TopicConfiguration' must exist");
-  }
   return true;
 }
 

diff --git a/src/rgw/rgw_rest_pubsub.cc b/src/rgw/rgw_rest_pubsub.cc
index 32fd0b581a40af6a8ac7e18a43c3fd1d5f0a2b28..c20cbe7c2333e14f8712f492a474d1a4a5048f2e 100644 (file)
--- a/src/rgw/rgw_rest_pubsub.cc
+++ b/src/rgw/rgw_rest_pubsub.cc
@@ -423,6 +423,50 @@ auto find_unique_topic(const rgw_pubsub_bucket_topics& bucket_topics, const std:
 }
 }
 
+int remove_notification_by_topic(const DoutPrefixProvider *dpp, const std::string& topic_name, const RGWPubSub::BucketRef& b, optional_yield y, RGWPubSub& ps) {
+  int op_ret = b->remove_notification(dpp, topic_name, y);
+  if (op_ret < 0) {
+    ldpp_dout(dpp, 1) << "failed to remove notification of topic '" << topic_name << "', ret=" << op_ret << dendl;
+  }
+  op_ret = ps.remove_topic(dpp, topic_name, y);
+  if (op_ret < 0) {
+    ldpp_dout(dpp, 1) << "failed to remove auto-generated topic '" << topic_name << "', ret=" << op_ret << dendl;
+  }
+  return op_ret;
+}
+
+int delete_all_notifications(const DoutPrefixProvider *dpp, const rgw_pubsub_bucket_topics& bucket_topics, const RGWPubSub::BucketRef& b, optional_yield y, RGWPubSub& ps) {
+  // delete all notifications of on a bucket
+  for (const auto& topic : bucket_topics.topics) {
+    // remove the auto generated subscription of the topic (if exist)
+    rgw_pubsub_topic_subs topic_subs;
+    int op_ret = ps.get_topic(topic.first, &topic_subs);
+    for (const auto& topic_sub_name : topic_subs.subs) {
+      auto sub = ps.get_sub(topic_sub_name);
+      rgw_pubsub_sub_config sub_conf;
+      op_ret = sub->get_conf(&sub_conf);
+      if (op_ret < 0) {
+        ldpp_dout(dpp, 1) << "failed to get subscription '" << topic_sub_name << "' info, ret=" << op_ret << dendl;
+        return op_ret;
+      }
+      if (!sub_conf.s3_id.empty()) {
+        // S3 notification, has autogenerated subscription
+        const auto& sub_topic_name = sub_conf.topic;
+        op_ret = sub->unsubscribe(dpp, sub_topic_name, y);
+        if (op_ret < 0) {
+          ldpp_dout(dpp, 1) << "failed to remove auto-generated subscription '" << topic_sub_name << "', ret=" << op_ret << dendl;
+          return op_ret;
+        }
+      }
+    }
+    op_ret = remove_notification_by_topic(dpp, topic.first, b, y, ps);
+    if (op_ret < 0) {
+      return op_ret;
+    }
+  }
+  return 0;
+}
+
 // command (S3 compliant): PUT /<bucket name>?notification
 // a "notification" and a subscription will be auto-generated
 // actual configuration is XML encoded in the body of the message
@@ -456,6 +500,7 @@ class RGWPSCreateNotif_ObjStore_S3 : public RGWPSCreateNotifOp {
     }
     try {
       // NotificationConfigurations is mandatory
+      // It can be empty which means we delete all the notifications
       RGWXMLDecoder::decode_xml("NotificationConfiguration", configurations, &parser, true);
     } catch (RGWXMLDecoder::err& err) {
       ldpp_dout(this, 1) << "failed to parse XML payload. error: " << err << dendl;
@@ -497,6 +542,7 @@ void RGWPSCreateNotif_ObjStore_S3::execute(optional_yield y) {
   ps.emplace(static_cast<rgw::sal::RadosStore*>(store), s->owner.get_id().tenant);
   auto b = ps->get_bucket(bucket_info.bucket);
   ceph_assert(b);
+
   std::string data_bucket_prefix = "";
   std::string data_oid_prefix = "";
   bool push_only = true;
@@ -511,6 +557,19 @@ void RGWPSCreateNotif_ObjStore_S3::execute(optional_yield y) {
     }
   }
 
+  if(configurations.list.empty()) {
+    // get all topics on a bucket
+    rgw_pubsub_bucket_topics bucket_topics;
+    op_ret = b->get_topics(&bucket_topics);
+    if (op_ret < 0) {
+      ldpp_dout(this, 1) << "failed to get list of topics from bucket '" << bucket_info.bucket.name << "', ret=" << op_ret << dendl;
+      return;
+    }
+
+    op_ret = delete_all_notifications(this, bucket_topics, b, y, *ps);
+    return;
+  }
+
   for (const auto& c : configurations.list) {
     const auto& notif_name = c.id;
     if (notif_name.empty()) {
@@ -615,17 +674,6 @@ private:
     return 0;
   }
 
-  void remove_notification_by_topic(const std::string& topic_name, const RGWPubSub::BucketRef& b, optional_yield y) {
-    op_ret = b->remove_notification(this, topic_name, y);
-    if (op_ret < 0) {
-      ldpp_dout(this, 1) << "failed to remove notification of topic '" << topic_name << "', ret=" << op_ret << dendl;
-    }
-    op_ret = ps->remove_topic(this, topic_name, y);
-    if (op_ret < 0) {
-      ldpp_dout(this, 1) << "failed to remove auto-generated topic '" << topic_name << "', ret=" << op_ret << dendl;
-    }
-  }
-
 public:
   void execute(optional_yield y) override;
   const char* name() const override { return "pubsub_notification_delete_s3"; }
@@ -661,7 +709,7 @@ void RGWPSDeleteNotif_ObjStore_S3::execute(optional_yield y) {
         ldpp_dout(this, 1) << "failed to remove auto-generated subscription '" << notif_name << "', ret=" << op_ret << dendl;
         return;
       }
-      remove_notification_by_topic(unique_topic_name, b, y);
+      op_ret = remove_notification_by_topic(this, unique_topic_name, b, y, *ps);
       return;
     }
     // notification to be removed is not found - considered success
@@ -669,31 +717,7 @@ void RGWPSDeleteNotif_ObjStore_S3::execute(optional_yield y) {
     return;
   }
 
-  // delete all notification of on a bucket
-  for (const auto& topic : bucket_topics.topics) {
-    // remove the auto generated subscription of the topic (if exist)
-    rgw_pubsub_topic_subs topic_subs;
-    op_ret = ps->get_topic(topic.first, &topic_subs);
-    for (const auto& topic_sub_name : topic_subs.subs) {
-      auto sub = ps->get_sub(topic_sub_name);
-      rgw_pubsub_sub_config sub_conf;
-      op_ret = sub->get_conf(&sub_conf);
-      if (op_ret < 0) {
-        ldpp_dout(this, 1) << "failed to get subscription '" << topic_sub_name << "' info, ret=" << op_ret << dendl;
-        return;
-      }
-      if (!sub_conf.s3_id.empty()) {
-        // S3 notification, has autogenerated subscription
-        const auto& sub_topic_name = sub_conf.topic;
-        op_ret = sub->unsubscribe(this, sub_topic_name, y);
-        if (op_ret < 0) {
-          ldpp_dout(this, 1) << "failed to remove auto-generated subscription '" << topic_sub_name << "', ret=" << op_ret << dendl;
-          return;
-        }
-      }
-    }
-    remove_notification_by_topic(topic.first, b, y);
-  }
+  op_ret = delete_all_notifications(this, bucket_topics, b, y, *ps);
 }
 
 // command (S3 compliant): GET /bucket?notification[=<notification-id>]

diff --git a/src/test/rgw/bucket_notification/test_bn.py b/src/test/rgw/bucket_notification/test_bn.py
index ec6b74d661714d8c453038754e13f439bb37397d..8d47085dbcc2cab74e4d8bd3d092f5c0dbede0ee 100644 (file)
--- a/src/test/rgw/bucket_notification/test_bn.py
+++ b/src/test/rgw/bucket_notification/test_bn.py
@@ -683,6 +683,63 @@ def test_ps_s3_notification_on_master():
     conn.delete_bucket(bucket_name)
 
 
+@attr('basic_test')
+def test_ps_s3_notification_on_master_empty_config():
+    """ test s3 notification set/get/delete on master with empty config """
+    hostname = get_ip()
+
+    conn = connection()
+
+    zonegroup = 'default'
+
+    # create bucket
+    bucket_name = gen_bucket_name()
+    bucket = conn.create_bucket(bucket_name)
+    topic_name = bucket_name + TOPIC_SUFFIX
+
+    # create s3 topic
+    endpoint_address = 'amqp://127.0.0.1:7001'
+    endpoint_args = 'push-endpoint='+endpoint_address+'&amqp-exchange=amqp.direct&amqp-ack-level=none'
+    topic_conf = PSTopicS3(conn, topic_name, zonegroup, endpoint_args=endpoint_args)
+    topic_arn = topic_conf.set_config()
+
+    # create s3 notification
+    notification_name = bucket_name + NOTIFICATION_SUFFIX
+    topic_conf_list = [{'Id': notification_name+'_1',
+                        'TopicArn': topic_arn,
+                        'Events': []
+                       }]
+    s3_notification_conf = PSNotificationS3(conn, bucket_name, topic_conf_list)
+    _, status = s3_notification_conf.set_config()
+    assert_equal(status/100, 2)
+
+    # get notifications on a bucket
+    response, status = s3_notification_conf.get_config(notification=notification_name+'_1')
+    assert_equal(status/100, 2)
+    assert_equal(response['NotificationConfiguration']['TopicConfiguration']['Topic'], topic_arn)
+
+    # create s3 notification again with empty configuration to check if it deletes or not
+    topic_conf_list = []
+
+    s3_notification_conf = PSNotificationS3(conn, bucket_name, topic_conf_list)
+    _, status = s3_notification_conf.set_config()
+    assert_equal(status/100, 2)
+
+    # make sure that the notification is now deleted
+    response, status = s3_notification_conf.get_config()
+    try:
+        check = response['NotificationConfiguration']
+    except KeyError as e:
+        assert_equal(status/100, 2)
+    else:
+        assert False
+
+    # cleanup
+    topic_conf.del_config()
+    # delete the bucket
+    conn.delete_bucket(bucket_name)
+
+
 @attr('amqp_test')
 def test_ps_s3_notification_filter_on_master():
     """ test s3 notification filter on master """