Skip to content

Ensure uniqueness of overlay-base database cache keys #3232

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
merged 4 commits into from
Oct 27, 2025
Merged

Ensure uniqueness of overlay-base database cache keys #3232

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c4b7372
Add overlay-base database cache key tests
cklin Sep 24, 2025
b4ce335
Ensure uniqueness of overlay-base database cache keys
kaspersv Oct 24, 2025
cbcae45
Reorder components of overlay-base cache key postfix
kaspersv Oct 24, 2025
66759e5
Improve error handling for overlay-base cache key creation
kaspersv Oct 24, 2025
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
4 changes: 3 additions & 1 deletion lib/analyze-action.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 41 additions & 0 deletions src/overlay-database-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import * as gitUtils from "./git-utils";
import { getRunnerLogger } from "./logging";
import {
downloadOverlayBaseDatabaseFromCache,
getCacheRestoreKeyPrefix,
getCacheSaveKey,
OverlayDatabaseMode,
writeBaseDatabaseOidsFile,
writeOverlayChangesFile,
Expand Down Expand Up @@ -261,3 +263,42 @@ test(
},
false,
);

test("overlay-base database cache keys remain stable", async (t) => {
const config = createTestConfig({ languages: ["python", "javascript"] });
const codeQlVersion = "2.23.0";
const commitOid = "abc123def456";

sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
sinon.stub(gitUtils, "getCommitOid").resolves(commitOid);
sinon.stub(actionsUtil, "getWorkflowRunID").returns(12345);
sinon.stub(actionsUtil, "getWorkflowRunAttempt").returns(1);

const saveKey = await getCacheSaveKey(config, codeQlVersion, "checkout-path");
const expectedSaveKey =
"codeql-overlay-base-database-1-c5666c509a2d9895-javascript_python-2.23.0-12345-1-abc123def456";
t.is(
saveKey,
expectedSaveKey,
"Cache save key changed unexpectedly. " +
"This may indicate breaking changes in the cache key generation logic.",
);

const restoreKeyPrefix = await getCacheRestoreKeyPrefix(
config,
codeQlVersion,
);
const expectedRestoreKeyPrefix =
"codeql-overlay-base-database-1-c5666c509a2d9895-javascript_python-2.23.0-";
t.is(
restoreKeyPrefix,
expectedRestoreKeyPrefix,
"Cache restore key prefix changed unexpectedly. " +
"This may indicate breaking changes in the cache key generation logic.",
);

t.true(
saveKey.startsWith(restoreKeyPrefix),
`Expected save key "${saveKey}" to start with restore key prefix "${restoreKeyPrefix}"`,
);
});
15 changes: 11 additions & 4 deletions src/overlay-database-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import * as path from "path";

import * as actionsCache from "@actions/cache";

import { getRequiredInput, getTemporaryDirectory } from "./actions-util";
import {
getRequiredInput,
getTemporaryDirectory,
getWorkflowRunAttempt,
getWorkflowRunID,
} from "./actions-util";
import { getAutomationID } from "./api-client";
import { type CodeQL } from "./codeql";
import { type Config } from "./config-utils";
Expand Down Expand Up @@ -448,17 +453,19 @@ export async function downloadOverlayBaseDatabaseFromCache(
* The key consists of the restore key prefix (which does not include the
* commit SHA) and the commit SHA of the current checkout.
*/
async function getCacheSaveKey(
export async function getCacheSaveKey(
config: Config,
codeQlVersion: string,
checkoutPath: string,
): Promise<string> {
const runId = getWorkflowRunID();
const attemptId = getWorkflowRunAttempt();
Copy link
Member

@mbg mbg Oct 24, 2025

Choose a reason for hiding this comment

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

These two functions may throw errors if the corresponding environment variables are not defined. I don't see any reason why that would be a concern since they should always be set in Actions, but I thought I'd mention it.

Copy link
Contributor Author

@kaspersv kaspersv Oct 24, 2025

Choose a reason for hiding this comment

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

It is a corner-case of a corner-case, but it doesn't add much code to handle it. I've added error handling in commit 4, such that we default to 1 in the unlikely case these functions fail.

const sha = await getCommitOid(checkoutPath);
const restoreKeyPrefix = await getCacheRestoreKeyPrefix(
config,
codeQlVersion,
);
return `${restoreKeyPrefix}${sha}`;
return `${restoreKeyPrefix}${runId}-${attemptId}-${sha}`;
}

/**
Expand All @@ -475,7 +482,7 @@ async function getCacheSaveKey(
* not include the commit SHA. This allows us to restore the most recent
* compatible overlay-base database.
*/
async function getCacheRestoreKeyPrefix(
export async function getCacheRestoreKeyPrefix(
config: Config,
codeQlVersion: string,
): Promise<string> {
Expand Down
Loading