Merge branch 'master' into peterb154/#12016-2-domains-2-stacks

CONTRIBUTING.md

@@ -382,6 +382,31 @@ Here are a few useful commands:
    evaluate only the rule specified [awslint README](./packages/awslint/README.md)
    for details on include/exclude rule patterns.
 
+
+#### jsii-rosetta
+
+**jsii-rosetta** can be used to verify that all code examples included in documentation for a package (including those
+in `README.md`) successfully compile against the library they document. It is recommended to run it to ensure all
+examples are still accurate. Successfully building examples is also necessary to ensure the best possible translation to
+other supported languages (`C#`, `Java`, `Python`, ...).
+
+> Note that examples may use libraries that are not part of the `dependencies` or `devDependencies` of the documented
+> package. For example `@aws-cdk/core` contains mainy examples that leverage libraries built *on top of it* (such as
+> `@aws-cdk/aws-sns`). Such libraries must be built (using `yarn build`) before **jsii-rosetta** can verify that
+> examples are correct.
+
+To run **jsii-rosetta** in *strict* mode (so that it always fails when encountering examples that fail to compile), use
+the following command:
+
+```console
+$ yarn rosetta:extract --strict
+```
+
+For more information on how you can address examples that fail compiling due to missing fixtures (declarations that are
+necessary for the example to compile, but which would distract the reader away from what is being demonstrated), you
+might need to introduce [rosetta fixtures](https://github.com/aws/jsii/tree/main/packages/jsii-rosetta#fixtures). Refer
+to the [Examples](#examples) section.
+
 ### cfn2ts
 
 This tool is used to generate our low-level CloudFormation resources
@@ -685,6 +710,8 @@ can be used in these cases.
 
 ### Examples
 
+#### Fixture Files
+
 Examples typed in fenced code blocks (looking like `'''ts`, but then with backticks
 instead of regular quotes) will be automatically extrated, compiled and translated
 to other languages when the bindings are generated.
@@ -694,7 +721,7 @@ a *fixture*, which looks like this:
 
 ```
 '''ts fixture=with-bucket
-bucket.addLifecycleTransition({ ... });
+bucket.addLifecycleTransition({ ...props });
 '''
 ```
 
@@ -717,8 +744,8 @@ contain three slashes to achieve the same effect:
 ```
 /**
  * @example
- * /// fixture=with-bucket
- * bucket.addLifecycleTransition({ ... });
+ *   /// fixture=with-bucket
+ *   bucket.addLifecycleTransition({ ...props });
  */
 ```
 
@@ -732,12 +759,52 @@ the current package.
 For a practical example of how making sample code compilable works, see the
 `aws-ec2` package.
 
+#### Recommendations
+
+In order to offer a consistent documentation style throughout the AWS CDK
+codebase, example code should follow the following recommendations (there may be
+cases where some of those do not apply - good judgement is to be applied):
+
+- Types from the documented module should be **un-qualified**
+
+  ```ts
+  // An example in the @aws-cdk/core library, which defines Duration
+  Duration.minutes(15);
+  ```
+
+- Types from other modules should be **qualified**
+
+  ```ts
+  // An example in the @aws-cdk/core library, using something from @aws-cdk/aws-s3
+  const bucket = new s3.Bucket(this, 'Bucket');
+  // ...rest of the example...
+  ```
+
+- Within `.ts-fixture` files, make use of `declare` statements instead of
+  writing a compatible value (this will make your fixtures more durable):
+
+  ```ts
+  // An hypothetical 'rosetta/default.ts-fixture' file in `@aws-cdk/core`
+  import * as kms from '@aws-cdk/aws-kms';
+  import * as s3 from '@aws-cdk/aws-s3';
+  import { StackProps } from '@aws-cdk/core';
+
+  declare const kmsKey: kms.IKey;
+  declare const bucket: s3.Bucket;
+
+  declare const props: StackProps;
+  ```
+
+> Those recommendations are not verified or enforced by automated tooling. Pull
+> request reviewers may however request that new sample code is edited to meet
+> those requirements as needed.
+
+#### Checking a single package
+
 Examples of all packages are extracted and compiled as part of the packaging
 step. If you are working on getting rid of example compilation errors of a
-single package, you can run `scripts/compile-samples` on the package by itself.
-
-For now, non-compiling examples will not yet block the build, but at some point
-in the future they will.
+single package, you can run `yarn rosetta:extract --strict` in the package's
+directory (see the [**jsii-rosetta**](#jsii-rosetta) section).
 
 ### Feature Flags
 

packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts

@@ -1351,6 +1351,45 @@ test('Can set autoScalingGroupName', () => {
   }));
 });
 
+test('can use Vpc imported from unparseable list tokens', () => {
+  // GIVEN
+  const stack = new cdk.Stack();
+
+  const vpcId = cdk.Fn.importValue('myVpcId');
+  const availabilityZones = cdk.Fn.split(',', cdk.Fn.importValue('myAvailabilityZones'));
+  const publicSubnetIds = cdk.Fn.split(',', cdk.Fn.importValue('myPublicSubnetIds'));
+  const privateSubnetIds = cdk.Fn.split(',', cdk.Fn.importValue('myPrivateSubnetIds'));
+  const isolatedSubnetIds = cdk.Fn.split(',', cdk.Fn.importValue('myIsolatedSubnetIds'));
+
+  const vpc = ec2.Vpc.fromVpcAttributes(stack, 'importedVpc', {
+    vpcId,
+    availabilityZones,
+    publicSubnetIds,
+    privateSubnetIds,
+    isolatedSubnetIds,
+  });
+
+  // WHEN
+  new autoscaling.AutoScalingGroup(stack, 'ecs-ec2-asg', {
+    instanceType: new ec2.InstanceType('t2.micro'),
+    machineImage: new ec2.AmazonLinuxImage(),
+    minCapacity: 1,
+    maxCapacity: 1,
+    desiredCapacity: 1,
+    vpc,
+    allowAllOutbound: false,
+    associatePublicIpAddress: false,
+    vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE },
+  });
+
+  // THEN
+  expect(stack).to(haveResourceLike('AWS::AutoScaling::AutoScalingGroup', {
+    VPCZoneIdentifier: {
+      'Fn::Split': [',', { 'Fn::ImportValue': 'myPrivateSubnetIds' }],
+    },
+  }));
+});
+
 function mockSecurityGroup(stack: cdk.Stack) {
   return ec2.SecurityGroup.fromSecurityGroupId(stack, 'MySG', 'most-secure');
 }

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

