Address comments on the PR

This commit is contained in:
Óscar San José
2026-04-14 15:10:54 +02:00
parent e74c1eea4a
commit 020856ca4e
8 changed files with 121 additions and 60 deletions

View File

@@ -46,6 +46,35 @@ export const getOptionalInput = function (name: string): string | undefined {
return value.length > 0 ? value : undefined;
};
/**
* Resolves the effective tools input by combining workflow input and repository properties.
* The explicit `tools` workflow input takes precedence. If none is provided,
* fall back to the repository property (if set).
*
* @param repositoryProperties - The loaded repository properties object
* @param toolsPropertyName - The name of the tools property to look up
* @returns An object containing the effective tools input and whether it came from repository property
*/
export function resolveToolsInput(
repositoryProperties: Record<string, any>,
toolsPropertyName: string,
): {
effectiveToolsInput: string | undefined;
toolsInputFromRepositoryProperty: boolean;
} {
const toolsWorkflowInput = getOptionalInput("tools");
const toolsPropertyValue: string | undefined =
repositoryProperties[toolsPropertyName];
const effectiveToolsInput = toolsWorkflowInput ?? toolsPropertyValue;
const toolsInputFromRepositoryProperty =
toolsWorkflowInput === undefined && toolsPropertyValue !== undefined;
return {
effectiveToolsInput,
toolsInputFromRepositoryProperty,
};
}
export function getTemporaryDirectory(): string {
const value = process.env["CODEQL_ACTION_TEMP"];
return value !== undefined && value !== ""

View File

@@ -323,7 +323,7 @@ export async function setupCodeQL(
features: FeatureEnablement,
logger: Logger,
checkVersion: boolean,
toolsInputFromRepositoryProperty = false,
toolsInputFromRepositoryProperty: boolean,
): Promise<{
codeql: CodeQL;
toolsDownloadStatusReport?: ToolsDownloadStatusReport;

View File

@@ -78,6 +78,7 @@ test.serial("loadPropertiesFromApi loads known properties", async (t) => {
url: "",
data: [
{ property_name: "github-codeql-extra-queries", value: "+queries" },
{ property_name: "github-codeql-tools", value: "toolcache" },
{ property_name: "unknown-property", value: "something" },
] satisfies properties.GitHubPropertiesResponse,
});
@@ -87,30 +88,12 @@ test.serial("loadPropertiesFromApi loads known properties", async (t) => {
logger,
mockRepositoryNwo,
);
t.deepEqual(response, { "github-codeql-extra-queries": "+queries" });
t.deepEqual(response, {
"github-codeql-extra-queries": "+queries",
"github-codeql-tools": "toolcache",
});
});
test.serial(
"loadPropertiesFromApi loads github-codeql-tools property",
async (t) => {
sinon.stub(api, "getRepositoryProperties").resolves({
headers: {},
status: 200,
url: "",
data: [
{ property_name: "github-codeql-tools", value: "toolcache" },
] satisfies properties.GitHubPropertiesResponse,
});
const logger = getRunnerLogger(true);
const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
const response = await properties.loadPropertiesFromApi(
logger,
mockRepositoryNwo,
);
t.deepEqual(response, { "github-codeql-tools": "toolcache" });
},
);
test.serial("loadPropertiesFromApi parses true boolean property", async (t) => {
sinon.stub(api, "getRepositoryProperties").resolves({
headers: {},

View File

@@ -15,6 +15,7 @@ import {
getRequiredInput,
getTemporaryDirectory,
persistInputs,
resolveToolsInput,
} from "./actions-util";
import { AnalysisKind, getAnalysisKinds } from "./analyses";
import { getGitHubVersion, GitHubApiCombinedDetails } from "./api-client";
@@ -141,6 +142,7 @@ async function sendCompletedStatusReport(
toolsFeatureFlagsValid: boolean | undefined,
toolsSource: ToolsSource,
toolsVersion: string,
effectiveToolsInput: string | undefined,
overlayBaseDatabaseStats: OverlayBaseDatabaseDownloadStats | undefined,
dependencyCachingResults: DependencyCacheRestoreStatusReport | undefined,
logger: Logger,
@@ -165,7 +167,7 @@ async function sendCompletedStatusReport(
const initStatusReport: InitStatusReport = {
...statusReportBase,
tools_input: getOptionalInput("tools") || "",
tools_input: effectiveToolsInput || "",
tools_resolved_version: toolsVersion,
tools_source: toolsSource || ToolsSource.Unknown,
workflow_languages: workflowLanguages || "",
@@ -220,6 +222,7 @@ async function run(startedAt: Date) {
let toolsSource: ToolsSource;
let toolsVersion: string;
let zstdAvailability: ZstdAvailability | undefined;
let effectiveToolsInput: string | undefined;
try {
initializeEnvironment(getActionVersion());
@@ -302,12 +305,13 @@ async function run(startedAt: Date) {
// Determine the effective tools input.
// The explicit `tools` workflow input takes precedence. If none is provided,
// fall back to the 'github-codeql-tools' repository property (if set).
const toolsWorkflowInput = getOptionalInput("tools");
const toolsPropertyValue: string | undefined =
repositoryPropertiesResult.orElse({})[RepositoryPropertyName.TOOLS];
const effectiveToolsInput = toolsWorkflowInput ?? toolsPropertyValue;
const resolvedToolsInput = resolveToolsInput(
repositoryPropertiesResult.orElse({}),
RepositoryPropertyName.TOOLS,
);
effectiveToolsInput = resolvedToolsInput.effectiveToolsInput;
const toolsInputFromRepositoryProperty =
toolsWorkflowInput === undefined && toolsPropertyValue !== undefined;
resolvedToolsInput.toolsInputFromRepositoryProperty;
const initCodeQLResult = await initCodeQL(
effectiveToolsInput,
@@ -791,6 +795,7 @@ async function run(startedAt: Date) {
toolsFeatureFlagsValid,
toolsSource,
toolsVersion,
effectiveToolsInput,
overlayBaseDatabaseStats,
dependencyCachingStatus,
logger,
@@ -808,6 +813,7 @@ async function run(startedAt: Date) {
toolsFeatureFlagsValid,
toolsSource,
toolsVersion,
effectiveToolsInput,
overlayBaseDatabaseStats,
dependencyCachingStatus,
logger,

View File

@@ -41,7 +41,7 @@ export async function initCodeQL(
defaultCliVersion: CodeQLDefaultVersionInfo,
features: FeatureEnablement,
logger: Logger,
toolsInputFromRepositoryProperty = false,
toolsInputFromRepositoryProperty: boolean,
): Promise<{
codeql: CodeQL;
toolsDownloadStatusReport?: ToolsDownloadStatusReport;

View File

@@ -6,11 +6,16 @@ import {
getOptionalInput,
getRequiredInput,
getTemporaryDirectory,
resolveToolsInput,
} from "./actions-util";
import { getGitHubVersion } from "./api-client";
import { CodeQL } from "./codeql";
import { EnvVar } from "./environment";
import { initFeatures } from "./feature-flags";
import {
loadPropertiesFromApi,
RepositoryPropertyName,
} from "./feature-flags/properties";
import { initCodeQL } from "./init";
import { getActionsLogger, Logger } from "./logging";
import { getRepositoryNwo } from "./repository";
@@ -46,6 +51,7 @@ async function sendCompletedStatusReport(
toolsFeatureFlagsValid: boolean | undefined,
toolsSource: ToolsSource,
toolsVersion: string,
effectiveToolsInput: string | undefined,
logger: Logger,
error?: Error,
): Promise<void> {
@@ -66,7 +72,7 @@ async function sendCompletedStatusReport(
const initStatusReport: InitStatusReport = {
...statusReportBase,
tools_input: getOptionalInput("tools") || "",
tools_input: effectiveToolsInput || "",
tools_resolved_version: toolsVersion,
tools_source: toolsSource || ToolsSource.Unknown,
workflow_languages: "",
@@ -97,6 +103,7 @@ async function run(startedAt: Date): Promise<void> {
let toolsFeatureFlagsValid: boolean | undefined;
let toolsSource: ToolsSource;
let toolsVersion: string;
let effectiveToolsInput: string | undefined;
try {
initializeEnvironment(getActionVersion());
@@ -121,6 +128,11 @@ async function run(startedAt: Date): Promise<void> {
logger,
);
const repositoryPropertiesResult = await loadPropertiesFromApi(
logger,
repositoryNwo,
);
const jobRunUuid = uuidV4();
logger.info(`Job run UUID is ${jobRunUuid}.`);
core.exportVariable(EnvVar.JOB_RUN_UUID, jobRunUuid);
@@ -140,14 +152,27 @@ async function run(startedAt: Date): Promise<void> {
gitHubVersion.type,
);
toolsFeatureFlagsValid = codeQLDefaultVersionInfo.toolsFeatureFlagsValid;
// Determine the effective tools input.
// The explicit `tools` workflow input takes precedence. If none is provided,
// fall back to the 'github-codeql-tools' repository property (if set).
const resolvedToolsInput = resolveToolsInput(
repositoryPropertiesResult,
RepositoryPropertyName.TOOLS,
);
effectiveToolsInput = resolvedToolsInput.effectiveToolsInput;
const toolsInputFromRepositoryProperty =
resolvedToolsInput.toolsInputFromRepositoryProperty;
const initCodeQLResult = await initCodeQL(
getOptionalInput("tools"),
effectiveToolsInput,
apiDetails,
getTemporaryDirectory(),
gitHubVersion.type,
codeQLDefaultVersionInfo,
features,
logger,
toolsInputFromRepositoryProperty,
);
codeql = initCodeQLResult.codeql;
toolsDownloadStatusReport = initCodeQLResult.toolsDownloadStatusReport;
@@ -183,6 +208,7 @@ async function run(startedAt: Date): Promise<void> {
toolsFeatureFlagsValid,
toolsSource,
toolsVersion,
effectiveToolsInput,
logger,
);
}

View File

@@ -8,6 +8,7 @@ import * as sinon from "sinon";
import * as actionsUtil from "./actions-util";
import * as api from "./api-client";
import { Feature, FeatureEnablement } from "./feature-flags";
import { RepositoryPropertyName } from "./feature-flags/properties";
import { getRunnerLogger } from "./logging";
import * as setupCodeql from "./setup-codeql";
import * as tar from "./tar";
@@ -112,6 +113,7 @@ test.serial(
false,
features,
getRunnerLogger(true),
false,
);
t.is(source.sourceType, "download");
@@ -135,6 +137,7 @@ test.serial(
false,
features,
getRunnerLogger(true),
false,
);
t.is(source.toolsVersion, LINKED_CLI_VERSION.cliVersion);
@@ -160,6 +163,7 @@ test.serial(
false,
features,
logger,
false,
);
// First, ensure that the CLI version is the linked version, so that backwards
@@ -213,6 +217,7 @@ test.serial(
SAMPLE_DEFAULT_CLI_VERSION,
features,
logger,
false,
);
// Basic sanity check that the version we got back is indeed
@@ -268,6 +273,7 @@ test.serial(
SAMPLE_DEFAULT_CLI_VERSION,
features,
logger,
false,
);
// Basic sanity check that the version we got back is indeed the version that the
@@ -322,6 +328,7 @@ test.serial(
false,
features,
logger,
false,
);
// Check that the `CodeQLToolsSource` object matches our expectations.
@@ -383,6 +390,7 @@ test.serial(
false,
features,
logger,
false,
);
// Check that the `CodeQLToolsSource` object matches our expectations.
@@ -437,6 +445,7 @@ test.serial(
false,
features,
logger,
false,
);
// Check that the toolcache functions were called with the expected arguments
@@ -504,6 +513,7 @@ const toolcacheInputFallbackMacro = test.macro({
false,
features,
logger,
false,
);
// Check that the toolcache functions were called with the expected arguments
@@ -612,7 +622,7 @@ test.serial(
t.is(source.toolsVersion, latestToolcacheVersion);
const expectedMessages: string[] = [
`Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the 'github-codeql-tools' repository property.`,
`Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the '${RepositoryPropertyName.TOOLS}' repository property.`,
`CLI version ${latestToolcacheVersion} is the latest version in the toolcache.`,
`Using CodeQL CLI version ${latestToolcacheVersion} from toolcache at ${latestVersionPath}`,
];
@@ -657,8 +667,8 @@ test.serial(
t.is(source.toolsVersion, SAMPLE_DEFAULT_CLI_VERSION.cliVersion);
const expectedMessages: string[] = [
`Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the 'github-codeql-tools' repository property.`,
`Found no CodeQL CLI in the toolcache, ignoring the 'github-codeql-tools' repository property...`,
`Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the '${RepositoryPropertyName.TOOLS}' repository property.`,
`Found no CodeQL CLI in the toolcache, ignoring the '${RepositoryPropertyName.TOOLS}' repository property...`,
];
for (const expectedMessage of expectedMessages) {
t.assert(

View File

@@ -17,6 +17,7 @@ import {
Feature,
FeatureEnablement,
} from "./feature-flags";
import { RepositoryPropertyName } from "./feature-flags/properties";
import { Logger } from "./logging";
import * as tar from "./tar";
import {
@@ -264,6 +265,24 @@ async function findOverridingToolsInCache(
return undefined;
}
/**
* Creates a user-friendly description of the tools input origin for log messages.
*
* @param toolsInput The tools input value
* @param toolsInputFromRepositoryProperty Whether the tools input came from repository property
* @returns A description like "'tools: toolcache'" or "the 'github-codeql-tools' repository property"
*/
function getToolsInputOriginDescription(
toolsInput: string | undefined,
toolsInputFromRepositoryProperty: boolean,
): string {
if (toolsInputFromRepositoryProperty) {
return `the '${RepositoryPropertyName.TOOLS}' repository property`;
} else {
return `'tools: ${toolsInput}'`;
}
}
/**
* Determines where the CodeQL CLI we want to use comes from. This can be from a local file,
* the Actions toolcache, or a download.
@@ -286,7 +305,7 @@ export async function getCodeQLSource(
tarSupportsZstd: boolean,
features: FeatureEnablement,
logger: Logger,
toolsInputFromRepositoryProperty = false,
toolsInputFromRepositoryProperty: boolean,
): Promise<CodeQLToolsSource> {
// If there is an explicit `tools` input, it's not one of the reserved values, and it doesn't appear
// to point to a URL, then we assume it is a local path and use the CLI from there.
@@ -361,7 +380,7 @@ export async function getCodeQLSource(
);
} else {
logger.info(
`Using the latest CodeQL CLI nightly, as requested by 'tools: ${toolsInput}'.`,
`Using the latest CodeQL CLI nightly, as requested by ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)}.`,
);
}
toolsInput = await getNightlyToolsUrl(logger);
@@ -386,7 +405,7 @@ export async function getCodeQLSource(
tagName = defaults.bundleVersion;
logger.info(
`'tools: ${toolsInput}' was requested, so using CodeQL version ${cliVersion}, the version shipped with the Action.`,
`${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)} was requested, so using CodeQL version ${cliVersion}, the version shipped with the Action.`,
);
if (toolsInput === "latest") {
@@ -414,15 +433,9 @@ export async function getCodeQLSource(
// If `toolsInput === "toolcache"`, try to find the latest version of the CLI that's available in the toolcache
// and use that. We perform this check here since we can set `cliVersion` directly and don't want to default to
// the linked version.
if (toolsInputFromRepositoryProperty) {
logger.info(
`Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the 'github-codeql-tools' repository property.`,
);
} else {
logger.info(
`Attempting to use the latest CodeQL CLI version in the toolcache, as requested by 'tools: ${toolsInput}'.`,
);
}
logger.info(
`Attempting to use the latest CodeQL CLI version in the toolcache, as requested by ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)}.`,
);
latestToolcacheVersion = getLatestToolcacheVersion(logger);
if (latestToolcacheVersion) {
@@ -432,23 +445,17 @@ export async function getCodeQLSource(
if (latestToolcacheVersion === undefined) {
if (allowToolcacheValue) {
if (toolsInputFromRepositoryProperty) {
logger.info(
`Found no CodeQL CLI in the toolcache, ignoring the 'github-codeql-tools' repository property...`,
);
} else {
logger.info(
`Found no CodeQL CLI in the toolcache, ignoring 'tools: ${toolsInput}'...`,
);
}
logger.info(
`Found no CodeQL CLI in the toolcache, ignoring ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)}...`,
);
} else {
if (allowToolcacheValueFF) {
logger.warning(
`Ignoring 'tools: ${toolsInput}' because the workflow was not triggered dynamically.`,
`Ignoring ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)} because the workflow was not triggered dynamically.`,
);
} else {
logger.info(
`Ignoring 'tools: ${toolsInput}' because the feature is not enabled.`,
logger.warning(
`Ignoring ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)} because the feature is not enabled.`,
);
}
}
@@ -808,7 +815,7 @@ export async function setupCodeQLBundle(
defaultCliVersion: CodeQLDefaultVersionInfo,
features: FeatureEnablement,
logger: Logger,
toolsInputFromRepositoryProperty = false,
toolsInputFromRepositoryProperty: boolean,
): Promise<SetupCodeQLResult> {
if (!(await util.isBinaryAccessible("tar", logger))) {
throw new util.ConfigurationError(