aws / aws/aws-cdk

codebuild: PipelineProject Construct: Add option to suppress creation of a AWS::Logs::ResourcePolicy when using logging via aws-logs.LogGroups or be descriptive about the behavior

Open
#24,656 5 comments 0 reactions 0 assignees View on GitHub
@aws-cdk/aws-codebuild effort/medium feature-request p2
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
2d 3h
Merged PRs (30d)
83

Description

### Describe the feature

Add in an optional parameter to "disable" (or remove) the auto-generated Resource Policy, or add to the description of the CloudWatchLoggingOptions interface that a AWS::Logs::ResourcePolicy will be auto-generated (just to help others avoid the headache of the weird behavior)
- https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_codebuild.CloudWatchLoggingOptions.html

### Use Case

Related to issues "https://github.com/aws/aws-cdk/issues/17615" and "https://github.com/aws/aws-cdk/issues/17544", I believe it may be a good workaround to the unexpected Resource Policy being added without the user's knowledge when using the PipelineProject Construct.

### Proposed Solution

We can add another parameter to the CloudWatchLoggingOptions interface (something like "suppressResourcePolicy")
- https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_codebuild.CloudWatchLoggingOptions.html

On the handler, it simply checks if logging is configured and if it's enabled/disabled.

- https://github.com/aws/aws-cdk/blob/60a5b2a5d44474b1d58ceaf3fabee73836aa9882/packages/%40aws-cdk/aws-codebuild/lib/project.ts#L1452-L1466

After "cloudWatchLogs.logGroup?.grantWrite(this);" is executed, we can then do another check to see if we should do remove it or not:
```
if ( cloudWatchLogs.suppressResourcePolicy )
cloudWatchLogs.logGroup?.node.tryRemoveChild("Policy")
```

I wasn't able to find any another potential ways that '.node.tryRemoveChild("Policy")' wouldn't be applicable except if the "Id" may not be "Policy", but I wasn't see where the Id may be changed either.

In the case that the resource Id on tree.json isn't consistent, it can also just be explicitly stated on the documentation that this will be auto-populated given some certain conditions. This way users don't get too confused with the unexpected resource and can try to remediate it themselves by removing it from the LogGroup that it's tied to.

### Other Information

I believe the issue of a ResourcePolicy being added unexpectedly is due to the behavior of the "grantWrite()" method being called when checking if logging is being defined or not.

- https://github.com/aws/aws-cdk/blob/60a5b2a5d44474b1d58ceaf3fabee73836aa9882/packages/%40aws-cdk/aws-codebuild/lib/project.ts#L1452-L1466

To my understanding of how this works, essentially grantWrite() will try to do the right thing by applying some policy to a valid Grantable-type object, and in this case it would be some IAM Role.

Given that the stack is account agnostic and an IAM Role is being used similar to the following:

```
#!/usr/bin/env node
import 'source-map-support/register';
import * as cdk from 'aws-cdk-lib';
import * as logs from 'aws-cdk-lib/aws-logs';
import * as iam from 'aws-cdk-lib/aws-iam';
import * as codebuild from 'aws-cdk-lib/aws-codebuild';

const app = new cdk.App();

const stack = new cdk.Stack(app, "LogLimitStack", {
env:{ region: "us-west-2" }
})
const logGroup = new logs.LogGroup(this, 'my-log-group-label', {
logGroupName: 'my-log-group-name',
removalPolicy: cdk.RemovalPolicy.RETAIN,
retention: logs.RetentionDays.ONE_WEEK,
});
const importedRole = iam.Role.fromRoleArn(this, 'ImportedRole',
`arn:aws:iam:::role/my-codebuild-role`
);
const project = new codebuild.PipelineProject(this, 'my-build-project-label', {
projectName: 'my-build-project-name',
role: importedRole,
buildSpec: codebuild.BuildSpec.fromObject({
version: '0.2',
phases: {
build: {
commands: [ 'echo "Hello, CodeBuild!"' ],
},
},
}),
logging : {
cloudWatch: { logGroup: logGroup }
}
});
```
The synthesized template will include the AWS::Logs::ResourcePolicy under the above conditions:
```
myloggrouplabel0A41AD36:
Type: AWS::Logs::LogGroup
Properties:
LogGroupName: my-log-group-name
RetentionInDays: 7
UpdateReplacePolicy: Retain
DeletionPolicy: Retain
Metadata:
aws:cdk:path: LogLimitStack/my-log-group-label/Resource
myloggrouplabelPolicyResourcePolicy95FDC6C6:
Type: AWS::Logs::ResourcePolicy
Properties:
PolicyDocument:
Fn::Join:
- ""
- - '{"Statement":[{"Action":["logs:CreateLogStream","logs:PutLogEvents"],"Effect":"Allow","Principal":{"AWS":""},"Resource":"'
- Fn::GetAtt:
- myloggrouplabel0A41AD36
- Arn
- '"}],"Version":"2012-10-17"}'
PolicyName: LogLimitStackmyloggrouplabelPolicy23DE1910
Metadata:
aws:cdk:path: LogLimitStack/my-log-group-label/Policy/ResourcePolicy
```

