Skip to content

Support for writing params of type List during discovery #1283

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
Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions spec/fixtures/sources/commonjs-params/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ params.defineInt("ANOTHER_INT", {
},
});

params.defineList("LIST_PARAM", {input: { multiSelect: { options: [{ value: "c" }, { value: "d" }, { value: "e" }]}}})

params.defineSecret("SUPER_SECRET_FLAG");

// N.B: invocation of the precanned internal params should not affect the manifest
Expand Down
34 changes: 34 additions & 0 deletions spec/params/params.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ describe("Params value extraction", () => {
process.env.PI = "3.14159";
process.env.TRUE = "true";
process.env.FALSE = "false";
process.env.LIST = JSON.stringify(["a", "b", "c"]);
process.env.BAD_LIST = JSON.stringify(["a", 22, "c"]);
process.env.ESCAPED_LIST = JSON.stringify(["f\to\no"]);
process.env.A_SECRET_STRING = "123456supersecret";
});

Expand All @@ -42,6 +45,9 @@ describe("Params value extraction", () => {
delete process.env.PI;
delete process.env.TRUE;
delete process.env.FALSE;
delete process.env.LIST;
delete process.env.BAD_LIST;
delete process.env.ESCAPED_LIST;
delete process.env.A_SECRET_STRING;
});

Expand All @@ -61,6 +67,11 @@ describe("Params value extraction", () => {
const falseParam = params.defineBoolean("FALSE");
expect(falseParam.value()).to.be.false;

const listParam = params.defineList("LIST");
expect(listParam.value()).to.deep.equal(["a", "b", "c"]);

const listParamWithEscapes = params.defineList("ESCAPED_LIST");
expect(listParamWithEscapes.value()).to.deep.equal(["f\to\no"]);
const secretParam = params.defineSecret("A_SECRET_STRING");
expect(secretParam.value()).to.equal("123456supersecret");
});
Expand Down Expand Up @@ -97,6 +108,17 @@ describe("Params value extraction", () => {

const stringToBool = params.defineBoolean("A_STRING");
expect(stringToBool.value()).to.equal(false);

const listToInt = params.defineInt("LIST");
expect(listToInt.value()).to.equal(0);
});

it("falls back on the javascript zero values in case a list param's is unparsable as string[]", () => {
const notAllStrings = params.defineList("BAD_LIST");
Copy link
Contributor

Choose a reason for hiding this comment

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

no action required - but i can see this being pretty frustrating if we silent fall back to an empty list if the list isn't made up of all string elements :/.

I don't think we expect this to happen in real life since environment variable will almost always be strings anyway?

expect(notAllStrings.value()).to.deep.equal([]);

const intToList = params.defineList("AN_INT");
expect(intToList.value()).to.deep.equal([]);
});

it("returns a boolean value for Comparison expressions", () => {
Expand Down Expand Up @@ -177,6 +199,18 @@ describe("Params value extraction", () => {
expect(trueParam.cmp("<=", false).value()).to.be.false;
});

it("can test list params for equality but not < or >", () => {
const p1 = params.defineList("LIST");
const p2 = params.defineList("ESCAPED_LIST");

expect(p1.equals(p1).value()).to.be.true;
expect(p1.notEquals(p1).value()).to.be.false;
expect(p1.equals(p2).value()).to.be.false;
expect(p1.notEquals(p2).value()).to.be.true;

expect(() => p1.greaterThan(p1).value()).to.throw;
Copy link
Member

Choose a reason for hiding this comment

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

This is why I wish we had done operators in the type system with strong types for each expression type =/

});

it("can select the output of a ternary expression based on the comparison", () => {
const trueExpr = params.defineString("A_STRING").equals(params.defineString("SAME_STRING"));
expect(trueExpr.thenElse(1, 0).value()).to.equal(1);
Expand Down
7 changes: 7 additions & 0 deletions spec/runtime/loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,13 @@ describe("loadStack", () => {
},
},
},
{
name: "LIST_PARAM",
type: "list",
input: {
multiSelect: { options: [{ value: "c" }, { value: "d" }, { value: "e" }] },
},
},
{ name: "SUPER_SECRET_FLAG", type: "secret" },
],
},
Expand Down
14 changes: 14 additions & 0 deletions src/params/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
ParamOptions,
SecretParam,
StringParam,
ListParam,
InternalExpression,
} from "./types";

Expand Down Expand Up @@ -164,3 +165,16 @@ export function defineFloat(name: string, options: ParamOptions<number> = {}): F
registerParam(param);
return param;
}

