Skip to content

Commit

Permalink
Add check to fail CI if any dependencies are unowned (#206679)
Browse files Browse the repository at this point in the history
## Summary
- Updates `scripts/dependency_ownership` to use the
`@kbn/dev-cli-runner` for consistency with other CI-related CLIs.
- Adds a new `failIfUnowned` flag to exit with an error code if any
dependencies are unowned.
- Adds a new dependency ownership check to `quick_checks` and `renovate`
CI steps.


From a CI run, the additional quick check executes successfully in 3
seconds:
```sh
info [quick-checks] Passed check: /opt/buildkite-agent/builds/bk-agent-prod-gcp-abc123/elastic/kibana-pull-request/kibana/.buildkite/scripts/steps/checks/dependencies_missing_owner.sh in 3s
```

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
legrego and kibanamachine authored Jan 16, 2025
1 parent 2f6b9f6 commit 395e494
Show file tree
Hide file tree
Showing 13 changed files with 195 additions and 207 deletions.
8 changes: 8 additions & 0 deletions .buildkite/scripts/steps/checks/dependencies_missing_owner.sh
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions .buildkite/scripts/steps/checks/quick_checks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions .buildkite/scripts/steps/renovate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ set -euo pipefail

echo '--- Renovate: validation'
.buildkite/scripts/steps/checks/renovate.sh
.buildkite/scripts/steps/checks/dependencies_missing_owner.sh
79 changes: 74 additions & 5 deletions dev_docs/contributing/third_party_dependencies.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
```
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
119 changes: 0 additions & 119 deletions packages/kbn-dependency-ownership/src/cli.test.ts

This file was deleted.

Loading

0 comments on commit 395e494

Please sign in to comment.