Skip to content
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

feat(scheduler-alpha): target properties override #27603

Merged
merged 9 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
16 changes: 7 additions & 9 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -485,16 +485,14 @@ CDK integration tests.

### Step 4: Pull Request

* Create a commit with your changes and push them to a
[fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo).
* Create a [fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo) of the CDK repository.
* Create a new branch for your change, and push the change commits on it.
> [!IMPORTANT]
> We will not be able to accept your contribution if you do not allow commits to your PR branch, as it introduces
> friction in our review process and breaks our automation that syncs with the main branch. In these scenarios, we will close
> your pull request and ask that you recreate it with the necessary permissions.
> This means that you must contribute from a fork within your personal account (as opposed to an organization owned account) and also develop
> your contribution on a branch other than `main`. See
> [Allowing changes to a pull request branch created from a fork](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork)
> for more information.
> Your pull request must be based off of a branch in a personal account (not an organization owned account, and not the `main` branch).
> You must also have the setting enabled that [allows the CDK team to push changes to your branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork) (this setting is enabled by default for personal accounts,
> and cannot be enabled for organization owned accounts).
> The reason for this is that our automation needs to synchronize your branch with our `main` after it has been approved, and
> we cannot do that if we cannot push to your branch.

> [!NOTE]
> CDK core members can push to a branch on the AWS CDK repo (naming convention: `<user>/<feature-bug-name>`).
Expand Down
47 changes: 36 additions & 11 deletions INTEGRATION_TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ All Construct libraries in the CDK code base have integration tests that serve t
This is done by running `yarn integ` which will run `cdk deploy` across all of the integration tests in that package.
If you are developing a new integration test or for some other reason want to work on a single integration test
over and over again without running through all the integration tests you can do so using
`yarn integ integ.test-name.js` .Remember to set up AWS credentials before doing this.
`yarn integ integ.test-name.js`. Remember to set up AWS credentials before doing this.
3. (Optionally) Acts as a way to validate that constructs set up the CloudFormation resources as expected.
A successful CloudFormation deployment does not mean that the resources are set up correctly.

Expand Down Expand Up @@ -75,8 +75,7 @@ you have good test coverage.

### Creating a Test

An integration tests is any file located in the `test/` directory that has a name that starts with `integ.`
(e.g. `integ.*.ts`).
Integration tests for stable modules live in `@aws-cdk-testing/framework-integ/test/MODULE_NAME/test/`. Alpha module integ tests still live in their `test/` directories. Names of integration tests start with integ (e.g. `integ.*.ts`).

To create a new integration test, first create a new file, for example `integ.my-new-construct.ts`.
The contents of this file should be a CDK app. For example, a very simple integration test for a
Expand All @@ -86,7 +85,7 @@ _integ.lambda.ts_
```ts
import * as iam from 'aws-cdk-lib/aws-iam';
import * as cdk from 'aws-cdk-lib/core';
import * as lambda from '../lib';
import * as lambda from 'aws-cdk-lib/aws-lambda';
import * as integ from '@aws-cdk/integ-tests-alpha';

const app = new cdk.App();
Expand All @@ -102,8 +101,6 @@ const fn = new lambda.Function(stack, 'MyLambda', {
new integ.IntegTest(app, 'LambdaTest', {
testCases: [stack],
});

app.synth();
```

To run the test you would run:
Expand Down Expand Up @@ -243,11 +240,13 @@ want to validate that the _integration_ connecting `StepFunctions` to the `Event
way to do that is to actually trigger the `StateMachine` and validate that it was successful.

```ts
import * as integ from '@aws-cdk/integ-tests-alpha';

declare const app: App;
declare const sm: sfn.StateMachine;
declare const stack: Stack;

