Skip to content

Commit

Permalink
chore: move testrunning to separate workflow
Browse files Browse the repository at this point in the history
Necessary for permissions.
  • Loading branch information
rix0rrr committed Jan 14, 2025
1 parent 4acaf77 commit ea9e240
Show file tree
Hide file tree
Showing 6 changed files with 222 additions and 86 deletions.
1 change: 1 addition & 0 deletions .gitattributes

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

113 changes: 113 additions & 0 deletions .github/workflows/build-and-integ.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

84 changes: 0 additions & 84 deletions .github/workflows/build.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .projen/files.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

108 changes: 106 additions & 2 deletions .projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as pj from 'projen';
// https://github.com/microsoft/TypeScript/issues/60159
const TYPESCRIPT_VERSION = '5.6';

const APPROVAL_ENVIRONMENT = 'integ-approval';
const TEST_ENVIRONMENT = 'run-tests';

// This is for the test workflow, to know which artifacts to zip up
Expand Down Expand Up @@ -229,11 +230,100 @@ for (const compiledDir of compiledDirs) {
cliInteg.gitignore.addPatterns('!resources/**/*.js');

//////////////////////////////////////////////////////////////////////
// Add a job for running the tests to the global 'build' workflow
// Add a workflow for running the tests
//
// This MUST be a separate workflow that runs in privileged context. We have a couple
// of options:
//
// - `workflow_run`: we can trigger a privileged workflow run after the unprivileged
// `pull_request` workflow finishes and reuse its output artifacts. The
// problem is that the second run is disconnected from the PR so we would need
// to script in visibility for approvals and success (by posting comments, for
// example)
// - Use only a `pull_request_target` workflow on the PR: this either would run
// a privileged workflow on any user code submission (might be fine given the
// workflow's `permissions`, but I'm sure this will make our security team uneasy
// anyway), OR this would mean any build needs human confirmation which means slow
// feedback.
// - Use a `pull_request` for a regular fast-feedback build, and a separate
// `pull_request_target` for the integ tests. This means we're building twice.
//
// Ultimately, our build isn't heavy enough to put in a lot of effort deduping
// it, so we'll go with the simplest solution which is the last one: 2
// independent workflows.
//
// projen doesn't make it easy to copy the relevant parts of the 'build' workflow,
// so they're unfortunately duplicated here.

repo.buildWorkflow?.addPostBuildJob("run-tests", {
const buildWorkflow = repo.buildWorkflow;
const runTestsWorkflow = repo.github?.addWorkflow('build-and-integ');
if (!buildWorkflow || !runTestsWorkflow) {
throw new Error('Expected build and run tests workflow');
}
((buildWorkflow as any).workflow as pj.github.GithubWorkflow)


runTestsWorkflow.on({
pullRequestTarget: {
branches: ['main'],
},
});
// The 'build' part runs on the 'integ-approval' environment, which requires
// approval. The actual runs access the real environment, not requiring approval
// anymore.
runTestsWorkflow.addJob('build', {
environment: APPROVAL_ENVIRONMENT,
runsOn: workflowRunsOn,
permissions: {
contents: pj.github.workflows.JobPermission.READ,
},
env: {
CI: 'true',
},
steps: [
{
name: 'Checkout',
uses: 'actions/checkout@v4',
with: {
ref: '${{ github.event.pull_request.head.ref }}',
repository: '${{ github.event.pull_request.head.repo.full_name }}',
}
},
{
name: 'Setup Node.js',
uses: 'actions/setup-node@v4',
with: {
'node-version': 'lts/*',
}
},
{
name: 'Install dependencies',
run: 'yarn install --check-files',
},
{
name: 'build',
run: 'npx projen build',
},
{
name: 'Backup artifact permissions',
run: 'cd packages/@aws-cdk-testing/cli-integ/dist/js && getfacl -R . > permissions-backup.acl',
continueOnError: true,
},
{
name: 'Upload artifact',
uses: 'actions/[email protected]',
with: {
name: 'build-artifact',
path: 'packages/@aws-cdk-testing/cli-integ/dist/js',
overwrite: 'true',
}
},
],
});
runTestsWorkflow.addJob('integ', {
environment: TEST_ENVIRONMENT,
runsOn: workflowRunsOn,
needs: ['build'],
permissions: {
contents: pj.github.workflows.JobPermission.READ,
idToken: pj.github.workflows.JobPermission.WRITE,
Expand All @@ -244,6 +334,7 @@ repo.buildWorkflow?.addPostBuildJob("run-tests", {
// This is not actually a canary, but this prevents the tests from making
// assumptions about the availability of source packages.
IS_CANARY: 'true',
CI: 'true',
},
strategy: {
failFast: false,
Expand All @@ -265,6 +356,19 @@ repo.buildWorkflow?.addPostBuildJob("run-tests", {
},
},
steps: [
{
name: 'Download build artifacts',
uses: 'actions/download-artifact@v4',
with: {
name: 'build-artifact',
path: 'packages/@aws-cdk-testing/cli-integ/dist/js',
}
},
{
name: 'Restore build artifact permissions',
run: 'cd packages/@aws-cdk-testing/cli-integ/dist/js && setfacl --restore=permissions-backup.acl',
continueOnError: true,
},
{
name: 'Set up JDK 18',
if: 'matrix.suite == \'init-java\' || matrix.suite == \'cli-integ-tests\'',
Expand Down

0 comments on commit ea9e240

Please sign in to comment.