Skip to content
Open
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
154 changes: 109 additions & 45 deletions src/apphosting/yaml.spec.ts
Original file line number Diff line number Diff line change
@@ -1,52 +1,116 @@
import { expect } from "chai";
import { AppHostingYamlConfig } from "./yaml";

describe("merge", () => {
it("merges incoming apphosting yaml config with precendence", () => {
const apphostingYaml = AppHostingYamlConfig.empty();
apphostingYaml.env = {
ENV_1: { value: "env_1" },
ENV_2: { value: "env_2" },
SECRET: { secret: "secret_1" },
};

const incomingAppHostingYaml = AppHostingYamlConfig.empty();
incomingAppHostingYaml.env = {
ENV_1: { value: "incoming_env_1" },
ENV_3: { value: "incoming_env_3" },
SECRET_2: { value: "incoming_secret_2" },
};

apphostingYaml.merge(incomingAppHostingYaml);
expect(apphostingYaml.env).to.deep.equal({
ENV_1: { value: "incoming_env_1" },
ENV_2: { value: "env_2" },
ENV_3: { value: "incoming_env_3" },
SECRET: { secret: "secret_1" },
SECRET_2: { value: "incoming_secret_2" },
import * as sinon from "sinon";
import { AppHostingYamlConfig, toEnvMap, toEnvList } from "./yaml";
import * as configModule from "./config";
import * as utils from "../utils";
import * as fsutils from "../fsutils";
import { FirebaseError } from "../error";

describe("apphosting/yaml", () => {
let fileExistsStub: sinon.SinonStub;
let readFileStub: sinon.SinonStub;
let storeStub: sinon.SinonStub;

beforeEach(() => {
fileExistsStub = sinon.stub(fsutils, "fileExistsSync");
readFileStub = sinon.stub(utils, "readFileFromDirectory");
storeStub = sinon.stub(configModule, "store");
});

afterEach(() => {
sinon.restore();
});

describe("loadFromFile", () => {
it("should successfully load configuration with env vars", async () => {
fileExistsStub.returns(true);
readFileStub.resolves({ source: "env:\n - variable: FOO\n value: bar" });

const res = await AppHostingYamlConfig.loadFromFile("apphosting.yaml");

expect(res.filename).to.equal("apphosting.yaml");
expect(res.env).to.deep.equal({ FOO: { value: "bar" } });
});

it("should throw if file does not exist", async () => {
fileExistsStub.returns(false);

await expect(AppHostingYamlConfig.loadFromFile("missing.yaml")).to.be.rejectedWith(
FirebaseError,
/Cannot load missing.yaml from given path/,
);
});

it("should return empty env if file contains no env", async () => {
fileExistsStub.returns(true);
readFileStub.resolves({ source: "runConfig:\n cpu: 2" });

const res = await AppHostingYamlConfig.loadFromFile("apphosting.yaml");

expect(res.env).to.deep.equal({});
});
});

describe("empty", () => {
it("should create an empty config", () => {
const res = AppHostingYamlConfig.empty();
expect(res.env).to.deep.equal({});
});
});

it("conditionally allows secrets to become plaintext", () => {
const apphostingYaml = AppHostingYamlConfig.empty();
apphostingYaml.env = {
API_KEY: { secret: "api_key" },
};

const incomingYaml = AppHostingYamlConfig.empty();
incomingYaml.env = {
API_KEY: { value: "plaintext" },
};

expect(() =>
apphostingYaml.merge(incomingYaml, /* alllowSecretsToBecomePlaintext */ false),
).to.throw("Cannot convert secret to plaintext in apphosting yaml");

expect(() =>
apphostingYaml.merge(incomingYaml, /* alllowSecretsToBecomePlaintext */ true),
).to.not.throw();
expect(apphostingYaml.env).to.deep.equal({
API_KEY: { value: "plaintext" },
describe("merge", () => {
it("should override variables from incoming config", () => {
const base = AppHostingYamlConfig.empty();
base.env = { FOO: { value: "1" }, BAR: { secret: "sec" } };

const other = AppHostingYamlConfig.empty();
other.env = { FOO: { value: "2" }, BAZ: { value: "3" } };

base.merge(other, true);

expect(base.env).to.deep.equal({
FOO: { value: "2" },
BAR: { secret: "sec" },
BAZ: { value: "3" },
});
});

it("should throw when a secret turns into plaintext and allowSecretsToBecomePlaintext is false", () => {
const base = AppHostingYamlConfig.empty();
base.env = { DB_PASS: { secret: "my-secret" } };

const other = AppHostingYamlConfig.empty();
other.env = { DB_PASS: { value: "plaintext" } };

expect(() => base.merge(other, false)).to.throw(/Cannot convert secret to plaintext/);
});
});

describe("utilities", () => {
it("toEnvMap", () => {
const list = [{ variable: "FOO", value: "bar" }];
const map = toEnvMap(list);
expect(map).to.deep.equal({ FOO: { value: "bar" } });
});

it("toEnvList", () => {
const map = { FOO: { value: "bar" } };
const list = toEnvList(map);
expect(list).to.deep.equal([{ variable: "FOO", value: "bar" }]);
});
});

describe("upsertFile", () => {
it("should parse, merge, and store successfully", async () => {
fileExistsStub.returns(true);
readFileStub.resolves({ source: "env:\n - variable: FOO\n value: bar" });

const conf = AppHostingYamlConfig.empty();
conf.env = { BAZ: { value: "qux" } };

await conf.upsertFile("apphosting.yaml");

expect(storeStub).to.have.been.calledOnce;
});
});
});
2 changes: 1 addition & 1 deletion src/apphosting/yaml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@

const file = await readFileFromDirectory(dirname(filePath), basename(filePath));
config.filename = path.basename(filePath);
const loadedAppHostingYaml = (await wrappedSafeLoad(file.source)) ?? {};

Check warning on line 35 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value

if (loadedAppHostingYaml.env) {

Check warning on line 37 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .env on an `any` value
config.env = toEnvMap(loadedAppHostingYaml.env);

Check warning on line 38 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .env on an `any` value

Check warning on line 38 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `Env[]`
}

return config;
Expand All @@ -45,15 +45,15 @@
* Simply returns an empty AppHostingYamlConfig (no environment variables
* or secrets).
*/
static empty() {

Check warning on line 48 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
return new AppHostingYamlConfig();
}

/**
* Merges this AppHostingYamlConfig with another config, the incoming config
* has precedence if there are any conflicting configurations.
* */

Check warning on line 55 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Should be no multiple asterisks on end lines
merge(other: AppHostingYamlConfig, allowSecretsToBecomePlaintext: boolean = true) {

Check warning on line 56 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Type boolean trivially inferred from a boolean literal, remove type annotation

Check warning on line 56 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
if (!allowSecretsToBecomePlaintext) {
const wereSecrets = Object.entries(this.env)
.filter(([, env]) => env.secret)
Expand All @@ -75,12 +75,12 @@
* Loads the given file if it exists and updates it. If
* it does not exist a new file will be created.
*/
async upsertFile(filePath: string) {

Check warning on line 78 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function
let yamlConfigToWrite: Config = {};

if (fileExistsSync(filePath)) {
const file = await readFileFromDirectory(dirname(filePath), basename(filePath));
yamlConfigToWrite = await wrappedSafeLoad(file.source);

Check warning on line 83 in src/apphosting/yaml.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
}

yamlConfigToWrite.env = toEnvList(this.env);
Expand All @@ -95,7 +95,7 @@
envs.map((env) => {
const variable = env.variable;
const tmp = { ...env };
delete (env as any).variable;
delete (tmp as any).variable;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use of any as an escape hatch is discouraged by the repository style guide (Line 38). You can avoid it by casting to Partial<Env>, which allows the deletion of the required variable property while maintaining better type safety than any.

Suggested change
delete (tmp as any).variable;
delete (tmp as Partial<Env>).variable;
References
  1. Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. (link)

return [variable, tmp];
}),
);
Expand Down
Loading