Merge pull request #14 from aws-samples/amalema/wfh-comments

Comments on WorkingFromHome CDK code

WorkingFromHome/ClientConnection/AdConnectorCustomResource/src/main/java/com/ilmlf/adconnector/customresource/IsCompleteHandler.java

@@ -37,6 +37,11 @@ public HashMap<String, Object> handleRequest(
 
     HashMap<String, Object> response = new HashMap();
 
+    /*
+      Check for the completion of OnEventHandler actions requires that we
+        check the currently listed directories in the Directory Service,
+        hence fetch them here.
+    */
     String physicalResourceId = event.getPhysicalResourceId();
     List<DirectoryDescription> directoryDescriptions =
         DirectoryClient.builder()
@@ -49,8 +54,16 @@ public HashMap<String, Object> handleRequest(
     Map<String, String> data = Map.of("DirectoryId", physicalResourceId);
 
     Boolean isComplete;
+
+    /*
+      For deletion requests, check that the list of returned directories is empty.
+    */
     if (event.getRequestType().equals("Delete")) {
       isComplete = directoryDescriptions.isEmpty();
+    /*
+      For "Create" requests, check that the first (and only) directory in the list
+      has stage 'Active', i.e. it is an Active Directory directory.
+    */
     } else {
       isComplete = directoryDescriptions.get(0).stage().equals(DirectoryStage.ACTIVE);
     }

WorkingFromHome/ClientConnection/AdConnectorCustomResource/src/main/java/com/ilmlf/adconnector/customresource/OnEventHandler.java

@@ -44,6 +44,10 @@ public HashMap<String, Object> handleRequest(
     System.out.println("action:" + event.getRequestType() + " AD Connector");
     String requestType = event.getRequestType();
     switch (requestType) {
+      /*
+        For create requests, attempt to connect to the on-premise Active Directory
+          using the AWS Directory Service.
+      */
       case "Create":
         directoryId =
             directoryClient
@@ -64,6 +68,10 @@ public HashMap<String, Object> handleRequest(
         break;
       case "Update":
         break;
+      /*
+        For delete requests, remove the connection to the on-premise Active Directory within the
+          AWS Directory service
+      */
       case "Delete":
         directoryClient.deleteDirectory(
             DeleteDirectoryRequest.builder().directoryId(directoryId).build());

WorkingFromHome/ClientConnection/src/main/java/com/ilmlf/clientconnection/AdConnectorConstruct.java

@@ -81,20 +81,37 @@ public AdConnectorConstruct(
       throws IOException {
     super(scope, id);
 
+    /*
+      Java CDK custom resources need to be packaged for use
+      Packaging instructions are provided here
+    */
     List<String> adConnectorCustomResourcePackagingInstructions =
         Arrays.asList(
             "/bin/sh",
             "-c",
             "mvn clean install "
                 + "&& cp /asset-input/target/AdConnectorCustomResource.jar /asset-output/");
 
+    /*
+      The package builder is configured...
+      - using our previous packaging instructions
+      - to use a Java 11 run time
+    */
     BundlingOptions.Builder builderOptions =
         BundlingOptions.builder()
             .command(adConnectorCustomResourcePackagingInstructions)
             .image(Runtime.JAVA_11.getBundlingImage())
             .user("root")
             .outputType(ARCHIVED);
 
+    /*
+      The onEventHandler function makes API calls to create AWS resources
+      Here we specify:
+      - that our OnEventHandler.java is defined in the ./AdConnectorCustomResource directory
+      - that the resulting AWS Lambda function should:
+        - have a memory size of 1024 MB
+        - time out after 10 seconds
+    */
     Function onEventHandler =
         new Function(
             this,
@@ -109,8 +126,6 @@ public AdConnectorConstruct(
                             .assetHash(hashDirectory("./AdConnectorCustomResource/src", false))
                             .bundling(
                                 builderOptions
-                                    // TODO: add capability to use local bundling (.local) instead
-                                    // of docker one
                                     .command(adConnectorCustomResourcePackagingInstructions)
                                     .build())
                             .build()))
@@ -120,13 +135,22 @@ public AdConnectorConstruct(
                 .logRetention(RetentionDays.ONE_WEEK)
                 .build());
 
+    /*
+      Our OnEventHandler needs to access our DomainAdminPassword secret.
+      Hence, we grant the GetSecretValue permission to the lambda function.
+    */
     onEventHandler.addToRolePolicy(
         new PolicyStatement(
             PolicyStatementProps.builder()
                 .actions(singletonList("secretsmanager:GetSecretValue"))
                 .resources(Collections.singletonList(props.secretId))
                 .build()));
 
+    /*
+      Our OnEventHandler also needs to make various VPC API calls.
+      The VPC service is located under EC2, and hence most of the
+      permissions we grant are of the form ec2:*
+    */
     onEventHandler.addToRolePolicy(
         new PolicyStatement(
             PolicyStatementProps.builder()
@@ -146,6 +170,11 @@ public AdConnectorConstruct(
                 .resources(Collections.singletonList("*"))
                 .build()));
 
+    /*
+      The isCompleteHandler is a function that is periodically called to check
+      if the resource creation processes initiated in onEventHandler have completed.
+      Here we are defining it in a similar way to how we defined the OnEventHandler component.
+    */
     Function isCompleteHandler =
         new Function(
             this,
@@ -169,13 +198,21 @@ public AdConnectorConstruct(
                 .logRetention(RetentionDays.ONE_WEEK)
                 .build());
 
+    /*
+      The IsCompleteHandler checks if the Active Directory connection has
+      completed by checking if any Active Directories are listed in the Directory Service.
+      Hence we must grant the Directory Service's 'DescribeDirectories' permission.
+    */
     isCompleteHandler.addToRolePolicy(
         new PolicyStatement(
             PolicyStatementProps.builder()
                 .actions(singletonList("ds:DescribeDirectories"))
                 .resources(Collections.singletonList("*"))
                 .build()));
 
+    /*
+      The Provider associates the onEvent and isComplete handlers to the custom resource.
+    */
     Provider provider =
         new Provider(
             scope,
@@ -185,13 +222,24 @@ public AdConnectorConstruct(
                 .isCompleteHandler(isCompleteHandler)
                 .build());
 
+    /*
+      Define the properties that will be passed to our lambda handler functions.
+      - vpcId is the identifier of the VPC in which to create the AD Connector
+      - domainName is the Active Directory Domain Name
+      - dnsIps is a list of IP addresses for the DNS hosts of the on-premise infrastructure
+      - subnetIds are the identifiers of the subnets in which to place the AD connector
+      - secretId is the ID of the Secrets Manager 'DomainAdminPassword' secret
+    */
     TreeMap resourceProperties = new TreeMap();
     resourceProperties.put("vpcId", props.vpcId);
     resourceProperties.put("domainName", props.domainName);
     resourceProperties.put("dnsIps", props.dnsIps);
     resourceProperties.put("subnetIds", props.subnetIds);
     resourceProperties.put("secretId", props.secretId);
 
+    /*
+      Finally, create the CDK Custom Resource using all the previous parts we defined.
+    */
     CustomResource resource =
         new CustomResource(
             scope,

WorkingFromHome/ClientConnection/src/main/java/com/ilmlf/clientconnection/ClientConnectionApp.java

@@ -31,12 +31,15 @@ private ClientConnectionApp() {
   public static void main(final String[] args) throws IOException {
     App app = new App();
 
+    /*
+      Create the Client VPN stack in our specified account and region
+    */
     new ClientConnectionStack(app, "ClientConnectionStack", StackProps.builder()
         .env(Environment.builder()
             .account("433621526002")
             .region("eu-west-1")
             .build())
-        .description("VPN Clients Connection Stack (uksb-1rsq7ledl)")
+        .description("Client VPN Connection infrastructure Stack (uksb-1rsq7ledl)")
         .build());
 
     app.synth();

WorkingFromHome/ClientConnection/src/main/java/com/ilmlf/clientconnection/ClientConnectionStack.java

@@ -34,16 +34,29 @@ public ClientConnectionStack(final Construct scope, final String id, final Stack
       throws IOException {
     super(scope, id, props);
 
+    /*
+      Fetch values from the cdk.context.json (found at the root of ClientConnection folder)
+      - vpcId is the identifier of the AWS VPC in which to create the Client VPN and AD Connector
+      - domain is the domain name for the Active Directory
+      - dnsIps is the list of IP addresses of DNS hosts of our on-premise infrastructure
+    */
     String vpcId = this.getNode().tryGetContext("vpcId").toString();
     String domainName = this.getNode().tryGetContext("domain").toString();
     Object dnsIps = this.getNode().tryGetContext("dns");
 
+    /*
+      We initialise a VPC object using the passed-in VPC identifier.
+      This will allow us to perform CDK operations on it easier in the next steps.
+    */
     VpcLookupOptions vpcLookupOptions = new VpcLookupOptions.Builder()
         .vpcId(vpcId)
         .build();
-
     IVpc vpc = Vpc.fromLookup(this, "Vpc", vpcLookupOptions);
 
+    /*
+      Create a Client VPN setup using our own defined CDK construct (defined in ./ClientVpnConstruct.java)
+      We pass as props the values we fetched from cdk.context.json previously.
+    */
     new ClientVpnConstruct(this, "ClientVpn", ClientVpnConstruct.ClientVpnProps.builder()
         .vpc(vpc)
         .domainName(domainName)

WorkingFromHome/ClientConnection/src/main/java/com/ilmlf/clientconnection/ClientVpnConstruct.java

@@ -54,11 +54,23 @@ public ClientVpnConstruct(
       throws IOException {
     super(scope, id);
 
+    /*
+      Fetch values from the cdk.context.json (found at the root of ClientConnection folder)
+      - clientVpnCertificate is the Amazon Resource Name (ARN) of the Amazon Certificate Manager
+        certificate that is used to authenticate connections to the Client VPN.
+      - clientVpnCidr is the VPC CIDR range that can be reached through the Client VPN
+      - onPremiseCidr is the CIDR range of the on-premise resources
+      - DomainAdminSecretArn is the ARN of the Secrets Manager 'DomainAdminPassword' secret
+    */
     String serverCertificateArn = this.getNode().tryGetContext("clientVpnCertificate").toString();
     String cidr = this.getNode().tryGetContext("clientVpnCidr").toString();
     String onPremiseCidr = this.getNode().tryGetContext("onPremiseCidr").toString();
     String secretId = this.getNode().tryGetContext("DomainAdminSecretArn").toString();
 
+    /*
+      Create the AD Connector using our own CDK Custom Resource.
+      This requires that we pass in property values that we have already fetched.
+    */
     AdConnectorConstruct adConnector =
         new AdConnectorConstruct(
             this,
@@ -74,6 +86,11 @@ public ClientVpnConstruct(
                 .secretId(secretId)
                 .build());
 
+    /*
+      The Client VPN is created using a method on the VPC object that we passed in as a property.
+      We specify the authentication certificate, and that users must authenticate through the
+      active directory connection that we created with the AD Connector Construct.
+    */
     ClientVpnEndpoint clientVpn =
         props.vpc.addClientVpnEndpoint(
             "VpnClientEndpoint",
@@ -86,8 +103,10 @@ public ClientVpnConstruct(
                 .splitTunnel(true)
                 .build());
 
-    props
-        .vpc
+    /*
+      For each of the private subnets in the VPC, we must add a route between it and the Client VPN.
+    */
+    props.vpc
         .getPrivateSubnets()
         .forEach(
             (subnet) ->

WorkingFromHome/ClientConnection/src/test/java/com/ilmlf/clientconnection/ClientConnectionTest.snap

@@ -28,16 +28,16 @@ com.ilmlf.clientconnection.ClientConnectionTest.testStack=[
         "Description": "S3 key for asset version \"67b7823b74bc135986aa72f889d6a8da058d0c4a20cbc2dfc6f78995fdd2fc24\"",
         "Type": "String"
       },
-      "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fArtifactHash0944B33B": {
-        "Description": "Artifact hash for asset \"770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9f\"",
+      "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82ArtifactHash1D0BFBA7": {
+        "Description": "Artifact hash for asset \"9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82\"",
         "Type": "String"
       },
-      "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3Bucket51003488": {
-        "Description": "S3 bucket for asset \"770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9f\"",
+      "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3BucketEF4FFE05": {
+        "Description": "S3 bucket for asset \"9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82\"",
         "Type": "String"
       },
-      "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3VersionKey6DFEA61E": {
-        "Description": "S3 key for asset version \"770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9f\"",
+      "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3VersionKey8CA45E81": {
+        "Description": "S3 key for asset version \"9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82\"",
         "Type": "String"
       },
       "AssetParametersc691172cdeefa2c91b5a2907f9d81118e47597634943344795f1a844192dd49cArtifactHash627DAAA7": {
@@ -86,7 +86,7 @@ com.ilmlf.clientconnection.ClientConnectionTest.testStack=[
         "Properties": {
           "Code": {
             "S3Bucket": {
-              "Ref": "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3Bucket51003488"
+              "Ref": "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3BucketEF4FFE05"
             },
             "S3Key": {
               "Fn::Join": [
@@ -99,7 +99,7 @@ com.ilmlf.clientconnection.ClientConnectionTest.testStack=[
                         "Fn::Split": [
                           "||",
                           {
-                            "Ref": "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3VersionKey6DFEA61E"
+                            "Ref": "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3VersionKey8CA45E81"
                           }
                         ]
                       }
@@ -112,7 +112,7 @@ com.ilmlf.clientconnection.ClientConnectionTest.testStack=[
                         "Fn::Split": [
                           "||",
                           {
-                            "Ref": "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3VersionKey6DFEA61E"
+                            "Ref": "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3VersionKey8CA45E81"
                           }
                         ]
                       }
@@ -218,7 +218,7 @@ com.ilmlf.clientconnection.ClientConnectionTest.testStack=[
         "Properties": {
           "Code": {
             "S3Bucket": {
-              "Ref": "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3Bucket51003488"
+              "Ref": "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3BucketEF4FFE05"
             },
             "S3Key": {
               "Fn::Join": [
@@ -231,7 +231,7 @@ com.ilmlf.clientconnection.ClientConnectionTest.testStack=[
                         "Fn::Split": [
                           "||",
                           {
-                            "Ref": "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3VersionKey6DFEA61E"
+                            "Ref": "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3VersionKey8CA45E81"
                           }
                         ]
                       }
@@ -244,7 +244,7 @@ com.ilmlf.clientconnection.ClientConnectionTest.testStack=[
                         "Fn::Split": [
                           "||",
                           {
-                            "Ref": "AssetParameters770523fe8cda2b705b21def7a947b03a454811e05e040e6d776a351d646ece9fS3VersionKey6DFEA61E"
+                            "Ref": "AssetParameters9928b84f27c47952d9e87dd4122e8367b71e66c90243006b113be2ea7c38cc82S3VersionKey8CA45E81"
                           }
                         ]
                       }

WorkingFromHome/SiteToSiteConnection/src/main/java/com/ilmlf/sitetositeconnection/SiteToSiteConnectionApp.java

@@ -17,16 +17,27 @@
 import software.amazon.awscdk.core.Environment;
 import software.amazon.awscdk.core.StackProps;
 
-public class SiteToSiteConnectionApp {
+public final class SiteToSiteConnectionApp {
+  private SiteToSiteConnectionApp() {
+    throw new IllegalStateException("Entrypoint class");
+  }
+
+  /**
+   * CDK Entrypoint.
+   * @param args for cdk
+   */
   public static void main(final String[] args) {
     App app = new App();
 
+    /*
+      Create the Site-to-Site VPN stack in our specified account and region
+    */
     new SiteToSiteConnectionStack(
         app,
         "SiteToSiteConnectionStack",
         StackProps.builder()
             .env(Environment.builder().account("433621526002").region("eu-west-1").build())
-            .description("Site to Site Base infra Stack (uksb-1rsq7leb5)")
+            .description("Site-to-Site VPN infrastructure Stack (uksb-1rsq7leb5)")
             .build());
     app.synth();
   }