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

chore(release): 2.137.0 #29788

Merged
merged 20 commits into from
Apr 10, 2024
Merged

chore(release): 2.137.0 #29788

merged 20 commits into from
Apr 10, 2024

Conversation

aws-cdk-automation
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation commented Apr 10, 2024

See CHANGELOG

msambol and others added 19 commits April 6, 2024 03:36
#29542)

Closes #29532.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Issue # (if applicable)

Closes #<issue number here>.

### Reason for this change



### Description of changes



### Description of how you validated changes



### Checklist
- [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…kerImageAsset ECR repository (#29766)

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

### Issue # (if applicable)

#13327 (Only a docs update, does not fix the underlying issue)

### Reason for this change

In cdkv2, it is no longer possible to add permissions on the repository for `dockerImageAsset`, but the docs were not updated.

### Description of changes

Updates documentation for aws-ecr-assets. Mentions that it is no longer possible to grant repository permissions on `dockerImageAsset` for cross-account access and provides possible alternate routes.

### Description of how you validated changes

Rebuild the affected doc.



No, docs change only

### Checklist
- [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…9419)

### Issue # (if applicable)

Closes #29204.

### Reason for this change

CloudFront's `KeyValueStore` has the option to initialize the store using a file, imported with an `ImportSource`. The CloudFormation resource does not support inline declarations, so the current L2 construct does not support inline either. To provide easier 

### Description of changes

Added the `InlineImportSource`, along with the companion static method to `cloudfront.KeyValueStore`.

#### 📄 Temporary Files

Since inline declarations are not supported by the underlying L1 construct, I have resorted to creating a temporary file which will be handled as a CDK asset. This is done utilizing elements of the cdk core, and follows the pattern used by the `s3-deployment`. This does also couple the KeyValueStore to the assets in S3, though the only other option would seem to be creating its own S3 bucket.

This is the only case I'm aware of in CDK of handling a temporary file like this. As far as I'm aware, all design principles are met, but this seems something of an edge case.

Open to suggestions and reviews.

### Description of how you validated changes

New Unit Tests and Integration Test implemented

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…29586)

### Issue # (if applicable)

Closes #29585.

### Reason for this change

ALB supports some attributes that is not configurable from CDK
- `routing.http.preserve_host_header.enabled`
- `routing.http.x_amzn_tls_version_and_cipher_suite.enabled`
- `routing.http.xff_client_port.enabled`
- `routing.http.xff_header_processing.mode`
- `waf.fail_open.enabled`

### Description of changes

Added some props to `ApplicationLoadBalancerProps`.
- `preserveHostHeader`
- `xAmznTlsVersionAndCipherSuiteHeaders`
- `preserveXffClientPort`
- `xffHeaderProcessingMode`
- `wafFailOpen`

### Description of how you validated changes

Added both unit and integ tests.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Issue # (if applicable)

Closes #29720

Might fix other issues, will need check the backlog

### Reason for this change

The user data script used to initialize NAT instances was at present incorrect, assuming an incorrect default interface name. I wanted to both update this script, and allow CDK users to update this user data if needed in the future, both in case of an issue with the script or just to extend/override the initialization procedure.

### Description of changes

Apologies for the non-atomic PR, as I was working on the unit and integration tests, I noticed additional issues and missing features. These are the initially targeted changes:

* Add `userData` field to `NatInstanceProps`, allowing the user to overwrite the NAT instances initialization script
  * Add static `NatInstanceProps.DEFAULT_USER_DATA_COMMANDS` field, allowing the user to more easily extend the default script
* Fix default `userData` to query the instance `default` network interface, instead of always using `eth0`

These were found along the way:

* Add getter `gatewayInstances` method to `NatInstanceProps`, allowing the user to interact with the created NAT instances once they're attached to the VPC
  * This allowed me to give the instance role additional permissions, but has more potential uses
* Fix `creditSpecification` not being implemented correctly
* Updated existing unit and integ tests to increase coverage
* Add warning in `README` about potential issues with NAT instances as is implemented

### Description of how you validated changes

I've updated and added unit and integration tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Issue # (if applicable)

Closes #29700

I've also opened #29701 to catch similar issues at synth time in the future

### Reason for this change

When running `httpApiCall('url').expect`, the `AssertionResults` output logical ID would be overridden with an invalid name, containing the URL slashes.

This issue was not noticed earlier because, as far as I can tell, assertions were only made with `Token`/`Ref` URLs, e.g. `apigw.DomainName`

### Description of changes

* Remove non-alphanumeric characters from the overridden `AssertionResults` output logical ID
* I've also added a bit of documentation to `ExpectedResult`, I noticed it was slightly lacking while creating the integration test

### Description of how you validated changes

I've added unit and integration tests. The integration tests include both tests with API Gateway, to cover unresolved URLs, and to https://httpbin.org/ to test this fix

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Issue # (if applicable)

Closes #29044.

### Reason for this change

When associating between the source GraphQL API and the merged API using `fromSourceApis`, generated association resource doesn't depend on the source GraphQL schema.

```ts
// This API has `CfnGraphQLSchema` by `definition` prop
const firstApi = new appsync.GraphqlApi(stack, 'FirstSourceAPI', {
  name: 'FirstSourceAPI',
  definition: appsync.Definition.fromFile(path.join(__dirname, 'appsync.merged-api-1.graphql')),
});

// This merged API generates `CfnSourceApiAssociation`
new appsync.GraphqlApi(stack, 'MergedAPI', {
  name: 'MergedAPI',
  definition: appsync.Definition.fromSourceApis({
    sourceApis: [
      {
        sourceApi: firstApi,
        mergeType: appsync.MergeType.MANUAL_MERGE,
      },
    ],
  }),
});
```

The same is true if the `SourceApiAssociation` construct is used explicitly.

```ts
// This API has `CfnGraphQLSchema` by `definition` prop
const firstApi = new appsync.GraphqlApi(stack, 'FirstSourceAPI', {
  name: 'FirstSourceAPI',
  definition: appsync.Definition.fromFile(path.join(__dirname, 'appsync.merged-api-1.graphql')),
});

// This merged API does not generate `CfnSourceApiAssociation`
const mergedApi = new appsync.GraphqlApi(stack, 'MergedAPI', {
  name: 'MergedAPI',
  definition: appsync.Definition.fromSourceApis({
    sourceApis: [],
    mergedApiExecutionRole: mergedApiExecutionRole,
  }),
});

// This construct has `CfnSourceApiAssociation`
new appsync.SourceApiAssociation(stack, 'SourceApiAssociation1', {
  sourceApi: firstApi,
  mergedApi: mergedApi,
  mergeType: appsync.MergeType.MANUAL_MERGE,
  mergedApiExecutionRole: mergedApiExecutionRole,
});
```

### Description of changes

The `sourceApi` passed by the `fromSourceApis` method or the `SourceApiAssociation` construct has `addSchemaDependency` method. Using this method, we can make the association to depend on the schema in the `sourceApi`.
But, if the api is an IMPORTED resource to begin with, it has no schema in the CDK layer, so there is nothing we can do about it. (The method does nothing.)

### Description of how you validated changes

Both unit and existing integ tests.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Issue # (if applicable)

### Reason for this change

The link to the `cloud-assembly-schema` in the [design/cloud-assembly.md](https://github.com/aws/aws-cdk/blob/main/design/cloud-assembly.md) is broken.

![page-1](https://github.com/aws/aws-cdk/assets/24818752/20e66831-bf5c-47c4-90d1-485c3ba0da39)

![page-2](https://github.com/aws/aws-cdk/assets/24818752/0ccee86e-58ff-49b2-9997-cc55ab022605)

### Description of changes

Changed the link to correct link.

### Description of how you validated changes

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds a `Tag` class to the assertions library that permits assertions against tags on synthesized CDK stacks.

Tags on AWS resources can be checked via assertions with the Template class, but since stack tags only appear on the cloud assembly manifest as a sibling of the template, a separate assertion mechanism is required.

> [!NOTE]
> Previously submitted as #27633, which was closed automatically while waiting for review.
>
> This PR is complete to the best of my knowledge, needing only an exemption from integration tests. As far as I can tell, this area of the library has no integration tests directly associated.

### API

```ts
class Tags {
  public static fromStack(stack: Stack) : Tags;
  public hasValues(props: any): void;
  public all(): { [key: string]: string };
}
```

### Usage

This new class permits tests of the form:

```ts
import { App, Stack } from 'aws-cdk-lib';
import { Tags } from 'aws-cdk-lib/assertions';

const app = new App();
const stack = new Stack(app, 'MyStack', {
  tags: {
    'tag-name': 'tag-value'
  }
});

const tags = Tags.fromStack(stack);

// using a default 'objectLike' Matcher
tags.hasValues({
  'tag-name': 'tag-value'
});

// or another Matcher
tags.hasValues({
  'tag-name': Match.anyValue()
});
```

You can also get the set of tags to test them in other ways:

```ts
tags.all()
```

## Issues

### No tags case

One might expect that the case where no tags are present would match `undefined` or `null`, but since the Cloud Assembly API defaults tags to `{}` when none are present, this isn't possible. It's also not practical to directly test the `artifact.manifest.properties.tags` value directly, as there is a legacy case that the API handles. This means that the `artifact.tags` property is the best value to check against.

The tests for this PR show that matching with `Match.absent()` will fail when there are no tags, but testing against the empty object will succeed.

I think that this behaviour (defaulting to empty) will be OK, but potentially require a callout on the assertion method.

### API method naming

The current suggested API went through some evolution, starting with:

```ts
class Tags {
  public static fromStack(stack: Stack) : Tags;
  public hasTags(props: any): void;
  public getTags():  { [key: string]: string };
}
```
But this stuttered, and `getTags()` wasn't compatible with Java.

I considered:

```ts
class Tags {
  public static fromStack(stack: Stack) : Tags;
  public hasValues(props: any): void;
  public values():  { [key: string]: string };
}
```
and
```ts
class Tags {
  public static fromStack(stack: Stack) : Tags;
  public has(props: any): void;
  public all():  { [key: string]: string };
}
```

... before settling on a mix of the two. I think the current iteration fits with the rest of the `assertions` API and makes sense by itself, but very open to changes.

Closes #27620.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
### Reason for this change

Docs currently list an example for an invalid route53 action name

### Description of changes

Fix action name in docs

### Description of how you validated changes

N/A

### Checklist
- [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…StagingBucket` is encrypted with customer managed KMS key (#29540)

### Issue #25100

Closes #25100.

### Reason for this change

When the CDK bootstrap stack's `StagingBucket` is encrypted with a customer managed KMS key whose key policy does not include wildcard KMS permissions similar to those of the S3 managed KMS key, `BucketDeployment` fails because the Lambda's execution role doesn't get the `kms:Decrypt` permission necessary to download from the bucket.

In addition, if one of the sources for `BucketDeployment` comes from the `Source.bucket` static method, and the bucket is an "out-of-app imported reference" created from `s3.Bucket.fromBucketName`, the bucket's `encryptionKey` attribute is `null` and the current code won't add the `kms:Decrypt` permission on the bucket's encryption key to the Lambda's execution role. If this bucket is additionally encrypted with a customer managed KMS key without sufficient resource-based policy, the deployment fails as well.

### Description of changes

It's not easy to make the code "just work" in every situation, because there's no way to pinpoint the source bucket's encryption key ARN without using another custom resource, which is a heavy-lifting and it's hard to give this new Lambda a reasonable and minimal set of execution role permissions.

Therefore, this PR resolves the issue by changing `BucketDeployment.handlerRole` from `private readonly` to `public readonly`, and adding documentations on how to handle errors resulting from "not authorized to perform: kms:Decrypt". The current code allows customizing `handlerRole` by passing in `BucketDeploymentProps.role`. This change makes the customization easier because users don't need to manually add the S3 permissions.

The only code change is on the visibility of `BucketDeployment.handlerRole`. All other changes are documentations.

I proposed 4 possible changes in my comment to Issue #25100, and only the first one (changing visibility) is pursued in this PR. The second one was abandoned because the CFN export `CdkBootstrap-hnb659fds-FileAssetKeyArn` is deprecated.

### Description of how you validated changes

I wrote a CDK app which uses the `BucketDeployment` construct. After manually adding relevant KMS permissions to the Lambda execution role, I verified that the bucket deployment worked in the following two scenarios:

- My personal account; bootstrap stack's `StagingBucket` encrypted with a custom KMS key which only has the default key policy.
- GoDaddy corporate account; `StagingBucket` encrypted with a KMS key from an AWS Organization management account.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
… of branch. (#29787)

### Reason for this change

It is better to use an immutable commit reference for workflows that require approval, since the branch's head commit can change between the time of approval and workflow run execution.

### Description of changes

Updated the `ref` parameter to use the SHA from the pull_request event instead of the ref.

### Description of how you validated changes



### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@aws-cdk-automation aws-cdk-automation added auto-approve pr/no-squash This PR should be merged instead of squash-merging it labels Apr 10, 2024
@github-actions github-actions bot added the p2 label Apr 10, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team April 10, 2024 20:20
@GavinZZ GavinZZ added the pr/do-not-merge This PR should not be merged at this time. label Apr 10, 2024
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Apr 10, 2024
CHANGELOG.v2.md Outdated Show resolved Hide resolved
@GavinZZ GavinZZ removed the pr/do-not-merge This PR should not be merged at this time. label Apr 10, 2024
@aws-cdk-automation
Copy link
Collaborator Author

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 2c058ad
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

mergify bot commented Apr 10, 2024

Thank you for contributing! Your pull request will be automatically updated and merged without squashing (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit bb90b4c into v2-release Apr 10, 2024
14 of 15 checks passed
@mergify mergify bot deleted the bump/2.137.0 branch April 10, 2024 21:32
@aws-cdk-automation
Copy link
Collaborator Author

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-approve p2 pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. pr/no-squash This PR should be merged instead of squash-merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.