Skip to content
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- App Hosting emulator can now load secret env vars. (#8305)
- Fix webframeworks deployments when using multiple hosting sites in `firebase.json`. (#8314)
- Add a new command to setup a cleanup policy for functions artifacts. (#8268)
- Support 3rd party builders for Angular. (#7557)
Expand Down
87 changes: 22 additions & 65 deletions src/apphosting/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,51 +244,32 @@
let loadFromFileStub: sinon.SinonStub;
let baseAppHostingYaml: AppHostingYamlConfig;
let stagingAppHostingYaml: AppHostingYamlConfig;
let baseEnv: Record<string, Omit<config.Env, "variable">>;
let stagingEnv: Record<string, Omit<config.Env, "variable">>;

beforeEach(() => {
baseEnv = {
ENV_1: { value: "base_env_1" },
ENV_3: { value: "base_env_3" },
SECRET_1: { secret: "base_secret_1" },
SECRET_2: { secret: "base_secret_2" },
SECRET_3: { secret: "base_secret_3" },
};
baseAppHostingYaml = AppHostingYamlConfig.empty();
baseAppHostingYaml.addEnvironmentVariable({
variable: "ENV_1",
value: "base_env_1",
});
baseAppHostingYaml.addEnvironmentVariable({
variable: "ENV_3",
value: "base_env_3",
});
baseAppHostingYaml.addSecret({
variable: "SECRET_1",
secret: "base_secret_1",
});
baseAppHostingYaml.addSecret({
variable: "SECRET_2",
secret: "base_secret_2",
});
baseAppHostingYaml.addSecret({
variable: "SECRET_3",
secret: "base_secret_3",
});
baseAppHostingYaml.env = { ...baseEnv };

stagingEnv = {
ENV_1: { value: "staging_env_1" },
ENV_2: { value: "staging_env_2" },
SECRET_1: { secret: "staging_secret_1" },
SECRET_2: { secret: "staging_secret_2" },
};
stagingAppHostingYaml = AppHostingYamlConfig.empty();
stagingAppHostingYaml.addEnvironmentVariable({
variable: "ENV_1",
value: "staging_env_1",
});
stagingAppHostingYaml.addEnvironmentVariable({
variable: "ENV_2",
value: "staging_env_2",
});
stagingAppHostingYaml.addSecret({
variable: "SECRET_1",
secret: "staging_secret_1",
});
stagingAppHostingYaml.addSecret({
variable: "SECRET_2",
secret: "staging_secret_2",
});
stagingAppHostingYaml.env = { ...stagingEnv };

loadFromFileStub = sinon.stub(AppHostingYamlConfig, "loadFromFile");
loadFromFileStub.callsFake(async (filePath) => {
if (filePath?.includes("apphosting.staging.yaml")) {

Check warning on line 272 in src/apphosting/config.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .includes on an `any` value

Check warning on line 272 in src/apphosting/config.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe call of an `any` typed value
return Promise.resolve(stagingAppHostingYaml);
}
return Promise.resolve(baseAppHostingYaml);
Expand All @@ -304,39 +285,15 @@
"/parent/cwd/apphosting.staging.yaml",
"/parent/cwd/apphosting.yaml",
);
expect(JSON.stringify(resultingConfig.environmentVariables)).to.equal(
JSON.stringify([
{ variable: "ENV_1", value: "staging_env_1" },
{ variable: "ENV_3", value: "base_env_3" },
{ variable: "ENV_2", value: "staging_env_2" },
]),
);

expect(JSON.stringify(resultingConfig.secrets)).to.equal(
JSON.stringify([
{ variable: "SECRET_1", secret: "staging_secret_1" },
{ variable: "SECRET_2", secret: "staging_secret_2" },
{ variable: "SECRET_3", secret: "base_secret_3" },
]),
);
expect(resultingConfig.env).to.deep.equal({
...baseEnv,
...stagingEnv,
});
});

it("returns appropriate config if only base file was selected", async () => {
const resultingConfig = await config.loadConfigForEnvironment("/parent/cwd/apphosting.yaml");
expect(JSON.stringify(resultingConfig.environmentVariables)).to.equal(
JSON.stringify([
{ variable: "ENV_1", value: "base_env_1" },
{ variable: "ENV_3", value: "base_env_3" },
]),
);

expect(JSON.stringify(resultingConfig.secrets)).to.equal(
JSON.stringify([
{ variable: "SECRET_1", secret: "base_secret_1" },
{ variable: "SECRET_2", secret: "base_secret_2" },
{ variable: "SECRET_3", secret: "base_secret_3" },
]),
);
expect(resultingConfig.env).to.deep.equal(baseEnv);
});
});
});
14 changes: 8 additions & 6 deletions src/apphosting/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
projectId?: string,
userGivenConfigFile?: string,
): Promise<void> {
const choices = await prompt.prompt({}, [

Check warning on line 199 in src/apphosting/config.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
{
type: "checkbox",
name: "configurations",
Expand All @@ -209,7 +209,7 @@
* TODO: Update when supporting additional configurations. Currently only
* Secrets are exportable.
*/
if (!choices.configurations.includes(SECRET_CONFIG)) {

Check warning on line 212 in src/apphosting/config.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .configurations on an `any` value

Check warning on line 212 in src/apphosting/config.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe call of an `any` typed value
logger.info("No configs selected to export");
return;
}
Expand All @@ -227,24 +227,26 @@
}

const configToExport = await loadConfigToExportSecrets(cwd, userGivenConfigFile);
const secretsToExport = configToExport.secrets;
const secretsToExport = Object.entries(configToExport.env)
.filter(([, env]) => env.secret)
.map(([variable, env]) => {
return { variable, ...env };
});
if (!secretsToExport) {
logger.info("No secrets found to export in the chosen App Hosting config files");
return;
}

const secretMaterial = await fetchSecrets(projectId, secretsToExport);
for (const [key, value] of secretMaterial) {
localAppHostingConfig.addEnvironmentVariable({
variable: key,
value: value,
localAppHostingConfig.env[key] = {
value,
availability: ["RUNTIME"],
});
};
}

// remove secrets to avoid confusion as they are not read anyways.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can delete this comment

localAppHostingConfig.clearSecrets();
localAppHostingConfig.upsertFile(localAppHostingConfigPath);

Check warning on line 249 in src/apphosting/config.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
logger.info(`Wrote secrets as environment variables to ${APPHOSTING_LOCAL_YAML_FILE}.`);

updateOrCreateGitignore(projectRoot, [APPHOSTING_LOCAL_YAML_FILE]);
Expand Down Expand Up @@ -301,7 +303,7 @@
);
}

userGivenConfigFilePath = allConfigs.get(userGivenConfigFile)!;

Check warning on line 306 in src/apphosting/config.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Forbidden non-null assertion
} else {
userGivenConfigFilePath = await promptForAppHostingYaml(
allConfigs,
Expand All @@ -310,10 +312,10 @@
}

if (userGivenConfigFile === APPHOSTING_BASE_YAML_FILE) {
return AppHostingYamlConfig.loadFromFile(allConfigs.get(APPHOSTING_BASE_YAML_FILE)!);

Check warning on line 315 in src/apphosting/config.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Forbidden non-null assertion
}

const baseFilePath = allConfigs.get(APPHOSTING_BASE_YAML_FILE)!;

Check warning on line 318 in src/apphosting/config.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Forbidden non-null assertion
return await loadConfigForEnvironment(userGivenConfigFilePath, baseFilePath);
}