the PipelineProject will invoke the grantWrite() under the hood, and grantWrite() will infer information based on what role information is currently passed to the PipelineProject where it'll be created if an aws_iam.IRole is supplied or not:
- https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-codebuild/lib/project.ts#L1035-L1041

In the case of an imported role, to my understanding of the grant() method being used by "grantWrite()" this seems to try inferring if the IAM Role to reference is part of the same account or not via "addToPrincipalOrResource()". Because the stack is account agnostic, it then infers this to potentially be cross-account since we can't guarantee that the imported IAM role will be from the same target account.

Testing this out with a simple stack reflects this behavior when just isolating it to executing "grantWrite()" against an imported role resulting in the same ResourcePolicy to appear without my consent
```
const app = new cdk.App();

const stack = new cdk.Stack(app, "LogLimitStack", {
env:{ region: "us-west-2" }
})
const logGroup = new logs.LogGroup(stack, 'my-log-group-label', {
logGroupName: 'my-log-group-name',
removalPolicy: cdk.RemovalPolicy.RETAIN,
retention: logs.RetentionDays.ONE_WEEK,
});
const importedRole = iam.Role.fromRoleArn(stack, 'ImportedRole',
`arn:aws:iam:::role/my-codebuild-role`
);
logGroup.grantWrite(importedRole)
```
But when manually creating a role and granting it write permissions, the resource policy disappears within the account agnostic stack:
```
const app = new cdk.App();

const stack = new cdk.Stack(app, "LogLimitStack", {
env:{ region: "us-west-2" }
})
const logGroup = new logs.LogGroup(stack, 'my-log-group-label', {
logGroupName: 'my-log-group-name',
removalPolicy: cdk.RemovalPolicy.RETAIN,
retention: logs.RetentionDays.ONE_WEEK,
});
const actualRole = new iam.Role(stack, 'ActualCBRole', {
assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com'),
});
logGroup.grantWrite(actualRole)
```

Because this stems from the LogGroup Construct primarily, I was expecting this to potentially be documented explicitly on the aws_logs overview, or the LogGroup Construct document. I found on the overview that this may be implied:
```
Be aware that any ARNs or tokenized values passed to the resource policy will be converted into AWS Account IDs.
```
- https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_logs-readme.html

But given the unexpected results not being explicit/implied on the aws_codebuild.PipelineProject docs, it's understanding how this may be missed as users would have to dig into the source to eventually see this. But this is assuming my findings are correct and would like to hopefully get this cross-checked by others to confirm my understanding of this behavior.

Overall, this seems to be an expected but unfortunate outcome due to how "addToPrincipalOrResource()" infers information about the CDK Stack.

- https://github.com/aws/aws-cdk/blob/60a5b2a5d44474b1d58ceaf3fabee73836aa9882/packages/%40aws-cdk/aws-logs/lib/log-group.ts#L173-L175
- https://github.com/aws/aws-cdk/blob/60a5b2a5d44474b1d58ceaf3fabee73836aa9882/packages/%40aws-cdk/aws-logs/lib/log-group.ts#L193-L202
- https://github.com/aws/aws-cdk/blob/9d1093f133ea38ac7d0e9c5cca7e2ea91e71a754/packages/%40aws-cdk/aws-iam/lib/grant.ts#L122-L158

So, instead of modifying the behavior of "addToPrincipalOrResource()", instead it may just be easier to be clear with how the PipelineProject Construct configures logging automatically on behalf of the user. Otherwise, it may be easier to give users the option to disable it themselves by adding another property and checking if it's set to true or not.

### Acknowledgements

- [X] I may be able to implement this feature request
- [ ] This feature might incur a breaking change

### CDK version used

2.69.0

### Environment details (OS name and version, etc.)

Windows 10 | Node v16.13.0

Contributor guide

Open the contributing guide

Research direction

Start with packages/@aws-cdk/aws-codebuild/lib/project.ts at the CloudWatch logging configuration and review the linked CloudWatchLoggingOptions documentation. Then trace the related grant behavior in packages/@aws-cdk/aws-logs/lib/log-group.ts and packages/@aws-cdk/aws-iam/lib/grant.ts. Done means the chosen suppression or documentation behavior is explicit and the synthesized resource policy behavior is covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
devops, infrastructure
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.