Skip to content

Refactor: prepare debug artifacts for artifact upgrades #2475

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.

Sign up for GitHub

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

Merged
angelapwen merged 8 commits into from
Sep 13, 2024
Merged

Refactor: prepare debug artifacts for artifact upgrades #2475

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b296f26
Refactor: upload all available debug artifacts in init-post
angelapwen Sep 11, 2024
c098b25
Only upload `upload-sarif` debug artifacts at most once
angelapwen Sep 11, 2024
4ba2440
Rebuild: add transpiled files
angelapwen Sep 11, 2024
82ce313
Remove unused helper file
angelapwen Sep 11, 2024
d4bfd40
Use `.push` rather than `.concat`
angelapwen Sep 11, 2024
cb7faf5
Refactor: move combined SARIF debug artifact logic to `debug-artifact`
angelapwen Sep 12, 2024
e771680
Rename `upload-debug-artifacts` to `combined-sarif-artifacts`
angelapwen Sep 12, 2024
bc660fc
Copy SARIF file to database location rather than move
angelapwen Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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.
  • Loading branch information
@angelapwen
angelapwen committed Sep 11, 2024
commit b296f2676c59e1cadc443ea4d3ed387d4f4e59a2
57 changes: 0 additions & 57 deletions src/analyze-action-post-helper.test.ts
View file
Open in desktop

This file was deleted.

20 changes: 2 additions & 18 deletions src/analyze-action-post-helper.ts
View file
Open in desktop
Comment thread
angelapwen marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
@@ -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<void>,
) {
export async function run() {
const logger = getActionsLogger();

const config = await getConfig(actionsUtil.getTemporaryDirectory(), logger);
Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/analyze-action-post.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/analyze-action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
134 changes: 62 additions & 72 deletions src/debug-artifacts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Comment thread
henrymercer marked this conversation as resolved.
Outdated
filesToUpload = filesToUpload.concat(sarifInDbLocation);
Copy link
Copy Markdown
Contributor

@aeisenberg aeisenberg Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: Avoids reassigning a variable and the declaration above can be turned into a const.

Suggested change
filesToUpload = filesToUpload.concat(sarifInDbLocation);
filesToUpload.push(...sarifInDbLocation);

Copy link
Copy Markdown
Contributor Author

@angelapwen angelapwen Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, makes sense — I had been meaning to make this change!

Copy link
Copy Markdown
Contributor Author

@angelapwen angelapwen Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to use the ... operator when I know it's a single file like the sarifInDbLocation variable?

}
}

// Add any log files
const databaseDirectory = getCodeQLDatabasePath(config, lang);
const logsDirectory = path.resolve(databaseDirectory, "log");
if (doesDirectoryExist(logsDirectory)) {
filesToUpload = filesToUpload.concat(listFolder(logsDirectory));
Comment thread
angelapwen marked this conversation as resolved.
Outdated
}

// 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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}`,
);
}
}
}
3 changes: 3 additions & 0 deletions src/environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 6 additions & 12 deletions src/init-action-post-helper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,19 @@ 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"),
createFeatures([]),
getRunnerLogger(true),
);

t.assert(uploadDatabaseBundleSpy.notCalled);
t.assert(uploadLogsSpy.notCalled);
t.assert(uploadAllAvailableDebugArtifactsSpy.notCalled);
t.assert(printDebugLogsSpy.notCalled);
});
});
Expand All @@ -60,22 +57,19 @@ 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"),
createFeatures([]),
getRunnerLogger(true),
);

t.assert(uploadDatabaseBundleSpy.called);
t.assert(uploadLogsSpy.called);
t.assert(uploadAllAvailableDebugArtifactsSpy.called);
t.assert(printDebugLogsSpy.called);
});
});
Expand Down
7 changes: 2 additions & 5 deletions src/init-action-post-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,10 @@ export async function tryUploadSarifIfRunFailed(
}

export async function run(
uploadDatabaseBundleDebugArtifact: (
uploadAllAvailableDebugArtifacts: (
config: Config,
logger: Logger,
) => Promise<void>,
uploadLogsDebugArtifact: (config: Config) => Promise<void>,
printDebugLogs: (config: Config) => Promise<void>,
config: Config,
repositoryNwo: RepositoryNwo,
Expand Down Expand Up @@ -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);
}

Expand Down
3 changes: 1 addition & 2 deletions src/init-action-post.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ async function runWrapper() {
}

uploadFailedSarifResult = await initActionPostHelper.run(
debugArtifacts.uploadDatabaseBundleDebugArtifact,
debugArtifacts.uploadLogsDebugArtifact,
debugArtifacts.uploadAllAvailableDebugArtifacts,
printDebugLogs,
config,
repositoryNwo,
Expand Down