Expand Down
229 changes: 43 additions & 186 deletions src/apphosting/yaml.spec.ts
Original file line number Diff line number Diff line change
@@ -1,195 +1,52 @@
import { expect } from "chai";
import { AppHostingYamlConfig } from "./yaml";

describe("yaml", () => {
describe("environment variables", () => {
let apphostingYaml: AppHostingYamlConfig;
beforeEach(() => {
apphostingYaml = AppHostingYamlConfig.empty();
});

it("adds environment variables and retrieves them correctly", () => {
apphostingYaml.addEnvironmentVariable({
variable: "TEST_1",
value: "value_1",
});

apphostingYaml.addEnvironmentVariable({
variable: "TEST_2",
value: "value_2",
});

expect(JSON.stringify(apphostingYaml.environmentVariables)).to.equal(
JSON.stringify([
{
variable: "TEST_1",
value: "value_1",
},
{
variable: "TEST_2",
value: "value_2",
},
]),
);
});

it("overwrites stored environment variable if another is added with same name", () => {
apphostingYaml.addEnvironmentVariable({
variable: "TEST",
value: "value",
});

apphostingYaml.addEnvironmentVariable({
variable: "TEST",
value: "overwritten_value",
});

expect(JSON.stringify(apphostingYaml.environmentVariables)).to.equal(
JSON.stringify([{ variable: "TEST", value: "overwritten_value" }]),
);
describe("merge", () => {
it("merges incoming apphosting yaml config with precendence", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
it("merges incoming apphosting yaml config with precendence", () => {
it("merges incoming apphosting.yaml config with precendence", () => {

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically incorrect, no? It's about merging apphosting.local.yaml over apphosting.emulators.yaml over apphosting.yaml

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" },
});
});

describe("secrets", () => {
let apphostingYaml: AppHostingYamlConfig;
beforeEach(() => {
apphostingYaml = AppHostingYamlConfig.empty();
});

it("adds environment variables and retrieves them correctly", () => {
apphostingYaml.addSecret({
variable: "TEST_1",
secret: "value_1",
});

apphostingYaml.addSecret({
variable: "TEST_2",
secret: "value_2",
});

expect(JSON.stringify(apphostingYaml.secrets)).to.equal(
JSON.stringify([
{
variable: "TEST_1",
secret: "value_1",
},
{
variable: "TEST_2",
secret: "value_2",
},
]),
);
});

it("overwrites stored environment variable if another is added with same name", () => {
apphostingYaml.addSecret({
variable: "TEST",
secret: "value",
});

apphostingYaml.addSecret({
variable: "TEST",
secret: "overwritten_value",
});

expect(JSON.stringify(apphostingYaml.secrets)).to.equal(
JSON.stringify([{ variable: "TEST", secret: "overwritten_value" }]),
);
});

it("should clear secrets when clearSecrets is called", () => {
apphostingYaml.addSecret({
variable: "TEST",
secret: "value",
});

apphostingYaml.addSecret({
variable: "TEST",
secret: "overwritten_value",
});

apphostingYaml.clearSecrets();
expect(JSON.stringify(apphostingYaml.secrets)).to.equal(JSON.stringify([]));
});
});

describe("merge", () => {
let apphostingYaml: AppHostingYamlConfig;
beforeEach(() => {
apphostingYaml = AppHostingYamlConfig.empty();
});

it("merges incoming apphosting yaml config with precendence", () => {
apphostingYaml.addEnvironmentVariable({
variable: "ENV_1",
value: "env_1",
});
apphostingYaml.addEnvironmentVariable({
variable: "ENV_2",
value: "env_2",
});
apphostingYaml.addSecret({
variable: "SECRET_1",
secret: "secret_1",
});
apphostingYaml.addSecret({
variable: "SECRET_2",
secret: "secret_2",
});

const incomingAppHostingYaml = AppHostingYamlConfig.empty();
incomingAppHostingYaml.addEnvironmentVariable({
variable: "ENV_1",
value: "incoming_env_1",
});
incomingAppHostingYaml.addEnvironmentVariable({
variable: "ENV_3",
value: "incoming_env_3",
});
incomingAppHostingYaml.addSecret({
variable: "SECRET_2",
secret: "incoming_secret_1",
});
incomingAppHostingYaml.addSecret({
variable: "SECRET_3",
secret: "incoming_secret_3",
});

apphostingYaml.merge(incomingAppHostingYaml);

expect(JSON.stringify(apphostingYaml.environmentVariables)).to.equal(
JSON.stringify([
{
variable: "ENV_1",
value: "incoming_env_1",
},
{
variable: "ENV_2",
value: "env_2",
},
{
variable: "ENV_3",
value: "incoming_env_3",
},
]),
);

expect(JSON.stringify(apphostingYaml.secrets)).to.equal(
JSON.stringify([
{
variable: "SECRET_1",
secret: "secret_1",
},
{
variable: "SECRET_2",
secret: "incoming_secret_1",
},
{
variable: "SECRET_3",
secret: "incoming_secret_3",
},
]),
);
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" },
});
});
});
Loading
Loading