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

Add support for apphosting.emulator.yaml #8333

Merged
merged 3 commits into from
Mar 24, 2025
Merged
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: 11 additions & 0 deletions src/apphosting/config.ts
Original file line number Diff line number Diff line change
@@ -14,8 +14,19 @@
import { updateOrCreateGitignore } from "../utils";
import { getOrPromptProject } from "../management/projects";

// Common config across all environments
export const APPHOSTING_BASE_YAML_FILE = "apphosting.yaml";

// Modern version of local configuration that is intended to be safe to commit.
// In order to ensure safety, values that are secret environment variables in
// apphosting.yaml cannot be made plaintext in apphosting.emulators.yaml
export const APPHOSTING_EMULATORS_YAML_FILE = "apphosting.emulator.yaml";

// Legacy/undocumented version of local configuration that is allowed to store
// values that are secrets in preceeding files as plaintext. It is not safe
// to commit to SCM
export const APPHOSTING_LOCAL_YAML_FILE = "apphosting.local.yaml";

export const APPHOSTING_YAML_FILE_REGEX = /^apphosting(\.[a-z0-9_]+)?\.yaml$/;

export interface RunConfig {
@@ -196,7 +207,7 @@
projectId?: string,
userGivenConfigFile?: string,
): Promise<void> {
const choices = await prompt.prompt({}, [

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

GitHub Actions / lint (20)

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

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

GitHub Actions / lint (20)

Unsafe member access .configurations on an `any` value

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

GitHub Actions / lint (20)

Unsafe call of an `any` typed value
logger.info("No configs selected to export");
return;
}
@@ -246,7 +257,7 @@
}

// remove secrets to avoid confusion as they are not read anyways.
localAppHostingConfig.upsertFile(localAppHostingConfigPath);

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

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]);
@@ -303,7 +314,7 @@
);
}

userGivenConfigFilePath = allConfigs.get(userGivenConfigFile)!;

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

GitHub Actions / lint (20)

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

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

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

GitHub Actions / lint (20)

Forbidden non-null assertion
}

const baseFilePath = allConfigs.get(APPHOSTING_BASE_YAML_FILE)!;

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

GitHub Actions / lint (20)

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

2 changes: 1 addition & 1 deletion src/apphosting/yaml.ts
Original file line number Diff line number Diff line change
@@ -31,10 +31,10 @@

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

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

GitHub Actions / lint (20)

Unsafe assignment of an `any` value

if (loadedAppHostingYaml.env) {

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

GitHub Actions / lint (20)

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

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

GitHub Actions / lint (20)

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

return config;
@@ -59,7 +59,7 @@
.map(([key]) => key);
if (wereSecrets.some((key) => other.env[key]?.value)) {
throw new FirebaseError(
`Cannot convert secret to plaintext in ${other.filename ? other.filename : "apphosting yaml"}`,
`Cannot convert secret to plaintext in ${other.filename ?? "apphosting yaml"}`,
);
}
}
63 changes: 54 additions & 9 deletions src/emulator/apphosting/config.spec.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import * as path from "path";
import * as utils from "./developmentServer";

import * as sinon from "sinon";
import { expect } from "chai";
import { getLocalAppHostingConfiguration } from "./config";
import * as configImport from "../../apphosting/config";
import { AppHostingYamlConfig } from "../../apphosting/yaml";
import { FirebaseError } from "../../error";

