code-pipeline: Existing S3 buckets are not given the correct permissions

[alastair-watts-avrios]

When an S3 bucket is imported into a cross-account pipeline for use in an S3DeployAction it is hard to give it the correct permissions. We use an approach of having a tooling account and three different environment accounts. We have a final stage where we want to deploy some files from a pipeline in the tooling account to an existing S3 bucket in each env.
The bucket can actually be referenced using the pipeline as the scope in Bucket.fromBucketAttributes or Bucket.fromBucketName and a role and policy is created. However this is created all in the tooling account so doesn't have any effect once the pipeline step is actually triggered. To get around this we can create a new stack in the correct account and use that as the scope. This adds 7 extra cloud formations templates to our build output (since we have multiple pipelines). Currently we have gone with an approach of mimicking some of the code from getOtherStackIfActionIsCrossAccount to get the cross account support stack. Would be useful if this could somehow be spotted and the role created in the cross account stack when needed.

Reproduction Steps

const test-account-bucket = s3.Bucket.fromBucketName(tooling-account-stack, bucketName, bucketName);
const deploy-action = new actions.S3DeployAction({
        actionName,
        input,
        bucket,
    });
tooling-account-pipeline.addStage({
            stageName,
            actions: [deploy-action],
        });

What did you expect to happen?

A role to be created in the cross account stack since the bucket exists in the test account. Or a buildtime error.

What actually happened?

Failed to run the step and hard to debug since all the permissions seem to be present.

Environment

• CDK CLI Version :
• Framework Version: 1.94.1
• Node.js Version: v14.16.0
• OS : macOS Big Sur 11.2.3
• Language (Version): TypeScript (4.2.3)

Other

I can see two ways this may be solvable:

• Have S3 deploy actions that use an existing bucket be attached to an account rather than a resource. I assume this has a high chance of breaking other things.
• create a special case in getOtherStackIfActionIsCrossAccount to use the second half of the method when the resource is an existing s3.

Happy to contribute but am unsure what solution(s) fit best. Am aware this is an edge case with multiple work arounds.

---

This is 🐛 Bug Report

[skinny85]

Hi @alastair-watts-avrios ,

thanks for opening the issue, but I think you're making a logical mistake here.

From your description, it seems like the Buckets to be deployed are in the target env, not in the tooling account.

But look how you are importing them:

const testAccountBucket = s3.Bucket.fromBucketName(toolingAccountStack, bucketName, bucketName);

You are importing it into the toolingAccountStack by name. How is the CDK supposed to know it actually lives in the target env??

The solution you mentioned (importing it into the target env) is the correct one. Yes, it adds additional Stacks, but that's because you need additional resources in the target env to be able to deploy to them from the tooling account.

Alternatively, you can look into the CDK Pipelines module. When you bootstrap your target envs using it, it will create Roles that can be used for the deployment (using something like fromRoleArn()).

Thanks,
Adam

[alastair-watts-avrios]

The problem is we ARE using pipelines cdk and it DOES create roles but they don't work (because the tooling account can't give permissions).

If you compare this to the cloud formation deploy action (where admittedly you can supply an account here) this creates the roles in the cross-account-support stack so they work. I understand that this difference comes from the fact that one is "resource" backed and the other is "account" backed but this is rather unintuitive. The documentation on the Action properties isn't actually consistent.

  /**
   * The account the Action is supposed to live in.
   * For Actions backed by resources,
   * this is inferred from the Stack {@link resource} is part of.
   * However, some Actions, like the CloudFormation ones,
   * are not backed by any resource, and they still might want to be cross-account.
   * In general, a concrete Action class should specify either {@link resource},
   * or {@link account} - but not both.
   */
  readonly account?: string;

  /**
   * The optional resource that is backing this Action.
   * This is used for automatically handling Actions backed by
   * resources from a different account and/or region.
   */
  readonly resource?: IResource;

We see that the account documentation clearly states that it will use the stack the resource is part of (which is what happens) but the resource documentation mentions resources from (which is what I would expect). Pipelines cdk should care about where that resource EXISTS rather than where it is DEFINED. Using that (if it actually can) it could put the roles in the correct cross account stack.

I did try using the fromBucketAttributes which lets you specify what account the bucket is in but pipelines doesn't pay attention to this.

The issue is that the synth works and there a roles and policies that make sense but the pipeline fails saying there are not enough permissions (I did try adding the putObjectAcl permissions it suggested but obviously tooling can't create a policy that gives access to a different account)

[skinny85]

Sorry, I have problems following what you're saying.

Let's establish some basic facts:

1. If you use

const testAccountBucket = s3.Bucket.fromBucketName(toolingAccountStack, bucketName, bucketName);

, then this will not work, and cannot work. That's because you're telling CDK the target Bucket lives in the tooling account, which is not the case.

2. If you instead do

const testAccountBucket = s3.Bucket.fromBucketName(targetEnvStack, bucketName, bucketName);

, you wrote:

To get around this we can create a new stack in the correct account and use that as the scope. This adds 7 extra cloud formations templates to our build output (since we have multiple pipelines).

So, does doing s3.Bucket.fromBucketName(targetEnvStack, bucketName, bucketName) work, or no?

[alastair-watts-avrios]

Yes. 1. doesn't work. 2. does.

1 is frustrating because it appears to work and gives no useful error. But I guess this is a limitation of cdk in general. there is no way for it to know where that bucket lives during the synth step.

But

    const bucket = s3.Bucket.fromBucketAttributes(toolingAccountStack, bucketName, {
        bucketName,
        account: targetAccount
    });

now tells cdk that the target bucket lives in the target account. However the result is the same as for 1.

My point is that there is NO way to tell the S3DeployAction that it lives in another account without creating another stack and therefore template manually whereas for actions backed by an account this is trivial.

Looking further into the IResource interface there is:

  /**
   * The environment this resource belongs to.
   * For resources that are created and managed by the CDK
   * (generally, those created by creating new class instances like Role, Bucket, etc.),
   * this is always the same as the environment of the stack they belong to;
   * however, for imported resources
   * (those obtained from static methods like fromRoleArn, fromBucketName, etc.),
   * that might be different than the stack they were imported into.
   */
  readonly env: ResourceEnvironment;

So why does Pipeline.getOtherStackIfActionIsCrossAccount refer to Stack.of(action.actionProperties.resource).account rather than preferring action.actionProperties.resource.env.account if available? (The documentation even suggests that this should work for fromBucketName though I agree with you that I am not sure how it can?).

I have raised #14217 as a draft as this is what I would expect the behaviour to be.

[skinny85]

Thanks for the explanation @alastair-watts-avrios! I finally understood what the problem was.

I've submitted #14224 fixing this bug.

Thanks,
Adam

[alastair-watts-avrios]

Thanks for your help. I will close my PR as it was far from production ready anyway. Hopefully your change will be released soon.

Glad I was able to clarify the issue.

[github-actions]

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