/**
* Declare a list param.
*
* @param name The name of the environment variable to use to load the param.
* @param options Configuration options for the param.
* @returns A Param with a `string[]` return type for `.value`.
*/
export function defineList(name: string, options: ParamOptions<string[]> = {}): ListParam {
const param = new ListParam(name, options);
registerParam(param);
return param;
}
82 changes: 74 additions & 8 deletions src/params/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,26 @@ export abstract class Expression<T extends string | number | boolean | string[]>
}
}

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;
}

function valueOf<T extends string | number | boolean | string[]>(arg: T | Expression<T>): T {
return arg instanceof Expression ? arg.runtimeValue() : arg;
}
/**
* Returns how an entity (either an Expression or a literal value) should be represented in CEL.
* - Expressions delegate to the .toString() method, which is used by the WireManifest
* - Strings have to be quoted explicitly
* - Arrays are represented as []-delimited, parsable JSON
* - Numbers and booleans are not quoted explicitly
*/
function refOf<T extends string | number | boolean | string[]>(arg: T | Expression<T>): string {
return arg instanceof Expression ? arg.toString() : quoteIfString(arg).toString();
if (arg instanceof Expression) {
return arg.toString();
} else if (typeof arg === "string") {
return `"${arg}"`;
} else if (Array.isArray(arg)) {
return JSON.stringify(arg);
} else {
return arg.toString();
}
}

/**
Expand Down Expand Up @@ -123,9 +133,9 @@ export class CompareExpression<
const right = valueOf(this.rhs);
switch (this.cmp) {
case "==":
return left === right;
return Array.isArray(left) ? this.arrayEquals(left, right as string[]) : left === right;
case "!=":
return left !== right;
return Array.isArray(left) ? !this.arrayEquals(left, right as string[]) : left !== right;
case ">":
return left > right;
case ">=":
Expand All @@ -139,6 +149,11 @@ export class CompareExpression<
}
}

/** @internal */
arrayEquals(a: string[], b: string[]): boolean {
return a.every((item) => b.includes(item)) && b.every((item) => a.includes(item));
}

toString() {
const rhsStr = refOf(this.rhs);
return `${this.lhs} ${this.cmp} ${rhsStr}`;
Expand All @@ -159,6 +174,7 @@ type ParamValueType = "string" | "list" | "boolean" | "int" | "float" | "secret"
type ParamInput<T> =
| { text: TextInput<T> }
| { select: SelectInput<T> }
| { multiSelect: MultiSelectInput }
| { resource: ResourceInput };

/**
Expand Down Expand Up @@ -201,6 +217,15 @@ export interface SelectInput<T = unknown> {
options: Array<SelectOptions<T>>;
}

/**
* Specifies that a Param's value should be determined by having the user select
* a subset from a list of pre-canned options interactively at deploy-time.
* Will result in errors if used on Params of type other than string[].
*/
export interface MultiSelectInput {
options: Array<SelectOptions<string>>;
}

/**
* One of the options provided to a SelectInput, containing a value and
* optionally a human-readable label to display in the selection interface.
Expand Down Expand Up @@ -464,3 +489,44 @@ export class BooleanParam extends Param<boolean> {
return new TernaryExpression(this, ifTrue, ifFalse);
}
}

/**
* A parametrized value of String[] type that will be read from .env files
* if present, or prompted for by the CLI if missing.
*/
export class ListParam extends Param<string[]> {
static type: ParamValueType = "list";

/** @internal */
runtimeValue(): string[] {
const val = JSON.parse(process.env[this.name]);
if (!Array.isArray(val) || !(val as string[]).every((v) => typeof v === "string")) {
return [];
}
return val as string[];
}

/** @hidden */
// eslint-disable-next-line @typescript-eslint/no-unused-vars
greaterThan(rhs: string[] | Expression<string[]>): CompareExpression<string[]> {
throw new Error(">/< comparison operators not supported on params of type List");
}

/** @hidden */
// eslint-disable-next-line @typescript-eslint/no-unused-vars
greaterThanOrEqualTo(rhs: string[] | Expression<string[]>): CompareExpression<string[]> {
throw new Error(">/< comparison operators not supported on params of type List");
}

/** @hidden */
// eslint-disable-next-line @typescript-eslint/no-unused-vars
lessThan(rhs: string[] | Expression<string[]>): CompareExpression<string[]> {
throw new Error(">/< comparison operators not supported on params of type List");
}

/** @hidden */
// eslint-disable-next-line @typescript-eslint/no-unused-vars
lessThanorEqualTo(rhs: string[] | Expression<string[]>): CompareExpression<string[]> {
throw new Error(">/< comparison operators not supported on params of type List");
}
}