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.62k stars 3.91k forks source link

[pipelines] generate CodeBuild reports within @aws-cdk/cdk-pipelines #10464

Open nbaillie opened 4 years ago

nbaillie commented 4 years ago

Description

To be able to easily use @aws-cdk/assert within @aws-cdk/cdk-pipelines. and use the reports function of CodeBuild.

Use Case

Running tests on CDK using @aws-cdk/assert to increase the ability to Fail Fast and implement TDD.

Proposed Solution

I have considered 2 options so far: ( there may be other options, and there may well be some other/better way to do this )

Option1: add a testAction that adds another stage to codepipeline using another codebuild project :

const pipeline = new CdkPipeline(this, 'Pipeline', {
      pipelineName: 'PipelineWithTests',
      sourceAction: ...
      synthAction: ...
      testAction: simpleTestAction(...)
    });

Where the new testAction includes support for reports in the buildSpec.

Option2: Enable adding reports to the existing SimpleSynthAction(and npm yarn variants) buildSpec: adding the needed changes to the build step commands "yarn build && yarn test". and adding along the lines of the bellow to the build spec perhaps with a testEnable Flag in the SynthAction.

reports:
  jest_reports:
    files:
      - <report filename>
    file-format: JUNITXML
    base-directory: <test report directory>

Other

I understand that this can be done by creating a codeBuild on its own and adding to the pipeline, but that breaks the elegance and simplicity of using cdk-pipelines which dose a great job at simplifying the codepipeline/buid experience.

I would expect that jest tests would be ran by yarn or npm as described in the @aws-cdk/assert docs. Reference: test-report-jest


This is a :rocket: Feature Request

BrianFarnhill commented 3 years ago

Looking at this, I think it could be made pretty simple by separating this out in to two different changes:

  1. Add support for a testCommand attribute to go along with the ones like install, build etc.
  2. Add an ability to specify report outputs

the report outputs could look like this:

    const synthAction = pipelines.SimpleSynthAction.standardNpmSynth({
        buildCommand: 'npm run build',
        testCommand: 'npm run test', //This would be new and doesn't exist today
        testReports: [{
            name: "GroupName" | "GroupArn", // standard syntax for the buildspec, if its not the ARN then the group gets created based on the name
            reportType: "Test" | "CodeCoverage",
            files: string[],
            fileFormat: "JUNITXML" | "OTHER_TYPES" | etc...,
            baseDirectory: string,
        }]
    });

This way the only real change we are making is that the testReports attribute feeds in to the generation of the buildspec, and you can still run your test command however you need to based on the type of project you have. This should mean we don't hit issues around different languages and ways of running tests (since you just use the testCommand to trigger the execution). The only thing I can think of here is that we would need to look at making sure that if you specify the testReports property I'm talkng about adding here, that we automatically add IAM permissions to put stuff in those groups in to the IAM policy for the build as well.

That's my thoughts on it at least - I'll keep giving it some thought though, and keen to hear other opinions.

nbaillie commented 3 years ago

@BrianFarnhill if you drop back to SimpleSynthAction from SimpleSynthAction.standardNpmSynth it already has the testCommand available. So would just be to do the second part as you have described above.

synthAction: new SimpleSynthAction({
        sourceArtifact,
        cloudAssemblyArtifact,
        installCommand: "yarn install --frozen-lockfile",
        buildCommand: `yarn build`,
        testCommands: [`yarn test unit`],
        synthCommand: `npx cdk synth`
}),

would become

synthAction: new SimpleSynthAction({
        sourceArtifact,
        cloudAssemblyArtifact,
        installCommand: "yarn install --frozen-lockfile",
        buildCommand: `yarn build`,
        testCommands: [`yarn test unit`],
        synthCommand: `npx cdk synth`,
        testReports: [{
            name: "GroupName" | "GroupArn",
            reportType: "Test" | "CodeCoverage",
            files: string[],
            fileFormat: "JUNITXML" | "OTHER_TYPES" | etc...,
            baseDirectory: string,
        }]
}),
mmuller88 commented 3 years ago

Until then you can hack the cdk a bit like that

new codebuild.ReportGroup(this, 'reportGroup', { reportGroupName: 'testReportGroup' });

const cfnBuildProject = pipeline.node.findAll().filter(child => child.node.id === 'CdkBuildProject')[0].node.defaultChild as codebuild.CfnProject;
    cfnBuildProject.source = {
      ...cfnBuildProject.source,
      buildSpec: JSON.stringify({
        version: '0.2',
        phases: {
          pre_build: {
            commands: ['yarn install--frozen - lockfile'],
          },
          build: {
            commands: ['yarn build', 'yarn test', 'npx cdk synth'],
          },
        },
        reports: {
          testReportGroup: {
            'files': ['**/*'],
            'base-directory': 'test-reports',
            'discard-paths': 'no',
          },
        },
        artifacts: { 'base-directory': 'cdk.out', 'files': '**/*' },
      }),

    };
samlaf commented 3 years ago

I found a (possibly better?) way to do it using CodePipelines as opposed to the older CdkPipelines Api. They mention here that the argument to synth can be either a ShellStep or a (more involved but customizable) CodeBuildStep. Note that synth is the new synthAction: image

