Fix tests

moj-analytical-services · Aug 30, 2024 · c3ca179 · c3ca179
1 parent 123d59a
commit c3ca179
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 117 deletions.
diff --git a/__main__.py b/__main__.py
@@ -61,7 +61,7 @@
     BucketPolicy(
         resource_name=f"{name}-bucket-policy",
         bucket=pull_bucket.id,
-        policy=bucket_policy.json,
+        policy=bucket_policy,
         opts=ResourceOptions(parent=pull_bucket),
     )
 

diff --git a/data_engineering_exports/pull.py b/data_engineering_exports/pull.py
@@ -1,17 +1,13 @@
 from typing import Dict
 
-from pulumi_aws.iam import (
-    GetPolicyDocumentStatementArgs,
-    GetPolicyDocumentStatementPrincipalArgs,
-    GetPolicyDocumentStatementConditionArgs,
-)
+from pulumi_aws.iam import GetPolicyDocumentStatementArgs
 from pulumi_aws.iam.get_policy_document import (
     get_policy_document,
     AwaitableGetPolicyDocumentResult,
 )
 
 
-def create_pull_bucket_policy(args: Dict[str, str]) -> AwaitableGetPolicyDocumentResult:
+def create_pull_bucket_policy(args: Dict[str, str]) -> Dict:
     """Create policy for a bucket to permit get access for a specific list of ARNs.
     The ARNs can be from another account.
 
@@ -24,108 +20,80 @@ def create_pull_bucket_policy(args: Dict[str, str]) -> AwaitableGetPolicyDocumen
 
     Returns
     -------
-    AwaitableGetPolicyDocumentResult
-        Pulumi output of the get_policy_document function.
+    Dict
+        AWS bucket policy.
     """
     bucket_arn = args.pop("bucket_arn")
     pull_arns = args.pop("pull_arns")
     allow_push = args.pop("allow_push", False)
     writable_actions = [
-        "s3:GetObject",
-        "s3:GetObjectAcl",
-        "s3:GetObjectVersion",
-        "s3:DeleteObject",
-        "s3:DeleteObjectVersion",
-        "s3:PutObject",
-        "s3:PutObjectAcl",
-        "s3:PutObjectTagging",
         "s3:RestoreObject",
-    ]
-    standard_actions = [
-        "s3:GetObject",
-        "s3:GetObjectAcl",
+        "s3:PutObjectTagging",
+        "s3:PutObjectAcl",
+        "s3:PutObject",
         "s3:GetObjectVersion",
+        "s3:GetObjectAcl",
+        "s3:GetObject",
+        "s3:DeleteObjectVersion",
+        "s3:DeleteObject",
     ]
+    standard_actions = ["s3:GetObjectVersion", "s3:GetObjectAcl", "s3:GetObject"]
     if allow_push:
-        bucket_policy = get_policy_document(
-            statements=[
-                GetPolicyDocumentStatementArgs(
-                    actions=writable_actions,
-                    principals=[
-                        GetPolicyDocumentStatementPrincipalArgs(
-                            identifiers=pull_arns, type="AWS"
-                        )
-                    ],
-                    resources=[bucket_arn + "/*"],
-                ),
-                GetPolicyDocumentStatementArgs(
-                    actions=["s3:ListBucket"],
-                    principals=[
-                        GetPolicyDocumentStatementPrincipalArgs(
-                            identifiers=pull_arns, type="AWS"
-                        )
-                    ],
-                    resources=[bucket_arn],
-                ),
-                GetPolicyDocumentStatementArgs(
-                    effect="Deny",
-                    actions=["s3:*"],
-                    principals=[
-                        GetPolicyDocumentStatementPrincipalArgs(
-                            identifiers=["*"], type="AWS"
-                        )
-                    ],
-                    resources=[bucket_arn, bucket_arn + "/*"],
-                    conditions=[
-                        GetPolicyDocumentStatementConditionArgs(
-                            test="NumericLessThan",
-                            variable="s3:TlsVersion",
-                            values=["1.2"],
-                        ),
-                    ],
-                ),
-            ]
-        )
+        bucket_policy = {
+            "Version": "2012-10-17",
+            "Statement": [
+                {
+                    "Sid": "",
+                    "Effect": "Allow",
+                    "Action": writable_actions,
+                    "Principal": {"AWS": pull_arns},
+                    "Resource": bucket_arn + "/*",
+                },
+                {
+                    "Sid": "",
+                    "Effect": "Allow",
+                    "Action": "s3:ListBucket",
+                    "Principal": {"AWS": pull_arns},
+                    "Resource": bucket_arn,
+                },
+                {
+                    "Sid": "",
+                    "Effect": "Deny",
+                    "Action": "s3:*",
+                    "Principal": "*",
+                    "Resource": [bucket_arn, bucket_arn + "/*"],
+                    "Condition": {"NumericLessThan": {"s3:TlsVersion": "1.2"}},
+                },
+            ],
+        }
     else:
