fix(codepipeline): x-env ECS deployment lacking support stack-dependency (#24053)

In the case of a cross-account AND cross-region ECS deployment, a dependency between the support stacks that is necessary is missing.

This cannot be fixed globally. Because the resources reference each other bidirectionally (user -> bucket, bucket -> user), the only way to fix this is to do it locally: in the CodePipeline module, where we can have the knowledge that we use generated names and that everything will work out if we deploy the role before the bucket.

All CodePipeline Actions must have this fix eventually, but since people may have crazy stack setups in which addition of this dependency may introduce a cyclic dependency (breaking the synth), we're rolling this fix out with limited blast radius.

Follow-up in #24050, and suggest a good clean-up in #24051.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/ecs-deploy-action.ts

@@ -4,6 +4,7 @@ import * as iam from '@aws-cdk/aws-iam';
 import { Lazy } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { Action } from '../action';
+import { forceSupportStackDependency } from '../private/stack-dependency';
 
 /**
  * Configuration for replacing a placeholder string in the ECS task
@@ -177,6 +178,7 @@ export class CodeDeployEcsDeployAction extends Action {
 
     // the Action's Role needs to read from the Bucket to get artifacts
     options.bucket.grantRead(options.role);
+    forceSupportStackDependency(options.bucket, options.role);
 
     const taskDefinitionTemplateArtifact = determineTaskDefinitionArtifact(this.actionProps);
     const appSpecTemplateArtifact = determineAppSpecArtifact(this.actionProps);

packages/@aws-cdk/aws-codepipeline-actions/lib/private/stack-dependency.ts

@@ -0,0 +1,27 @@
+import * as iam from '@aws-cdk/aws-iam';
+import * as s3 from '@aws-cdk/aws-s3';
+import { Resource, Stack } from '@aws-cdk/core';
+
+/**
+ * Create a dependency between the stack of the replication bucket and the stack of the action role
+ *
+ * If the deployment action happens in across-account/cross-region fashion, we
+ * create two support stacks (stack R for the cross-account role, and stack B for the
+ * cross-region replication bucket), but these stacks are not related to each
+ * other by default.
+ *
+ * To make it more interesting, if these are roles with autogenerated names, the
+ * stacks have bidirectional policies: the bucket and key (B) refer to the role
+ * (R), and the role (R) refers to the bucket and key (B). This is an
+ * unfortunate way of setting up the policies, and it should really be
+ * completely replaced with a tag-based mechanism.
+ *
+ * Until then, we've determined that deployment accidentally works fine if we deploy
+ * the account stack R first, followed by the region stack B. So explicitly establish
+ * this dependency in CodePipeline Actions.
+ */
+export function forceSupportStackDependency(bucket: s3.IBucket, role: iam.IRole) {
+  if (Resource.isOwnedResource(bucket) && Resource.isOwnedResource(role)) {
+    Stack.of(bucket).addDependency(Stack.of(role), `replication bucket {${bucket.node.path}} to action role {${role}}`);
+  }
+}

packages/@aws-cdk/aws-codepipeline-actions/test/codedeploy/ecs-deploy-action.test.ts

@@ -197,6 +197,38 @@ describe('CodeDeploy ECS Deploy Action', () => {
 
     });
   });
+
+  test('cross-account cross-region deployment has correct dependency between support stacks', () => {
+    // GIVEN
+    const stackEnv: cdk.Environment = { account: '111111111111', region: 'us-east-1' };
+    const deployEnv: cdk.Environment = { account: '222222222222', region: 'us-east-2' };
+
+    const app = new cdk.App();
+    const stack = new cdk.Stack(app, 'Pipe', { env: stackEnv });
+    const deploymentGroup = codedeploy.EcsDeploymentGroup.fromEcsDeploymentGroupAttributes(stack, 'Group', {
+      application: codedeploy.EcsApplication.fromEcsApplicationArn(stack, 'Application',
+        `arn:aws:codedeploy:${deployEnv.region}:${deployEnv.account}:application:MyApplication`),
+      deploymentGroupName: 'MyGroup',
+    });
+
+    // WHEN
+    addCodeDeployECSCodePipeline(stack, {
+      actionName: 'DeployECS',
+      deploymentGroup,
+      taskDefinitionTemplateInput: new codepipeline.Artifact('Artifact'),
+      appSpecTemplateInput: new codepipeline.Artifact('Artifact2'),
+    });
+
+    // THEN - dependency from region stack to account stack
+    // (region stack has bucket, account stack has role)
+    const asm = app.synth();
+
+    const stacks = Object.fromEntries(asm.stacks.map(s => [s.stackName, s]));
+    expect(Object.keys(stacks)).toContain('Pipe-support-us-east-2');
+    expect(Object.keys(stacks)).toContain('Pipe-support-222222222222');
+
+    expect(stacks['Pipe-support-us-east-2'].dependencies).toContain(stacks['Pipe-support-222222222222']);
+  });
 });
 
 function addEcsDeploymentGroup(stack: cdk.Stack): codedeploy.IEcsDeploymentGroup {

packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.healthchecks-multiple-application-load-balanced-ecs-service.ts

@@ -67,12 +67,6 @@ applicationMultipleTargetGroupsFargateService.targetGroups[1].configureHealthChe
   healthyHttpCodes: '200',
 });
 
