-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
CodePipeline: when providing custom Pipeline Role, cross-region support stacks cannot be deployed #19881
Comments
related to #19835 |
Thanks for reporting @cprice404. Yes, you're right that we shouldn't add Role to the Resource Policy of the Key in this case. I wonder what triggers this behavior though! The CodePipeline construct just constructs the Role itself if you don't pass it, so it shouldn't be any different. Can you show how you create the Role, and how do you pass it to CodePipeline? Thanks, |
@skinny85 I'm creating it with code very similar to what I pasted in my comment on the other ticket: I did have to add this because CFN wasn't waiting for the policy to be created before it started trying to create the pipeline: const pipelineCfnResource = pipeline.node.defaultChild as CfnResource;
pipelineCfnResource.addDependsOn(
pipelineRolePolicy.node.defaultChild as CfnResource
); |
That might be a different bug in the What's |
Sorry, edited it to match the code from the snippet on the other ticket. It's just the policy that I build up with the s3/kms/assumerole permissions, which I had to do because the auto-generated policy exceeds the max IAM policy size. |
So basically it's this: // We need to create a policy statement that grants access to the
// KMS key that CDK generated for accessing the pipeline's s3 artifact
// buckets. To do this, we will need to know CDK's "uniqueId" for the
// key, so that we can reference it in the policy. Here we are calling
// a vendored copy of their function that creates the unique id, because
// the function isn't exported in the CDK library.
const artifactBucketEncryptionKeyCdkUniqueId = makeUniqueId([
`pipeline-${pipelineName}`,
'ArtifactsBucketEncryptionKey',
'Resource',
]);
const pipelineRolePolicyName = `pipelinepolicy-${pipelineName}`;
const pipelineRolePolicy = new iam.Policy(
scope,
pipelineRolePolicyName,
{
policyName: pipelineRolePolicyName,
// These policy statements were built up by hand after examining the
// statements in the CDK-generated policy for several pipelines.
statements: [
// Grants the pipeline access to the s3 artifact buckets.
new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
actions: [
's3:Abort*',
's3:DeleteObject*',
's3:GetBucket*',
's3:GetObject*',
's3:List*',
's3:PutObject',
's3:PutObjectLegalHold',
's3:PutObjectRetention',
's3:PutObjectTagging',
's3:PutObjectVersionTagging',
],
resources: [`arn:aws:s3:::${pipelineName}-pipeline*`],
}),
// Grants the pipeline access to the KMS key that is used for the s3 artifact buckets.
new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
actions: [
'kms:Decrypt',
'kms:DescribeKey',
'kms:Encrypt',
'kms:GenerateDataKey*',
'kms:ReEncrypt*',
],
resources: [
Fn.getAtt(artifactBucketEncryptionKeyCdkUniqueId, 'Arn').toString(),
],
}),
// Grants the pipeline AssumeRole permissions into the various CDK-generated roles, for
// cross-account / cross-region deploys etc.
new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
actions: ['sts:AssumeRole'],
resources: targetAccounts.map(targetAccount => {
// Here we are granting permissions to assume some roles that are generated by CDK
// for use in pipelines. We need to wild-card them so that the policy doesn't get
// too large for IAM. The roles will begin with a string like `cacheadmin-pipeline`,
// unless the name of the pipeline is too long; CDK will truncate the prefix portion
// after 25 chars, and the suffix varies per role, so we just use the first 25 chars.
const pipelineRoleNamePrefix = `${pipelineName}-pipeline`.substr(
0,
25
);
return `arn:aws:iam::${targetAccount}:role/${pipelineRoleNamePrefix}*`;
}),
}),
],
}
);
const pipelineRoleName = `pipelinerole-${pipelineName}`;
const pipelineRole = new iam.Role(scope, pipelineRoleName, {
roleName: pipelineRoleName,
assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com'),
});
pipelineRolePolicy.attachToRole(pipelineRole);
const pipeline = new codepipeline.Pipeline(
this,
`pipeline-foo`,
{
pipelineName: 'foo',
crossAccountKeys: true,
// passing the role via `withoutPolicyUpdates` prevents CDK from trying to manually manage the policy. See:
role: pipelineRole.withoutPolicyUpdates()
}
);
const pipelineCfnResource = pipeline.node.defaultChild as CfnResource;
pipelineCfnResource.addDependsOn(
pipelineRolePolicy.node.defaultChild as CfnResource
);
|
OK, thanks. Can you explain more about your Pipeline? What makes it cross-region? Is it also cross-account at the same time? |
Also, |
Agh! Sorry. I manually edited the code to sanitize/simplify it before posting and I made a typo there. It's fixed now, it should have been
Yes. The pipeline has several stages in it, which deploy CFN stacks to our different accounts. We have a separate account for each region that we deploy to, so it is both cross-account and cross-region. Thanks very much for looking into this! Sorry for the typos. |
No worries 🙂. Can you show the entire Key resource in the support Stack - once before using your Role (so using the Role CDK generates), and then again, after passing your Role to the Thanks, |
Before (using auto-generated role):
|
After (passing custom role):
|
Thanks for the info. I can reproduce the issue. I have a hunch of what it might be, but I'll confirm tomorrow. |
But definitely the problem is not the custom Role, but Can you try passing |
I tried without |
Yes, but does removing {
"Action": [
"kms:Decrypt",
"kms:DescribeKey",
"kms:Encrypt",
"kms:GenerateDataKey*",
"kms:ReEncrypt*"
],
"Effect": "Allow",
"Principal": {
"AWS": {
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":iam::111111111111:role/pipelinerole-core-infrastructure"
]
]
}
},
"Resource": "*"
}, statement from the Key policy) |
Yes, it appears that the statement above does not get added if we don't use I would guess that most folks who are opting-in to managing their own Role, though, are doing it because of the IAM policy size issue, and |
So the problem is here: aws-cdk/packages/@aws-cdk/aws-kms/lib/key.ts Lines 228 to 230 in aa834be
So, as long as you're using the Role generated by the CodePipeline construct, the above expression returns Now, the simplest solution would be to add @rix0rrr any ideas here? Looks like the hackiness of |
Re-assigning Rico, so he has a chance to see and comment on this one. |
`principalIsANewlyCreatedResource()` was an imperfect solution to determining whether or not a principal already existed or if it was managed by CDK. This caused a deployment error, so this PR replaces it with `Resource.isOwnedResource()` which returns true iff a resource was created by CDK. This also updates the return type of `isResource()` to `is Resource`, because we already have a `is CfnResource` in `CfnResource`. Closes #19881. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Describe the bug
When trying to come up with a workaround for the maximum policy size bug mentioned here and elsewhere, I created my own IAM role to explicitly pass in to my Pipelines.
As soon as I did this, the cross-region support stacks for my Pipelines were changed slightly; their KMS KeyPolicy got a new PolicyStatement that looks like this:
The policy now explicitly references the new IAM role that I created for my Pipeline.
However, that role does not exist until the Pipeline stack itself is deployed, and the order of the actions created by CodePipeline attempts to deploy the support stacks first. So the support stacks fail to deploy because this IAM role does not exist yet.
Expected Behavior
Explicitly passing in an IAM role to the constructor of my Pipelines will allow me to avoid the maximum IAM policy size bugs, while still deploying my Pipeline stacks and support stacks.
Current Behavior
The support stacks fail to deploy with
Resource handler returned message: "Policy contains a statement with one or more invalid principals. (Service: Kms, Status Code: 400, Request ID: ..., Extended Request ID: null)" (RequestToken: ..., HandlerErrorCode: InvalidRequest)
, because they reference an IAM role arn that has not been created yet.Reproduction Steps
Create an IAM role, pass it in to the constructor of a Pipeline object, observe that the support stacks that are generated now explicitly reference the ARN of this IAM role in their KeyPolicy.
Possible Solution
Do not add the Role to the KeyPolicy. It is unnecessary, as the Role is in the same account as the Key, and the KeyPolicy already grants permission to the account:
Additional Information/Context
No response
CDK CLI Version
2.19.0
Framework Version
2.19.0
Node.js Version
v16.5.0
OS
Amazon Linux 2
Language
Typescript
Language Version
TypeScript 4.4.2
Other information
No response
The text was updated successfully, but these errors were encountered: