addressing PR comments, fixing optional field issue, and updating lan…

…guage

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

@@ -35,7 +35,7 @@ enables organizations to create and manage catalogs of products for their end us
 - [Constraints](#constraints)
   - [Tag update constraint](#tag-update-constraint)
   - [Notify on stack events](#notify-on-stack-events)
-  - [Cloudformation parameters constraint](#cloudformation-parameters-constraint)
+  - [CloudFormation parameters constraint](#cloudformation-parameters-constraint)
   - [Set launch role](#set-launch-role)
   - [Deploy with StackSets](#deploy-with-stacksets)
 
@@ -229,7 +229,7 @@ portfolio.notifyOnStackEvents(product, topic2, {
 });
 ```
 
-### Cloudformation parameters constraint
+### CloudFormation parameters constraint
 
 Allows you to configure the options that are available to end users when they launch a product.
 A provisioning rule consists of one or more assertions that narrow the allowable values for parameters in a product.
@@ -244,7 +244,7 @@ see [AWS Rule Functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/Us
 import * as cdk from '@aws-cdk/core';
 
 portfolio.constrainCloudFormationParameters(product, {
-  assertion: {
+  rule: {
     ruleName: 'testInstanceType',
     condition: cdk.Fn.conditionEquals(cdk.Fn.ref('Environment'), 'test'),
     assertions: [{

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

@@ -75,7 +75,7 @@ export interface TemplateRuleAssertion {
   readonly assert: cdk.ICfnRuleConditionExpression;
 
   /**
-   * The description for the asssertion
+   * The description for the asssertion.
    * @default - no description provided for the assertion.
    */
   readonly description?: string;
@@ -92,7 +92,7 @@ export interface TemplateRule {
 
   /**
    * Specify when to apply rule with a rule-specific intrinsic function.
-   * @default - No rule condition provided.
+   * @default - no rule condition provided
    */
   readonly condition?: cdk.ICfnRuleConditionExpression;
 
@@ -105,7 +105,7 @@ export interface TemplateRule {
 /**
  * Properties for provisoning rule constraint.
  */
-export interface ProvisioningRuleOptions extends CommonConstraintOptions {
+export interface CloudFormationRuleConstraintOptions extends CommonConstraintOptions {
   /**
    * The rule with condition and assertions to apply to template.
    */

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

@@ -3,8 +3,8 @@ import * as sns from '@aws-cdk/aws-sns';
 import * as cdk from '@aws-cdk/core';
 import { MessageLanguage } from './common';
 import {
-  CommonConstraintOptions, StackSetsConstraintOptions,
-  TagUpdateConstraintOptions, ProvisioningRuleOptions,
+  CloudFormationRuleConstraintOptions, CommonConstraintOptions,
+  StackSetsConstraintOptions, TagUpdateConstraintOptions,
 } from './constraints';
 import { AssociationManager } from './private/association-manager';
 import { hashValues } from './private/util';
@@ -108,7 +108,7 @@ export interface IPortfolio extends cdk.IResource {
    * @param product A service catalog product.
    * @param options options for the constraint.
    */
-  constrainCloudFormationParameters(product:IProduct, options: ProvisioningRuleOptions): void;
+  constrainCloudFormationParameters(product:IProduct, options: CloudFormationRuleConstraintOptions): void;
 
   /**
    * Force users to assume a certain role when launching a product.
@@ -146,7 +146,7 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
   }
 
   public addProduct(product: IProduct): void {
-    AssociationManager.associateProductWithPortfolio(this, product);
+    AssociationManager.associateProductWithPortfolio(this, product, undefined);
   }
 
   public shareWithAccount(accountId: string, options: PortfolioShareOptions = {}): void {
@@ -171,7 +171,7 @@ abstract class PortfolioBase extends cdk.Resource implements IPortfolio {
     AssociationManager.notifyOnStackEvents(this, product, topic, options);
   }
 
-  public constrainCloudFormationParameters(product: IProduct, options: ProvisioningRuleOptions): void {
+  public constrainCloudFormationParameters(product: IProduct, options: CloudFormationRuleConstraintOptions): void {
     AssociationManager.constrainCloudFormationParameters(this, product, options);
   }
 

packages/@aws-cdk/aws-servicecatalog/lib/private/association-manager.ts

@@ -2,7 +2,7 @@ import * as iam from '@aws-cdk/aws-iam';
 import * as sns from '@aws-cdk/aws-sns';
 import * as cdk from '@aws-cdk/core';
 import {
-  CommonConstraintOptions, ProvisioningRuleOptions, StackSetsConstraintOptions,
+  CloudFormationRuleConstraintOptions, CommonConstraintOptions, StackSetsConstraintOptions,
   TagUpdateConstraintOptions, TemplateRule, TemplateRuleAssertion,
 } from '../constraints';
 import { IPortfolio } from '../portfolio';
@@ -17,7 +17,7 @@ import { InputValidator } from './validation';
 
 export class AssociationManager {
   public static associateProductWithPortfolio(
-    portfolio: IPortfolio, product: IProduct, options?: CommonConstraintOptions,
+    portfolio: IPortfolio, product: IProduct, options: CommonConstraintOptions | undefined,
   ): { associationKey: string, cfnPortfolioProductAssociation: CfnPortfolioProductAssociation } {
     InputValidator.validateLength(this.prettyPrintAssociation(portfolio, product), 'description', 0, 2000, options?.description);
     const associationKey = hashValues(portfolio.node.addr, product.node.addr, product.stack.node.addr);
@@ -37,7 +37,7 @@ export class AssociationManager {
   }
 
   public static constrainTagUpdates(portfolio: IPortfolio, product: IProduct, options: TagUpdateConstraintOptions): void {
-    const association = this.associateProductWithPortfolio(portfolio, product);
+    const association = this.associateProductWithPortfolio(portfolio, product, options);
     const constructId = `ResourceUpdateConstraint${association.associationKey}`;
 
     if (!portfolio.node.tryFindChild(constructId)) {
@@ -57,7 +57,7 @@ export class AssociationManager {
   }
 
   public static notifyOnStackEvents(portfolio: IPortfolio, product: IProduct, topic: sns.ITopic, options: CommonConstraintOptions): void {
-    const association = this.associateProductWithPortfolio(portfolio, product);
+    const association = this.associateProductWithPortfolio(portfolio, product, options);
     const constructId = `LaunchNotificationConstraint${hashValues(topic.node.addr, topic.stack.node.addr, association.associationKey)}`;
 
     if (!portfolio.node.tryFindChild(constructId)) {
@@ -78,9 +78,9 @@ export class AssociationManager {
 
   public static constrainCloudFormationParameters(
     portfolio: IPortfolio, product: IProduct,
-    options: ProvisioningRuleOptions,
+    options: CloudFormationRuleConstraintOptions,
   ): void {
-    const association = this.associateProductWithPortfolio(portfolio, product);
+    const association = this.associateProductWithPortfolio(portfolio, product, options);
     const constructId = `LaunchTemplateConstraint${hashValues(association.associationKey, options.rule.ruleName)}`;
 
     if (!portfolio.node.tryFindChild(constructId)) {
@@ -100,7 +100,7 @@ export class AssociationManager {
   }
 
   public static setLaunchRole(portfolio: IPortfolio, product: IProduct, launchRole: iam.IRole, options: CommonConstraintOptions): void {
-    const association = this.associateProductWithPortfolio(portfolio, product);
+    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)}`);
@@ -124,7 +124,7 @@ export class AssociationManager {
   }
 
   public static deployWithStackSets(portfolio: IPortfolio, product: IProduct, options: StackSetsConstraintOptions) {
-    const association = this.associateProductWithPortfolio(portfolio, product);
+    const association = this.associateProductWithPortfolio(portfolio, product, options);
     // Check if a launch role has already been set.
     if (portfolio.node.tryFindChild(this.launchRoleConstraintLogicalId(association.associationKey))) {
       throw new Error(`Cannot configure StackSet deployment when a launch role is already defined for association ${this.prettyPrintAssociation(portfolio, product)}`);
@@ -209,7 +209,7 @@ export class AssociationManager {
         AssertDescription: assertion.description,
       });
       return formattedAssertions;
-    }, Array<{ Assert: string, AssertDescription: string | undefined }>());
+    }, new Array<{ Assert: string, AssertDescription: string | undefined }>());
   };
 }
 

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

@@ -83,7 +83,9 @@ portfolio.constrainCloudFormationParameters(product, {
   rule: {
     ruleName: 'SubnetsinVPC',
     assertions: [{
-      assert: cdk.Fn.conditionEachMemberIn(cdk.Fn.valueOfAll('AWs::EC2::Subnet::Id', 'VpcId'), cdk.Fn.refAll('AWS::EC2::VPC::Id')),
+      assert: cdk.Fn.conditionEachMemberIn(
+        cdk.Fn.valueOfAll('AWs::EC2::Subnet::Id', 'VpcId'),
+        cdk.Fn.refAll('AWS::EC2::VPC::Id')),
       description: 'test description',
     }],
   },

packages/@aws-cdk/aws-servicecatalog/test/portfolio.test.ts

@@ -459,7 +459,7 @@ describe('portfolio associations and product constraints', () => {
     }).toThrowError(`Topic ${topic} is already subscribed to association`);
   }),
 
-  test('set provisioning rule', () => {
+  test('creates a CloudFormation parameters constraint', () => {
     portfolio.addProduct(product);
     portfolio.constrainCloudFormationParameters(product, {
       rule: {
@@ -489,7 +489,7 @@ describe('portfolio associations and product constraints', () => {
     });
   }),
 
-  test('set provisioning rule still creates without explicit association', () => {
+  test('CloudFormation parameters constraint still creates without explicit association', () => {
     portfolio.constrainCloudFormationParameters(product, {
       rule: {
         ruleName: 'Rule',
@@ -508,7 +508,7 @@ describe('portfolio associations and product constraints', () => {
     expect(stack).toHaveResourceLike('AWS::ServiceCatalog::LaunchTemplateConstraint');
   }),
 
-  test('set multiple provisioning rules', () => {
+  test('set multiple CloudFormation parameters constraints', () => {
     portfolio.constrainCloudFormationParameters(product, {
       rule: {
         ruleName: 'Rule01',
@@ -532,7 +532,7 @@ describe('portfolio associations and product constraints', () => {
     expect(stack).toCountResources('AWS::ServiceCatalog::LaunchTemplateConstraint', 2);
   }),
 
-  test('fails to set a duplicate provisioning rule', () => {
+  test('fails to set a duplicate CloudFormation parameters constraint', () => {
     portfolio.constrainCloudFormationParameters(product, {
       rule: {
         ruleName: 'Rule01',

packages/@aws-cdk/core/lib/cfn-condition.ts

@@ -94,7 +94,8 @@ export interface ICfnConditionExpression extends IResolvable {}
  */
 export interface ICfnRuleConditionExpression extends ICfnConditionExpression {
   /**
-   * This field is here for typing service catalog rule specific intrinsic functions.
+   * This field is only needed to defeat TypeScript's structural typing.
+   * It is never used.
    */
   readonly disambiguator: boolean;
 }

packages/@aws-cdk/core/lib/cfn-fn.ts

@@ -605,7 +605,7 @@ class FnCidr extends FnBase {
 }
 
 class FnConditionBase extends Intrinsic implements ICfnRuleConditionExpression {
-  readonly disambiguator: boolean = true;
+  readonly disambiguator = true;
   constructor(type: string, value: any) {
     super({ [type]: value });
   }