aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.51k stars 3.85k forks source link

(codepipeline_actions): CodeDeployEcsDeployAction doesn't return the role created by default #18648

Open angarg12 opened 2 years ago

angarg12 commented 2 years ago

What is the problem?

If you create a CodeDeployEcsDeployAction and don't pass a role as a parameter, the codepipeline will generate a default one for you (as seen in pipeline.ts). However this role cannot be retrieved from the CodeDeployEcsDeployAction object.

Reproduction Steps

    const deploymentGroup = codedeploy.EcsDeploymentGroup.fromEcsDeploymentGroupAttributes(stack, "DeploymentGroup", {
      application: EcsApplication.fromEcsApplicationName(stack, "ECSApplication", "test"),
      deploymentGroupName: "test",
    });

    const action = new codepipeline_actions.CodeDeployEcsDeployAction({
      actionName: "Deploy",
      deploymentGroup: deploymentGroup,
      taskDefinitionTemplateInput: new codepipeline.Artifact("dummy"),
      appSpecTemplateInput: new codepipeline.Artifact("dummy2"),
    });

    const pipeline = new codepipeline.Pipeline(stack, "Pipeline", {
      stages: [
        {
          stageName: "DeployECS",
          actions: [action],
        },
      ],
    });

    const actionRole = action.actionProperties.role;
    // actionRole is always undefined
    console.log(actionRole);

What did you expect to happen?

action.actionProperties.role returns the default role created by codepipeline.

What actually happened?

action.actionProperties.role is undefined.

CDK CLI Version

1.129.0

Framework Version

No response

Node.js Version

14.18.1

OS

Amazon Linux 2

Language

Typescript

Language Version

No response

Other information

No response

skinny85 commented 2 years ago

Hey @angarg12,

that is true, that role is not easily available.

What you can do is create a role yourself, and pass it to the Action:

    const actionRole = new iam.Role(this, 'EcsDeployActionRole', {
      //
    });

    const action = new codepipeline_actions.CodeDeployEcsDeployAction({
      // properties as above...
      role: actionRole,
    });

    const pipeline = new codepipeline.Pipeline(stack, "Pipeline", {
      stages: [
        {
          stageName: "DeployECS",
          actions: [action],
        },
      ],
    });

Out of curiosity, why do you need access to the Role?

Thanks, Adam

angarg12 commented 2 years ago

Hi,

That's the workaround I'm using at the moment.

I need the role to attach a codedeploy:GetDeploymentConfig policy to it. This is the error that I see when trying to do a Codedeploy B/G deployment otherwise.

Insufficient permissions
User: xxx is not authorized to perform: codedeploy:GetDeploymentConfig on resource: yyy because no identity-based policy allows the codedeploy:GetDeploymentConfig action
skinny85 commented 2 years ago

Hmm, does that error happen in the pipeline? I wonder what happens, because we add the codedeploy:GetDeploymentConfig permission to the Role here?

angarg12 commented 2 years ago

Yes it does.

I think I found superficially what is happening (although not why).

This is the policy I use in my custom role

    ecsDeployRole.attachInlinePolicy(
      new iam.Policy(this, "DeployECSPolicy", {
        statements: [
          new am.PolicyStatement({
            actions: ["codedeploy:GetDeploymentConfig"],
            effect: iam.Effect.ALLOW,
            resources: ["*"],
          }),
        ],
      })
    );

And this is the policy generated by default:

  "Action": "codedeploy:GetDeploymentConfig",
  "Effect": "Allow",
  "Resource": {
    "Fn::Join": [
      "",
      [
        "arn:",
        {
          "Ref": "AWS::Partition"
        },
        ":codedeploy:",
        {
          "Ref": "AWS::Region"
        },
        ":",
        {
          "Ref": "AWS::AccountId"
        },
        ":deploymentconfig:CodeDeployDefault.ECSAllAtOnce"
      ]
    ]
  }
},

The thing is my deployment group is not using ECSAllAtOnce, but out own custom deployment config. That's why the default role fails, but my own (with * resources) works.

I confirmed in the console that the deployment group is pointing to the right deployment configuration. I'll dig deeper to figure out where the ECSAllAtOnce is coming from.

skinny85 commented 2 years ago

I wonder if it's this part?

    const deploymentGroup = codedeploy.EcsDeploymentGroup.fromEcsDeploymentGroupAttributes(stack, "DeploymentGroup", {
      application: EcsApplication.fromEcsApplicationName(stack, "ECSApplication", "test"),
      deploymentGroupName: "test",
    });

You're not passing the deploymentConfig property there, so I think CDK assumes you're using CodeDeployDefault.ECSAllAtOnce.

I think if you pass the correct configuration in the deploymentConfig property, this problem might go away (I hope 😉).

angarg12 commented 2 years ago

That works.

I (incorrectly) assumed that fromEcsDeploymentGroupAttributes would point to the existing Deployment Group in the account, which has the correct DeploymentConfig. Now it works as expected.

skinny85 commented 2 years ago

Nice!

I'm thinking whether it's worth it to add this callout somewhere in our docs...? This actually looks like a tricky sharp edge (maybe we should have made deploymentConfig in fromEcsDeploymentGroupAttributes() required, but now we can't do that, as that would be a breaking change 😕).

What do you think @angarg12?

angarg12 commented 2 years ago

I can see how this might be a bit of a corner case that not many people will encounter.

It was unintuitive that the value defaults to CodeDeployDefault.ECSAllAtOnce without telling. Adding that information at least to the API docs might not have prevented it, but might have helped root causing the issue 🙂

skinny85 commented 2 years ago

Yep, makes sense!

Would you be interested in submitting a PR for adding documentation for this that would (hopefully 😉) prevented you from making that mistake? Our "Contributing" guide: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md.

Thanks, Adam

angarg12 commented 2 years ago

Sure, I'm busy at the moment but I would be able to take this by the end of the month.

skinny85 commented 2 years ago

Thank you!

tim-finnigan commented 1 year ago

Hi @angarg12 just wanted to check in, were you still planning to create a PR to update the documentation?

angarg12 commented 1 year ago

Not anymore, thanks!