[7.0] [dev/build] fix missing flag checks (#35187) (#35193)

* [dev/build] fix missing flag checks (#35187)

* [dev/build] fix missing flag checks

With the upgrade to the new version of getopts, boolean flags are now defaulting to `false`, which prevents the `undefined` checks in place for the `--rpm`, `--deb`, `--docker` flags. Or order to ensure that they can be checked for existence we can default the flags to `null`, which gives these "boolean" flags a third possible state, only possible when the flags are not specified from the command line.

* [dev/build] break out args processing and test

* reindent help text

* always return unknownFlags

* fix lint error

packages/kbn-dev-utils/src/tooling_log/log_levels.d.ts

@@ -24,6 +24,9 @@ export interface ParsedLogLevel {
   flags: { [key in LogLevel]: boolean };
 }
 
-export function pickLevelFromFlags(flags: { [key: string]: any }): LogLevel;
+export function pickLevelFromFlags(
+  flags: { [key: string]: any },
+  options?: { default?: LogLevel }
+): LogLevel;
 
 export function parseLogLevel(level: LogLevel): ParsedLogLevel;

src/dev/build/args.test.ts

@@ -0,0 +1,148 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { ToolingLog } from '@kbn/dev-utils';
+
+import { readCliArgs } from './args';
+
+const fn = (...subArgs: string[]) => {
+  const result = readCliArgs(['node', 'scripts/build', ...subArgs]);
+  (result as any).log = result.log instanceof ToolingLog ? '<ToolingLog>' : String(result.log);
+  return result;
+};
+
+it('renders help if `--help` passed', () => {
+  expect(fn('--help')).toMatchInlineSnapshot(`
+Object {
+  "log": "undefined",
+  "showHelp": true,
+  "unknownFlags": Array [],
+}
+`);
+});
+
+it('build default and oss dist for current platform, without packages, by default', () => {
+  expect(fn()).toMatchInlineSnapshot(`
+Object {
+  "buildArgs": Object {
+    "buildDefaultDist": true,
+    "buildOssDist": true,
+    "createArchives": true,
+    "createDebPackage": false,
+    "createDockerPackage": false,
+    "createRpmPackage": false,
+    "downloadFreshNode": true,
+    "isRelease": false,
+    "targetAllPlatforms": false,
+    "versionQualifier": "",
+  },
+  "log": "<ToolingLog>",
+  "showHelp": false,
+  "unknownFlags": Array [],
+}
+`);
+});
+
+it('builds packages if --all-platforms is passed', () => {
+  expect(fn('--all-platforms')).toMatchInlineSnapshot(`
+Object {
+  "buildArgs": Object {
+    "buildDefaultDist": true,
+    "buildOssDist": true,
+    "createArchives": true,
+    "createDebPackage": true,
+    "createDockerPackage": true,
+    "createRpmPackage": true,
+    "downloadFreshNode": true,
+    "isRelease": false,
+    "targetAllPlatforms": true,
+    "versionQualifier": "",
+  },
+  "log": "<ToolingLog>",
+  "showHelp": false,
+  "unknownFlags": Array [],
+}
+`);
+});
+
+it('limits packages if --rpm passed with --all-platforms', () => {
+  expect(fn('--all-platforms', '--rpm')).toMatchInlineSnapshot(`
+Object {
+  "buildArgs": Object {
+    "buildDefaultDist": true,
+    "buildOssDist": true,
+    "createArchives": true,
+    "createDebPackage": false,
+    "createDockerPackage": false,
+    "createRpmPackage": true,
+    "downloadFreshNode": true,
+    "isRelease": false,
+    "targetAllPlatforms": true,
+    "versionQualifier": "",
+  },
+  "log": "<ToolingLog>",
+  "showHelp": false,
+  "unknownFlags": Array [],
+}
+`);
+});
+
+it('limits packages if --deb passed with --all-platforms', () => {
+  expect(fn('--all-platforms', '--deb')).toMatchInlineSnapshot(`
+Object {
+  "buildArgs": Object {
+    "buildDefaultDist": true,
+    "buildOssDist": true,
+    "createArchives": true,
+    "createDebPackage": true,
+    "createDockerPackage": false,
+    "createRpmPackage": false,
+    "downloadFreshNode": true,
+    "isRelease": false,
+    "targetAllPlatforms": true,
+    "versionQualifier": "",
+  },
+  "log": "<ToolingLog>",
+  "showHelp": false,
+  "unknownFlags": Array [],
+}
+`);
+});
+
+it('limits packages if --docker passed with --all-platforms', () => {
+  expect(fn('--all-platforms', '--docker')).toMatchInlineSnapshot(`
+Object {
+  "buildArgs": Object {
+    "buildDefaultDist": true,
+    "buildOssDist": true,
+    "createArchives": true,
+    "createDebPackage": false,
+    "createDockerPackage": true,
+    "createRpmPackage": false,
+    "downloadFreshNode": true,
+    "isRelease": false,
+    "targetAllPlatforms": true,
+    "versionQualifier": "",
+  },
+  "log": "<ToolingLog>",
+  "showHelp": false,
+  "unknownFlags": Array [],
+}
+`);
+});

src/dev/build/args.ts

@@ -0,0 +1,122 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { pickLevelFromFlags, ToolingLog } from '@kbn/dev-utils';
+import getopts from 'getopts';
+
+interface ParsedArgs {
+  showHelp: boolean;
+  unknownFlags: string[];
+  log?: ToolingLog;
+  buildArgs?: {
+    [key: string]: any;
+  };
+}
+
+export function readCliArgs(argv: string[]): ParsedArgs {
+  const unknownFlags: string[] = [];
+  const flags = getopts(argv, {
+    boolean: [
+      'oss',
+      'no-oss',
+      'skip-archives',
+      'skip-os-packages',
+      'rpm',
+      'deb',
+      'docker',
+      'release',
+      'skip-node-download',
+      'verbose',
+      'debug',
+      'all-platforms',
+      'verbose',
+      'quiet',
+      'silent',
+      'debug',
+      'help',
+    ],
+    alias: {
+      v: 'verbose',
+      d: 'debug',
+    },
+    default: {
+      debug: true,
+      rpm: null,
+      deb: null,
+      docker: null,
+      oss: null,
+      'version-qualifier': '',
+    },
+    unknown: flag => {
+      unknownFlags.push(flag);
+      return false;
+    },
+  });
+
+  if (unknownFlags.length || flags.help) {
+    return {
+      showHelp: true,
+      unknownFlags,
+    };
+  }
+
+  // In order to build a docker image we always need
+  // to generate all the platforms
+  if (flags.docker) {
+    flags['all-platforms'] = true;
+  }
+
+  const log = new ToolingLog({
+    level: pickLevelFromFlags(flags, {
+      default: flags.debug === false ? 'info' : 'debug',
+    }),
+    writeTo: process.stdout,
+  });
+
+  function isOsPackageDesired(name: string) {
+    if (flags['skip-os-packages'] || !flags['all-platforms']) {
+      return false;
+    }
+
+    // build all if no flags specified
+    if (flags.rpm === null && flags.deb === null && flags.docker === null) {
+      return true;
+    }
+
+    return Boolean(flags[name]);
+  }
+
+  return {
+    showHelp: false,
+    unknownFlags: [],
+    log,
+    buildArgs: {
+      isRelease: Boolean(flags.release),
+      versionQualifier: flags['version-qualifier'],
+      buildOssDist: flags.oss !== false,
+      buildDefaultDist: !flags.oss,
+      downloadFreshNode: !Boolean(flags['skip-node-download']),
+      createArchives: !Boolean(flags['skip-archives']),
+      createRpmPackage: isOsPackageDesired('rpm'),
+      createDebPackage: isOsPackageDesired('deb'),
+      createDockerPackage: isOsPackageDesired('docker'),
+      targetAllPlatforms: Boolean(flags['all-platforms']),
+    },
+  };
+}

src/dev/build/cli.js

@@ -19,53 +19,24 @@
 
 import { resolve } from 'path';
 
-import getopts from 'getopts';
 import dedent from 'dedent';
 import chalk from 'chalk';
 
-import { ToolingLog, pickLevelFromFlags } from '@kbn/dev-utils';
 import { buildDistributables } from './build_distributables';
 import { isErrorLogged } from './lib';
+import { readCliArgs } from './args';
 
 // ensure the cwd() is always the repo root
 process.chdir(resolve(__dirname, '../../../'));
 
-const unknownFlags = [];
-const flags = getopts(process.argv.slice(0), {
-  boolean: [
-    'oss',
-    'no-oss',
-    'skip-archives',
-    'skip-os-packages',
-    'rpm',
-    'deb',
-    'docker',
-    'release',
-    'skip-node-download',
-    'verbose',
-    'debug',
-    'all-platforms'
-  ],
-  alias: {
-    v: 'verbose',
-    d: 'debug',
-  },
-  default: {
-    debug: true,
-    'version-qualifier': ''
-  },
-  unknown: (flag) => {
-    unknownFlags.push(flag);
-  }
-});
+const { showHelp, unknownFlags, log, buildArgs } = readCliArgs(process.argv);
 
-if (unknownFlags.length && !flags.help) {
+if (unknownFlags.length) {
   const pluralized = unknownFlags.length > 1 ? 'flags' : 'flag';
   console.log(chalk`\n{red Unknown ${pluralized}: ${unknownFlags.join(', ')}}\n`);
-  flags.help = true;
 }
 
-if (flags.help) {
+if (showHelp) {
   console.log(
     dedent(chalk`
       {dim usage:} node scripts/build
@@ -91,45 +62,7 @@ if (flags.help) {
   process.exit(1);
 }
 
-// In order to build a docker image we always need
-// to generate all the platforms
-if (flags.docker) {
-  flags['all-platforms'] = true;
-}
-
-const log = new ToolingLog({
-  level: pickLevelFromFlags(flags, {
-    default: flags.debug === false ? 'info' : 'debug'
-  }),
-  writeTo: process.stdout
-});
-
-function isOsPackageDesired(name) {
-  if (flags['skip-os-packages'] || !flags['all-platforms']) {
-    return false;
-  }
-
-  // build all if no flags specified
-  if (flags.rpm === undefined && flags.deb === undefined && flags.docker === undefined) {
-    return true;
-  }
-
-  return Boolean(flags[name]);
-}
-
-buildDistributables({
-  log,
-  isRelease: Boolean(flags.release),
-  versionQualifier: flags['version-qualifier'],
-  buildOssDist: flags.oss !== false,
-  buildDefaultDist: !flags.oss,
-  downloadFreshNode: !Boolean(flags['skip-node-download']),
-  createArchives: !Boolean(flags['skip-archives']),
-  createRpmPackage: isOsPackageDesired('rpm'),
-  createDebPackage: isOsPackageDesired('deb'),
-  createDockerPackage: isOsPackageDesired('docker'),
-  targetAllPlatforms: Boolean(flags['all-platforms']),
-}).catch(error => {
+buildDistributables({ log, ...buildArgs }).catch(error => {
   if (!isErrorLogged(error)) {
     log.error('Uncaught error');
     log.error(error);