From 87a33865a561f1ee7a0344d2d94a52b02d448fbe Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Fri, 5 Nov 2021 18:47:45 -0400
Subject: [PATCH 01/19] feat(aws-servicecatalog): support local launch role
 name in launch role constraint

---
 .../@aws-cdk/aws-servicecatalog/README.md     | 14 ++++
 .../aws-servicecatalog/lib/portfolio.ts       | 14 ++++
 .../lib/private/association-manager.ts        | 65 +++++++++++++------
 .../aws-servicecatalog/test/portfolio.test.ts | 32 +++++++++
 4 files changed, 105 insertions(+), 20 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/README.md b/packages/@aws-cdk/aws-servicecatalog/README.md
index 2d7694d3e84b6..042e299de784d 100644
--- a/packages/@aws-cdk/aws-servicecatalog/README.md
+++ b/packages/@aws-cdk/aws-servicecatalog/README.md
@@ -307,6 +307,20 @@ const launchRole = new iam.Role(this, 'LaunchRole', {
 portfolio.setLaunchRole(product, launchRole);
 ```
 
+You can also set the launch role to use a role of a given name that exists in the local account. This can be useful when sharing a portfolio with multiple accounts, as
+the launch role in the launch role constraint will be account-agnostic resulting in fewer resources needing to be created per shared account.
+A role with the given role name must exist in the account that created the launch constraint and the account of the user who launches a product with this launch constraint.
+
+```ts fixture=portfolio-product
+import * as iam from '@aws-cdk/aws-iam';
+
+const launchRole = new iam.Role(this, 'LaunchRole', {
+  assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
+});
+
+portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
+```
+
 See [Launch Constraint](https://docs.aws.amazon.com/servicecatalog/latest/adminguide/constraints-launch.html) documentation
 to understand permissions roles need.
 
diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
index 3056a48e19777..a498de720de3a 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
@@ -119,6 +119,16 @@ export interface IPortfolio extends cdk.IResource {
    */
   setLaunchRole(product: IProduct, launchRole: iam.IRole, options?: CommonConstraintOptions): void;
 
+  /**
+   * Force users to assume a certain role when launching a product. The role will be referenced by name
+   * in the local account. This is useful when sharing the portfolio with multiple accounts.
+   *
+   * @param product A service catalog product.
+   * @param launchRoleName The name of the IAM role a user must assume when provisioning the product. A role with this name must exist in the account.
+   * @param options options for the constraint.
+   */
+  setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options?: CommonConstraintOptions): void;
+
   /**
    * Configure deployment options using AWS Cloudformaiton StackSets
    *
@@ -179,6 +189,10 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
     AssociationManager.setLaunchRole(this, product, launchRole, options);
   }
 
+  public setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options: CommonConstraintOptions = {}): void {
+    AssociationManager.setLocalLaunchRoleName(this, product, launchRoleName, options);
+  }
+
   public deployWithStackSets(product: IProduct, options: StackSetsConstraintOptions) {
     AssociationManager.deployWithStackSets(this, product, options);
   }
diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts b/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
index bf5a68a8e70d3..65bf4055896d9 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
@@ -15,6 +15,18 @@ import { TagOptions } from '../tag-options';
 import { hashValues } from './util';
 import { InputValidator } from './validation';
 
+interface LaunchRoleProp {
+  readonly roleArn: string,
+  readonly localRoleName?: never,
+}
+
+interface LaunchRoleNameProp {
+  readonly localRoleName: string,
+  readonly roleArn?: never,
+}
+
+type LaunchRoleConstraintRoleProp = LaunchRoleProp | LaunchRoleNameProp;
+
 export class AssociationManager {
   public static associateProductWithPortfolio(
     portfolio: IPortfolio, product: IProduct, options: CommonConstraintOptions | undefined,
@@ -100,27 +112,15 @@ export class AssociationManager {
   }
 
   public static setLaunchRole(portfolio: IPortfolio, product: IProduct, launchRole: iam.IRole, options: CommonConstraintOptions): void {
-    const association = this.associateProductWithPortfolio(portfolio, product, options);
-    // Check if a stackset deployment constraint has already been configured.
-    if (portfolio.node.tryFindChild(this.stackSetConstraintLogicalId(association.associationKey))) {
-      throw new Error(`Cannot set launch role when a StackSet rule is already defined for association ${this.prettyPrintAssociation(portfolio, product)}`);
-    }
-
-    const constructId = this.launchRoleConstraintLogicalId(association.associationKey);
-    if (!portfolio.node.tryFindChild(constructId)) {
-      const constraint = new CfnLaunchRoleConstraint(portfolio as unknown as cdk.Resource, constructId, {
-        acceptLanguage: options.messageLanguage,
-        description: options.description,
-        portfolioId: portfolio.portfolioId,
-        productId: product.productId,
-        roleArn: launchRole.roleArn,
-      });
+    this.setLaunchRoleConstraint(portfolio, product, options, {
+      roleArn: launchRole.roleArn,
+    });
+  }
 
-      // Add dependsOn to force proper order in deployment.
-      constraint.addDependsOn(association.cfnPortfolioProductAssociation);
-    } else {
-      throw new Error(`Cannot set multiple launch roles for association ${this.prettyPrintAssociation(portfolio, product)}`);
-    }
+  public static setLocalLaunchRoleName(portfolio: IPortfolio, product: IProduct, launchRoleName: string, options: CommonConstraintOptions): void {
+    this.setLaunchRoleConstraint(portfolio, product, options, {
+      localRoleName: launchRoleName,
+    });
   }
 
   public static deployWithStackSets(portfolio: IPortfolio, product: IProduct, options: StackSetsConstraintOptions) {
@@ -179,6 +179,31 @@ export class AssociationManager {
     };
   }
 
+  private static setLaunchRoleConstraint(portfolio: IPortfolio, product: IProduct, options: CommonConstraintOptions,
+    roleProp: LaunchRoleConstraintRoleProp): void {
+    const association = this.associateProductWithPortfolio(portfolio, product, options);
+    // Check if a stackset deployment constraint has already been configured.
+    if (portfolio.node.tryFindChild(this.stackSetConstraintLogicalId(association.associationKey))) {
+      throw new Error(`Cannot set launch role when a StackSet rule is already defined for association ${this.prettyPrintAssociation(portfolio, product)}`);
+    }
+
+    const constructId = this.launchRoleConstraintLogicalId(association.associationKey);
+    if (!portfolio.node.tryFindChild(constructId)) {
+      const constraint = new CfnLaunchRoleConstraint(portfolio as unknown as cdk.Resource, constructId, {
+        acceptLanguage: options.messageLanguage,
+        description: options.description,
+        portfolioId: portfolio.portfolioId,
+        productId: product.productId,
+        ...roleProp,
+      });
+
+      // Add dependsOn to force proper order in deployment.
+      constraint.addDependsOn(association.cfnPortfolioProductAssociation);
+    } else {
+      throw new Error(`Cannot set multiple launch roles for association ${this.prettyPrintAssociation(portfolio, product)}`);
+    }
+  }
+
   private static stackSetConstraintLogicalId(associationKey: string): string {
     return `StackSetConstraint${associationKey}`;
   }
diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 8f9a27a96a940..84f60270fed0c 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -591,6 +591,23 @@ describe('portfolio associations and product constraints', () => {
       });
     }),
 
+    test('set a launch role constraint using local role name', () => {
+      portfolio.addProduct(product);
+
+      portfolio.setLocalLaunchRoleName(product, 'LaunchRole', {
+        description: 'set launch role description',
+        messageLanguage: servicecatalog.MessageLanguage.EN,
+      });
+
+      Template.fromStack(stack).hasResourceProperties('AWS::ServiceCatalog::LaunchRoleConstraint', {
+        PortfolioId: { Ref: 'MyPortfolio59CCA9C9' },
+        ProductId: { Ref: 'MyProduct49A3C587' },
+        Description: 'set launch role description',
+        AcceptLanguage: 'en',
+        LocalRoleName: 'LaunchRole',
+      });
+    }),
+
     test('set launch role constraint still adds without explicit association', () => {
       portfolio.setLaunchRole(product, launchRole);
 
@@ -609,6 +626,21 @@ describe('portfolio associations and product constraints', () => {
       }).toThrowError(/Cannot set multiple launch roles for association/);
     }),
 
+    test('fails to add multiple set launch roles local launch role first', () => {
+      portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
+
+      expect(() => {
+        portfolio.setLaunchRole(product, launchRole);
+      }).toThrowError(/Cannot set multiple launch roles for association/);
+    }),
+
+    test('fails to add multiple set launch roles local launch role second', () => {
+      portfolio.setLaunchRole(product, launchRole);
+      expect(() => {
+        portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
+      }).toThrowError(/Cannot set multiple launch roles for association/);
+    }),
+
     test('fails to set launch role if stackset rule is already defined', () => {
       portfolio.deployWithStackSets(product, {
         accounts: ['012345678901', '012345678901'],

From 0d4915dae47c849be7948e2dddec55083505c578 Mon Sep 17 00:00:00 2001
From: Dillon <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 16:53:26 -0500
Subject: [PATCH 02/19] Update packages/@aws-cdk/aws-servicecatalog/README.md

Co-authored-by: arcrank <arcrank@gmail.com>
---
 packages/@aws-cdk/aws-servicecatalog/README.md | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/README.md b/packages/@aws-cdk/aws-servicecatalog/README.md
index 042e299de784d..d7e64a63b18c5 100644
--- a/packages/@aws-cdk/aws-servicecatalog/README.md
+++ b/packages/@aws-cdk/aws-servicecatalog/README.md
@@ -307,9 +307,10 @@ const launchRole = new iam.Role(this, 'LaunchRole', {
 portfolio.setLaunchRole(product, launchRole);
 ```
 
-You can also set the launch role to use a role of a given name that exists in the local account. This can be useful when sharing a portfolio with multiple accounts, as
-the launch role in the launch role constraint will be account-agnostic resulting in fewer resources needing to be created per shared account.
-A role with the given role name must exist in the account that created the launch constraint and the account of the user who launches a product with this launch constraint.
+You can also set the launch role using just the name of a role which is locally deployed in end user accounts.
+This is useful for when  roles and users are separately managed outside of the CDK.
+The given role must exist in both the account that creates the launch role constraint, 
+as well as in any end user accounts that wish to provision a product with the launch role. 
 
 ```ts fixture=portfolio-product
 import * as iam from '@aws-cdk/aws-iam';

From aa05c51d6e3450c5efe73283e8c4f440a61f529f Mon Sep 17 00:00:00 2001
From: Dillon <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 16:53:48 -0500
Subject: [PATCH 03/19] Update
 packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts

Co-authored-by: Adam Ruka <adamruka85@gmail.com>
---
 packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
index a498de720de3a..e1474809389b4 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
@@ -120,8 +120,9 @@ export interface IPortfolio extends cdk.IResource {
   setLaunchRole(product: IProduct, launchRole: iam.IRole, options?: CommonConstraintOptions): void;
 
   /**
-   * Force users to assume a certain role when launching a product. The role will be referenced by name
-   * in the local account. This is useful when sharing the portfolio with multiple accounts.
+   * Force users to assume a certain role when launching a product.
+   * The role will be referenced by name in the local account.
+   * This is useful when sharing the portfolio with multiple accounts.
    *
    * @param product A service catalog product.
    * @param launchRoleName The name of the IAM role a user must assume when provisioning the product. A role with this name must exist in the account.

From e88edf75c2415ca460e493e547456f38025593d3 Mon Sep 17 00:00:00 2001
From: Dillon <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 16:53:57 -0500
Subject: [PATCH 04/19] Update
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts

Co-authored-by: Adam Ruka <adamruka85@gmail.com>
---
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 84f60270fed0c..5a0cc2aa9ac72 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -626,7 +626,7 @@ describe('portfolio associations and product constraints', () => {
       }).toThrowError(/Cannot set multiple launch roles for association/);
     }),
 
-    test('fails to add multiple set launch roles local launch role first', () => {
+    test('fails to add multiple set launch roles - local launch role first', () => {
       portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
 
       expect(() => {

From 30fad55c645b9b68dd38454a1618664e6afcceb0 Mon Sep 17 00:00:00 2001
From: Dillon <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 16:54:04 -0500
Subject: [PATCH 05/19] Update
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts

Co-authored-by: Adam Ruka <adamruka85@gmail.com>
---
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 5a0cc2aa9ac72..8ce040678cb8a 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -634,7 +634,7 @@ describe('portfolio associations and product constraints', () => {
       }).toThrowError(/Cannot set multiple launch roles for association/);
     }),
 
-    test('fails to add multiple set launch roles local launch role second', () => {
+    test('fails to add multiple set launch roles - local launch role second', () => {
       portfolio.setLaunchRole(product, launchRole);
       expect(() => {
         portfolio.setLocalLaunchRoleName(product, 'LaunchRole');

From d7faffda17582f9473aecf8a283a0f128d6a1aa8 Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 17:24:22 -0500
Subject: [PATCH 06/19] fixed readme role name

---
 packages/@aws-cdk/aws-servicecatalog/README.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/README.md b/packages/@aws-cdk/aws-servicecatalog/README.md
index d7e64a63b18c5..1f905c963a756 100644
--- a/packages/@aws-cdk/aws-servicecatalog/README.md
+++ b/packages/@aws-cdk/aws-servicecatalog/README.md
@@ -315,11 +315,13 @@ as well as in any end user accounts that wish to provision a product with the la
 ```ts fixture=portfolio-product
 import * as iam from '@aws-cdk/aws-iam';
 
+const roleName = 'MyRole';
 const launchRole = new iam.Role(this, 'LaunchRole', {
+  roleName: roleName,
   assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
 });
 
-portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
+portfolio.setLocalLaunchRoleName(product, roleName);
 ```
 
 See [Launch Constraint](https://docs.aws.amazon.com/servicecatalog/latest/adminguide/constraints-launch.html) documentation

From 89c6f3e430d85709feb814c8f7980aae5a3b55aa Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 17:25:42 -0500
Subject: [PATCH 07/19] fixed readme wording

---
 packages/@aws-cdk/aws-servicecatalog/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/README.md b/packages/@aws-cdk/aws-servicecatalog/README.md
index 1f905c963a756..3ae7d51e1f36e 100644
--- a/packages/@aws-cdk/aws-servicecatalog/README.md
+++ b/packages/@aws-cdk/aws-servicecatalog/README.md
@@ -325,7 +325,7 @@ portfolio.setLocalLaunchRoleName(product, roleName);
 ```
 
 See [Launch Constraint](https://docs.aws.amazon.com/servicecatalog/latest/adminguide/constraints-launch.html) documentation
-to understand permissions roles need.
+to understand the permissions roles need.
 
 ### Deploy with StackSets
 

From a4e8047700ec9b704bc64d4705f8998c5c3ac5be Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 17:30:16 -0500
Subject: [PATCH 08/19] fixed readme wording for portfolio local launch role

---
 packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
index e1474809389b4..1177da9faed2e 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
@@ -125,7 +125,7 @@ export interface IPortfolio extends cdk.IResource {
    * This is useful when sharing the portfolio with multiple accounts.
    *
    * @param product A service catalog product.
-   * @param launchRoleName The name of the IAM role a user must assume when provisioning the product. A role with this name must exist in the account.
+   * @param launchRoleName The name of the IAM role a user must assume when provisioning the product. A role with this name must exist in the account where the portolio is created and the accounts it is shared with.
    * @param options options for the constraint.
    */
   setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options?: CommonConstraintOptions): void;

From 926b53c17c8d38417934e5746d7a73f45c627072 Mon Sep 17 00:00:00 2001
From: Dillon <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 17:30:48 -0500
Subject: [PATCH 09/19] Update
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts

Co-authored-by: Adam Ruka <adamruka85@gmail.com>
---
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 8ce040678cb8a..dbb81dd1acae3 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -632,7 +632,7 @@ describe('portfolio associations and product constraints', () => {
       expect(() => {
         portfolio.setLaunchRole(product, launchRole);
       }).toThrowError(/Cannot set multiple launch roles for association/);
-    }),
+    });
 
     test('fails to add multiple set launch roles - local launch role second', () => {
       portfolio.setLaunchRole(product, launchRole);

From 8d5440fc723eeb30c3620bd8934af6a542bd945a Mon Sep 17 00:00:00 2001
From: Dillon <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 17:30:54 -0500
Subject: [PATCH 10/19] Update
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts

Co-authored-by: Adam Ruka <adamruka85@gmail.com>
---
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index dbb81dd1acae3..6c59b8ca58570 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -639,7 +639,7 @@ describe('portfolio associations and product constraints', () => {
       expect(() => {
         portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
       }).toThrowError(/Cannot set multiple launch roles for association/);
-    }),
+    });
 
     test('fails to set launch role if stackset rule is already defined', () => {
       portfolio.deployWithStackSets(product, {

From ab205f449c2f6142cb1ca2f97f08a1358fd9311c Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 17:32:01 -0500
Subject: [PATCH 11/19] empty line added

---
 packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 6c59b8ca58570..7946f658dab83 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -636,6 +636,7 @@ describe('portfolio associations and product constraints', () => {
 
     test('fails to add multiple set launch roles - local launch role second', () => {
       portfolio.setLaunchRole(product, launchRole);
+
       expect(() => {
         portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
       }).toThrowError(/Cannot set multiple launch roles for association/);

From 7309c4b07636ff074f002cbf1cc255039068c56f Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 18:42:08 -0500
Subject: [PATCH 12/19] add an api that accepts a role and uses its name as
 well as an api that accepts a name and creates the role

---
 .../@aws-cdk/aws-servicecatalog/README.md     | 16 ++++++-
 .../aws-servicecatalog/lib/portfolio.ts       | 37 +++++++++++++--
 .../aws-servicecatalog/test/portfolio.test.ts | 45 ++++++++++++++++++-
 3 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/README.md b/packages/@aws-cdk/aws-servicecatalog/README.md
index 3ae7d51e1f36e..b0d337f6d9939 100644
--- a/packages/@aws-cdk/aws-servicecatalog/README.md
+++ b/packages/@aws-cdk/aws-servicecatalog/README.md
@@ -308,10 +308,12 @@ portfolio.setLaunchRole(product, launchRole);
 ```
 
 You can also set the launch role using just the name of a role which is locally deployed in end user accounts.
-This is useful for when  roles and users are separately managed outside of the CDK.
+This is useful for when roles and users are separately managed outside of the CDK.
 The given role must exist in both the account that creates the launch role constraint, 
 as well as in any end user accounts that wish to provision a product with the launch role. 
 
+You can do this by passing in the role with an explicitly set name:
+
 ```ts fixture=portfolio-product
 import * as iam from '@aws-cdk/aws-iam';
 
@@ -321,7 +323,17 @@ const launchRole = new iam.Role(this, 'LaunchRole', {
   assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
 });
 
-portfolio.setLocalLaunchRoleName(product, roleName);
+portfolio.setLocalLaunchRole(product, launchRole);
+```
+
+Or you can simply pass in a role name and CDK will create a role with that name that trusts service catalog in the account:
+
+```ts fixture=portfolio-product
+import * as iam from '@aws-cdk/aws-iam';
+
+const roleName = 'MyRole';
+
+const launchRole: iam.IRole = portfolio.setLocalLaunchRoleName(product, roleName);
 ```
 
 See [Launch Constraint](https://docs.aws.amazon.com/servicecatalog/latest/adminguide/constraints-launch.html) documentation
diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
index 1177da9faed2e..d409bc0c9383b 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
@@ -122,16 +122,29 @@ export interface IPortfolio extends cdk.IResource {
   /**
    * Force users to assume a certain role when launching a product.
    * The role will be referenced by name in the local account.
+   * A role with this name will automatically be created and assumable by Service Catalog in this account.
    * This is useful when sharing the portfolio with multiple accounts.
    *
    * @param product A service catalog product.
    * @param launchRoleName The name of the IAM role a user must assume when provisioning the product. A role with this name must exist in the account where the portolio is created and the accounts it is shared with.
    * @param options options for the constraint.
    */
-  setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options?: CommonConstraintOptions): void;
+  setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options?: CommonConstraintOptions): iam.IRole;
 
   /**
-   * Configure deployment options using AWS Cloudformaiton StackSets
+   * Force users to assume a certain role when launching a product.
+   * The role name will be referenced by name in the local account.
+   * If you do not explicitly set the role name, one will be generated
+   * This is useful when sharing the portfolio with multiple accounts.
+   *
+   * @param product A service catalog product.
+   * @param launchRole The IAM role a user must assume when provisioning the product. A role with this name must exist in the account where the portolio is created and the accounts it is shared with.
+   * @param options options for the constraint.
+   */
+  setLocalLaunchRole(product: IProduct, launchRole: iam.IRole, options?: CommonConstraintOptions): void;
+
+  /**
+   * Configure deployment options using AWS Cloudformation StackSets
    *
    * @param product A service catalog product.
    * @param options Configuration options for the constraint.
@@ -190,8 +203,14 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
     AssociationManager.setLaunchRole(this, product, launchRole, options);
   }
 
-  public setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options: CommonConstraintOptions = {}): void {
-    AssociationManager.setLocalLaunchRoleName(this, product, launchRoleName, options);
+  public setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options: CommonConstraintOptions = {}): iam.IRole {
+    const launchRole: iam.IRole = this.createBaseLaunchRole(launchRoleName);
+    AssociationManager.setLocalLaunchRoleName(this, product, launchRole.roleName, options);
+    return launchRole;
+  }
+
+  public setLocalLaunchRole(product: IProduct, launchRole: iam.IRole, options: CommonConstraintOptions = {}): void {
+    AssociationManager.setLocalLaunchRoleName(this, product, launchRole.roleName, options);
   }
 
   public deployWithStackSets(product: IProduct, options: StackSetsConstraintOptions) {
@@ -214,6 +233,16 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
     }
   }
 
+  /**
+   * If setting a local launch role by name, this will create a launch role that trusts service catalog with that name.
+   */
+  private createBaseLaunchRole(roleName: string): iam.Role {
+    return new iam.Role(this.stack, `LaunchRole${roleName}`, {
+      roleName: roleName,
+      assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
+    });
+  }
+
   /**
    * Create a unique id based off the L1 CfnPortfolio or the arn of an imported portfolio.
    */
diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 7946f658dab83..6ca689a7533fe 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -594,7 +594,7 @@ describe('portfolio associations and product constraints', () => {
     test('set a launch role constraint using local role name', () => {
       portfolio.addProduct(product);
 
-      portfolio.setLocalLaunchRoleName(product, 'LaunchRole', {
+      portfolio.setLocalLaunchRoleName(product, 'LocalLaunchRole', {
         description: 'set launch role description',
         messageLanguage: servicecatalog.MessageLanguage.EN,
       });
@@ -604,7 +604,24 @@ describe('portfolio associations and product constraints', () => {
         ProductId: { Ref: 'MyProduct49A3C587' },
         Description: 'set launch role description',
         AcceptLanguage: 'en',
-        LocalRoleName: 'LaunchRole',
+        LocalRoleName: { Ref: 'LaunchRoleLocalLaunchRoleF14290D5' },
+      });
+    }),
+
+    test('set a launch role constraint using local role', () => {
+      portfolio.addProduct(product);
+
+      portfolio.setLocalLaunchRole(product, launchRole, {
+        description: 'set launch role description',
+        messageLanguage: servicecatalog.MessageLanguage.EN,
+      });
+
+      Template.fromStack(stack).hasResourceProperties('AWS::ServiceCatalog::LaunchRoleConstraint', {
+        PortfolioId: { Ref: 'MyPortfolio59CCA9C9' },
+        ProductId: { Ref: 'MyProduct49A3C587' },
+        Description: 'set launch role description',
+        AcceptLanguage: 'en',
+        LocalRoleName: { Ref: 'LaunchRole2CFB2E44' },
       });
     }),
 
@@ -634,6 +651,30 @@ describe('portfolio associations and product constraints', () => {
       }).toThrowError(/Cannot set multiple launch roles for association/);
     });
 
+    test('fails to add multiple set local launch roles - local launch role first', () => {
+      portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
+
+      expect(() => {
+        portfolio.setLocalLaunchRole(product, launchRole);
+      }).toThrowError(/Cannot set multiple launch roles for association/);
+    });
+
+    test('fails to add multiple set local launch roles - local launch role name first', () => {
+      portfolio.setLocalLaunchRole(product, launchRole);
+
+      expect(() => {
+        portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
+      }).toThrowError(/Cannot set multiple launch roles for association/);
+    });
+
+    test('fails to add multiple set launch roles - local launch role second', () => {
+      portfolio.setLaunchRole(product, launchRole);
+
+      expect(() => {
+        portfolio.setLocalLaunchRole(product, launchRole);
+      }).toThrowError(/Cannot set multiple launch roles for association/);
+    });
+
     test('fails to add multiple set launch roles - local launch role second', () => {
       portfolio.setLaunchRole(product, launchRole);
 

From 720cafa7c01d0e3f517c1aba19a3e060a95d4a38 Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 18:54:29 -0500
Subject: [PATCH 13/19] add test for imported local launch role

---
 .../aws-servicecatalog/test/portfolio.test.ts | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 6ca689a7533fe..09b7d608f6a0c 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -625,6 +625,25 @@ describe('portfolio associations and product constraints', () => {
       });
     }),
 
+    test('set a launch role constraint using imported local role', () => {
+      portfolio.addProduct(product);
+
+      const importedLaunchRole = iam.Role.fromRoleArn(portfolio.stack, 'ImportedLaunchRole', 'arn:aws:iam::123456789012:role/ImportedLaunchRole');
+
+      portfolio.setLocalLaunchRole(product, importedLaunchRole, {
+        description: 'set launch role description',
+        messageLanguage: servicecatalog.MessageLanguage.EN,
+      });
+
+      Template.fromStack(stack).hasResourceProperties('AWS::ServiceCatalog::LaunchRoleConstraint', {
+        PortfolioId: { Ref: 'MyPortfolio59CCA9C9' },
+        ProductId: { Ref: 'MyProduct49A3C587' },
+        Description: 'set launch role description',
+        AcceptLanguage: 'en',
+        LocalRoleName: 'ImportedLaunchRole',
+      });
+    }),
+
     test('set launch role constraint still adds without explicit association', () => {
       portfolio.setLaunchRole(product, launchRole);
 

From 91c08d12dce037211bea6b4a96b7d03634123154 Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Mon, 8 Nov 2021 21:08:48 -0500
Subject: [PATCH 14/19] made role options clearer and correctly named

---
 .../lib/private/association-manager.ts        | 28 +++++++++----------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts b/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
index 65bf4055896d9..f3558ce0a42c0 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
@@ -15,18 +15,6 @@ import { TagOptions } from '../tag-options';
 import { hashValues } from './util';
 import { InputValidator } from './validation';
 
-interface LaunchRoleProp {
-  readonly roleArn: string,
-  readonly localRoleName?: never,
-}
-
-interface LaunchRoleNameProp {
-  readonly localRoleName: string,
-  readonly roleArn?: never,
-}
-
-type LaunchRoleConstraintRoleProp = LaunchRoleProp | LaunchRoleNameProp;
-
 export class AssociationManager {
   public static associateProductWithPortfolio(
     portfolio: IPortfolio, product: IProduct, options: CommonConstraintOptions | undefined,
@@ -180,7 +168,7 @@ export class AssociationManager {
   }
 
   private static setLaunchRoleConstraint(portfolio: IPortfolio, product: IProduct, options: CommonConstraintOptions,
-    roleProp: LaunchRoleConstraintRoleProp): void {
+    roleOptions: LaunchRoleConstraintRoleOptions): void {
     const association = this.associateProductWithPortfolio(portfolio, product, options);
     // Check if a stackset deployment constraint has already been configured.
     if (portfolio.node.tryFindChild(this.stackSetConstraintLogicalId(association.associationKey))) {
@@ -194,7 +182,8 @@ export class AssociationManager {
         description: options.description,
         portfolioId: portfolio.portfolioId,
         productId: product.productId,
-        ...roleProp,
+        roleArn: roleOptions.roleArn,
+        localRoleName: roleOptions.localRoleName,
       });
 
       // Add dependsOn to force proper order in deployment.
@@ -238,3 +227,14 @@ export class AssociationManager {
   };
 }
 
+interface LaunchRoleArnOption {
+  readonly roleArn: string,
+  readonly localRoleName?: never,
+}
+
+interface LaunchRoleNameOption {
+  readonly localRoleName: string,
+  readonly roleArn?: never,
+}
+
+type LaunchRoleConstraintRoleOptions = LaunchRoleArnOption | LaunchRoleNameOption;

From 9ade69d66b5490733cb1bed0367e24e04593bbba Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Tue, 9 Nov 2021 00:10:38 -0500
Subject: [PATCH 15/19] adding validation that a role name must be set on a
 role used as a local launch role

---
 .../@aws-cdk/aws-servicecatalog/lib/portfolio.ts    |  6 +++---
 .../aws-servicecatalog/lib/private/validation.ts    | 13 +++++++++++++
 .../aws-servicecatalog/test/portfolio.test.ts       | 11 +++++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
index d409bc0c9383b..1a5b54f0e5973 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
@@ -133,12 +133,11 @@ export interface IPortfolio extends cdk.IResource {
 
   /**
    * Force users to assume a certain role when launching a product.
-   * The role name will be referenced by name in the local account.
-   * If you do not explicitly set the role name, one will be generated
+   * The role name will be referenced by name in the local account and must be set explicitly.
    * This is useful when sharing the portfolio with multiple accounts.
    *
    * @param product A service catalog product.
-   * @param launchRole The IAM role a user must assume when provisioning the product. A role with this name must exist in the account where the portolio is created and the accounts it is shared with.
+   * @param launchRole The IAM role a user must assume when provisioning the product. A role with this name must exist in the account where the portolio is created and the accounts it is shared with. The role name must be set explicitly.
    * @param options options for the constraint.
    */
   setLocalLaunchRole(product: IProduct, launchRole: iam.IRole, options?: CommonConstraintOptions): void;
@@ -210,6 +209,7 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
   }
 
   public setLocalLaunchRole(product: IProduct, launchRole: iam.IRole, options: CommonConstraintOptions = {}): void {
+    InputValidator.validateRoleNameSetForLocalLaunchRole(launchRole);
     AssociationManager.setLocalLaunchRoleName(this, product, launchRole.roleName, options);
   }
 
diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts b/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts
index 3beaa42552eff..ecfba80f30643 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts
@@ -1,3 +1,4 @@
+import * as iam from '@aws-cdk/aws-iam';
 import * as cdk from '@aws-cdk/core';
 
 /**
@@ -36,6 +37,18 @@ export class InputValidator {
     this.validateRegex(resourceName, inputName, /^[\w\d.%+\-]+@[a-z\d.\-]+\.[a-z]{2,4}$/i, inputString);
   }
 
+  /**
+  * Validates that a role being used as a local launch role has the role name set
+  */
+  public static validateRoleNameSetForLocalLaunchRole(role: iam.IRole): void {
+    if (role.node.defaultChild) {
+      const roleName = (role.node.defaultChild as iam.CfnRole).roleName;
+      if (cdk.Token.isUnresolved(roleName)) {
+        throw new Error(`Role ${role.node.id} used for Local Launch Role must have roleName explicitly set`);
+      }
+    }
+  }
+
   private static truncateString(string: string, maxLength: number): string {
     if (string.length > maxLength) {
       return string.substring(0, maxLength) + '[truncated]';
diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 09b7d608f6a0c..ee0ef8cbf5097 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -568,6 +568,7 @@ describe('portfolio associations and product constraints', () => {
         assumedBy: new iam.AccountRootPrincipal(),
       });
       launchRole = new iam.Role(stack, 'LaunchRole', {
+        roleName: 'LaunchRole',
         assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
       });
     }),
@@ -662,6 +663,16 @@ describe('portfolio associations and product constraints', () => {
       }).toThrowError(/Cannot set multiple launch roles for association/);
     }),
 
+    test('local launch role must have roleName explicitly set', () => {
+      const otherLaunchRole = new iam.Role(stack, 'otherLaunchRole', {
+        assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
+      });
+
+      expect(() => {
+        portfolio.setLocalLaunchRole(product, otherLaunchRole);
+      }).toThrowError(/Role otherLaunchRole used for Local Launch Role must have roleName explicitly set/);
+    }),
+
     test('fails to add multiple set launch roles - local launch role first', () => {
       portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
 

From 3a96a198fae66015153a659ebfa95ee4c9c6a0ee Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Tue, 9 Nov 2021 00:14:44 -0500
Subject: [PATCH 16/19] onelined roleName

---
 packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts b/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts
index ecfba80f30643..cd70006fbe373 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/private/validation.ts
@@ -42,8 +42,7 @@ export class InputValidator {
   */
   public static validateRoleNameSetForLocalLaunchRole(role: iam.IRole): void {
     if (role.node.defaultChild) {
-      const roleName = (role.node.defaultChild as iam.CfnRole).roleName;
-      if (cdk.Token.isUnresolved(roleName)) {
+      if (cdk.Token.isUnresolved((role.node.defaultChild as iam.CfnRole).roleName)) {
         throw new Error(`Role ${role.node.id} used for Local Launch Role must have roleName explicitly set`);
       }
     }

From 498cf0f7f6026f0505cef623e6af7af4ba60407d Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Tue, 9 Nov 2021 11:42:31 -0500
Subject: [PATCH 17/19] changed test semicolons to commas

---
 .../@aws-cdk/aws-servicecatalog/test/portfolio.test.ts | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index ee0ef8cbf5097..8dfd943d1a8d4 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -679,7 +679,7 @@ describe('portfolio associations and product constraints', () => {
       expect(() => {
         portfolio.setLaunchRole(product, launchRole);
       }).toThrowError(/Cannot set multiple launch roles for association/);
-    });
+    }),
 
     test('fails to add multiple set local launch roles - local launch role first', () => {
       portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
@@ -687,7 +687,7 @@ describe('portfolio associations and product constraints', () => {
       expect(() => {
         portfolio.setLocalLaunchRole(product, launchRole);
       }).toThrowError(/Cannot set multiple launch roles for association/);
-    });
+    }),
 
     test('fails to add multiple set local launch roles - local launch role name first', () => {
       portfolio.setLocalLaunchRole(product, launchRole);
@@ -695,7 +695,7 @@ describe('portfolio associations and product constraints', () => {
       expect(() => {
         portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
       }).toThrowError(/Cannot set multiple launch roles for association/);
-    });
+    }),
 
     test('fails to add multiple set launch roles - local launch role second', () => {
       portfolio.setLaunchRole(product, launchRole);
@@ -703,7 +703,7 @@ describe('portfolio associations and product constraints', () => {
       expect(() => {
         portfolio.setLocalLaunchRole(product, launchRole);
       }).toThrowError(/Cannot set multiple launch roles for association/);
-    });
+    }),
 
     test('fails to add multiple set launch roles - local launch role second', () => {
       portfolio.setLaunchRole(product, launchRole);
@@ -711,7 +711,7 @@ describe('portfolio associations and product constraints', () => {
       expect(() => {
         portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
       }).toThrowError(/Cannot set multiple launch roles for association/);
-    });
+    }),
 
     test('fails to set launch role if stackset rule is already defined', () => {
       portfolio.deployWithStackSets(product, {

From 64de0ef12bad7bdc0751f8c09a63650cc62ecef7 Mon Sep 17 00:00:00 2001
From: Dillon <dponzo18@gmail.com>
Date: Tue, 9 Nov 2021 20:32:34 -0500
Subject: [PATCH 18/19] Update
 packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts

Co-authored-by: Adam Ruka <adamruka85@gmail.com>
---
 .../aws-servicecatalog/lib/private/association-manager.ts   | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts b/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
index f3558ce0a42c0..b92fb2483ad54 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts
@@ -167,8 +167,10 @@ export class AssociationManager {
     };
   }
 
-  private static setLaunchRoleConstraint(portfolio: IPortfolio, product: IProduct, options: CommonConstraintOptions,
-    roleOptions: LaunchRoleConstraintRoleOptions): void {
+  private static setLaunchRoleConstraint(
+    portfolio: IPortfolio, product: IProduct, options: CommonConstraintOptions,
+    roleOptions: LaunchRoleConstraintRoleOptions,
+  ): void {
     const association = this.associateProductWithPortfolio(portfolio, product, options);
     // Check if a stackset deployment constraint has already been configured.
     if (portfolio.node.tryFindChild(this.stackSetConstraintLogicalId(association.associationKey))) {

From d6f99ade00e168dc8aaea2c1d28246bd38168f3c Mon Sep 17 00:00:00 2001
From: Dillon Ponzo <dponzo18@gmail.com>
Date: Tue, 9 Nov 2021 20:42:21 -0500
Subject: [PATCH 19/19] address pr comments

---
 .../@aws-cdk/aws-servicecatalog/README.md     |  3 +--
 .../aws-servicecatalog/lib/portfolio.ts       | 22 ++++++++-----------
 .../aws-servicecatalog/test/portfolio.test.ts | 16 +++++++-------
 3 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/packages/@aws-cdk/aws-servicecatalog/README.md b/packages/@aws-cdk/aws-servicecatalog/README.md
index b0d337f6d9939..bbc82e13e2d7b 100644
--- a/packages/@aws-cdk/aws-servicecatalog/README.md
+++ b/packages/@aws-cdk/aws-servicecatalog/README.md
@@ -317,9 +317,8 @@ You can do this by passing in the role with an explicitly set name:
 ```ts fixture=portfolio-product
 import * as iam from '@aws-cdk/aws-iam';
 
-const roleName = 'MyRole';
 const launchRole = new iam.Role(this, 'LaunchRole', {
-  roleName: roleName,
+  roleName: 'MyRole',
   assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
 });
 
diff --git a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
index 1a5b54f0e5973..36d267d022519 100644
--- a/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/lib/portfolio.ts
@@ -112,6 +112,9 @@ export interface IPortfolio extends cdk.IResource {
 
   /**
    * Force users to assume a certain role when launching a product.
+   * This sets the launch role using the role arn which is tied to the account this role exists in.
+   * This is useful if you will be provisioning products from the account where this role exists.
+   * If you intend to share the portfolio across accounts, use a local launch role.
    *
    * @param product A service catalog product.
    * @param launchRole The IAM role a user must assume when provisioning the product.
@@ -121,7 +124,7 @@ export interface IPortfolio extends cdk.IResource {
 
   /**
    * Force users to assume a certain role when launching a product.
-   * The role will be referenced by name in the local account.
+   * The role will be referenced by name in the local account instead of a static role arn.
    * A role with this name will automatically be created and assumable by Service Catalog in this account.
    * This is useful when sharing the portfolio with multiple accounts.
    *
@@ -133,7 +136,7 @@ export interface IPortfolio extends cdk.IResource {
 
   /**
    * Force users to assume a certain role when launching a product.
-   * The role name will be referenced by name in the local account and must be set explicitly.
+   * The role name will be referenced by in the local account and must be set explicitly.
    * This is useful when sharing the portfolio with multiple accounts.
    *
    * @param product A service catalog product.
@@ -203,7 +206,10 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
   }
 
   public setLocalLaunchRoleName(product: IProduct, launchRoleName: string, options: CommonConstraintOptions = {}): iam.IRole {
-    const launchRole: iam.IRole = this.createBaseLaunchRole(launchRoleName);
+    const launchRole: iam.IRole = new iam.Role(this, `LaunchRole${launchRoleName}`, {
+      roleName: launchRoleName,
+      assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
+    });
     AssociationManager.setLocalLaunchRoleName(this, product, launchRole.roleName, options);
     return launchRole;
   }
@@ -233,16 +239,6 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
     }
   }
 
-  /**
-   * If setting a local launch role by name, this will create a launch role that trusts service catalog with that name.
-   */
-  private createBaseLaunchRole(roleName: string): iam.Role {
-    return new iam.Role(this.stack, `LaunchRole${roleName}`, {
-      roleName: roleName,
-      assumedBy: new iam.ServicePrincipal('servicecatalog.amazonaws.com'),
-    });
-  }
-
   /**
    * Create a unique id based off the L1 CfnPortfolio or the arn of an imported portfolio.
    */
diff --git a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
index 8dfd943d1a8d4..43a283c157f80 100644
--- a/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
+++ b/packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts
@@ -605,7 +605,7 @@ describe('portfolio associations and product constraints', () => {
         ProductId: { Ref: 'MyProduct49A3C587' },
         Description: 'set launch role description',
         AcceptLanguage: 'en',
-        LocalRoleName: { Ref: 'LaunchRoleLocalLaunchRoleF14290D5' },
+        LocalRoleName: { Ref: 'MyPortfolioLaunchRoleLocalLaunchRoleB2E6E22A' },
       });
     }),
 
@@ -660,7 +660,7 @@ describe('portfolio associations and product constraints', () => {
 
       expect(() => {
         portfolio.setLaunchRole(product, otherLaunchRole);
-      }).toThrowError(/Cannot set multiple launch roles for association/);
+      }).toThrow(/Cannot set multiple launch roles for association/);
     }),
 
     test('local launch role must have roleName explicitly set', () => {
@@ -670,7 +670,7 @@ describe('portfolio associations and product constraints', () => {
 
       expect(() => {
         portfolio.setLocalLaunchRole(product, otherLaunchRole);
-      }).toThrowError(/Role otherLaunchRole used for Local Launch Role must have roleName explicitly set/);
+      }).toThrow(/Role otherLaunchRole used for Local Launch Role must have roleName explicitly set/);
     }),
 
     test('fails to add multiple set launch roles - local launch role first', () => {
@@ -678,7 +678,7 @@ describe('portfolio associations and product constraints', () => {
 
       expect(() => {
         portfolio.setLaunchRole(product, launchRole);
-      }).toThrowError(/Cannot set multiple launch roles for association/);
+      }).toThrow(/Cannot set multiple launch roles for association/);
     }),
 
     test('fails to add multiple set local launch roles - local launch role first', () => {
@@ -686,7 +686,7 @@ describe('portfolio associations and product constraints', () => {
 
       expect(() => {
         portfolio.setLocalLaunchRole(product, launchRole);
-      }).toThrowError(/Cannot set multiple launch roles for association/);
+      }).toThrow(/Cannot set multiple launch roles for association/);
     }),
 
     test('fails to add multiple set local launch roles - local launch role name first', () => {
@@ -694,7 +694,7 @@ describe('portfolio associations and product constraints', () => {
 
       expect(() => {
         portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
-      }).toThrowError(/Cannot set multiple launch roles for association/);
+      }).toThrow(/Cannot set multiple launch roles for association/);
     }),
 
     test('fails to add multiple set launch roles - local launch role second', () => {
@@ -702,7 +702,7 @@ describe('portfolio associations and product constraints', () => {
 
       expect(() => {
         portfolio.setLocalLaunchRole(product, launchRole);
-      }).toThrowError(/Cannot set multiple launch roles for association/);
+      }).toThrow(/Cannot set multiple launch roles for association/);
     }),
 
     test('fails to add multiple set launch roles - local launch role second', () => {
@@ -710,7 +710,7 @@ describe('portfolio associations and product constraints', () => {
 
       expect(() => {
         portfolio.setLocalLaunchRoleName(product, 'LaunchRole');
-      }).toThrowError(/Cannot set multiple launch roles for association/);
+      }).toThrow(/Cannot set multiple launch roles for association/);
     }),
 
     test('fails to set launch role if stackset rule is already defined', () => {