From b296f2676c59e1cadc443ea4d3ed387d4f4e59a2 Mon Sep 17 00:00:00 2001 From: Angela P Wen Date: Wed, 11 Sep 2024 15:09:29 -0700 Subject: [PATCH] Refactor: upload all available debug artifacts in init-post Previously, we uploaded SARIF artifacts in the `analyze-post` step and database and log artifacts in the `init-post` step. As we migrate to the updated `artifact` dependencies, we want to switch to uploading all artifacts in one step. In order to upload all artifacts in one go and maintain the artifacts at the root of the debug directory, we first move SARIF artifacts to the database directory. This should not affect any other consumers of the SARIF file as this occurs in the `init-post` step. --- src/analyze-action-post-helper.test.ts | 57 ----------- src/analyze-action-post-helper.ts | 20 +--- src/analyze-action-post.ts | 2 +- src/analyze-action.ts | 1 + src/debug-artifacts.ts | 134 ++++++++++++------------- src/environment.ts | 3 + src/init-action-post-helper.test.ts | 18 ++-- src/init-action-post-helper.ts | 7 +- src/init-action-post.ts | 3 +- 9 files changed, 78 insertions(+), 167 deletions(-) delete mode 100644 src/analyze-action-post-helper.test.ts diff --git a/src/analyze-action-post-helper.test.ts b/src/analyze-action-post-helper.test.ts deleted file mode 100644 index 628b2b6f7..000000000 --- a/src/analyze-action-post-helper.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -import test from "ava"; -import * as sinon from "sinon"; - -import * as actionsUtil from "./actions-util"; -import * as analyzeActionPostHelper from "./analyze-action-post-helper"; -import * as configUtils from "./config-utils"; -import { setupTests } from "./testing-utils"; -import * as util from "./util"; - -setupTests(test); - -test("post: analyze action with debug mode off", async (t) => { - return await util.withTmpDir(async (tmpDir) => { - process.env["RUNNER_TEMP"] = tmpDir; - - const gitHubVersion: util.GitHubVersion = { - type: util.GitHubVariant.DOTCOM, - }; - sinon.stub(configUtils, "getConfig").resolves({ - debugMode: false, - gitHubVersion, - languages: [], - packs: [], - } as unknown as configUtils.Config); - - const uploadSarifSpy = sinon.spy(); - - await analyzeActionPostHelper.run(uploadSarifSpy); - - t.assert(uploadSarifSpy.notCalled); - }); -}); - -test("post: analyze action with debug mode on", async (t) => { - return await util.withTmpDir(async (tmpDir) => { - process.env["RUNNER_TEMP"] = tmpDir; - - const gitHubVersion: util.GitHubVersion = { - type: util.GitHubVariant.DOTCOM, - }; - sinon.stub(configUtils, "getConfig").resolves({ - debugMode: true, - gitHubVersion, - languages: [], - packs: [], - } as unknown as configUtils.Config); - - const requiredInputStub = sinon.stub(actionsUtil, "getRequiredInput"); - requiredInputStub.withArgs("output").returns("fake-output-dir"); - - const uploadSarifSpy = sinon.spy(); - - await analyzeActionPostHelper.run(uploadSarifSpy); - - t.assert(uploadSarifSpy.called); - }); -}); diff --git a/src/analyze-action-post-helper.ts b/src/analyze-action-post-helper.ts index 3da0125a6..dc4040794 100644 --- a/src/analyze-action-post-helper.ts +++ b/src/analyze-action-post-helper.ts @@ -1,15 +1,8 @@ -import * as core from "@actions/core"; - import * as actionsUtil from "./actions-util"; -import { Config, getConfig } from "./config-utils"; +import { getConfig } from "./config-utils"; import { getActionsLogger } from "./logging"; -export async function run( - uploadSarifDebugArtifact: ( - config: Config, - outputDir: string, - ) => Promise, -) { +export async function run() { const logger = getActionsLogger(); const config = await getConfig(actionsUtil.getTemporaryDirectory(), logger); @@ -18,13 +11,4 @@ export async function run( "Config file could not be found at expected location. Did the 'init' action fail to start?", ); } - - // Upload Actions SARIF artifacts for debugging - if (config?.debugMode) { - core.info( - "Debug mode is on. Uploading available SARIF files as Actions debugging artifact...", - ); - const outputDir = actionsUtil.getRequiredInput("output"); - await uploadSarifDebugArtifact(config, outputDir); - } } diff --git a/src/analyze-action-post.ts b/src/analyze-action-post.ts index f6d4d894b..75cb6e4f6 100644 --- a/src/analyze-action-post.ts +++ b/src/analyze-action-post.ts @@ -12,7 +12,7 @@ import { wrapError } from "./util"; async function runWrapper() { try { - await analyzeActionPostHelper.run(debugArtifacts.uploadSarifDebugArtifact); + await analyzeActionPostHelper.run(); // Also run the upload-sarif post action since we're potentially running // the same steps in the analyze action. diff --git a/src/analyze-action.ts b/src/analyze-action.ts index 5d642399d..cb567a764 100644 --- a/src/analyze-action.ts +++ b/src/analyze-action.ts @@ -230,6 +230,7 @@ async function run() { const apiDetails = getApiDetails(); const outputDir = actionsUtil.getRequiredInput("output"); + core.exportVariable(EnvVar.SARIF_RESULTS_OUTPUT_DIR, outputDir); const threads = util.getThreadsFlag( actionsUtil.getOptionalInput("threads") || process.env["CODEQL_THREADS"], logger, diff --git a/src/debug-artifacts.ts b/src/debug-artifacts.ts index 5b060bc85..62c17761f 100644 --- a/src/debug-artifacts.ts +++ b/src/debug-artifacts.ts @@ -10,6 +10,7 @@ import { getRequiredInput } from "./actions-util"; import { dbIsFinalized } from "./analyze"; import { getCodeQL } from "./codeql"; import { Config } from "./config-utils"; +import { EnvVar } from "./environment"; import { Language } from "./languages"; import { Logger } from "./logging"; import { @@ -23,6 +24,67 @@ export function sanitizeArifactName(name: string): string { return name.replace(/[^a-zA-Z0-9_\\-]+/g, ""); } +export async function uploadAllAvailableDebugArtifacts( + config: Config, + logger: Logger, +) { + let filesToUpload: string[] = []; + + const analyzeActionOutputDir = process.env[EnvVar.SARIF_RESULTS_OUTPUT_DIR]; + for (const lang of config.languages) { + // Add any SARIF files, if they exist + if ( + analyzeActionOutputDir !== undefined && + fs.existsSync(analyzeActionOutputDir) && + fs.lstatSync(analyzeActionOutputDir).isDirectory() + ) { + const sarifFile = path.resolve(analyzeActionOutputDir, `${lang}.sarif`); + // Move SARIF to DB location so that they can be uploaded with the same root directory as the other artifacts. + if (fs.existsSync(sarifFile)) { + const sarifInDbLocation = path.resolve( + config.dbLocation, + `${lang}.sarif`, + ); + fs.renameSync(sarifFile, sarifInDbLocation); + filesToUpload = filesToUpload.concat(sarifInDbLocation); + } + } + + // Add any log files + const databaseDirectory = getCodeQLDatabasePath(config, lang); + const logsDirectory = path.resolve(databaseDirectory, "log"); + if (doesDirectoryExist(logsDirectory)) { + filesToUpload = filesToUpload.concat(listFolder(logsDirectory)); + } + + // Multilanguage tracing: there are additional logs in the root of the cluster + const multiLanguageTracingLogsDirectory = path.resolve( + config.dbLocation, + "log", + ); + if (doesDirectoryExist(multiLanguageTracingLogsDirectory)) { + filesToUpload = filesToUpload.concat( + listFolder(multiLanguageTracingLogsDirectory), + ); + } + + // Add database bundle + let databaseBundlePath: string; + if (!dbIsFinalized(config, lang, logger)) { + databaseBundlePath = await createPartialDatabaseBundle(config, lang); + } else { + databaseBundlePath = await createDatabaseBundleCli(config, lang); + } + filesToUpload = filesToUpload.concat(databaseBundlePath); + } + + await uploadDebugArtifacts( + filesToUpload, + config.dbLocation, + config.debugArtifactName, + ); +} + export async function uploadDebugArtifacts( toUpload: string[], rootDir: string, @@ -63,50 +125,6 @@ export async function uploadDebugArtifacts( } } -export async function uploadSarifDebugArtifact( - config: Config, - outputDir: string, -) { - if (!doesDirectoryExist(outputDir)) { - return; - } - - let toUpload: string[] = []; - for (const lang of config.languages) { - const sarifFile = path.resolve(outputDir, `${lang}.sarif`); - if (fs.existsSync(sarifFile)) { - toUpload = toUpload.concat(sarifFile); - } - } - await uploadDebugArtifacts(toUpload, outputDir, config.debugArtifactName); -} - -export async function uploadLogsDebugArtifact(config: Config) { - let toUpload: string[] = []; - for (const language of config.languages) { - const databaseDirectory = getCodeQLDatabasePath(config, language); - const logsDirectory = path.resolve(databaseDirectory, "log"); - if (doesDirectoryExist(logsDirectory)) { - toUpload = toUpload.concat(listFolder(logsDirectory)); - } - } - - // Multilanguage tracing: there are additional logs in the root of the cluster - const multiLanguageTracingLogsDirectory = path.resolve( - config.dbLocation, - "log", - ); - if (doesDirectoryExist(multiLanguageTracingLogsDirectory)) { - toUpload = toUpload.concat(listFolder(multiLanguageTracingLogsDirectory)); - } - - await uploadDebugArtifacts( - toUpload, - config.dbLocation, - config.debugArtifactName, - ); -} - /** * If a database has not been finalized, we cannot run the `codeql database bundle` * command in the CLI because it will return an error. Instead we directly zip @@ -150,31 +168,3 @@ async function createDatabaseBundleCli( ); return databaseBundlePath; } - -export async function uploadDatabaseBundleDebugArtifact( - config: Config, - logger: Logger, -) { - for (const language of config.languages) { - try { - let databaseBundlePath: string; - if (!dbIsFinalized(config, language, logger)) { - databaseBundlePath = await createPartialDatabaseBundle( - config, - language, - ); - } else { - databaseBundlePath = await createDatabaseBundleCli(config, language); - } - await uploadDebugArtifacts( - [databaseBundlePath], - config.dbLocation, - config.debugArtifactName, - ); - } catch (error) { - core.info( - `Failed to upload database debug bundle for ${config.debugDatabaseName}-${language}: ${error}`, - ); - } - } -} diff --git a/src/environment.ts b/src/environment.ts index 7caaa53a5..43a56d688 100644 --- a/src/environment.ts +++ b/src/environment.ts @@ -64,6 +64,9 @@ export enum EnvVar { ODASA_TRACER_CONFIGURATION = "ODASA_TRACER_CONFIGURATION", + /** The value of the `output` input for the analyze action. */ + SARIF_RESULTS_OUTPUT_DIR = "CODEQL_ACTION_SARIF_RESULTS_OUTPUT_DIR", + /** * What percentage of the total amount of RAM over 8 GB that the Action should reserve for the * system. diff --git a/src/init-action-post-helper.test.ts b/src/init-action-post-helper.test.ts index 13c1ff75a..13390c56c 100644 --- a/src/init-action-post-helper.test.ts +++ b/src/init-action-post-helper.test.ts @@ -35,13 +35,11 @@ test("post: init action with debug mode off", async (t) => { packs: [], } as unknown as configUtils.Config); - const uploadDatabaseBundleSpy = sinon.spy(); - const uploadLogsSpy = sinon.spy(); + const uploadAllAvailableDebugArtifactsSpy = sinon.spy(); const printDebugLogsSpy = sinon.spy(); await initActionPostHelper.run( - uploadDatabaseBundleSpy, - uploadLogsSpy, + uploadAllAvailableDebugArtifactsSpy, printDebugLogsSpy, createTestConfig({ debugMode: false }), parseRepositoryNwo("github/codeql-action"), @@ -49,8 +47,7 @@ test("post: init action with debug mode off", async (t) => { getRunnerLogger(true), ); - t.assert(uploadDatabaseBundleSpy.notCalled); - t.assert(uploadLogsSpy.notCalled); + t.assert(uploadAllAvailableDebugArtifactsSpy.notCalled); t.assert(printDebugLogsSpy.notCalled); }); }); @@ -60,13 +57,11 @@ test("post: init action with debug mode on", async (t) => { process.env["GITHUB_REPOSITORY"] = "github/codeql-action-fake-repository"; process.env["RUNNER_TEMP"] = tmpDir; - const uploadDatabaseBundleSpy = sinon.spy(); - const uploadLogsSpy = sinon.spy(); + const uploadAllAvailableDebugArtifactsSpy = sinon.spy(); const printDebugLogsSpy = sinon.spy(); await initActionPostHelper.run( - uploadDatabaseBundleSpy, - uploadLogsSpy, + uploadAllAvailableDebugArtifactsSpy, printDebugLogsSpy, createTestConfig({ debugMode: true }), parseRepositoryNwo("github/codeql-action"), @@ -74,8 +69,7 @@ test("post: init action with debug mode on", async (t) => { getRunnerLogger(true), ); - t.assert(uploadDatabaseBundleSpy.called); - t.assert(uploadLogsSpy.called); + t.assert(uploadAllAvailableDebugArtifactsSpy.called); t.assert(printDebugLogsSpy.called); }); }); diff --git a/src/init-action-post-helper.ts b/src/init-action-post-helper.ts index b614cb2bc..172fd4c26 100644 --- a/src/init-action-post-helper.ts +++ b/src/init-action-post-helper.ts @@ -158,11 +158,10 @@ export async function tryUploadSarifIfRunFailed( } export async function run( - uploadDatabaseBundleDebugArtifact: ( + uploadAllAvailableDebugArtifacts: ( config: Config, logger: Logger, ) => Promise, - uploadLogsDebugArtifact: (config: Config) => Promise, printDebugLogs: (config: Config) => Promise, config: Config, repositoryNwo: RepositoryNwo, @@ -211,9 +210,7 @@ export async function run( logger.info( "Debug mode is on. Uploading available database bundles and logs as Actions debugging artifacts...", ); - await uploadDatabaseBundleDebugArtifact(config, logger); - await uploadLogsDebugArtifact(config); - + await uploadAllAvailableDebugArtifacts(config, logger); await printDebugLogs(config); } diff --git a/src/init-action-post.ts b/src/init-action-post.ts index 122bd84eb..d15a1f8ad 100644 --- a/src/init-action-post.ts +++ b/src/init-action-post.ts @@ -64,8 +64,7 @@ async function runWrapper() { } uploadFailedSarifResult = await initActionPostHelper.run( - debugArtifacts.uploadDatabaseBundleDebugArtifact, - debugArtifacts.uploadLogsDebugArtifact, + debugArtifacts.uploadAllAvailableDebugArtifacts, printDebugLogs, config, repositoryNwo,