From 35fea80f8c075412b1df195aa0d28d256e18c67c Mon Sep 17 00:00:00 2001 From: Kalpesh Pandya Date: Mon, 15 Nov 2021 13:04:17 +0530 Subject: [PATCH] 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 --- src/rgw/rgw_pubsub.cc | 3 - src/rgw/rgw_rest_pubsub.cc | 98 +++++++++++++-------- src/test/rgw/bucket_notification/test_bn.py | 57 ++++++++++++ 3 files changed, 118 insertions(+), 40 deletions(-) diff --git a/src/rgw/rgw_pubsub.cc b/src/rgw/rgw_pubsub.cc index 6963ba8fda5..67b9fa41446 100644 --- 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 32fd0b581a4..c20cbe7c233 100644 --- 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 /?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(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[=] diff --git a/src/test/rgw/bucket_notification/test_bn.py b/src/test/rgw/bucket_notification/test_bn.py index ec6b74d6617..8d47085dbcc 100644 --- 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 """ -- 2.39.5