Introduce type error when CodeQL is needed

This commit is contained in:
Henry Mercer
2026-01-05 15:35:45 +00:00
parent 0d648eb4d1
commit 35d39dfdb3
3 changed files with 131 additions and 102 deletions
+58 -49
View File
@@ -10,6 +10,8 @@ import {
FeatureEnablement,
Features,
FEATURE_FLAGS_FILE_NAME,
FeatureConfig,
FeatureWithoutCLI,
} from "./feature-flags";
import { getRunnerLogger } from "./logging";
import { parseRepositoryNwo } from "./repository";
@@ -46,7 +48,7 @@ test(`All features are disabled if running against GHES`, async (t) => {
for (const feature of Object.values(Feature)) {
t.deepEqual(
await features.getValue(feature, includeCodeQlIfRequired(feature)),
await getFeatureIncludingCodeQlIfRequired(features, feature),
featureConfig[feature].defaultValue,
);
}
@@ -75,9 +77,7 @@ test(`Feature flags are requested in GHEC-DR`, async (t) => {
for (const feature of Object.values(Feature)) {
// Ensure we have gotten a response value back from the Mock API
t.assert(
await features.getValue(feature, includeCodeQlIfRequired(feature)),
);
t.assert(await getFeatureIncludingCodeQlIfRequired(features, feature));
}
// And that we haven't bailed preemptively.
@@ -104,7 +104,7 @@ test("API response missing and features use default value", async (t) => {
for (const feature of Object.values(Feature)) {
t.assert(
(await features.getValue(feature, includeCodeQlIfRequired(feature))) ===
(await getFeatureIncludingCodeQlIfRequired(features, feature)) ===
featureConfig[feature].defaultValue,
);
}
@@ -124,7 +124,7 @@ test("Features use default value if they're not returned in API response", async
for (const feature of Object.values(Feature)) {
t.assert(
(await features.getValue(feature, includeCodeQlIfRequired(feature))) ===
(await getFeatureIncludingCodeQlIfRequired(features, feature)) ===
featureConfig[feature].defaultValue,
);
}
@@ -151,7 +151,7 @@ test("Include no more than 25 features in each API request", async (t) => {
// from the API.
const feature = Object.values(Feature)[0];
await t.notThrowsAsync(async () =>
features.getValue(feature, includeCodeQlIfRequired(feature)),
getFeatureIncludingCodeQlIfRequired(features, feature),
);
});
});
@@ -165,8 +165,7 @@ test("Feature flags exception is propagated if the API request errors", async (t
const someFeature = Object.values(Feature)[0];
await t.throwsAsync(
async () =>
features.getValue(someFeature, includeCodeQlIfRequired(someFeature)),
async () => getFeatureIncludingCodeQlIfRequired(features, someFeature),
{
message:
"Encountered an error while trying to determine feature enablement: Error: some error message",
@@ -190,9 +189,9 @@ for (const feature of Object.keys(featureConfig)) {
// retrieve the values of the actual features
const actualFeatureEnablement: { [feature: string]: boolean } = {};
for (const f of Object.keys(featureConfig)) {
actualFeatureEnablement[f] = await features.getValue(
actualFeatureEnablement[f] = await getFeatureIncludingCodeQlIfRequired(
features,
f as Feature,
includeCodeQlIfRequired(f),
);
}
@@ -210,19 +209,16 @@ for (const feature of Object.keys(featureConfig)) {
// feature should be disabled initially
t.assert(
!(await features.getValue(
!(await getFeatureIncludingCodeQlIfRequired(
features,
feature as Feature,
includeCodeQlIfRequired(feature),
)),
);
// set env var to true and check that the feature is now enabled
process.env[featureConfig[feature].envVar] = "true";
t.assert(
await features.getValue(
feature as Feature,
includeCodeQlIfRequired(feature),
),
await getFeatureIncludingCodeQlIfRequired(features, feature as Feature),
);
});
});
@@ -236,18 +232,15 @@ for (const feature of Object.keys(featureConfig)) {
// feature should be enabled initially
t.assert(
await features.getValue(
feature as Feature,
includeCodeQlIfRequired(feature),
),
await getFeatureIncludingCodeQlIfRequired(features, feature as Feature),
);
// set env var to false and check that the feature is now disabled
process.env[featureConfig[feature].envVar] = "false";
t.assert(
!(await features.getValue(
!(await getFeatureIncludingCodeQlIfRequired(
features,
feature as Feature,
includeCodeQlIfRequired(feature),
)),
);
});
@@ -264,13 +257,16 @@ for (const feature of Object.keys(featureConfig)) {
const expectedFeatureEnablement = initializeFeatures(true);
mockFeatureFlagApiEndpoint(200, expectedFeatureEnablement);
await t.throwsAsync(async () => features.getValue(feature as Feature), {
message: `Internal error: A ${
featureConfig[feature].minimumVersion !== undefined
? "minimum version"
: "required tools feature"
} is specified for feature ${feature}, but no instance of CodeQL was provided.`,
});
await t.throwsAsync(
async () => features.getValue(feature as FeatureWithoutCLI),
{
message: `Internal error: A ${
featureConfig[feature].minimumVersion !== undefined
? "minimum version"
: "required tools feature"
} is specified for feature ${feature}, but no instance of CodeQL was provided.`,
},
);
});
});
}
@@ -354,9 +350,9 @@ test("Feature flags are saved to disk", async (t) => {
);
t.true(
await features.getValue(
await getFeatureIncludingCodeQlIfRequired(
features,
Feature.QaTelemetryEnabled,
includeCodeQlIfRequired(Feature.QaTelemetryEnabled),
),
"Feature flag should be enabled initially",
);
@@ -382,9 +378,9 @@ test("Feature flags are saved to disk", async (t) => {
(features as any).gitHubFeatureFlags.cachedApiResponse = undefined;
t.false(
await features.getValue(
await getFeatureIncludingCodeQlIfRequired(
features,
Feature.QaTelemetryEnabled,
includeCodeQlIfRequired(Feature.QaTelemetryEnabled),
),
"Feature flag should be enabled after reading from cached file",
);
@@ -399,9 +395,9 @@ test("Environment variable can override feature flag cache", async (t) => {
const cachedFeatureFlags = path.join(tmpDir, FEATURE_FLAGS_FILE_NAME);
t.true(
await features.getValue(
await getFeatureIncludingCodeQlIfRequired(
features,
Feature.QaTelemetryEnabled,
includeCodeQlIfRequired(Feature.QaTelemetryEnabled),
),
"Feature flag should be enabled initially",
);
@@ -413,9 +409,9 @@ test("Environment variable can override feature flag cache", async (t) => {
process.env.CODEQL_ACTION_QA_TELEMETRY = "false";
t.false(
await features.getValue(
await getFeatureIncludingCodeQlIfRequired(
features,
Feature.QaTelemetryEnabled,
includeCodeQlIfRequired(Feature.QaTelemetryEnabled),
),
"Feature flag should be disabled after setting env var",
);
@@ -512,7 +508,7 @@ for (const variant of [GitHubVariant.DOTCOM, GitHubVariant.GHEC_DR]) {
test("legacy feature flags should end with _enabled", async (t) => {
for (const [feature, config] of Object.entries(featureConfig)) {
if (config.legacyApi) {
if ((config satisfies FeatureConfig as FeatureConfig).legacyApi) {
t.assert(
feature.endsWith("_enabled"),
`legacy feature ${feature} should end with '_enabled'`,
@@ -523,7 +519,7 @@ test("legacy feature flags should end with _enabled", async (t) => {
test("non-legacy feature flags should not end with _enabled", async (t) => {
for (const [feature, config] of Object.entries(featureConfig)) {
if (!config.legacyApi) {
if (!(config satisfies FeatureConfig as FeatureConfig).legacyApi) {
t.false(
feature.endsWith("_enabled"),
`non-legacy feature ${feature} should not end with '_enabled'`,
@@ -534,7 +530,7 @@ test("non-legacy feature flags should not end with _enabled", async (t) => {
test("non-legacy feature flags should not start with codeql_action_", async (t) => {
for (const [feature, config] of Object.entries(featureConfig)) {
if (!config.legacyApi) {
if (!(config satisfies FeatureConfig as FeatureConfig).legacyApi) {
t.false(
feature.startsWith("codeql_action_"),
`non-legacy feature ${feature} should not start with 'codeql_action_'`,
@@ -573,12 +569,25 @@ function setUpFeatureFlagTests(
* Returns an argument to pass to `getValue` that if required includes a CodeQL object meeting the
* minimum version or tool feature requirements specified by the feature.
*/
function includeCodeQlIfRequired(feature: string) {
return featureConfig[feature].minimumVersion !== undefined ||
featureConfig[feature].toolsFeature !== undefined
? mockCodeQLVersion(
"9.9.9",
Object.fromEntries(Object.values(ToolsFeature).map((v) => [v, true])),
)
: undefined;
function getFeatureIncludingCodeQlIfRequired(
features: FeatureEnablement,
feature: Feature,
) {
const config = featureConfig[
feature
] satisfies FeatureConfig as FeatureConfig;
if (
config.minimumVersion === undefined &&
config.toolsFeature === undefined
) {
return features.getValue(feature as FeatureWithoutCLI);
}
return features.getValue(
feature,
mockCodeQLVersion(
"9.9.9",
Object.fromEntries(Object.values(ToolsFeature).map((v) => [v, true])),
),
);
}
+72 -52
View File
@@ -26,16 +26,8 @@ export interface CodeQLDefaultVersionInfo {
toolsFeatureFlagsValid?: boolean;
}
export interface FeatureEnablement {
/** Gets the default version of the CodeQL tools. */
getDefaultCliVersion(
variant: util.GitHubVariant,
): Promise<CodeQLDefaultVersionInfo>;
getValue(feature: Feature, codeql?: CodeQL): Promise<boolean>;
}
/**
* Feature enablement as returned by the GitHub API endpoint.
* Features as named by the GitHub API endpoint.
*
* Do not include the `codeql_action_` prefix as this is stripped by the API
* endpoint.
@@ -82,37 +74,36 @@ export enum Feature {
ValidateDbConfig = "validate_db_config",
}
export const featureConfig: Record<
Feature,
{
/**
* Default value in environments where the feature flags API is not available,
* such as GitHub Enterprise Server.
*/
defaultValue: boolean;
/**
* Environment variable for explicitly enabling or disabling the feature.
*
* This overrides enablement status from the feature flags API.
*/
envVar: string;
/**
* Whether the feature flag is part of the legacy feature flags API (defaults to false).
*
* These feature flags are included by default in the API response and do not need to be
* explicitly requested.
*/
legacyApi?: boolean;
/**
* Minimum version of the CLI, if applicable.
*
* Prefer using `ToolsFeature`s for future flags.
*/
minimumVersion: string | undefined;
/** Required tools feature, if applicable. */
toolsFeature?: ToolsFeature;
}
> = {
export type FeatureConfig = {
/**
* Default value in environments where the feature flags API is not available,
* such as GitHub Enterprise Server.
*/
defaultValue: boolean;
/**
* Environment variable for explicitly enabling or disabling the feature.
*
* This overrides enablement status from the feature flags API.
*/
envVar: string;
/**
* Whether the feature flag is part of the legacy feature flags API (defaults to false).
*
* These feature flags are included by default in the API response and do not need to be
* explicitly requested.
*/
legacyApi?: boolean;
/**
* Minimum version of the CLI, if applicable.
*
* Prefer using `ToolsFeature`s for future flags.
*/
minimumVersion: string | undefined;
/** Required tools feature, if applicable. */
toolsFeature?: ToolsFeature;
};
export const featureConfig = {
[Feature.AllowToolcacheInput]: {
defaultValue: false,
envVar: "CODEQL_ACTION_ALLOW_TOOLCACHE_INPUT",
@@ -305,7 +296,29 @@ export const featureConfig: Record<
envVar: "CODEQL_ACTION_VALIDATE_DB_CONFIG",
minimumVersion: undefined,
},
};
} satisfies Record<Feature, FeatureConfig>;
/** A feature whose enablement does not depend on the version of the CodeQL CLI. */
export type FeatureWithoutCLI = {
[K in Feature]: (typeof featureConfig)[K] extends
| {
minimumVersion: string;
}
| {
toolsFeature: ToolsFeature;
}
? never
: K;
}[keyof typeof featureConfig];
export interface FeatureEnablement {
/** Gets the default version of the CodeQL tools. */
getDefaultCliVersion(
variant: util.GitHubVariant,
): Promise<CodeQLDefaultVersionInfo>;
getValue(feature: FeatureWithoutCLI): Promise<boolean>;
getValue(feature: Feature, codeql: CodeQL): Promise<boolean>;
}
/**
* A response from the GitHub API that contains feature flag enablement information for the CodeQL
@@ -358,31 +371,35 @@ export class Features implements FeatureEnablement {
* @throws if a `minimumVersion` is specified for the feature, and `codeql` is not provided.
*/
async getValue(feature: Feature, codeql?: CodeQL): Promise<boolean> {
if (!codeql && featureConfig[feature].minimumVersion) {
// Narrow the type to FeatureConfig to avoid type errors. To avoid unsafe use of `as`, we
// check that the required properties exist using `satisfies`.
const config = featureConfig[
feature
] satisfies FeatureConfig as FeatureConfig;
if (!codeql && config.minimumVersion) {
throw new Error(
`Internal error: A minimum version is specified for feature ${feature}, but no instance of CodeQL was provided.`,
);
}
if (!codeql && featureConfig[feature].toolsFeature) {
if (!codeql && config.toolsFeature) {
throw new Error(
`Internal error: A required tools feature is specified for feature ${feature}, but no instance of CodeQL was provided.`,
);
}
const envVar = (
process.env[featureConfig[feature].envVar] || ""
).toLocaleLowerCase();
const envVar = (process.env[config.envVar] || "").toLocaleLowerCase();
// Do not use this feature if user explicitly disables it via an environment variable.
if (envVar === "false") {
this.logger.debug(
`Feature ${feature} is disabled via the environment variable ${featureConfig[feature].envVar}.`,
`Feature ${feature} is disabled via the environment variable ${config.envVar}.`,
);
return false;
}
// Never use this feature if the CLI version explicitly can't support it.
const minimumVersion = featureConfig[feature].minimumVersion;
const minimumVersion = config.minimumVersion;
if (codeql && minimumVersion) {
if (!(await util.codeQlVersionAtLeast(codeql, minimumVersion))) {
this.logger.debug(
@@ -399,7 +416,7 @@ export class Features implements FeatureEnablement {
);
}
}
const toolsFeature = featureConfig[feature].toolsFeature;
const toolsFeature = config.toolsFeature;
if (codeql && toolsFeature) {
if (!(await codeql.supportsFeature(toolsFeature))) {
this.logger.debug(
@@ -419,7 +436,7 @@ export class Features implements FeatureEnablement {
// Use this feature if user explicitly enables it via an environment variable.
if (envVar === "true") {
this.logger.debug(
`Feature ${feature} is enabled via the environment variable ${featureConfig[feature].envVar}.`,
`Feature ${feature} is enabled via the environment variable ${config.envVar}.`,
);
return true;
}
@@ -435,7 +452,7 @@ export class Features implements FeatureEnablement {
return apiValue;
}
const defaultValue = featureConfig[feature].defaultValue;
const defaultValue = config.defaultValue;
this.logger.debug(
`Feature ${feature} is ${
defaultValue ? "enabled" : "disabled"
@@ -631,7 +648,10 @@ class GitHubFeatureFlags {
}
try {
const featuresToRequest = Object.entries(featureConfig)
.filter(([, config]) => !config.legacyApi)
.filter(
([, config]) =>
!(config satisfies FeatureConfig as FeatureConfig).legacyApi,
)
.map(([f]) => f);
const FEATURES_PER_REQUEST = 25;
+1 -1
View File
@@ -316,7 +316,7 @@ export function createFeatures(enabledFeatures: Feature[]): FeatureEnablement {
throw new Error("not implemented");
},
getValue: async (feature) => {
return enabledFeatures.includes(feature);
return enabledFeatures.includes(feature as Feature);
},
};
}