Merge branch 'mr/forestier/fix-sns-topic-policy' into 'master'

Fix TopicPolicy and QueuePolicy generation

See merge request it/e3-aws!5

src/e3/aws/troposphere/s3/bucket.py

@@ -99,11 +99,11 @@ def add_notification_configuration(
     ) -> None:
         """Add a configuration to bucket notification rules.
 
-        :param event: the S3 bucket event for which to invoke the Lambda function
-        :param function: function to invoke when the specified event type occurs
+        :param event: the S3 bucket event for which to invoke or notify the target
+        :param target: target to invoke or notify when the specified event type occurs
         :param permission_suffix: a name suffix for permissions or policy objects
         :param s3_filter: the filtering rules that determine which objects invoke
-            the AWS Lambda function
+            or notify the target
         """
         params = {"Event": event}
         if s3_filter:
@@ -132,7 +132,7 @@ def add_notification_configuration(
     def notification_setup(
         self,
     ) -> tuple[s3.NotificationConfiguration, list[AWSObject]]:
-        """Return notifcation configuration and associated resources."""
+        """Return notification configuration and associated resources."""
         notification_resources = []
         notification_config = None
         params = {}
@@ -166,15 +166,13 @@ def notification_setup(
                 }
             )
             # Add policy allowing to publish to topics
-            for _, topic, suffix in self.topic_configurations:
+            for _, topic, _ in self.topic_configurations:
                 if topic:
