Merge pull request #1009 from github/aeisenberg/remote-nested-queries

Remote queries: Handle nested queries

Author: rneatherway

extensions/ql-vscode/src/run-remote-query.ts

@@ -11,13 +11,19 @@ import { getRemoteControllerRepo, getRemoteRepositoryLists, setRemoteControllerR
 import { tmpDir } from './run-queries';
 import { ProgressCallback, UserCancellationException } from './commandRunner';
 import { OctokitResponse } from '@octokit/types/dist-types';
-
 interface Config {
   repositories: string[];
   ref?: string;
   language?: string;
 }
 
+export interface QlPack {
+  name: string;
+  version: string;
+  dependencies: { [key: string]: string };
+  defaultSuite?: Record<string, unknown>[];
+  defaultSuiteFile?: string;
+}
 interface RepoListQuickPickItem extends QuickPickItem {
   repoList: string[];
 }
@@ -33,6 +39,11 @@ interface QueriesResponse {
  */
 const REPO_REGEX = /^(?:[a-zA-Z0-9]+-)*[a-zA-Z0-9]+\/[a-zA-Z0-9-_]+$/;
 
+/**
+ * Well-known names for the query pack used by the server.
+ */
+const QUERY_PACK_NAME = 'codeql-remote/query';
+
 /**
  * Gets the repositories to run the query against.
  */
@@ -89,13 +100,9 @@ async function generateQueryPack(cliServer: cli.CodeQLCliServer, queryFile: stri
   base64Pack: string,
   language: string
 }> {
-  const originalPackRoot = path.dirname(queryFile);
-  // TODO this assumes that the qlpack.yml is in the same directory as the query file, but in reality,
-  // the file could be in a parent directory.
-  const targetQueryFileName = path.join(queryPackDir, path.basename(queryFile));
-
-  // the server is expecting the query file to be named `query.ql`. Rename it here.
-  const renamedQueryFile = path.join(queryPackDir, 'query.ql');
+  const originalPackRoot = await findPackRoot(queryFile);
+  const packRelativePath = path.relative(originalPackRoot, queryFile);
+  const targetQueryFileName = path.join(queryPackDir, packRelativePath);
 
   let language: string | undefined;
   if (await fs.pathExists(path.join(originalPackRoot, 'qlpack.yml'))) {
@@ -125,9 +132,6 @@ async function generateQueryPack(cliServer: cli.CodeQLCliServer, queryFile: stri
         })
     });
 
-    // ensure the qlpack.yml has a valid name
-    await ensureQueryPackName(queryPackDir);
-
     void logger.log(`Copied ${copiedCount} files to ${queryPackDir}`);
 
     language = await findLanguage(cliServer, Uri.file(targetQueryFileName));
@@ -138,13 +142,11 @@ async function generateQueryPack(cliServer: cli.CodeQLCliServer, queryFile: stri
 
     // copy only the query file to the query pack directory
     // and generate a synthetic query pack
-    // TODO this has a limitation that query packs inside of a workspace will not resolve its peer dependencies.
-    // Something to work on later. For now, we will only support query packs that are not in a workspace.
     void logger.log(`Copying ${queryFile} to ${queryPackDir}`);
     await fs.copy(queryFile, targetQueryFileName);
     void logger.log('Generating synthetic query pack');
     const syntheticQueryPack = {
-      name: 'codeql-remote/query',
+      name: QUERY_PACK_NAME,
       version: '0.0.0',
       dependencies: {
         [`codeql/${language}-all`]: '*',
@@ -156,7 +158,7 @@ async function generateQueryPack(cliServer: cli.CodeQLCliServer, queryFile: stri
     throw new UserCancellationException('Could not determine language.');
   }
 
-  await fs.rename(targetQueryFileName, renamedQueryFile);
+  await ensureNameAndSuite(queryPackDir, packRelativePath);
 
   const bundlePath = await getPackedBundlePath(queryPackDir);
   void logger.log(`Compiling and bundling query pack from ${queryPackDir} to ${bundlePath}. (This may take a while.)`);
@@ -170,23 +172,24 @@ async function generateQueryPack(cliServer: cli.CodeQLCliServer, queryFile: stri
   };
 }
 
-/**
- * Ensure that the qlpack.yml has a valid name. For local purposes,
- * Anonymous packs and names that are not prefixed by a scope (ie `<foo>/`)
- * are sufficient. But in order to create a pack, the name must be prefixed.
- *
- * @param queryPackDir the directory containing the query pack.
- */
-async function ensureQueryPackName(queryPackDir: string) {
-  const pack = yaml.safeLoad(await fs.readFile(path.join(queryPackDir, 'qlpack.yml'), 'utf8')) as { name: string; };
-  if (!pack.name || !pack.name.includes('/')) {
-    if (!pack.name) {
-      pack.name = 'codeql-remote/query';
-    } else if (!pack.name.includes('/')) {
-      pack.name = `codeql-remote/${pack.name}`;
+async function findPackRoot(queryFile: string): Promise<string> {
+  // recursively find the directory containing qlpack.yml
+  let dir = path.dirname(queryFile);
+  while (!(await fs.pathExists(path.join(dir, 'qlpack.yml')))) {
+    dir = path.dirname(dir);
+    if (isFileSystemRoot(dir)) {
+      // there is no qlpack.yml in this direcory or any parent directory.
+      // just use the query file's directory as the pack root.
+      return path.dirname(queryFile);
     }
-    await fs.writeFile(path.join(queryPackDir, 'qlpack.yml'), yaml.safeDump(pack));
   }
+
+  return dir;
+}
+
+function isFileSystemRoot(dir: string): boolean {
+  const pathObj = path.parse(dir);
+  return pathObj.root === dir && pathObj.base === '';
 }
 
 async function createRemoteQueriesTempDirectory() {
@@ -413,3 +416,27 @@ export async function attemptRerun(
     void showAndLogErrorMessage(error);
   }
 }
+
+/**
+ * Updates the default suite of the query pack. This is used to ensure
+ * only the specified query is run.
+ *
+ * Also, ensure the query pack name is set to the name expected by the server.
+ *
+ * @param queryPackDir The directory containing the query pack
+ * @param packRelativePath The relative path to the query pack from the root of the query pack
+ */
+async function ensureNameAndSuite(queryPackDir: string, packRelativePath: string): Promise<void> {
+  const packPath = path.join(queryPackDir, 'qlpack.yml');
+  const qlpack = yaml.safeLoad(await fs.readFile(packPath, 'utf8')) as QlPack;
+  delete qlpack.defaultSuiteFile;
+
+  qlpack.name = QUERY_PACK_NAME;
+
+  qlpack.defaultSuite = [{
+    description: 'Query suite for remote query'
+  }, {
+    query: packRelativePath.replace(/\\/g, '/')
+  }];
+  await fs.writeFile(packPath, yaml.safeDump(qlpack));
+}

extensions/ql-vscode/src/vscode-tests/cli-integration/data-remote-qlpack-nested/not-in-pack.ql

@@ -0,0 +1,2 @@
+// This file should not be included the remote query pack.
+select 1

extensions/ql-vscode/src/vscode-tests/cli-integration/data-remote-qlpack-nested/otherfolder/lib.qll

@@ -0,0 +1,3 @@
+int number() {
+  result = 1
+}

extensions/ql-vscode/src/vscode-tests/cli-integration/data-remote-qlpack-nested/qlpack.yml

@@ -0,0 +1,4 @@
+name: github/remote-query-pack
+version: 0.0.0
+dependencies:
+  codeql/javascript-all: '*'

extensions/ql-vscode/src/vscode-tests/cli-integration/data-remote-qlpack-nested/subfolder/in-pack.ql

@@ -0,0 +1,4 @@
+import javascript
+import otherfolder.lib
+
+select number()

extensions/ql-vscode/src/vscode-tests/cli-integration/data-remote-qlpack/qlpack.yml

@@ -1,5 +1,4 @@
 name: github/remote-query-pack
 version: 0.0.0
-extractor: javascript
 dependencies:
   codeql/javascript-all: '*'

extensions/ql-vscode/src/vscode-tests/cli-integration/run-remote-query.test.ts

@@ -4,14 +4,16 @@ import * as sinon from 'sinon';
 import { CancellationToken, extensions, QuickPickItem, Uri, window } from 'vscode';
 import 'mocha';
 import * as fs from 'fs-extra';
+import * as os from 'os';
 import * as yaml from 'js-yaml';
 
-import { runRemoteQuery } from '../../run-remote-query';
+import { QlPack, runRemoteQuery } from '../../run-remote-query';
 import { Credentials } from '../../authentication';
 import { CliVersionConstraint, CodeQLCliServer } from '../../cli';
 import { CodeQLExtensionInterface } from '../../extension';
 import { setRemoteControllerRepo, setRemoteRepositoryLists } from '../../config';
 import { UserCancellationException } from '../../commandRunner';
+import { lte } from 'semver';
 
 describe('Remote queries', function() {
   const baseDir = path.join(__dirname, '../../../src/vscode-tests/cli-integration');
@@ -80,8 +82,7 @@ describe('Remote queries', function() {
     const queryPackDir = path.join(queryPackRootDir, 'query-pack');
     printDirectoryContents(queryPackDir);
 
-    // in-pack.ql renamed to query.ql
-    expect(fs.existsSync(path.join(queryPackDir, 'query.ql'))).to.be.true;
+    expect(fs.existsSync(path.join(queryPackDir, 'in-pack.ql'))).to.be.true;
     expect(fs.existsSync(path.join(queryPackDir, 'lib.qll'))).to.be.true;
     expect(fs.existsSync(path.join(queryPackDir, 'qlpack.yml'))).to.be.true;
 
@@ -96,7 +97,7 @@ describe('Remote queries', function() {
     const compiledPackDir = path.join(queryPackDir, '.codeql/pack/github/remote-query-pack/0.0.0/');
     printDirectoryContents(compiledPackDir);
 
-    expect(fs.existsSync(path.join(compiledPackDir, 'query.ql'))).to.be.true;
+    expect(fs.existsSync(path.join(compiledPackDir, 'in-pack.ql'))).to.be.true;
     expect(fs.existsSync(path.join(compiledPackDir, 'lib.qll'))).to.be.true;
     expect(fs.existsSync(path.join(compiledPackDir, 'qlpack.yml'))).to.be.true;
     // depending on the cli version, we should have one of these files
@@ -105,6 +106,7 @@ describe('Remote queries', function() {
       fs.existsSync(path.join(compiledPackDir, 'codeql-pack.lock.yml'))
     ).to.be.true;
     expect(fs.existsSync(path.join(compiledPackDir, 'not-in-pack.ql'))).to.be.false;
+    verifyQlPack(path.join(compiledPackDir, 'qlpack.yml'), 'in-pack.ql', '0.0.0', await pathSerializationBroken());
 
     // dependencies
     const libraryDir = path.join(compiledPackDir, '.codeql/libraries/codeql');
@@ -129,8 +131,8 @@ describe('Remote queries', function() {
 
     const queryPackDir = path.join(queryPackRootDir, 'query-pack');
     printDirectoryContents(queryPackDir);
-    // in-pack.ql renamed to query.ql
-    expect(fs.existsSync(path.join(queryPackDir, 'query.ql'))).to.be.true;
+
+    expect(fs.existsSync(path.join(queryPackDir, 'in-pack.ql'))).to.be.true;
     expect(fs.existsSync(path.join(queryPackDir, 'qlpack.yml'))).to.be.true;
     // depending on the cli version, we should have one of these files
     expect(
@@ -143,8 +145,10 @@ describe('Remote queries', function() {
     // the compiled pack
     const compiledPackDir = path.join(queryPackDir, '.codeql/pack/codeql-remote/query/0.0.0/');
     printDirectoryContents(compiledPackDir);
-    expect(fs.existsSync(path.join(compiledPackDir, 'query.ql'))).to.be.true;
+    expect(fs.existsSync(path.join(compiledPackDir, 'in-pack.ql'))).to.be.true;
     expect(fs.existsSync(path.join(compiledPackDir, 'qlpack.yml'))).to.be.true;
+    verifyQlPack(path.join(compiledPackDir, 'qlpack.yml'), 'in-pack.ql', '0.0.0', await pathSerializationBroken());
+
     // depending on the cli version, we should have one of these files
     expect(
       fs.existsSync(path.join(compiledPackDir, 'qlpack.lock.yml')) ||
@@ -165,6 +169,60 @@ describe('Remote queries', function() {
     expect(packNames).to.deep.equal(['javascript-all', 'javascript-upgrades']);
   });
 
+  it('should run a remote query that is nested inside a qlpack', async () => {
+    const fileUri = getFile('data-remote-qlpack-nested/subfolder/in-pack.ql');
+
+    const queryPackRootDir = (await runRemoteQuery(cli, credentials, fileUri, true, progress, token))!;
+
+    // to retrieve the list of repositories
+    expect(showQuickPickSpy).to.have.been.calledOnce;
+
+    // check a few files that we know should exist and others that we know should not
+
+    // the tarball to deliver to the server
+    printDirectoryContents(queryPackRootDir);
+    expect(fs.readdirSync(queryPackRootDir).find(f => f.startsWith('qlpack-') && f.endsWith('-generated.tgz'))).not.to.be.undefined;
+
+    const queryPackDir = path.join(queryPackRootDir, 'query-pack');
+    printDirectoryContents(queryPackDir);
+
+    expect(fs.existsSync(path.join(queryPackDir, 'subfolder/in-pack.ql'))).to.be.true;
+    expect(fs.existsSync(path.join(queryPackDir, 'qlpack.yml'))).to.be.true;
+    // depending on the cli version, we should have one of these files
+    expect(
+      fs.existsSync(path.join(queryPackDir, 'qlpack.lock.yml')) ||
+      fs.existsSync(path.join(queryPackDir, 'codeql-pack.lock.yml'))
+    ).to.be.true;
+    expect(fs.existsSync(path.join(queryPackDir, 'otherfolder/lib.qll'))).to.be.true;
+    expect(fs.existsSync(path.join(queryPackDir, 'not-in-pack.ql'))).to.be.false;
+
+    // the compiled pack
+    const compiledPackDir = path.join(queryPackDir, '.codeql/pack/github/remote-query-pack/0.0.0/');
+    printDirectoryContents(compiledPackDir);
+    expect(fs.existsSync(path.join(compiledPackDir, 'otherfolder/lib.qll'))).to.be.true;
+    expect(fs.existsSync(path.join(compiledPackDir, 'subfolder/in-pack.ql'))).to.be.true;
+    expect(fs.existsSync(path.join(compiledPackDir, 'qlpack.yml'))).to.be.true;
+    verifyQlPack(path.join(compiledPackDir, 'qlpack.yml'), 'subfolder/in-pack.ql', '0.0.0', await pathSerializationBroken());
+
+    // depending on the cli version, we should have one of these files
+    expect(
+      fs.existsSync(path.join(compiledPackDir, 'qlpack.lock.yml')) ||
+      fs.existsSync(path.join(compiledPackDir, 'codeql-pack.lock.yml'))
+    ).to.be.true;
+    expect(fs.existsSync(path.join(compiledPackDir, 'not-in-pack.ql'))).to.be.false;
+    // should have generated a correct qlpack file
+    const qlpackContents: any = yaml.safeLoad(fs.readFileSync(path.join(compiledPackDir, 'qlpack.yml'), 'utf8'));
+    expect(qlpackContents.name).to.equal('codeql-remote/query');
+    expect(qlpackContents.version).to.equal('0.0.0');
+    expect(qlpackContents.dependencies?.['codeql/javascript-all']).to.equal('*');
+
+    // dependencies
+    const libraryDir = path.join(compiledPackDir, '.codeql/libraries/codeql');
+    printDirectoryContents(libraryDir);
+    const packNames = fs.readdirSync(libraryDir).sort();
+    expect(packNames).to.deep.equal(['javascript-all', 'javascript-upgrades']);
+  });
+
   it('should cancel a run before uploading', async () => {
     const fileUri = getFile('data-remote-no-qlpack/in-pack.ql');
 
@@ -180,6 +238,41 @@ describe('Remote queries', function() {
     }
   });
 
+  function verifyQlPack(qlpackPath: string, queryPath: string, packVersion: string, pathSerializationBroken: boolean) {
+    const qlPack = yaml.safeLoad(fs.readFileSync(qlpackPath, 'utf8')) as QlPack;
+
+    if (pathSerializationBroken) {
+      // the path serialization is broken, so we force it to be the path in the pack to be same as the query path
+      qlPack.defaultSuite![1].query = queryPath;
+    }
+
+    // don't check the build metadata since it is variable
+    delete (qlPack as any).buildMetadata;
+
+    expect(qlPack).to.deep.equal({
+      name: 'codeql-remote/query',
+      version: packVersion,
+      dependencies: {
+        'codeql/javascript-all': '*',
+      },
+      library: false,
+      defaultSuite: [{
+        description: 'Query suite for remote query'
+      }, {
+        query: queryPath
+      }]
+    });
+  }
+
+  /**
+   * In version 2.7.2 and earlier, relative paths were not serialized correctly inside the qlpack.yml file.
+   * So, ignore part of the test for these versions.
+   *
+   * @returns true if path serialization is broken in this run
+   */
+  async function pathSerializationBroken() {
+    return lte((await cli.getVersion()), '2.7.2') && os.platform() === 'win32';
+  }
   function getFile(file: string): Uri {
     return Uri.file(path.join(baseDir, file));
   }