From a1a2983aeee2f878a44605d5488935d697b6f2b6 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 17 Oct 2022 16:16:55 -0700 Subject: [PATCH 01/19] first commit: param lists --- src/deploy/functions/cel.ts | 66 ++++++++++++++++++++++----- src/deploy/functions/params.ts | 16 ++++++- src/test/deploy/functions/cel.spec.ts | 10 ++++ 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index 96286124551..0b9f08c4e64 100644 --- a/src/deploy/functions/cel.ts +++ b/src/deploy/functions/cel.ts @@ -10,8 +10,8 @@ type TernaryExpression = CelExpression; type LiteralTernaryExpression = CelExpression; type DualTernaryExpression = CelExpression; -type Literal = string | number | boolean; -type L = "string" | "number" | "boolean"; +type Literal = string | number | boolean | string[]; +type L = "string" | "number" | "boolean" | "string[]"; const paramRegexp = /params\.(\S+)/; const CMP = /((?:!=)|(?:==)|(?:>=)|(?:<=)|>|<)/.source; // !=, ==, >=, <=, >, < @@ -98,7 +98,8 @@ function assertType(wantType: L, paramName: string, paramValue: ParamValue) { if ( (wantType === "string" && !paramValue.legalString) || (wantType === "number" && !paramValue.legalNumber) || - (wantType === "boolean" && !paramValue.legalBoolean) + (wantType === "boolean" && !paramValue.legalBoolean) || + (wantType === "string[]" && !paramValue.legalList) ) { throw new ExprParseError(`Illegal type coercion of param ${paramName} to type ${wantType}`); } @@ -111,6 +112,8 @@ function readParamValue(wantType: L, paramName: string, paramValue: ParamValue): return paramValue.asNumber(); } else if (wantType === "boolean") { return paramValue.asBoolean(); + } else if (wantType === "string[]") { + return paramValue.asList(); } else { assertExhaustive(wantType); } @@ -286,9 +289,9 @@ function resolveTernary( const comparisonExpr = `{{ params.${match[1]} ${match[2]} ${match[3]} }}`; const isTrue = resolveComparison(comparisonExpr, params); if (isTrue) { - return resolveParamOrLiteral(wantType, match[4], params); + return resolveParamListOrLiteral(wantType, match[4], params); } else { - return resolveParamOrLiteral(wantType, match[5], params); + return resolveParamListOrLiteral(wantType, match[5], params); } } @@ -308,9 +311,9 @@ function resolveDualTernary( const comparisonExpr = `{{ params.${match[1]} ${match[2]} params.${match[3]} }}`; const isTrue = resolveDualComparison(comparisonExpr, params); if (isTrue) { - return resolveParamOrLiteral(wantType, match[4], params); + return resolveParamListOrLiteral(wantType, match[4], params); } else { - return resolveParamOrLiteral(wantType, match[5], params); + return resolveParamListOrLiteral(wantType, match[5], params); } } @@ -342,20 +345,22 @@ function resolveLiteralTernary( } if (paramValue.asBoolean()) { - return resolveParamOrLiteral(wantType, match[2], params); + return resolveParamListOrLiteral(wantType, match[2], params); } else { - return resolveParamOrLiteral(wantType, match[3], params); + return resolveParamListOrLiteral(wantType, match[3], params); } } -function resolveParamOrLiteral( +function resolveParamListOrLiteral( wantType: L, field: string, params: Record ): Literal { const match = paramRegexp.exec(field); if (!match) { - return resolveLiteral(wantType, field); + return wantType === "string[]" + ? resolveList("string", field, params) + : resolveLiteral(wantType, field); } const paramValue = params[match[1]]; if (!paramValue) { @@ -364,7 +369,44 @@ function resolveParamOrLiteral( return readParamValue(wantType, match[1], paramValue); } -function resolveLiteral(wantType: L, value: string): Literal { +/** + * A List in Functions CEL is a []-bracketed string with comma-seperated values that can be: + * - A double quoted string literal + * - A reference to a param value (params.FOO) which must resolve with type string + * - A sub-CEL expression {{ params.BAR == 0 ? "a" : "b" }} which must resolve with type string + */ +function resolveList( + wantType: "string", + list: string, + params: Record +): string[] { + if (!list.startsWith("[") || !list.endsWith("]")) { + throw new ExprParseError(""); + } + const rv: string[] = []; + const entries = list.slice(1, -1).split(","); + + for (const entry of entries) { + const trimmed = entry.trim(); + if (trimmed.startsWith('"') && trimmed.endsWith('"')) { + rv.push(trimmed.slice(1, -1)); + } else if (trimmed.startsWith("{{") && trimmed.endsWith("}}")) { + rv.push(resolveExpression("string", trimmed, params) as string); + } else { + const paramMatch = paramRegexp.exec(trimmed); + if (!paramMatch) { + throw new ExprParseError(""); + } else if (!(paramMatch[1] in params)) { + throw new ExprParseError(""); + } + rv.push(resolveParamListOrLiteral("string", trimmed, params) as string); + } + } + + return rv; +} + +function resolveLiteral(wantType: Omit, value: string): Literal { if (paramRegexp.exec(value)) { throw new ExprParseError( "CEL tried to evaluate param." + value + " in a context which only permits literal values" diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 5ea90edb081..5a9b12db67f 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -206,18 +206,24 @@ export class ParamValue { legalBoolean: boolean; // Whether this param value can be sensibly interpreted as a number legalNumber: boolean; + // Whether this param value can be sensibly interpreted as a list + legalList: boolean; constructor( private readonly rawValue: string, readonly internal: boolean, - types: { string?: boolean; boolean?: boolean; number?: boolean } + types: { string?: boolean; boolean?: boolean; number?: boolean; list?: boolean } ) { this.legalString = types.string || false; this.legalBoolean = types.boolean || false; this.legalNumber = types.number || false; + this.legalList = types.list || false; } toString(): string { + if (this.legalList) { + return encodeURI(JSON.stringify(this.rawValue)); + } return this.rawValue; } @@ -229,6 +235,14 @@ export class ParamValue { return ["true", "y", "yes", "1"].includes(this.rawValue); } + asList(): string[] { + const parsed = JSON.parse(decodeURI(this.rawValue)); + if (!Array.isArray(parsed)) { + // uh-oh + } + return parsed; + } + asNumber(): number { return +this.rawValue; } diff --git a/src/test/deploy/functions/cel.spec.ts b/src/test/deploy/functions/cel.spec.ts index 98d6799aa06..1f46dd7298f 100644 --- a/src/test/deploy/functions/cel.spec.ts +++ b/src/test/deploy/functions/cel.spec.ts @@ -13,6 +13,16 @@ function boolV(value: boolean): ParamValue { } describe("CEL evaluation", () => { + describe("String list resolution", () => { + it("can pull lists directly otu of paramvalues", () => { + expect( + resolveExpression("string[]", "{{ params.FOO }}", { + FOO: new ParamValue("[1]", false, { list: true }), + }) + ); + }); + }); + describe("Identity expressions", () => { it("raises when the referenced parameter does not exist", () => { expect(() => { From ace8a2b4c34a9ed02645793c7c019fe1fb0fb417 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 17 Oct 2022 16:19:29 -0700 Subject: [PATCH 02/19] love too have initial trivial test case be broken --- src/test/deploy/functions/cel.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/deploy/functions/cel.spec.ts b/src/test/deploy/functions/cel.spec.ts index 1f46dd7298f..d1b53d160f0 100644 --- a/src/test/deploy/functions/cel.spec.ts +++ b/src/test/deploy/functions/cel.spec.ts @@ -17,9 +17,9 @@ describe("CEL evaluation", () => { it("can pull lists directly otu of paramvalues", () => { expect( resolveExpression("string[]", "{{ params.FOO }}", { - FOO: new ParamValue("[1]", false, { list: true }), + FOO: new ParamValue('["1"]', false, { list: true }), }) - ); + ).to.deep.equal(["1"]); }); }); From 7c2dbd2b30ebf350c80fec7b62b829bc9499e3ca Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 17 Oct 2022 23:59:59 -0700 Subject: [PATCH 03/19] maybe fix it so it actually works --- src/deploy/functions/build.ts | 14 ++- src/deploy/functions/cel.ts | 123 +++++++++++++++++--------- src/deploy/functions/params.ts | 22 +++-- src/test/deploy/functions/cel.spec.ts | 54 ++++++++++- 4 files changed, 163 insertions(+), 50 deletions(-) diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index 40d884b3d76..0087a9034a1 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -52,8 +52,9 @@ export interface RequiredApi { // expressions. // `Expression == Expression` is an Expression // `Expression ? Expression : Expression` is an Expression -export type Expression = string; // eslint-disable-line +export type Expression = string; // eslint-disable-line export type Field = T | Expression | null; +export type FieldList = Expression | (string | Expression)[] | null; // A service account must either: // 1. Be a project-relative email that ends with "@" (e.g. database-users@) @@ -314,6 +315,7 @@ function envWithTypes( string: true, boolean: true, number: true, + list: true, }; for (const param of definedParams) { if (param.name === envName) { @@ -322,18 +324,28 @@ function envWithTypes( string: true, boolean: false, number: false, + list: false, }; } else if (param.type === "int") { providedType = { string: false, boolean: false, number: true, + list: false, }; } else if (param.type === "boolean") { providedType = { string: false, boolean: true, number: false, + list: false, + }; + } else if (param.type === "list") { + providedType = { + string: false, + boolean: false, + number: false, + list: true, }; } } diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index 0b9f08c4e64..93158bb0ef7 100644 --- a/src/deploy/functions/cel.ts +++ b/src/deploy/functions/cel.ts @@ -75,6 +75,10 @@ export function resolveExpression( expr: CelExpression, params: Record ): Literal { + // N.B: List literals [] can contain CEL inside them, so we need to process them + // first and resolve them. This isn't (and can't be) recursive, but the fact that + // we only support string[] types mostly saves us here. + expr = preprocessLists(wantType, expr, params); // N.B: Since some of these regexps are supersets of others--anything that is // params\.(\S+) is also (.+)--the order in which they are tested matters if (isIdentityExpression(expr)) { @@ -94,6 +98,66 @@ export function resolveExpression( } } +/** + * Replaces all lists in a CEL expression string, which can contain string-type CEL + * subexpressions or references to params, with their literal resolved values. + * Not recursive. + */ +function preprocessLists( + wantType: L, + expr: CelExpression, + params: Record +): CelExpression { + let rv = expr; + const listMatcher = /\[[^\[\]]*\]/g; + let match: RegExpMatchArray | null; + while ((match = listMatcher.exec(expr)) != null) { + const list = match[0]; + const resolved = resolveList("string", list, params); + rv = rv.replace(list, JSON.stringify(resolved)); + } + return rv; +} + +/** + * A List in Functions CEL is a []-bracketed string with comma-seperated values that can be: + * - A double quoted string literal + * - A reference to a param value (params.FOO) which must resolve with type string + * - A sub-CEL expression {{ params.BAR == 0 ? "a" : "b" }} which must resolve with type string + */ +function resolveList( + wantType: "string", + list: string, + params: Record +): string[] { + if (!list.startsWith("[") || !list.endsWith("]")) { + throw new ExprParseError(""); + } else if (list === "[]") { + return []; + } + const rv: string[] = []; + const entries = list.slice(1, -1).split(","); + + for (const entry of entries) { + const trimmed = entry.trim(); + if (trimmed.startsWith('"') && trimmed.endsWith('"')) { + rv.push(trimmed.slice(1, -1)); + } else if (trimmed.startsWith("{{") && trimmed.endsWith("}}")) { + rv.push(resolveExpression("string", trimmed, params) as string); + } else { + const paramMatch = paramRegexp.exec(trimmed); + if (!paramMatch) { + throw new ExprParseError(`Malformed list component ${trimmed}`); + } else if (!(paramMatch[1] in params)) { + throw new ExprParseError(""); + } + rv.push(resolveParamListOrLiteral("string", trimmed, params) as string); + } + } + + return rv; +} + function assertType(wantType: L, paramName: string, paramValue: ParamValue) { if ( (wantType === "string" && !paramValue.legalString) || @@ -307,7 +371,6 @@ function resolveDualTernary( if (!match) { throw new ExprParseError("Malformed CEL ternary expression '" + expr + "'"); } - const comparisonExpr = `{{ params.${match[1]} ${match[2]} params.${match[3]} }}`; const isTrue = resolveDualComparison(comparisonExpr, params); if (isTrue) { @@ -358,9 +421,7 @@ function resolveParamListOrLiteral( ): Literal { const match = paramRegexp.exec(field); if (!match) { - return wantType === "string[]" - ? resolveList("string", field, params) - : resolveLiteral(wantType, field); + return resolveLiteral(wantType, field); } const paramValue = params[match[1]]; if (!paramValue) { @@ -369,51 +430,27 @@ function resolveParamListOrLiteral( return readParamValue(wantType, match[1], paramValue); } -/** - * A List in Functions CEL is a []-bracketed string with comma-seperated values that can be: - * - A double quoted string literal - * - A reference to a param value (params.FOO) which must resolve with type string - * - A sub-CEL expression {{ params.BAR == 0 ? "a" : "b" }} which must resolve with type string - */ -function resolveList( - wantType: "string", - list: string, - params: Record -): string[] { - if (!list.startsWith("[") || !list.endsWith("]")) { - throw new ExprParseError(""); - } - const rv: string[] = []; - const entries = list.slice(1, -1).split(","); - - for (const entry of entries) { - const trimmed = entry.trim(); - if (trimmed.startsWith('"') && trimmed.endsWith('"')) { - rv.push(trimmed.slice(1, -1)); - } else if (trimmed.startsWith("{{") && trimmed.endsWith("}}")) { - rv.push(resolveExpression("string", trimmed, params) as string); - } else { - const paramMatch = paramRegexp.exec(trimmed); - if (!paramMatch) { - throw new ExprParseError(""); - } else if (!(paramMatch[1] in params)) { - throw new ExprParseError(""); - } - rv.push(resolveParamListOrLiteral("string", trimmed, params) as string); - } - } - - return rv; -} - -function resolveLiteral(wantType: Omit, value: string): Literal { +function resolveLiteral(wantType: L, value: string): Literal { if (paramRegexp.exec(value)) { throw new ExprParseError( "CEL tried to evaluate param." + value + " in a context which only permits literal values" ); } - if (wantType === "number") { + if (wantType === "string[]") { + // N.B: value being a literal list that can just be JSON.parsed should be guaranteed + // by the preprocessLists() invocation at the beginning of CEL resolution + const parsed = JSON.parse(value); + if (!Array.isArray(parsed)) { + throw new ExprParseError(""); + } + for (const shouldBeString of parsed) { + if (typeof shouldBeString !== "string") { + throw new ExprParseError(""); + } + } + return parsed as string[]; + } else if (wantType === "number") { if (isNaN(+value)) { throw new ExprParseError("CEL literal " + value + " does not seem to be a number"); } diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 5a9b12db67f..17141917125 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -83,7 +83,7 @@ export function resolveBoolean( type ParamInput = TextInput | SelectInput | ResourceInput; -type ParamBase = { +type ParamBase = { // name of the param. Will be exposed as an environment variable with this name name: string; @@ -136,6 +136,10 @@ export interface BooleanParam extends ParamBase { type: "boolean"; } +export interface ListParam extends ParamBase { + type: "list"; +} + export interface TextInput { // eslint-disable-line text: { example?: string; @@ -187,8 +191,8 @@ interface SecretParam { description?: string; } -export type Param = StringParam | IntParam | BooleanParam | SecretParam; -type RawParamValue = string | number | boolean; +export type Param = StringParam | IntParam | BooleanParam | ListParam | SecretParam; +type RawParamValue = string | number | boolean | string[]; /** * A type which contains the resolved value of a param, and metadata ensuring @@ -292,6 +296,8 @@ function canSatisfyParam(param: Param, value: RawParamValue): boolean { return typeof value === "number" && Number.isInteger(value); } else if (param.type === "boolean") { return typeof value === "boolean"; + } else if (param.type === "list") { + return Array.isArray(value); } else if (param.type === "secret") { return false; } @@ -450,6 +456,8 @@ async function promptParam( } else if (param.type === "boolean") { const provided = await promptBooleanParam(param, resolvedDefault as boolean | undefined); return new ParamValue(provided.toString(), false, { boolean: true }); + } else if (param.type === "list") { + throw new FirebaseError(`Unimplemented`); } else if (param.type === "secret") { throw new FirebaseError( `Somehow ended up trying to interactively prompt for secret parameter ${param.name}, which should never happen.` @@ -598,6 +606,10 @@ async function promptResourceString( } type retryInput = { message: string }; +function shouldRetry(obj: any): obj is retryInput { + return (obj as retryInput).message !== undefined; +} + async function promptText( prompt: string, input: TextInput, @@ -620,7 +632,7 @@ async function promptText( } } const converted = converter(res); - if (typeof converted === "object") { + if (shouldRetry(converted)) { logger.error(converted.message); return promptText(prompt, input, resolvedDefault, converter); } @@ -647,7 +659,7 @@ async function promptSelect( }), }); const converted = converter(response); - if (typeof converted === "object") { + if (shouldRetry(converted)) { logger.error(converted.message); return promptSelect(prompt, input, resolvedDefault, converter); } diff --git a/src/test/deploy/functions/cel.spec.ts b/src/test/deploy/functions/cel.spec.ts index d1b53d160f0..a6b939b313e 100644 --- a/src/test/deploy/functions/cel.spec.ts +++ b/src/test/deploy/functions/cel.spec.ts @@ -14,13 +14,65 @@ function boolV(value: boolean): ParamValue { describe("CEL evaluation", () => { describe("String list resolution", () => { - it("can pull lists directly otu of paramvalues", () => { + it("can pull lists directly out of paramvalues", () => { expect( resolveExpression("string[]", "{{ params.FOO }}", { FOO: new ParamValue('["1"]', false, { list: true }), }) ).to.deep.equal(["1"]); }); + + it("can handle literals in a list", () => { + expect( + resolveExpression("string[]", '{{ params.FOO == params.FOO ? ["asdf"] : [] }}', { + FOO: numberV(1), + }) + ).to.deep.equal(["asdf"]); + }); + + it("can handle CEL expressions in a list", () => { + expect( + resolveExpression("string[]", "{{ params.FOO == params.FOO ? [{{ params.BAR }}] : [] }}", { + FOO: numberV(1), + BAR: stringV("asdf"), + }) + ).to.deep.equal(["asdf"]); + }); + + it("can handle direct references to string params in a list", () => { + expect( + resolveExpression("string[]", "{{ params.FOO == params.FOO ? [params.BAR] : [] }}", { + FOO: numberV(1), + BAR: stringV("asdf"), + }) + ).to.deep.equal(["asdf"]); + }); + + it("can handle a list with multiple elements", () => { + expect( + resolveExpression( + "string[]", + '{{ params.FOO == params.FOO ? ["foo", params.BAR, {{ params.BAR }} ] : [] }}', + { + FOO: numberV(1), + BAR: stringV("asdf"), + } + ) + ).to.deep.equal(["foo", "asdf", "asdf"]); + }); + + it("isn't picky about whitespace around the commas", () => { + expect( + resolveExpression( + "string[]", + '{{ params.FOO == params.FOO ? ["foo",params.BAR,{{ params.BAR }}] : [] }}', + { + FOO: numberV(1), + BAR: stringV("asdf"), + } + ) + ).to.deep.equal(["foo", "asdf", "asdf"]); + }); }); describe("Identity expressions", () => { From faa1eeb32fa686dc164c9ca892d4fb6f0fddc6a2 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 19 Oct 2022 14:04:03 -0700 Subject: [PATCH 04/19] handle list comparisons --- src/deploy/functions/cel.ts | 37 ++++++++- src/test/deploy/functions/cel.spec.ts | 104 ++++++++++++++++++++++++-- 2 files changed, 130 insertions(+), 11 deletions(-) diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index 93158bb0ef7..9f1d1e1cf34 100644 --- a/src/deploy/functions/cel.ts +++ b/src/deploy/functions/cel.ts @@ -28,6 +28,15 @@ const ternaryRegexp = new RegExp( ); const literalTernaryRegexp = /{{ params\.(\S+) \? (.+) : (.+) }/; +/** + * An array equality test for use on resolved list literal ParamValues only; + * skips a lot of the null/undefined/object-y/nested-list checks that something + * like Underscore's isEqual() would make because args have to be string[]. + */ +function listEquals(a: string[], b: string[]): boolean { + return a.every((item) => b.includes(item)) && b.every((item) => a.includes(item)); +} + /** * Determines if something is a string that looks vaguely like a CEL expression. * No guarantees as to whether it'll actually evaluate. @@ -221,9 +230,9 @@ function resolveComparison( const test = function (a: Literal, b: Literal): boolean { switch (cmp) { case "!=": - return a !== b; + return Array.isArray(a) ? !listEquals(a, b as string[]) : a !== b; case "==": - return a === b; + return Array.isArray(a) ? listEquals(a, b as string[]) : a === b; case ">=": return a >= b; case "<=": @@ -254,6 +263,14 @@ function resolveComparison( } else if (lhsVal.legalBoolean) { rhs = resolveLiteral("boolean", match[3]); return test(lhsVal.asBoolean(), rhs); + } else if (lhsVal.legalList) { + if (!["==", "!="].includes(cmp)) { + throw new ExprParseError( + `Unsupported comparison operation ${cmp} on list operands in expression ${expr}` + ); + } + rhs = resolveLiteral("string[]", match[3]); + return test(lhsVal.asList(), rhs); } else { throw new ExprParseError( `Could not infer type of param ${lhsName} used in comparison operation` @@ -277,9 +294,9 @@ function resolveDualComparison( const test = function (a: Literal, b: Literal): boolean { switch (cmp) { case "!=": - return a !== b; + return Array.isArray(a) ? !listEquals(a, b as string[]) : a !== b; case "==": - return a === b; + return Array.isArray(a) ? listEquals(a, b as string[]) : a === b; case ">=": return a >= b; case "<=": @@ -330,6 +347,18 @@ function resolveDualComparison( ); } return test(lhsVal.asBoolean(), rhsVal.asBoolean()); + } else if (lhsVal.legalList) { + if (!rhsVal.legalList) { + throw new ExprParseError( + `CEL comparison expression ${expr} has type mismatch between the operands` + ); + } + if (!["==", "!="].includes(cmp)) { + throw new ExprParseError( + `Unsupported comparison operation ${cmp} on list operands in expression ${expr}` + ); + } + return test(lhsVal.asList(), rhsVal.asList()); } else { throw new ExprParseError( `could not infer type of param ${lhsName} used in comparison operation` diff --git a/src/test/deploy/functions/cel.spec.ts b/src/test/deploy/functions/cel.spec.ts index a6b939b313e..8b63d5f948c 100644 --- a/src/test/deploy/functions/cel.spec.ts +++ b/src/test/deploy/functions/cel.spec.ts @@ -3,13 +3,31 @@ import { resolveExpression, ExprParseError } from "../../../deploy/functions/cel import { ParamValue } from "../../../deploy/functions/params"; function stringV(value: string): ParamValue { - return new ParamValue(value, false, { string: true, number: false, boolean: false }); + return new ParamValue(value, false, { string: true, number: false, boolean: false, list: false }); } function numberV(value: number): ParamValue { - return new ParamValue(value.toString(), false, { string: false, number: true, boolean: false }); + return new ParamValue(value.toString(), false, { + string: false, + number: true, + boolean: false, + list: false, + }); } function boolV(value: boolean): ParamValue { - return new ParamValue(value.toString(), false, { string: false, number: false, boolean: true }); + return new ParamValue(value.toString(), false, { + string: false, + number: false, + boolean: true, + list: false, + }); +} +function listV(value: string[]): ParamValue { + return new ParamValue(JSON.stringify(value), false, { + string: false, + number: false, + boolean: false, + list: true, + }); } describe("CEL evaluation", () => { @@ -17,7 +35,7 @@ describe("CEL evaluation", () => { it("can pull lists directly out of paramvalues", () => { expect( resolveExpression("string[]", "{{ params.FOO }}", { - FOO: new ParamValue('["1"]', false, { list: true }), + FOO: listV(["1"]), }) ).to.deep.equal(["1"]); }); @@ -52,7 +70,7 @@ describe("CEL evaluation", () => { expect( resolveExpression( "string[]", - '{{ params.FOO == params.FOO ? ["foo", params.BAR, {{ params.BAR }} ] : [] }}', + '{{ params.FOO == params.FOO ? [ "foo", params.BAR, {{ params.BAR }} ] : [] }}', { FOO: numberV(1), BAR: stringV("asdf"), @@ -65,13 +83,85 @@ describe("CEL evaluation", () => { expect( resolveExpression( "string[]", - '{{ params.FOO == params.FOO ? ["foo",params.BAR,{{ params.BAR }}] : [] }}', + '{{ params.FOO == params.FOO ? ["foo ",params.BAR ,{{ params.BAR }}] : [] }}', { FOO: numberV(1), BAR: stringV("asdf"), } ) - ).to.deep.equal(["foo", "asdf", "asdf"]); + ).to.deep.equal(["foo ", "asdf", "asdf"]); + }); + + it("can do == comparisons between lists", () => { + expect( + resolveExpression("boolean", "{{ params.FOO == params.FOO }}", { + FOO: listV(["a", "2", "false"]), + }) + ).to.be.true; + expect( + resolveExpression("boolean", '{{ params.FOO == ["a", "2", "false"] }}', { + FOO: listV(["a", "2", "false"]), + }) + ).to.be.true; + expect( + resolveExpression("boolean", "{{ params.FOO != params.FOO }}", { + FOO: listV(["a", "2", "false"]), + }) + ).to.be.false; + expect( + resolveExpression("boolean", '{{ params.FOO != ["a", "2", "false"] }}', { + FOO: listV(["a", "2", "false"]), + }) + ).to.be.false; + expect( + resolveExpression("boolean", "{{ params.FOO == params.BAR }}", { + FOO: listV(["a", "2", "false"]), + BAR: listV(["b", "-2", "true"]), + }) + ).to.be.false; + expect( + resolveExpression("boolean", '{{ params.FOO == ["a", "2", "false"] }}', { + FOO: listV(["b", "-2", "true"]), + }) + ).to.be.false; + expect( + resolveExpression("boolean", "{{ params.FOO != params.BAR }}", { + FOO: listV(["a", "2", "false"]), + BAR: listV(["b", "-2", "true"]), + }) + ).to.be.true; + expect( + resolveExpression("boolean", '{{ params.FOO != ["a", "2", "false"] }}', { + FOO: listV(["b", "-2", "true"]), + }) + ).to.be.true; + }); + + it("throws if asked to do type comparisons between lists", () => { + expect(() => + resolveExpression("boolean", "{{ params.FOO > params.BAR }}", { + FOO: listV(["a", "2", "false"]), + BAR: listV(["b", "-2", "true"]), + }) + ).to.throw(ExprParseError); + expect(() => + resolveExpression("boolean", "{{ params.FOO >= params.BAR }}", { + FOO: listV(["a", "2", "false"]), + BAR: listV(["b", "-2", "true"]), + }) + ).to.throw(ExprParseError); + expect(() => + resolveExpression("boolean", "{{ params.FOO < params.BAR }}", { + FOO: listV(["a", "2", "false"]), + BAR: listV(["b", "-2", "true"]), + }) + ).to.throw(ExprParseError); + expect(() => + resolveExpression("boolean", "{{ params.FOO <= params.BAR }}", { + FOO: listV(["a", "2", "false"]), + BAR: listV(["b", "-2", "true"]), + }) + ).to.throw(ExprParseError); }); }); From af42549b808eaa345851de53102cbd5747802b13 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Thu, 20 Oct 2022 14:09:23 -0700 Subject: [PATCH 05/19] actually support a real list param --- src/deploy/functions/build.ts | 8 ++-- src/deploy/functions/params.ts | 15 +++++++ .../functions/runtimes/discovery/parsing.ts | 11 ++++- .../functions/runtimes/discovery/v1alpha1.ts | 4 +- .../runtimes/discovery/v1alpha1.spec.ts | 42 +++++++++++++++++++ 5 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index 0087a9034a1..d5c0bd989ea 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -235,7 +235,7 @@ export type Endpoint = Triggered & { // defaults to ["us-central1"], overridable in firebase-tools with // process.env.FIREBASE_FUNCTIONS_DEFAULT_REGION - region?: string[]; + region?: FieldList; // The Cloud project associated with this endpoint. project: string; @@ -426,9 +426,11 @@ export function toBackend( for (const endpointId of Object.keys(build.endpoints)) { const bdEndpoint = build.endpoints[endpointId]; - let regions = bdEndpoint.region; - if (typeof regions === "undefined") { + let regions: string[] = []; + if (!bdEndpoint.region) { regions = [api.functionsDefaultRegion]; + } else { + regions = params.resolveList(bdEndpoint.region, paramValues); } for (const region of regions) { const trigger = discoverTrigger(bdEndpoint, region, r); diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index d8cf1a5a5f4..7db421bcea0 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -66,6 +66,21 @@ export function resolveString( return output; } +export function resolveList( + from: build.FieldList, + paramValues: Record +): string[] { + if (!from) { + return []; + } else if (Array.isArray(from)) { + return from.map((entry) => resolveString(entry, paramValues)); + } else if (typeof from === "string") { + return resolveExpression("string[]", from, paramValues) as string[]; + } else { + assertExhaustive(from); + } +} + /** * Resolves a boolean field in a Build to an an actual boolean value. * Fields can be literal or an expression written in a subset of the CEL specification. diff --git a/src/deploy/functions/runtimes/discovery/parsing.ts b/src/deploy/functions/runtimes/discovery/parsing.ts index 396af1e15af..0a33e18174f 100644 --- a/src/deploy/functions/runtimes/discovery/parsing.ts +++ b/src/deploy/functions/runtimes/discovery/parsing.ts @@ -34,6 +34,7 @@ export type FieldType = | `Field${NullSuffix}` | `Field${NullSuffix}` | `Field${NullSuffix}` + | `List${NullSuffix}` | ((t: T) => boolean); export type Schema = { @@ -49,7 +50,7 @@ export function requireKeys(prefix: string, yaml: T, ...keys: } for (const key of keys) { if (!yaml[key]) { - throw new FirebaseError(`Expected key ${prefix + key}`); + throw new FirebaseError(`Expected key ${prefix + key.toString()}`); } } } @@ -106,6 +107,14 @@ export function assertKeyTypes( } continue; } + if (schemaType === "List") { + if (typeof value !== "string" && !Array.isArray(value)) { + throw new FirebaseError( + `Expected ${fullKey} to be a field list (array or list expression); was ${typeof value}` + ); + } + continue; + } if (value === null) { if (schemaType.endsWith("?")) { diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.ts index be232b4cf06..ca6b4bb2109 100644 --- a/src/deploy/functions/runtimes/discovery/v1alpha1.ts +++ b/src/deploy/functions/runtimes/discovery/v1alpha1.ts @@ -63,7 +63,7 @@ export type WireEndpoint = build.Triggered & // We now use "serviceAccount" but maintain backwards compatability in the // wire format for the time being. serviceAccountEmail?: string | null; - region?: string[]; + region?: build.FieldList; entryPoint: string; platform?: build.FunctionsPlatform; secretEnvironmentVariables?: Array | null; @@ -121,7 +121,7 @@ function parseRequiredAPIs(manifest: WireManifest): build.RequiredApi[] { function assertBuildEndpoint(ep: WireEndpoint, id: string): void { const prefix = `endpoints[${id}]`; assertKeyTypes(prefix, ep, { - region: "array", + region: "List", platform: (platform) => build.AllFunctionsPlatforms.includes(platform), entryPoint: "string", availableMemoryMb: (mem) => mem === null || isCEL(mem) || build.isValidMemoryOption(mem), diff --git a/src/test/deploy/functions/runtimes/discovery/v1alpha1.spec.ts b/src/test/deploy/functions/runtimes/discovery/v1alpha1.spec.ts index f38610358ae..3793e69a667 100644 --- a/src/test/deploy/functions/runtimes/discovery/v1alpha1.spec.ts +++ b/src/test/deploy/functions/runtimes/discovery/v1alpha1.spec.ts @@ -609,6 +609,48 @@ describe("buildFromV1Alpha", () => { expect(parsed).to.deep.equal(expected); }); + it("allows both CEL and lists containing CEL in FieldList typed keys", () => { + const yamlCEL: v1alpha1.WireManifest = { + specVersion: "v1alpha1", + endpoints: { + id: { + ...MIN_WIRE_ENDPOINT, + httpsTrigger: {}, + region: "{{ params.REGION }}", + }, + }, + }; + const parsedCEL = v1alpha1.buildFromV1Alpha1(yamlCEL, PROJECT, REGION, RUNTIME); + const expectedCEL: build.Build = build.of({ + id: { + ...DEFAULTED_ENDPOINT, + region: "{{ params.REGION }}", + httpsTrigger: {}, + }, + }); + expect(parsedCEL).to.deep.equal(expectedCEL); + + const yamlList: v1alpha1.WireManifest = { + specVersion: "v1alpha1", + endpoints: { + id: { + ...MIN_WIRE_ENDPOINT, + httpsTrigger: {}, + region: ["{{ params.FOO }}", "BAR", "params.BAZ"], + }, + }, + }; + const parsedList = v1alpha1.buildFromV1Alpha1(yamlList, PROJECT, REGION, RUNTIME); + const expectedList: build.Build = build.of({ + id: { + ...DEFAULTED_ENDPOINT, + region: ["{{ params.FOO }}", "BAR", "params.BAZ"], + httpsTrigger: {}, + }, + }); + expect(parsedList).to.deep.equal(expectedList); + }); + it("copies schedules", () => { const scheduleTrigger: build.ScheduleTrigger = { schedule: "every 5 minutes", From 5bd29401beebe8a01cc1f732627c647645bcf12e Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 26 Oct 2022 14:57:43 -0700 Subject: [PATCH 06/19] um, placeholder input methods? --- src/deploy/functions/params.ts | 102 +++++++++++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 4 deletions(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 7db421bcea0..b49e9cd9288 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -66,6 +66,9 @@ export function resolveString( return output; } +/** + * + */ export function resolveList( from: build.FieldList, paramValues: Record @@ -96,7 +99,7 @@ export function resolveBoolean( return resolveExpression("boolean", from, paramValues) as boolean; } -type ParamInput = TextInput | SelectInput | ResourceInput; +type ParamInput = TextInput | SelectInput | ListSelectInput | ResourceInput; type ParamBase = { // name of the param. Will be exposed as an environment variable with this name @@ -138,6 +141,12 @@ export function isSelectInput(input: ParamInput): input is SelectInput export function isResourceInput(input: ParamInput): input is ResourceInput { return {}.hasOwnProperty.call(input, "resource"); } +/** + * Determines whether an Input field value can be coerced to ListSelectInput. + */ +export function isListSelectInput(input: ParamInput): input is ListSelectInput { + return {}.hasOwnProperty.call(input, "listSelect"); +} export interface StringParam extends ParamBase { type: "string"; @@ -191,6 +200,12 @@ interface ResourceInput { }; } +interface ListSelectInput { + listSelect: { + options: Array>; + }; +} + interface SecretParam { type: "secret"; @@ -472,7 +487,8 @@ async function promptParam( const provided = await promptBooleanParam(param, resolvedDefault as boolean | undefined); return new ParamValue(provided.toString(), false, { boolean: true }); } else if (param.type === "list") { - throw new FirebaseError(`Unimplemented`); + const provided = await promptList(param, resolvedDefault as string[] | undefined); + return new ParamValue(JSON.stringify(provided), false, { list: true }); } else if (param.type === "secret") { throw new FirebaseError( `Somehow ended up trying to interactively prompt for secret parameter ${param.name}, which should never happen.` @@ -481,6 +497,52 @@ async function promptParam( assertExhaustive(param); } +async function promptList(param: ListParam, resolvedDefault?: string[]): Promise { + if (!param.input) { + const defaultToText: TextInput = { text: {} }; + param.input = defaultToText; + } + let prompt: string; + const stringToList = (res: string): string[] | retryInput => { + const converted = JSON.parse(res) || []; + if (!Array.isArray(converted)) { + return { message: `"${res}" is not an array` }; + } + for (const entry of converted) { + if (typeof entry !== "string") { + return { message: `Parsed array contains a non-string element ${entry}` }; + } + } + return converted as string[]; + }; + + if (isSelectInput(param.input)) { + throw new FirebaseError("List params cannot have non-list selector inputs"); + } else if (isListSelectInput(param.input)) { + prompt = `Select a value for ${param.label || param.name}:`; + if (param.description) { + prompt += ` \n(${param.description})`; + } + prompt += "\nSelect an option with the arrow keys, and use Enter to confirm your choice. "; + return promptSelectMultiple( + prompt, + param.input, + resolvedDefault, + (res: string[]) => res + ); + } else if (isTextInput(param.input)) { + prompt = `Enter a list of strings for ${param.label || param.name}:`; + if (param.description) { + prompt += ` \n(${param.description})`; + } + return promptText(prompt, param.input, resolvedDefault, stringToList); + } else if (isResourceInput(param.input)) { + throw new FirebaseError("Boolean params cannot have Cloud Resource selector inputs"); + } else { + assertExhaustive(param.input); + } +} + async function promptBooleanParam( param: BooleanParam, resolvedDefault?: boolean @@ -499,6 +561,8 @@ async function promptBooleanParam( } prompt += "\nSelect an option with the arrow keys, and use Enter to confirm your choice. "; return promptSelect(prompt, param.input, resolvedDefault, isTruthyInput); + } else if (isListSelectInput(param.input)) { + throw new FirebaseError("Non-list params cannot have list selector inputs"); } else if (isTextInput(param.input)) { prompt = `Enter a boolean value for ${param.label || param.name}:`; if (param.description) { @@ -529,6 +593,8 @@ async function promptStringParam( prompt += ` \n(${param.description})`; } return promptResourceString(prompt, param.input, projectId, resolvedDefault); + } else if (isListSelectInput(param.input)) { + throw new FirebaseError("Non-list params cannot have list selector inputs"); } else if (isSelectInput(param.input)) { prompt = `Select a value for ${param.label || param.name}:`; if (param.description) { @@ -569,8 +635,9 @@ async function promptIntParam(param: IntParam, resolvedDefault?: number): Promis } return +res; }); - } - if (isTextInput(param.input)) { + } else if (isListSelectInput(param.input)) { + throw new FirebaseError("Non-list params cannot have list selector inputs"); + } else if (isTextInput(param.input)) { prompt = `Enter an integer value for ${param.label || param.name}:`; if (param.description) { prompt += ` \n(${param.description})`; @@ -683,3 +750,30 @@ async function promptSelect( } return converted; } + +async function promptSelectMultiple( + prompt: string, + input: ListSelectInput, + resolvedDefault: T[] | undefined, + converter: (res: string[]) => T[] | retryInput +): Promise { + const response = await promptOnce({ + name: "input", + type: "checkbox", + default: resolvedDefault, + message: prompt, + choices: input.listSelect.options.map((option: SelectOptions): ListItem => { + return { + checked: false, + name: option.label, + value: option.value.toString(), + }; + }), + }); + const converted = converter(response); + if (shouldRetry(converted)) { + logger.error(converted.message); + return promptSelectMultiple(prompt, input, resolvedDefault, converter); + } + return converted; +} From d628d9f9d5ded888b50cf8aeefa0c62365d70863 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Tue, 1 Nov 2022 12:47:34 -0700 Subject: [PATCH 07/19] .env lists as comma seperated strings instead of serialized objects --- src/deploy/functions/params.ts | 15 ++++++--------- src/test/deploy/functions/cel.spec.ts | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index b49e9cd9288..65dd0236bea 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -254,10 +254,11 @@ export class ParamValue { this.legalList = types.list || false; } + static fromList(ls: string[], delimiter = ","): string { + return ls.join(delimiter); + } + toString(): string { - if (this.legalList) { - return encodeURI(JSON.stringify(this.rawValue)); - } return this.rawValue; } @@ -269,12 +270,8 @@ export class ParamValue { return ["true", "y", "yes", "1"].includes(this.rawValue); } - asList(): string[] { - const parsed = JSON.parse(decodeURI(this.rawValue)); - if (!Array.isArray(parsed)) { - // uh-oh - } - return parsed; + asList(delimiter = ","): string[] { + return this.rawValue.split(delimiter); } asNumber(): number { diff --git a/src/test/deploy/functions/cel.spec.ts b/src/test/deploy/functions/cel.spec.ts index 8b63d5f948c..fa289455780 100644 --- a/src/test/deploy/functions/cel.spec.ts +++ b/src/test/deploy/functions/cel.spec.ts @@ -22,7 +22,7 @@ function boolV(value: boolean): ParamValue { }); } function listV(value: string[]): ParamValue { - return new ParamValue(JSON.stringify(value), false, { + return new ParamValue(ParamValue.fromList(value), false, { string: false, number: false, boolean: false, From 1c6df4eb4ea65bc0c6c90b3db4968f2885c5bb2b Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Tue, 1 Nov 2022 14:29:36 -0700 Subject: [PATCH 08/19] -what if the previous commit but actually working --- src/deploy/functions/params.ts | 27 +++++++++++++++++++++------ src/test/deploy/functions/cel.spec.ts | 7 +------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 65dd0236bea..f0da23c44df 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -67,7 +67,9 @@ export function resolveString( } /** - * + * Resolves a FieldList in a Build to an an actual string[] value. + * FieldLists can be a list of string | Expression, or a single + * Expression. */ export function resolveList( from: build.FieldList, @@ -162,6 +164,8 @@ export interface BooleanParam extends ParamBase { export interface ListParam extends ParamBase { type: "list"; + + delimiter?: string; } export interface TextInput { // eslint-disable-line @@ -242,6 +246,8 @@ export class ParamValue { legalNumber: boolean; // Whether this param value can be sensibly interpreted as a list legalList: boolean; + // What delimiter to use between fields when reading/writing to .env format + delimiter: string; constructor( private readonly rawValue: string, @@ -252,10 +258,17 @@ export class ParamValue { this.legalBoolean = types.boolean || false; this.legalNumber = types.number || false; this.legalList = types.list || false; + this.delimiter = ","; + } + + static fromList(ls: string[], delimiter = ","): ParamValue { + const pv = new ParamValue(ls.join(delimiter), false, { list: true }); + pv.setDelimiter(delimiter); + return pv; } - static fromList(ls: string[], delimiter = ","): string { - return ls.join(delimiter); + setDelimiter(delimiter: string) { + this.delimiter = delimiter; } toString(): string { @@ -270,8 +283,8 @@ export class ParamValue { return ["true", "y", "yes", "1"].includes(this.rawValue); } - asList(delimiter = ","): string[] { - return this.rawValue.split(delimiter); + asList(): string[] { + return this.rawValue.split(this.delimiter); } asNumber(): number { @@ -306,6 +319,8 @@ function resolveDefaultCEL( return resolveString(expr, currentEnv); case "int": return resolveInt(expr, currentEnv); + case "list": + return resolveList(expr, currentEnv); default: throw new FirebaseError( "Build specified parameter with default " + expr + " of unsupported type" @@ -485,7 +500,7 @@ async function promptParam( return new ParamValue(provided.toString(), false, { boolean: true }); } else if (param.type === "list") { const provided = await promptList(param, resolvedDefault as string[] | undefined); - return new ParamValue(JSON.stringify(provided), false, { list: true }); + return ParamValue.fromList(provided, param.delimiter); } else if (param.type === "secret") { throw new FirebaseError( `Somehow ended up trying to interactively prompt for secret parameter ${param.name}, which should never happen.` diff --git a/src/test/deploy/functions/cel.spec.ts b/src/test/deploy/functions/cel.spec.ts index fa289455780..1083953131c 100644 --- a/src/test/deploy/functions/cel.spec.ts +++ b/src/test/deploy/functions/cel.spec.ts @@ -22,12 +22,7 @@ function boolV(value: boolean): ParamValue { }); } function listV(value: string[]): ParamValue { - return new ParamValue(ParamValue.fromList(value), false, { - string: false, - number: false, - boolean: false, - list: true, - }); + return ParamValue.fromList(value); } describe("CEL evaluation", () => { From cf66cd0632466dbbef91e0fb812bc0c014bdca5e Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Thu, 3 Nov 2022 14:32:07 -0700 Subject: [PATCH 09/19] smaller style fixes --- src/deploy/functions/build.ts | 4 ++-- src/deploy/functions/params.ts | 4 ++-- src/deploy/functions/runtimes/discovery/v1alpha1.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/deploy/functions/build.ts b/src/deploy/functions/build.ts index d5c0bd989ea..d5c323c7372 100644 --- a/src/deploy/functions/build.ts +++ b/src/deploy/functions/build.ts @@ -54,7 +54,7 @@ export interface RequiredApi { // `Expression ? Expression : Expression` is an Expression export type Expression = string; // eslint-disable-line export type Field = T | Expression | null; -export type FieldList = Expression | (string | Expression)[] | null; +export type ListField = Expression | (string | Expression)[] | null; // A service account must either: // 1. Be a project-relative email that ends with "@" (e.g. database-users@) @@ -235,7 +235,7 @@ export type Endpoint = Triggered & { // defaults to ["us-central1"], overridable in firebase-tools with // process.env.FIREBASE_FUNCTIONS_DEFAULT_REGION - region?: FieldList; + region?: ListField; // The Cloud project associated with this endpoint. project: string; diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index f0da23c44df..2f4796acf90 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -72,7 +72,7 @@ export function resolveString( * Expression. */ export function resolveList( - from: build.FieldList, + from: build.ListField, paramValues: Record ): string[] { if (!from) { @@ -701,7 +701,7 @@ async function promptResourceString( type retryInput = { message: string }; function shouldRetry(obj: any): obj is retryInput { - return (obj as retryInput).message !== undefined; + return typeof obj === "object" && (obj as retryInput).message !== undefined; } async function promptText( diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.ts index ca6b4bb2109..9bb50081d11 100644 --- a/src/deploy/functions/runtimes/discovery/v1alpha1.ts +++ b/src/deploy/functions/runtimes/discovery/v1alpha1.ts @@ -63,7 +63,7 @@ export type WireEndpoint = build.Triggered & // We now use "serviceAccount" but maintain backwards compatability in the // wire format for the time being. serviceAccountEmail?: string | null; - region?: build.FieldList; + region?: build.ListField; entryPoint: string; platform?: build.FunctionsPlatform; secretEnvironmentVariables?: Array | null; From 5cf448008d5abfaf4f8e187d8809098d9b2cb45d Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 7 Nov 2022 23:00:45 -0800 Subject: [PATCH 10/19] some work on input methods --- src/deploy/functions/params.ts | 58 ++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 2f4796acf90..e655a2cdaa6 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -499,7 +499,7 @@ async function promptParam( const provided = await promptBooleanParam(param, resolvedDefault as boolean | undefined); return new ParamValue(provided.toString(), false, { boolean: true }); } else if (param.type === "list") { - const provided = await promptList(param, resolvedDefault as string[] | undefined); + const provided = await promptList(param, projectId, resolvedDefault as string[] | undefined); return ParamValue.fromList(provided, param.delimiter); } else if (param.type === "secret") { throw new FirebaseError( @@ -509,24 +509,16 @@ async function promptParam( assertExhaustive(param); } -async function promptList(param: ListParam, resolvedDefault?: string[]): Promise { +async function promptList( + param: ListParam, + projectId: string, + resolvedDefault?: string[] +): Promise { if (!param.input) { const defaultToText: TextInput = { text: {} }; param.input = defaultToText; } let prompt: string; - const stringToList = (res: string): string[] | retryInput => { - const converted = JSON.parse(res) || []; - if (!Array.isArray(converted)) { - return { message: `"${res}" is not an array` }; - } - for (const entry of converted) { - if (typeof entry !== "string") { - return { message: `Parsed array contains a non-string element ${entry}` }; - } - } - return converted as string[]; - }; if (isSelectInput(param.input)) { throw new FirebaseError("List params cannot have non-list selector inputs"); @@ -547,9 +539,15 @@ async function promptList(param: ListParam, resolvedDefault?: string[]): Promise if (param.description) { prompt += ` \n(${param.description})`; } - return promptText(prompt, param.input, resolvedDefault, stringToList); + return promptText(prompt, param.input, resolvedDefault, (res: string): string[] => { + return res.split(param.delimiter || ","); + }); } else if (isResourceInput(param.input)) { - throw new FirebaseError("Boolean params cannot have Cloud Resource selector inputs"); + prompt = `Select values for ${param.label || param.name}:`; + if (param.description) { + prompt += ` \n(${param.description})`; + } + return promptResourceStrings(prompt, param.input, projectId); } else { assertExhaustive(param.input); } @@ -699,6 +697,34 @@ async function promptResourceString( } } +async function promptResourceStrings( + prompt: string, + input: ResourceInput, + projectId: string +): Promise { + const notFound = new FirebaseError(`No instances of ${input.resource.type} found.`); + switch (input.resource.type) { + case "storage.googleapis.com/Bucket": + const buckets = await listBuckets(projectId); + if (buckets.length === 0) { + throw notFound; + } + const forgedInput: ListSelectInput = { + listSelect: { + options: buckets.map((bucketName: string): SelectOptions => { + return { label: bucketName, value: bucketName }; + }), + }, + }; + return promptSelectMultiple(prompt, forgedInput, undefined, (res: string[]) => res); + default: + logger.warn( + `Warning: unknown resource type ${input.resource.type}; defaulting to raw text input...` + ); + return promptText(prompt, { text: {} }, undefined, (res: string) => res.split(",")); + } +} + type retryInput = { message: string }; function shouldRetry(obj: any): obj is retryInput { return typeof obj === "object" && (obj as retryInput).message !== undefined; From 8782c044db8ddcaf5df0ee7e9baff99c811837b6 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 16 Nov 2022 00:08:00 -0800 Subject: [PATCH 11/19] assorted changes per reviews --- src/deploy/functions/params.ts | 34 +++++++++++++++++++-------------- src/deploy/functions/prepare.ts | 5 +++-- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index e655a2cdaa6..3b6ba5f5543 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -101,7 +101,7 @@ export function resolveBoolean( return resolveExpression("boolean", from, paramValues) as boolean; } -type ParamInput = TextInput | SelectInput | ListSelectInput | ResourceInput; +type ParamInput = TextInput | SelectInput | MultiSelectInput | ResourceInput; type ParamBase = { // name of the param. Will be exposed as an environment variable with this name @@ -146,8 +146,8 @@ export function isResourceInput(input: ParamInput): input is ResourceInput /** * Determines whether an Input field value can be coerced to ListSelectInput. */ -export function isListSelectInput(input: ParamInput): input is ListSelectInput { - return {}.hasOwnProperty.call(input, "listSelect"); +export function isMultiSelectInput(input: ParamInput): input is MultiSelectInput { + return {}.hasOwnProperty.call(input, "multiSelect"); } export interface StringParam extends ParamBase { @@ -204,8 +204,8 @@ interface ResourceInput { }; } -interface ListSelectInput { - listSelect: { +interface MultiSelectInput { + multiSelect: { options: Array>; }; } @@ -271,10 +271,16 @@ export class ParamValue { this.delimiter = delimiter; } + // Returns this param's representation as it should be in .env files toString(): string { return this.rawValue; } + // Returns this param's representatiom as it should be in process.env during runtime + toSDK(): string { + return this.legalList ? JSON.stringify(this.asList()) : this.toString() + } + asString(): string { return this.rawValue; } @@ -522,7 +528,7 @@ async function promptList( if (isSelectInput(param.input)) { throw new FirebaseError("List params cannot have non-list selector inputs"); - } else if (isListSelectInput(param.input)) { + } else if (isMultiSelectInput(param.input)) { prompt = `Select a value for ${param.label || param.name}:`; if (param.description) { prompt += ` \n(${param.description})`; @@ -535,7 +541,7 @@ async function promptList( (res: string[]) => res ); } else if (isTextInput(param.input)) { - prompt = `Enter a list of strings for ${param.label || param.name}:`; + prompt = `Enter a list of strings (delimiter: ${param.delimiter ? param.delimiter : ','}) for ${param.label || param.name}:`; if (param.description) { prompt += ` \n(${param.description})`; } @@ -571,7 +577,7 @@ async function promptBooleanParam( } prompt += "\nSelect an option with the arrow keys, and use Enter to confirm your choice. "; return promptSelect(prompt, param.input, resolvedDefault, isTruthyInput); - } else if (isListSelectInput(param.input)) { + } else if (isMultiSelectInput(param.input)) { throw new FirebaseError("Non-list params cannot have list selector inputs"); } else if (isTextInput(param.input)) { prompt = `Enter a boolean value for ${param.label || param.name}:`; @@ -603,7 +609,7 @@ async function promptStringParam( prompt += ` \n(${param.description})`; } return promptResourceString(prompt, param.input, projectId, resolvedDefault); - } else if (isListSelectInput(param.input)) { + } else if (isMultiSelectInput(param.input)) { throw new FirebaseError("Non-list params cannot have list selector inputs"); } else if (isSelectInput(param.input)) { prompt = `Select a value for ${param.label || param.name}:`; @@ -645,7 +651,7 @@ async function promptIntParam(param: IntParam, resolvedDefault?: number): Promis } return +res; }); - } else if (isListSelectInput(param.input)) { + } else if (isMultiSelectInput(param.input)) { throw new FirebaseError("Non-list params cannot have list selector inputs"); } else if (isTextInput(param.input)) { prompt = `Enter an integer value for ${param.label || param.name}:`; @@ -709,8 +715,8 @@ async function promptResourceStrings( if (buckets.length === 0) { throw notFound; } - const forgedInput: ListSelectInput = { - listSelect: { + const forgedInput: MultiSelectInput = { + multiSelect: { options: buckets.map((bucketName: string): SelectOptions => { return { label: bucketName, value: bucketName }; }), @@ -791,7 +797,7 @@ async function promptSelect( async function promptSelectMultiple( prompt: string, - input: ListSelectInput, + input: MultiSelectInput, resolvedDefault: T[] | undefined, converter: (res: string[]) => T[] | retryInput ): Promise { @@ -800,7 +806,7 @@ async function promptSelectMultiple( type: "checkbox", default: resolvedDefault, message: prompt, - choices: input.listSelect.options.map((option: SelectOptions): ListItem => { + choices: input.multiSelect.options.map((option: SelectOptions): ListItem => { return { checked: false, name: option.label, diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index 876a434ee16..5d9a51b7eeb 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -128,11 +128,12 @@ export async function prepare( let hasEnvsFromParams = false; wantBackend.environmentVariables = envs; for (const envName of Object.keys(resolvedEnvs)) { - const envValue = resolvedEnvs[envName]?.toString(); + const isList = resolvedEnvs[envName]?.legalList + const envValue = resolvedEnvs[envName]?.toSDK(); if ( envValue && !resolvedEnvs[envName].internal && - !Object.prototype.hasOwnProperty.call(wantBackend.environmentVariables, envName) + (!Object.prototype.hasOwnProperty.call(wantBackend.environmentVariables, envName) || isList) ) { wantBackend.environmentVariables[envName] = envValue; hasEnvsFromParams = true; From bd77603ad371d763bcc6af872c6da7e55833d4de Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Wed, 16 Nov 2022 00:53:15 -0800 Subject: [PATCH 12/19] eslint fix --- src/deploy/functions/params.ts | 6 ++++-- src/deploy/functions/prepare.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 3b6ba5f5543..94060bdc0b5 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -278,7 +278,7 @@ export class ParamValue { // Returns this param's representatiom as it should be in process.env during runtime toSDK(): string { - return this.legalList ? JSON.stringify(this.asList()) : this.toString() + return this.legalList ? JSON.stringify(this.asList()) : this.toString(); } asString(): string { @@ -541,7 +541,9 @@ async function promptList( (res: string[]) => res ); } else if (isTextInput(param.input)) { - prompt = `Enter a list of strings (delimiter: ${param.delimiter ? param.delimiter : ','}) for ${param.label || param.name}:`; + prompt = `Enter a list of strings (delimiter: ${param.delimiter ? param.delimiter : ","}) for ${ + param.label || param.name + }:`; if (param.description) { prompt += ` \n(${param.description})`; } diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index 5d9a51b7eeb..2f7979d20e0 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -128,7 +128,7 @@ export async function prepare( let hasEnvsFromParams = false; wantBackend.environmentVariables = envs; for (const envName of Object.keys(resolvedEnvs)) { - const isList = resolvedEnvs[envName]?.legalList + const isList = resolvedEnvs[envName]?.legalList; const envValue = resolvedEnvs[envName]?.toSDK(); if ( envValue && From afcf6f31505a77c9b1aca3a401d60dace6d670cf Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Sun, 18 Dec 2022 23:05:37 -0800 Subject: [PATCH 13/19] Update src/deploy/functions/cel.ts Co-authored-by: Daniel Lee --- src/deploy/functions/cel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index 9f1d1e1cf34..e4cc29d27c5 100644 --- a/src/deploy/functions/cel.ts +++ b/src/deploy/functions/cel.ts @@ -140,7 +140,7 @@ function resolveList( params: Record ): string[] { if (!list.startsWith("[") || !list.endsWith("]")) { - throw new ExprParseError(""); + throw new ExprParseError("Invalid list: must start with '[' and end with ']'"); } else if (list === "[]") { return []; } From b2de4f65d5a1a834ea7c8bbbb853c147cc2c5548 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Sun, 18 Dec 2022 23:05:50 -0800 Subject: [PATCH 14/19] Update src/deploy/functions/params.ts Co-authored-by: Daniel Lee --- src/deploy/functions/params.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 70ce8af0635..abaca5f1b22 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -144,7 +144,7 @@ export function isResourceInput(input: ParamInput): input is ResourceInput return {}.hasOwnProperty.call(input, "resource"); } /** - * Determines whether an Input field value can be coerced to ListSelectInput. + * Determines whether an Input field value can be coerced to MultiSelectInput. */ export function isMultiSelectInput(input: ParamInput): input is MultiSelectInput { return {}.hasOwnProperty.call(input, "multiSelect"); From c8aa2ef31832880dc004aab894ae69ee1ae2ef6b Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Sun, 18 Dec 2022 23:05:57 -0800 Subject: [PATCH 15/19] Update src/deploy/functions/params.ts Co-authored-by: Daniel Lee --- src/deploy/functions/params.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index abaca5f1b22..29c85e880c5 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -612,7 +612,7 @@ async function promptStringParam( } return promptResourceString(prompt, param.input, projectId, resolvedDefault); } else if (isMultiSelectInput(param.input)) { - throw new FirebaseError("Non-list params cannot have list selector inputs"); + throw new FirebaseError("Non-list params cannot have multi selector inputs"); } else if (isSelectInput(param.input)) { prompt = `Select a value for ${param.label || param.name}:`; if (param.description) { From 8cc6f89ff72b798632dbb4af0e152fefcf7f5ce8 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Sun, 18 Dec 2022 23:06:02 -0800 Subject: [PATCH 16/19] Update src/deploy/functions/params.ts Co-authored-by: Daniel Lee --- src/deploy/functions/params.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 29c85e880c5..8bc1f20db7f 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -654,7 +654,7 @@ async function promptIntParam(param: IntParam, resolvedDefault?: number): Promis return +res; }); } else if (isMultiSelectInput(param.input)) { - throw new FirebaseError("Non-list params cannot have list selector inputs"); + throw new FirebaseError("Non-list params cannot have multi selector inputs"); } else if (isTextInput(param.input)) { prompt = `Enter an integer value for ${param.label || param.name}:`; if (param.description) { From b54d4891d7ac32a8a12c90ea424c24233ada0919 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Sun, 18 Dec 2022 23:06:16 -0800 Subject: [PATCH 17/19] Update src/deploy/functions/params.ts Co-authored-by: Daniel Lee --- src/deploy/functions/params.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deploy/functions/params.ts b/src/deploy/functions/params.ts index 8bc1f20db7f..18c000e5683 100644 --- a/src/deploy/functions/params.ts +++ b/src/deploy/functions/params.ts @@ -580,7 +580,7 @@ async function promptBooleanParam( prompt += "\nSelect an option with the arrow keys, and use Enter to confirm your choice. "; return promptSelect(prompt, param.input, resolvedDefault, isTruthyInput); } else if (isMultiSelectInput(param.input)) { - throw new FirebaseError("Non-list params cannot have list selector inputs"); + throw new FirebaseError("Non-list params cannot have multi selector inputs"); } else if (isTextInput(param.input)) { prompt = `Enter a boolean value for ${param.label || param.name}:`; if (param.description) { From eb54228f2c5059266e8e0e672d232576ff7ce46b Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Sun, 18 Dec 2022 23:09:52 -0800 Subject: [PATCH 18/19] more error messages filled in --- src/deploy/functions/cel.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index e4cc29d27c5..c31392854af 100644 --- a/src/deploy/functions/cel.ts +++ b/src/deploy/functions/cel.ts @@ -158,7 +158,7 @@ function resolveList( if (!paramMatch) { throw new ExprParseError(`Malformed list component ${trimmed}`); } else if (!(paramMatch[1] in params)) { - throw new ExprParseError(""); + throw new ExprParseError(`List expansion referenced nonexistent param ${paramMatch[1]}`); } rv.push(resolveParamListOrLiteral("string", trimmed, params) as string); } @@ -471,11 +471,11 @@ function resolveLiteral(wantType: L, value: string): Literal { // by the preprocessLists() invocation at the beginning of CEL resolution const parsed = JSON.parse(value); if (!Array.isArray(parsed)) { - throw new ExprParseError(""); + throw new ExprParseError(`CEL tried to read non-list ${JSON.stringify(parsed)} as a list`); } for (const shouldBeString of parsed) { if (typeof shouldBeString !== "string") { - throw new ExprParseError(""); + throw new ExprParseError(`Evaluated CEL list ${JSON.stringify(parsed)} contained non-string values`); } } return parsed as string[]; From 65c5635f53bf8b86d3634d8ad6196593b7194bb5 Mon Sep 17 00:00:00 2001 From: Victor Fan Date: Mon, 19 Dec 2022 02:10:35 -0800 Subject: [PATCH 19/19] eslint fix --- src/deploy/functions/cel.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/deploy/functions/cel.ts b/src/deploy/functions/cel.ts index c31392854af..9e694eb3acd 100644 --- a/src/deploy/functions/cel.ts +++ b/src/deploy/functions/cel.ts @@ -475,7 +475,9 @@ function resolveLiteral(wantType: L, value: string): Literal { } for (const shouldBeString of parsed) { if (typeof shouldBeString !== "string") { - throw new ExprParseError(`Evaluated CEL list ${JSON.stringify(parsed)} contained non-string values`); + throw new ExprParseError( + `Evaluated CEL list ${JSON.stringify(parsed)} contained non-string values` + ); } } return parsed as string[];