diff --git a/.github/scripts/build.js b/.github/scripts/build.js index 3e791dbc5..09e74d3a3 100644 --- a/.github/scripts/build.js +++ b/.github/scripts/build.js @@ -5,7 +5,8 @@ // These tests can be run from the dev container using the run-tests.sh script // const { createHash } = require('crypto'); -const { create } = require('domain'); + +const SKIP_DEPLOYMENT_FLAG = "skip_deployment"; async function getCommandFromComment({ core, context, github }) { const commentUsername = context.payload.comment.user.login; @@ -51,6 +52,7 @@ async function getCommandFromComment({ core, context, github }) { const commentBody = context.payload.comment.body; const commentFirstLine = commentBody.split("\n")[0]; let command = "none"; + let skipDeployment = false; const trimmedFirstLine = commentFirstLine.trim(); if (trimmedFirstLine[0] === "/") { // only allow actions for users with write access @@ -83,6 +85,7 @@ async function getCommandFromComment({ core, context, github }) { const runTests = await handleTestCommand({ core, github }, parts, "tests", runId, { number: prNumber, authorUsername: prAuthorUsername, repoOwner, repoName, headSha: prHeadSha, refId: prRefId, details: pr }, { username: commentUsername, link: commentLink }); if (runTests) { command = "run-tests"; + skipDeployment = commandHasSkipDeploymentFlag(parts); } break; } @@ -92,12 +95,18 @@ async function getCommandFromComment({ core, context, github }) { const runTests = await handleTestCommand({ core, github }, parts, "extended tests", runId, { number: prNumber, authorUsername: prAuthorUsername, repoOwner, repoName, headSha: prHeadSha, refId: prRefId, details: pr }, { username: commentUsername, link: commentLink }); if (runTests) { command = "run-tests-extended"; + skipDeployment = commandHasSkipDeploymentFlag(parts); } break; } case "/test-extended-aad": { + if (commandHasSkipDeploymentFlag(parts)) { + await addUnsupportedSkipDeploymentFlagComment({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, commandText); + break; + } + const runTests = await handleTestCommand({ core, github }, parts, "extended AAD tests", runId, { number: prNumber, authorUsername: prAuthorUsername, repoOwner, repoName, headSha: prHeadSha, refId: prRefId, details: pr }, { username: commentUsername, link: commentLink }); if (runTests) { command = "run-tests-extended-aad"; @@ -107,6 +116,11 @@ async function getCommandFromComment({ core, context, github }) { case "/test-shared-services": { + if (commandHasSkipDeploymentFlag(parts)) { + await addUnsupportedSkipDeploymentFlagComment({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, commandText); + break; + } + const runTests = await handleTestCommand({ core, github }, parts, "shared service tests", runId, { number: prNumber, authorUsername: prAuthorUsername, repoOwner, repoName, headSha: prHeadSha, refId: prRefId, details: pr }, { username: commentUsername, link: commentLink }); if (runTests) { command = "run-tests-shared-services"; @@ -116,6 +130,11 @@ async function getCommandFromComment({ core, context, github }) { case "/test-backups": { + if (commandHasSkipDeploymentFlag(parts)) { + await addUnsupportedSkipDeploymentFlagComment({ github }, repoOwner, repoName, prNumber, commentUsername, commentLink, commandText); + break; + } + const runTests = await handleTestCommand({ core, github }, parts, "backup tests", runId, { number: prNumber, authorUsername: prAuthorUsername, repoOwner, repoName, headSha: prHeadSha, refId: prRefId, details: pr }, { username: commentUsername, link: commentLink }); if (runTests) { command = "run-tests-backups"; @@ -147,6 +166,7 @@ async function getCommandFromComment({ core, context, github }) { break; } } + logAndSetOutput(core, "skipDeployment", skipDeployment.toString()); logAndSetOutput(core, "command", command); return command; } @@ -168,19 +188,19 @@ async function handleTestCommand({ core, github }, commandParts, testDescription const prAuthorHasWriteAccess = await userHasWriteAccessToRepo({ core, github }, pr.authorUsername, pr.repoOwner, pr.repoName); const externalPr = !prAuthorHasWriteAccess; if (externalPr) { - if (commandParts.length === 1) { + const commentSha = getShaFromCommandParts(commandParts); + if (!commentSha) { const message = `:warning: When using \`${command}\` on external PRs, the SHA of the checked commit must be specified`; await addActionComment({ github }, pr.repoOwner, pr.repoName, pr.number, comment.username, comment.link, message); return false; } - const commentSha = commandParts[1]; if (commentSha.length < 7) { const message = `:warning: When specifying a commit SHA it must be at least 7 characters (received \`${commentSha}\`)`; await addActionComment({ github }, pr.repoOwner, pr.repoName, pr.number, comment.username, comment.link, message); return false; } if (!pr.headSha.startsWith(commentSha)) { - const message = `:warning: The specified SHA \`${commentSha}\` is not the latest commit on the PR. Please validate the latest commit and re-run \`/test\``; + const message = `:warning: The specified SHA \`${commentSha}\` is not the latest commit on the PR. Please validate the latest commit and re-run \`${command}\``; await addActionComment({ github }, pr.repoOwner, pr.repoName, pr.number, comment.username, comment.link, message); return false; } @@ -192,6 +212,19 @@ async function handleTestCommand({ core, github }, commandParts, testDescription } +function commandHasSkipDeploymentFlag(commandParts) { + return commandParts.slice(1).includes(SKIP_DEPLOYMENT_FLAG); +} + +function getShaFromCommandParts(commandParts) { + return commandParts.slice(1).find(part => part !== SKIP_DEPLOYMENT_FLAG); +} + +async function addUnsupportedSkipDeploymentFlagComment({ github }, repoOwner, repoName, prNumber, commentUser, commentLink, command) { + const message = `:warning: \`${SKIP_DEPLOYMENT_FLAG}\` is only supported for \`/test\` and \`/test-extended\`. Please re-run \`${command}\` without \`${SKIP_DEPLOYMENT_FLAG}\`.`; + await addActionComment({ github }, repoOwner, repoName, prNumber, commentUser, commentLink, message); +} + async function prContainsNonDocChanges(github, repoOwner, repoName, prNumber) { const prFilesResponse = await github.paginate(github.rest.pulls.listFiles, { owner: repoOwner, @@ -255,8 +288,8 @@ async function showHelp({ github }, repoOwner, repoName, prNumber, commentUser, const body = `${leadingContent} You can use the following commands: -    /test - build, deploy and run smoke tests on a PR -    /test-extended - build, deploy and run smoke & extended tests on a PR +    /test [] [skip_deployment] - build, deploy and run smoke tests on a PR +    /test-extended [] [skip_deployment] - build, deploy and run smoke & extended tests on a PR     /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR     /test-shared-services - test the deployment of shared services on a PR build     /test-backups - build, deploy and run backup tests on a PR diff --git a/.github/scripts/build.test.js b/.github/scripts/build.test.js index 95e6b9b97..b19f70af4 100644 --- a/.github/scripts/build.test.js +++ b/.github/scripts/build.test.js @@ -125,6 +125,27 @@ describe('getCommandFromComment', () => { expect(outputFor(mockCoreSetOutput, 'command')).toBe('run-tests'); }); + test(`should set skipDeployment to 'true' when skip_deployment is supplied`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test skip_deployment', + pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES, + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'command')).toBe('run-tests'); + expect(outputFor(mockCoreSetOutput, 'skipDeployment')).toBe('true'); + }); + + test(`should set skipDeployment to 'false' by default`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test', + pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES, + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'skipDeployment')).toBe('false'); + }); + test(`should set nonDocsChanges to 'true'`, async () => { const context = createCommentContext({ username: 'admin', @@ -331,6 +352,20 @@ describe('getCommandFromComment', () => { }); }) + describe(`for '/test 2345678 skip_deployment' for external PR`, () => { + test(`should set command to 'run-tests' and skipDeployment to 'true'`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test 2345678 skip_deployment', + pullRequestNumber: PR_NUMBER.FORK_NON_DOCS_CHANGES, + authorUsername: 'non-contributor', + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'command')).toBe('run-tests'); + expect(outputFor(mockCoreSetOutput, 'skipDeployment')).toBe('true'); + }); + }) + describe(`for '/test 2345678' for external PR (i.e. with latest commit SHA specified but extra space after test)`, () => { test(`should set command to 'run-tests'`, async () => { const context = createCommentContext({ @@ -370,6 +405,16 @@ describe('getCommandFromComment', () => { expect(outputFor(mockCoreSetOutput, 'command')).toBe('run-tests-extended'); }); + test(`should set skipDeployment to 'true' when skip_deployment is supplied`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test-extended skip_deployment', + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'command')).toBe('run-tests-extended'); + expect(outputFor(mockCoreSetOutput, 'skipDeployment')).toBe('true'); + }); + test(`should add comment with run link`, async () => { const context = createCommentContext({ username: 'admin', @@ -410,6 +455,21 @@ describe('getCommandFromComment', () => { bodyMatcher: /Running extended AAD tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/, }); }); + + test(`should warn and not run if skip_deployment is supplied`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test-extended-aad skip_deployment', + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'command')).toBe('none'); + expect(mockGithubRestIssuesCreateComment).toHaveComment({ + owner: 'someOwner', + repo: 'someRepo', + issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES, + bodyMatcher: /`skip_deployment` is only supported for `\/test` and `\/test-extended`\. Please re-run `\/test-extended-aad` without `skip_deployment`\./, + }); + }); }); describe(`for '/test-shared-services'`, () => { @@ -436,6 +496,21 @@ describe('getCommandFromComment', () => { bodyMatcher: /Running shared service tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/, }); }); + + test(`should warn and not run if skip_deployment is supplied`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test-shared-services skip_deployment', + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'command')).toBe('none'); + expect(mockGithubRestIssuesCreateComment).toHaveComment({ + owner: 'someOwner', + repo: 'someRepo', + issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES, + bodyMatcher: /`skip_deployment` is only supported for `\/test` and `\/test-extended`\. Please re-run `\/test-shared-services` without `skip_deployment`\./, + }); + }); }); describe(`for '/test-backups'`, () => { @@ -462,6 +537,21 @@ describe('getCommandFromComment', () => { bodyMatcher: /Running backup tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/, }); }); + + test(`should warn and not run if skip_deployment is supplied`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test-backups skip_deployment', + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'command')).toBe('none'); + expect(mockGithubRestIssuesCreateComment).toHaveComment({ + owner: 'someOwner', + repo: 'someRepo', + issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES, + bodyMatcher: /`skip_deployment` is only supported for `\/test` and `\/test-extended`\. Please re-run `\/test-backups` without `skip_deployment`\./, + }); + }); }); describe(`for '/test-extended' for external PR (i.e. without commit SHA specified)`, () => { @@ -517,7 +607,7 @@ describe('getCommandFromComment', () => { owner: 'someOwner', repo: 'someRepo', issue_number: PR_NUMBER.FORK_NON_DOCS_CHANGES, - bodyMatcher: /The specified SHA `00000000` is not the latest commit on the PR. Please validate the latest commit and re-run `\/test`/, + bodyMatcher: /The specified SHA `00000000` is not the latest commit on the PR. Please validate the latest commit and re-run `\/test-extended`/, }); }); }) @@ -580,6 +670,20 @@ describe('getCommandFromComment', () => { }); }) + describe(`for '/test-extended skip_deployment 2345678' for external PR`, () => { + test(`should set command to 'run-tests-extended' and skipDeployment to 'true'`, async () => { + const context = createCommentContext({ + username: 'admin', + body: '/test-extended skip_deployment 2345678', + pullRequestNumber: PR_NUMBER.FORK_NON_DOCS_CHANGES, + authorUsername: 'non-contributor', + }); + await getCommandFromComment({ core, context, github }); + expect(outputFor(mockCoreSetOutput, 'command')).toBe('run-tests-extended'); + expect(outputFor(mockCoreSetOutput, 'skipDeployment')).toBe('true'); + }); + }) + describe(`for '/test-force-approve'`, () => { test(`should set command to 'test-force-approve'`, async () => { const context = createCommentContext({ diff --git a/.github/workflows/deploy_tre_reusable.yml b/.github/workflows/deploy_tre_reusable.yml index 2141aea5f..ad9e56c8b 100644 --- a/.github/workflows/deploy_tre_reusable.yml +++ b/.github/workflows/deploy_tre_reusable.yml @@ -45,6 +45,11 @@ on: # yamllint disable-line rule:truthy type: string default: "" required: false + skipDeployment: + description: Skip deployment jobs and run only the requested E2E tests against an existing environment + type: boolean + default: false + required: false secrets: AAD_TENANT_ID: description: "" @@ -120,6 +125,7 @@ concurrency: "deploy-${{ inputs.ciGitRef }}" jobs: deploy_management: name: Deploy Management + if: ${{ !inputs.skipDeployment }} runs-on: ubuntu-latest permissions: id-token: write @@ -832,6 +838,13 @@ jobs: e2e_tests_smoke: name: "Run E2E Tests (Smoke)" + # When skipDeployment is true, the deploy jobs in needs are intentionally skipped. + # always() lets this job still evaluate its guard and run against the existing PR environment. + if: >- + ${{ always() && + (inputs.skipDeployment || !contains(needs.*.result, 'skipped')) && + !contains(needs.*.result, 'failure') && + !contains(needs.*.result, 'cancelled') }} runs-on: ubuntu-latest environment: ${{ inputs.environmentName }} needs: [deploy_shared_services, register_bundles, deploy_ui] @@ -876,7 +889,12 @@ jobs: e2e_tests_custom: name: "Run E2E Tests" - if: ${{ inputs.e2eTestsCustomSelector != '' }} + # Same as smoke tests: skipDeployment intentionally bypasses deploy jobs but still runs E2E tests. + if: >- + ${{ always() && inputs.e2eTestsCustomSelector != '' && + (inputs.skipDeployment || !contains(needs.*.result, 'skipped')) && + !contains(needs.*.result, 'failure') && + !contains(needs.*.result, 'cancelled') }} runs-on: ubuntu-latest environment: ${{ inputs.environmentName }} needs: @@ -937,7 +955,9 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} # For PR builds triggered from comment builds, the GITHUB_REF is set to main # so the checks aren't automatically associated with the PR - # If prHeadSha is specified then explicity mark the checks for that SHA + # If prHeadSha is specified then explicitly mark the checks for that SHA. + # This still runs after skipDeployment builds because the summary job needs the E2E jobs, + # not the skipped deploy jobs. - name: Report check status if: inputs.prHeadSha != '' uses: LouisBrunner/checks-action@dfcbcf801bff1ea7f1414824fc28f2cd697b35da # v3.0.0 diff --git a/.github/workflows/pr_comment_bot.yml b/.github/workflows/pr_comment_bot.yml index a2d5e140c..39bd448b9 100644 --- a/.github/workflows/pr_comment_bot.yml +++ b/.github/workflows/pr_comment_bot.yml @@ -30,8 +30,9 @@ jobs: prRef: ${{ steps.check_command.outputs.prRef }} prHeadSha: ${{ steps.check_command.outputs.prHeadSha }} prRefId: ${{ steps.check_command.outputs.prRefId }} - branchRefId: ${{ steps.check_command.outputs.branchRefid }} + branchRefId: ${{ steps.check_command.outputs.branchRefId }} ciGitRef: ${{ steps.check_command.outputs.ciGitRef }} + skipDeployment: ${{ steps.check_command.outputs.skipDeployment }} steps: # Ensure we have the script file for the github-script action to use - name: Checkout @@ -56,7 +57,8 @@ jobs: echo "prHeadSha : ${{ steps.check_command.outputs.prHeadSha }}" echo "prRefId : ${{ steps.check_command.outputs.prRefId }}" echo "ciGitRef : ${{ steps.check_command.outputs.ciGitRef }}" - echo "branchRefId : ${{ steps.check_command.outputs.prRefId }}" + echo "branchRefId : ${{ steps.check_command.outputs.branchRefId }}" + echo "skipDeploy : ${{ steps.check_command.outputs.skipDeployment }}" # If we don't run the actual deploy (see the run_test job below) we won't receive a check-run status, # and will have to send it "manually" @@ -180,6 +182,8 @@ jobs: environmentName: CICD E2E_TESTS_NUMBER_PROCESSES: 1 DEVCONTAINER_TAG: ${{ needs.pr_comment.outputs.prRefId }} + # github-script outputs strings; compare explicitly before passing to the typed workflow input. + skipDeployment: ${{ needs.pr_comment.outputs.skipDeployment == 'true' }} secrets: AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} diff --git a/docs/tre-developers/github-pr-bot-commands.md b/docs/tre-developers/github-pr-bot-commands.md index e95be1823..5a8b6af8e 100644 --- a/docs/tre-developers/github-pr-bot-commands.md +++ b/docs/tre-developers/github-pr-bot-commands.md @@ -12,7 +12,7 @@ These commands can only be run when commented by a user who is identified as a r This command will cause the pr-comment-bot to respond with a comment listing the available commands. -### `/test []` +### `/test [] [skip_deployment]` This command runs the build, deploy, and smoke tests for a PR. @@ -21,6 +21,10 @@ For PRs from maintainers (i.e. users with write access to microsoft/AzureTRE), ` For other PRs, the checks below should be carried out. Once satisfied that the PR is safe to run tests against, you should use `/test ` where `` is the SHA for the commit that you have verified. You can use the full or short form of the SHA, but it must be at least 7 characters (GitHub UI shows 7 characters). +If the PR validation environment is already deployed, add `skip_deployment` to skip the build and deployment jobs and run the smoke tests against the existing environment. For PRs from forks, include both the SHA and `skip_deployment`, for example `/test skip_deployment`. + +The `skip_deployment` flag is only supported by `/test` and `/test-extended`. + **IMPORTANT** This command works on PRs from forks, and makes the deployment secrets available. @@ -32,7 +36,7 @@ Check for changes to anything that is run during the build/deploy/test cycle, in - modifications to scripts - new python packages being installed -### `/test-extended []` / `/test-extended-aad []`/ `/test-shared-services []` +### `/test-extended [] [skip_deployment]` / `/test-extended-aad []`/ `/test-shared-services []` This command runs the build, deploy, and smoke & extended / shared services tests for a PR. @@ -43,6 +47,10 @@ If a change has been made which would affect any of the core shared services, ma For other PRs, the checks below should be carried out. Once satisfied that the PR is safe to run tests against, you should use `/test-extended ` where `` is the SHA for the commit that you have verified. You can use the full or short form of the SHA, but it must be at least 7 characters (GitHub UI shows 7 characters). +If the PR validation environment is already deployed, add `skip_deployment` to `/test-extended` to skip the build and deployment jobs and run the smoke and extended tests against the existing environment. For PRs from forks, include both the SHA and `skip_deployment`, for example `/test-extended skip_deployment`. + +The `skip_deployment` flag is not supported by `/test-extended-aad` or `/test-shared-services`. + **IMPORTANT** As with `/test`, this command works on PRs from forks, and makes the deployment secrets available.