PLAT-6563: monitoring s3 bucket object ownership

Set ownership to BucketOwnerPreferred.
dominodatalab · Apr 19, 2023 · 40fd442 · 40fd442
1 parent e1ed1bb
commit 40fd442
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 7 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -19,7 +19,7 @@ jobs:
     runs-on: ubuntu-latest
 
     env:
-      DEPLOYER_IMAGE: quay.io/domino/deployer:develop.f72b81d5db7e04cc48478d310eafa4abb927ce7f
+      DEPLOYER_IMAGE: quay.io/domino/deployer:release-v62.latest
       AWS_REGION: us-west-2
 
     defaults:
@@ -47,7 +47,7 @@ jobs:
     - name: Install dependencies
       run: |
         pip install -r requirements.txt
-        pip install awscli==1.25.57 build
+        pip install awscli build
     - name: Install aws-cdk
       run: npm install -g aws-cdk@$(pip freeze | grep aws-cdk.core | sed -e 's/.*==//')
     - name: Lint with flake8/black/isort

diff --git a/cdk/domino_cdk/config/iam.py b/cdk/domino_cdk/config/iam.py
@@ -3,7 +3,13 @@
 
 # Future TODO item: Incorporate IAM reqs into the provisioning
 # classes so we can generate exact perms for a given deployment
-def generate_iam(stack_name: str, aws_account_id: str, region: str, manual: bool = False, use_bastion: bool = False):
+def generate_iam(
+    stack_name: str,
+    aws_account_id: str,
+    region: str,
+    manual: bool = False,
+    use_bastion: bool = False,
+):
     partition = Fact.require_fact(region, FactName.PARTITION)
 
     if manual:
@@ -61,6 +67,7 @@ def do_cf():
             "s3:PutAccountPublicAccessBlock",
             "s3:PutBucketAcl",
             "s3:PutBucketLogging",
+            "s3:PutBucketOwnershipControls",
             "s3:PutBucketPolicy",
             "s3:PutBucketTagging",
             "s3:PutBucketVersioning",

diff --git a/cdk/domino_cdk/provisioners/s3.py b/cdk/domino_cdk/provisioners/s3.py
@@ -8,14 +8,23 @@
     Bucket,
     BucketAccessControl,
     BucketEncryption,
+    ObjectOwnership,
 )
 from aws_cdk.region_info import Fact, FactName
 
 from domino_cdk import config
 
 
 class DominoS3Provisioner:
-    def __init__(self, parent: cdk.Construct, construct_id: str, name: str, s3: config.S3, nest: bool, **kwargs):
+    def __init__(
+        self,
+        parent: cdk.Construct,
+        construct_id: str,
+        name: str,
+        s3: config.S3,
+        nest: bool,
+        **kwargs,
+    ):
         self.parent = parent
         self.scope = cdk.NestedStack(self.parent, construct_id, **kwargs) if nest else self.parent
         self.monitoring_bucket: Optional[Bucket] = None
@@ -28,6 +37,7 @@ def _provision_bucket(
         attrs: config.S3.BucketList.Bucket,
         server_access_logs_bucket: Optional[Bucket] = None,
         require_encryption: bool = True,
+        object_ownership: Optional[ObjectOwnership] = None,
         **kwargs,
     ) -> Bucket:
         use_sse_kms_key = False
@@ -50,6 +60,7 @@ def _provision_bucket(
             server_access_logs_prefix=f"{bucket_name}/" if server_access_logs_bucket else None,
             server_access_logs_bucket=server_access_logs_bucket,
             versioned=True,
+            object_ownership=object_ownership,
             **kwargs,
         )
 
@@ -99,11 +110,13 @@ def provision_buckets(self, stack_name: str, s3: config.S3):
                 s3.buckets.monitoring,
                 access_control=BucketAccessControl.LOG_DELIVERY_WRITE,
                 require_encryption=False,
+                object_ownership=ObjectOwnership.BUCKET_OWNER_PREFERRED,
             )
 
             region = cdk.Stack.of(self.scope).region
             self.monitoring_bucket.grant_put(
-                iam.AccountPrincipal(Fact.require_fact(region, FactName.ELBV2_ACCOUNT)), "*"
+                iam.AccountPrincipal(Fact.require_fact(region, FactName.ELBV2_ACCOUNT)),
+                "*",
             )
 
             self.monitoring_bucket.add_to_resource_policy(
@@ -132,10 +145,19 @@ def provision_buckets(self, stack_name: str, s3: config.S3):
                 )
             )
 
-            cdk.CfnOutput(self.parent, "monitoring-bucket-output", value=self.monitoring_bucket.bucket_name)
+            cdk.CfnOutput(
+                self.parent,
+                "monitoring-bucket-output",
+                value=self.monitoring_bucket.bucket_name,
+            )
 
         self.buckets = {
-            bucket: self._provision_bucket(stack_name, bucket, attrs, server_access_logs_bucket=self.monitoring_bucket)
+            bucket: self._provision_bucket(
+                stack_name,
+                bucket,
+                attrs,
+                server_access_logs_bucket=self.monitoring_bucket,
+            )
             for bucket, attrs in vars(s3.buckets).items()
             if attrs and bucket != "monitoring"  # skipping NoneType bucket is for tests, config prevents loading
         }