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

(codebuild): ReportGroup grants test permissions when set to CODE_COVERAGE #21534

Closed
ghdoergeloh opened this issue Aug 10, 2022 · 4 comments · Fixed by #21656
Closed

(codebuild): ReportGroup grants test permissions when set to CODE_COVERAGE #21534

ghdoergeloh opened this issue Aug 10, 2022 · 4 comments · Fixed by #21656
Labels
@aws-cdk/aws-codebuild Related to AWS CodeBuild bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@ghdoergeloh
Copy link

Describe the bug

grantWrite on a ReportGroup only applies 'codebuild:CreateReport', 'codebuild:UpdateReport', 'codebuild:BatchPutTestCases'. If you use a ReportGroup with type: "CODE_COVERAGE" it still sets those actions as allowed, but misses the 'codebuild:BatchPutCodeCoverages'.

Expected Behavior

new ReportGroup(...,{type: ReportGroupType.TEST} ).grantWrite(codeBuildStep)
should add a policy statement with 'codebuild:CreateReport', 'codebuild:UpdateReport', 'codebuild:BatchPutTestCases'

while

new ReportGroup(...,{type: ReportGroupType.CODE_COVERAGE} ).grantWrite(codeBuildStep)
should add a policy statement with 'codebuild:CreateReport', 'codebuild:UpdateReport', 'codebuild:BatchPutCodeCoverages'

Current Behavior

new ReportGroup(...,{type: ReportGroupType.CODE_COVERAGE} ).grantWrite(codeBuildStep)
adds a policy statement with 'codebuild:CreateReport', 'codebuild:UpdateReport', 'codebuild:BatchPutTestCases'

Reproduction Steps

    const testReports = new ReportGroup(this, 'TestReports', {
      type: ReportGroupType.TEST,
    });
    const coverageReports = new ReportGroup(this, 'CoverageReports', {
      type: ReportGroupType.CODE_COVERAGE,
    });

    const buildAndTestStep = new CodeBuildStep('BuildAndTestStep', {
    ...
      partialBuildSpec: BuildSpec.fromObject({
      ...
        reports: {
          [coverageReports.reportGroupArn]: {
            'file-format': 'CLOVERXML',
            files: ['coverage/clover.xml'],
          },
          [testReports.reportGroupArn]: {
            'file-format': 'JUNITXML',
            files: ['**/junit.xml'],
          },
        },
      }),
    });
    ...
    const pipeline = new CodePipeline(this, 'Pipeline', {
      synth: buildAndTestStep,
    });
    
    pipeline.buildPipeline();

    testReports.grantWrite(buildAndTestStep);
    coverageReports.grantWrite(buildAndTestStep);

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.37.0

Framework Version

No response

Node.js Version

16.15.1

OS

Ubuntu 22.04 LTS (Linux 5.15.0-43)

Language

Typescript

Language Version

4.7.4

Other information

No response

@ghdoergeloh ghdoergeloh added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 10, 2022
@github-actions github-actions bot added the @aws-cdk/aws-codebuild Related to AWS CodeBuild label Aug 10, 2022
@peterwoodworth peterwoodworth changed the title (module name): (short issue description) (codebuild): ReportGroup grants test permissions when set to CODE_COVERAGE Aug 12, 2022
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2022
@peterwoodworth
Copy link
Contributor

Yeah it looks like this got overlooked when we added the code coverage option. We can adjust the grantwrite method to consider what type of ReportGroup is getting created here

public grantWrite(identity: iam.IGrantable): iam.Grant {
const ret = iam.Grant.addToPrincipal({
grantee: identity,
actions: [
'codebuild:CreateReport',
'codebuild:UpdateReport',
'codebuild:BatchPutTestCases',
],
resourceArns: [this.reportGroupArn],
});

I'm marking this as p2, which means we won't be able to get to this issue soon, but are willing to review any PRs for this issue 🙂

@daschaa
Copy link
Contributor

daschaa commented Aug 16, 2022

@peterwoodworth Since I introduced this bug, I will take care of it. 🤕

@peterwoodworth
Copy link
Contributor

Thanks a bunch @daschaa 🙂

@mergify mergify bot closed this as completed in #21656 Aug 18, 2022
mergify bot pushed a commit that referenced this issue Aug 18, 2022
…_COVERAGE (#21656)

Fixes #21534

----

### 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

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] 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*
@github-actions
Copy link

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

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
…_COVERAGE (aws#21656)

Fixes aws#21534

----

### 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

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] 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*
ghdoergeloh added a commit to ghdoergeloh/cdk-monorepo-template that referenced this issue Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-codebuild Related to AWS CodeBuild bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants