Skip to content

Commit

Permalink
Update /internal/setRules endpoint; fix ruleset file watcher (#4337)
Browse files Browse the repository at this point in the history 
* Update internal/setRules endpoint to create new storage manager with updated ruleset; fix file watcher

* Address PR feedback

* Add CHANGELOG entry
  • Loading branch information
@tohhsinpei
tohhsinpei committed Mar 30, 2022
1 parent 366aece commit 7757ea1
Show file tree
Hide file tree
Showing 6 changed files with 265 additions and 103 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- Fixes bug where resumable uploads were not setting custom metadata on upload (#3398).
- Fixes bug where GCS metadataUpdate cloud functions were triggered in incorrect situations (#3398).
- Fixes bug where quoted escape sequences in .env files were incompletely unescaped. (#4270)
- Fixes Storage Emulator ruleset file watcher (#4337).
68 changes: 23 additions & 45 deletions scripts/storage-emulator-integration/rules/manager.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,18 @@ import { RulesetOperationMethod, SourceFile } from "../../../src/emulator/storag
import { isPermitted } from "../../../src/emulator/storage/rules/utils";
import { readFile } from "../../../src/fsutils";

const EMULATOR_LOAD_RULESET_DELAY_MS = 2000;

describe("Storage Rules Manager", function () {
const rulesRuntime = new StorageRulesRuntime();
const rules = [
{ resource: "bucket_0", rules: StorageRulesFiles.readWriteIfTrue },
{ resource: "bucket_1", rules: StorageRulesFiles.readWriteIfAuth },
];
const opts = { method: RulesetOperationMethod.GET, file: {}, path: "/b/bucket_2/o/" };
const opts = { method: RulesetOperationMethod.GET, file: {}, path: "/b/bucket_0/o/" };
let rulesManager: StorageRulesManager;

// eslint-disable-next-line @typescript-eslint/no-invalid-this
this.timeout(TIMEOUT_LONG);

beforeEach(async () => {
await rulesRuntime.start();

rulesManager = createStorageRulesManager(rules, rulesRuntime);
await rulesManager.start();
});

afterEach(async () => {
Expand All @@ -40,47 +35,28 @@ describe("Storage Rules Manager", function () {
});

it("should load multiple rulesets on start", async () => {
const rules = [
{ resource: "bucket_0", rules: StorageRulesFiles.readWriteIfTrue },
{ resource: "bucket_1", rules: StorageRulesFiles.readWriteIfAuth },
];
rulesManager = createStorageRulesManager(rules, rulesRuntime);
await rulesManager.start();

const bucket0Ruleset = rulesManager.getRuleset("bucket_0");
expect(await isPermitted({ ...opts, ruleset: bucket0Ruleset! })).to.be.true;
expect(await isPermitted({ ...opts, path: "/b/bucket_0/o/", ruleset: bucket0Ruleset! })).to.be
.true;

const bucket1Ruleset = rulesManager.getRuleset("bucket_1");
expect(await isPermitted({ ...opts, ruleset: bucket1Ruleset! })).to.be.false;
expect(await isPermitted({ ...opts, path: "/b/bucket_1/o/", ruleset: bucket1Ruleset! })).to.be
.false;
});

it("should load single ruleset on start", async () => {
const otherRulesManager = createStorageRulesManager(
StorageRulesFiles.readWriteIfTrue,
rulesRuntime
);
await otherRulesManager.start();
rulesManager = createStorageRulesManager(StorageRulesFiles.readWriteIfTrue, rulesRuntime);
await rulesManager.start();

const ruleset = otherRulesManager.getRuleset("default");
const ruleset = rulesManager.getRuleset("bucket");
expect(await isPermitted({ ...opts, ruleset: ruleset! })).to.be.true;

await otherRulesManager.stop();
});

it("should load ruleset on update with SourceFile object", async () => {
expect(rulesManager.getRuleset("bucket_2")).to.be.undefined;
await rulesManager.updateSourceFile(StorageRulesFiles.readWriteIfTrue, "bucket_2");
expect(rulesManager.getRuleset("bucket_2")).not.to.be.undefined;
});

it("should set source file", async () => {
await rulesManager.updateSourceFile(StorageRulesFiles.readWriteIfTrue, "bucket_2");

expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket_2")! })).to.be
.true;

const issues = await rulesManager.updateSourceFile(
StorageRulesFiles.readWriteIfAuth,
"bucket_2"
);

expect(issues.errors.length).to.equal(0);
expect(issues.warnings.length).to.equal(0);
expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket_2")! })).to.be
.false;
});

it("should reload ruleset on changes to source file", async () => {
Expand All @@ -91,15 +67,17 @@ describe("Storage Rules Manager", function () {
persistence.appendBytes(fileName, Buffer.from(StorageRulesFiles.readWriteIfTrue.content));

const sourceFile = getSourceFile(testDir, fileName);
await rulesManager.updateSourceFile(sourceFile, "bucket_2");
expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket_2")! })).to.be
.true;
rulesManager = createStorageRulesManager(sourceFile, rulesRuntime);
await rulesManager.start();

expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket")! })).to.be.true;

// Write new rules to file
persistence.deleteFile(fileName);
persistence.appendBytes(fileName, Buffer.from(StorageRulesFiles.readWriteIfAuth.content));

expect(await isPermitted(opts)).to.be.false;
await new Promise((resolve) => setTimeout(resolve, EMULATOR_LOAD_RULESET_DELAY_MS));
expect(await isPermitted({ ...opts, ruleset: rulesManager.getRuleset("bucket")! })).to.be.false;
});
});

Expand Down
141 changes: 140 additions & 1 deletion scripts/storage-emulator-integration/tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as puppeteer from "puppeteer";
import { Bucket, Storage, CopyOptions } from "@google-cloud/storage";
import supertest = require("supertest");

import { IMAGE_FILE_BASE64 } from "../../src/test/emulators/fixtures";
import { IMAGE_FILE_BASE64, StorageRulesFiles } from "../../src/test/emulators/fixtures";
import { TriggerEndToEndTest } from "../integration-helpers/framework";
import {
createRandomFile,
Expand Down Expand Up @@ -1141,6 +1141,145 @@ describe("Storage emulator", () => {
});
});

emulatorSpecificDescribe("Internal Endpoints", () => {
before(async function (this) {
this.timeout(TEST_SETUP_TIMEOUT);
test = new TriggerEndToEndTest(FIREBASE_PROJECT, __dirname, emulatorConfig);
await test.startEmulators(["--only", "storage"]);
});

after(async () => {
await test.stopEmulators();
});

describe("setRules", () => {
it("should set single ruleset", async () => {
await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [StorageRulesFiles.readWriteIfTrue],
},
})
.expect(200);
});

it("should set multiple rules/resource objects", async () => {
await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [
{ resource: "bucket_0", ...StorageRulesFiles.readWriteIfTrue },
{ resource: "bucket_1", ...StorageRulesFiles.readWriteIfAuth },
],
},
})
.expect(200);
});

it("should overwrite single ruleset with multiple rules/resource objects", async () => {
await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [StorageRulesFiles.readWriteIfTrue],
},
})
.expect(200);

await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [
{ resource: "bucket_0", ...StorageRulesFiles.readWriteIfTrue },
{ resource: "bucket_1", ...StorageRulesFiles.readWriteIfAuth },
],
},
})
.expect(200);
});

it("should return 400 if rules.files array is missing", async () => {
const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({ rules: {} })
.expect(400)
.then((res) => res.body.message);

expect(errorMessage).to.equal("Request body must include 'rules.files' array");
});

it("should return 400 if rules.files array has missing name field", async () => {
const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [{ content: StorageRulesFiles.readWriteIfTrue.content }],
},
})
.expect(400)
.then((res) => res.body.message);

expect(errorMessage).to.equal(
"Each member of 'rules.files' array must contain 'name' and 'content'"
);
});

it("should return 400 if rules.files array has missing content field", async () => {
const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [{ name: StorageRulesFiles.readWriteIfTrue.name }],
},
})
.expect(400)
.then((res) => res.body.message);

expect(errorMessage).to.equal(
"Each member of 'rules.files' array must contain 'name' and 'content'"
);
});

it("should return 400 if rules.files array has missing resource field", async () => {
const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [
{ resource: "bucket_0", ...StorageRulesFiles.readWriteIfTrue },
StorageRulesFiles.readWriteIfAuth,
],
},
})
.expect(400)
.then((res) => res.body.message);

expect(errorMessage).to.equal(
"Each member of 'rules.files' array must contain 'name', 'content', and 'resource'"
);
});

it("should return 400 if rules.files array has invalid content", async () => {
const errorMessage = await supertest(STORAGE_EMULATOR_HOST)
.put("/internal/setRules")
.send({
rules: {
files: [{ name: StorageRulesFiles.readWriteIfTrue.name, content: "foo" }],
},
})
.expect(400)
.then((res) => res.body.message);

expect(errorMessage).to.equal(
"There was an error updating rules, see logs for more details"
);
});
});
});

/**
* TODO(abhisun): Add test coverage to validate how many times various cloud functions are triggered.
*/
Expand Down
16 changes: 13 additions & 3 deletions src/emulator/storage/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { createApp } from "./server";
import { StorageLayer, StoredFile } from "./files";
import { EmulatorLogger } from "../emulatorLogger";
import { createStorageRulesManager, StorageRulesManager } from "./rules/manager";
import { StorageRulesRuntime } from "./rules/runtime";
import { StorageRulesIssues, StorageRulesRuntime } from "./rules/runtime";
import { SourceFile } from "./rules/types";
import express = require("express");
import {
Expand Down Expand Up @@ -42,7 +42,7 @@ export class StorageEmulator implements EmulatorInstance {

private _logger = EmulatorLogger.forEmulator(Emulators.STORAGE);
private _rulesRuntime: StorageRulesRuntime;
private _rulesManager: StorageRulesManager;
private _rulesManager!: StorageRulesManager;
private _files: Map<string, StoredFile> = new Map();
private _buckets: Map<string, CloudStorageBucketMetadata> = new Map();
private _cloudFunctions: StorageCloudFunctions;
Expand All @@ -54,7 +54,7 @@ export class StorageEmulator implements EmulatorInstance {

constructor(private args: StorageEmulatorArgs) {
this._rulesRuntime = new StorageRulesRuntime();
this._rulesManager = createStorageRulesManager(this.args.rules, this._rulesRuntime);
this._rulesManager = this.createRulesManager(this.args.rules);
this._cloudFunctions = new StorageCloudFunctions(args.projectId);
this._persistence = new Persistence(this.getPersistenceTmpDir());
this._uploadService = new UploadService(this._persistence);
Expand Down Expand Up @@ -141,6 +141,16 @@ export class StorageEmulator implements EmulatorInstance {
return this._app!;
}

async replaceRules(rules: SourceFile | RulesConfig[]): Promise<StorageRulesIssues> {
await this._rulesManager.stop();
this._rulesManager = this.createRulesManager(rules);
return this._rulesManager.start();
}

private createRulesManager(rules: SourceFile | RulesConfig[]): StorageRulesManager {
return createStorageRulesManager(rules, this._rulesRuntime);
}

private getPersistenceTmpDir(): string {
return `${tmpdir()}/firebase/storage/blobs`;
}
Expand Down

0 comments on commit 7757ea1

Please sign in to comment.