From 893ce6b64a6138cea9428803a13d287c05a93244 Mon Sep 17 00:00:00 2001 From: Tom Schoonjans Date: Wed, 5 May 2021 19:13:20 +0000 Subject: [PATCH] rgw: remove s3: prefix in eventName value of s3 event message structure This change is necessary to match the event message structure seen on AWS. Signed-off-by: Tom Schoonjans --- PendingReleaseNotes | 6 +++-- doc/radosgw/notifications.rst | 4 +-- src/rgw/rgw_notify.cc | 2 +- src/rgw/rgw_notify_event_type.cc | 4 +++ src/rgw/rgw_notify_event_type.h | 2 ++ src/rgw/rgw_sync_module_pubsub.cc | 2 +- src/test/rgw/bucket_notification/test_bn.py | 28 ++++++++++----------- src/test/rgw/rgw_multi/tests_ps.py | 8 +++--- 8 files changed, 32 insertions(+), 24 deletions(-) diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 7ff7509178f..145f89bfdbf 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -13,8 +13,10 @@ is consistent with the help message. * OSD: Ceph now uses mclock_scheduler as its default osd_op_queue to provide QoS. -* RGW: S3 bucket notification events now contain `eTag` instead of `etag`, fixing - a deviation from the message format observed on AWS. + +* RGW: S3 bucket notification events now contain an `eTag` key instead of `etag`, + and eventName values no longer carry the `s3:` prefix, fixing deviations from + the message format observed on AWS. >=16.0.0 -------- diff --git a/doc/radosgw/notifications.rst b/doc/radosgw/notifications.rst index 7e5dc2fdf07..959f517eb53 100644 --- a/doc/radosgw/notifications.rst +++ b/doc/radosgw/notifications.rst @@ -385,7 +385,7 @@ pushed or pulled using the pubsub sync module. For example: "eventSource":"ceph:s3", "awsRegion":"us-east-1", "eventTime":"2019-11-22T13:47:35.124724Z", - "eventName":"s3:ObjectCreated:Put", + "eventName":"ObjectCreated:Put", "userIdentity":{ "principalId":"tester" }, @@ -424,7 +424,7 @@ pushed or pulled using the pubsub sync module. For example: - awsRegion: zonegroup - eventTime: timestamp indicating when the event was triggered -- eventName: for list of supported events see: `S3 Notification Compatibility`_ +- eventName: for list of supported events see: `S3 Notification Compatibility`_. Note that the eventName values do not start with the `s3:` prefix. - userIdentity.principalId: user that triggered the change - requestParameters.sourceIPAddress: not supported - responseElements.x-amz-request-id: request ID of the original change diff --git a/src/rgw/rgw_notify.cc b/src/rgw/rgw_notify.cc index 38f6c0647a4..a32f1e7eeae 100644 --- a/src/rgw/rgw_notify.cc +++ b/src/rgw/rgw_notify.cc @@ -675,7 +675,7 @@ void populate_event_from_request(const req_state *s, EventType event_type, rgw_pubsub_s3_event& event) { event.eventTime = mtime; - event.eventName = to_string(event_type); + event.eventName = to_event_string(event_type); event.userIdentity = s->user->get_id().id; // user that triggered the change event.x_amz_request_id = s->req_id; // request ID of the original change event.x_amz_id_2 = s->host_id; // RGW on which the change was made diff --git a/src/rgw/rgw_notify_event_type.cc b/src/rgw/rgw_notify_event_type.cc index 10c77c28184..4af9a32f7e3 100644 --- a/src/rgw/rgw_notify_event_type.cc +++ b/src/rgw/rgw_notify_event_type.cc @@ -49,6 +49,10 @@ namespace rgw::notify { return "UNKNOWN_EVENT"; } + std::string to_event_string(EventType t) { + return to_string(t).substr(3); + } + EventType from_string(const std::string& s) { if (s == "s3:ObjectCreated:*" || s == "OBJECT_CREATE") return ObjectCreated; diff --git a/src/rgw/rgw_notify_event_type.h b/src/rgw/rgw_notify_event_type.h index 0d86bf3f321..f255bfd744e 100644 --- a/src/rgw/rgw_notify_event_type.h +++ b/src/rgw/rgw_notify_event_type.h @@ -27,6 +27,8 @@ namespace rgw::notify { std::string to_ceph_string(EventType t); + std::string to_event_string(EventType t); + EventType from_string(const std::string& s); // create a vector of event types from comma separated list of event types diff --git a/src/rgw/rgw_sync_module_pubsub.cc b/src/rgw/rgw_sync_module_pubsub.cc index 460bd4149c5..c37b5637907 100644 --- a/src/rgw/rgw_sync_module_pubsub.cc +++ b/src/rgw/rgw_sync_module_pubsub.cc @@ -269,7 +269,7 @@ static void make_s3_event_ref(CephContext *cct, const rgw_bucket& bucket, EventRef& e = *event; e->eventTime = mtime; - e->eventName = rgw::notify::to_string(event_type); + e->eventName = rgw::notify::to_event_string(event_type); // userIdentity: not supported in sync module // x_amz_request_id: not supported in sync module // x_amz_id_2: not supported in sync module diff --git a/src/test/rgw/bucket_notification/test_bn.py b/src/test/rgw/bucket_notification/test_bn.py index d657d5f9a30..63f5a7fef05 100644 --- a/src/test/rgw/bucket_notification/test_bn.py +++ b/src/test/rgw/bucket_notification/test_bn.py @@ -347,11 +347,11 @@ def verify_s3_records_by_elements(records, keys, exact_match=False, deletions=Fa assert_equal(key.etag[1:-1], record['s3']['object']['eTag']) if etags: assert_in(key.etag[1:-1], etags) - if deletions and 'ObjectRemoved' in record['eventName']: + if deletions and record['eventName'].startswith('ObjectRemoved'): key_found = True object_size = record['s3']['object']['size'] break - elif not deletions and 'ObjectCreated' in record['eventName']: + elif not deletions and record['eventName'].startswith('ObjectCreated'): key_found = True object_size = record['s3']['object']['size'] break @@ -363,11 +363,11 @@ def verify_s3_records_by_elements(records, keys, exact_match=False, deletions=Fa assert_equal(key.etag, record['s3']['object']['eTag']) if etags: assert_in(key.etag[1:-1], etags) - if deletions and 'ObjectRemoved' in record['eventName']: + if deletions and record['eventName'].startswith('ObjectRemoved'): key_found = True object_size = record['s3']['object']['size'] break - elif not deletions and 'ObjectCreated' in record['eventName']: + elif not deletions and record['eventName'].startswith('ObjectCreated'): key_found = True object_size = record['s3']['object']['size'] break @@ -1652,12 +1652,12 @@ def test_ps_s3_multipart_on_master(): events = receiver2.get_and_reset_events() assert_equal(len(events), 1) - assert_equal(events[0]['Records'][0]['eventName'], 's3:ObjectCreated:Post') + assert_equal(events[0]['Records'][0]['eventName'], 'ObjectCreated:Post') assert_equal(events[0]['Records'][0]['s3']['configurationId'], notification_name+'_2') events = receiver3.get_and_reset_events() assert_equal(len(events), 1) - assert_equal(events[0]['Records'][0]['eventName'], 's3:ObjectCreated:CompleteMultipartUpload') + assert_equal(events[0]['Records'][0]['eventName'], 'ObjectCreated:CompleteMultipartUpload') assert_equal(events[0]['Records'][0]['s3']['configurationId'], notification_name+'_3') print(events[0]['Records'][0]['s3']['object']['size']) @@ -2009,10 +2009,10 @@ def test_ps_s3_versioned_deletion_on_master(): delete_marker_create_events = 0 for event_list in events: for event in event_list['Records']: - if event['eventName'] == 's3:ObjectRemoved:Delete': + if event['eventName'] == 'ObjectRemoved:Delete': delete_events += 1 assert event['s3']['configurationId'] in [notification_name+'_1', notification_name+'_3'] - if event['eventName'] == 's3:ObjectRemoved:DeleteMarkerCreated': + if event['eventName'] == 'ObjectRemoved:DeleteMarkerCreated': delete_marker_create_events += 1 assert event['s3']['configurationId'] in [notification_name+'_1', notification_name+'_2'] @@ -2282,10 +2282,10 @@ def test_ps_s3_persistent_gateways_recovery(): creations = 0 deletions = 0 for event in events: - if event['Records'][0]['eventName'] == 's3:ObjectCreated:Put' and \ + if event['Records'][0]['eventName'] == 'ObjectCreated:Put' and \ key.name == event['Records'][0]['s3']['object']['key']: creations += 1 - elif event['Records'][0]['eventName'] == 's3:ObjectRemoved:Delete' and \ + elif event['Records'][0]['eventName'] == 'ObjectRemoved:Delete' and \ key.name == event['Records'][0]['s3']['object']['key']: deletions += 1 assert_equal(creations, 1) @@ -2365,11 +2365,11 @@ def test_ps_s3_persistent_multiple_gateways(): topic1_count = 0 topic2_count = 0 for event in events: - if event['Records'][0]['eventName'] == 's3:ObjectCreated:Put' and \ + if event['Records'][0]['eventName'] == 'ObjectCreated:Put' and \ key.name == event['Records'][0]['s3']['object']['key'] and \ topic1_opaque == event['Records'][0]['opaqueData']: topic1_count += 1 - elif event['Records'][0]['eventName'] == 's3:ObjectCreated:Put' and \ + elif event['Records'][0]['eventName'] == 'ObjectCreated:Put' and \ key.name == event['Records'][0]['s3']['object']['key'] and \ topic2_opaque == event['Records'][0]['opaqueData']: topic2_count += 1 @@ -2390,11 +2390,11 @@ def test_ps_s3_persistent_multiple_gateways(): topic1_count = 0 topic2_count = 0 for event in events: - if event['Records'][0]['eventName'] == 's3:ObjectRemoved:Delete' and \ + if event['Records'][0]['eventName'] == 'ObjectRemoved:Delete' and \ key.name == event['Records'][0]['s3']['object']['key'] and \ topic1_opaque == event['Records'][0]['opaqueData']: topic1_count += 1 - elif event['Records'][0]['eventName'] == 's3:ObjectRemoved:Delete' and \ + elif event['Records'][0]['eventName'] == 'ObjectRemoved:Delete' and \ key.name == event['Records'][0]['s3']['object']['key'] and \ topic2_opaque == event['Records'][0]['opaqueData']: topic2_count += 1 diff --git a/src/test/rgw/rgw_multi/tests_ps.py b/src/test/rgw/rgw_multi/tests_ps.py index 8d741828912..f2f27ff8f80 100644 --- a/src/test/rgw/rgw_multi/tests_ps.py +++ b/src/test/rgw/rgw_multi/tests_ps.py @@ -332,11 +332,11 @@ def verify_s3_records_by_elements(records, keys, exact_match=False, deletions=Fa for record in record_list['Records']: if record['s3']['bucket']['name'] == key.bucket.name and \ record['s3']['object']['key'] == key.name: - if deletions and 'ObjectRemoved' in record['eventName']: + if deletions and record['eventName'].startswith('ObjectRemoved'): key_found = True object_size = record['s3']['object']['size'] break - elif not deletions and 'ObjectCreated' in record['eventName']: + elif not deletions and record['eventName'].startswith('ObjectCreated'): key_found = True object_size = record['s3']['object']['size'] break @@ -344,11 +344,11 @@ def verify_s3_records_by_elements(records, keys, exact_match=False, deletions=Fa for record in records['Records']: if record['s3']['bucket']['name'] == key.bucket.name and \ record['s3']['object']['key'] == key.name: - if deletions and 'ObjectRemoved' in record['eventName']: + if deletions and record['eventName'].startswith('ObjectRemoved'): key_found = True object_size = record['s3']['object']['size'] break - elif not deletions and 'ObjectCreated' in record['eventName']: + elif not deletions and record['eventName'].startswith('ObjectCreated'): key_found = True object_size = record['s3']['object']['size'] break -- 2.39.5