robojerk/codeql-action

Merge pull request #1567 from github/aeisenberg/config-parsing-ghes

Browse source 
Add default values to feature flags
This commit is contained in:
Andrew Eisenberg 2023-03-08 09:44:44 -08:00 • committed by GitHub
commit f13b180fb8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 41 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

15
lib/feature-flags.js generated
View file

@ -43,18 +43,22 @@ exports.featureConfig = {
[Feature.DisableKotlinAnalysisEnabled]: {
envVar: "CODEQL_DISABLE_KOTLIN_ANALYSIS",
minimumVersion: undefined,
defaultValue: false,
},
[Feature.CliConfigFileEnabled]: {
envVar: "CODEQL_PASS_CONFIG_TO_CLI",
minimumVersion: "2.11.6",
defaultValue: true,
},
[Feature.MlPoweredQueriesEnabled]: {
envVar: "CODEQL_ML_POWERED_QUERIES",
minimumVersion: "2.7.5",
defaultValue: false,
},
[Feature.UploadFailedSarifEnabled]: {
envVar: "CODEQL_ACTION_UPLOAD_FAILED_SARIF",
minimumVersion: "2.11.3",
defaultValue: false,
},
};
exports.FEATURE_FLAGS_FILE_NAME = "cached-feature-flags.json";
@ -103,7 +107,8 @@ class Features {
return true;
}
// Ask the GitHub API if the feature is enabled.
return await this.gitHubFeatureFlags.getValue(feature);
return ((await this.gitHubFeatureFlags.getValue(feature)) ??
exports.featureConfig[feature].defaultValue);
}
}
exports.Features = Features;
@ -179,13 +184,13 @@ class GitHubFeatureFlags {
async getValue(feature) {
const response = await this.getAllFeatures();
if (response === undefined) {
this.logger.debug(`No feature flags API response for ${feature}, considering it disabled.`);
return false;
this.logger.debug(`No feature flags API response for ${feature}.`);
return undefined;
}
const featureEnablement = response[feature];
if (featureEnablement === undefined) {
this.logger.debug(`Feature '${feature}' undefined in API response, considering it disabled.`);
return false;
this.logger.debug(`Feature '${feature}' undefined in API response.`);
return undefined;
}
return !!featureEnablement;
}

2
lib/feature-flags.js.map
View file

File diff suppressed because one or more lines are too long

12
lib/feature-flags.test.js generated
View file

@ -53,7 +53,7 @@ for (const variant of ALL_FEATURES_DISABLED_VARIANTS) {
const loggedMessages = [];
const featureEnablement = setUpFeatureFlagTests(tmpDir, (0, testing_utils_1.getRecordingLogger)(loggedMessages), variant.gitHubVersion);
for (const feature of Object.values(feature_flags_1.Feature)) {
t.false(await featureEnablement.getValue(feature, includeCodeQlIfRequired(feature)));
t.deepEqual(await featureEnablement.getValue(feature, includeCodeQlIfRequired(feature)), feature_flags_1.featureConfig[feature].defaultValue);
}
t.assert(loggedMessages.find((v) => v.type === "debug" &&
v.message ===
@ -61,24 +61,24 @@ for (const variant of ALL_FEATURES_DISABLED_VARIANTS) {
});
});
}
(0, ava_1.default)("API response missing", async (t) => {
(0, ava_1.default)("API response missing and features use default value", async (t) => {
await (0, util_1.withTmpDir)(async (tmpDir) => {
const loggedMessages = [];
const featureEnablement = setUpFeatureFlagTests(tmpDir, (0, testing_utils_1.getRecordingLogger)(loggedMessages));
(0, testing_utils_1.mockFeatureFlagApiEndpoint)(403, {});
for (const feature of Object.values(feature_flags_1.Feature)) {
t.assert((await featureEnablement.getValue(feature, includeCodeQlIfRequired(feature))) === false);
t.assert((await featureEnablement.getValue(feature, includeCodeQlIfRequired(feature))) === feature_flags_1.featureConfig[feature].defaultValue);
}
assertAllFeaturesUndefinedInApi(t, loggedMessages);
});
});
(0, ava_1.default)("Features are disabled if they're not returned in API response", async (t) => {
(0, ava_1.default)("Features use default value if they're not returned in API response", async (t) => {
await (0, util_1.withTmpDir)(async (tmpDir) => {
const loggedMessages = [];
const featureEnablement = setUpFeatureFlagTests(tmpDir, (0, testing_utils_1.getRecordingLogger)(loggedMessages));
(0, testing_utils_1.mockFeatureFlagApiEndpoint)(200, {});
for (const feature of Object.values(feature_flags_1.Feature)) {
t.assert((await featureEnablement.getValue(feature, includeCodeQlIfRequired(feature))) === false);
t.assert((await featureEnablement.getValue(feature, includeCodeQlIfRequired(feature))) === feature_flags_1.featureConfig[feature].defaultValue);
}
assertAllFeaturesUndefinedInApi(t, loggedMessages);
});
@ -283,7 +283,7 @@ function assertAllFeaturesUndefinedInApi(t, loggedMessages) {
for (const feature of Object.keys(feature_flags_1.featureConfig)) {
t.assert(loggedMessages.find((v) => v.type === "debug" &&
v.message.includes(feature) &&
v.message.includes("considering it disabled")) !== undefined);
v.message.includes("undefined in API response")) !== undefined);
}
}
function initializeFeatures(initialValue) {

2
lib/feature-flags.test.js.map
View file

File diff suppressed because one or more lines are too long

15
src/feature-flags.test.ts
View file

@ -54,11 +54,12 @@ for (const variant of ALL_FEATURES_DISABLED_VARIANTS) {
);
for (const feature of Object.values(Feature)) {
t.false(
t.deepEqual(
await featureEnablement.getValue(
feature,
includeCodeQlIfRequired(feature)
)
),
featureConfig[feature].defaultValue
);
}
@ -74,7 +75,7 @@ for (const variant of ALL_FEATURES_DISABLED_VARIANTS) {
});
}
test("API response missing", async (t) => {
test("API response missing and features use default value", async (t) => {
await withTmpDir(async (tmpDir) => {
const loggedMessages: LoggedMessage[] = [];
const featureEnablement = setUpFeatureFlagTests(
@ -89,14 +90,14 @@ test("API response missing", async (t) => {
(await featureEnablement.getValue(
feature,
includeCodeQlIfRequired(feature)
)) === false
)) === featureConfig[feature].defaultValue
);
}
assertAllFeaturesUndefinedInApi(t, loggedMessages);
});
});
test("Features are disabled if they're not returned in API response", async (t) => {
test("Features use default value if they're not returned in API response", async (t) => {
await withTmpDir(async (tmpDir) => {
const loggedMessages: LoggedMessage[] = [];
const featureEnablement = setUpFeatureFlagTests(
@ -111,7 +112,7 @@ test("Features are disabled if they're not returned in API response", async (t)
(await featureEnablement.getValue(
feature,
includeCodeQlIfRequired(feature)
)) === false
)) === featureConfig[feature].defaultValue
);
}
@ -471,7 +472,7 @@ function assertAllFeaturesUndefinedInApi(
(v) =>
v.type === "debug" &&
(v.message as string).includes(feature) &&
(v.message as string).includes("considering it disabled")
(v.message as string).includes("undefined in API response")
) !== undefined
);
}

27
src/feature-flags.ts
View file

@ -42,23 +42,27 @@ export enum Feature {
export const featureConfig: Record<
Feature,
{ envVar: string; minimumVersion: string | undefined }
{ envVar: string; minimumVersion: string | undefined; defaultValue: boolean }
> = {
[Feature.DisableKotlinAnalysisEnabled]: {
envVar: "CODEQL_DISABLE_KOTLIN_ANALYSIS",
minimumVersion: undefined,
defaultValue: false,
},
[Feature.CliConfigFileEnabled]: {
envVar: "CODEQL_PASS_CONFIG_TO_CLI",
minimumVersion: "2.11.6",
defaultValue: true,
},
[Feature.MlPoweredQueriesEnabled]: {
envVar: "CODEQL_ML_POWERED_QUERIES",
minimumVersion: "2.7.5",
defaultValue: false,
},
[Feature.UploadFailedSarifEnabled]: {
envVar: "CODEQL_ACTION_UPLOAD_FAILED_SARIF",
minimumVersion: "2.11.3",
defaultValue: false,
},
};
@ -141,11 +145,14 @@ export class Features implements FeatureEnablement {
return true;
}
// Ask the GitHub API if the feature is enabled.
return await this.gitHubFeatureFlags.getValue(feature);
return (
(await this.gitHubFeatureFlags.getValue(feature)) ??
featureConfig[feature].defaultValue
);
}
}
class GitHubFeatureFlags implements FeatureEnablement {
class GitHubFeatureFlags {
private cachedApiResponse: GitHubFeatureFlagsApiResponse | undefined;
// We cache whether the feature flags were accessed or not in order to accurately report whether flags were
@ -251,20 +258,16 @@ class GitHubFeatureFlags implements FeatureEnablement {
return { version: maxCliVersion, toolsFeatureFlagsValid: true };
}
async getValue(feature: Feature): Promise<boolean> {
async getValue(feature: Feature): Promise<boolean | undefined> {
const response = await this.getAllFeatures();
if (response === undefined) {
this.logger.debug(
`No feature flags API response for ${feature}, considering it disabled.`
);
return false;
this.logger.debug(`No feature flags API response for ${feature}.`);
return undefined;
}
const featureEnablement = response[feature];
if (featureEnablement === undefined) {
this.logger.debug(
`Feature '${feature}' undefined in API response, considering it disabled.`
);
return false;
this.logger.debug(`Feature '${feature}' undefined in API response.`);
return undefined;
}
return !!featureEnablement;
}