Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[aws-codepipeline-actions] support cross account/region ecs deployment #11199

Closed
2 tasks
stocks29 opened this issue Oct 29, 2020 · 8 comments · Fixed by #16997
Closed
2 tasks

[aws-codepipeline-actions] support cross account/region ecs deployment #11199

stocks29 opened this issue Oct 29, 2020 · 8 comments · Fixed by #16997
Labels
@aws-cdk/aws-codepipeline-actions effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p1

Comments

@stocks29
Copy link

Add account/region params and setup permissions so an EcsDeployAction in a pipeline in Account A can deploy to a different region in Account B.

Use Case

To keep pipeline accounts separate from environment accounts and to support multi-region deployments.

Proposed Solution

I don't have an elegant proposed solution but I was able to achieve a workaround described below.

Other

I was able to cobble together a workaround by:

  1. copy EcsDeployAction to a new module (I called it EcsMultiAccountDeploy in examples below), adding in props for account and region which are passed to super and removing the role code.
  2. granting the deploy role from the cdk boostrap process the ability to deploy to ecs in each environment account via a cdk stack.
  3. passing the deploy role from the cdk bootstrap to the new deploy action described in step 1.

There may be a better approach but this worked for me. The reason for using the deploy role from cdk bootstrap is because it already has access to the build artifacts. Maybe there could be a way to augment the permissions of these roles during bootstrapping.

Below are the snippets of my workaround.

Important pieces from EcsMultiAccountDeploy

export interface EcsMultiAccountDeployActionProps extends EcsDeployActionProps {
  account: string | undefined;
  region: string | undefined;
}

  ...

  constructor(props: EcsMultiAccountDeployActionProps) {
    super({
      ...props,
      category: codepipeline.ActionCategory.DEPLOY,
      provider: 'ECS',
      artifactBounds: deployArtifactBounds(),
      inputs: [determineInputArtifact(props)],
      resource: props.service,
    });

    this.props = props;
  }

In the build stack which contains the pipeline:

// this is the deploy role created by the cdk bootstrap process
const deployRole = iam.Role.fromRoleArn(this, `${stageName}EcsDeployRole`, deployRoleArn);

const action = new EcsMultiAccountDeployAction({
  actionName: `${stageName}EcsDeploy`,
  service: ecsService,
  input: appArtifact,
  runOrder: stage.nextSequentialRunOrder(),
  account: env.account,
  region: env.region,
  role: deployRole,
});

stage.addActions(action);

In the application stack:

  updateDeployRole(props: UpdateDeployRoleProps) {
    const { deployRoleArn } = props;

    // this is the deploy role created by the cdk bootstrap process
    const role = iam.Role.fromRoleArn(this, 'BootstrapDeployRole', deployRoleArn);

    // the below is necessary for the ecs depoly
    role.addToPrincipalPolicy(new iam.PolicyStatement({
      actions: [
        'ecs:DescribeServices',
        'ecs:DescribeTaskDefinition',
        'ecs:DescribeTasks',
        'ecs:ListTasks',
        'ecs:RegisterTaskDefinition',
        'ecs:UpdateService',
      ],
      resources: ['*'],
    }));

    role.addToPrincipalPolicy(new iam.PolicyStatement({
      actions: ['iam:PassRole'],
      resources: ['*'],
      conditions: {
        StringEqualsIfExists: {
          'iam:PassedToService': [
            'ec2.amazonaws.com',
            'ecs-tasks.amazonaws.com',
          ],
        },
      },
    }));
  }
  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@stocks29 stocks29 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 29, 2020
@skinny85
Copy link
Contributor

Hey @stocks29 ,

thanks for opening the issue. Do you mind showing the code that creates props.service in your code?

Thanks,
Adam

@skinny85 skinny85 added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 29, 2020
@stocks29
Copy link
Author

stocks29 commented Oct 30, 2020

