-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add new apphosting:env:import commmand #9206
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
Open
inlined
wants to merge
7
commits into
master
Choose a base branch
from
inlined.env-import
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+464
−1
Open
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
ea02e90
Add new apphosting:env:import commmand
inlined e5a26d5
Merge branch 'master' into inlined.env-import
inlined f03d821
PR Fixes
inlined 60a3bce
Merge branch 'master' into inlined.env-import
inlined faac277
Robo feedabck
inlined c7691d2
Merge branch 'master' into inlined.env-import
inlined 8fe10ee
Merge branch 'master' into inlined.env-import
inlined File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,188 @@ | ||
import { expect } from "chai"; | ||
import * as sinon from "sinon"; | ||
import * as env from "./env"; | ||
import * as promptNS from "../prompt"; | ||
import * as config from "./config"; | ||
import * as gcsmNS from "../gcp/secretManager"; | ||
import * as secretsNS from "./secrets"; | ||
import * as utilsNS from "../utils"; | ||
import { Document } from "yaml"; | ||
|
||
describe("env", () => { | ||
let prompt: sinon.SinonStubbedInstance<typeof promptNS>; | ||
let gcsm: sinon.SinonStubbedInstance<typeof gcsmNS>; | ||
let secrets: sinon.SinonStubbedInstance<typeof secretsNS>; | ||
let utils: sinon.SinonStubbedInstance<typeof utilsNS>; | ||
|
||
function makeDocument(...envs: config.Env[]): Document { | ||
const doc = new Document(); | ||
for (const e of envs) { | ||
config.upsertEnv(doc, e); | ||
} | ||
return doc; | ||
} | ||
|
||
beforeEach(() => { | ||
prompt = sinon.stub(promptNS); | ||
gcsm = sinon.stub(gcsmNS); | ||
secrets = sinon.stub(secretsNS); | ||
utils = sinon.stub(utilsNS); | ||
|
||
utils.logLabeledWarning.resolves(); | ||
prompt.input.rejects(new Error("Should not be called")); | ||
gcsm.accessSecretVersion.rejects(new Error("Should not be called")); | ||
gcsm.addVersion.rejects(new Error("Should not be called")); | ||
secrets.upsertSecret.rejects(new Error("Should not be called")); | ||
}); | ||
|
||
afterEach(() => { | ||
sinon.verifyAndRestore(); | ||
}); | ||
|
||
it("should diffEnvs", async () => { | ||
gcsm.accessSecretVersion | ||
.withArgs("test-project", "matching-secret", "latest") | ||
.resolves("unchanged"); | ||
gcsm.accessSecretVersion | ||
.withArgs("test-project", "changed-secret", "latest") | ||
.resolves("original-value"); | ||
gcsm.accessSecretVersion | ||
.withArgs("test-project", "error-secret", "latest") | ||
.rejects(new Error("Cannot access secret")); | ||
|
||
const existingEnv = makeDocument( | ||
{ variable: "MATCHING_PLAIN", value: "existing" }, | ||
{ variable: "CHANGED_PLAIN", value: "original-value" }, | ||
{ variable: "UNREFERENCED_PLAIN", value: "existing" }, | ||
|
||
{ variable: "MATCHING_SECRET", secret: "matching-secret" }, | ||
{ variable: "UNREFERENCED_SECRET", secret: "unreferenced-secret" }, | ||
{ variable: "CHANGED_SECRET", secret: "changed-secret" }, | ||
{ variable: "ERROR_SECRET", secret: "error-secret" }, | ||
); | ||
|
||
const importingEnv = { | ||
MATCHING_PLAIN: "existing", | ||
CHANGED_PLAIN: "new-value", | ||
NEW_PLAIN: "new", | ||
|
||
MATCHING_SECRET: "unchanged", | ||
NEW_SECRET: "new", | ||
CHANGED_SECRET: "changed-value", | ||
ERROR_SECRET: "attempted-value", | ||
}; | ||
|
||
await expect(env.diffEnvs("test-project", importingEnv, existingEnv)).to.eventually.deep.equal({ | ||
newVars: ["NEW_PLAIN", "NEW_SECRET"], | ||
matched: ["MATCHING_PLAIN", "MATCHING_SECRET"], | ||
conflicts: ["CHANGED_PLAIN", "CHANGED_SECRET", "ERROR_SECRET"], | ||
}); | ||
expect(gcsm.accessSecretVersion).to.have.been.calledThrice; | ||
expect(utils.logLabeledWarning).to.have.been.calledWith( | ||
"apphosting", | ||
"Cannot read value of existing secret error-secret to see if it has changed. Assuming it has changed.", | ||
); | ||
}); | ||
|
||
describe("confirmConflicts", () => { | ||
it("should return an empty array if no conflicts", async () => { | ||
const result = await env.confirmConflicts([]); | ||
expect(result).to.be.empty; | ||
}); | ||
|
||
it("should prompt the user to resolve conflicts", async () => { | ||
prompt.checkbox.resolves(["FOO"]); | ||
const result = await env.confirmConflicts(["FOO", "BAZ"]); | ||
expect(result).to.deep.equal(["FOO"]); | ||
expect(prompt.checkbox).to.have.been.calledOnce; | ||
}); | ||
}); | ||
|
||
describe("chooseNewSecrets", () => { | ||
it("should return an empty array if no vars", async () => { | ||
const result = await env.chooseNewSecrets([]); | ||
expect(result).to.be.empty; | ||
}); | ||
|
||
it("should suggest which values to store as secrets", async () => { | ||
prompt.checkbox.resolves(["MY_KEY"]); | ||
const result = await env.chooseNewSecrets(["FOO", "BAZ", "MY_KEY", "MY_SECRET"]); | ||
expect(result).to.deep.equal(["MY_KEY"]); | ||
expect(prompt.checkbox).to.have.been.calledWithMatch({ | ||
message: | ||
"Sensitive data should be stored in Cloud Secrets Manager so that access to its value is protected. Which variables are sensitive?", | ||
choices: [ | ||
{ value: "FOO", checked: false }, | ||
{ value: "BAZ", checked: false }, | ||
{ value: "MY_KEY", checked: true }, | ||
{ value: "MY_SECRET", checked: true }, | ||
], | ||
}); | ||
}); | ||
}); | ||
|
||
describe("importEnv", () => { | ||
// We could break this into multiple tests, but the same code is execrcised in all cases. | ||
it("should keep existing secrets as secrets, prompt for new vars to be secrets, and store only selected info", async () => { | ||
const existingEnv = makeDocument( | ||
{ variable: "EXISTING_PLAIN1", value: "existing" }, | ||
{ variable: "EXISTING_PLAIN2", value: "existing" }, | ||
{ variable: "EXISTING_SECRET1", secret: "existing-secret1" }, | ||
{ variable: "EXISTING_SECRET2", secret: "existing-secret2" }, | ||
); | ||
|
||
const importingEnv = { | ||
EXISTING_PLAIN1: "new", | ||
EXISTING_PLAIN2: "new", | ||
NEW_PLAIN: "new", | ||
EXISTING_SECRET1: "new", | ||
EXISTING_SECRET2: "new", | ||
NEW_SECRET: "new", | ||
}; | ||
|
||
sinon.stub(env, "diffEnvs").resolves({ | ||
newVars: ["NEW_PLAIN", "NEW_SECRET"], | ||
conflicts: ["EXISTING_PLAIN1", "EXISTING_PLAIN2", "EXISTING_SECRET1", "EXISTING_SECRET2"], | ||
matched: [], | ||
}); | ||
// Leave #2 alone and verify that they haven't been modified | ||
sinon.stub(env, "confirmConflicts").resolves(["EXISTING_PLAIN1", "EXISTING_SECRET1"]); | ||
// Verify that only new variables are offered to be stored as secrets | ||
sinon.stub(env, "chooseNewSecrets").resolves(["NEW_SECRET"]); | ||
secrets.upsertSecret.withArgs("test-project", "NEW_SECRET").resolves(); | ||
gcsm.addVersion.withArgs("test-project", "NEW_SECRET", "new").resolves(); | ||
gcsm.addVersion.withArgs("test-project", "existing-secret1", "new").resolves(); | ||
|
||
const createdSecrets = await env.importEnv("test-project", importingEnv, existingEnv); | ||
|
||
// Confirm new variables are not part of the confirm prompt | ||
expect(env.confirmConflicts).calledWithMatch([ | ||
"EXISTING_PLAIN1", | ||
"EXISTING_PLAIN2", | ||
"EXISTING_SECRET1", | ||
"EXISTING_SECRET2", | ||
]); | ||
|
||
// Confirm that variables which already existed are not asked to be stored as secrets | ||
expect(env.chooseNewSecrets).calledWithMatch(["NEW_PLAIN", "NEW_SECRET"]); | ||
|
||
// Confirm that we don't unnecessarily upsert existing secrets | ||
expect(secrets.upsertSecret).to.have.been.calledOnceWith("test-project", "NEW_SECRET"); | ||
|
||
// Confirm that we updated the versions of the secrets we were asked to update | ||
expect(gcsm.addVersion).to.have.been.calledWith("test-project", "NEW_SECRET", "new"); | ||
expect(gcsm.addVersion).to.have.been.calledWith("test-project", "existing-secret1", "new"); | ||
|
||
// Confirm that we return the list of created secrets for furhter IAM granting | ||
expect(createdSecrets).to.deep.equal(["NEW_SECRET"]); | ||
|
||
// Confirm that the existing env was properly updated | ||
expect(config.findEnv(existingEnv, "EXISTING_PLAIN1")?.value).to.equal("new"); | ||
expect(config.findEnv(existingEnv, "EXISTING_PLAIN2")?.value).to.equal("existing"); | ||
expect(config.findEnv(existingEnv, "NEW_PLAIN")?.value).to.equal("new"); | ||
expect(config.findEnv(existingEnv, "EXISTING_SECRET1")?.secret).to.equal("existing-secret1"); | ||
expect(config.findEnv(existingEnv, "EXISTING_SECRET2")?.secret).to.equal("existing-secret2"); | ||
expect(config.findEnv(existingEnv, "NEW_SECRET")?.secret).to.equal("NEW_SECRET"); | ||
}); | ||
}); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
import * as clc from "colorette"; | ||
|
||
import { FirebaseError } from "../error"; | ||
import * as secretManager from "../gcp/secretManager"; | ||
import * as prompt from "../prompt"; | ||
import * as config from "./config"; | ||
import { Document } from "yaml"; | ||
import * as secrets from "./secrets"; | ||
import * as utils from "../utils"; | ||
|
||
const dynamicDispatch = exports as { | ||
diffEnvs: typeof diffEnvs; | ||
confirmConflicts: typeof confirmConflicts; | ||
chooseNewSecrets: typeof chooseNewSecrets; | ||
}; | ||
|
||
export interface DiffResults { | ||
newVars: string[]; | ||
matched: string[]; | ||
conflicts: string[]; | ||
} | ||
|
||
export async function diffEnvs( | ||
projectId: string, | ||
envs: Record<string, string>, | ||
doc: Document, | ||
): Promise<DiffResults> { | ||
const newVars: string[] = []; | ||
const matched: string[] = []; | ||
const conflicts: string[] = []; | ||
|
||
// Note: Can conceivably optimize this by parallelizing lookups of secret values with fetchSecrets. | ||
// Unlikely to actually cause noticeable benefits. | ||
for (const [key, value] of Object.entries(envs)) { | ||
const existingEnv = config.findEnv(doc, key); | ||
if (!existingEnv) { | ||
newVars.push(key); | ||
continue; | ||
} | ||
|
||
let match = false; | ||
if (existingEnv.value) { | ||
match = existingEnv.value === value; | ||
} else { | ||
try { | ||
match = | ||
value === | ||
(await secretManager.accessSecretVersion(projectId, existingEnv.secret!, "latest")); | ||
} catch (err) { | ||
utils.logLabeledWarning( | ||
"apphosting", | ||
`Cannot read value of existing secret ${existingEnv.secret!} to see if it has changed. Assuming it has changed.`, | ||
); | ||
} | ||
} | ||
|
||
(match ? matched : conflicts).push(key); | ||
} | ||
return { newVars, matched, conflicts }; | ||
} | ||
inlined marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export async function confirmConflicts(conflicts: string[]): Promise<string[]> { | ||
if (!conflicts.length) { | ||
return conflicts; | ||
} | ||
|
||
const overwrite = await prompt.checkbox<string>({ | ||
message: | ||
"The following variables have different values in apphosting.yaml. Which would you like to overwrite?", | ||
choices: conflicts, | ||
}); | ||
return overwrite; | ||
} | ||
|
||
export async function chooseNewSecrets(vars: string[]): Promise<string[]> { | ||
if (!vars.length) { | ||
return vars; | ||
} | ||
|
||
return await prompt.checkbox<string>({ | ||
message: | ||
"Sensitive data should be stored in Cloud Secrets Manager so that access to its value is protected. Which variables are sensitive?", | ||
choices: vars.map((name) => ({ | ||
value: name, | ||
checked: name.includes("KEY") || name.includes("SECRET"), | ||
})), | ||
}); | ||
} | ||
|
||
/** | ||
* Merges a .env file with a YAML document including uploading, but not necessarily granting permission, to secrets. | ||
* We're using a YAML doc and not worrying about file saving or granting permissions so that the caller can swap out whether | ||
* this is a local yaml (for which env) or whether this is for remote env. | ||
* @returns A list of secrets which were created and may need access granted. | ||
*/ | ||
export async function importEnv( | ||
projectId: string, | ||
envs: Record<string, string>, | ||
doc: Document, | ||
): Promise<string[]> { | ||
let { newVars, conflicts } = await dynamicDispatch.diffEnvs(projectId, envs, doc); | ||
|
||
conflicts = await dynamicDispatch.confirmConflicts(conflicts); | ||
const newSecrets = await dynamicDispatch.chooseNewSecrets(newVars); | ||
|
||
for (const key of conflicts) { | ||
const existingEnv = config.findEnv(doc, key); | ||
if (!existingEnv) { | ||
throw new FirebaseError(`Internal error: expected existing env for ${key}`, { exit: 1 }); | ||
} | ||
if (existingEnv.value) { | ||
existingEnv.value = envs[key]; | ||
config.upsertEnv(doc, existingEnv); | ||
} else { | ||
const secretValue = envs[key]; | ||
const version = await secretManager.addVersion(projectId, existingEnv.secret!, secretValue); | ||
utils.logSuccess( | ||
`Created new secret version ${secretManager.toSecretVersionResourceName(version)}`, | ||
); | ||
} | ||
} | ||
|
||
const newPlaintext = newVars.filter((v) => !newSecrets.includes(v)); | ||
for (const key of newPlaintext) { | ||
config.upsertEnv(doc, { variable: key, value: envs[key] }); | ||
} | ||
|
||
// NOTE: not doing this in parallel to avoid interleaving log lines in a way that might be confusing. | ||
for (const key of newSecrets) { | ||
// TODO: (How) do we support secrets in a specific location? Not investing deeply right now since everything in App Hosting | ||
// is curreently global jurrisdiction and we may be chaging to REP managing secrets locality instead of UMMR anyway. | ||
const created = await secrets.upsertSecret(projectId, key); | ||
if (created) { | ||
utils.logSuccess(`Created new secret projects/${projectId}/secrets/${key}`); | ||
} | ||
|
||
const version = await secretManager.addVersion(projectId, key, envs[key]); | ||
utils.logSuccess( | ||
`Created new secret version ${secretManager.toSecretVersionResourceName(version)}`, | ||
); | ||
utils.logBullet( | ||
`You can access the contents of the secret's latest value with ${clc.bold(`firebase apphosting:secrets:access ${key}\n`)}`, | ||
); | ||
|
||
config.upsertEnv(doc, { variable: key, secret: key }); | ||
} | ||
return newSecrets; | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.