-        bucket_policy = get_policy_document(
-            statements=[
-                GetPolicyDocumentStatementArgs(
-                    actions=standard_actions,
-                    principals=[
-                        GetPolicyDocumentStatementPrincipalArgs(
-                            identifiers=pull_arns, type="AWS"
-                        )
-                    ],
-                    resources=[bucket_arn + "/*"],
-                ),
-                GetPolicyDocumentStatementArgs(
-                    actions=["s3:ListBucket"],
-                    principals=[
-                        GetPolicyDocumentStatementPrincipalArgs(
-                            identifiers=pull_arns, type="AWS"
-                        )
-                    ],
-                    resources=[bucket_arn],
-                ),
-                GetPolicyDocumentStatementArgs(
-                    effect="Deny",
-                    actions=["s3:*"],
-                    principals=[
-                        GetPolicyDocumentStatementPrincipalArgs(
-                            identifiers=["*"], type="AWS"
-                        )
-                    ],
-                    resources=[bucket_arn, bucket_arn + "/*"],
-                    conditions=[
-                        GetPolicyDocumentStatementConditionArgs(
-                            test="NumericLessThan",
-                            variable="s3:TlsVersion",
-                            values=["1.2"],
-                        ),
-                    ],
-                ),
-            ]
-        )
+        bucket_policy = {
+            "Version": "2012-10-17",
+            "Statement": [
+                {
+                    "Sid": "",
+                    "Effect": "Allow",
+                    "Action": standard_actions,
+                    "Principal": {"AWS": pull_arns},
+                    "Resource": bucket_arn + "/*",
+                },
+                {
+                    "Sid": "",
+                    "Effect": "Allow",
+                    "Action": "s3:ListBucket",
+                    "Principal": {"AWS": pull_arns},
+                    "Resource": bucket_arn,
+                },
+                {
+                    "Sid": "",
+                    "Effect": "Deny",
+                    "Action": "s3:*",
+                    "Principal": "*",
+                    "Resource": [bucket_arn, bucket_arn + "/*"],
+                    "Condition": {"NumericLessThan": {"s3:TlsVersion": "1.2"}},
+                },
+            ],
+        }
     return bucket_policy
 
 

diff --git a/tests/test_pull.py b/tests/test_pull.py
@@ -20,30 +20,29 @@ def test_create_pull_bucket_policy():
     )
     expected = [
         {
-            "actions": ["s3:GetObject", "s3:GetObjectAcl", "s3:GetObjectVersion"],
-            "principals": [{"identifiers": ["arn-one", "arn-two"], "type": "AWS"}],
-            "resources": ["arn:aws:s3:::test-bucket/*"],
+            "Sid": "",
+            "Effect": "Allow",
+            "Action": ["s3:GetObjectVersion", "s3:GetObjectAcl", "s3:GetObject"],
+            "Principal": {"AWS": ["arn-one", "arn-two"]},
+            "Resource": "arn:aws:s3:::test-bucket/*",
         },
         {
-            "actions": ["s3:ListBucket"],
-            "principals": [{"identifiers": ["arn-one", "arn-two"], "type": "AWS"}],
-            "resources": ["arn:aws:s3:::test-bucket"],
+            "Sid": "",
+            "Effect": "Allow",
+            "Action": "s3:ListBucket",
+            "Principal": {"AWS": ["arn-one", "arn-two"]},
+            "Resource": "arn:aws:s3:::test-bucket",
         },
         {
-            "actions": ["s3:*"],
-            "conditions": [
-                {
-                    "test": "NumericLessThan",
-                    "variable": "s3:TlsVersion",
-                    "values": ["1.2"],
-                }
-            ],
-            "effect": "Deny",
-            "principals": [{"identifiers": ["*"], "type": "AWS"}],
-            "resources": ["arn:aws:s3:::test-bucket", "arn:aws:s3:::test-bucket/*"],
+            "Sid": "",
+            "Effect": "Deny",
+            "Action": "s3:*",
+            "Principal": "*",
+            "Resource": ["arn:aws:s3:::test-bucket", "arn:aws:s3:::test-bucket/*"],
+            "Condition": {"NumericLessThan": {"s3:TlsVersion": "1.2"}},
         },
     ]
-    return Output.all(policy.statements, expected).apply(
+    return Output.all(policy["Statement"], expected).apply(
         assert_pulumi_output_equals_expected
     )