fix(servicecatalogappregistry): RAM Share is replaced on every change to Application #24760

liwewang-amazon · 2023-03-23T06:09:33Z

Application.shareApplication can be called multiple times which generates different resources within the same Application construct. The share construct id generation was based on child length of Application construct. This makes every change to the Application construct can unnecessarily replace the RAM share when the share actually needs no update.

Similar to the solution for addAttributeGroup introduced in the previous update, this commit starts to require both construct id and share name in the Application.shareApplication method input, which are used to create RAM share construct.

For ApplicationAssociator, Application.shareApplication is called for cross-account stack associations. Share construct id depends on the node unique id of the application in ApplicationAssociator and the node unique id of the stack to be associated. This reduces unnecessary share replacement in ApplicationAssociator stack.

BREAKING CHANGE: This commit involves share replacement during the deployment of ApplicationAssociator due to share construct id update. After this change, frequent share replacements due to structural change in Application construct should be avoided. Application.shareApplication starts to require construct id (first argument) and share name (added in ShareOption) as input.

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

<body> Testing done ------------------------------------- * Related items ------------------------------------ * SIM/auto-cut ticket

aws-cdk-automation

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.

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

rix0rrr

Please make sure that your PR title confirms to the conventional commit standard (fix, feat, chore) and that it is written in a style that will reflect correctly in the change log. In particular, if this is a bug fix, please describe the bug that was fixed, not the change you made (See Contributing Guide, Pull Requests)

mergify · 2023-03-23T16:58:13Z

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).

aws-cdk-automation · 2023-03-23T17:41:31Z

AWS CodeBuild CI Report

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

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

mergify · 2023-03-23T17:41:52Z

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).

… to Application (aws#24760) `Application.shareApplication` can be called multiple times which generates different resources within the same `Application` construct. The share construct id generation was based on child length of `Application` construct. This makes every change to the `Application` construct can unnecessarily replace the RAM share when the share actually needs no update. Similar to the solution for `addAttributeGroup` introduced in the [previous update](aws#24672), this commit starts to require both construct id and share name in the `Application.shareApplication` method input, which are used to create RAM share construct. For `ApplicationAssociator`, `Application.shareApplication` is called for cross-account stack associations. Share construct id depends on the node unique id of the application in `ApplicationAssociator` and the node unique id of the stack to be associated. This reduces unnecessary share replacement in `ApplicationAssociator` stack. BREAKING CHANGE: This commit involves share replacement during the deployment of `ApplicationAssociator` due to share construct id update. After this change, frequent share replacements due to structural change in `Application` construct should be avoided. `Application.shareApplication` starts to require construct id (first argument) and share name (added in `ShareOption`) as input. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

liwewang-amazon added 4 commits March 22, 2023 22:44

Change Share ID generation

333ebc8

<body> Testing done ------------------------------------- * Related items ------------------------------------ * SIM/auto-cut ticket

Minor fix on the doc and share id change on the test.

a3935ad

<body> Testing done ------------------------------------- * Related items ------------------------------------ * SIM/auto-cut ticket

Fix README.md

20893a2

<body> Testing done ------------------------------------- * Related items ------------------------------------ * SIM/auto-cut ticket

Change id to make sure it is the same for each synth.

e1b0772

<body> Testing done ------------------------------------- * Related items ------------------------------------ * SIM/auto-cut ticket

github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Mar 23, 2023

aws-cdk-automation requested a review from a team March 23, 2023 06:09

liwewang-amazon changed the title ~~fix(servicecatalog): change application share construct id and name generation.~~ fix(servicecatalogappregistry): change application share construct id and name generation. Mar 23, 2023

aws-cdk-automation previously requested changes Mar 23, 2023

View reviewed changes

liwewang-amazon marked this pull request as ready for review March 23, 2023 06:54

rix0rrr requested changes Mar 23, 2023

View reviewed changes

liwewang-amazon changed the title ~~fix(servicecatalogappregistry): change application share construct id and name generation.~~ fix(servicecatalogappregistry): Application RAM Share is unnecesarry replaced after Application construct update. Mar 23, 2023

rix0rrr approved these changes Mar 23, 2023

View reviewed changes

rix0rrr changed the title ~~fix(servicecatalogappregistry): Application RAM Share is unnecesarry replaced after Application construct update.~~ fix(servicecatalogappregistry): RAM Share replaced on every change to Application Mar 23, 2023

rix0rrr changed the title ~~fix(servicecatalogappregistry): RAM Share replaced on every change to Application~~ fix(servicecatalogappregistry): RAM Share is replaced on every change to Application Mar 23, 2023

Merge branch 'main' into share_name

24eeb98

mergify bot merged commit 8977d0d into aws:main Mar 23, 2023

liwewang-amazon deleted the share_name branch March 27, 2023 23:22

filletofish mentioned this pull request Jul 2, 2023

feat(scheduler): ScheduleGroup #26196

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(servicecatalogappregistry): RAM Share is replaced on every change to Application #24760

fix(servicecatalogappregistry): RAM Share is replaced on every change to Application #24760

liwewang-amazon commented Mar 23, 2023 •

edited

Loading

aws-cdk-automation left a comment

rix0rrr left a comment •

edited

Loading

mergify bot commented Mar 23, 2023

aws-cdk-automation commented Mar 23, 2023

mergify bot commented Mar 23, 2023

fix(servicecatalogappregistry): RAM Share is replaced on every change to Application #24760

fix(servicecatalogappregistry): RAM Share is replaced on every change to Application #24760

Conversation

liwewang-amazon commented Mar 23, 2023 • edited Loading

aws-cdk-automation left a comment

Choose a reason for hiding this comment

rix0rrr left a comment • edited Loading

Choose a reason for hiding this comment

mergify bot commented Mar 23, 2023

aws-cdk-automation commented Mar 23, 2023

AWS CodeBuild CI Report

mergify bot commented Mar 23, 2023

liwewang-amazon commented Mar 23, 2023 •

edited

Loading

rix0rrr left a comment •

edited

Loading