Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sdk): local bucket files can't be accessed in simulator #7143

Merged
merged 3 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions packages/@winglang/sdk/src/shared-aws/bucket.inflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,6 @@ export class BucketClient implements IAwsBucketClient {
throw new Error("Cannot provide public url for a non-public bucket");
}

if (!(await this.exists(key))) {
throw new Error(
`Cannot provide public url for a non-existent key (key=${key})`
);
}

const region = await this.getLocation();

return encodeURI(
Expand All @@ -401,11 +395,6 @@ export class BucketClient implements IAwsBucketClient {
// Set the S3 command
switch (action) {
case BucketSignedUrlAction.DOWNLOAD:
if (!(await this.exists(key))) {
throw new Error(
`Cannot provide signed url for a non-existent key (key=${key})`
);
}
s3Command = new GetObjectCommand({
Bucket: this.bucketName,
Key: key,
Expand Down
5 changes: 0 additions & 5 deletions packages/@winglang/sdk/src/shared-azure/bucket.inflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,6 @@ export class BucketClient implements IBucketClient {
if (!accessPolicy?.blobPublicAccess) {
throw new Error("Cannot provide public url for a non-public bucket");
}
if (!(await this.exists(key))) {
throw new Error(
`Cannot provide public url for a non-existent key (key=${key})`
);
}

return encodeURI(
`https://${this.storageAccount}.blob.core.windows.net/${this.bucketName}/${key}`
Expand Down
11 changes: 0 additions & 11 deletions packages/@winglang/sdk/src/shared-gcp/bucket.inflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,6 @@ export class BucketClient implements IBucketClient {
throw new Error("Cannot provide public url for a non-public bucket");
}

if (!(await this.exists(key))) {
throw new Error(
`Cannot provide public url for a non-existent key (key=${key})`
);
}

return `https://storage.googleapis.com/${this.bucketName}/${key}`;
}

Expand All @@ -257,11 +251,6 @@ export class BucketClient implements IBucketClient {
// Set the GCS action
switch (action) {
case BucketSignedUrlAction.DOWNLOAD:
if (!(await this.exists(key))) {
throw new Error(
`Cannot provide signed url for a non-existent key (key=${key})`
);
}
gcsAction = "read";
break;
case BucketSignedUrlAction.UPLOAD:
Expand Down
38 changes: 11 additions & 27 deletions packages/@winglang/sdk/src/target-sim/bucket.inflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as crypto from "crypto";
import * as fs from "fs";
import { Server } from "http";
import { dirname, join } from "path";
import { pathToFileURL } from "url";
import express from "express";
import mime from "mime-types";
import { BucketAttributes, BucketSchema } from "./schema-resources";
Expand Down Expand Up @@ -141,22 +140,24 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance {
return;
});

// Handle signed URL downloads.
// Handle file accesses and signed URL downloads.
this.app.get("*", (req, res) => {
const action = req.query.action;
if (action !== BucketSignedUrlAction.DOWNLOAD) {
return res.status(403).send("Operation not allowed");
}
if (!this._public) {
if (action !== BucketSignedUrlAction.DOWNLOAD) {
return res.status(403).send("Operation not allowed");
}

const validUntil = req.query.validUntil?.toString();
if (!validUntil || Date.now() > parseInt(validUntil)) {
return res.status(403).send("Signed URL has expired");
const validUntil = req.query.validUntil?.toString();
if (!validUntil || Date.now() > parseInt(validUntil)) {
return res.status(403).send("Signed URL has expired");
}
}

const key = req.path.slice(1); // remove leading slash
const hash = this.hashKey(key);
const filename = join(this._fileDir, hash);
return res.download(filename);
return res.sendFile(filename);
});
}

Expand Down Expand Up @@ -458,15 +459,7 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance {
return this.context.withTrace({
message: `Public URL (key=${key}).`,
activity: async () => {
const filePath = join(this._fileDir, key);

if (!this._metadata.has(key)) {
throw new Error(
`Cannot provide public url for an non-existent key (key=${key})`
);
}

return pathToFileURL(filePath).href;
return new URL(key, this.url).toString();
},
});
}
Expand All @@ -480,15 +473,6 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance {
// a POJO with seconds, but TypeScript thinks otherwise.
const duration = options?.duration?.seconds ?? 900;

if (
action === BucketSignedUrlAction.DOWNLOAD &&
!(await this.exists(key))
) {
throw new Error(
`Cannot provide signed url for a non-existent key (key=${key})`
);
}

const url = new URL(key, this.url);
url.searchParams.set("action", action);
url.searchParams.set(
Expand Down
56 changes: 0 additions & 56 deletions packages/@winglang/sdk/test/shared-aws/bucket.inflight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,38 +254,6 @@ test("Given a non public bucket when reaching to a key public url it should thro
);
});

test("Given a public bucket when reaching to a non-existent key, public url it should throw an error", async () => {
// GIVEN
let error;
const BUCKET_NAME = "BUCKET_NAME";
const KEY = "KEY";

s3Mock.on(GetPublicAccessBlockCommand, { Bucket: BUCKET_NAME }).resolves({
PublicAccessBlockConfiguration: {
BlockPublicAcls: false,
BlockPublicPolicy: false,
RestrictPublicBuckets: false,
IgnorePublicAcls: false,
},
});
s3Mock
.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
.rejects(new NotFound({ message: "Object not found", $metadata: {} }));

// WHEN
const client = new BucketClient({ $bucketName: BUCKET_NAME });
try {
await client.publicUrl(KEY);
} catch (err) {
error = err;
}

// THEN
expect(error?.message).toBe(
"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 its public url", async () => {
// GIVEN
const BUCKET_NAME = "BUCKET_NAME";
Expand Down Expand Up @@ -557,30 +525,6 @@ test("tryDelete a non-existent object from the bucket", async () => {
expect(objectTryDelete).toEqual(false);
});

test("Given a bucket when reaching to a non-existent key, signed url it should throw an error", async () => {
// GIVEN
let error;
const BUCKET_NAME = "BUCKET_NAME";
const KEY = "KEY";

s3Mock
.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
.rejects(new NotFound({ message: "Object not found", $metadata: {} }));

// WHEN
const client = new BucketClient({ $bucketName: BUCKET_NAME });
try {
await client.signedUrl(KEY);
} catch (err) {
error = err;
}

// THEN
expect(error?.message).toBe(
`Cannot provide signed url for a non-existent key (key=${KEY})`
);
});

// Skipped due to issue with mocking getSignedUrl:
// https://github.com/m-radzikowski/aws-sdk-client-mock/issues/62
test.skip("Given a bucket, when giving one of its keys, we should get its signed url", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,31 +409,6 @@ test("Given a non public bucket when reaching to a key public url it should thro
);
});

test("Given a public bucket when reaching to a non existent key, public url it should throw an error", async () => {
// GIVEN
const BUCKET_NAME = "BUCKET_NAME";
const STORAGE_NAME = "STORAGE_NAME";
const KEY = "non-existent-key";

// WHEN
const client = new BucketClient({
$bucketName: BUCKET_NAME,
$storageAccount: STORAGE_NAME,
$blobServiceClient: mockBlobServiceClient,
});
TEST_PATH = "sad";
// @ts-expect-error - accessing private property
client.containerClient.getAccessPolicy = vi
.fn()
.mockResolvedValue({ blobPublicAccess: "container" });
client.exists = vi.fn().mockResolvedValue(false);

// THEN
await expect(() => client.publicUrl(KEY)).rejects.toThrowError(
`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 its public url", async () => {
// GIVEN
const BUCKET_NAME = "BUCKET_NAME";
Expand Down
86 changes: 30 additions & 56 deletions packages/@winglang/sdk/test/target-sim/bucket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ test("Given a non public bucket when reaching to a key public url it should thro
await s.stop();
});

test("Given a public bucket when reaching to a non existent key, public url it should throw an error", async () => {
test("Given a public bucket, when giving one of its keys, we should get its public url", async () => {
// GIVEN
const app = new SimApp();
new cloud.Bucket(app, "my_bucket", { public: true });
Expand All @@ -583,15 +583,18 @@ test("Given a public bucket when reaching to a non existent key, public url it s
const client = s.getResource("/my_bucket") as cloud.IBucketClient;

const KEY = "KEY";
const VALUE = "VALUE";

// WHEN
await client.put(KEY, VALUE);
const response = await client.publicUrl(KEY);

// THEN
await expect(() => client.publicUrl(KEY)).rejects.toThrowError(
/Cannot provide public url for an non-existent key/
);
await s.stop();
expect(response).toMatch(/https?:\/\/(localhost|127\.0\.0\.1):\d+\/KEY/);
});

test("Given a public bucket, when giving one of its keys, we should get its public url", async () => {
test("accessing the publicUrl of a valid key should return the file contents", async () => {
// GIVEN
const app = new SimApp();
new cloud.Bucket(app, "my_bucket", { public: true });
Expand All @@ -604,12 +607,31 @@ test("Given a public bucket, when giving one of its keys, we should get its publ

// WHEN
await client.put(KEY, VALUE);
const response = await client.publicUrl(KEY);
const publicUrl = await client.publicUrl(KEY);

// THEN
const response = await fetch(publicUrl);
const text = await response.text();
expect(text).toBe(VALUE);
});

test("accessing the publicUrl of a non existent key should throw an error", async () => {
// GIVEN
const app = new SimApp();
new cloud.Bucket(app, "my_bucket", { public: true });

const s = await app.startSimulator();
const client = s.getResource("/my_bucket") as cloud.IBucketClient;

const KEY = "KEY";

// WHEN
const publicUrl = await client.publicUrl(KEY);
const response = await fetch(publicUrl);

// THEN
expect(response.status).toBe(404);
await s.stop();
// file paths are different on windows and linux
expect(response.endsWith("KEY")).toBe(true);
});

test("check if an object exists in the bucket", async () => {
Expand Down Expand Up @@ -994,51 +1016,3 @@ test("signedUrl doesn't accept multipart uploads yet", async () => {

await s.stop();
});

// Deceided to seperate this feature in a different release,(see https://github.com/winglang/wing/issues/4143)

// test("Given a bucket when reaching to a non existent key, signed url it should throw an error", async () => {
// //GIVEN
// let error;
// const app = new SimApp();
// new cloud.Bucket(app, "my_bucket", { public: true });

// const s = await app.startSimulator();
// const client = s.getResource("/my_bucket") as cloud.IBucketClient;

// const KEY = "KEY";

// // WHEN
// try {
// await client.signedUrl(KEY);
// } catch (err) {
// error = err;
// }

// expect(error?.message).toBe(
// "Cannot provide signed url for an non-existent key (key=${KEY})"
// );
// // THEN
// await s.stop();
// });

// test("Given a bucket, when giving one of its keys, we should get it's signed url", async () => {
// // GIVEN
// const app = new SimApp();
// new cloud.Bucket(app, "my_bucket", { public: true });

// const s = await app.startSimulator();
// const client = s.getResource("/my_bucket") as cloud.IBucketClient;

// const KEY = "KEY";
// const VALUE = "VALUE";

// // WHEN
// await client.put(KEY, VALUE);
// const response = await client.signedUrl(KEY);

// // THEN
// await s.stop();
// const filePath = `${client.fileDir}/${KEY}`;
// expect(response).toEqual(url.pathToFileURL(filePath).searchParams.append("Expires","86400"));
// });
6 changes: 1 addition & 5 deletions tests/sdk_tests/bucket/public_url.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ test "publicUrl" {

let publicUrl = publicBucket.publicUrl("file1.txt");
assert(publicUrl != "");

// TODO: works in aws, doesn't work in sim since publicUrl is returning a path to the file, remove condition when #2833 is resolved.
if (util.env("WING_TARGET") != "sim") {
assert(http.get(publicUrl).body == "Foo");
}
assert(http.get(publicUrl).body == "Foo");

assertThrows(BUCKET_NOT_PUBLIC_ERROR, () => {
privateBucket.publicUrl("file2.txt");
Expand Down
19 changes: 0 additions & 19 deletions tests/sdk_tests/bucket/signed_url.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,6 @@ test "signedUrl GET (explicit)" {
expect.equal(output, VALUE);
}

test "signedUrl GET with non-existent key" {
let assertThrows = (expected: str, block: (): void) => {
let var error = false;
try {
block();
} catch actual {
expect.equal(actual, expected);
error = true;
}
expect.equal(error, true);
};
let UNEXISTING_KEY = "no-such-file.txt";
let OBJECT_DOES_NOT_EXIST_ERROR = "Cannot provide signed url for a non-existent key (key={UNEXISTING_KEY})";

assertThrows(OBJECT_DOES_NOT_EXIST_ERROR, () => {
bucket.signedUrl(UNEXISTING_KEY);
});
}

test "signedUrl PUT" {
let KEY = "tempfile.txt";
let VALUE = "Hello, Wing!";
Expand Down
Loading