Hence my solution looked something like:

const pipeline = new CodePipeline(this, ...)
const cdkPipelineStage = new MyCdkPipelineStage(this, ...)
const testReports = new ReportGroup(this, 'TestReports', {
  removalPolicy: RemovalPolicy.DESTROY,
});
const testStep = new CodeBuildStep('Test', {
  partialBuildSpec: BuildSpec.fromObject({
    reports: {
      [testReports.reportGroupArn]:{
          files: [
              '**/*',
          ],
          'base-directory': 'test/newman',
          'discard-paths': true,
      }
    },
  }),
  installCommands: [...],
  commands: [...]
})
const testStage = pipeline.addStage(cdkPipelineStage, {
  post: [ testStep ]
})

pipeline.buildPipeline();
testReports.grantWrite(testStep.grantPrincipal);
bilalq commented 2 years ago

Thanks for sharing that example, @samlaf. That seems to be the only sane way of doing this right now. This really should be easier to do without the extra complexity of having to create and permission grant ReportGroup definitions manually.

bilalq commented 2 years ago

Based on the sample you shared, this is a working example I came up with to get both test and coverage reports working:

import { BuildSpec, ReportGroup, CfnReportGroup } from '@aws-cdk/aws-codebuild'
import * as cdk from '@aws-cdk/core'
import * as pipelines from '@aws-cdk/pipelines'
import * as iam from '@aws-cdk/aws-iam'

interface SomePipelineProps extends cdk.StackProps {
  // ...
}

export class SomePipeline extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props: SomePipelineProps) {
    super(scope, id, props)

    const repo = 'Your Repo here'
    const branch = 'Your branch here'
    const codeStarConnection = pipelines.CodePipelineSource.connection(repo, branch, {
      connectionArn: 'Your CodeStar connection ARN here',
      triggerOnPush: true
    })

    const jestReportGroup = new ReportGroup(this, 'JestReportGroup', {})
    const unitTestCoverageGroup = new ReportGroup(this, 'CoverageReportGroup', {})
    const cfnUnitTestCoverageGroup = unitTestCoverageGroup.node.defaultChild as CfnReportGroup
    cfnUnitTestCoverageGroup.type = 'CODE_COVERAGE'

    const customSynthStep = new pipelines.CodeBuildStep('ExperimentalSynth', {
      input: codeStarConnection,
      env: {
        CI: 'true'
      },
      installCommands: ['npm ci'],
      commands: ['npm run build', 'npm test', 'npx cdk synth'],
      partialBuildSpec: BuildSpec.fromObject({
        version: '0.2',
        // Make sure your jest config outputs to locations that match what's here
        reports: {
          [jestReportGroup.reportGroupArn]: {
            files: ['jest-results.xml'],
            'file-format': 'JUNITXML',
            'base-directory': 'build/reports/unit-test'
          },
          [unitTestCoverageGroup.reportGroupArn]: {
            files: ['clover.xml'],
            'file-format': 'CLOVERXML',
            'base-directory': 'build/reports/coverage'
          }
        }
      }),
      primaryOutputDirectory: 'cdk.out'
    })

    const pipeline = new pipelines.CodePipeline(this, 'SomePipeline', {
      pipelineName: 'SomePipeline',
      synth: customSynthStep
    })

    // Do whatever else you want on your pipeline
    // ...

    // Granting access here has to happen after calling buildPipeline so the CloudAssembly references resolve
    // All of this is only necessary becasue of limitations discussed in https://github.com/aws/aws-cdk/issues/10464
    pipeline.buildPipeline()
    jestReportGroup.grantWrite(customSynthStep.grantPrincipal)
    unitTestCoverageGroup.grantWrite(customSynthStep.grantPrincipal)
    // This next grant is necessary because the grantWrite command won't add the permission needed for coverage reports
    // https://github.com/aws/aws-cdk/issues/14279
    iam.Grant.addToPrincipal({
      grantee: customSynthStep.grantPrincipal,
      actions: ['codebuild:BatchPutCodeCoverages'],
      resourceArns: [unitTestCoverageGroup.reportGroupArn]
    })
  }
}
adambiggs commented 2 years ago

Thanks @bilalq your example worked great!

Would be much better if there was a clean API for this in CDK pipelines though.

moltar commented 2 years ago

I have almost the same setup as @bilalq, but I get the following error:

Error in UPLOAD_ARTIFACTS phase: [arn:aws:codebuild:us-east-1:123:report-group/Cypress: [error creating report: AccessDeniedException: User: arn:aws:sts::123:assumed-role/Pipeline-CodePipelinePPSTGCypressTestRole875C9A71-XTKHID4CWNBM/AWSCodeBuild-62f3a3b1-951c-4f56-beaa-7d223103b8ce is not authorized to perform: codebuild:CreateReport on resource: arn:aws:codebuild:us-east-1:123:report-group/Cypress because no identity-based policy allows the codebuild:CreateReport action

Any ideas?

github-actions[bot] commented 10 months ago

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.