fix(core): updates logical id validation regex #26174
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
github-actions
bot
added
bug
github-actions
bot
added
the
beginning-contributor
aws-cdk-automation
previously requested changes
Jun 29, 2023
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
…aws#26030) From aws#24963, unnecessary js and d.ts files which had been built for integration test were pushed. This PR removes these files. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…s and writers (aws#26006) The [`_createInstances`](https://github.com/aws/aws-cdk/blob/4c9016a264c2fec9c0e0e3fae1d7c4216c964b31/packages/aws-cdk-lib/aws-rds/lib/cluster.ts#L635) function was not providing a default `monitoringRole` value with enabled monitoring. This fix creates a default role as done by the [legacy code](https://github.com/aws/aws-cdk/blob/4c9016a264c2fec9c0e0e3fae1d7c4216c964b31/packages/aws-cdk-lib/aws-rds/lib/cluster.ts#L1228). Closes aws#25941. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…lved token (aws#25749) Previously, when we import a SSM parameter by `ssm.StringParameter.fromStringParameterAttributes`, we use `CfnParameter` to get the value. ```json "Parameters": { "importsqsstringparamParameter": { "Type": "AWS::SSM::Parameter::Value<String>", "Default": { "Fn::ImportValue": "some-exported-value-holding-the-param-name" } }, ``` However, `Parameters.<Name>.Default` only allows a concrete string value. If it contains e.g. intrinsic functions, we get an error like this from CFn: `Template format error: Every Default member must be a string.` This PR changes the behavior of `fromStringParameterAttributes` method. Now it uses `CfnDynamicReference` instead of `CfnParameter` if a parameter name contains unresolved tokens. Since previously the case when `Token.isUnresolved(attrs.parameterName) == true` just resulted in a deployment error, this is not a breaking change. Closes aws#17094 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…6153) ### Describe the feature Add Lambda Layer Insights new version to CLOUDWATCH_LAMBDA_INSIGHTS_ARNS (packages/aws-cdk-lib/region-info/build-tools/fact-tables.ts) Also fixes missing ARNs for version 1.0.178.0 and others ### Use Case Take advantage of the bug fixes in - https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Lambda-Insights-extension-versionsx86-64.html - https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/Lambda-Insights-extension-versionsARM.html ### Proposed Solution Add missing ARNs and adds latest Lambda Layer Version (1.0.229.0) Closes aws#26150. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Add new IoT topic rule action to send IoT messages to Step Functions State Machines. Closes aws#17698. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Updates alpha modules with custom resources to use NodeJS 18 and v3 of the AWS SDK. Since these CRs are pretty self-contained and safe to update, their handler code and runtime versions can be updated without any breakage for users. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds the new `spec2cdk` tool. This tool is designed to replace the existing `cfn2ts`, but uses `@aws-cdk/aws-service-spec` as a datatsource. In a first step we make the tool and available, without using it for the build. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…into fix-logical-id
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
This PR adds test cases for S3 buckets using kms keys. Some existing tests were also modified. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
The generated examples for QuickSight are too large. Bump the `cdk-generate-synthetic-examples` package to merge a fix. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR adds the documentation in the readme about `internetFacing` on classic load balancer . > [CONTRIBUTING GUIDE]: https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md > [DESIGN GUIDELINES]: https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md Closes #<issue number here>. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ns (aws#26204) This is in response to an internal ticket where someone indicated that their 2 datapoints cross-account Alarm was not working. AWS Support responded with the above reasoning. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Caused by a check for `constructor.name` which `esbuild` may rename. Replace with a checkable symbol. Closes aws#26169. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR adds ExecutionClass for Glue job's L2 construct. This allows you to specify `FLEX` option. Closes aws#22224 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
The first iteration of [@aws-cdk/cli-lib-alpha](https://docs.aws.amazon.com/cdk/api/v2/docs/cli-lib-alpha-readme.html) doesn't support the bootstrap command that is mandatory to deploy a new app via CDK. This PR introduces the bootstrap command for the CLI. Related: aws#15851 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ambda Construct from a different Construct Library (aws#26206) Fix the "is construct" check to ensure one can add a LambdaSubscription to an sns topic using the "addSubscription" method regardless of whether it came from a symlinked construct library or not. Closes aws#24788. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…llowance (aws#26173) And that maximum allowance right now is 20 (it's actually 21 but we are rounding down for safety). ```bash repos: 2, size: 24820 bytes repos: 3, size: 26189 bytes repos: 4, size: 27558 bytes repos: 5, size: 28927 bytes repos: 6, size: 30296 bytes repos: 7, size: 31665 bytes repos: 8, size: 33034 bytes repos: 9, size: 34403 bytes repos: 10, size: 35772 bytes repos: 11, size: 37148 bytes repos: 12, size: 38524 bytes repos: 13, size: 39900 bytes repos: 14, size: 41276 bytes repos: 15, size: 42652 bytes repos: 16, size: 44028 bytes repos: 17, size: 45404 bytes repos: 18, size: 46780 bytes repos: 19, size: 48156 bytes repos: 20, size: 49532 bytes repos: 21, size: 50908 bytes ``` Consistently, each additional repo adds 1369 bytes to the template. Before adding the inline functions, each additional repo was roughly 1200 bytes. Current rough breakdown of size in the integ test (1 s3 bucket and 2 ecr repos): 25KB ~ 2KB for node version mapping ~ 6KB for auto delete images (4KB is the inline function) ~ 6KB for auto delete objects (4KB is the inline function) Based on these calculations, it is semi reasonable that the repo allowance has gone from 38 to 21. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ux2-x86_64-standard:5.0` (aws#26058) Added support for Amazon Linux 2023 CodeBuild Image: https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-available.html `aws/codebuild/amazonlinux2-x86_64-standard:5.0` i.e you can now reference the latest codebuild image via the `codebuild.LinuxBuildImage.AMAZON_LINUX_2_5` enum rather than having to resolve it manually like `LinuxBuildImage.codeBuildImage('aws/codebuild/amazonlinux2-x86_64-standard:5.0')` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Closes aws#26224 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Update JSDOC for `user` to clarify that this is a `user string` and not restricted to a user name. Reason for this change: When viewing the documentation from an IDE it is unclear that something other than a user name can be used for example 100:101 ie UID and GID. I have not fully read the following: [CONTRIBUTING GUIDE]: https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md [DESIGN GUIDELINES]: https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md This commit/pull request only changes JSDoc. Closes aws#26230 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR enables the new Deployment Alarms feature in ECS for Service L2s [blog post](https://aws.amazon.com/blogs/containers/automate-rollbacks-for-amazon-ecs-rolling-deployments-with-cloudwatch-alarms/). This PR contains changes after the first round of revision by the CDK team. Continuation of aws#25346 **Why are so many integration tests impacted by this change? There are 2 reasons:** 1. This PR changes the ECS L2s to set the default configuration for the `CfnService.deploymentConfiguration.alarms` property to: ``` alarmNames: [], rollback: false, enable: false, ``` This is necessary, because adding deployment alarms, deploying your CFN stack, then removing the deployment alarms from the CFN template, and deploying again WILL NOT remove the deployment alarms from the service. To remove previously configured deployment alarms, you must explicitly use the configuration shown above. Making this update will cause no interruption to existing ECS services, and does not trigger any update to the service itself during the CFN update. The ECS UpdateService API is stateful, meaning that if a field is not present in the CloudFormation object, it will be ignored in the update. This was originally implemented due to the problems with desiredCount resetting to bad values after autoscaling changed it, but requires us to set an explicit disableDeploymentAlarms() method to cause the service update to behave correctly. 2. Most ECS integ tests have not been executed in a long time. So, many of the changes are bring the snapshots up to date with the latest integ test format, or lambda layers, etc. **Detailed list of changes:** - entire alarm based rollback task into single commit - resolving merge conflicts - integ files modules updated to resolve merge conflicts - regarding merge conflicts issue - integ files import changes for merge conflicts - adding this change for PR not to abondended - chore: refactor createAlarm & remove unnecessary unit tests - chore: fix api but failing unit tests - chore: fix validation - chore: readme changes - chore: fix integ test, remove unnecessary ec2 test - chore: refactor enableAlarms for clarity - test: fix integ test, finally - docs: update docstrings for new props and methods ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
`0.0.0` is not a valid version according to the scripts in our repo. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…into fix-logical-id
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
aws-cdk-automation
added
the
pr/needs-community-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The CloudFormation documentation indicates that logical ID's must be only alphanumeric, yet the regex in the logical ID validation file is /^[A-Za-z][A-Za-z0-9]{1,254}$/. This causes logical IDs such as 030ffab42b5f448fb39750B452 to fail even though they are valid according to the CloudFormation standard.
Expected Behavior
Logical IDs such as 030ffab42b5f448fb39750B452 to be considered valid to the CDK when synthesizing a stack
Current Behavior
Logical IDs such as 030ffab42b5f448fb39750B452 are not considered valid when synthesizing a stack.
Error: Resolution error: Logical ID must adhere to the regular expression: /^[A-Za-z][A-Za-z0-9]{1,254}$/, got '030ffab42b5f448fb39750B452'.
Fix
Removes the constraint from the logical ID regex that denotes that the first character of a logical ID must be a letter.
Closes #26075.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license