-                    topic_policy = topic.allow_publish_policy(
+                    topic_policy_name = topic.add_allow_service_to_publish_statement(
                         service="s3",
-                        name_suffix=suffix,
                         condition={"ArnLike": {"aws:SourceArn": self.arn}},
                     )
-                    notification_resources.append(topic_policy)
-                    self.depends_on.append(topic_policy)
+                    self.depends_on.append(topic_policy_name)
         if self.queue_configurations:
             params.update(
                 {
@@ -184,15 +182,13 @@ def notification_setup(
                     ]
                 }
             )
-            for _, queue, suffix in self.queue_configurations:
+            for _, queue, _ in self.queue_configurations:
                 if queue:
-                    queue_policy = queue.allow_service_to_write(
+                    queue_policy_name = queue.add_allow_service_to_write_statement(
                         service="s3",
-                        name_suffix=suffix,
                         condition={"ArnLike": {"aws:SourceArn": self.arn}},
                     )
-                    notification_resources.append(queue_policy)
-                    self.depends_on.append(queue_policy)
+                    self.depends_on.append(queue_policy_name)
 
         if params:
             notification_config = s3.NotificationConfiguration(

src/e3/aws/troposphere/sns/__init__.py

@@ -3,7 +3,7 @@
 from e3.aws import name_to_id
 from e3.aws.troposphere import Construct
 from e3.aws.troposphere.iam.policy_document import PolicyDocument
-from e3.aws.troposphere.iam.policy_statement import Allow
+from e3.aws.troposphere.iam.policy_statement import Allow, PolicyStatement
 
 from troposphere import sns, GetAtt, Ref
 
@@ -29,6 +29,11 @@ def __init__(self, name: str, kms_master_key_id: str | None = None):
         self.subscriptions: list[sns.Subscription] = []
         self.optional_resources: list[AWSObject] = []
         self.kms_master_key_id = kms_master_key_id
+        self.topic_policy_statements: list[PolicyStatement] = []
+
+    def _get_topic_policy_name(self) -> str:
+        """Return the TopicPolicy name."""
+        return name_to_id(f"{self.name}Policy")
 
     def add_lambda_subscription(
         self, function: Function, delivery_policy: dict | None = None
@@ -58,29 +63,24 @@ def add_lambda_subscription(
             ]
         )
 
-    def allow_publish_policy(
-        self, service: str, name_suffix: str, condition: ConditionType | None = None
-    ) -> sns.TopicPolicy:
-        """Return a policy allowing a service to publish to the topic.
+    def add_allow_service_to_publish_statement(
+        self, service: str, condition: ConditionType | None = None
+    ) -> str:
+        """Add a statement in TopicPolicy allowing a service to publish to the topic.
 
         :param service: service allowed to publish
-        :param name_suffix: a suffix used in the object name
         :param condition: condition to be able to publish
+        :return: the TopicPolicy name for depends_on settings
         """
-        return sns.TopicPolicy(
-            name_to_id(f"{self.name}Policy{name_suffix}"),
-            Topics=[self.ref],
-            PolicyDocument=PolicyDocument(
-                statements=[
-                    Allow(
-                        action="sns:Publish",
-                        resource=self.ref,
-                        principal={"Service": f"{service}.amazonaws.com"},
-                        condition=condition,
-                    )
-                ]
-            ).as_dict,
+        self.topic_policy_statements.append(
+            Allow(
+                action="sns:Publish",
+                resource=self.ref,
+                principal={"Service": f"{service}.amazonaws.com"},
+                condition=condition,
+            )
         )
+        return self._get_topic_policy_name()
 
     @property
     def arn(self) -> GetAtt:
@@ -99,6 +99,20 @@ def resources(self, stack: Stack) -> list[AWSObject]:
         if self.kms_master_key_id is not None:
             params["KmsMasterKeyId"] = self.kms_master_key_id
 
+        # Add Topic policy to optional resources if any statement
+        if self.topic_policy_statements:
+            self.optional_resources.extend(
+                [
+                    sns.TopicPolicy(
+                        self._get_topic_policy_name(),
+                        Topics=[self.ref],
+                        PolicyDocument=PolicyDocument(
+                            statements=self.topic_policy_statements,
+                        ).as_dict,
+                    )
+                ]
+            )
+
         return [
             sns.Topic(
                 name_to_id(self.name),

src/e3/aws/troposphere/sqs/__init__.py

@@ -3,7 +3,7 @@
 from e3.aws import name_to_id
 from e3.aws.troposphere import Construct
 from e3.aws.troposphere.iam.policy_document import PolicyDocument
-from e3.aws.troposphere.iam.policy_statement import Allow
+from e3.aws.troposphere.iam.policy_statement import Allow, PolicyStatement
 
 from troposphere import sns, sqs, GetAtt, Ref
 
@@ -15,7 +15,7 @@
 
 
 class Queue(Construct):
-    """A SQS Topic."""
+    """A SQS Queue."""
 
     def __init__(
         self,
@@ -26,7 +26,7 @@ def __init__(
     ) -> None:
         """Initialize a SQS.
 
-        :param name: topic name
+        :param name: queue name
         :param fifo: Set the queue type to fifo
         :param visibility_timeout: set the length of time during which a message will be
             unavailable after a message is delivered from the queue
@@ -49,25 +49,30 @@ def __init__(
                 "maxReceiveCount": "3",
             }
         self.optional_resources: list[AWSObject] = []
+        self.queue_policy_statements: list[PolicyStatement] = []
 
-    def allow_service_to_write(
-        self, service: str, name_suffix: str, condition: Optional[ConditionType] = None
-    ) -> sqs.QueuePolicy:
-        """Enable a given service to send a message."""
-        return sqs.QueuePolicy(
-            name_to_id(f"{self.name}Policy{name_suffix}"),
-            Queues=[self.ref],
-            PolicyDocument=PolicyDocument(
-                statements=[
-                    Allow(
-                        action="sqs:SendMessage",
-                        resource=self.arn,
-                        principal={"Service": f"{service}.amazonaws.com"},
-                        condition=condition,
-                    )
-                ]
-            ).as_dict,
+    def _get_queue_policy_name(self) -> str:
+        """Return the QueuePolicy name."""
+        return name_to_id(f"{self.name}Policy")
+
+    def add_allow_service_to_write_statement(
+        self, service: str, condition: Optional[ConditionType] = None
+    ) -> str:
+        """Add a statement in QueuePolicy allowing a service to send msg to the queue.
+
+        :param service: service allowed to send message
+        :param condition: condition to be able to send message
+        :return: the QueuePolicy name for depends_on settings
+        """
+        self.queue_policy_statements.append(
+            Allow(
+                action="sqs:SendMessage",
+                resource=self.arn,
+                principal={"Service": f"{service}.amazonaws.com"},
+                condition=condition,
+            )
         )
+        return self._get_queue_policy_name()
 
     def subscribe_to_sns_topic(
         self, topic_arn: str, delivery_policy: dict | None = None
@@ -87,15 +92,13 @@ def subscribe_to_sns_topic(
         if delivery_policy:
             sub_params.update({"DeliveryPolicy": delivery_policy})
 
+        self.add_allow_service_to_write_statement(
+            service="sns",
+            condition={"ArnLike": {"aws:SourceArn": topic_arn}},
+        )
+
         self.optional_resources.extend(
-            [
-                sns.SubscriptionResource(name_to_id(f"{self.name}Sub"), **sub_params),
-                self.allow_service_to_write(
-                    service="sns",
-                    name_suffix="Sub",
-                    condition={"ArnLike": {"aws:SourceArn": topic_arn}},
-                ),
-            ]
+            [sns.SubscriptionResource(name_to_id(f"{self.name}Sub"), **sub_params)]
         )
 
     @property
@@ -110,6 +113,19 @@ def ref(self) -> Ref:
 
     def resources(self, stack: Stack) -> list[AWSObject]:
         """Compute AWS resources for the construct."""
+        # Add Queue policy to optional resources if any statement
+        if self.queue_policy_statements:
+            self.optional_resources.extend(
+                [
+                    sqs.QueuePolicy(
+                        self._get_queue_policy_name(),
+                        Queues=[self.ref],
+                        PolicyDocument=PolicyDocument(
+                            statements=self.queue_policy_statements
+                        ).as_dict,
+                    )
+                ]
+            )
         return [
             sqs.Queue.from_dict(name_to_id(self.name), self.attr),
             *self.optional_resources,

tests/tests_e3_aws/troposphere/s3/bucket.json

@@ -85,8 +85,8 @@
         },
         "Type": "AWS::S3::Bucket",
         "DependsOn": [
-            "TestTopicPolicyTpUpload",
-            "TestQueuePolicyFileEvent"
+            "TestTopicPolicy",
+            "TestQueuePolicy"
         ]
     },
     "TestBucketPolicy": {
@@ -129,7 +129,7 @@
         },
         "Type": "AWS::Lambda::Permission"
     },
-    "TestTopicPolicyTpUpload": {
+    "TestTopicPolicy": {
         "Properties": {
             "Topics": [
                 {
@@ -159,7 +159,7 @@
         },
         "Type": "AWS::SNS::TopicPolicy"
     },
-    "TestQueuePolicyFileEvent": {
+    "TestQueuePolicy": {
         "Properties": {
             "PolicyDocument": {
                 "Statement": [

tests/tests_e3_aws/troposphere/s3/s3_test.py

@@ -31,9 +31,7 @@ def test_bucket(stack: Stack) -> None:
         handler="app.main",
     )
 
-    stack.add(topic_test)
     stack.add(lambda_test)
-    stack.add(queue_test)
 
     bucket = Bucket(name="test-bucket")
     bucket.add_notification_configuration(
@@ -46,6 +44,8 @@ def test_bucket(stack: Stack) -> None:
         event="s3:ObjectCreated:*", target=queue_test, permission_suffix="FileEvent"
     )
     stack.add(bucket)
+    stack.add(topic_test)
+    stack.add(queue_test)
 
     with open(os.path.join(TEST_DIR, "bucket.json")) as fd:
         expected_template = json.load(fd)

tests/tests_e3_aws/troposphere/sqs/sqs_test.py

@@ -33,7 +33,7 @@
         "Properties": {"QueueName": "myqueue", "VisibilityTimeout": 30},
         "Type": "AWS::SQS::Queue",
     },
-    "MyqueuePolicySub": {
+    "MyqueuePolicy": {
         "Properties": {
             "PolicyDocument": {
                 "Statement": [