const testCase = new IntegTest(app, 'PutEvents', {
const testCase = new integ.IntegTest(app, 'PutEvents', {
testCases: [stack],
});

Expand All @@ -262,21 +261,47 @@ const describe = testCase.assertions.awsApiCall('StepFunctions', 'describeExecut
});

// assert the results
describe.expect(ExpectedResult.objectLike({
describe.expect(integ.ExpectedResult.objectLike({
status: 'SUCCEEDED',
}));
```

Not every test requires an assertion. We typically do not need to assert CloudFormation behavior. For example, if we create an S3 Bucket
If we want to pick out certain values from the api call response, we can use the `assertAtPath()` method, as in the [integ.pipeline-with-additional-inputs.ts](https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk-testing/framework-integ/test/pipelines/test/integ.pipeline-with-additional-inputs.ts) integ test. Note that using the `outputPaths` optional parameter on the `awsApiCall()` function often interacts poorly with the `expect()` function.

```ts
import * as integ from '@aws-cdk/integ-tests-alpha';

declare const app: App;
declare const stack: Stack;
declare const pipelineName: string;
declare const expectedString: string;

const testCase = new integ.IntegTest(app, 'PipelineAdditionalInputsTest', {
testCases: [stack],
});

const source = testCase.assertions.awsApiCall('CodePipeline', 'GetPipeline', {
name: pipelineName,
});

// assert the value at the given path matches the expected string
// the numbers index arryas in the json response object
source.assertAtPath('pipeline.stages.0.actions.0.name', integ.ExpectedResult.stringLikeRegexp(expectedString));
```
A helpful trick is to deploy the integ test with `--no-clean` and then make the api call locally. We can then trace the path to specific values easily. For example, `> aws codepipeline get-pipeline --name MyFirstPipeline`.

Adding assertions is prefered on all new integ tests; however, it is not strictly required. We typically do not need to assert CloudFormation behavior. For example, if we create an S3 Bucket
with Encryption, we do not need to assert that Encryption is set on the bucket. We can trust that the CloudFormation behavior works.
Some things you should look for in deciding if the test needs an assertion:

- Integrations between services (i.e. integration libraries like `aws-cdk-lib/aws-lambda-destinations`, `aws-cdk-lib/aws-stepfunctions-tasks`, etc)
- Anything that bundles or deploys custom code (i.e. does a Lambda function bundled with `aws-cdk-lib/aws-lambda-nodejs` still invoke or did we break bundling behavior)
- Integrations between services (i.e. integration libraries like `aws-cdk-lib/aws-lambda-destinations`, `aws-cdk-lib/aws-stepfunctions-tasks`, etc).
- All custom resources. Must assert the expected behavior of the lambda is correct.
- Anything that bundles or deploys custom code (i.e. does a Lambda function bundled with `aws-cdk-lib/aws-lambda-nodejs` still invoke or did we break bundling behavior).
- IAM/Networking connections.
- This one is a bit of a judgement call. Most things do not need assertions, but sometimes we handle complicated configurations involving IAM permissions or
Networking access.


## Running Integration Tests

Most of the time you will only need to run integration tests for an individual module (i.e. `aws-lambda`). Other times you may need to run tests across multiple modules.
Expand Down
26 changes: 24 additions & 2 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,12 @@ class LambdaHotswapStack extends cdk.Stack {
handler: 'index.handler',
description: process.env.DYNAMIC_LAMBDA_PROPERTY_VALUE ?? "description",
environment: {
SomeVariable: process.env.DYNAMIC_LAMBDA_PROPERTY_VALUE ?? "environment",
}
SomeVariable:
process.env.DYNAMIC_LAMBDA_PROPERTY_VALUE ?? "environment",
ImportValueVariable: process.env.USE_IMPORT_VALUE_LAMBDA_PROPERTY
? cdk.Fn.importValue(TEST_EXPORT_OUTPUT_NAME)
: "no-import",
},
});

new cdk.CfnOutput(this, 'FunctionName', { value: fn.functionName });
Expand Down Expand Up @@ -343,6 +347,22 @@ class ConditionalResourceStack extends cdk.Stack {
}
}

const TEST_EXPORT_OUTPUT_NAME = 'test-export-output';

class ExportValueStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);

// just need any resource to exist within the stack
const topic = new sns.Topic(this, 'Topic');

new cdk.CfnOutput(this, 'ExportValueOutput', {
exportName: TEST_EXPORT_OUTPUT_NAME,
value: topic.topicArn,
});
}
}

class BundlingStage extends cdk.Stage {
constructor(parent, id, props) {
super(parent, id, props);
Expand Down Expand Up @@ -450,6 +470,8 @@ switch (stackSet) {

new ImportableStack(app, `${stackPrefix}-importable-stack`);

new ExportValueStack(app, `${stackPrefix}-export-value-stack`);

new BundlingStage(app, `${stackPrefix}-bundling-stage`);
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,48 @@ integTest('hotswap deployment supports Lambda function\'s description and enviro
expect(deployOutput).toContain(`Lambda Function '${functionName}' hotswapped!`);
}));

integTest('hotswap deployment supports Fn::ImportValue intrinsic', withDefaultFixture(async (fixture) => {
// GIVEN
try {
await fixture.cdkDeploy('export-value-stack');
const stackArn = await fixture.cdkDeploy('lambda-hotswap', {
captureStderr: false,
modEnv: {
DYNAMIC_LAMBDA_PROPERTY_VALUE: 'original value',
USE_IMPORT_VALUE_LAMBDA_PROPERTY: 'true',
},
});

// WHEN
const deployOutput = await fixture.cdkDeploy('lambda-hotswap', {
options: ['--hotswap'],
captureStderr: true,
onlyStderr: true,
modEnv: {
DYNAMIC_LAMBDA_PROPERTY_VALUE: 'new value',
USE_IMPORT_VALUE_LAMBDA_PROPERTY: 'true',
},
});

const response = await fixture.aws.cloudFormation('describeStacks', {
StackName: stackArn,
});
const functionName = response.Stacks?.[0].Outputs?.[0].OutputValue;

// THEN

// The deployment should not trigger a full deployment, thus the stack's status must remains
// "CREATE_COMPLETE"
expect(response.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE');
expect(deployOutput).toContain(`Lambda Function '${functionName}' hotswapped!`);

} finally {
// Ensure cleanup in reverse order due to use of import/export
await fixture.cdkDestroy('lambda-hotswap');
await fixture.cdkDestroy('export-value-stack');
}
}));

async function listChildren(parent: string, pred: (x: string) => Promise<boolean>) {
const ret = new Array<string>();
for (const child of await fs.readdir(parent, { encoding: 'utf-8' })) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,88 @@
"Principal": "*",
"PrincipalOrgID": "o-xxxxxxxxxx"
}
},
"MyLambdaFunctionUrlC2055677": {
"Type": "AWS::Lambda::Url",
"Properties": {
"AuthType": "AWS_IAM",
"TargetFunctionArn": {
"Fn::GetAtt": [
"MyLambdaCCE802FB",
"Arn"
]
}
}
},
"MyLambdaInvokeSz2P2C4jOiX4AmIs1ANCq2qfq8PhgVeKtRAVyAkFmM7C8BE4B5": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
"Fn::GetAtt": [
"MyLambdaCCE802FB",
"Arn"
]
},
"Principal": "*",
"PrincipalOrgID": "o-mmmmmmmmmm"
}
},
"MyLambdaInvokeFcyXBRX02EWa52GlFECQiCzDt0fdRUDi4mo4foC5aU41318F58": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
"FunctionName": {
"Fn::GetAtt": [
"MyLambdaCCE802FB",
"Arn"
]
},
"Principal": "apigateway.amazonaws.com"
}
},
"MyRoleF48FFE04": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "lambda.amazonaws.com"
}
}
],
"Version": "2012-10-17"
}
}
},
"MyRoleDefaultPolicyA36BE1DD": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "lambda:InvokeFunctionUrl",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"MyLambdaCCE802FB",
"Arn"
]
}
}
],
"Version": "2012-10-17"
},
"PolicyName": "MyRoleDefaultPolicyA36BE1DD",
"Roles": [
{
"Ref": "MyRoleF48FFE04"
}
]
}
}
},
"Parameters": {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading