Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 830c231

Browse filesBrowse files
mbgmbg
authored
Merge pull request #4023 from github/mbg/ff/remove-new-remote-file-addresses-ff
Promote `NewRemoteFileAddresses` FF
2 parents 69fd9e9 + dd35309 commit 830c231
Copy full SHA for 830c231

9 files changed

+25-112Lines changed: 25 additions & 112 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎.github/workflows/__start-proxy.yml‎

Copy file name to clipboardExpand all lines: .github/workflows/__start-proxy.yml
-1Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎lib/entry-points.js‎

Copy file name to clipboardExpand all lines: lib/entry-points.js
+4-25Lines changed: 4 additions & 25 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎pr-checks/checks/start-proxy.yml‎

Copy file name to clipboardExpand all lines: pr-checks/checks/start-proxy.yml
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ steps:
4848
CODEQL_PROXY_HOST: ${{ steps.proxy.outputs.proxy_host }}
4949
CODEQL_PROXY_PORT: ${{ steps.proxy.outputs.proxy_port }}
5050
CODEQL_PROXY_CA_CERTIFICATE: ${{ steps.proxy.outputs.proxy_ca_certificate }}
51-
CODEQL_ACTION_NEW_REMOTE_FILE_ADDRESSES: "true"
5251
with:
5352
languages: java
5453
tools: ${{ steps.prepare-test.outputs.tools-url }}
Collapse file

‎src/config-utils.test.ts‎

Copy file name to clipboardExpand all lines: src/config-utils.test.ts
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2607,9 +2607,7 @@ test.serial(
26072607
const getRemoteConfig = sinon.stub(file, "getRemoteConfig").resolves({});
26082608

26092609
// Construct the basic test target.
2610-
const target = callee(configUtils.loadUserConfig)
2611-
.withDefaultActionsEnv()
2612-
.withFeatures([Feature.NewRemoteFileAddresses]);
2610+
const target = callee(configUtils.loadUserConfig).withDefaultActionsEnv();
26132611

26142612
// Utility function to assert that `targetWithArgs` has identified
26152613
// the input as a remote file address.
Collapse file

‎src/config-utils.ts‎

Copy file name to clipboardExpand all lines: src/config-utils.ts
+4-8Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -489,11 +489,7 @@ export async function loadUserConfig(
489489
apiDetails: api.GitHubApiCombinedDetails,
490490
tempDir: string,
491491
): Promise<UserConfig> {
492-
const allowNewFormat = await actionState.features.getValue(
493-
Feature.NewRemoteFileAddresses,
494-
);
495-
496-
if (isLocal(configFile, allowNewFormat)) {
492+
if (isLocal(configFile)) {
497493
if (configFile !== userConfigFromActionPath(tempDir)) {
498494
// If the config file is not generated by the Action, it should be relative to the workspace.
499495
configFile = path.resolve(workspacePath, configFile);
@@ -512,7 +508,7 @@ export async function loadUserConfig(
512508
// Drop the explicit prefix if it is present. Since `REMOTE_PATH_PREFIX` is chosen
513509
// to not conflict with permissible characters in "owner" or "repo" components,
514510
// this does not risk removing valid parts of either component by accident.
515-
if (allowNewFormat && isExplicitRemotePath(configFile)) {
511+
if (isExplicitRemotePath(configFile)) {
516512
configFile = configFile.substring(REMOTE_PATH_PREFIX.length);
517513
}
518514
return await getRemoteConfig(actionState, configFile, apiDetails);
@@ -1326,7 +1322,7 @@ function containsAtRef(configPath: string): boolean {
13261322
* @param configPath The path to test.
13271323
* @returns True if it is local, or false otherwise.
13281324
*/
1329-
function isLocal(configPath: string, allowNewFormat: boolean): boolean {
1325+
function isLocal(configPath: string): boolean {
13301326
// If the path starts with `LOCAL_PATH_PREFIX`, it is explicitly local.
13311327
// This allows local paths that would otherwise contain '@'
13321328
// to be used with a `LOCAL_PATH_PREFIX` prefix.
@@ -1335,7 +1331,7 @@ function isLocal(configPath: string, allowNewFormat: boolean): boolean {
13351331
}
13361332
// If the path starts with `REMOTE_PATH_PREFIX`, it is explicitly remote.
13371333
// This allows users to resolve ambiguity by specifying `REMOTE_PATH_PREFIX`.
1338-
if (allowNewFormat && isExplicitRemotePath(configPath)) {
1334+
if (isExplicitRemotePath(configPath)) {
13391335
return false;
13401336
}
13411337

Collapse file

‎src/config/file.test.ts‎

Copy file name to clipboardExpand all lines: src/config/file.test.ts
+2-3Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ test.serial("getRemoteConfig uses proxy when it is supposed to", async (t) => {
108108

109109
// Should use it when the FF is enabled and the environment variables are set.
110110
await target
111-
.withFeatures([Feature.ProxyApiRequests, Feature.NewRemoteFileAddresses])
111+
.withFeatures([Feature.ProxyApiRequests])
112112
.withEnv((env) => {
113113
env.set(RegistryProxyVars.PROXY_HOST, "localhost");
114114
env.set(RegistryProxyVars.PROXY_PORT, "1234");
@@ -118,7 +118,6 @@ test.serial("getRemoteConfig uses proxy when it is supposed to", async (t) => {
118118

119119
// But not when the FF is not enabled.
120120
await target
121-
.withFeatures([Feature.NewRemoteFileAddresses])
122121
.withEnv((env) => {
123122
env.set(RegistryProxyVars.PROXY_HOST, "localhost");
124123
env.set(RegistryProxyVars.PROXY_PORT, "1234");
@@ -128,7 +127,7 @@ test.serial("getRemoteConfig uses proxy when it is supposed to", async (t) => {
128127

129128
// And not when the environment variables aren't set.
130129
await target
131-
.withFeatures([Feature.ProxyApiRequests, Feature.NewRemoteFileAddresses])
130+
.withFeatures([Feature.ProxyApiRequests])
132131
.notLogs(t, "Using private registry proxy at 'http://localhost:1234'")
133132
.throws(t, { message: errorMessage });
134133
});
Collapse file

‎src/config/remote-file.test.ts‎

Copy file name to clipboardExpand all lines: src/config/remote-file.test.ts
+14-53Lines changed: 14 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ import test from "ava";
22
import sinon from "sinon";
33

44
import { ActionsEnvVars } from "../environment";
5-
import * as errors from "../error-messages";
6-
import { Feature } from "../feature-flags";
75
import { callee } from "../testing-utils";
86
import { ConfigurationError } from "../util";
97

@@ -75,15 +73,7 @@ test("parseRemoteFileAddress accepts full remote addresses", async (t) => {
7573
for (const newFormatInput of newFormatInputs) {
7674
const targetWithArgs = target.withArgs(newFormatInput.input);
7775

78-
// Should fail when the FF is not enabled.
79-
await targetWithArgs
80-
.withFeatures([])
81-
.throws(t, { instanceOf: ConfigurationError });
82-
83-
// And pass when the FF is enabled.
84-
await targetWithArgs
85-
.withFeatures([Feature.NewRemoteFileAddresses])
86-
.passes(t.deepEqual, newFormatInput.expected);
76+
await targetWithArgs.passes(t.deepEqual, newFormatInput.expected);
8777
}
8878
});
8979

@@ -138,15 +128,7 @@ test("parseRemoteFileAddress accepts remote address without an owner", async (t)
138128
for (const testCase of testCases) {
139129
const targetWithArgs = target.withArgs(testCase.input);
140130

141-
// Should fail when the FF is not enabled.
142-
await targetWithArgs
143-
.withFeatures([])
144-
.throws(t, { instanceOf: ConfigurationError });
145-
146-
// And pass when the FF is enabled.
147-
await targetWithArgs
148-
.withFeatures([Feature.NewRemoteFileAddresses])
149-
.passes(t.deepEqual, testCase.expected);
131+
await targetWithArgs.passes(t.deepEqual, testCase.expected);
150132
}
151133
});
152134

@@ -160,9 +142,7 @@ test("parseRemoteFileAddress throws for invalid `GITHUB_REPOSITORY`", async (t)
160142
sinon.define(env, "getRequired", getRequired);
161143
});
162144

163-
await target
164-
.withFeatures([Feature.NewRemoteFileAddresses])
165-
.throws(t, { instanceOf: Error });
145+
await target.throws(t, { instanceOf: Error });
166146

167147
t.assert(getRequired.calledOnceWith(ActionsEnvVars.GITHUB_REPOSITORY));
168148
});
@@ -194,31 +174,19 @@ test("parseRemoteFileAddress accepts remote address without a path", async (t) =
194174
for (const testCase of testCases) {
195175
const targetWithArgs = target.withArgs(testCase.input);
196176

197-
// Should fail when the FF is not enabled.
198-
await targetWithArgs
199-
.withFeatures([])
200-
.throws(t, { instanceOf: ConfigurationError });
201-
202-
// And pass when the FF is enabled.
203-
await targetWithArgs
204-
.withFeatures([Feature.NewRemoteFileAddresses])
205-
.passes(t.deepEqual, testCase.expected);
177+
await targetWithArgs.passes(t.deepEqual, testCase.expected);
206178
}
207179
});
208180

209181
test("parseRemoteFileAddress accepts remote address without a ref", async (t) => {
210182
const target = callee(parseRemoteFileAddress).withArgs("owner/repo:path");
211183

212-
// Should only accept the input if the FF is enabled.
213-
await target.withFeatures([]).throws(t);
214-
await target
215-
.withFeatures([Feature.NewRemoteFileAddresses])
216-
.passes(t.deepEqual, {
217-
owner: "owner",
218-
repo: "repo",
219-
path: "path",
220-
ref: DEFAULT_CONFIG_FILE_REF,
221-
} satisfies RemoteFileAddress);
184+
await target.passes(t.deepEqual, {
185+
owner: "owner",
186+
repo: "repo",
187+
path: "path",
188+
ref: DEFAULT_CONFIG_FILE_REF,
189+
} satisfies RemoteFileAddress);
222190
});
223191

224192
test("parseRemoteFileAddress rejects invalid values", async (t) => {
@@ -251,18 +219,11 @@ test("parseRemoteFileAddress rejects invalid values", async (t) => {
251219
for (const testInput of testInputs) {
252220
const targetWithArgs = target.withArgs(testInput);
253221

254-
// Should throw both when the new format is and isn't accepted.
255-
await targetWithArgs.withFeatures([]).throws(t, {
222+
await targetWithArgs.throws(t, {
223+
// When the new format is accepted, there are some more specific
224+
// errors in some cases. It is sufficient for us to check that
225+
// an exception is thrown.
256226
instanceOf: ConfigurationError,
257-
message: errors.getConfigFileRepoOldFormatInvalidMessage(testInput),
258227
});
259-
await targetWithArgs
260-
.withFeatures([Feature.NewRemoteFileAddresses])
261-
.throws(t, {
262-
// When the new format is accepted, there are some more specific
263-
// errors in some cases. It is sufficient for us to check that
264-
// an exception is thrown.
265-
instanceOf: ConfigurationError,
266-
});
267228
}
268229
});
Collapse file

‎src/config/remote-file.ts‎

Copy file name to clipboardExpand all lines: src/config/remote-file.ts
-11Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { ActionState } from "../action-common";
22
import { ActionsEnvVars, ReadOnlyEnv } from "../environment";
33
import * as errorMessages from "../error-messages";
4-
import { Feature } from "../feature-flags";
54
import { ConfigurationError, Failure, Result, Success } from "../util";
65

76
/** Represents remote file addresses. */
@@ -126,16 +125,6 @@ export async function parseRemoteFileAddress(
126125
return oldFormatAddressResult.value;
127126
}
128127

129-
// If the FF for the new format is not enabled, throw the old format error.
130-
const allowNewFormat = await actionState.features.getValue(
131-
Feature.NewRemoteFileAddresses,
132-
);
133-
if (!allowNewFormat) {
134-
throw new ConfigurationError(
135-
errorMessages.getConfigFileRepoOldFormatInvalidMessage(configFile),
136-
);
137-
}
138-
139128
// retrieve the various parts of the config location, and ensure they're present
140129
const newFormatAddressResult = parseNewRemoteFileAddress(
141130
actionState.env,
Collapse file

‎src/feature-flags.ts‎

Copy file name to clipboardExpand all lines: src/feature-flags.ts
-7Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,6 @@ export enum Feature {
9494
ForceNightly = "force_nightly",
9595
IgnoreGeneratedFiles = "ignore_generated_files",
9696
JavaNetworkDebugging = "java_network_debugging",
97-
/** Allow the new remote file address format. */
98-
NewRemoteFileAddresses = "new_remote_file_addresses",
9997
OverlayAnalysis = "overlay_analysis",
10098
OverlayAnalysisCodeScanningCpp = "overlay_analysis_code_scanning_cpp",
10199
OverlayAnalysisCodeScanningCsharp = "overlay_analysis_code_scanning_csharp",
@@ -266,11 +264,6 @@ export const featureConfig = {
266264
envVar: "CODEQL_ACTION_JAVA_NETWORK_DEBUGGING",
267265
minimumVersion: undefined,
268266
},
269-
[Feature.NewRemoteFileAddresses]: {
270-
defaultValue: false,
271-
envVar: "CODEQL_ACTION_NEW_REMOTE_FILE_ADDRESSES",
272-
minimumVersion: undefined,
273-
},
274267
[Feature.OverlayAnalysis]: {
275268
defaultValue: false,
276269
envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS",

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.