Skip to content

Commit f657c4e

Browse files
committed
Use getDefaultCliVersion for start-proxy
1 parent c0fc915 commit f657c4e

File tree

4 files changed

+139
-51
lines changed

4 files changed

+139
-51
lines changed

lib/start-proxy-action.js

Lines changed: 9 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/start-proxy-action.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as core from "@actions/core";
55

66
import * as actionsUtil from "./actions-util";
77
import { getGitHubVersion } from "./api-client";
8-
import { Feature, FeatureEnablement, initFeatures } from "./feature-flags";
8+
import { FeatureEnablement, initFeatures } from "./feature-flags";
99
import { KnownLanguage } from "./languages";
1010
import { getActionsLogger, Logger } from "./logging";
1111
import { getRepositoryNwo } from "./repository";
@@ -98,7 +98,7 @@ async function run(startedAt: Date) {
9898
};
9999

100100
// Start the Proxy
101-
const proxyBin = await getProxyBinaryPath(logger);
101+
const proxyBin = await getProxyBinaryPath(logger, features);
102102
const proxyInfo = await startProxy(
103103
proxyBin,
104104
proxyConfig,

src/start-proxy.test.ts

Lines changed: 110 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,27 @@ import sinon from "sinon";
77

88
import * as apiClient from "./api-client";
99
import * as defaults from "./defaults.json";
10+
import { setUpFeatureFlagTests } from "./feature-flags/testing-util";
1011
import { KnownLanguage } from "./languages";
1112
import { getRunnerLogger, Logger } from "./logging";
1213
import * as startProxyExports from "./start-proxy";
1314
import { parseLanguage } from "./start-proxy";
1415
import * as statusReport from "./status-report";
1516
import {
1617
checkExpectedLogMessages,
18+
createFeatures,
1719
getRecordingLogger,
1820
makeTestToken,
21+
RecordingLogger,
1922
setupTests,
2023
withRecordingLoggerAsync,
2124
} from "./testing-utils";
22-
import { ConfigurationError } from "./util";
25+
import {
26+
ConfigurationError,
27+
GitHubVariant,
28+
GitHubVersion,
29+
withTmpDir,
30+
} from "./util";
2331

2432
setupTests(test);
2533

@@ -347,8 +355,18 @@ test("parseLanguage", async (t) => {
347355
t.deepEqual(parseLanguage(""), undefined);
348356
});
349357

350-
function mockGetReleaseByTag(assets?: Array<{ name: string; url?: string }>) {
351-
const mockClient = sinon.stub(apiClient, "getApiClient");
358+
function mockGetApiClient(endpoints: any) {
359+
return (
360+
sinon
361+
.stub(apiClient, "getApiClient")
362+
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
363+
.returns({ rest: endpoints } as any)
364+
);
365+
}
366+
367+
type ReleaseAssets = Array<{ name: string; url?: string }>;
368+
369+
function mockGetReleaseByTag(assets?: ReleaseAssets) {
352370
const getReleaseByTag =
353371
assets === undefined
354372
? sinon.stub().rejects()
@@ -359,21 +377,28 @@ function mockGetReleaseByTag(assets?: Array<{ name: string; url?: string }>) {
359377
url: "GET /repos/:owner/:repo/releases/tags/:tag",
360378
});
361379

362-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
363-
mockClient.returns({
364-
rest: {
365-
repos: {
366-
getReleaseByTag,
367-
},
368-
},
369-
} as any);
370-
return mockClient;
380+
return mockGetApiClient({ repos: { getReleaseByTag } });
381+
}
382+
383+
function mockOfflineFeatures(tempDir: string, logger: Logger) {
384+
// Using GHES ensures that we are using `OfflineFeatures`.
385+
const gitHubVersion = {
386+
type: GitHubVariant.GHES,
387+
version: "3.0.0",
388+
};
389+
sinon.stub(apiClient, "getGitHubVersion").resolves(gitHubVersion);
390+
391+
return setUpFeatureFlagTests(tempDir, logger, gitHubVersion);
371392
}
372393

373-
test("getDownloadUrl returns fallback when `getLinkedRelease` rejects", async (t) => {
394+
test("getDownloadUrl returns fallback when `getReleaseByVersion` rejects", async (t) => {
374395
mockGetReleaseByTag();
375396

376-
const info = await startProxyExports.getDownloadUrl(getRunnerLogger(true));
397+
const features = createFeatures([]);
398+
const info = await startProxyExports.getDownloadUrl(
399+
getRunnerLogger(true),
400+
features,
401+
);
377402

378403
t.is(info.version, startProxyExports.UPDATEJOB_PROXY_VERSION);
379404
t.is(
@@ -383,33 +408,48 @@ test("getDownloadUrl returns fallback when `getLinkedRelease` rejects", async (t
383408
});
384409

385410
test("getDownloadUrl returns fallback when there's no matching release asset", async (t) => {
411+
const logger = new RecordingLogger();
386412
const testAssets = [[], [{ name: "foo" }]];
387413

388-
for (const assets of testAssets) {
389-
const stub = mockGetReleaseByTag(assets);
390-
const info = await startProxyExports.getDownloadUrl(getRunnerLogger(true));
414+
await withTmpDir(async (tempDir) => {
415+
const features = mockOfflineFeatures(tempDir, logger);
391416

392-
t.is(info.version, startProxyExports.UPDATEJOB_PROXY_VERSION);
393-
t.is(
394-
info.url,
395-
startProxyExports.getFallbackUrl(startProxyExports.getProxyPackage()),
396-
);
417+
for (const assets of testAssets) {
418+
const stub = mockGetReleaseByTag(assets);
419+
const info = await startProxyExports.getDownloadUrl(
420+
getRunnerLogger(true),
421+
features,
422+
);
397423

398-
stub.restore();
399-
}
424+
t.is(info.version, startProxyExports.UPDATEJOB_PROXY_VERSION);
425+
t.is(
426+
info.url,
427+
startProxyExports.getFallbackUrl(startProxyExports.getProxyPackage()),
428+
);
429+
430+
stub.restore();
431+
}
432+
});
400433
});
401434

402435
test("getDownloadUrl returns matching release asset", async (t) => {
436+
const logger = new RecordingLogger();
403437
const assets = [
404438
{ name: "foo", url: "other-url" },
405439
{ name: startProxyExports.getProxyPackage(), url: "url-we-want" },
406440
];
407441
mockGetReleaseByTag(assets);
408442

409-
const info = await startProxyExports.getDownloadUrl(getRunnerLogger(true));
443+
await withTmpDir(async (tempDir) => {
444+
const features = mockOfflineFeatures(tempDir, logger);
445+
const info = await startProxyExports.getDownloadUrl(
446+
getRunnerLogger(true),
447+
features,
448+
);
410449

411-
t.is(info.version, defaults.cliVersion);
412-
t.is(info.url, "url-we-want");
450+
t.is(info.version, defaults.cliVersion);
451+
t.is(info.url, "url-we-want");
452+
});
413453
});
414454

415455
test("credentialToStr - hides passwords", (t) => {
@@ -560,13 +600,15 @@ test(
560600
);
561601

562602
test("getProxyBinaryPath - returns path from tool cache if available", async (t) => {
603+
const logger = new RecordingLogger();
563604
mockGetReleaseByTag();
564605

565-
await withRecordingLoggerAsync(async (logger) => {
606+
await withTmpDir(async (tempDir) => {
566607
const toolcachePath = "/path/to/proxy/dir";
567608
sinon.stub(toolcache, "find").returns(toolcachePath);
568609

569-
const path = await startProxyExports.getProxyBinaryPath(logger);
610+
const features = mockOfflineFeatures(tempDir, logger);
611+
const path = await startProxyExports.getProxyBinaryPath(logger, features);
570612

571613
t.assert(path);
572614
t.is(
@@ -577,12 +619,31 @@ test("getProxyBinaryPath - returns path from tool cache if available", async (t)
577619
});
578620

579621
test("getProxyBinaryPath - downloads proxy if not in cache", async (t) => {
622+
const logger = new RecordingLogger();
623+
const expectedTag = "codeql-bundle-v2.20.1";
624+
const expectedParams = {
625+
owner: "github",
626+
repo: "codeql-action",
627+
tag: expectedTag,
628+
};
580629
const downloadUrl = "url-we-want";
581-
mockGetReleaseByTag([
582-
{ name: startProxyExports.getProxyPackage(), url: downloadUrl },
583-
]);
630+
const assets = [
631+
{
632+
name: startProxyExports.getProxyPackage(),
633+
url: downloadUrl,
634+
},
635+
];
584636

585-
await withRecordingLoggerAsync(async (logger) => {
637+
const getReleaseByTag = sinon.stub();
638+
getReleaseByTag.withArgs(sinon.match(expectedParams)).resolves({
639+
status: 200,
640+
data: { assets },
641+
headers: {},
642+
url: "GET /repos/:owner/:repo/releases/tags/:tag",
643+
});
644+
mockGetApiClient({ repos: { getReleaseByTag } });
645+
646+
await withTmpDir(async (tempDir) => {
586647
const toolcachePath = "/path/to/proxy/dir";
587648
const find = sinon.stub(toolcache, "find").returns("");
588649
const getApiDetails = sinon.stub(apiClient, "getApiDetails").returns({
@@ -603,8 +664,22 @@ test("getProxyBinaryPath - downloads proxy if not in cache", async (t) => {
603664
.resolves(extractedPath);
604665
const cacheDir = sinon.stub(toolcache, "cacheDir").resolves(toolcachePath);
605666

606-
const path = await startProxyExports.getProxyBinaryPath(logger);
607-
667+
const gitHubVersion: GitHubVersion = {
668+
type: GitHubVariant.DOTCOM,
669+
};
670+
sinon.stub(apiClient, "getGitHubVersion").resolves(gitHubVersion);
671+
672+
const features = setUpFeatureFlagTests(tempDir, logger, gitHubVersion);
673+
const getDefaultCliVersion = sinon
674+
.stub(features, "getDefaultCliVersion")
675+
.resolves({ cliVersion: "2.20.1", tagName: expectedTag });
676+
const path = await startProxyExports.getProxyBinaryPath(logger, features);
677+
678+
t.assert(getDefaultCliVersion.calledOnce);
679+
sinon.assert.calledOnceWithMatch(
680+
getReleaseByTag,
681+
sinon.match(expectedParams),
682+
);
608683
t.assert(find.calledOnce);
609684
t.assert(getApiDetails.calledOnce);
610685
t.assert(getAuthorizationHeaderFor.calledOnce);

src/start-proxy.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import {
77
getApiClient,
88
getApiDetails,
99
getAuthorizationHeaderFor,
10+
getGitHubVersion,
1011
} from "./api-client";
1112
import * as artifactScanner from "./artifact-scanner";
1213
import { Config } from "./config-utils";
1314
import * as defaults from "./defaults.json";
15+
import { FeatureEnablement } from "./feature-flags";
1416
import { KnownLanguage } from "./languages";
1517
import { Logger } from "./logging";
1618
import {
@@ -391,15 +393,15 @@ export function getFallbackUrl(proxyPackage: string): string {
391393

392394
/**
393395
* Uses the GitHub API to obtain information about the CodeQL CLI bundle release
394-
* that is pointed at by `defaults.json`.
396+
* that is tagged by `version`.
395397
*
396398
* @returns The response from the GitHub API.
397399
*/
398-
async function getLinkedRelease() {
400+
async function getReleaseByVersion(version: string) {
399401
return getApiClient().rest.repos.getReleaseByTag({
400402
owner: "github",
401403
repo: "codeql-action",
402-
tag: defaults.bundleVersion,
404+
tag: version,
403405
});
404406
}
405407

@@ -412,12 +414,18 @@ async function getLinkedRelease() {
412414
*/
413415
export async function getDownloadUrl(
414416
logger: Logger,
417+
features: FeatureEnablement,
415418
): Promise<{ url: string; version: string }> {
416419
const proxyPackage = getProxyPackage();
417420

418421
try {
419-
// Try to retrieve information about the CLI bundle release pointed at by `defaults.json`.
420-
const cliRelease = await getLinkedRelease();
422+
// Retrieve information about the CLI version we should use. This will be either the linked
423+
// version, or the one enabled by FFs.
424+
const gitHubVersion = await getGitHubVersion();
425+
const versionInfo = await features.getDefaultCliVersion(gitHubVersion.type);
426+
427+
// Try to retrieve information about the CLI bundle release identified by `versionInfo`.
428+
const cliRelease = await getReleaseByVersion(versionInfo.tagName);
421429

422430
// Search the release's assets to find the one we are looking for.
423431
for (const asset of cliRelease.data.assets) {
@@ -548,9 +556,12 @@ export function getProxyFilename() {
548556
* @param logger The logger to use.
549557
* @returns The path to the proxy binary.
550558
*/
551-
export async function getProxyBinaryPath(logger: Logger): Promise<string> {
559+
export async function getProxyBinaryPath(
560+
logger: Logger,
561+
features: FeatureEnablement,
562+
): Promise<string> {
552563
const proxyFileName = getProxyFilename();
553-
const proxyInfo = await getDownloadUrl(logger);
564+
const proxyInfo = await getDownloadUrl(logger, features);
554565

555566
let proxyBin = toolcache.find(proxyFileName, proxyInfo.version);
556567
if (!proxyBin) {

0 commit comments

Comments
 (0)