feat(servicecatalog): initial implementation of the Product construct #15185

wanjacki · 2021-06-17T20:02:14Z

This is the first minimal release of an L2 construct for a service catalog product. In a future PR,
functionality to associate with a Portfolio and add constraints will be added.

Testing done

yarn build && yarn test
yarn integ

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

Co-authored-by: Aidan Crank arcrank@gmail.com
Co-authored-by: Dillon Ponzo dponzo18@gmail.com

This is the first minimal release of an L2 construct for a service catalog product. In a future PR, functionality to associate with a Portfolio and add constraints will be added. Testing done ------------------ * `yarn build && yarn test` * `yarn integ` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* Co-authored-by: Aidan Crank <arcrank@gmail.com> Co-authored-by: Dillon Ponzo <dponzo18@gmail.com>

gitpod-io · 2021-06-17T20:02:16Z

skinny85

Thanks for submitting @wanjacki!

Before I review, I have some questions about the provisioningArtifacts property.

Can you explain what is it used for? How do customers use the many versions of a given Product?

Also, I think we should discuss how is that CloudFormation template that is pointed to by the templateUrl supposed to be created. I think it will be a very interesting area of defining how a Product works in the CDK.

skinny85

In general, I'm not a huge fan of this PR as it stands today. The L2 that is presented here is basically identical to the CfnCloudFormationProduct class that is in the CDK today. We look to improve things in our L2 by raising the level of abstraction. This doesn't do that.

I think the key here will be templateUrl property of ProvisioningArtifactProperties. We need to think about how to model the CloudFormation template of a Product in the CDK.

I think step 1 is learning about the Asset concept in the CDK: https://docs.aws.amazon.com/cdk/latest/guide/assets.html. I would encourage you to take a look at how Lambda Code is modeled in the CDK (https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-lambda.Code.html), as it has a very similar challenge to what we're facing here.

Also, if you could reply to my question in #15185 (review), I would appreciate it!

Thanks for your contribution!
Adam

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

packages/@aws-cdk/aws-servicecatalog/package.json

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

Pull request has been modified.

skinny85

Looks great! I love the Asset-based template.

A few minor comments before we merge this in.

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

packages/@aws-cdk/aws-servicecatalog/README.md

packages/@aws-cdk/aws-servicecatalog/test/development-environment.template

packages/@aws-cdk/aws-servicecatalog/README.md

packages/@aws-cdk/aws-servicecatalog/lib/template.ts

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

Pull request has been modified.

pgarbe · 2021-06-25T08:28:46Z

Would love to see support for AssetTemplate.fromStack(myStack). But it's tricky as described in #11896

skinny85 · 2021-06-27T16:03:04Z

Would love to see support for AssetTemplate.fromStack(myStack). But it's tricky as described in #11896

We'll get there, don't worry 😉. We actually have a plan on how to make it even better than that issue talks about!

That will be a separate PR though 🙂.

wanjacki · 2021-06-28T17:11:24Z

@skinny85 Hey Adam, I addressed all the comments, please let me know if there's anything that needs to be changed.

skinny85 · 2021-06-28T17:18:43Z

@skinny85 Hey Adam, I addressed all the comments, please let me know if there's anything that needs to be changed.

