From 101558a99a184e1110827bd293f2a8220a4b4992 Mon Sep 17 00:00:00 2001 From: Chris Rybicki Date: Mon, 2 Oct 2023 16:46:06 -0400 Subject: [PATCH] feat(sdk): allow re-adding environment variable value to function or service (#4379) If the same environment variable is added multiple times, and we know for sure that the values are identical, then repeated calls should be OK since the operations are commutative (addEnvironment method calls can still be safely reordered). ## Checklist - [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [ ] Description explains motivation and solution - [ ] Tests added (always) - [ ] Docs updated (only required for features) - [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*. --- examples/tests/sdk_tests/function/env.test.w | 15 ++ libs/wingsdk/src/cloud/function.ts | 6 +- libs/wingsdk/src/cloud/service.ts | 6 +- libs/wingsdk/src/shared-tf/tokens.ts | 13 +- .../function/env.test.w_compile_tf-aws.md | 249 ++++++++++++++++++ .../sdk_tests/function/env.test.w_test_sim.md | 12 + 6 files changed, 293 insertions(+), 8 deletions(-) create mode 100644 examples/tests/sdk_tests/function/env.test.w create mode 100644 tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_compile_tf-aws.md create mode 100644 tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_test_sim.md diff --git a/examples/tests/sdk_tests/function/env.test.w b/examples/tests/sdk_tests/function/env.test.w new file mode 100644 index 00000000000..c9d163e05e6 --- /dev/null +++ b/examples/tests/sdk_tests/function/env.test.w @@ -0,0 +1,15 @@ +bring cloud; +bring util; + +let f1 = new cloud.Function(inflight (): str => { + assert(util.env("FOO1") == "bar"); + assert(util.env("FOO2") == "baz"); + return "ok"; +}, env: { FOO1: "bar" }); + +f1.addEnvironment("FOO1", "bar"); // same key-value pair can be added multiple times +f1.addEnvironment("FOO2", "baz"); + +test "addEnvironment" { + assert(f1.invoke("") == "ok"); +} diff --git a/libs/wingsdk/src/cloud/function.ts b/libs/wingsdk/src/cloud/function.ts index ef2634c78fc..ff9bc386e4b 100644 --- a/libs/wingsdk/src/cloud/function.ts +++ b/libs/wingsdk/src/cloud/function.ts @@ -116,8 +116,10 @@ export abstract class Function extends Resource implements IInflightHost { * Add an environment variable to the function. */ public addEnvironment(name: string, value: string) { - if (this._env[name] !== undefined) { - throw new Error(`Environment variable "${name}" already set.`); + if (this._env[name] !== undefined && this._env[name] !== value) { + throw new Error( + `Environment variable "${name}" already set with a different value.` + ); } this._env[name] = value; } diff --git a/libs/wingsdk/src/cloud/service.ts b/libs/wingsdk/src/cloud/service.ts index 74782e64a4f..c89fac43c67 100644 --- a/libs/wingsdk/src/cloud/service.ts +++ b/libs/wingsdk/src/cloud/service.ts @@ -111,8 +111,10 @@ export abstract class Service extends Resource implements IInflightHost { * Add an environment variable to the function. */ public addEnvironment(name: string, value: string) { - if (this._env[name] !== undefined) { - throw new Error(`Environment variable "${name}" already set.`); + if (this._env[name] !== undefined && this._env[name] !== value) { + throw new Error( + `Environment variable "${name}" already set with a different value.` + ); } this._env[name] = value; } diff --git a/libs/wingsdk/src/shared-tf/tokens.ts b/libs/wingsdk/src/shared-tf/tokens.ts index c16492c9c32..2b8e8040053 100644 --- a/libs/wingsdk/src/shared-tf/tokens.ts +++ b/libs/wingsdk/src/shared-tf/tokens.ts @@ -8,6 +8,8 @@ import { IInflightHost } from "../std"; * Tokens values are captured as environment variable, and resolved through the compilation target token mechanism. */ export class CdkTfTokens extends Tokens { + private _jsonEncodeCache = new Map(); + /** * Returns true is the given value is a CDKTF token. */ @@ -34,10 +36,13 @@ export class CdkTfTokens extends Tokens { } const envName = this.envName(JSON.stringify(value)); - const envValue = Fn.jsonencode(value); - // the same token might be bound multiple times by different variables/inflight contexts - if (host.env[envName] === undefined) { - host.addEnvironment(envName, envValue); + + // Fn.jsonencode produces a fresh CDKTF token each time, so we cache the results + let envValue = this._jsonEncodeCache.get(envName); + if (!envValue) { + envValue = Fn.jsonencode(value); + this._jsonEncodeCache.set(envName, envValue); } + host.addEnvironment(envName, envValue); } } diff --git a/tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_compile_tf-aws.md new file mode 100644 index 00000000000..b57caba8f45 --- /dev/null +++ b/tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_compile_tf-aws.md @@ -0,0 +1,249 @@ +# [env.test.w](../../../../../../examples/tests/sdk_tests/function/env.test.w) | compile | tf-aws + +## inflight.$Closure1-1.js +```js +module.exports = function({ $util_Util }) { + class $Closure1 { + constructor({ }) { + const $obj = (...args) => this.handle(...args); + Object.setPrototypeOf($obj, this); + return $obj; + } + async handle() { + {((cond) => {if (!cond) throw new Error("assertion failed: util.env(\"FOO1\") == \"bar\"")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })((await $util_Util.env("FOO1")),"bar")))}; + {((cond) => {if (!cond) throw new Error("assertion failed: util.env(\"FOO2\") == \"baz\"")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })((await $util_Util.env("FOO2")),"baz")))}; + return "ok"; + } + } + return $Closure1; +} + +``` + +## inflight.$Closure2-1.js +```js +module.exports = function({ $f1 }) { + class $Closure2 { + constructor({ }) { + const $obj = (...args) => this.handle(...args); + Object.setPrototypeOf($obj, this); + return $obj; + } + async handle() { + {((cond) => {if (!cond) throw new Error("assertion failed: f1.invoke(\"\") == \"ok\"")})((((a,b) => { try { return require('assert').deepStrictEqual(a,b) === undefined; } catch { return false; } })((await $f1.invoke("")),"ok")))}; + } + } + return $Closure2; +} + +``` + +## main.tf.json +```json +{ + "//": { + "metadata": { + "backend": "local", + "stackName": "root", + "version": "0.17.0" + }, + "outputs": { + "root": { + "Default": { + "cloud.TestRunner": { + "TestFunctionArns": "WING_TEST_RUNNER_FUNCTION_ARNS" + } + } + } + } + }, + "output": { + "WING_TEST_RUNNER_FUNCTION_ARNS": { + "value": "[]" + } + }, + "provider": { + "aws": [ + {} + ] + }, + "resource": { + "aws_iam_role": { + "cloudFunction_IamRole_5A4430DC": { + "//": { + "metadata": { + "path": "root/Default/Default/cloud.Function/IamRole", + "uniqueId": "cloudFunction_IamRole_5A4430DC" + } + }, + "assume_role_policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Action\":\"sts:AssumeRole\",\"Principal\":{\"Service\":\"lambda.amazonaws.com\"},\"Effect\":\"Allow\"}]}" + } + }, + "aws_iam_role_policy": { + "cloudFunction_IamRolePolicy_618BF987": { + "//": { + "metadata": { + "path": "root/Default/Default/cloud.Function/IamRolePolicy", + "uniqueId": "cloudFunction_IamRolePolicy_618BF987" + } + }, + "policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Action\":\"none:null\",\"Resource\":\"*\"}]}", + "role": "${aws_iam_role.cloudFunction_IamRole_5A4430DC.name}" + } + }, + "aws_iam_role_policy_attachment": { + "cloudFunction_IamRolePolicyAttachment_288B9653": { + "//": { + "metadata": { + "path": "root/Default/Default/cloud.Function/IamRolePolicyAttachment", + "uniqueId": "cloudFunction_IamRolePolicyAttachment_288B9653" + } + }, + "policy_arn": "arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole", + "role": "${aws_iam_role.cloudFunction_IamRole_5A4430DC.name}" + } + }, + "aws_lambda_function": { + "cloudFunction": { + "//": { + "metadata": { + "path": "root/Default/Default/cloud.Function/Default", + "uniqueId": "cloudFunction" + } + }, + "architectures": [ + "arm64" + ], + "environment": { + "variables": { + "FOO1": "bar", + "FOO2": "baz", + "WING_FUNCTION_NAME": "cloud-Function-c8d2eca1", + "WING_TARGET": "tf-aws" + } + }, + "function_name": "cloud-Function-c8d2eca1", + "handler": "index.handler", + "publish": true, + "role": "${aws_iam_role.cloudFunction_IamRole_5A4430DC.arn}", + "runtime": "nodejs18.x", + "s3_bucket": "${aws_s3_bucket.Code.bucket}", + "s3_key": "${aws_s3_object.cloudFunction_S3Object_71908BAD.key}", + "timeout": 60, + "vpc_config": { + "security_group_ids": [], + "subnet_ids": [] + } + } + }, + "aws_s3_bucket": { + "Code": { + "//": { + "metadata": { + "path": "root/Default/Code", + "uniqueId": "Code" + } + }, + "bucket_prefix": "code-c84a50b1-" + } + }, + "aws_s3_object": { + "cloudFunction_S3Object_71908BAD": { + "//": { + "metadata": { + "path": "root/Default/Default/cloud.Function/S3Object", + "uniqueId": "cloudFunction_S3Object_71908BAD" + } + }, + "bucket": "${aws_s3_bucket.Code.bucket}", + "key": "", + "source": "" + } + } + } +} +``` + +## preflight.js +```js +const $stdlib = require('@winglang/sdk'); +const $plugins = ((s) => !s ? [] : s.split(';'))(process.env.WING_PLUGIN_PATHS); +const $outdir = process.env.WING_SYNTH_DIR ?? "."; +const $wing_is_test = process.env.WING_IS_TEST === "true"; +const std = $stdlib.std; +const cloud = $stdlib.cloud; +const util = $stdlib.util; +class $Root extends $stdlib.std.Resource { + constructor(scope, id) { + super(scope, id); + class $Closure1 extends $stdlib.std.Resource { + constructor(scope, id, ) { + super(scope, id); + (std.Node.of(this)).hidden = true; + } + static _toInflightType(context) { + return ` + require("./inflight.$Closure1-1.js")({ + $util_Util: ${context._lift($stdlib.core.toLiftableModuleType(util.Util, "@winglang/sdk/util", "Util"))}, + }) + `; + } + _toInflight() { + return ` + (await (async () => { + const $Closure1Client = ${$Closure1._toInflightType(this)}; + const client = new $Closure1Client({ + }); + if (client.$inflight_init) { await client.$inflight_init(); } + return client; + })()) + `; + } + _getInflightOps() { + return ["handle", "$inflight_init"]; + } + } + class $Closure2 extends $stdlib.std.Resource { + constructor(scope, id, ) { + super(scope, id); + (std.Node.of(this)).hidden = true; + } + static _toInflightType(context) { + return ` + require("./inflight.$Closure2-1.js")({ + $f1: ${context._lift(f1)}, + }) + `; + } + _toInflight() { + return ` + (await (async () => { + const $Closure2Client = ${$Closure2._toInflightType(this)}; + const client = new $Closure2Client({ + }); + if (client.$inflight_init) { await client.$inflight_init(); } + return client; + })()) + `; + } + _getInflightOps() { + return ["handle", "$inflight_init"]; + } + _registerBind(host, ops) { + if (ops.includes("handle")) { + $Closure2._registerBindObject(f1, host, ["invoke"]); + } + super._registerBind(host, ops); + } + } + const f1 = this.node.root.newAbstract("@winglang/sdk.cloud.Function",this,"cloud.Function",new $Closure1(this,"$Closure1"),{ env: ({"FOO1": "bar"}) }); + (f1.addEnvironment("FOO1","bar")); + (f1.addEnvironment("FOO2","baz")); + this.node.root.new("@winglang/sdk.std.Test",std.Test,this,"test:addEnvironment",new $Closure2(this,"$Closure2")); + } +} +const $App = $stdlib.core.App.for(process.env.WING_TARGET); +new $App({ outdir: $outdir, name: "env.test", rootConstruct: $Root, plugins: $plugins, isTestEnvironment: $wing_is_test, entrypointDir: process.env['WING_SOURCE_DIR'], rootId: process.env['WING_ROOT_ID'] }).synth(); + +``` + diff --git a/tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_test_sim.md b/tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_test_sim.md new file mode 100644 index 00000000000..c5fa4bcb9a3 --- /dev/null +++ b/tools/hangar/__snapshots__/test_corpus/sdk_tests/function/env.test.w_test_sim.md @@ -0,0 +1,12 @@ +# [env.test.w](../../../../../../examples/tests/sdk_tests/function/env.test.w) | test | sim + +## stdout.log +```log +pass ─ env.test.wsim » root/env0/test:addEnvironment + + +Tests 1 passed (1) +Test Files 1 passed (1) +Duration +``` +