Apply suggestions from code review

This commit is contained in:
Andrew Eisenberg 2022-09-06 10:41:32 -07:00
parent 7e086b240c
commit bf97a6da5b
7 changed files with 55 additions and 24 deletions

View file

@ -2253,12 +2253,13 @@ test("downloadPacks-no-registries", async (t) => {
go: ["c", "d"],
python: ["e", "f"],
},
undefined,
undefined, // registries
sampleApiDetails,
tmpDir,
logger
);
// Expecting packs to be downloaded once for java and once for python
t.deepEqual(packDownloadStub.callCount, 2);
// no config file was created, so pass `undefined` as the config file path
t.deepEqual(packDownloadStub.firstCall.args, [["a", "b"], undefined]);
@ -2283,7 +2284,7 @@ test("downloadPacks-with-registries", async (t) => {
{
url: "https://containers.GHEHOSTNAME1/v2/",
packages: "semmle/*",
token: "still-a-token",
token: "still-not-a-token",
},
];
@ -2292,7 +2293,7 @@ test("downloadPacks-with-registries", async (t) => {
packDownloadStub.callsFake((packs, configFile) => {
t.deepEqual(configFile, expectedConfigFile);
// verify the env vars were set correctly
t.deepEqual(process.env.GITHUB_TOKEN, "token");
t.deepEqual(process.env.GITHUB_TOKEN, sampleApiDetails.auth);
t.deepEqual(
process.env.CODEQL_REGISTRIES_AUTH,
"http://ghcr.io=not-a-token,https://containers.GHEHOSTNAME1/v2/=still-a-token"
@ -2300,7 +2301,7 @@ test("downloadPacks-with-registries", async (t) => {
// verify the config file contents were set correctly
const config = yaml.load(fs.readFileSync(configFile, "utf8")) as {
registries: configUtils.SafeRegistryConfig[];
registries: configUtils.RegistryConfigNoCredentials[];
};
t.deepEqual(
config.registries,

View file

@ -61,7 +61,7 @@ export interface UserConfig {
export type QueryFilter = ExcludeQueryFilter | IncludeQueryFilter;
export type RegistryConfig = SafeRegistryConfig & {
export type RegistryConfigWithCredentials = RegistryConfigNoCredentials & {
// Token to use when downloading packs from this registry.
token: string;
};
@ -70,7 +70,7 @@ export type RegistryConfig = SafeRegistryConfig & {
* The list of registries and the associated pack globs that determine where each
* pack can be downloaded from.
*/
export interface SafeRegistryConfig {
export interface RegistryConfigNoCredentials {
// URL of a package registry, eg- https://ghcr.io/v2/
url: string;
@ -1721,15 +1721,15 @@ export async function initConfig(
return config;
}
function parseRegistries(registriesInput: string | undefined) {
function parseRegistries(
registriesInput: string | undefined
): RegistryConfigWithCredentials[] | undefined {
try {
return registriesInput ? yaml.l(registriesInput) : undefined;
return registriesInput
? (yaml.load(registriesInput) as RegistryConfigWithCredentials[])
: undefined;
} catch (e) {
throw new Error(
`Invalid registries input. Must be a JSON string, but got: ${
e instanceof Error ? e.message : String(e)
}`
);
throw new Error("Invalid registries input. Must be a YAML string.");
}
}
@ -1834,7 +1834,7 @@ export async function downloadPacks(
codeQL: CodeQL,
languages: Language[],
packs: Packs,
registries: RegistryConfig[] | undefined,
registries: RegistryConfigWithCredentials[] | undefined,
apiDetails: api.GitHubApiDetails,
tmpDir: string,
logger: Logger
@ -1888,7 +1888,9 @@ export async function downloadPacks(
);
}
function createRegistriesBlock(registries: RegistryConfig[]) {
function createRegistriesBlock(registries: RegistryConfigWithCredentials[]): {
registries: RegistryConfigNoCredentials[];
} {
// be sure to remove the `token` field from the registry before writing it to disk.
const safeRegistries = registries.map((registry) => ({
url: registry.url,
@ -1900,6 +1902,18 @@ function createRegistriesBlock(registries: RegistryConfig[]) {
return qlconfig;
}
/**
* Create a temporary environment based on the existing environment and overridden
* by the given environment variables that are passed in as arguments.
*
* Use this new environment in the context of the given operation. After completing
* the operation, restore the original environment.
*
* This function does not support un-setting environment variables.
*
* @param env
* @param operation
*/
async function wrapEnvironment(
env: Record<string, string | undefined>,
operation: Function