describe("environments", () => {
let joinStub: sinon.SinonStub;
let loggerStub: sinon.SinonStub;
let loadAppHostingYamlStub: sinon.SinonStub;
let listAppHostingFilesInPathStub: sinon.SinonStub;
const apphostingYamlEnvOne = {
@@ -26,6 +22,12 @@ describe("environments", () => {
randomSecretOne: { secret: "SECRET_ONE_FROM_CONFIG_TWO" },
randomSecretFour: { secret: "SECRET_FOUR_FROM_CONFIG_TWO" },
};
const apphostingYamlSecretToPlaintext = {
randomSecretOne: { value: "RANDOM_SECRET_ONE_PLAINTEXT" },
randomSecretTwo: { value: "RANDOM_SECRET_TWO_PLAINTEXT" },
randomSecretThree: { value: "RANDOM_SECRET_THREE_PLAINTEXT" },
randomSecretFour: { value: "RANDOM_SECRET_FOUR_PLAINTEXT" },
};

// Configs used for stubs
const apphostingYamlConfigOne = AppHostingYamlConfig.empty();
@@ -34,16 +36,15 @@ describe("environments", () => {
const apphostingYamlConfigTwo = AppHostingYamlConfig.empty();
apphostingYamlConfigTwo.env = { ...apphostingYamlEnvTwo };

const apphostingYamlConfigSecretsToPlaintext = AppHostingYamlConfig.empty();
apphostingYamlConfigSecretsToPlaintext.env = { ...apphostingYamlSecretToPlaintext };

beforeEach(() => {
loadAppHostingYamlStub = sinon.stub(AppHostingYamlConfig, "loadFromFile");
joinStub = sinon.stub(path, "join");
loggerStub = sinon.stub(utils, "logger");
listAppHostingFilesInPathStub = sinon.stub(configImport, "listAppHostingFilesInPath");
});

afterEach(() => {
joinStub.restore();
loggerStub.restore();
sinon.verifyAndRestore();
});

@@ -85,14 +86,58 @@ describe("environments", () => {
.returns(apphostingYamlConfigOne);
loadAppHostingYamlStub
.withArgs("/parent/apphosting.local.yaml")
.returns(apphostingYamlConfigSecretsToPlaintext);

const apphostingConfig = await getLocalAppHostingConfiguration("./");

expect(apphostingConfig.env).to.deep.equal({
...apphostingYamlEnvOne,
...apphostingYamlSecretToPlaintext,
});
});

it("should allow merging all three file types", async () => {
listAppHostingFilesInPathStub.returns([
"/parent/cwd/apphosting.yaml",
"/parent/cwd/apphosting.emulator.yaml",
"/parent/apphosting.local.yaml",
]);

// Second config takes precedence
loadAppHostingYamlStub
.withArgs("/parent/cwd/apphosting.yaml")
.returns(apphostingYamlConfigOne);
loadAppHostingYamlStub
.withArgs("/parent/cwd/apphosting.emulator.yaml")
.returns(apphostingYamlConfigTwo);
loadAppHostingYamlStub
.withArgs("/parent/apphosting.local.yaml")
.returns(apphostingYamlConfigSecretsToPlaintext);

const apphostingConfig = await getLocalAppHostingConfiguration("./");

expect(apphostingConfig.env).to.deep.equal({
...apphostingYamlEnvOne,
...apphostingYamlEnvTwo,
...apphostingYamlSecretToPlaintext,
});
});

it("Should not allow apphosting.emulator.yaml to convert secrets to plaintext", async () => {
listAppHostingFilesInPathStub.returns([
"/parent/cwd/apphosting.yaml",
"/parent/cwd/apphosting.emulator.yaml",
]);

// Second config takes precedence
loadAppHostingYamlStub
.withArgs("/parent/cwd/apphosting.yaml")
.returns(apphostingYamlConfigOne);
loadAppHostingYamlStub
.withArgs("/parent/cwd/apphosting.emulator.yaml")
.returns(apphostingYamlConfigSecretsToPlaintext);

await expect(getLocalAppHostingConfiguration("./")).to.be.rejectedWith(FirebaseError);
});
});
});
48 changes: 29 additions & 19 deletions src/emulator/apphosting/config.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,51 @@
import { basename } from "path";
import {
APPHOSTING_BASE_YAML_FILE,
APPHOSTING_EMULATORS_YAML_FILE,
APPHOSTING_LOCAL_YAML_FILE,
listAppHostingFilesInPath,
} from "../../apphosting/config";
import { AppHostingYamlConfig } from "../../apphosting/yaml";

/**
* Loads in apphosting.yaml & apphosting.local.yaml, giving
* apphosting.local.yaml precedence if present.
* Loads in apphosting.yaml, apphosting.emulator.yaml & apphosting.local.yaml as an
* overriding union. In order to keep apphosting.emulator.yaml safe to commit,
* users cannot change a secret environment variable to plaintext.
* apphosting.local.yaml can, however, for reverse compatibility, though its existence
* will be downplayed and tooling will not assist in creating or managing it.
*/
export async function getLocalAppHostingConfiguration(
backendDir: string,
): Promise<AppHostingYamlConfig> {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update the JSDoc to reflect the new behaviour of us loading in the apphosting.emulator.yaml file as well.

const appHostingConfigPaths = listAppHostingFilesInPath(backendDir);
// generate a map to make it easier to interface between file name and it's path
const fileNameToPathMap: Map<string, string> = new Map();
for (const path of appHostingConfigPaths) {
const fileName = basename(path);
fileNameToPathMap.set(fileName, path);
}
const fileNameToPathMap = Object.fromEntries(
appHostingConfigPaths.map((path) => [basename(path), path]),
);

const output = AppHostingYamlConfig.empty();

const baseFilePath = fileNameToPathMap.get(APPHOSTING_BASE_YAML_FILE);
const localFilePath = fileNameToPathMap.get(APPHOSTING_LOCAL_YAML_FILE);
const baseFilePath = fileNameToPathMap[APPHOSTING_BASE_YAML_FILE];
const emulatorsFilePath = fileNameToPathMap[APPHOSTING_EMULATORS_YAML_FILE];
const localFilePath = fileNameToPathMap[APPHOSTING_LOCAL_YAML_FILE];

if (baseFilePath) {
// N.B. merging from empty helps tests stay hermetic. I previously ran into a test bug where
// using the returned value as the base caused the test stub to be modified and tests would succeed
// independently but would fail as part of a suite.
const baseFile = await AppHostingYamlConfig.loadFromFile(baseFilePath);
output.merge(baseFile, /* allowSecretsToBecomePlaintext= */ false);
}

// apphosting.local.yaml or apphosting.yaml are not required to run the emulator
if (!baseFilePath && !localFilePath) {
return AppHostingYamlConfig.empty();
if (emulatorsFilePath) {
const emulatorsConfig = await AppHostingYamlConfig.loadFromFile(emulatorsFilePath);
output.merge(emulatorsConfig, /* allowSecretsToBecomePlaintext= */ false);
}

// If one of them exists ...
if (!baseFilePath || !localFilePath) {
return await AppHostingYamlConfig.loadFromFile((baseFilePath || localFilePath)!);
if (localFilePath) {
const localYamlConfig = await AppHostingYamlConfig.loadFromFile(localFilePath);
output.merge(localYamlConfig, /* allowSecretsToBecomePlaintext= */ true);
}

const localYamlConfig = await AppHostingYamlConfig.loadFromFile(localFilePath);
const baseConfig = await AppHostingYamlConfig.loadFromFile(baseFilePath);
baseConfig.merge(localYamlConfig);
return baseConfig;
return output;
}
Loading