From 5f389e970c68da94cd208fe881047a6364a68a0d Mon Sep 17 00:00:00 2001
From: Adam Ruka <adamruka@amazon.com>
Date: Wed, 31 Mar 2021 02:45:56 -0700
Subject: [PATCH] fix(dynamodb): table with replicas fails to deploy with
 "Unresolved resource dependencies" error (#13889)

When creating the Custom Resources that implement the global tables functionality,
we add dependencies between them, as you can't create replicas of the same Table concurrently.
However, if the Stack the Table is part of is env-agnostic,
we also add a CFN Condition to the Custom Resource that checks whether the given region is the deployed-to region,
and skip creating the replica in that case (as the Table itself acts as the replica in this case).
But that Condition is not compatible with the dependency clause,
as the resource will not exist if the Condition is false.

Use a trick, and instead of using a DependsOn,
add a CFN metadata that refers to the other Custom Resource through a Ref expression,
which adds an implicit dependency,
and wrap the entire Metadata in a Fn::If,
guarded by the same Condition the other Custom Resource uses.

Noticed by a customer in https://github.com/aws/aws-cdk/issues/13671#issuecomment-810538273.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 packages/@aws-cdk/aws-dynamodb/lib/table.ts   | 23 +++++++++++++++----
 ....global-replicas-provisioned.expected.json | 14 ++++++++++-
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/packages/@aws-cdk/aws-dynamodb/lib/table.ts b/packages/@aws-cdk/aws-dynamodb/lib/table.ts
index a958bf135546e..ed8f6a786d03c 100644
--- a/packages/@aws-cdk/aws-dynamodb/lib/table.ts
+++ b/packages/@aws-cdk/aws-dynamodb/lib/table.ts
@@ -3,7 +3,7 @@ import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
 import * as iam from '@aws-cdk/aws-iam';
 import * as kms from '@aws-cdk/aws-kms';
 import {
-  Aws, CfnCondition, CfnCustomResource, CustomResource, Duration,
+  Aws, CfnCondition, CfnCustomResource, CfnResource, CustomResource, Duration,
   Fn, IResource, Lazy, Names, RemovalPolicy, Resource, Stack, Token,
 } from '@aws-cdk/core';
 import { Construct } from 'constructs';
@@ -1477,7 +1477,8 @@ export class Table extends TableBase {
     this.grant(onEventHandlerPolicy, 'dynamodb:*');
     this.grant(isCompleteHandlerPolicy, 'dynamodb:DescribeTable');
 
-    let previousRegion;
+    let previousRegion: CustomResource | undefined;
+    let previousRegionCondition: CfnCondition | undefined;
     for (const region of new Set(regions)) { // Remove duplicates
       // Use multiple custom resources because multiple create/delete
       // updates cannot be combined in a single API call.
@@ -1498,8 +1499,9 @@ export class Table extends TableBase {
       // Deploy time check to prevent from creating a replica in the region
       // where this stack is deployed. Only needed for environment agnostic
       // stacks.
+      let createReplica: CfnCondition | undefined;
       if (Token.isUnresolved(stack.region)) {
-        const createReplica = new CfnCondition(this, `StackRegionNotEquals${region}`, {
+        createReplica = new CfnCondition(this, `StackRegionNotEquals${region}`, {
           expression: Fn.conditionNot(Fn.conditionEquals(region, Aws.REGION)),
         });
         const cfnCustomResource = currentRegion.node.defaultChild as CfnCustomResource;
@@ -1518,9 +1520,22 @@ export class Table extends TableBase {
       // have multiple table updates at the same time. The `isCompleteHandler`
       // of the provider waits until the replica is in an ACTIVE state.
       if (previousRegion) {
-        currentRegion.node.addDependency(previousRegion);
+        if (previousRegionCondition) {
+          // we can't simply use a Dependency,
+          // because the previousRegion is protected by the "different region" Condition,
+          // and you can't have Fn::If in DependsOn.
+          // Instead, rely on Ref adding a dependency implicitly!
+          const previousRegionCfnResource = previousRegion.node.defaultChild as CfnResource;
+          const currentRegionCfnResource = currentRegion.node.defaultChild as CfnResource;
+          currentRegionCfnResource.addMetadata('DynamoDbReplicationDependency',
+            Fn.conditionIf(previousRegionCondition.logicalId, previousRegionCfnResource.ref, Aws.NO_VALUE));
+        } else {
+          currentRegion.node.addDependency(previousRegion);
+        }
       }
+
       previousRegion = currentRegion;
+      previousRegionCondition = createReplica;
     }
 
     // Permissions in the destination regions (outside of the loop to
diff --git a/packages/@aws-cdk/aws-dynamodb/test/integ.global-replicas-provisioned.expected.json b/packages/@aws-cdk/aws-dynamodb/test/integ.global-replicas-provisioned.expected.json
index af1e8defceed6..ce43b532ea7c6 100644
--- a/packages/@aws-cdk/aws-dynamodb/test/integ.global-replicas-provisioned.expected.json
+++ b/packages/@aws-cdk/aws-dynamodb/test/integ.global-replicas-provisioned.expected.json
@@ -199,7 +199,6 @@
         "Region": "eu-west-3"
       },
       "DependsOn": [
-        "TableReplicauseast28A15C236",
         "TableSourceTableAttachedManagedPolicyawscdkdynamodbglobalreplicasprovisionedawscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRoleBE2B1C1A5DC546D2",
         "TableSourceTableAttachedManagedPolicyawscdkdynamodbglobalreplicasprovisionedawscdkawsdynamodbReplicaProviderOnEventHandlerServiceRoleD9856B771F8F2CCB",
         "TableWriteScalingTargetE5669214",
@@ -207,6 +206,19 @@
       ],
       "UpdateReplacePolicy": "Delete",
       "DeletionPolicy": "Delete",
+      "Metadata": {
+        "DynamoDbReplicationDependency": {
+          "Fn::If": [
+            "TableStackRegionNotEqualsuseast2D20A1E77",
+            {
+              "Ref": "TableReplicauseast28A15C236"
+            },
+            {
+              "Ref": "AWS::NoValue"
+            }
+          ]
+        }
+      },
       "Condition": "TableStackRegionNotEqualseuwest302B3591C"
     },
     "TableWriteScalingTargetE5669214": {