diff --git a/.buildkite/scripts/steps/checks/dependencies_missing_owner.sh b/.buildkite/scripts/steps/checks/dependencies_missing_owner.sh new file mode 100755 index 0000000000000..abb6780900208 --- /dev/null +++ b/.buildkite/scripts/steps/checks/dependencies_missing_owner.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash + +set -euo pipefail + +source .buildkite/scripts/common/util.sh + +echo --- Check for NPM dependencies missing owners +node scripts/dependency_ownership.js --missingOwner --failIfUnowned diff --git a/.buildkite/scripts/steps/checks/quick_checks.txt b/.buildkite/scripts/steps/checks/quick_checks.txt index d6b6698b78f46..f3e6009c8a9c7 100644 --- a/.buildkite/scripts/steps/checks/quick_checks.txt +++ b/.buildkite/scripts/steps/checks/quick_checks.txt @@ -19,3 +19,4 @@ .buildkite/scripts/steps/checks/native_modules.sh .buildkite/scripts/steps/checks/test_files_missing_owner.sh .buildkite/scripts/steps/checks/styled_components_mapping.sh +.buildkite/scripts/steps/checks/dependencies_missing_owner.sh diff --git a/.buildkite/scripts/steps/renovate.sh b/.buildkite/scripts/steps/renovate.sh index cc4583e3da216..0ddfe95d1b8d6 100755 --- a/.buildkite/scripts/steps/renovate.sh +++ b/.buildkite/scripts/steps/renovate.sh @@ -4,3 +4,4 @@ set -euo pipefail echo '--- Renovate: validation' .buildkite/scripts/steps/checks/renovate.sh +.buildkite/scripts/steps/checks/dependencies_missing_owner.sh diff --git a/dev_docs/contributing/third_party_dependencies.mdx b/dev_docs/contributing/third_party_dependencies.mdx index ea8eb9cd154a9..fbb85f929193c 100644 --- a/dev_docs/contributing/third_party_dependencies.mdx +++ b/dev_docs/contributing/third_party_dependencies.mdx @@ -41,19 +41,18 @@ Should you find yourself evaluating a new dependency, here are some specific thi 1. **Is there already another dependency that offers similar functionality?** If so, adding a new dependency may not be necessary. Prefer one way to do things and use what's already there, unless there is an important reason not to do so. 2. **Does this dependency appear to be well-maintained?** A dependency that hasn't been updated in years is usually more of a -liability than an asset. Make sure the depedency has recent activity, that bugs and security vulnerabilities appear to be addressed +liability than an asset. Make sure the dependency has recent activity, that bugs and security vulnerabilities appear to be addressed in a timely manner, and that there is active participation from the maintainers and community. 3. **How large is the dependency?** For client-side plugins, heavy dependencies can have a real impact on user experience, especially if they are included in the initial page bundle and not loaded asynchronously. In some cases it might make more sense -to roll your own rather than include a bloated depedency, especially if you are only using a single piece of functionality. +to roll your own rather than include a bloated dependency, especially if you are only using a single piece of functionality. 4. **Does this dependency have a license that's compatible with Kibana's?** Most common open source licenses such as BSD, MIT, and Apache 2.0/1.1 are okay to use with Kibana. Others may not be, or may require special attribution. 5. **Will this dependency need to be prebuilt?** Due to our build process, native module dependencies are only supported for development (`devDependencies`), and are not supported for production (`dependencies`). 6. **Am I committed to maintaining this dependency?** Once you add a dependency to the `package.json`, someone else isn't going to keep it updated for you. That means you will be responsible for updating it regularly, keeping an eye out for security vulnerabilities, -and dealing with any breaking changes that may arise during an upgrade. We recommend (and will soon require) relying on the renovate bot to help keep the -dependency updated; be sure to mark your ownership of the package in the -[`renovate.json`](https://github.com/elastic/kibana/blob/main/renovate.json`) file. +and dealing with any breaking changes that may arise during an upgrade. Dependency ownership is tracked by the +[`renovate.json`](https://github.com/elastic/kibana/blob/main/renovate.json`) file. See the section on Dependency ownership below for more information. If you have any questions about whether adding a dependency is appropriate, feel free to reach out to one of the following teams on Github: @@ -72,3 +71,73 @@ on Github: Using an existing dependency is typically preferred over adding a new one. Please consult with the owning team before using an existing dependency, as they may have specific guidelines or concerns about its use. + +## Dependency ownership + +All dependencies must be owned by at least one team. This team is responsible for ensuring the dependency is kept up to date, and for addressing any issues that arise with the dependency. +Dependency ownership is tracked in the `renovate.json` file in the root of the Kibana repository. If you are adding a new dependency, be sure to add your team as the owner in this file. + +### Example configuration +Here is an example configuration for a dependency in the `renovate.json` file: + +```json + { + //[1] + "groupName": "my-awesome-dependency", + "matchDepNames": [ + "my-awesome-dependency", + "@types/my-awesome-dependency" + ], + // [2] + "reviewers": [ + "team:my-team-name" + ], + // [3] + "matchBaseBranches": [ + "main" + ], + // [4] + "labels": [ + "Team:My-Team-Label", + "release_note:skip", + "backport:all-open" + ], + // [5] + "minimumReleaseAge": "7 days", + // [6] + "enabled": true + } +``` + +[1] `groupName`: The rule group. Renovate will raise a single PR for all dependencies within a group. Consider creating logical groups to make upgrades easier to review. + +[2] `reviewers`: `team:my-team-name` will correspond to a GitHub group named `@elastic/my-team-name`. This group should contain all members of the team responsible for the dependency. Multiple teams can be added as reviewers if necessary. + +[3] `matchBaseBranches`: The branches that the rule will apply to. This should be set to `main` for most dependencies. + +[4] `labels`: Labels to apply to the PRs created by Renovate. The `Team:My-Team-Label` label should be replaced with your team's GitHub label from the Kibana repository. The `release_note:skip` and `backport:all-open` labels are used to control the release process and should not be changed without first consulting the AppEx Platform Security team. + +[5] `minimumReleaseAge`: The minimum age of a release before it can be upgraded. This is set to `7 days` to allow time for any issues to be identified and resolved before upgrading. You may adjust this value as needed. + +[6] `enabled`: Must be set to `true` to satisfy dependency ownership requirements. Consult the AppEx Platform Security team before disabling this setting. + +### Dependency ownership tooling + +The `./scripts/dependency_ownership.js` script can be used to validate the `renovate.json` file and ensure that all dependencies are owned by a team. +```sh +node scripts/dependency_ownership.js + +Runs a dev task + +Options: + --dependency, -d Show who owns the given dependency + --owner, -o Show dependencies owned by the given owner + --missingOwner Show dependencies that are not owned by any team + --outputPath, -f Specify the output file to save results as JSON + --failIfUnowned Fail if any dependencies are not owned by any team + --verbose, -v Log verbosely + --debug Log debug messages (less than verbose) + --quiet Only log errors + --silent Don't log anything + --help Show this message +``` \ No newline at end of file diff --git a/package.json b/package.json index b810762d2b831..6a9f410575bbf 100644 --- a/package.json +++ b/package.json @@ -1155,7 +1155,6 @@ "json-stable-stringify": "^1.0.1", "json-stringify-pretty-compact": "1.2.0", "json-stringify-safe": "5.0.1", - "jsonpath-plus": "^10.2.0", "jsonwebtoken": "^9.0.2", "jsts": "^1.6.2", "kea": "^2.6.0", diff --git a/packages/kbn-dependency-ownership/src/cli.test.ts b/packages/kbn-dependency-ownership/src/cli.test.ts deleted file mode 100644 index f3bfc7feea34d..0000000000000 --- a/packages/kbn-dependency-ownership/src/cli.test.ts +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import { configureYargs } from './cli'; -import { identifyDependencyOwnership } from './dependency_ownership'; - -jest.mock('chalk', () => ({ - green: jest.fn((str) => str), - yellow: jest.fn((str) => str), - cyan: jest.fn((str) => str), - magenta: jest.fn((str) => str), - blue: jest.fn((str) => str), - bold: { magenta: jest.fn((str) => str), blue: jest.fn((str) => str) }, -})); - -jest.mock('./dependency_ownership', () => ({ - identifyDependencyOwnership: jest.fn(), -})); - -jest.mock('./cli', () => ({ - ...jest.requireActual('./cli'), - runCLI: jest.fn(), -})); - -describe('dependency-ownership CLI', () => { - const parser = configureYargs() - .fail((message: string) => { - throw new Error(message); - }) - .exitProcess(false); - - beforeEach(() => { - jest.spyOn(console, 'log').mockImplementation(() => {}); - }); - - afterEach(() => { - jest.resetAllMocks(); - }); - - it('should parse the dependency option correctly', () => { - const argv = parser.parse(['--dependency', 'lodash']); - expect(argv).toMatchObject({ - dependency: 'lodash', - }); - - expect(identifyDependencyOwnership).toHaveBeenCalledWith( - expect.objectContaining({ dependency: 'lodash' }) - ); - }); - - it('should parse the owner option correctly', () => { - const argv = parser.parse(['--owner', '@elastic/kibana-core']); - expect(argv).toMatchObject({ - owner: '@elastic/kibana-core', - }); - - expect(identifyDependencyOwnership).toHaveBeenCalledWith( - expect.objectContaining({ owner: '@elastic/kibana-core' }) - ); - }); - - it('should parse the missing-owner option correctly', () => { - const argv = parser.parse(['--missing-owner']); - expect(argv).toMatchObject({ - missingOwner: true, - }); - - expect(identifyDependencyOwnership).toHaveBeenCalledWith( - expect.objectContaining({ missingOwner: true }) - ); - }); - - it('should parse the output-path option correctly', () => { - const argv = parser.parse([ - '--output-path', - './output.json', - '--owner', - '@elastic/kibana-core', - ]); - - expect(argv).toMatchObject({ - owner: '@elastic/kibana-core', - outputPath: './output.json', - }); - - expect(identifyDependencyOwnership).toHaveBeenCalledWith( - expect.objectContaining({ owner: '@elastic/kibana-core' }) - ); - }); - - it('should support aliases for options', () => { - const argv1 = parser.parse(['-d', 'lodash', '-f', './out.json']); - expect(argv1).toMatchObject({ - dependency: 'lodash', - outputPath: './out.json', - }); - - const argv2 = parser.parse(['-o', '@elastic/kibana-core', '-f', './out.json']); - - expect(argv2).toMatchObject({ - owner: '@elastic/kibana-core', - outputPath: './out.json', - }); - }); - - it('should throw an error for invalid flag combinations', () => { - expect(() => { - parser.parse(['--dependency', 'lodash', '--missing-owner']); - }).toThrow('You must provide either a dependency, owner, or missingOwner flag to search for'); - - expect(identifyDependencyOwnership).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/kbn-dependency-ownership/src/cli.ts b/packages/kbn-dependency-ownership/src/cli.ts index 3023e0da3e5de..00c8d6cb7f0fd 100644 --- a/packages/kbn-dependency-ownership/src/cli.ts +++ b/packages/kbn-dependency-ownership/src/cli.ts @@ -7,109 +7,109 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +import { run } from '@kbn/dev-cli-runner'; +import { createFailError } from '@kbn/dev-cli-errors'; import nodePath from 'path'; -import yargs from 'yargs'; -import chalk from 'chalk'; import fs from 'fs'; -import { identifyDependencyOwnership } from './dependency_ownership'; +import { DependenciesByOwner, identifyDependencyOwnership } from './dependency_ownership'; interface CLIArgs { dependency?: string; owner?: string; missingOwner?: boolean; outputPath?: string; + failIfUnowned?: boolean; } -export const configureYargs = () => { - return yargs(process.argv.slice(2)) - .command( - '*', - chalk.green('Identify the dependency ownership'), - (y) => { - y.version(false) - .option('dependency', { - alias: 'd', - describe: chalk.yellow('Show who owns the given dependency'), - type: 'string', - }) - .option('owner', { - alias: 'o', - type: 'string', - describe: chalk.magenta('Show dependencies owned by the given owner'), - }) - .option('missing-owner', { - describe: chalk.cyan('Show dependencies that are not owned by any team'), - type: 'boolean', - }) - .option('output-path', { - alias: 'f', - describe: chalk.blue('Specify the output file to save results as JSON'), - type: 'string', - }) - .check(({ dependency, owner, missingOwner }: Partial) => { - const notProvided = [dependency, owner, missingOwner].filter( - (arg) => arg === undefined - ); +export async function identifyDependencyOwnershipCLI() { + await run( + async ({ log, flags }) => { + // Check if flags are valid + const { dependency, owner, missingOwner, outputPath, failIfUnowned } = flags as CLIArgs; + if (!dependency && !owner && !missingOwner) { + throw createFailError( + 'You must provide either a dependency, owner, or missingOwner flag. Use --help for more information.' + ); + } - if (notProvided.length === 1) { - throw new Error( - 'You must provide either a dependency, owner, or missingOwner flag to search for' - ); - } + if (failIfUnowned && !missingOwner) { + throw createFailError( + 'You must provide the missingOwner flag to use the failIfUnowned flag' + ); + } - return true; - }) - .example( - '--owner @elastic/kibana-core', - chalk.blue('Searches for all dependencies owned by the Kibana Core team') - ); - }, - async (argv: CLIArgs) => { - const { dependency, owner, missingOwner, outputPath } = argv; + if (owner) { + log.write(`Searching for dependencies owned by ${owner}...\n`); + } - if (owner) { - console.log(chalk.yellow(`Searching for dependencies owned by ${owner}...\n`)); - } + const result = identifyDependencyOwnership({ dependency, owner, missingOwner }); + if (failIfUnowned) { + const { prodDependencies = [] as string[], devDependencies = [] as string[] } = + result as DependenciesByOwner; - try { - const result = identifyDependencyOwnership({ dependency, owner, missingOwner }); + const uncoveredDependencies = [...prodDependencies, ...devDependencies]; + if (uncoveredDependencies.length > 0) { + log.write('Dependencies without an owner:'); + log.write(uncoveredDependencies.map((dep) => ` - ${dep}`).join('\n')); + throw createFailError( + `Found ${uncoveredDependencies.length} dependencies without an owner. Please update \`renovate.json\` to include these dependencies.\nVisit https://docs.elastic.dev/kibana-dev-docs/third-party-dependencies#dependency-ownership for more information.` + ); + } else { + log.success('All dependencies have an owner'); + } + } - if (outputPath) { - const isJsonFile = nodePath.extname(outputPath) === '.json'; - const outputFile = isJsonFile - ? outputPath - : nodePath.join(outputPath, 'dependency-ownership.json'); + if (outputPath) { + const isJsonFile = nodePath.extname(outputPath) === '.json'; + const outputFile = isJsonFile + ? outputPath + : nodePath.join(outputPath, 'dependency-ownership.json'); - const outputDir = nodePath.dirname(outputFile); + const outputDir = nodePath.dirname(outputFile); - if (!fs.existsSync(outputDir)) { - fs.mkdirSync(outputDir, { recursive: true }); - } + if (!fs.existsSync(outputDir)) { + fs.mkdirSync(outputDir, { recursive: true }); + } - fs.writeFile(outputFile, JSON.stringify(result, null, 2), (err) => { - if (err) { - console.error(chalk.red(`Failed to save results to ${outputFile}: ${err.message}`)); - } else { - console.log(chalk.green(`Results successfully saved to ${outputFile}`)); - } - }); + fs.writeFile(outputFile, JSON.stringify(result, null, 2), (err) => { + if (err) { + throw createFailError(`Failed to save results to ${outputFile}: ${err.message}`); } else { - console.log(chalk.yellow('No output file specified, displaying results below:\n')); - console.log(JSON.stringify(result, null, 2)); + log.success(`Results successfully saved to ${outputFile}`); } - } catch (error) { - console.error('Error fetching dependency ownership:', error.message); - } + }); + } else { + log.debug('No output file specified, displaying results below:'); + log.success(JSON.stringify(result, null, 2)); } - ) - .help(); -}; + }, + { + description: `A CLI tool for analyzing package ownership.`, + usage: 'node scripts/dependency_ownership --dependency ', + flags: { + string: ['dependency', 'owner', 'outputPath'], + boolean: ['missingOwner', 'failIfUnowned'], + alias: { + d: 'dependency', + o: 'owner', + f: 'outputPath', + }, + help: ` + --dependency, -d Show who owns the given dependency + --owner, -o Show dependencies owned by the given owner + --missingOwner Show dependencies that are not owned by any team + --outputPath, -f Specify the output file to save results as JSON + --failIfUnowned Fail if any dependencies are not owned by any team + `, + }, + } + ); +} export const runCLI = () => { - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - configureYargs().argv; + identifyDependencyOwnershipCLI(); }; if (!process.env.JEST_WORKER_ID) { diff --git a/packages/kbn-dependency-ownership/src/dependency_ownership.ts b/packages/kbn-dependency-ownership/src/dependency_ownership.ts index 53b6dc7275afa..7a384dc12b79a 100644 --- a/packages/kbn-dependency-ownership/src/dependency_ownership.ts +++ b/packages/kbn-dependency-ownership/src/dependency_ownership.ts @@ -18,7 +18,7 @@ interface GetDependencyOwnershipParams { missingOwner?: boolean; } -interface DependenciesByOwner { +export interface DependenciesByOwner { prodDependencies: string[]; devDependencies: string[]; } diff --git a/packages/kbn-dependency-ownership/src/rule.test.ts b/packages/kbn-dependency-ownership/src/rule.test.ts index 9ec6a6ff181af..f268e8b138bb9 100644 --- a/packages/kbn-dependency-ownership/src/rule.test.ts +++ b/packages/kbn-dependency-ownership/src/rule.test.ts @@ -11,6 +11,7 @@ import { ruleCoversDependency } from './rule'; describe('ruleCoversDependency', () => { const mockRule = { + groupName: 'mock', matchPackageNames: ['lodash'], matchPackagePatterns: ['^react'], matchDepNames: ['@testing-library/react'], diff --git a/packages/kbn-dependency-ownership/src/rule.ts b/packages/kbn-dependency-ownership/src/rule.ts index fae5b712f759e..b884891d68cc1 100644 --- a/packages/kbn-dependency-ownership/src/rule.ts +++ b/packages/kbn-dependency-ownership/src/rule.ts @@ -8,6 +8,7 @@ */ export interface RenovatePackageRule { + groupName: string; matchPackageNames?: string[]; matchDepNames?: string[]; matchPackagePatterns?: string[]; @@ -19,9 +20,15 @@ export interface RenovatePackageRule { } export function ruleFilter(rule: RenovatePackageRule) { + // Explicit list of rules that are allowed to be disabled. + const allowedDisabledRules = [ + 'bazel', // Per operations team. This is slated for removal, and does not make sense to track. + 'typescript', // These updates are always handled manually + 'webpack', // While we are in the middle of a webpack upgrade. TODO: Remove this once we are done. + ]; return ( - // Only include rules that are enabled - rule.enabled !== false && + // Only include rules that are enabled or explicitly allowed to be disabled + (allowedDisabledRules.includes(rule.groupName) || rule.enabled !== false) && // Only include rules that have a team reviewer rule.reviewers?.some((reviewer) => reviewer.startsWith('team:')) ); diff --git a/packages/kbn-dependency-ownership/tsconfig.json b/packages/kbn-dependency-ownership/tsconfig.json index 3587c78fa0269..8c7e455371c9f 100644 --- a/packages/kbn-dependency-ownership/tsconfig.json +++ b/packages/kbn-dependency-ownership/tsconfig.json @@ -17,5 +17,7 @@ ], "kbn_references": [ "@kbn/repo-info", + "@kbn/dev-cli-runner", + "@kbn/dev-cli-errors", ], } diff --git a/renovate.json b/renovate.json index d2d617bb1cf96..78e472e639a6d 100644 --- a/renovate.json +++ b/renovate.json @@ -3824,6 +3824,25 @@ "minimumReleaseAge": "7 days", "enabled": true }, + { + "groupName": "oas", + "matchDepNames": [ + "oas" + ], + "reviewers": [ + "team:security-scalability" + ], + "matchBaseBranches": [ + "main" + ], + "labels": [ + "Team:Security-Scalability", + "release_note:skip", + "backport:all-open" + ], + "minimumReleaseAge": "7 days", + "enabled": true + }, { "groupName": "seedrandom", "matchDepNames": [ diff --git a/yarn.lock b/yarn.lock index d0387d3cb0433..34e72d6ad644a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -22994,7 +22994,7 @@ jsonify@~0.0.0: resolved "https://registry.yarnpkg.com/jsonify/-/jsonify-0.0.0.tgz#2c74b6ee41d93ca51b7b5aaee8f503631d252a73" integrity sha1-LHS27kHZPKUbe1qu6PUDYx0lKnM= -jsonpath-plus@^10.0.0, jsonpath-plus@^10.2.0: +jsonpath-plus@^10.0.0: version "10.2.0" resolved "https://registry.yarnpkg.com/jsonpath-plus/-/jsonpath-plus-10.2.0.tgz#84d680544d9868579cc7c8f59bbe153a5aad54c4" integrity sha512-T9V+8iNYKFL2n2rF+w02LBOT2JjDnTjioaNFrxRy0Bv1y/hNsqR/EBK7Ojy2ythRHwmz2cRIls+9JitQGZC/sw==