Skip to content

Commit

Permalink
fix(dynamodb): table with replicas fails to deploy with "Unresolved r…
Browse files Browse the repository at this point in the history
…esource 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 #13671 (comment).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 authored Mar 31, 2021
1 parent df5c133 commit 5c99d0d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
23 changes: 19 additions & 4 deletions packages/@aws-cdk/aws-dynamodb/lib/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,26 @@
"Region": "eu-west-3"
},
"DependsOn": [
"TableReplicauseast28A15C236",
"TableSourceTableAttachedManagedPolicyawscdkdynamodbglobalreplicasprovisionedawscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRoleBE2B1C1A5DC546D2",
"TableSourceTableAttachedManagedPolicyawscdkdynamodbglobalreplicasprovisionedawscdkawsdynamodbReplicaProviderOnEventHandlerServiceRoleD9856B771F8F2CCB",
"TableWriteScalingTargetE5669214",
"TableWriteScalingTargetTrackingD78DCCD8"
],
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete",
"Metadata": {
"DynamoDbReplicationDependency": {
"Fn::If": [
"TableStackRegionNotEqualsuseast2D20A1E77",
{
"Ref": "TableReplicauseast28A15C236"
},
{
"Ref": "AWS::NoValue"
}
]
}
},
"Condition": "TableStackRegionNotEqualseuwest302B3591C"
},
"TableWriteScalingTargetE5669214": {
Expand Down

0 comments on commit 5c99d0d

Please sign in to comment.