Skip to content

fix(core): Add stage prefix to stack name shortening process#25359

Merged
mergify[bot] merged 65 commits intoaws:mainfrom
QustodioPablo:QustodioPablo/fix-stack-name-too-long
Jun 12, 2023
Merged

fix(core): Add stage prefix to stack name shortening process#25359
mergify[bot] merged 65 commits intoaws:mainfrom
QustodioPablo:QustodioPablo/fix-stack-name-too-long

Conversation

@QustodioPablo
Copy link
Copy Markdown
Contributor

There is an issue in the stack name generation process where the prefix generated from assembly's stage name is not taken into account when shortening a stack name to meet the requirement of being equal or less than 128 characters longs.

This can lead to generating an invalid stack name greater than 128 characters: since the stack name is shortened to 128 characters, when the prefix is added, the limit is exceeded.

Current solution:

  • Adding a feature flag
    • With the feature flag on, the prefix is processed within the generateUniqueName function.
    • With the feature flag off, stack name generation is not changed

Fixes #23628

NOTE: This PR was previously opened, but it was merged before I was able to add a feature flag, which ended up adding breaking changes and the changes of the PR were rolled back. Old PR: #24443

@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Apr 28, 2023

@aws-cdk-automation aws-cdk-automation requested a review from a team April 28, 2023 15:11
@github-actions github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. p2 labels Apr 28, 2023
Copy link
Copy Markdown
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@QustodioPablo
Copy link
Copy Markdown
Contributor Author

Exemption Request: The previous PR was exempt from adding integration tests

@aws-cdk-automation aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Apr 28, 2023
@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 May 8, 2023
@QustodioPablo
Copy link
Copy Markdown
Contributor Author

Clarification Request: Can a maintainer confirm if this PR needs integration tests?

@aws-cdk-automation aws-cdk-automation added the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label May 10, 2023
@QustodioPablo
Copy link
Copy Markdown
Contributor Author

Pls review ;_;

@QustodioPablo
Copy link
Copy Markdown
Contributor Author

@TheRealAmazonKendra I reopened this PR adding a feature flag, could you add an exception for integration tests as in the previous PR?

otaviomacedo
otaviomacedo previously approved these changes Jun 9, 2023
@otaviomacedo otaviomacedo added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Jun 9, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review June 9, 2023 13:31

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@mergify mergify bot dismissed stale reviews from otaviomacedo and corymhall June 12, 2023 16:06

Pull request has been modified.

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

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

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

@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 Jun 12, 2023
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 12, 2023

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

@mergify mergify bot merged commit 79c58ed into aws:main Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. p2 pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aws-eks: Imported cluster yields invalid long stack name