fix(servicecatalog): Make possible to have assets in different products in the same portfolio #26039

ViktarKhomich · 2023-06-19T14:23:57Z

fix(servicecatalog): Make possible to have assets in different products in the same portfolio (#25189)

Currently it's not possible to have assets (ex. lambdas) in different products in the same portfolio. The reason for that is that asset bucket will be deployed
within the portfolio stack and name for its deployment is hardcoded what leads to the exception 'There is already a Construct with name' at synth step.

Closes #25189

Exemption Request

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

… in the same portfolio (aws#25189)

gitpod-io · 2023-06-19T14:24:07Z

ViktarKhomich · 2023-06-19T14:24:18Z

Hi Aws Cdk Team! Could you please run integration test for me?
yarn integ test/aws-servicecatalog/test/integ.product.js --no-clean --update-on-failed

When I am doing it on my own, I am getting this error (region seems to be set in env variables):

❌ Building assets failed: Error: Building Assets Failed: Error: Could not assume role in target account using current credentials (which are for account ....) Inaccessible host: 'sts.test-region.amazonaws.com' at port 'undefined'. This service may not be available in the 'test-region' region. . Please make sure that this role exists in the account. If it doesn't exist, (re)-bootstrap the environment with the right '--trust', using the latest version of the CDK CLI.

But I deployed stack manually and it has worked.

Thank you in advance!

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.

mrgrain · 2023-06-19T14:26:12Z

Exemption Request

corymhall · 2023-06-27T10:20:05Z

packages/aws-cdk-lib/aws-servicecatalog/lib/private/product-stack-synthesizer.ts

@@ -30,7 +30,7 @@ export class ProductStackSynthesizer extends cdk.StackSynthesizer {
        cdk.Annotations.of(parentStack).addWarning('[WARNING] Bucket Policy Permissions cannot be added to' +
          ' referenced Bucket. Please make sure your bucket has the correct permissions');
      }
-      this.bucketDeployment = new BucketDeployment(parentStack, 'AssetsBucketDeployment', {
+      this.bucketDeployment = new BucketDeployment(parentStack, `${cdk.Names.uniqueId(this.boundStack)}-AssetsBucketDeployment`, {


Since the bucket deployment can be shared within the same stack what if we did something like this:

this.bucketDeployment = parentStack.tryFindChild('AssetsBucketDeployment') ?? new BucketDeployment(...)

corymhall · 2023-06-27T10:25:15Z

Hi Aws Cdk Team! Could you please run integration test for me? yarn integ test/aws-servicecatalog/test/integ.product.js --no-clean --update-on-failed

When I am doing it on my own, I am getting this error (region seems to be set in env variables):

It looks like this test might not be able to work with the update workflow. You can run the test with --disable-update-workflow or add the stackUpdateWorkflow: false to IntegTest.

mrgrain · 2023-07-07T11:23:58Z

@ViktarKhomich Happy to run the integ tests once you've addressed the previously received feedback.

aws-cdk-automation · 2023-07-11T00:10:17Z

This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

aws-cdk-automation · 2023-07-18T00:11:00Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

aws-cdk-automation · 2023-07-19T00:12:56Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

… in the same portfolio (aws#25189) Check for the existing AssetDeploymentBucket and if it is, use it otherwise create a new one

kaizencc · 2023-08-09T14:17:49Z

Hi @ViktarKhomich, are you still available to address feedback? We're happy to run the integ tests once you've addressed the previously received feedback.

aws-cdk-automation · 2023-08-09T14:36:06Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 0431fd9
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2023-08-10T00:08:53Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

ViktarKhomich · 2023-08-10T08:16:11Z

Hi @kaizencc, yes, I am still available. I did changes regarding the comment above, but PR seems to be closed automatically again. Can we re-open it? Thank you!

kaizencc · 2023-08-11T16:13:01Z

Reopened -- please make sure that the build succeeds and that you can merge from main to get the latest changes as well @ViktarKhomich

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

aws-cdk-automation · 2023-08-12T00:07:05Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

We have a label, `pr-linter/do-not-close` that has not been doing its job. See #26039. It is because we are expecting a comma-separated-list, without spaces: https://github.com/rix0rrr/close-stale-prs/blob/ffeb148adbaf7402e77bc4eafce8ed3d3c40f29c/src/index.ts#L22 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

kaizencc · 2023-08-15T21:35:57Z

Okay I think I fixed the pr-linter/do-not-close label in a separate PR, and my hope is that this PR does not close automaticaly anymore. Sorry about the headache here

mrgrain · 2023-08-25T18:45:39Z

Closing in favor of #26885

Thanks for you effort! Even though it didn't get merged in the end, it was useful for the other PR.

Viktar Khomich and others added 5 commits May 2, 2023 23:49

fix(servicecatalog): Make possible to have assets in several products…

4e2c1de

… in the same portfolio (aws#25189)

Merge branch 'main' into main

8914f3b

Merge branch 'main' into main

11d93f6

Merge branch 'aws:main' into main

0d731eb

Merge branch 'aws:main' into main

51c6272

github-actions bot added bug This issue is a bug. effort/large Large work item – several weeks of effort p1 labels Jun 19, 2023

aws-cdk-automation requested a review from a team June 19, 2023 14:24

github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Jun 19, 2023

aws-cdk-automation previously requested changes Jun 19, 2023

View reviewed changes

aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Jun 19, 2023

ViktarKhomich changed the title ~~Exemption Request~~ fix(servicecatalog): Make possible to have assets in different products in the same portfolio (#25189) Exemption Request Jun 19, 2023

mrgrain added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 26, 2023

corymhall suggested changes Jun 27, 2023

View reviewed changes

corymhall removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 27, 2023

mrgrain changed the title ~~fix(servicecatalog): Make possible to have assets in different products in the same portfolio (#25189) Exemption Request~~ fix(servicecatalog): Make possible to have assets in different products in the same portfolio Jul 12, 2023

aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Jul 18, 2023

aws-cdk-automation closed this Jul 18, 2023

mrgrain reopened this Jul 18, 2023

mrgrain added the pr-linter/do-not-close The PR linter will not close this PR while this label is present label Jul 18, 2023

mrgrain self-assigned this Jul 18, 2023

aws-cdk-automation closed this Jul 19, 2023

github-actions bot mentioned this pull request Jul 26, 2023

Monthly PRs metrics report - June 2023 #26516

Closed

fix(servicecatalog): Make possible to have assets in several products…

0431fd9

… in the same portfolio (aws#25189) Check for the existing AssetDeploymentBucket and if it is, use it otherwise create a new one

kaizencc reopened this Aug 9, 2023

github-actions bot added effort/medium Medium work item – several days of effort p2 and removed p1 effort/large Large work item – several weeks of effort labels Aug 9, 2023

aws-cdk-automation closed this Aug 10, 2023

kaizencc reopened this Aug 11, 2023

aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Aug 12, 2023

aws-cdk-automation closed this Aug 12, 2023

kaizencc mentioned this pull request Aug 15, 2023

chore: pr-linter/do-not-close never worked #26762

Merged

kaizencc reopened this Aug 15, 2023

kaizencc removed the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Aug 25, 2023

mrgrain closed this Aug 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(servicecatalog): Make possible to have assets in different products in the same portfolio #26039

fix(servicecatalog): Make possible to have assets in different products in the same portfolio #26039

ViktarKhomich commented Jun 19, 2023

gitpod-io bot commented Jun 19, 2023

ViktarKhomich commented Jun 19, 2023

aws-cdk-automation left a comment

mrgrain commented Jun 19, 2023

corymhall Jun 27, 2023

corymhall commented Jun 27, 2023

mrgrain commented Jul 7, 2023

aws-cdk-automation commented Jul 11, 2023

aws-cdk-automation commented Jul 18, 2023

aws-cdk-automation commented Jul 19, 2023

kaizencc commented Aug 9, 2023

aws-cdk-automation commented Aug 9, 2023

aws-cdk-automation commented Aug 10, 2023

ViktarKhomich commented Aug 10, 2023

kaizencc commented Aug 11, 2023

aws-cdk-automation commented Aug 12, 2023

kaizencc commented Aug 15, 2023

mrgrain commented Aug 25, 2023

fix(servicecatalog): Make possible to have assets in different products in the same portfolio #26039

fix(servicecatalog): Make possible to have assets in different products in the same portfolio #26039

Conversation

ViktarKhomich commented Jun 19, 2023

gitpod-io bot commented Jun 19, 2023

ViktarKhomich commented Jun 19, 2023

aws-cdk-automation left a comment

Choose a reason for hiding this comment

mrgrain commented Jun 19, 2023

corymhall Jun 27, 2023

Choose a reason for hiding this comment

corymhall commented Jun 27, 2023

mrgrain commented Jul 7, 2023

aws-cdk-automation commented Jul 11, 2023

aws-cdk-automation commented Jul 18, 2023

aws-cdk-automation commented Jul 19, 2023

kaizencc commented Aug 9, 2023

aws-cdk-automation commented Aug 9, 2023

AWS CodeBuild CI Report

aws-cdk-automation commented Aug 10, 2023

ViktarKhomich commented Aug 10, 2023

kaizencc commented Aug 11, 2023

aws-cdk-automation commented Aug 12, 2023

kaizencc commented Aug 15, 2023

mrgrain commented Aug 25, 2023