For sure. See below. It relies on naming conventions for the cluster and service names since, aside from security groups and the vpcId, passing service/cluster information back to the pipeline stack resulted in cross-stack errors. I was actually surprised that the same didn't happen for security groups and the vpcId.

    // This role is generated by the cdk bootstrap process. It's generally used for deploying cloudformation to env account/regions
    // but since it already has access to the artifact bucket/key and it's in account b, we'll use it for ecs deploys as well
    // the service stack will handle adding ecs permissions to this role.
    const deployRoleArn = `arn:aws:iam::${env.account}:role/cdk-hnb659fds-deploy-role-${env.account}-${env.region}`;

    const serviceStage = new ServiceStage(this, stageName, {
      env,
      ecrRepoArn: ecrRepo.repositoryArn,
      ecrRepoName: ecrRepo.repositoryName,
      clusterName,
      serviceName,
      containerName: imageName,
      deployRoleArn,
      hostname,
    });

    const stage = pipeline.addApplicationStage(serviceStage);
    const securityGroups = serviceStage.securityGroupIds.value.split(',')
      .map((id: string, index: number) => ec2.SecurityGroup.fromSecurityGroupId(self, `${stageName}SG${index}`, id));

    const vpc = ec2.Vpc.fromVpcAttributes(this, `${stageName}Vpc`, {
      vpcId: serviceStage.vpcId.value,
      availabilityZones: serviceStage.availZones.value.split(',')
    });

    const cluster = ecs.Cluster.fromClusterAttributes(this, `${stageName}Cluster`, { clusterName, vpc, securityGroups });

    const ecsService = ecs.FargateService.fromFargateServiceAttributes(this, `${stageName}EcsService`, { cluster, serviceName }); 

    const deployRole = iam.Role.fromRoleArn(this, `${stageName}EcsDeployRole`, deployRoleArn);

    const action = new EcsMultiAccountDeployAction({
      actionName: `${stageName}EcsDeploy`,
      service: ecsService,
      input: appArtifact,
      runOrder: stage.nextSequentialRunOrder(),
      account: env.account,
      region: env.region,
      role: deployRole,
    });

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 31, 2020
@skinny85
Copy link
Contributor

Ok. So the issue here is that you're importing your service into the same Stack the CodePipeline is in, with a name. And because of that, the CDK has no way of knowing that service is defined in a different account/region.

Now, there are 2 options:

  1. Import the service into a Stack that belongs to the same account & region the actual service is in. In that case, you don't need the EcsMultiAccountDeployAction. The CodePipeline construct will figure out itself that the Service is from a different account/region, and will do the correct setup for you.
  2. Import the service by ARN instead of by name. Since the ARN contains the account and region, that should be enough for CDK to figure out it needs to do the same setup as in point 1.

However, the problem is that option .2 does not currently work in the CDK, because the account and region of the imported Service is not filled out correctly. This needs to be corrected here. For this reason, I'm keeping this issue open as a feature request (if you'd like to contribute this functionality to the CDK @stocks29 , that would be awesome - take a look at how Bucket handles that for some inspiration). Here's our Contributing guide: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md ).

@skinny85 skinny85 added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1 and removed needs-triage This issue or PR still needs to be triaged. labels Nov 16, 2020
@ryanpagel
Copy link

@skinny85 How would I get the above to work if I don't have a reference to the cross account service in the stack I am deploying?

@skinny85
Copy link
Contributor

@ryanpagel import it using a from*() method into a Stack that belongs to the same account/region the service is in.

@robert-lilleker
Copy link

Hi, multi regional ecs deployments would be very useful to us. Has there been any progress on this?

@robert-lilleker
Copy link

I've created a pr for the issue #15944

@skinny85 skinny85 added the in-progress This issue is being actively worked on. label Aug 25, 2021
@mergify mergify bot closed this as completed in #16997 Oct 15, 2021
mergify bot pushed a commit that referenced this issue Oct 15, 2021
…#16997)

This is a fix for the region issue raised by #11199 allowing multi regional ecs deployments

fixes #11199

supersedes #15944, merged master and added tests

----

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

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…aws#16997)

This is a fix for the region issue raised by aws#11199 allowing multi regional ecs deployments

fixes aws#11199

supersedes aws#15944, merged master and added tests

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-codepipeline-actions effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p1
Projects
None yet
4 participants