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-ecs] TaskDefinition that uses cross account ECR repository image throws an error #10233

Closed
idm-ryou opened this issue Sep 8, 2020 · 7 comments
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container p1

Comments

@idm-ryou
Copy link

idm-ryou commented Sep 8, 2020

On v1.60.0(current latest release v.1.62.0 too), a TaskDefinition that uses cross account ECR repository image throws an error.
On earlier v1.59.0, same code works fine.

Reproduction Steps

Use CDK v1.60.0

import * as cdk from "@aws-cdk/core";
import * as ecr from "@aws-cdk/aws-ecr";
import * as ecs from "@aws-cdk/aws-ecs";

const app = new cdk.App();
const region = "us-east-1";
const stackA = new cdk.Stack(app, "stackA", {
  env: {
    region,
    account: "111111111111",
  },
});
const ecrRepo = new ecr.Repository(stackA, "testEcrRepo", {
  repositoryName: "test-ecr-repo",
});

const stackB = new cdk.Stack(app, "stackB", {
  env: {
    region,
    account: "222222222222",
  },
});
const taskDef = new ecs.FargateTaskDefinition(stackB, "testTaskDef");
taskDef.addContainer("test-container", {
  image: ecs.ContainerImage.fromEcrRepository(ecrRepo),
});
$ cdk --version
1.60.0 (build 8e3f53a)

$ cdk synth