Apologies for missing this. Will do. In the future, if you re-request my revision after pushing your changes (there's a button in the top-right of the PR page, next to my avatar), there's a smaller chance this will slip through the cracks.

skinny85

Looks great @wanjacki! A few last polishes, and this will be ready to be merged in.

packages/@aws-cdk/aws-servicecatalog/README.md

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

packages/@aws-cdk/aws-servicecatalog/lib/template.ts

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

…to product_l2_construct

Pull request has been modified.

skinny85

We're almost there! One tiny comment about naming.

packages/@aws-cdk/aws-servicecatalog/README.md

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

packages/@aws-cdk/aws-servicecatalog/test/integ.product.ts

Pull request has been modified.

skinny85

One last thing!

skinny85 · 2021-06-28T20:49:56Z

packages/@aws-cdk/aws-servicecatalog/lib/product.ts

+/**
+ * Properties of product version (also known as a provisioning artifact).
+ */
+export interface ProductVersions {


Sorry, I think there's been some misunderstanding.

The name of this interface should be CloudFormationProductVersion.

The name of the property is a different discussion.

ok I think i fixed it Adam, is this what you mean?

readonly productVersions: CloudFormationProductVersion[];

Pull request has been modified.

skinny85

Thanks for the patience and incorporating all of my comments @wanjacki - this is great!

mergify · 2021-06-28T21:39:45Z

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

aws-cdk-automation · 2021-06-28T22:18:35Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 4510222
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2021-06-28T22:19:32Z

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

…aws#15185) This is the first minimal release of an L2 construct for a service catalog product. In a future PR, functionality to associate with a Portfolio and add constraints will be added. Testing done ------------------ * `yarn build && yarn test` * `yarn integ` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* Co-authored-by: Aidan Crank <arcrank@gmail.com> Co-authored-by: Dillon Ponzo <dponzo18@gmail.com>

Merge branch 'master' into product_l2_construct

4e544ad

peterwoodworth assigned skinny85 Jun 17, 2021

peterwoodworth added the @aws-cdk/aws-servicecatalog Related to AWS Service Catalog label Jun 17, 2021

skinny85 reviewed Jun 17, 2021

View reviewed changes

skinny85 previously requested changes Jun 18, 2021

View reviewed changes

skinny85 added effort/small Small work item – less than a day of effort p1 labels Jun 22, 2021

Merge branch 'aws:master' into product_l2_construct

5bcdc04

wanjacki force-pushed the product_l2_construct branch from d48ed48 to 4dc696b Compare June 23, 2021 20:21

feat(servicecatalog): Fixed PR comments added local asset template

e42b1fc

wanjacki force-pushed the product_l2_construct branch from 4dc696b to e42b1fc Compare June 23, 2021 20:22

skinny85 previously requested changes Jun 23, 2021

View reviewed changes

feat(servicecatalog): Addressed PR Comments

4d5b67e

Merge branch 'master' into product_l2_construct

32dcf1f

skinny85 previously requested changes Jun 28, 2021

View reviewed changes

wanjacki added 2 commits June 28, 2021 12:10

feat(servicecatalog): Addressed PR comments, mostly minor name changes

18767fb

Merge branch 'product_l2_construct' of github.com:wanjacki/aws-cdk in…

caed18d

…to product_l2_construct

wanjacki requested a review from skinny85 June 28, 2021 19:13

skinny85 previously requested changes Jun 28, 2021

View reviewed changes

packages/@aws-cdk/aws-servicecatalog/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-servicecatalog/lib/product.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-servicecatalog/test/integ.product.ts Outdated Show resolved Hide resolved

wanjacki requested a review from skinny85 June 28, 2021 20:42

skinny85 previously requested changes Jun 28, 2021

View reviewed changes

feat(servicecatalog): renaming to productVersion

0826b09

wanjacki force-pushed the product_l2_construct branch from 27f137d to 0826b09 Compare June 28, 2021 21:01

skinny85 approved these changes Jun 28, 2021

View reviewed changes

Merge branch 'master' into product_l2_construct

4510222

mergify bot merged commit fe3e0f2 into aws:master Jun 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(servicecatalog): initial implementation of the Product construct #15185

feat(servicecatalog): initial implementation of the Product construct #15185

wanjacki commented Jun 17, 2021

gitpod-io bot commented Jun 17, 2021

skinny85 left a comment

skinny85 left a comment

skinny85 left a comment

pgarbe commented Jun 25, 2021

skinny85 commented Jun 27, 2021

wanjacki commented Jun 28, 2021

skinny85 commented Jun 28, 2021

skinny85 left a comment

skinny85 left a comment

skinny85 left a comment

skinny85 Jun 28, 2021

wanjacki Jun 28, 2021 •

edited

Loading

skinny85 Jun 28, 2021

skinny85 left a comment

mergify bot commented Jun 28, 2021

aws-cdk-automation commented Jun 28, 2021

mergify bot commented Jun 28, 2021

feat(servicecatalog): initial implementation of the Product construct #15185

feat(servicecatalog): initial implementation of the Product construct #15185

Conversation

wanjacki commented Jun 17, 2021

Testing done

gitpod-io bot commented Jun 17, 2021

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

pgarbe commented Jun 25, 2021

skinny85 commented Jun 27, 2021

wanjacki commented Jun 28, 2021

skinny85 commented Jun 28, 2021

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 Jun 28, 2021

Choose a reason for hiding this comment

wanjacki Jun 28, 2021 • edited Loading

Choose a reason for hiding this comment

skinny85 Jun 28, 2021

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Jun 28, 2021

aws-cdk-automation commented Jun 28, 2021

AWS CodeBuild CI Report

mergify bot commented Jun 28, 2021

wanjacki Jun 28, 2021 •

edited

Loading