Skip to content

Create an InternalExpression type which allows precanned access to env.FIREBASE_CONFIG #1231

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

Merged
merged 7 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 6 additions & 4 deletions spec/fixtures/sources/commonjs-params/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,20 @@ params.defineInt("ANOTHER_INT", {

params.defineSecret("SUPER_SECRET_FLAG");

// N.B: invocation of the precanned internal params should not affect the manifest

exports.v1http = functions.https.onRequest((req, resp) => {
resp.status(200).send("PASS");
resp.status(200).send(params.projectID);
});

exports.v1callable = functions.https.onCall(() => {
return "PASS";
return params.databaseURL;
});

exports.v2http = functionsv2.https.onRequest((req, resp) => {
resp.status(200).send("PASS");
resp.status(200).send(params.gcloudProject);
});

exports.v2callable = functionsv2.https.onCall(() => {
return "PASS";
return params.databaseURL;
});
34 changes: 34 additions & 0 deletions src/params/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
StringParam,
SecretParam,
Expression,
InternalExpression,
} from "./types";

export { ParamOptions, Expression };
Expand Down Expand Up @@ -64,6 +65,39 @@ export function clearParams() {
declaredParams.splice(0, declaredParams.length);
}

/**
* A builtin param that resolves to the default RTDB database URL associated
* with the project, without prompting the deployer. Empty string if none exists.
*/
export const databaseURL = new InternalExpression(
"DATABASE_URL",
(env: NodeJS.ProcessEnv) => JSON.parse(env.FIREBASE_CONFIG)?.databaseURL
);
/**
* A builtin param that resolves to the Cloud project ID associated with
* the project, without prompting the deployer.
*/
export const projectID = new InternalExpression(
"PROJECT_ID",
(env: NodeJS.ProcessEnv) => JSON.parse(env.FIREBASE_CONFIG)?.projectId
);
/**
* A builtin param that resolves to the Cloud project ID, without prompting
* the deployer.
*/
export const gcloudProject = new InternalExpression(
"GCLOUD_PROJECT",
(env: NodeJS.ProcessEnv) => JSON.parse(env.FIREBASE_CONFIG)?.projectId
);
/**
* A builtin param that resolves to the Cloud storage bucket associated
* with the function, without prompting the deployer.
*/
export const storageBucket = new InternalExpression(
"STORAGE_BUCKET",
(env: NodeJS.ProcessEnv) => JSON.parse(env.FIREBASE_CONFIG)?.storageBucket
);

/**
* Declares a secret param, that will persist values only in Cloud Secret Manager.
* Secrets are stored interally as bytestrings. Use ParamOptions.`as` to provide type
Expand Down
27 changes: 27 additions & 0 deletions src/params/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,33 @@ export abstract class Expression<T extends string | number | boolean | string[]>
}
}

/**
* A CEL expression which represents an internal Firebase variable. This class
* cannot be instantiated by developers, but we provide several canned instances
* of it to make available params that will never have to be defined at
* deployment time, and can always be read from process.env.
*/
export class InternalExpression extends Expression<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be marked @hidden or @beta or @Alpha or whatever so we don't expose it to our end user?

I wish we can use @internal, but I'm afraid that might break user code that tries to reference variables with that type 🤔

Copy link
Member

Choose a reason for hiding this comment

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

No, it should be @internal and the variable should be declared as an Expression. Type erasure is great for hiding implementation details and not tying our hands to them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. Done.

constructor(
private readonly name: string,
private readonly getter: (env: NodeJS.ProcessEnv) => string
) {
super();
}

value(): string {
return this.getter(process.env) || "";
}

toString(): string {
return `params.${this.name}`;
}

toCEL(): string {
throw new Error("An InternalExpression should never be marshalled for wire transmission.");
}
}

function quoteIfString<T extends string | number | boolean | string[]>(literal: T): T {
// TODO(vsfan@): CEL's string escape semantics are slightly different than Javascript's, what do we do here?
return typeof literal === "string" ? (`"${literal}"` as T) : literal;
Expand Down