PLAT-6563: monitoring s3 bucket object ownership

Set ownership to BucketOwnerPreferred.
dominodatalab · Apr 18, 2023 · 1046b85 · 1046b85
1 parent 588af32
commit 1046b85
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 11 deletions.
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
         }

diff --git a/convert/terraform/main.tf.json b/convert/terraform/main.tf.json
@@ -2,13 +2,15 @@
   "//": "This file is JSON for compatibility with Domino tooling",
   "module": {
     "eks": {
-      "source": "github.com/dominodatalab/terraform-aws-eks.git?ref=v2.0.0",
+      "source": "github.com/dominodatalab/terraform-aws-eks.git?ref=v2.0.2",
       "deploy_id": "${var.deploy_id}",
       "region": "${var.region}",
       "tags": "${var.tags}",
       "default_node_groups": "${var.default_node_groups}",
       "route53_hosted_zone_name": "${var.route53_hosted_zone_name}",
-      "bastion": { "enabled": true },
+      "bastion": {
+        "enabled": true
+      },
       "ssh_pvt_key_path": "${var.ssh_key_path}",
       "storage": {
         "s3": {
@@ -18,7 +20,10 @@
           "force_destroy_on_deletion": "${var.ecr_force_destroy_on_deletion}"
         }
       },
-      "kms": { "enabled": "${var.use_kms}", "key_id": "${var.kms_key_id}" },
+      "kms": {
+        "enabled": "${var.use_kms}",
+        "key_id": "${var.kms_key_id}"
+      },
       "eks": {
         "k8s_version": "${var.k8s_version}",
         "kubeconfig": {
@@ -65,9 +70,15 @@
     "EXECUTOR_EFS_AP_ID": {
       "value": "${module.eks.storage.efs.access_point.id}"
     },
-    "BASTION_IP": { "value": "${module.eks.bastion.public_ip}" },
-    "KMS_KEY_ID": { "value": "${try(module.eks.kms.key_id, null)}" },
-    "KMS_KEY_ARN": { "value": "${try(module.eks.kms.key_arn, null)}" },
+    "BASTION_IP": {
+      "value": "${module.eks.bastion.public_ip}"
+    },
+    "KMS_KEY_ID": {
+      "value": "${try(module.eks.kms.key_id, null)}"
+    },
+    "KMS_KEY_ARN": {
+      "value": "${try(module.eks.kms.key_arn, null)}"
+    },
     "CONTAINER_REGISTRY": {
       "value": "${module.eks.storage.ecr.container_registry}"
     }