/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/resource.ts:154
      throw new Error(`Cannot use resource '${this.node.path}' in a cross-environment fashion, ` +
            ^
Error: Resolution error: Resolution error: Resolution error: Resolution error: Resolution error: Cannot use resource 'stackB/testTaskDef/ExecutionRole' in a cross-environment fashion, the resource's physical name must be explicit set or use `PhysicalName.GENERATE_IF_NEEDED`..

What did you expect to happen?

cdk synth should succeeds like earlier v1.59.0

$ cdk --version
1.59.0 (build 1d082f4)

$ cdk synth stackA
Resources:
  testEcrRepo66D8A784:
    Type: AWS::ECR::Repository
    Properties:
      RepositoryName: test-ecr-repo
    UpdateReplacePolicy: Retain
    DeletionPolicy: Retain
    Metadata:
      aws:cdk:path: stackA/testEcrRepo/Resource

$ cdk synth stackB
Resources:
  testTaskDefTaskRole8C17AF3A:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: ecs-tasks.amazonaws.com
        Version: "2012-10-17"
    Metadata:
      aws:cdk:path: stackB/testTaskDef/TaskRole/Resource
  testTaskDef147F11A0:
    Type: AWS::ECS::TaskDefinition
    Properties:
      ContainerDefinitions:
        - Essential: true
          Image:
            Fn::Join:
              - ""
              - - Fn::Select:
                    - 4
                    - Fn::Split:
                        - ":"
                        - Fn::Join:
                            - ""
                            - - "arn:"
                              - Ref: AWS::Partition
                              - :ecr:us-east-1:111111111111:repository/test-ecr-repo
                - .dkr.ecr.
                - Fn::Select:
                    - 3
                    - Fn::Split:
                        - ":"
                        - Fn::Join:
                            - ""
                            - - "arn:"
                              - Ref: AWS::Partition
                              - :ecr:us-east-1:111111111111:repository/test-ecr-repo
                - "."
                - Ref: AWS::URLSuffix
                - /test-ecr-repo:latest
          Name: test-container
      Cpu: "256"
      ExecutionRoleArn:
        Fn::GetAtt:
          - testTaskDefExecutionRole814B3A38
          - Arn
      Family: stackBtestTaskDef150CF57D
      Memory: "512"
      NetworkMode: awsvpc
      RequiresCompatibilities:
        - FARGATE
      TaskRoleArn:
        Fn::GetAtt:
          - testTaskDefTaskRole8C17AF3A
          - Arn
    Metadata:
      aws:cdk:path: stackB/testTaskDef/Resource
  testTaskDefExecutionRole814B3A38:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: ecs-tasks.amazonaws.com
        Version: "2012-10-17"
    Metadata:
      aws:cdk:path: stackB/testTaskDef/ExecutionRole/Resource
  testTaskDefExecutionRoleDefaultPolicy270D14AB:
    Type: AWS::IAM::Policy
    Properties:
      PolicyDocument:
        Statement:
          - Action:
              - ecr:BatchCheckLayerAvailability
              - ecr:GetDownloadUrlForLayer
              - ecr:BatchGetImage
            Effect: Allow
            Resource:
              Fn::Join:
                - ""
                - - "arn:"
                  - Ref: AWS::Partition
                  - :ecr:us-east-1:111111111111:repository/test-ecr-repo
          - Action: ecr:GetAuthorizationToken
            Effect: Allow
            Resource: "*"
        Version: "2012-10-17"
      PolicyName: testTaskDefExecutionRoleDefaultPolicy270D14AB
      Roles:
        - Ref: testTaskDefExecutionRole814B3A38
    Metadata:
      aws:cdk:path: stackB/testTaskDef/ExecutionRole/DefaultPolicy/Resource

What actually happened?

cdk synth fails with an error.

Full stack trace
/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/resource.ts:154
      throw new Error(`Cannot use resource '${this.node.path}' in a cross-environment fashion, ` +
            ^
Error: Resolution error: Resolution error: Resolution error: Resolution error: Resolution error: Cannot use resource 'stackB/testTaskDef/ExecutionRole' in a cross-environment fashion, the resource's physical name must be explicit set or use `PhysicalName.GENERATE_IF_NEEDED`..

Object creation stack:
  at new PolicyDocument (/workspaces/aws-infra/node_modules/@aws-cdk/aws-iam/lib/policy-document.ts:48:30)
  at Repository.addToResourcePolicy (/workspaces/aws-infra/node_modules/@aws-cdk/aws-ecr/lib/repository.ts:471:29)
  at Function.addToPrincipalOrResource (/workspaces/aws-infra/node_modules/@aws-cdk/aws-iam/lib/grant.ts:147:45)
  at Repository.grant (/workspaces/aws-infra/node_modules/@aws-cdk/aws-ecr/lib/repository.ts:228:22)
  at Repository.grantPull (/workspaces/aws-infra/node_modules/@aws-cdk/aws-ecr/lib/repository.ts:241:22)
  at EcrImage.bind (/workspaces/aws-infra/node_modules/@aws-cdk/aws-ecs/lib/images/ecr.ts:29:21)
  at new ContainerDefinition (/workspaces/aws-infra/node_modules/@aws-cdk/aws-ecs/lib/container-definition.ts:382:36)
  at FargateTaskDefinition.addContainer (/workspaces/aws-infra/node_modules/@aws-cdk/aws-ecs/lib/base/task-definition.ts:387:12)
  at Object.<anonymous> (/workspaces/aws-infra/src/app2.ts:24:9)
  at Module._compile (internal/modules/cjs/loader.js:1137:30)
  at Module.m._compile (/workspaces/aws-infra/node_modules/ts-node/src/index.ts:858:23)
  at Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
  at Object.require.extensions.<computed> [as .ts] (/workspaces/aws-infra/node_modules/ts-node/src/index.ts:861:12)
  at Module.load (internal/modules/cjs/loader.js:985:32)
  at Function.Module._load (internal/modules/cjs/loader.js:878:14)
  at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
  at main (/workspaces/aws-infra/node_modules/ts-node/src/bin.ts:227:14)
  at Object.<anonymous> (/workspaces/aws-infra/node_modules/ts-node/src/bin.ts:513:3)
  at Module._compile (internal/modules/cjs/loader.js:1137:30)
  at Object.Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
  at Module.load (internal/modules/cjs/loader.js:985:32)
  at Function.Module._load (internal/modules/cjs/loader.js:878:14)
  at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
  at internal/main/run_main_module.js:17:47.
Object creation stack:
  at new LazyBase (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/lazy.ts:126:26)
  at new LazyAny (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/lazy.ts:181:5)
  at Function.anyValue (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/lazy.ts:115:12)
  at new Repository (/workspaces/aws-infra/node_modules/@aws-cdk/aws-ecr/lib/repository.ts:420:34)
  at Object.<anonymous> (/workspaces/aws-infra/src/app2.ts:13:17)
  at Module._compile (internal/modules/cjs/loader.js:1137:30)
  at Module.m._compile (/workspaces/aws-infra/node_modules/ts-node/src/index.ts:858:23)
  at Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
  at Object.require.extensions.<computed> [as .ts] (/workspaces/aws-infra/node_modules/ts-node/src/index.ts:861:12)
  at Module.load (internal/modules/cjs/loader.js:985:32)
  at Function.Module._load (internal/modules/cjs/loader.js:878:14)
  at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
  at main (/workspaces/aws-infra/node_modules/ts-node/src/bin.ts:227:14)
  at Object.<anonymous> (/workspaces/aws-infra/node_modules/ts-node/src/bin.ts:513:3)
  at Module._compile (internal/modules/cjs/loader.js:1137:30)
  at Object.Module._extensions..js (internal/modules/cjs/loader.js:1157:10)
  at Module.load (internal/modules/cjs/loader.js:985:32)
  at Function.Module._load (internal/modules/cjs/loader.js:878:14)
  at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
  at internal/main/run_main_module.js:17:47.
Object creation stack:
  at new Intrinsic (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/intrinsic.ts:28:26)
  at new PostResolveToken (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/util.ts:83:5)
  at Object.ignoreEmpty (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/util.ts:38:10)
  at CfnRepository._toCloudFormation (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/cfn-resource.ts:297:25)
  at /workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/refs.ts:120:58
  at Object.findTokens (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/resolve.ts:174:11)
  at findAllReferences (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/refs.ts:120:22)
  at Object.resolveReferences (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/refs.ts:24:17)
  at Object.prepareApp (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/prepare-app.ts:36:5)
  at Object.synthesize (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/synthesis.ts:21:3)
  at App.synth (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/stage.ts:175:23)
  at process.<anonymous> (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/app.ts:112:45)
  at Object.onceWrapper (events.js:422:26)
  at process.emit (events.js:315:20)
  at process.EventEmitter.emit (domain.js:483:12)
  at process.emit (/workspaces/aws-infra/node_modules/source-map-support/source-map-support.js:495:21).
Object creation stack:
  at new Intrinsic (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/intrinsic.ts:28:26)
  at new PostResolveToken (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/util.ts:83:5)
  at CfnRepository._toCloudFormation (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/cfn-resource.ts:295:29)
  at /workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/refs.ts:120:58
  at Object.findTokens (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/resolve.ts:174:11)
  at findAllReferences (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/refs.ts:120:22)
  at Object.resolveReferences (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/refs.ts:24:17)
  at Object.prepareApp (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/prepare-app.ts:36:5)
  at Object.synthesize (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/synthesis.ts:21:3)
  at App.synth (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/stage.ts:175:23)
  at process.<anonymous> (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/app.ts:112:45)
  at Object.onceWrapper (events.js:422:26)
  at process.emit (events.js:315:20)
  at process.EventEmitter.emit (domain.js:483:12)
  at process.emit (/workspaces/aws-infra/node_modules/source-map-support/source-map-support.js:495:21)
    at Role._enableCrossEnvironment (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/resource.ts:154:13)
    at Object.resolve (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/resource.ts:217:16)
    at RememberingTokenResolver.resolveToken (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/resolvable.ts:134:24)
    at RememberingTokenResolver.resolveToken (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/resolve.ts:187:18)
    at resolve (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/resolve.ts:133:29)
    at Object.resolve [as mapToken] (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/resolve.ts:49:32)
    at TokenizedStringFragments.mapTokens (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/string-fragments.ts:71:33)
    at RememberingTokenResolver.resolveString (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/resolvable.ts:155:22)
    at RememberingTokenResolver.resolveString (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/resolve.ts:191:23)
    at resolve (/workspaces/aws-infra/node_modules/@aws-cdk/core/lib/private/resolve.ts:91:31)

Environment

  • CLI Version :1.60.0
  • Framework Version:1.60.0
  • Node.js Version:v12.18.3
  • OS :Linux(Docker)
  • Language (Version):TypeScript (3.8.3)

Other

I suspect changes from #8280 affected this issue.


This is 🐛 Bug Report

@idm-ryou idm-ryou added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 8, 2020
@SomayaB SomayaB added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Sep 8, 2020
@abbottdev
Copy link

We're also getting this issue trying to use a "deployer" AWS account which does cross-account deployment to our dev/prod env - but we opted for that deployer account to host our ECS repository - we are however looking into using the ecs-assets library - I assume this would resolve the problem as a workaround?

@SoManyHs SoManyHs added needs-reproduction This issue needs reproduction. p1 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 23, 2020
@MrArnoldPalmer
Copy link
Contributor

This may be related/caused by this change that was included in v1.60.0.

@iamhopaul123
Copy link
Contributor

Hello @idm-ryou, @abbottdev, seems like there is already a pretty good explanation for the reason why this error occurs and solution to how to reference a cross account/region resource: https://stackoverflow.com/questions/62989129/resolution-error-cannot-use-resource-x-in-a-cross-environment-fashion-the-re.

I would recommend using the same account/region. However, if you want to reuse the repository, you could deploy stackA and get the ARN for the ECR repo then hardcode the repo ARN in stackB task definition. Please let me know if this still won't work for you.

@iamhopaul123 iamhopaul123 added guidance Question that needs advice or information. and removed bug This issue is a bug. needs-reproduction This issue needs reproduction. labels Jan 5, 2021
@SoManyHs SoManyHs added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jan 5, 2021
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jan 13, 2021
@garyd203
Copy link

garyd203 commented Feb 24, 2021

For people who are want to solve this bug for cross-account ECR usage (as per original bug report), I can't find a way to set the name of the internally generated execution role for the task definition. However, if you create an empty role then the TaskDefinition construct will add the necessary policies. Example in Python:

role = Role(
    scope,
    "ExecRole",
    assumed_by=ServicePrincipal(service="ecs-tasks.amazonaws.com"),
    role_name=PhysicalName.GENERATE_IF_NEEDED,
)

task_definition = FargateTaskDefinition(
    scope,
    "TaskDef",
    execution_role=role,
    family="my-task-family"
)

However, note that CDK will create a ECR repository policy that explicitly references the cross-account execution role, and ECR will validate this when you set the policy. This creates a circular deployment dependency where the IAM role needs to be deployed before the ECR repository, but the IAM role is deployed with an ECS task definition that depends on the ECR repository.

I believe you can break the circularity by doing an initial deployment with the (unused) IAM role, and then a second full deployment. Alternatively, I broke the circularity by forcing CDK to discard the over-constrained repository policy and setting my own using just the AWS account, like so:

ecr_repo.node.default_child.add_property_override(
        "RepositoryPolicyText",
        PolicyDocument(
            statements=[
                PolicyStatement(
                    actions=[
                        "ecr:BatchCheckLayerAvailability",
                        "ecr:BatchGetImage",
                        "ecr:GetDownloadUrlForLayer",
                    ],
                    effect=Effect.ALLOW,
                    principals=[AccountPrincipal(other_account_number)],
                )
            ],
        ).to_json(),
    )

In terms of security impact, for standard multiple-account use cases, It's hard to argue that there's any benefit to a policy that allows reads from only some roles in another account - the implication is that there are some roles in the other account which should be explicitly prohibited from accessing the repository and are also incapable of gaining access to one of the permitted roles, which suggests that the other account has multiple incompatible purposes. OTOH having a decoupled policy is a significant benefit for maintainability and deployability.

@SoManyHs SoManyHs reopened this Mar 29, 2021
@SoManyHs SoManyHs removed the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Mar 29, 2021
@SoManyHs SoManyHs removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 29, 2021
@peterwoodworth peterwoodworth removed the guidance Question that needs advice or information. label Oct 18, 2021
@peterwoodworth
Copy link
Contributor

It's been a while since this issue has been active. If you're finding this is still an issue on latest version, please create a new issue. Otherwise, I think it's a safe assumption that this has since been fixed

@github-actions
Copy link

github-actions bot commented May 1, 2023

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container p1
Projects
None yet
Development

No branches or pull requests

9 participants