Add tests for getOrInitCodeQL

src/analyze-action.ts

@@ -363,7 +363,9 @@ async function run(startedAt: Date) {
 
       uploadResults = await postProcessAndUploadSarif(
         logger,
+        config.tempDir,
         features,
+        async () => codeql,
         uploadKind,
         checkoutPath,
         outputDir,

src/init-action-post-helper.test.ts

@@ -602,6 +602,8 @@ async function testFailedSarifUpload(
     }
     t.true(
       uploadFiles.calledOnceWith(
+        sinon.match.string,
+        codeqlObject,
         sinon.match.string,
         sinon.match.string,
         category,

src/init-action-post-helper.ts

@@ -107,6 +107,8 @@ async function maybeUploadFailedSarif(
 
   logger.info(`Uploading failed SARIF file ${sarifFile}`);
   const uploadResult = await uploadLib.uploadFiles(
+    config.tempDir,
+    codeql,
     sarifFile,
     checkoutPath,
     category,

src/upload-lib.ts

@@ -11,8 +11,7 @@ import * as actionsUtil from "./actions-util";
 import * as analyses from "./analyses";
 import * as api from "./api-client";
 import { getGitHubVersion, wrapApiConfigurationError } from "./api-client";
-import { CodeQL, getCodeQL } from "./codeql";
-import { getConfig } from "./config-utils";
+import { type CodeQL } from "./codeql";
 import { readDiffRangesJsonFile } from "./diff-informed-analysis-utils";
 import { EnvVar } from "./environment";
 import { FeatureEnablement } from "./feature-flags";
@@ -183,6 +182,44 @@ async function shouldDisableCombineSarifFiles(
   return true;
 }
 
+/**
+ * Initialises a `CodeQL` instance that we can use to combine SARIF files.
+ */
+export async function minimalInitCodeQL(
+  logger: Logger,
+  gitHubVersion: GitHubVersion,
+  features: FeatureEnablement,
+): Promise<CodeQL> {
+  logger.info(
+    "Initializing CodeQL since the 'init' Action was not called before this step.",
+  );
+
+  const apiDetails = {
+    auth: actionsUtil.getRequiredInput("token"),
+    externalRepoAuth: actionsUtil.getOptionalInput("external-repository-token"),
+    url: getRequiredEnvParam("GITHUB_SERVER_URL"),
+    apiURL: getRequiredEnvParam("GITHUB_API_URL"),
+  };
+
+  const codeQLDefaultVersionInfo = await features.getDefaultCliVersion(
+    gitHubVersion.type,
+  );
+
+  const initCodeQLResult = await initCodeQL(
+    undefined, // There is no tools input on the upload action
+    apiDetails,
+    actionsUtil.getTemporaryDirectory(),
+    gitHubVersion.type,
+    codeQLDefaultVersionInfo,
+    features,
+    logger,
+  );
+
+  return initCodeQLResult.codeql;
+}
+
+export type CodeQLGetter = () => Promise<CodeQL>;
+
 // Takes a list of paths to sarif files and combines them together using the
 // CLI `github merge-results` command when all SARIF files are produced by
 // CodeQL. Otherwise, it will fall back to combining the files in the action.
@@ -190,8 +227,10 @@ async function shouldDisableCombineSarifFiles(
 async function combineSarifFilesUsingCLI(
   sarifFiles: string[],
   gitHubVersion: GitHubVersion,
-  features: FeatureEnablement,
+  _features: FeatureEnablement,
   logger: Logger,
+  getCodeQL: CodeQLGetter,
+  tempDir: string,
 ): Promise<SarifFile> {
   logger.info("Combining SARIF files using the CodeQL CLI");
 
@@ -229,45 +268,10 @@ async function combineSarifFilesUsingCLI(
     return combineSarifFiles(sarifFiles, logger);
   }
 
-  // Initialize CodeQL, either by using the config file from the 'init' step,
-  // or by initializing it here.
-  let codeQL: CodeQL;
-  let tempDir: string = actionsUtil.getTemporaryDirectory();
-
-  const config = await getConfig(tempDir, logger);
-  if (config !== undefined) {
-    codeQL = await getCodeQL(config.codeQLCmd);
-    tempDir = config.tempDir;
-  } else {
-    logger.info(
-      "Initializing CodeQL since the 'init' Action was not called before this step.",
-    );
-
-    const apiDetails = {
-      auth: actionsUtil.getRequiredInput("token"),
-      externalRepoAuth: actionsUtil.getOptionalInput(
-        "external-repository-token",
-      ),
-      url: getRequiredEnvParam("GITHUB_SERVER_URL"),
-      apiURL: getRequiredEnvParam("GITHUB_API_URL"),
-    };
-
-    const codeQLDefaultVersionInfo = await features.getDefaultCliVersion(
-      gitHubVersion.type,
-    );
-
-    const initCodeQLResult = await initCodeQL(
-      undefined, // There is no tools input on the upload action
-      apiDetails,
-      tempDir,
-      gitHubVersion.type,
-      codeQLDefaultVersionInfo,
-      features,
-      logger,
-    );
-
-    codeQL = initCodeQLResult.codeql;
-  }
+  // Obtain a `CodeQL` instance. For `analyze`, this is typically the instance that was used for running the queries.
+  // For `upload-sarif`, this either initialises a new instance or returns a previously initialised one if `getCodeQL`
+  // is called more than once.
+  const codeQL: CodeQL = await getCodeQL();
 
   const baseTempDir = path.resolve(tempDir, "combined-sarif");
   fs.mkdirSync(baseTempDir, { recursive: true });
@@ -673,6 +677,8 @@ export interface PostProcessingResults {
  *
  * @param logger The logger to use.
  * @param features Information about enabled features.
+ * @param getCodeQL A function to retrieve a `CodeQL` instance.
+ * @param tempPath A path to a temporary directory.
  * @param checkoutPath The path where the repo was checked out at.
  * @param sarifPaths The paths of the SARIF files to post-process.
  * @param category The analysis category.
@@ -684,6 +690,8 @@ export interface PostProcessingResults {
 export async function postProcessSarifFiles(
   logger: Logger,
   features: FeatureEnablement,
+  getCodeQL: CodeQLGetter,
+  tempPath: string,
   checkoutPath: string,
   sarifPaths: string[],
   category: string | undefined,
@@ -708,6 +716,8 @@ export async function postProcessSarifFiles(
       gitHubVersion,
       features,
       logger,
+      getCodeQL,
+      tempPath,
     );
   } else {
     const sarifPath = sarifPaths[0];
@@ -768,6 +778,8 @@ export async function writePostProcessedFiles(
  * to.
  */
 export async function uploadFiles(
+  tempDir: string,
+  codeql: CodeQL,
   inputSarifPath: string,
   checkoutPath: string,
   category: string | undefined,
@@ -781,6 +793,8 @@ export async function uploadFiles(
   );
 
   return uploadSpecifiedFiles(
+    tempDir,
+    codeql,
     sarifPaths,
     checkoutPath,
     category,
@@ -794,6 +808,8 @@ export async function uploadFiles(
  * Uploads the given array of SARIF files.
  */
 async function uploadSpecifiedFiles(
+  tempDir: string,
+  codeql: CodeQL,
   sarifPaths: string[],
   checkoutPath: string,
   category: string | undefined,
@@ -804,6 +820,8 @@ async function uploadSpecifiedFiles(
   const processingResults: PostProcessingResults = await postProcessSarifFiles(
     logger,
     features,
+    async () => codeql,
+    tempDir,
     checkoutPath,
     sarifPaths,
     category,

src/upload-sarif-action.ts

@@ -1,9 +1,9 @@
 import * as core from "@actions/core";
 
 import * as actionsUtil from "./actions-util";
-import { getActionVersion, getTemporaryDirectory } from "./actions-util";
 import * as analyses from "./analyses";
 import { getGitHubVersion } from "./api-client";
+import { getConfig } from "./config-utils";
 import { initFeatures } from "./feature-flags";
 import { Logger, getActionsLogger } from "./logging";
 import { getRepositoryNwo } from "./repository";
@@ -17,7 +17,11 @@ import {
   isThirdPartyAnalysis,
 } from "./status-report";
 import * as upload_lib from "./upload-lib";
-import { postProcessAndUploadSarif } from "./upload-sarif";
+import {
+  getOrInitCodeQL,
+  postProcessAndUploadSarif,
+  UploadSarifState,
+} from "./upload-sarif";
 import {
   ConfigurationError,
   checkActionVersion,
@@ -59,12 +63,13 @@ async function run(startedAt: Date) {
   // possible, and only use safe functions outside.
 
   const logger = getActionsLogger();
+  const state: UploadSarifState = { cachedCodeQL: undefined };
 
   try {
-    initializeEnvironment(getActionVersion());
+    initializeEnvironment(actionsUtil.getActionVersion());
 
     const gitHubVersion = await getGitHubVersion();
-    checkActionVersion(getActionVersion(), gitHubVersion);
+    checkActionVersion(actionsUtil.getActionVersion(), gitHubVersion);
 
     // Make inputs accessible in the `post` step.
     actionsUtil.persistInputs();
@@ -73,7 +78,7 @@ async function run(startedAt: Date) {
     const features = initFeatures(
       gitHubVersion,
       repositoryNwo,
-      getTemporaryDirectory(),
+      actionsUtil.getTemporaryDirectory(),
       logger,
     );
 
@@ -94,9 +99,20 @@ async function run(startedAt: Date) {
     const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
     const category = actionsUtil.getOptionalInput("category");
 
+    // Determine the temporary directory to use. If we are able to read a `Config` from a previous CodeQL Action
+    // step in the job, then use the temporary directory configured there. Otherwise, use our default.
+    let tempDir: string = actionsUtil.getTemporaryDirectory();
+
+    const config = await getConfig(tempDir, logger);
+    if (config !== undefined) {
+      tempDir = config.tempDir;
+    }
+
     const uploadResults = await postProcessAndUploadSarif(
       logger,
+      tempDir,
       features,
+      () => getOrInitCodeQL(state, logger, gitHubVersion, features, config),
       "always",
       checkoutPath,
       sarifPath,

src/upload-sarif.test.ts

@@ -5,15 +5,102 @@ import test, { ExecutionContext } from "ava";
 import * as sinon from "sinon";
 
 import { AnalysisKind, getAnalysisConfig } from "./analyses";
+import { getCodeQLForTesting } from "./codeql";
+import * as codeql from "./codeql";
 import { getRunnerLogger } from "./logging";
-import { createFeatures, setupTests } from "./testing-utils";
+import { createFeatures, createTestConfig, setupTests } from "./testing-utils";
 import { UploadResult } from "./upload-lib";
 import * as uploadLib from "./upload-lib";
-import { postProcessAndUploadSarif } from "./upload-sarif";
+import {
+  getOrInitCodeQL,
+  postProcessAndUploadSarif,
+  UploadSarifState,
+} from "./upload-sarif";
 import * as util from "./util";
 
 setupTests(test);
 
+test("getOrInitCodeQL - gets cached CodeQL instance when available", async (t) => {
+  const cachedCodeQL = await getCodeQLForTesting();
+  const getCodeQL = sinon.stub(codeql, "getCodeQL").resolves(undefined);
+  const minimalInitCodeQL = sinon
+    .stub(uploadLib, "minimalInitCodeQL")
+    .resolves(undefined);
+
+  const result = await getOrInitCodeQL(
+    { cachedCodeQL },
+    getRunnerLogger(true),
+    { type: util.GitHubVariant.GHES, version: "3.0" },
+    createFeatures([]),
+    undefined,
+  );
+
+  // Neither of the two functions to get a CodeQL instance were called.
+  t.true(getCodeQL.notCalled);
+  t.true(minimalInitCodeQL.notCalled);
+
+  // But we have an instance that refers to the same object as the one we put into the state.
+  t.truthy(result);
+  t.is(result, cachedCodeQL);
+});
+
+test("getOrInitCodeQL - uses minimalInitCodeQL when there's no config", async (t) => {
+  const newInstance = await getCodeQLForTesting();
+  const getCodeQL = sinon.stub(codeql, "getCodeQL").resolves(undefined);
+  const minimalInitCodeQL = sinon
+    .stub(uploadLib, "minimalInitCodeQL")
+    .resolves(newInstance);
+
+  const state: UploadSarifState = { cachedCodeQL: undefined };
+  const result = await getOrInitCodeQL(
+    state,
+    getRunnerLogger(true),
+    { type: util.GitHubVariant.GHES, version: "3.0" },
+    createFeatures([]),
+    undefined,
+  );
+
+  // Check that the right function was called.
+  t.true(getCodeQL.notCalled);
+  t.true(minimalInitCodeQL.calledOnce);
+
+  // And that we received the instance that we expected.
+  t.truthy(result);
+  t.is(result, newInstance);
+
+  // And that it was cached.
+  t.is(state.cachedCodeQL, newInstance);
+});
+
+test("getOrInitCodeQL - uses getCodeQL when there's a config", async (t) => {
+  const newInstance = await getCodeQLForTesting();
+  const getCodeQL = sinon.stub(codeql, "getCodeQL").resolves(newInstance);
+  const minimalInitCodeQL = sinon
+    .stub(uploadLib, "minimalInitCodeQL")
+    .resolves(undefined);
+  const config = createTestConfig({});
+
+  const state: UploadSarifState = { cachedCodeQL: undefined };
+  const result = await getOrInitCodeQL(
+    state,
+    getRunnerLogger(true),
+    { type: util.GitHubVariant.GHES, version: "3.0" },
+    createFeatures([]),
+    config,
+  );
+
+  // Check that the right function was called.
+  t.true(getCodeQL.calledOnce);
+  t.true(minimalInitCodeQL.notCalled);
+
+  // And that we received the instance that we expected.
+  t.truthy(result);
+  t.is(result, newInstance);
+
+  // And that it was cached.
+  t.is(state.cachedCodeQL, newInstance);
+});
+
 interface UploadSarifExpectedResult {
   uploadResult?: UploadResult;
   expectedFiles?: string[];
@@ -31,6 +118,8 @@ function mockPostProcessSarifFiles() {
         sinon.match.any,
         sinon.match.any,
         sinon.match.any,
+        sinon.match.any,
+        sinon.match.any,
         analysisConfig,
       )
       .resolves({ sarif: { runs: [] }, analysisKey: "", environment: "" });
@@ -73,7 +162,9 @@ const postProcessAndUploadSarifMacro = test.macro({
 
       const actual = await postProcessAndUploadSarif(
         logger,
+        tempDir,
         features,
+        async () => getCodeQLForTesting(),
         "always",
         "",
         testPath,
@@ -90,6 +181,8 @@ const postProcessAndUploadSarifMacro = test.macro({
             postProcessSarifFiles.calledWith(
               logger,
               features,
+              sinon.match.func,
+              tempDir,
               sinon.match.any,
               analysisKindResult.expectedFiles?.map(toFullPath) ??
                 fullSarifPaths,
@@ -221,7 +314,9 @@ test("postProcessAndUploadSarif doesn't upload if upload is disabled", async (t)
 
     const actual = await postProcessAndUploadSarif(
       logger,
+      tempDir,
       features,
+      () => getCodeQLForTesting(),
       "never",
       "",
       tempDir,
@@ -248,7 +343,9 @@ test("postProcessAndUploadSarif writes post-processed SARIF files if output dire
     const postProcessedOutPath = path.join(tempDir, "post-processed");
     const actual = await postProcessAndUploadSarif(
       logger,
+      tempDir,
       features,
+      () => getCodeQLForTesting(),
       "never",
       "",
       tempDir,

src/upload-sarif.ts

@@ -1,20 +1,57 @@
 import { UploadKind } from "./actions-util";
 import * as analyses from "./analyses";
+import type { CodeQL } from "./codeql";
+import * as codeql from "./codeql";
+import { Config } from "./config-utils";
 import { FeatureEnablement } from "./feature-flags";
 import { Logger } from "./logging";
 import * as upload_lib from "./upload-lib";
-import { unsafeEntriesInvariant } from "./util";
+import { GitHubVersion, unsafeEntriesInvariant } from "./util";
+
+export interface UploadSarifState {
+  /** The cached `CodeQL` instance, if any. */
+  cachedCodeQL: CodeQL | undefined;
+}
 
 // Maps analysis kinds to SARIF IDs.
 export type UploadSarifResults = Partial<
   Record<analyses.AnalysisKind, upload_lib.UploadResult>
 >;
 
+/** Get or initialise a `CodeQL` instance for use by the `upload-sarif` action. */
+export async function getOrInitCodeQL(
+  actionState: UploadSarifState,
+  logger: Logger,
+  gitHubVersion: GitHubVersion,
+  features: FeatureEnablement,
+  config: Config | undefined,
+): Promise<CodeQL> {
+  // Return the cached instance, if we have one.
+  if (actionState.cachedCodeQL !== undefined) return actionState.cachedCodeQL;
+
+  // If we have been able to load a `Config` from an earlier CodeQL Action step in the job,
+  // then use the CodeQL executable that we have used previously. Otherwise, initialise the
+  // CLI specifically for `upload-sarif`. Either way, we cache the instance.
+  if (config !== undefined) {
+    actionState.cachedCodeQL = await codeql.getCodeQL(config.codeQLCmd);
+  } else {
+    actionState.cachedCodeQL = await upload_lib.minimalInitCodeQL(
+      logger,
+      gitHubVersion,
+      features,
+    );
+  }
+
+  return actionState.cachedCodeQL;
+}
+
 /**
  * Finds SARIF files in `sarifPath`, post-processes them, and uploads them to the appropriate services.
  *
  * @param logger The logger to use.
+ * @param tempPath The path to the temporary directory.
  * @param features Information about enabled features.
+ * @param getCodeQL A function to retrieve a `CodeQL` instance.
  * @param uploadKind The kind of upload that is requested.
  * @param checkoutPath The path where the repository was checked out at.
  * @param sarifPath The path to the file or directory to upload.
@@ -25,7 +62,9 @@ export type UploadSarifResults = Partial<
  */
 export async function postProcessAndUploadSarif(
   logger: Logger,
+  tempPath: string,
   features: FeatureEnablement,
+  getCodeQL: upload_lib.CodeQLGetter,
   uploadKind: UploadKind,
   checkoutPath: string,
   sarifPath: string,
@@ -45,6 +84,8 @@ export async function postProcessAndUploadSarif(
     const postProcessingResults = await upload_lib.postProcessSarifFiles(
       logger,
       features,
+      getCodeQL,
+      tempPath,
       checkoutPath,
       sarifFiles,
       category,