@@ -405,10 +405,6 @@ export class InterfaceVpcEndpoint extends VpcEndpoint implements IInterfaceVpcEn
    * Imports an existing interface VPC endpoint.
    */
   public static fromInterfaceVpcEndpointAttributes(scope: Construct, id: string, attrs: InterfaceVpcEndpointAttributes): IInterfaceVpcEndpoint {
-    if (!attrs.securityGroups && !attrs.securityGroupId) {
-      throw new Error('Either `securityGroups` or `securityGroupId` must be specified.');
-    }
-
     const securityGroups = attrs.securityGroupId
       ? [SecurityGroup.fromSecurityGroupId(scope, 'SecurityGroup', attrs.securityGroupId)]
       : attrs.securityGroups;

packages/@aws-cdk/aws-ec2/lib/vpc.ts

@@ -1010,7 +1010,16 @@ export class Vpc extends VpcBase {
   ];
 
   /**
-   * Import an exported VPC
+   * Import a VPC by supplying all attributes directly
+   *
+   * NOTE: using `fromVpcAttributes()` with deploy-time parameters (like a `Fn.importValue()` or
+   * `CfnParameter` to represent a list of subnet IDs) sometimes accidentally works. It happens
+   * to work for constructs that need a list of subnets (like `AutoScalingGroup` and `eks.Cluster`)
+   * but it does not work for constructs that need individual subnets (like
+   * `Instance`). See https://github.com/aws/aws-cdk/issues/4118 for more
+   * information.
+   *
+   * Prefer to use `Vpc.fromLookup()` instead.
    */
   public static fromVpcAttributes(scope: Construct, id: string, attrs: VpcAttributes): IVpc {
     return new ImportedVpc(scope, id, attrs, false);
@@ -1927,7 +1936,21 @@ class ImportedSubnet extends Resource implements ISubnet, IPublicSubnet, IPrivat
     super(scope, id);
 
     if (!attrs.routeTableId) {
-      const ref = Token.isUnresolved(attrs.subnetId)
+      // The following looks a little weird, but comes down to:
+      //
+      // * Is the subnetId itself unresolved ({ Ref: Subnet }); or
+      // * Was it the accidentally extracted first element of a list-encoded
+      //   token? ({ Fn::ImportValue: Subnets } => ['#{Token[1234]}'] =>
+      //   '#{Token[1234]}'
+      //
+      // There's no other API to test for the second case than to the string back into
+      // a list and see if the combination is Unresolved.
+      //
+      // In both cases we can't output the subnetId literally into the metadata (because it'll
+      // be useless). In the 2nd case even, if we output it to metadata, the `resolve()` call
+      // that gets done on the metadata will even `throw`, because the '#{Token}' value will
+      // occur in an illegal position (not in a list context).
+      const ref = Token.isUnresolved(attrs.subnetId) || Token.isUnresolved([attrs.subnetId])
         ? `at '${Node.of(scope).path}/${id}'`
         : `'${attrs.subnetId}'`;
       // eslint-disable-next-line max-len

packages/@aws-cdk/aws-ec2/test/vpc-endpoint.test.ts

@@ -290,6 +290,24 @@ nodeunitShim({
       test.done();
     },
 
+    'import/export without security group'(test: Test) {
+      // GIVEN
+      const stack2 = new Stack();
+
+      // WHEN
+      const importedEndpoint = InterfaceVpcEndpoint.fromInterfaceVpcEndpointAttributes(stack2, 'ImportedEndpoint', {
+        vpcEndpointId: 'vpc-endpoint-id',
+        port: 80,
+      });
+      importedEndpoint.connections.allowDefaultPortFromAnyIpv4();
+
+      // THEN
+      test.deepEqual(importedEndpoint.vpcEndpointId, 'vpc-endpoint-id');
+      test.deepEqual(importedEndpoint.connections.securityGroups.length, 0);
+
+      test.done();
+    },
+
     'with existing security groups'(test: Test) {
       // GIVEN
       const stack = new Stack();

packages/@aws-cdk/aws-ec2/test/vpc.test.ts

@@ -1,5 +1,5 @@
 import { countResources, expect, haveResource, haveResourceLike, isSuperObject, MatchStyle } from '@aws-cdk/assert';
-import { CfnOutput, Lazy, Stack, Tags } from '@aws-cdk/core';
+import { CfnOutput, CfnResource, Fn, Lazy, Stack, Tags } from '@aws-cdk/core';
 import { nodeunitShim, Test } from 'nodeunit-shim';
 import {
   AclCidr, AclTraffic, BastionHostLinux, CfnSubnet, CfnVPC, SubnetFilter, DefaultInstanceTenancy, GenericLinuxImage,
@@ -1227,6 +1227,36 @@ nodeunitShim({
       test.done();
     },
 
+    'fromVpcAttributes using imported refs'(test: Test) {
+      // GIVEN
+      const stack = getTestStack();
+
+      const vpcId = Fn.importValue('myVpcId');
+      const availabilityZones = Fn.split(',', Fn.importValue('myAvailabilityZones'));
+      const publicSubnetIds = Fn.split(',', Fn.importValue('myPublicSubnetIds'));
+
+      // WHEN
+      const vpc = Vpc.fromVpcAttributes(stack, 'VPC', {
+        vpcId,
+        availabilityZones,
+        publicSubnetIds,
+      });
+
+      new CfnResource(stack, 'Resource', {
+        type: 'Some::Resource',
+        properties: {
+          subnetIds: vpc.selectSubnets().subnetIds,
+        },
+      });
+
+      // THEN - No exception
+      expect(stack).to(haveResource('Some::Resource', {
+        subnetIds: { 'Fn::Split': [',', { 'Fn::ImportValue': 'myPublicSubnetIds' }] },
+      }));
+
+      test.done();
+    },
+
     'select explicitly defined subnets'(test: Test) {
       // GIVEN
       const stack = getTestStack();