Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Allow "additive" queries in workflow by prefixing with "+"
Sam Partington committed Aug 28, 2020
1 parent 8229390 commit 82000c2
Showing 7 changed files with 159 additions and 22 deletions.
2 changes: 1 addition & 1 deletion init/action.yml
@@ -17,7 +17,7 @@ inputs:
description: Path of the config file to use
required: false
queries:
description: Comma-separated list of additional queries to run. By default, this overrides the same setting in a configuration file
description: Comma-separated list of additional queries to run. By default, this overrides the same setting in a configuration file; prefix with "+" to use both sets of queries.
required: false
runs:
using: 'node12'
27 changes: 18 additions & 9 deletions lib/config-utils.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/config-utils.js.map

Large diffs are not rendered by default.

54 changes: 54 additions & 0 deletions lib/config-utils.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/config-utils.test.js.map

Large diffs are not rendered by default.

64 changes: 64 additions & 0 deletions src/config-utils.test.ts
@@ -431,6 +431,70 @@ test("Multiple queries can be specified in workflow file, no config file require
});
});

test("Queries in workflow file can be added to the set of queries without overriding config file", async t => {
return await util.withTmpDir(async tmpDir => {
process.env['RUNNER_TEMP'] = tmpDir;
process.env['GITHUB_WORKSPACE'] = tmpDir;

const inputFileContents = `
name: my config
queries:
- uses: ./foo`;

fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
setInput('config-file', 'input');

// These queries shouldn't override anything, because the value is prefixed with "+"
setInput('queries', '+./additional1,./additional2');

fs.mkdirSync(path.join(tmpDir, 'foo'));
fs.mkdirSync(path.join(tmpDir, 'additional1'));
fs.mkdirSync(path.join(tmpDir, 'additional2'));

const resolveQueriesArgs: {queries: string[], extraSearchPath: string | undefined}[] = [];
const codeQL = setCodeQL({
resolveQueries: async function(queries: string[], extraSearchPath: string | undefined) {
resolveQueriesArgs.push({queries, extraSearchPath});
// Return what we're given, just in the right format for a resolved query
// This way we can test by seeing which returned items are in
// the final configuration.
const dummyResolvedQueries = {};
queries.forEach(q => { dummyResolvedQueries[q] = {}; });
return {
byLanguage: {
'javascript': dummyResolvedQueries,
},
noDeclaredLanguage: {},
multipleDeclaredLanguages: {},
};
},
});

setInput('languages', 'javascript');

const config = await configUtils.initConfig(tmpDir, tmpDir, codeQL);

// Check resolveQueries was called correctly
// It'll be called once for the default queries,
// once for each of additional1 and additional2,
// and once for './foo' from the config file
t.deepEqual(resolveQueriesArgs.length, 4);
t.deepEqual(resolveQueriesArgs[1].queries.length, 1);
t.regex(resolveQueriesArgs[1].queries[0], /.*\/additional1$/);
t.deepEqual(resolveQueriesArgs[2].queries.length, 1);
t.regex(resolveQueriesArgs[2].queries[0], /.*\/additional2$/);
t.deepEqual(resolveQueriesArgs[3].queries.length, 1);
t.regex(resolveQueriesArgs[3].queries[0], /.*\/foo$/);

// Now check that the end result contains all the queries
t.deepEqual(config.queries['javascript'].length, 4);
t.regex(config.queries['javascript'][0], /javascript-code-scanning.qls$/);
t.regex(config.queries['javascript'][1], /.*\/additional1$/);
t.regex(config.queries['javascript'][2], /.*\/additional2$/);
t.regex(config.queries['javascript'][3], /.*\/foo$/);
});
});

test("Invalid queries in workflow file handled correctly", async t => {
return await util.withTmpDir(async tmpDir => {
process.env['RUNNER_TEMP'] = tmpDir;
30 changes: 20 additions & 10 deletions src/config-utils.ts
@@ -525,25 +525,33 @@ async function getLanguages(): Promise<Language[]> {
return parsedLanguages;
}

/**
* Returns true if queries were provided in the workflow file
* (and thus added), otherwise false
*/
async function addQueriesFromWorkflowIfRequired(
codeQL: CodeQL,
languages: string[],
resultMap: { [language: string]: string[] },
tempDir: string
): Promise<boolean> {
const queryUses = core.getInput('queries');
) {
let queryUses = core.getInput('queries');
if (queryUses) {
queryUses = queryUses.trim();
queryUses = queryUses.replace(/^\+/, ''); // "+" means "don't override config file" - see shouldAddConfigFileQueries
for (const query of queryUses.split(',')) {
await parseQueryUses(languages, codeQL, resultMap, query, tempDir);
}
return true;
}
}

// Returns true if either no queries were provided in the workflow.
// or if the queries in the workflow were provided in "additive" mode,
// indicating that they shouldn't override the config queries but
// should instead be added in addition
function shouldAddConfigFileQueries(): boolean {
const queryUses = core.getInput('queries');
if (queryUses) {
return queryUses.trimStart().substr(0, 1) === '+';
}

return false;
return true;
}

/**
@@ -613,8 +621,10 @@ async function loadConfig(configFile: string, tempDir: string, toolCacheDir: str

// If queries were provided using `with` in the action configuration,
// they should take precedence over the queries in the config file
const addedQueriesFromAction = await addQueriesFromWorkflowIfRequired(codeQL, languages, queries, tempDir);
if (!addedQueriesFromAction && QUERIES_PROPERTY in parsedYAML) {
// unless they're prefixed with "+", in which case they supplement those
// in the config file.
await addQueriesFromWorkflowIfRequired(codeQL, languages, queries, tempDir);
if (shouldAddConfigFileQueries() && QUERIES_PROPERTY in parsedYAML) {
if (!(parsedYAML[QUERIES_PROPERTY] instanceof Array)) {
throw new Error(getQueriesInvalid(configFile));
}

0 comments on commit 82000c2

Please sign in to comment.