chore(sdk): optimizing bucket.exists() for aws targets (#3654)

I was randomly thinking that using `ListObjectsV2Command` may not be the most efficient way to check if an object exists in a S3 bucket, since you are asking AWS to look for all the objects that have a specific prefix even though the API returns only one object when filtering with `MaxKeys: 1`. I thought that probably just checking for the metadata of the single object using `HeadObjectCommand` would be enough to determine whether the object exists. *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
winglang · Aug 2, 2023 · a561686 · a561686
1 parent 03351fc
commit a561686
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 52 deletions.
diff --git a/libs/wingsdk/src/shared-aws/bucket.inflight.ts b/libs/wingsdk/src/shared-aws/bucket.inflight.ts
@@ -1,6 +1,8 @@
 import { Readable } from "stream";
 import * as consumers from "stream/consumers";
 import {
+  HeadObjectCommand,
+  HeadObjectCommandOutput,
   DeleteObjectCommand,
   GetObjectCommand,
   ListObjectsV2Command,
@@ -27,13 +29,13 @@ export class BucketClient implements IBucketClient {
    * @param key Key of the object
    */
   public async exists(key: string): Promise<boolean> {
-    const command = new ListObjectsV2Command({
+    const command = new HeadObjectCommand({
       Bucket: this.bucketName,
-      Prefix: key,
-      MaxKeys: 1,
+      Key: key,
     });
-    const resp: ListObjectsV2CommandOutput = await this.s3Client.send(command);
-    return !!resp.Contents && resp.Contents.length > 0;
+
+    const resp: HeadObjectCommandOutput = await this.s3Client.send(command);
+    return !!resp?.ContentLength;
   }
 
   /**
@@ -235,7 +237,7 @@ export class BucketClient implements IBucketClient {
 
     if (!(await this.exists(key))) {
       throw new Error(
-        `Cannot provide public url for an non-existent key (key=${key})`
+        `Cannot provide public url for a non-existent key (key=${key})`
       );
     }
 

diff --git a/libs/wingsdk/src/shared-aws/permissions.ts b/libs/wingsdk/src/shared-aws/permissions.ts
@@ -118,7 +118,8 @@ export function calculateBucketPermissions(
     ops.includes(cloud.BucketInflightMethods.LIST) ||
     ops.includes(cloud.BucketInflightMethods.TRY_GET) ||
     ops.includes(cloud.BucketInflightMethods.TRY_GET_JSON) ||
-    ops.includes(cloud.BucketInflightMethods.PUBLIC_URL)
+    ops.includes(cloud.BucketInflightMethods.PUBLIC_URL) ||
+    ops.includes(cloud.BucketInflightMethods.EXISTS)
   ) {
     actions.push(...["s3:GetObject*", "s3:GetBucket*"]);
   }

diff --git a/libs/wingsdk/test/shared-aws/bucket.inflight.test.ts b/libs/wingsdk/test/shared-aws/bucket.inflight.test.ts
@@ -1,5 +1,6 @@
 import { Readable } from "stream";
 import {
+  HeadObjectCommand,
   DeleteObjectCommand,
   GetBucketLocationCommand,
   GetObjectCommand,
@@ -159,9 +160,15 @@ test("delete object from the bucket with mustExist option", async () => {
   s3Mock
     .on(DeleteObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .resolves({});
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [{ Key: KEY }] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({
+    AcceptRanges: "bytes",
+    ContentLength: 3191,
+    ContentType: "image/jpeg",
+    ETag: "6805f2cfc46c0f04559748bb039d69ae",
+    LastModified: new Date("Thu, 15 Dec 2016 01:19:41 GMT"),
+    Metadata: {},
+    VersionId: "null",
+  });
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -179,9 +186,7 @@ test("delete non-existent object from the bucket with mustExist option", async (
   s3Mock
     .on(DeleteObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .resolves({});
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({});
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -237,9 +242,7 @@ test("Given a public bucket when reaching to a non existent key, public url it s
   s3Mock
     .on(GetBucketLocationCommand, { Bucket: BUCKET_NAME })
     .resolves({ LocationConstraint: "us-east-2" });
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({});
 
   //WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -250,11 +253,11 @@ test("Given a public bucket when reaching to a non existent key, public url it s
   }
   // THEN
   expect(error?.message).toBe(
-    "Cannot provide public url for an non-existent key (key=KEY)"
+    "Cannot provide public url for a non-existent key (key=KEY)"
   );
 });
 
-test("Given a public bucket, when giving one of its keys, we should get it's public url", async () => {
+test("Given a public bucket, when giving one of its keys, we should get its public url", async () => {
   // GIVEN
   const BUCKET_NAME = "BUCKET_NAME";
   const KEY = "KEY";
@@ -271,9 +274,15 @@ test("Given a public bucket, when giving one of its keys, we should get it's pub
   s3Mock
     .on(GetBucketLocationCommand, { Bucket: BUCKET_NAME })
     .resolves({ LocationConstraint: REGION });
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [{}] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({
+    AcceptRanges: "bytes",
+    ContentLength: 3191,
+    ContentType: "image/jpeg",
+    ETag: "6805f2cfc46c0f04559748bb039d69ae",
+    LastModified: new Date("Thu, 15 Dec 2016 01:19:41 GMT"),
+    Metadata: {},
+    VersionId: "null",
+  });
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -289,10 +298,16 @@ test("check that an object exists in the bucket", async () => {
   // GIVEN
   const BUCKET_NAME = "BUCKET_NAME";
   const KEY = "KEY";
-  const VALUE = "VALUE";
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [{ Key: KEY }] });
+
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({
+    AcceptRanges: "bytes",
+    ContentLength: 3191,
+    ContentType: "image/jpeg",
+    ETag: "6805f2cfc46c0f04559748bb039d69ae",
+    LastModified: new Date("Thu, 15 Dec 2016 01:19:41 GMT"),
+    Metadata: {},
+    VersionId: "null",
+  });
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -306,10 +321,8 @@ test("check that an object doesn't exist in the bucket", async () => {
   // GIVEN
   const BUCKET_NAME = "BUCKET_NAME";
   const KEY = "KEY";
-  const VALUE = "VALUE";
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [] });
+
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({});
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -327,9 +340,15 @@ test("tryGet an existing object from the bucket", async () => {
   s3Mock
     .on(GetObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .resolves({ Body: createMockStream(VALUE) });
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [{ Key: KEY }] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({
+    AcceptRanges: "bytes",
+    ContentLength: 3191,
+    ContentType: "image/jpeg",
+    ETag: "6805f2cfc46c0f04559748bb039d69ae",
+    LastModified: new Date("Thu, 15 Dec 2016 01:19:41 GMT"),
+    Metadata: {},
+    VersionId: "null",
+  });
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -347,9 +366,7 @@ test("tryGet a non-existent object from the bucket", async () => {
   s3Mock
     .on(GetObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .rejects(new Error("fake error"));
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({});
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -367,9 +384,15 @@ test("tryGetJson an existing object from the bucket", async () => {
   s3Mock
     .on(GetObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .resolves({ Body: createMockStream(JSON.stringify(VALUE)) });
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [{ Key: KEY }] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({
+    AcceptRanges: "bytes",
+    ContentLength: 3191,
+    ContentType: "image/jpeg",
+    ETag: "6805f2cfc46c0f04559748bb039d69ae",
+    LastModified: new Date("Thu, 15 Dec 2016 01:19:41 GMT"),
+    Metadata: {},
+    VersionId: "null",
+  });
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -387,9 +410,7 @@ test("tryGetJson a non-existent object from the bucket", async () => {
   s3Mock
     .on(GetObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .rejects(new Error("fake error"));
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({});
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -407,9 +428,15 @@ test("tryGetJson an existing non-Json object from the bucket", async () => {
   s3Mock
     .on(GetObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .resolves({ Body: createMockStream(VALUE) });
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [{ Key: KEY }] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({
+    AcceptRanges: "bytes",
+    ContentLength: 3191,
+    ContentType: "image/jpeg",
+    ETag: "6805f2cfc46c0f04559748bb039d69ae",
+    LastModified: new Date("Thu, 15 Dec 2016 01:19:41 GMT"),
+    Metadata: {},
+    VersionId: "null",
+  });
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -426,9 +453,15 @@ test("tryDelete an existing object from the bucket", async () => {
   s3Mock
     .on(DeleteObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .resolves({});
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [{ Key: KEY }] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({
+    AcceptRanges: "bytes",
+    ContentLength: 3191,
+    ContentType: "image/jpeg",
+    ETag: "6805f2cfc46c0f04559748bb039d69ae",
+    LastModified: new Date("Thu, 15 Dec 2016 01:19:41 GMT"),
+    Metadata: {},
+    VersionId: "null",
+  });
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);
@@ -445,9 +478,7 @@ test("tryDelete a non-existent object from the bucket", async () => {
   s3Mock
     .on(DeleteObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
     .resolves({});
-  s3Mock
-    .on(ListObjectsV2Command, { Bucket: BUCKET_NAME, Prefix: KEY, MaxKeys: 1 })
-    .resolves({ Contents: [] });
+  s3Mock.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY }).resolves({});
 
   // WHEN
   const client = new BucketClient(BUCKET_NAME);

diff --git a/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/delete.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/delete.w_compile_tf-aws.md
@@ -84,7 +84,7 @@ module.exports = function({ $b }) {
             "uniqueId": "testdelete_Handler_IamRolePolicy_D6FF0B67"
           }
         },
-        "policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Action\":[\"s3:List*\",\"s3:PutObject*\",\"s3:Abort*\",\"s3:DeleteObject*\",\"s3:DeleteObjectVersion*\",\"s3:PutLifecycleConfiguration*\"],\"Resource\":[\"${aws_s3_bucket.cloudBucket.arn}\",\"${aws_s3_bucket.cloudBucket.arn}/*\"],\"Effect\":\"Allow\"}]}",
+        "policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Action\":[\"s3:List*\",\"s3:PutObject*\",\"s3:Abort*\",\"s3:GetObject*\",\"s3:GetBucket*\",\"s3:DeleteObject*\",\"s3:DeleteObjectVersion*\",\"s3:PutLifecycleConfiguration*\"],\"Resource\":[\"${aws_s3_bucket.cloudBucket.arn}\",\"${aws_s3_bucket.cloudBucket.arn}/*\"],\"Effect\":\"Allow\"}]}",
         "role": "${aws_iam_role.testdelete_Handler_IamRole_0B29F48A.name}"
       }
     },

diff --git a/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/exists.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/sdk_tests/bucket/exists.w_compile_tf-aws.md
@@ -73,7 +73,7 @@ module.exports = function({ $b }) {
             "uniqueId": "testexists_Handler_IamRolePolicy_46744240"
           }
         },
-        "policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Action\":[\"s3:List*\",\"s3:PutObject*\",\"s3:Abort*\",\"s3:DeleteObject*\",\"s3:DeleteObjectVersion*\",\"s3:PutLifecycleConfiguration*\"],\"Resource\":[\"${aws_s3_bucket.cloudBucket.arn}\",\"${aws_s3_bucket.cloudBucket.arn}/*\"],\"Effect\":\"Allow\"}]}",
+        "policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Action\":[\"s3:List*\",\"s3:PutObject*\",\"s3:Abort*\",\"s3:GetObject*\",\"s3:GetBucket*\",\"s3:DeleteObject*\",\"s3:DeleteObjectVersion*\",\"s3:PutLifecycleConfiguration*\"],\"Resource\":[\"${aws_s3_bucket.cloudBucket.arn}\",\"${aws_s3_bucket.cloudBucket.arn}/*\"],\"Effect\":\"Allow\"}]}",
         "role": "${aws_iam_role.testexists_Handler_IamRole_4F58045B.name}"
       }
     },