From 77a9e720761bf0a7bb71821455fa48db09c46cd9 Mon Sep 17 00:00:00 2001 From: kavin Date: Fri, 2 Jul 2021 14:41:04 -0700 Subject: [PATCH 01/13] Adding event configuration for S3 bucket --- kingpin/actors/aws/s3.py | 148 ++++++++++++++++++++++++++++- kingpin/actors/aws/test/test_s3.py | 45 +++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index 6739e2fb..a69d38b3 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -323,7 +323,19 @@ class LifecycleConfig(SchemaCompareBase): 'noncurrent_version_transitions': { 'type': 'array', 'itmes': { - '$ref': '#/definitions/noncurrent_version_transition' + 'type': 'object', + 'required': ['storage_class'], + 'additionalProperties': False, + 'properties': { + 'noncurrent_days': { + 'type': ['string', 'integer'], + 'pattern': '^[0-9]+$', + }, + 'storage_class': { + 'type': 'string', + 'enum': ['GLACIER', 'STANDARD_IA'] + } + } } }, 'noncurrent_version_transition': { @@ -378,6 +390,59 @@ class LifecycleConfig(SchemaCompareBase): } +class NotificationConfiguration(SchemaCompareBase): + + """Provides JSON-Schema based validation of the supplied Notification Config. + + .. code-block:: json + + { + "queueconfigurations": [ + { + "queuearn": "ARN of the SQS queue", + "events": ["s3:ObjectCreated:*", "s3:ReducedRedundancyLostObject"], + "filter": { + "key": { + "filterrules:" [ + { + "name": "prefix|suffix", + "value": "string" + } + ] + } + } + } + ] + } + """ + + SCHEMA = { + 'type': ['object', 'null'], + 'required': ['queueconfigurations'], + 'properties': { + 'queueconfigurations': { + 'type': ['array'], + 'items': { + 'type': 'object', + 'additionalProperties': False, + 'required': ['queuearn', 'events'], + 'properties': { + 'queuearn': { + 'type': 'string' + }, + 'events': { + 'type': 'array' + }, + 'filter': { + 'type': 'object' + } + } + } + } + } + } + + class TaggingConfig(SchemaCompareBase): """Provides JSON-Schema based validation of the supplied tagging config. @@ -423,6 +488,7 @@ class Bucket(base.EnsurableAWSBaseActor): * Enable or Suspend Bucket Versioning. Note: It is impossible to actually _disable_ bucket versioning -- once it is enabled, you can only suspend it, or re-enable it. + * Enable Event Notification. (limited to SQS for now) **Note about Buckets with Files** @@ -506,6 +572,19 @@ class Bucket(base.EnsurableAWSBaseActor): (bool, None): Whether or not to enable Versioning on the bucket. If "None", then we don't manage versioning either way. Default: None + :notification_configuration: + (:py:class:`NotificationConfiguration`, None) + + If a dictionary is supplised, then it must conform to + :py:class:`NotificationConfiguration`, type and include mapping + queuearn & events + + If an empty dictionary is supplied, then Kingpin will explicitly remove + any Notification Configuration from the bucket. + + Finally, If None is supplies, Kingoin will ignore the checks entire on + this portion of the bucket configuration + **Examples** .. code-block:: json @@ -534,6 +613,14 @@ class Bucket(base.EnsurableAWSBaseActor): {"key": "my_key", "value": "billing-grp-1"}, ], "versioning": true, + "notification_configuration": { + "queueconfigurations": [ + { + queuearn: arn:aws:sqs:us-east-1:1234567:some_sqs, + events: ['s3:ObjectCreated:*'] + } + ] + } } } @@ -570,6 +657,7 @@ class Bucket(base.EnsurableAWSBaseActor): 'versioning': ((bool, None), None, ('Desired state of versioning on the bucket: ' 'true/false')), + 'notification_configuration': (NotificationConfiguration, None, '') } unmanaged_options = ['name', 'region'] @@ -1133,3 +1221,61 @@ def _set_tags(self): self.s3_conn.put_bucket_tagging, Bucket=self.option('name'), Tagging={'TagSet': tagset}) + + @gen.coroutine + def _get_notification_configuration(self): + if self.option('notification_configuration') is None: + raise gen.Return(None) + + if not self._bucket_exists: + raise gen.Return(None) + + try: + raw = yield self.api_call( + self.s3_conn.get_bucket_notification_configuration, + Bucket=self.option('name')) + except ClientError as e: + if 'NoSuchBucketNotificationConfiguration' in str(e): + raise gen.Return([]) + raise + + return gen.Return(raw) + + @gen.coroutine + def _compare_notification_configuration(self): + new = self.option('notification_configuration') + if new is None: + self.log.debug('No Notification Configuration') + raise gen.Return(True) + + exist = yield self._get_notification_configuration() + + diff = utils.diff_dicts(exist, new) + + if not diff: + self.log.debug('Notification Configurations match') + raise gen.Return(True) + + self.log.info('Bucket Notification Configuration ' + 'differs from Amazons:') + for line in diff.split('\n'): + self.log.info('Diff: %s' % line) + + raise gen.Return(False) + + @gen.coroutine + @dry('Would have added notification configurations') + def _set_notification_configuration(self): + configs = self.option('notification_configuration') + + if configs is None: + self.log.debug('No Notification Configurations') + raise gen.Return(None) + + config_list = self._snake_to_camel( + self.option('notification_configuration')) + self.log.info('Updating Bucket Notification Configuration') + yield self.api_call( + self.s3_conn.put_bucket_notification_configuration, + Bucket=self.option('name'), + NotificationConfiguration=config_list) diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index 9029c40b..e7fec63d 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -643,3 +643,48 @@ def test_set_tags(self): Tagging={'TagSet': [ {'Key': 'tag1', 'Value': 'v1'} ]})]) + + @testing.gen_test + def test_set_notification_configurations_none(self): + self.actor._options['notification_configuration'] = None + yield self.actor._set_notification_configuration() + self.assertFalse(self.actor.s3_conn.put_bucket_notification_configuration.called) + + @testing.gen_test + def test_set_notification_configurations(self): + self.actor._options['notification_configuration'] = { + "queueconfigurations": { + "queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", + "events": ["s3:ObjectCreated:*"] + } + } + yield self.actor._set_notification_configuration() + self.actor.s3_conn.put_bucket_notification_configuration.assert_has_calls([ + mock.call( + Bucket='test', + NotificationConfiguration={ + "Queueconfigurations": [{ + "Queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", + "Events": ["s3:ObjectCreated:*"] + }] + } + ) + ]) + + @testing.gen_test + def test_get_notification_configuration(self): + self.actor._bucket_exists = True + self.actor.s3_conn.get_bucket_notification_configuration.return_value = { + "Queueconfigurations": [{ + "Queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", + "Events": ["s3:ObjectCreated:*"] + }] + } + ret = yield self.actor._get_notification() + self.assertEqual(ret, { + "Queueconfigurations": + [{ + "Queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", + "Events": ["s3:ObjectCreated:*"] + }] + }) \ No newline at end of file From 0dcf22174aa3acdde3a7c3587db76459cf79567f Mon Sep 17 00:00:00 2001 From: kavin Date: Fri, 2 Jul 2021 14:53:12 -0700 Subject: [PATCH 02/13] Revert some test changes --- kingpin/actors/aws/s3.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index a69d38b3..96f873c3 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -323,19 +323,7 @@ class LifecycleConfig(SchemaCompareBase): 'noncurrent_version_transitions': { 'type': 'array', 'itmes': { - 'type': 'object', - 'required': ['storage_class'], - 'additionalProperties': False, - 'properties': { - 'noncurrent_days': { - 'type': ['string', 'integer'], - 'pattern': '^[0-9]+$', - }, - 'storage_class': { - 'type': 'string', - 'enum': ['GLACIER', 'STANDARD_IA'] - } - } + '$ref': '#/definitions/noncurrent_version_transition' } }, 'noncurrent_version_transition': { From 23b77fa7aa56e6511195cc1cc17d837d0d45f842 Mon Sep 17 00:00:00 2001 From: kavin Date: Mon, 12 Jul 2021 09:46:19 -0700 Subject: [PATCH 03/13] Adding events configurations to S3 bucket via notification_configuration --- kingpin/actors/aws/s3.py | 47 +++++---- kingpin/actors/aws/test/test_s3.py | 159 ++++++++++++++++++++++++----- 2 files changed, 159 insertions(+), 47 deletions(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index 96f873c3..502b3ebb 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -388,7 +388,7 @@ class NotificationConfiguration(SchemaCompareBase): "queueconfigurations": [ { "queuearn": "ARN of the SQS queue", - "events": ["s3:ObjectCreated:*", "s3:ReducedRedundancyLostObject"], + "events": ["s3:ObjectCreated:*"], "filter": { "key": { "filterrules:" [ @@ -561,17 +561,17 @@ class Bucket(base.EnsurableAWSBaseActor): "None", then we don't manage versioning either way. Default: None :notification_configuration: - (:py:class:`NotificationConfiguration`, None) + (:py:class:`NotificationConfiguration`, None) - If a dictionary is supplised, then it must conform to - :py:class:`NotificationConfiguration`, type and include mapping - queuearn & events + If a dictionary is supplised, then it must conform to + :py:class:`NotificationConfiguration`, type and include mapping + queuearn & events - If an empty dictionary is supplied, then Kingpin will explicitly remove - any Notification Configuration from the bucket. + If an empty dictionary is supplied, then Kingpin will explicitly remove + any Notification Configuration from the bucket. - Finally, If None is supplies, Kingoin will ignore the checks entire on - this portion of the bucket configuration + Finally, If None is supplies, Kingoin will ignore the checks entire on + this portion of the bucket configuration **Examples** @@ -604,8 +604,8 @@ class Bucket(base.EnsurableAWSBaseActor): "notification_configuration": { "queueconfigurations": [ { - queuearn: arn:aws:sqs:us-east-1:1234567:some_sqs, - events: ['s3:ObjectCreated:*'] + "queuearn": "arn:aws:sqs:us-east-1:1234567:some_sqs", + "events": ["s3:ObjectCreated:*"] } ] } @@ -1218,16 +1218,22 @@ def _get_notification_configuration(self): if not self._bucket_exists: raise gen.Return(None) - try: - raw = yield self.api_call( - self.s3_conn.get_bucket_notification_configuration, - Bucket=self.option('name')) - except ClientError as e: - if 'NoSuchBucketNotificationConfiguration' in str(e): - raise gen.Return([]) - raise + raw = yield self.api_call( + self.s3_conn.get_bucket_notification_configuration, + Bucket=self.option('name')) - return gen.Return(raw) + configurations = {} + for config_type in raw.keys(): + # Limiting to only QueueConfigurations for now + if config_type == 'QueueConfigurations': + queue_configs = [] + for config in raw['QueueConfigurations']: + config_dict = {} + for k, v in config.items(): + config_dict[k.lower()] = v + queue_configs.append(config_dict) + configurations['queueconfigurations'] = queue_configs + raise gen.Return(configurations) @gen.coroutine def _compare_notification_configuration(self): @@ -1237,7 +1243,6 @@ def _compare_notification_configuration(self): raise gen.Return(True) exist = yield self._get_notification_configuration() - diff = utils.diff_dicts(exist, new) if not diff: diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index e7fec63d..bd6c7b1d 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -52,6 +52,9 @@ def setUp(self): }, 'versioning': False, 'tags': [], + 'notification_configuration': { + 'queueconfigurations': [] + } }) self.actor.s3_conn = mock.MagicMock() @@ -648,43 +651,147 @@ def test_set_tags(self): def test_set_notification_configurations_none(self): self.actor._options['notification_configuration'] = None yield self.actor._set_notification_configuration() - self.assertFalse(self.actor.s3_conn.put_bucket_notification_configuration.called) + self.assertFalse( + self.actor + .s3_conn + .put_bucket_notification_configuration.called + ) @testing.gen_test - def test_set_notification_configurations(self): + def test_set_notification_configurations_with_valid_configs(self): self.actor._options['notification_configuration'] = { - "queueconfigurations": { + "queueconfigurations": [{ "queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "events": ["s3:ObjectCreated:*"] - } + }] } yield self.actor._set_notification_configuration() - self.actor.s3_conn.put_bucket_notification_configuration.assert_has_calls([ - mock.call( - Bucket='test', - NotificationConfiguration={ - "Queueconfigurations": [{ - "Queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", - "Events": ["s3:ObjectCreated:*"] - }] - } - ) - ]) + self.actor\ + .s3_conn\ + .put_bucket_notification_configuration\ + .assert_has_calls([ + mock.call( + Bucket='test', + NotificationConfiguration={ + "Queueconfigurations": [{ + "Queuearn": + "arn:aws:sqs:us-east-1:1234567:test_sqs", + "Events": ["s3:ObjectCreated:*"] + }] + } + ) + ]) + + @testing.gen_test + def test_set_notification_configurations_no_configs(self): + self.actor._options['notification_configuration'] = { + "queueconfigurations": [] + } + yield self.actor._set_notification_configuration() + self.actor.s3_conn\ + .put_bucket_notification_configuration\ + .assert_has_calls([]) + + @testing.gen_test + def test_set_notification_configurations_empty_queue_configs(self): + self.actor._options['notification_configuration'] = { + } + yield self.actor._set_notification_configuration() + self.actor\ + .s3_conn\ + .put_bucket_notification_configuration\ + .assert_has_calls([]) + + @testing.gen_test + def test_get_notification_configuration_with_existing_queueconfigs(self): + self.actor._bucket_exists = True + self.actor.s3_conn.get_bucket_notification_configuration\ + .return_value = { + "QueueConfigurations": [{ + "QueueArn": "arn:aws:sqs", + "Events": ["s3:ObjectCreated:*"] + }] + } + ret = yield self.actor._get_notification_configuration() + log.debug(ret) + self.assertEqual(type(ret), dict) + self.assertEqual(ret, {'queueconfigurations': [{ + "queuearn": "arn:aws:sqs", + "events": ["s3:ObjectCreated:*"] + }]}) + + @testing.gen_test + def test_get_notification_configuration_no_bucket(self): + self.actor._bucket_exists = False + ret = yield self.actor._get_notification_configuration() + self.assertEqual(ret, None) @testing.gen_test - def test_get_notification_configuration(self): + def test_get_notification_configuration_with_no_existing_configs(self): self.actor._bucket_exists = True - self.actor.s3_conn.get_bucket_notification_configuration.return_value = { - "Queueconfigurations": [{ - "Queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", - "Events": ["s3:ObjectCreated:*"] + # empty NotificationConfiguration dict + self.actor\ + .s3_conn\ + .get_bucket_notification_configuration\ + .return_value = {} + ret = yield self.actor._get_notification_configuration() + self.assertEqual(ret, {}) + + @testing.gen_test + def test_get_notification_configuration_with_no_config(self): + self.actor._bucket_exists = True + self.actor._options["notification_configuration"] = None + self.actor\ + .s3_conn\ + .get_bucket_notification_configuration\ + .return_value = {} + ret = yield self.actor._get_notification_configuration() + self.assertEqual(ret, None) + + @testing.gen_test + def test_compare_notification_configuration_with_new_config(self): + self.actor._options['notification_configuration'] = { + "queueconfigurations": [{ + "queuearn": + "arn:aws:sqs:us-east-1:1234567:test_sqs", + "events": ["s3:ObjectCreated:*"] }] } - ret = yield self.actor._get_notification() - self.assertEqual(ret, { - "Queueconfigurations": - [{ - "Queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", + self.actor\ + .s3_conn\ + .get_bucket_notification_configuration\ + .return_value = {} + ret = yield self.actor._compare_notification_configuration() + self.assertFalse(ret) + + @testing.gen_test + def test_compare_notification_configuration_with_no_updated_config(self): + self.actor._bucket_exists = True + self.actor._options['notification_configuration'] = { + "queueconfigurations": [{ + "queuearn": + "arn:aws:sqs:us-east-1:1234567:test_sqs", + "events": ["s3:ObjectCreated:*"] + }] + } + self.actor\ + .s3_conn\ + .get_bucket_notification_configuration\ + .return_value = { + "QueueConfigurations": [{ + "QueueArn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "Events": ["s3:ObjectCreated:*"] }] - }) \ No newline at end of file + } + ret = yield self.actor._compare_notification_configuration() + self.assertTrue(ret) + + @testing.gen_test + def test_compare_notification_configuration_with_no_config(self): + self.actor._options['notification_configuration'] = None + self.actor\ + .s3_conn\ + .get_bucket_notification_configuration\ + .return_value = {} + ret = yield self.actor._compare_notification_configuration() + self.assertTrue(ret) From dcc4f5887c8b454ad183d0148789efc1611b266d Mon Sep 17 00:00:00 2001 From: kavin Date: Mon, 12 Jul 2021 11:19:18 -0700 Subject: [PATCH 04/13] Updating the notification_configuration --- kingpin/actors/aws/s3.py | 37 +++++-------------- kingpin/actors/aws/test/test_s3.py | 58 ++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 39 deletions(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index 502b3ebb..fdb1f51d 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -387,18 +387,8 @@ class NotificationConfiguration(SchemaCompareBase): { "queueconfigurations": [ { - "queuearn": "ARN of the SQS queue", + "queue_arn": "ARN of the SQS queue", "events": ["s3:ObjectCreated:*"], - "filter": { - "key": { - "filterrules:" [ - { - "name": "prefix|suffix", - "value": "string" - } - ] - } - } } ] } @@ -415,14 +405,11 @@ class NotificationConfiguration(SchemaCompareBase): 'additionalProperties': False, 'required': ['queuearn', 'events'], 'properties': { - 'queuearn': { + 'queue_arn': { 'type': 'string' }, 'events': { 'type': 'array' - }, - 'filter': { - 'type': 'object' } } } @@ -604,8 +591,11 @@ class Bucket(base.EnsurableAWSBaseActor): "notification_configuration": { "queueconfigurations": [ { - "queuearn": "arn:aws:sqs:us-east-1:1234567:some_sqs", - "events": ["s3:ObjectCreated:*"] + "queue_arn": "arn:aws:sqs:us-east-1:1234567:some_sqs", + "events": [ + "s3:ObjectCreated:*", + "s3:ObjectRemoved*" + ] } ] } @@ -1222,18 +1212,7 @@ def _get_notification_configuration(self): self.s3_conn.get_bucket_notification_configuration, Bucket=self.option('name')) - configurations = {} - for config_type in raw.keys(): - # Limiting to only QueueConfigurations for now - if config_type == 'QueueConfigurations': - queue_configs = [] - for config in raw['QueueConfigurations']: - config_dict = {} - for k, v in config.items(): - config_dict[k.lower()] = v - queue_configs.append(config_dict) - configurations['queueconfigurations'] = queue_configs - raise gen.Return(configurations) + raise gen.Return(raw) @gen.coroutine def _compare_notification_configuration(self): diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index bd6c7b1d..35bd7665 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -661,7 +661,7 @@ def test_set_notification_configurations_none(self): def test_set_notification_configurations_with_valid_configs(self): self.actor._options['notification_configuration'] = { "queueconfigurations": [{ - "queuearn": "arn:aws:sqs:us-east-1:1234567:test_sqs", + "queue_arn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "events": ["s3:ObjectCreated:*"] }] } @@ -674,7 +674,7 @@ def test_set_notification_configurations_with_valid_configs(self): Bucket='test', NotificationConfiguration={ "Queueconfigurations": [{ - "Queuearn": + "QueueArn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "Events": ["s3:ObjectCreated:*"] }] @@ -682,6 +682,41 @@ def test_set_notification_configurations_with_valid_configs(self): ) ]) + @testing.gen_test + def test_set_notification_configurations_with_multiple_configs(self): + self.actor._options['notification_configuration'] = { + "queueconfigurations": [ + { + "queue_arn": "arn:aws:sqs:us-east-1:1:test1_sqs", + "events": ["s3:ObjectCreated:*"] + }, + { + "queue_arn": "arn:aws:sqs:us-east-1:1:test2_sqs", + "events": ["s3:ObjectCreated:*", "s3:ObjectRemoved:*"] + } + ] + } + yield self.actor._set_notification_configuration() + self.actor \ + .s3_conn \ + .put_bucket_notification_configuration \ + .assert_has_calls([mock.call( + Bucket='test', + NotificationConfiguration={ + "Queueconfigurations": [ + { + "QueueArn": "arn:aws:sqs:us-east-1:1:test1_sqs", + "Events": ["s3:ObjectCreated:*"] + }, + { + "QueueArn": "arn:aws:sqs:us-east-1:1:test2_sqs", + "Events": ["s3:ObjectCreated:*", + "s3:ObjectRemoved:*"] + } + ] + } + )]) + @testing.gen_test def test_set_notification_configurations_no_configs(self): self.actor._options['notification_configuration'] = { @@ -715,10 +750,13 @@ def test_get_notification_configuration_with_existing_queueconfigs(self): ret = yield self.actor._get_notification_configuration() log.debug(ret) self.assertEqual(type(ret), dict) - self.assertEqual(ret, {'queueconfigurations': [{ - "queuearn": "arn:aws:sqs", - "events": ["s3:ObjectCreated:*"] - }]}) + self.assertEqual(ret, + {'QueueConfigurations': [ + { + "QueueArn": "arn:aws:sqs", + "Events": ["s3:ObjectCreated:*"] + } + ]}) @testing.gen_test def test_get_notification_configuration_no_bucket(self): @@ -752,7 +790,7 @@ def test_get_notification_configuration_with_no_config(self): def test_compare_notification_configuration_with_new_config(self): self.actor._options['notification_configuration'] = { "queueconfigurations": [{ - "queuearn": + "queue_arn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "events": ["s3:ObjectCreated:*"] }] @@ -768,10 +806,10 @@ def test_compare_notification_configuration_with_new_config(self): def test_compare_notification_configuration_with_no_updated_config(self): self.actor._bucket_exists = True self.actor._options['notification_configuration'] = { - "queueconfigurations": [{ - "queuearn": + "QueueConfigurations": [{ + "QueueArn": "arn:aws:sqs:us-east-1:1234567:test_sqs", - "events": ["s3:ObjectCreated:*"] + "Events": ["s3:ObjectCreated:*"] }] } self.actor\ From 150b841fd4fd6f4e94946bfb1d26d47eb4825afc Mon Sep 17 00:00:00 2001 From: kavin Date: Wed, 14 Jul 2021 11:07:10 -0700 Subject: [PATCH 05/13] Updating events notification configuration with all snake case for keys that need to be camel cased --- kingpin/actors/aws/s3.py | 10 +++++----- kingpin/actors/aws/test/test_s3.py | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index fdb1f51d..e93b72c3 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -385,7 +385,7 @@ class NotificationConfiguration(SchemaCompareBase): .. code-block:: json { - "queueconfigurations": [ + "queue_configurations": [ { "queue_arn": "ARN of the SQS queue", "events": ["s3:ObjectCreated:*"], @@ -396,14 +396,14 @@ class NotificationConfiguration(SchemaCompareBase): SCHEMA = { 'type': ['object', 'null'], - 'required': ['queueconfigurations'], + 'required': ['queue_configurations'], 'properties': { - 'queueconfigurations': { + 'queue_configurations': { 'type': ['array'], 'items': { 'type': 'object', 'additionalProperties': False, - 'required': ['queuearn', 'events'], + 'required': ['queue_arn', 'events'], 'properties': { 'queue_arn': { 'type': 'string' @@ -589,7 +589,7 @@ class Bucket(base.EnsurableAWSBaseActor): ], "versioning": true, "notification_configuration": { - "queueconfigurations": [ + "queue_configurations": [ { "queue_arn": "arn:aws:sqs:us-east-1:1234567:some_sqs", "events": [ diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index 35bd7665..e8f5c7f1 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -53,7 +53,7 @@ def setUp(self): 'versioning': False, 'tags': [], 'notification_configuration': { - 'queueconfigurations': [] + 'queue_configurations': [] } }) self.actor.s3_conn = mock.MagicMock() @@ -660,7 +660,7 @@ def test_set_notification_configurations_none(self): @testing.gen_test def test_set_notification_configurations_with_valid_configs(self): self.actor._options['notification_configuration'] = { - "queueconfigurations": [{ + "queue_configurations": [{ "queue_arn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "events": ["s3:ObjectCreated:*"] }] @@ -673,7 +673,7 @@ def test_set_notification_configurations_with_valid_configs(self): mock.call( Bucket='test', NotificationConfiguration={ - "Queueconfigurations": [{ + "QueueConfigurations": [{ "QueueArn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "Events": ["s3:ObjectCreated:*"] @@ -685,7 +685,7 @@ def test_set_notification_configurations_with_valid_configs(self): @testing.gen_test def test_set_notification_configurations_with_multiple_configs(self): self.actor._options['notification_configuration'] = { - "queueconfigurations": [ + "queue_configurations": [ { "queue_arn": "arn:aws:sqs:us-east-1:1:test1_sqs", "events": ["s3:ObjectCreated:*"] @@ -703,7 +703,7 @@ def test_set_notification_configurations_with_multiple_configs(self): .assert_has_calls([mock.call( Bucket='test', NotificationConfiguration={ - "Queueconfigurations": [ + "QueueConfigurations": [ { "QueueArn": "arn:aws:sqs:us-east-1:1:test1_sqs", "Events": ["s3:ObjectCreated:*"] @@ -720,7 +720,7 @@ def test_set_notification_configurations_with_multiple_configs(self): @testing.gen_test def test_set_notification_configurations_no_configs(self): self.actor._options['notification_configuration'] = { - "queueconfigurations": [] + "queue_configurations": [] } yield self.actor._set_notification_configuration() self.actor.s3_conn\ @@ -789,7 +789,7 @@ def test_get_notification_configuration_with_no_config(self): @testing.gen_test def test_compare_notification_configuration_with_new_config(self): self.actor._options['notification_configuration'] = { - "queueconfigurations": [{ + "queue_configurations": [{ "queue_arn": "arn:aws:sqs:us-east-1:1234567:test_sqs", "events": ["s3:ObjectCreated:*"] From 23a5bcff078ea3f90febddd7b012a8aa313e0347 Mon Sep 17 00:00:00 2001 From: kavin Date: Wed, 14 Jul 2021 12:11:12 -0700 Subject: [PATCH 06/13] Defining events array for S3 events --- kingpin/actors/aws/s3.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index e93b72c3..97683ab7 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -409,7 +409,8 @@ class NotificationConfiguration(SchemaCompareBase): 'type': 'string' }, 'events': { - 'type': 'array' + 'type': 'array', + 'items': {'type': 'string'} } } } From 5184a4a7d1f8eb1835afb102928e9df2467bb6ec Mon Sep 17 00:00:00 2001 From: kavin Date: Wed, 14 Jul 2021 12:37:02 -0700 Subject: [PATCH 07/13] Removing debug log --- kingpin/actors/aws/test/test_s3.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index e8f5c7f1..4af0d1b9 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -748,7 +748,6 @@ def test_get_notification_configuration_with_existing_queueconfigs(self): }] } ret = yield self.actor._get_notification_configuration() - log.debug(ret) self.assertEqual(type(ret), dict) self.assertEqual(ret, {'QueueConfigurations': [ From 0181b35e6229a7ddba839833268b89559fc692f7 Mon Sep 17 00:00:00 2001 From: kavin Date: Wed, 14 Jul 2021 13:43:46 -0700 Subject: [PATCH 08/13] Removing extra line to trigger new build --- kingpin/actors/aws/test/test_s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index 4af0d1b9..55266a9f 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -831,4 +831,4 @@ def test_compare_notification_configuration_with_no_config(self): .get_bucket_notification_configuration\ .return_value = {} ret = yield self.actor._compare_notification_configuration() - self.assertTrue(ret) + self.assertTrue(ret) \ No newline at end of file From 7a2ea5a813e74a62cc8303faceb09eebcd448b71 Mon Sep 17 00:00:00 2001 From: kavin Date: Wed, 14 Jul 2021 14:25:23 -0700 Subject: [PATCH 09/13] Revert "Removing extra line to trigger new build" This reverts commit 0181b35e6229a7ddba839833268b89559fc692f7. --- kingpin/actors/aws/test/test_s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index 55266a9f..4af0d1b9 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -831,4 +831,4 @@ def test_compare_notification_configuration_with_no_config(self): .get_bucket_notification_configuration\ .return_value = {} ret = yield self.actor._compare_notification_configuration() - self.assertTrue(ret) \ No newline at end of file + self.assertTrue(ret) From 2baa53ae3cf81b92b0a2e0e23648d83080383b2d Mon Sep 17 00:00:00 2001 From: kavin Date: Wed, 14 Jul 2021 23:47:41 -0700 Subject: [PATCH 10/13] Updated s3 notificatin_configuration to always convert the config from snake_case to CamelCase --- kingpin/actors/aws/s3.py | 20 +++++--- kingpin/actors/aws/test/test_s3.py | 78 ++++++++++++++++-------------- 2 files changed, 53 insertions(+), 45 deletions(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index 97683ab7..512fea6a 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -662,6 +662,14 @@ def __init__(self, *args, **kwargs): if self.access_block is not None: self.access_block = self._snake_to_camel(self.access_block) + # If the NotificationConfiguration is anything but None, we parse + # it and pre-build the rules. + self.notification_configuration = \ + self.option('notification_configuration') + if self.notification_configuration is not None: + self.notification_configuration = \ + self._snake_to_camel(self.notification_configuration) + # Start out assuming the bucket doesn't exist. The _precache() method # will populate this with True if the bucket does exist. self._bucket_exists = False @@ -1203,7 +1211,7 @@ def _set_tags(self): @gen.coroutine def _get_notification_configuration(self): - if self.option('notification_configuration') is None: + if self.notification_configuration is None: raise gen.Return(None) if not self._bucket_exists: @@ -1217,7 +1225,7 @@ def _get_notification_configuration(self): @gen.coroutine def _compare_notification_configuration(self): - new = self.option('notification_configuration') + new = self.notification_configuration if new is None: self.log.debug('No Notification Configuration') raise gen.Return(True) @@ -1239,16 +1247,12 @@ def _compare_notification_configuration(self): @gen.coroutine @dry('Would have added notification configurations') def _set_notification_configuration(self): - configs = self.option('notification_configuration') - - if configs is None: + if self.notification_configuration is None: self.log.debug('No Notification Configurations') raise gen.Return(None) - config_list = self._snake_to_camel( - self.option('notification_configuration')) self.log.info('Updating Bucket Notification Configuration') yield self.api_call( self.s3_conn.put_bucket_notification_configuration, Bucket=self.option('name'), - NotificationConfiguration=config_list) + NotificationConfiguration=self.notification_configuration) diff --git a/kingpin/actors/aws/test/test_s3.py b/kingpin/actors/aws/test/test_s3.py index 4af0d1b9..dd23dd84 100644 --- a/kingpin/actors/aws/test/test_s3.py +++ b/kingpin/actors/aws/test/test_s3.py @@ -647,9 +647,29 @@ def test_set_tags(self): {'Key': 'tag1', 'Value': 'v1'} ]})]) + @testing.gen_test + def test_snake_to_camelcase_for_notification_configuration(self): + notification_configuration_snake_case = { + "queue_configurations": [{ + "queue_arn": + "arn:aws:sqs:us-east-1:1234567:test_sqs", + "events": ["s3:ObjectCreated:*"] + }] + } + + notification_configuration_camel_case = \ + self.actor._snake_to_camel(notification_configuration_snake_case) + + self.assertEqual(notification_configuration_camel_case, + {"QueueConfigurations": [{ + "QueueArn": + "arn:aws:sqs:us-east-1:1234567:test_sqs", + "Events": ["s3:ObjectCreated:*"] + }]}) + @testing.gen_test def test_set_notification_configurations_none(self): - self.actor._options['notification_configuration'] = None + self.actor.notification_configuration = None yield self.actor._set_notification_configuration() self.assertFalse( self.actor @@ -659,10 +679,10 @@ def test_set_notification_configurations_none(self): @testing.gen_test def test_set_notification_configurations_with_valid_configs(self): - self.actor._options['notification_configuration'] = { - "queue_configurations": [{ - "queue_arn": "arn:aws:sqs:us-east-1:1234567:test_sqs", - "events": ["s3:ObjectCreated:*"] + self.actor.notification_configuration = { + "QueueConfigurations": [{ + "QueueArn": "arn:aws:sqs:us-east-1:1234567:test_sqs", + "Events": ["s3:ObjectCreated:*"] }] } yield self.actor._set_notification_configuration() @@ -684,15 +704,15 @@ def test_set_notification_configurations_with_valid_configs(self): @testing.gen_test def test_set_notification_configurations_with_multiple_configs(self): - self.actor._options['notification_configuration'] = { - "queue_configurations": [ + self.actor.notification_configuration = { + "QueueConfigurations": [ { - "queue_arn": "arn:aws:sqs:us-east-1:1:test1_sqs", - "events": ["s3:ObjectCreated:*"] + "QueueArn": "arn:aws:sqs:us-east-1:1:test1_sqs", + "Events": ["s3:ObjectCreated:*"] }, { - "queue_arn": "arn:aws:sqs:us-east-1:1:test2_sqs", - "events": ["s3:ObjectCreated:*", "s3:ObjectRemoved:*"] + "QueueArn": "arn:aws:sqs:us-east-1:1:test2_sqs", + "Events": ["s3:ObjectCreated:*", "s3:ObjectRemoved:*"] } ] } @@ -719,8 +739,8 @@ def test_set_notification_configurations_with_multiple_configs(self): @testing.gen_test def test_set_notification_configurations_no_configs(self): - self.actor._options['notification_configuration'] = { - "queue_configurations": [] + self.actor.notification_configuration = { + "QueueConfigurations": [] } yield self.actor._set_notification_configuration() self.actor.s3_conn\ @@ -729,8 +749,7 @@ def test_set_notification_configurations_no_configs(self): @testing.gen_test def test_set_notification_configurations_empty_queue_configs(self): - self.actor._options['notification_configuration'] = { - } + self.actor.notification_configuration = {} yield self.actor._set_notification_configuration() self.actor\ .s3_conn\ @@ -763,35 +782,20 @@ def test_get_notification_configuration_no_bucket(self): ret = yield self.actor._get_notification_configuration() self.assertEqual(ret, None) - @testing.gen_test - def test_get_notification_configuration_with_no_existing_configs(self): - self.actor._bucket_exists = True - # empty NotificationConfiguration dict - self.actor\ - .s3_conn\ - .get_bucket_notification_configuration\ - .return_value = {} - ret = yield self.actor._get_notification_configuration() - self.assertEqual(ret, {}) - @testing.gen_test def test_get_notification_configuration_with_no_config(self): self.actor._bucket_exists = True - self.actor._options["notification_configuration"] = None - self.actor\ - .s3_conn\ - .get_bucket_notification_configuration\ - .return_value = {} + self.actor.notification_configuration = None ret = yield self.actor._get_notification_configuration() self.assertEqual(ret, None) @testing.gen_test def test_compare_notification_configuration_with_new_config(self): - self.actor._options['notification_configuration'] = { - "queue_configurations": [{ - "queue_arn": + self.actor.notification_configuration = { + "QueueConfigurations": [{ + "QueueArn": "arn:aws:sqs:us-east-1:1234567:test_sqs", - "events": ["s3:ObjectCreated:*"] + "Events": ["s3:ObjectCreated:*"] }] } self.actor\ @@ -804,7 +808,7 @@ def test_compare_notification_configuration_with_new_config(self): @testing.gen_test def test_compare_notification_configuration_with_no_updated_config(self): self.actor._bucket_exists = True - self.actor._options['notification_configuration'] = { + self.actor.notification_configuration = { "QueueConfigurations": [{ "QueueArn": "arn:aws:sqs:us-east-1:1234567:test_sqs", @@ -825,7 +829,7 @@ def test_compare_notification_configuration_with_no_updated_config(self): @testing.gen_test def test_compare_notification_configuration_with_no_config(self): - self.actor._options['notification_configuration'] = None + self.actor.notification_configuration = None self.actor\ .s3_conn\ .get_bucket_notification_configuration\ From 8a9c5cef5032660956bbdf2158947270224382c3 Mon Sep 17 00:00:00 2001 From: kavin Date: Thu, 15 Jul 2021 00:56:40 -0700 Subject: [PATCH 11/13] Adding logging to debug an issue with notification_configuration in s3 --- kingpin/actors/aws/s3.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index 512fea6a..b0d84ae6 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -1237,8 +1237,9 @@ def _compare_notification_configuration(self): self.log.debug('Notification Configurations match') raise gen.Return(True) - self.log.info('Bucket Notification Configuration ' - 'differs from Amazons:') + self.log.info('Bucket Notification Configuration differs:') + self.log.info(new) + self.log(exist) for line in diff.split('\n'): self.log.info('Diff: %s' % line) From 92b8ea1cf82ffad0aba2658b9f257c923b546609 Mon Sep 17 00:00:00 2001 From: kavin Date: Thu, 15 Jul 2021 01:01:30 -0700 Subject: [PATCH 12/13] Adding logging to debug an issue with notification_configuration in s3 --- kingpin/actors/aws/s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index b0d84ae6..f7d6064a 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -1239,7 +1239,7 @@ def _compare_notification_configuration(self): self.log.info('Bucket Notification Configuration differs:') self.log.info(new) - self.log(exist) + self.log.info(exist) for line in diff.split('\n'): self.log.info('Diff: %s' % line) From 0195efe8696f1fdcfe63085565b54774af9f4c83 Mon Sep 17 00:00:00 2001 From: kavin Date: Thu, 15 Jul 2021 01:37:35 -0700 Subject: [PATCH 13/13] get_bucket_notification_configuration to remove metadata --- kingpin/actors/aws/s3.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/kingpin/actors/aws/s3.py b/kingpin/actors/aws/s3.py index f7d6064a..69958b48 100644 --- a/kingpin/actors/aws/s3.py +++ b/kingpin/actors/aws/s3.py @@ -1221,7 +1221,13 @@ def _get_notification_configuration(self): self.s3_conn.get_bucket_notification_configuration, Bucket=self.option('name')) - raise gen.Return(raw) + existing_configurations = {} + for configuration in ['TopicConfigurations', + 'QueueConfigurations', + 'LambdaFunctionConfigurations']: + if configuration in raw: + existing_configurations[configuration] = raw[configuration] + raise gen.Return(existing_configurations) @gen.coroutine def _compare_notification_configuration(self): @@ -1238,8 +1244,6 @@ def _compare_notification_configuration(self): raise gen.Return(True) self.log.info('Bucket Notification Configuration differs:') - self.log.info(new) - self.log.info(exist) for line in diff.split('\n'): self.log.info('Diff: %s' % line)