-applicationMultipleTargetGroupsFargateService.loadBalancers[0]._enableCrossEnvironment;
-applicationMultipleTargetGroupsFargateService.loadBalancers[1]._enableCrossEnvironment;
-
-applicationMultipleTargetGroupsFargateService.listeners[0].listenerArn;
-applicationMultipleTargetGroupsFargateService.listeners[1].listenerArn;
-
 new IntegTest(app, 'Integ', { testCases: [stack] });
 
 app.synth();

packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.healthchecks-multiple-network-load-balanced-ecs-service.ts

@@ -50,9 +50,6 @@ networkMultipleTargetGroupsFargateService.targetGroups[0].configureHealthCheck({
 
 networkMultipleTargetGroupsFargateService.targetGroups[1].configureHealthCheck({});
 
-networkMultipleTargetGroupsFargateService.loadBalancers[0]._enableCrossEnvironment;
-networkMultipleTargetGroupsFargateService.loadBalancers[1]._enableCrossEnvironment;
-
 new IntegTest(app, 'Integ', { testCases: [stack] });
 
 app.synth();

packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.healthchecks-multiple-application-load-balanced-fargate-service.ts

@@ -66,12 +66,6 @@ applicationMultipleTargetGroupsFargateService.targetGroups[1].configureHealthChe
   healthyHttpCodes: '200',
 });
 
-applicationMultipleTargetGroupsFargateService.loadBalancers[0]._enableCrossEnvironment;
-applicationMultipleTargetGroupsFargateService.loadBalancers[1]._enableCrossEnvironment;
-
-applicationMultipleTargetGroupsFargateService.listeners[0].listenerArn;
-applicationMultipleTargetGroupsFargateService.listeners[1].listenerArn;
-
 new IntegTest(app, 'Integ', { testCases: [stack] });
 
 app.synth();

packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.healthchecks-multiple-network-load-balanced-fargate-service.ts

@@ -50,8 +50,6 @@ networkMultipleTargetGroupsFargateService.targetGroups[0].configureHealthCheck({
 
 networkMultipleTargetGroupsFargateService.targetGroups[1].configureHealthCheck({});
 
-networkMultipleTargetGroupsFargateService.loadBalancers[0]._enableCrossEnvironment;
-networkMultipleTargetGroupsFargateService.loadBalancers[1]._enableCrossEnvironment;
 
 new IntegTest(app, 'Integ', { testCases: [stack] });
 

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

@@ -297,7 +297,7 @@ export class CfnResource extends CfnRefElement {
       return;
     }
 
-    addDependency(this, target);
+    addDependency(this, target, `{${this.node.path}}.addDependency({${target.node.path}})`);
   }
 
   /**

packages/@aws-cdk/core/lib/stack.ts

@@ -605,7 +605,7 @@ export class Stack extends Construct implements ITaggable {
    * app, and also supports nested stacks.
    */
   public addDependency(target: Stack, reason?: string) {
-    addDependency(this, target, reason);
+    addDependency(this, target, reason ?? `{${this.node.path}}.addDependency({${target.node.path}})`);
   }
 
   /**
@@ -1718,3 +1718,4 @@ import { Fact, RegionInfo } from '@aws-cdk/region-info';
 import { deployTimeLookup } from './private/region-lookup';
 import { makeUniqueResourceName } from './private/unique-resource-name';import { PRIVATE_CONTEXT_DEFAULT_STACK_SYNTHESIZER } from './private/private-context';
 
+

packages/@aws-cdk/core/test/stack.test.ts

@@ -2231,7 +2231,6 @@ describe('regionalFact', () => {
       },
     });
   });
-
 });
 
 class StackWithPostProcessor extends Stack {