Merge pull request #29 from alliance-genome/managed_s3_bucket_permiss…

…ions

Managed s3 bucket permissions

pipeline/aws_infra/cdk_classes/aws_batch.py

@@ -16,7 +16,8 @@ class PaviExecutionEnvironment:
 
     compute_environment: aws_batch.ManagedEc2EcsComputeEnvironment
     job_queue: aws_batch.JobQueue
-    nf_workdir_bucket: s3.Bucket | s3.IBucket
+    nf_output_bucket: s3.Bucket | s3.IBucket
+    nf_bucket_access_policy: iam.ManagedPolicy
 
     def __init__(self, scope: Stack, env_suffix: str, shared_work_dir_bucket: Optional[str]) -> None:
         """
@@ -36,7 +37,7 @@ def __init__(self, scope: Stack, env_suffix: str, shared_work_dir_bucket: Option
             bucket_name = 'agr-pavi-pipeline-nextflow'
             if env_suffix:
                 bucket_name += f'-{env_suffix}'
-            self.nf_workdir_bucket = s3.Bucket(
+            self.nf_output_bucket = s3.Bucket(
                 scope=scope, id='pavi-pipeline-nf-workdir-bucket',
                 bucket_name=bucket_name,
                 access_control=s3.BucketAccessControl.PRIVATE,
@@ -46,10 +47,10 @@ def __init__(self, scope: Stack, env_suffix: str, shared_work_dir_bucket: Option
                 removal_policy=RemovalPolicy.RETAIN,
                 versioned=False
             )
-            cdk_tags.of(self.nf_workdir_bucket).add("Product", "PAVI")
-            cdk_tags.of(self.nf_workdir_bucket).add("Managed_by", "PAVI")
+            cdk_tags.of(self.nf_output_bucket).add("Product", "PAVI")  # type: ignore
+            cdk_tags.of(self.nf_output_bucket).add("Managed_by", "PAVI")  # type: ignore
         else:
-            self.nf_workdir_bucket = s3.Bucket.from_bucket_name(
+            self.nf_output_bucket = s3.Bucket.from_bucket_name(
                 scope=scope, id='pavi-pipeline-nf-workdir-bucket',
                 bucket_name=shared_work_dir_bucket)
 
@@ -60,31 +61,35 @@ def __init__(self, scope: Stack, env_suffix: str, shared_work_dir_bucket: Option
                     sid="S3BucketWriteAll",
                     effect=iam.Effect.ALLOW,
                     actions=['s3:Put*'],
-                    resources=[self.nf_workdir_bucket.bucket_arn + '/*']
+                    resources=[self.nf_output_bucket.bucket_arn + '/*']
                 ),
                 iam.PolicyStatement(
                     sid="S3BucketReadAll",
                     effect=iam.Effect.ALLOW,
                     actions=['s3:ListBucket*', 's3:Get*'],
-                    resources=[self.nf_workdir_bucket.bucket_arn, self.nf_workdir_bucket.bucket_arn + '/*']
+                    resources=[self.nf_output_bucket.bucket_arn, self.nf_output_bucket.bucket_arn + '/*']
                 )
             ]
         )
-        cdk_tags.of(s3_workdir_bucket_policy_doc).add("Product", "PAVI")
-        cdk_tags.of(s3_workdir_bucket_policy_doc).add("Managed_by", "PAVI")
+        s3_workdir_bucket_policy = iam.ManagedPolicy(scope, 'pavi-s3-nextflow-bucket-policy',
+                                                     managed_policy_name='agr-pavi-pipeline-nf-bucket-access',
+                                                     description='Grant required access to PAVI pipeline nextflow bucket to run nextflow pipelines using it.',
+                                                     document=s3_workdir_bucket_policy_doc)
+        cdk_tags.of(s3_workdir_bucket_policy).add("Product", "PAVI")  # type: ignore
+        cdk_tags.of(s3_workdir_bucket_policy).add("Managed_by", "PAVI")  # type: ignore
+
+        self.nf_bucket_access_policy = s3_workdir_bucket_policy
 
         instance_role = iam.Role(scope, 'pavi-pipeline-compute-environment-instance-role',
                                  description='Role granting permissions for Nextflow ECS execution',
                                  assumed_by=iam.ServicePrincipal('ec2.amazonaws.com'),  # type: ignore
                                  managed_policies=[
                                      iam.ManagedPolicy.from_aws_managed_policy_name("service-role/AmazonEC2ContainerServiceforEC2Role"),
-                                     iam.ManagedPolicy.from_managed_policy_name(scope, "iam-ecr-read-policy", "ReadOnlyAccessECR")
-                                 ],
-                                 inline_policies={
-                                     's3-workdir-policy': s3_workdir_bucket_policy_doc
-                                 })
-        cdk_tags.of(instance_role).add("Product", "PAVI")
-        cdk_tags.of(instance_role).add("Managed_by", "PAVI")
+                                     iam.ManagedPolicy.from_managed_policy_name(scope, "iam-ecr-read-policy", "ReadOnlyAccessECR"),
+                                     self.nf_bucket_access_policy
+                                 ])
+        cdk_tags.of(instance_role).add("Product", "PAVI")  # type: ignore
+        cdk_tags.of(instance_role).add("Managed_by", "PAVI")  # type: ignore
 
         ce_name = 'pavi_pipeline_ecs'
         if env_suffix:
@@ -100,8 +105,8 @@ def __init__(self, scope: Stack, env_suffix: str, shared_work_dir_bucket: Option
 
         self.compute_environment.apply_removal_policy(RemovalPolicy.DESTROY)
 
-        cdk_tags.of(self.compute_environment).add("Product", "PAVI")
-        cdk_tags.of(self.compute_environment).add("Managed_by", "PAVI")
+        cdk_tags.of(self.compute_environment).add("Product", "PAVI")  # type: ignore
+        cdk_tags.of(self.compute_environment).add("Managed_by", "PAVI")  # type: ignore
 
         self.compute_environment.tags.set_tag('Name', 'PAVI pipeline execution worker', priority=None, apply_to_launched_instances=True)
 
@@ -116,5 +121,5 @@ def __init__(self, scope: Stack, env_suffix: str, shared_work_dir_bucket: Option
 
         self.job_queue.add_compute_environment(self.compute_environment, order=1)  # type: ignore
 
-        cdk_tags.of(self.job_queue).add("Product", "PAVI")
-        cdk_tags.of(self.job_queue).add("Managed_by", "PAVI")
+        cdk_tags.of(self.job_queue).add("Product", "PAVI")  # type: ignore
+        cdk_tags.of(self.job_queue).add("Managed_by", "PAVI")  # type: ignore

pipeline/aws_infra/tests/unit/test_cdk_infra_stack.py

@@ -57,6 +57,16 @@ def test_pipeline_nf_s3_bucket() -> None:
     })
 
 
+# The S3 bucket policy is assigned to the GH-actions role for PAVI, in addition to PAVI-managed resources.
+# Changing the name of this policy might require the gh-actions role to be updated to include the new role.
+def test_pipeline_nf_s3_bucket_policy() -> None:
+    template.has_resource(type=ResourceType.of('AWS::IAM::ManagedPolicy').compliance_resource_type, props={
+        "Properties": {
+            "ManagedPolicyName": "agr-pavi-pipeline-nf-bucket-access"
+        }
+    })
+
+
 # Below tests check for resource that must be available in the stack,
 # but which can be replaced/renamed without manual interventions other than
 # updating all references to those resources to use the new name (check all code).