-
Notifications
You must be signed in to change notification settings - Fork 17
Major: rework environment configuration of CDK #109
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
Changes from 12 commits
4273be3
8e2c62e
b49fc51
3d5c37f
479fb8c
e30c850
04f65b2
c8597b5
316a292
58a609a
56237d4
dd3aaf1
06b3bdc
d58ccca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
name: Unit tests | ||
|
||
on: | ||
push: | ||
branches: | ||
- main | ||
pull_request: | ||
workflow_dispatch: | ||
inputs: | ||
node-version: | ||
required: false | ||
default: "22.x" | ||
|
||
# Only one pull-request triggered run should be executed at a time | ||
# (head_ref is only set for PR events, otherwise fallback to run_id which differs for every run). | ||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
|
||
jobs: | ||
unit-tests: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v2 | ||
|
||
- name: Use Node.js ${{ inputs.node-version }} | ||
uses: actions/setup-node@v2 | ||
with: | ||
node-version: ${{ inputs.node-version }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: same as above, newest version for that action is |
||
|
||
- name: Install project | ||
run: | | ||
npm install | ||
|
||
- name: Run tests | ||
run: | | ||
npm run test | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -28,12 +28,22 @@ To resolve this, set the `NODE_PATH` variable pointing to your AWS CDK's `node_m | |||||
$ export NODE_PATH=$NODE_PATH:/opt/homebrew/Cellar/aws-cdk/<CDK_VERSION>/libexec/lib/node_modules | ||||||
``` | ||||||
|
||||||
## Version support | ||||||
|
||||||
`cdklocal` supports all installed versions of the node `aws-cdk` package, however some complications are present for `aws-cdk >= 2.177.0`. | ||||||
|
||||||
For these CDK versions, we remove AWS configuration environment variables like `AWS_PROFILE` from the shell environment before invoking the `cdk` command, and explicitly set `AWS_ENDPOINT_URL` and `AWS_ENDPOINT_URL_S3` to target LocalStack. | ||||||
|
||||||
1. We do this because other environment variables may lead to a conflicting set of configuration options, where the wrong region is used to target LocalStack, or `cdklocal` tries to deploy into upstream AWS by mistake. If individual configuration variables are needed for the deploy process (e.g. `AwS_REGION`) these configuration variables can be propagated to the `cdk` command by configuring `AWS_ENVAR_ALLOWLIST`, for example: `AWS_ENVAR_ALLOWLIST=AWS_REGION,AWS_DEFAULT_REGION AWS_REGION=eu-central-1 cdklocal ...`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
also: Isn't the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I don't understand - what do you mean by "don't provide it in the following env arguments"? Do you mean in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean
|
||||||
2. If you are manually setting `AWS_ENDPOINT_URL`, the new value will continue to be read from the environment. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is confusing me a bit as it seems to conflict with the PR description:
|
||||||
|
||||||
## Configurations | ||||||
|
||||||
The following environment variables can be configured: | ||||||
|
||||||
* `AWS_ENDPOINT_URL`: The endpoint URL to connect to (combination of `USE_SSL`/`LOCALSTACK_HOSTNAME`/`EDGE_PORT` below) | ||||||
* `AWS_ENDPOINT_URL_S3`: The endpoint URL to connect to (combination of `USE_SSL`/`LOCALSTACK_HOSTNAME`/`EDGE_PORT` below) for S3 requests | ||||||
* `AWS_ENVAR_ALLOWLIST`: Allow specific `AWS_*` environment variables to be used by the CDK | ||||||
* `EDGE_PORT` (deprecated): Port under which LocalStack edge service is accessible (default: `4566`) | ||||||
* `LOCALSTACK_HOSTNAME` (deprecated): Target host under which LocalStack edge service is accessible (default: `localhost`) | ||||||
* `USE_SSL` (deprecated): Whether to use SSL to connect to the LocalStack endpoint, i.e., connect via HTTPS. | ||||||
|
@@ -78,6 +88,7 @@ $ awslocal sns list-topics | |||||
|
||||||
## Change Log | ||||||
|
||||||
* 3.0.0: Sanitise environment of configuration environment variables before deployment | ||||||
* 2.19.2: Fix SDK compatibility with aws-cdk versions >= 2.177.0 | ||||||
* 2.19.1: Fix SDK compatibility with older CDK versions; Fix patched bucket location in TemplateURL | ||||||
* 2.19.0: Add support for aws-cdk versions >= `2.167.0` | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
const { configureEnvironment, EnvironmentMisconfigurationError } = require("../src"); | ||
|
||
describe("configureEnvironment", () => { | ||
test("empty environment", () => { | ||
const env = {}; | ||
const allowListStr = ""; | ||
configureEnvironment(env, allowListStr); | ||
expect(env).toEqual({ | ||
AWS_ACCESS_KEY_ID: "test", | ||
AWS_SECRET_ACCESS_KEY: "test", | ||
AWS_REGION: "us-east-1", | ||
AWS_DEFAULT_REGION: "us-east-1", | ||
AWS_ENDPOINT_URL: "http://localhost.localstack.cloud:4566", | ||
AWS_ENDPOINT_URL_S3: "http://s3.localhost.localstack.cloud:4566", | ||
}); | ||
}); | ||
|
||
test("custom endpoint url", () => { | ||
const env = { | ||
AWS_ENDPOINT_URL: "http://foo.bar:4567", | ||
AWS_ENDPOINT_URL_S3: "http://foo.bar:4567", | ||
}; | ||
const allowListStr = ""; | ||
configureEnvironment(env, allowListStr); | ||
expect(env).toEqual({ | ||
AWS_ACCESS_KEY_ID: "test", | ||
AWS_SECRET_ACCESS_KEY: "test", | ||
AWS_REGION: "us-east-1", | ||
AWS_DEFAULT_REGION: "us-east-1", | ||
AWS_ENDPOINT_URL: "http://foo.bar:4567", | ||
AWS_ENDPOINT_URL_S3: "http://foo.bar:4567", | ||
}); | ||
}); | ||
|
||
test("custom endpoint url without specifying s3 url", () => { | ||
const env = { | ||
AWS_ENDPOINT_URL: "http://foo.bar:4567", | ||
}; | ||
const allowListStr = ""; | ||
expect(() => configureEnvironment(env, allowListStr)).toThrow(EnvironmentMisconfigurationError); | ||
}); | ||
|
||
test("strip extra configuration envars", () => { | ||
const env = { | ||
AWS_PROFILE: "my-profile", | ||
}; | ||
const allowListStr = ""; | ||
configureEnvironment(env, allowListStr); | ||
expect(env).toEqual({ | ||
AWS_ACCESS_KEY_ID: "test", | ||
AWS_SECRET_ACCESS_KEY: "test", | ||
AWS_REGION: "us-east-1", | ||
AWS_DEFAULT_REGION: "us-east-1", | ||
AWS_ENDPOINT_URL: "http://localhost.localstack.cloud:4566", | ||
AWS_ENDPOINT_URL_S3: "http://s3.localhost.localstack.cloud:4566", | ||
}); | ||
}); | ||
|
||
test("allowlist of profile", () => { | ||
const env = { | ||
AWS_PROFILE: "my-profile", | ||
}; | ||
const allowListStr = "AWS_PROFILE"; | ||
configureEnvironment(env, allowListStr); | ||
expect(env).toEqual({ | ||
AWS_ACCESS_KEY_ID: "test", | ||
AWS_SECRET_ACCESS_KEY: "test", | ||
AWS_PROFILE: "my-profile", | ||
AWS_REGION: "us-east-1", | ||
AWS_DEFAULT_REGION: "us-east-1", | ||
AWS_ENDPOINT_URL: "http://localhost.localstack.cloud:4566", | ||
AWS_ENDPOINT_URL_S3: "http://s3.localhost.localstack.cloud:4566", | ||
}); | ||
}); | ||
|
||
test("credentials overriding", () => { | ||
const env = { | ||
AWS_ACCESS_KEY_ID: "something", | ||
AWS_SECRET_ACCESS_KEY: "else", | ||
}; | ||
const allowListStr = "AWS_PROFILE,AWS_SECRET_ACCESS_KEY,AWS_ACCESS_KEY_ID"; | ||
configureEnvironment(env, allowListStr); | ||
expect(env).toEqual({ | ||
AWS_ACCESS_KEY_ID: "something", | ||
AWS_SECRET_ACCESS_KEY: "else", | ||
AWS_REGION: "us-east-1", | ||
AWS_DEFAULT_REGION: "us-east-1", | ||
AWS_ENDPOINT_URL: "http://localhost.localstack.cloud:4566", | ||
AWS_ENDPOINT_URL_S3: "http://s3.localhost.localstack.cloud:4566", | ||
}); | ||
}); | ||
|
||
test("region overriding", () => { | ||
const env = { | ||
AWS_REGION: "eu-central-1", | ||
AWS_DEFAULT_REGION: "eu-central-1", | ||
}; | ||
const allowListStr = "AWS_REGION,AWS_DEFAULT_REGION"; | ||
configureEnvironment(env, allowListStr); | ||
expect(env).toEqual({ | ||
AWS_ACCESS_KEY_ID: "test", | ||
AWS_SECRET_ACCESS_KEY: "test", | ||
AWS_REGION: "eu-central-1", | ||
AWS_DEFAULT_REGION: "eu-central-1", | ||
AWS_ENDPOINT_URL: "http://localhost.localstack.cloud:4566", | ||
AWS_ENDPOINT_URL_S3: "http://s3.localhost.localstack.cloud:4566", | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
module.exports = { | ||
testEnvironment: "node", | ||
verbose: true, | ||
coverageDirectory: "coverage", | ||
collectCoverageFrom: ["src/**/*.{js,jsx,ts,tsx}"], | ||
testMatch: ["**/__tests__/**/*.[jt]s?(x)", "**/?(*.)+(spec|test).[jt]s?(x)"], | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: could use the newest version (
v4
) if there's no specific reason to stay on a lower one