[9.0] [Renovate] - Enhance dependency ownership checks (elastic#231962) (elastic#233530)

legrego · web-flow · commit 008c502926e6 · 2025-09-02T16:03:56.000+02:00
# Backport This will backport the following commits from `main` to `9.0`: - [[Renovate] - Enhance dependency ownership checks (elastic#231962)](elastic#231962)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)
diff --git a/dev_docs/contributing/third_party_dependencies.mdx b/dev_docs/contributing/third_party_dependencies.mdx
@@ -102,7 +102,7 @@ Here is an example configuration for a dependency in the `renovate.json` file:
       "release_note:skip",
       "backport:all-open",
       "effort:low",
-      "risk:high"
+      "upgrade-risk:high"
     ],
     // [5]
     "minimumReleaseAge": "7 days",
@@ -117,7 +117,7 @@ Here is an example configuration for a dependency in the `renovate.json` file:
 
 [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. Include an `effort:low|medium|high` label to indicate the level of effort required to update the codebase, and a `risk:low|medium|high` label to indicate the level of testing required to be confident in the changes. 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.
+[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. Include an `effort:low|medium|high` label to indicate the level of effort required to update the codebase, and an `upgrade-risk:low|medium|high` label to indicate the level of testing required to be confident in the changes. 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.
 
diff --git a/package.json b/package.json
@@ -1607,7 +1607,6 @@
     "@types/json5": "^2.2.0",
     "@types/jsonwebtoken": "^9.0.0",
     "@types/license-checker": "15.0.0",
-    "@types/loader-utils": "^2.0.3",
     "@types/lodash": "^4.17.16",
     "@types/lz-string": "^1.5.0",
     "@types/mapbox__vector-tile": "1.3.0",
diff --git a/packages/kbn-dependency-ownership/src/cli.ts b/packages/kbn-dependency-ownership/src/cli.ts
@@ -46,8 +46,11 @@ export async function identifyDependencyOwnershipCLI() {
 
       const result = identifyDependencyOwnership({ dependency, owner, missingOwner });
       if (failIfUnowned) {
-        const { prodDependencies = [] as string[], devDependencies = [] as string[] } =
-          result as DependenciesByOwner;
+        const {
+          prodDependencies = [] as string[],
+          devDependencies = [] as string[],
+          invalidRenovateRules = [] as string[],
+        } = result as DependenciesByOwner;
 
         const uncoveredDependencies = [...prodDependencies, ...devDependencies];
         if (uncoveredDependencies.length > 0) {
@@ -56,9 +59,17 @@ export async function identifyDependencyOwnershipCLI() {
           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 (invalidRenovateRules.length > 0) {
+          log.write('Invalid renovate rules:');
+          log.write(invalidRenovateRules.map((rule) => ` - ${rule}`).join('\n'));
+          throw createFailError(
+            `Found ${invalidRenovateRules.length} invalid renovate rules. Please update \`renovate.json\` to fix these errors.\nVisit https://docs.elastic.dev/kibana-dev-docs/third-party-dependencies#dependency-ownership for more information.`
+          );
+        }
+
+        log.success('All dependencies have an owner');
       }
 
       if (outputPath) {
diff --git a/packages/kbn-dependency-ownership/src/dependency_ownership.test.ts b/packages/kbn-dependency-ownership/src/dependency_ownership.test.ts
@@ -7,8 +7,10 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
+import type { DependenciesByOwner } from './dependency_ownership';
 import { identifyDependencyOwnership } from './dependency_ownership';
 import { parseConfig } from './parse_config';
+import { ruleFilter } from './rule';
 
 jest.mock('./parse_config', () => ({
   parseConfig: jest.fn(),
@@ -18,23 +20,33 @@ describe('identifyDependencyOwnership', () => {
   const mockConfig = {
     renovateRules: [
       {
+        groupName: 'core-libs',
         reviewers: ['team:elastic', 'team:infra'],
         matchPackageNames: ['lodash', 'react'],
         enabled: true,
       },
       {
+        groupName: 'testing-libs',
         reviewers: ['team:ui'],
-        matchPackageNames: ['@testing-library/react'],
+        matchPackageNames: ['@testing-library/react', 'undefined-package'],
         enabled: true,
       },
       {
+        groupName: 'resolved-libs',
+        reviewers: ['team:resolved'],
+        matchPackageNames: ['some-resolved-lib'],
+        enabled: true,
+      },
+      {
+        groupName: 'disabled-libs',
         reviewers: ['team:disabled-team'],
         matchPackageNames: ['disabled-package'],
         enabled: false, // Disabled rule
       },
-    ],
-    packageDependencies: ['lodash', 'react'],
+    ].filter(ruleFilter),
+    packageDependencies: ['lodash', 'react', 'disabled-package'],
     packageDevDependencies: ['jest', '@testing-library/react'],
+    packageResolutions: ['**/some-resolved-lib'],
   };
 
   beforeEach(() => {
@@ -80,29 +92,42 @@ describe('identifyDependencyOwnership', () => {
   });
 
   it('returns uncovered dependencies when missingOwner is true', () => {
-    const result = identifyDependencyOwnership({ missingOwner: true });
-    expect(result).toEqual({
-      prodDependencies: [],
+    const { prodDependencies, devDependencies } = identifyDependencyOwnership({
+      missingOwner: true,
+    }) as DependenciesByOwner;
+    expect({ prodDependencies, devDependencies }).toEqual({
+      prodDependencies: ['disabled-package'],
       devDependencies: ['jest'],
     });
   });
 
+  it('returns renovate rule errors for undeclared dependencies', () => {
+    const { invalidRenovateRules } = identifyDependencyOwnership({
+      missingOwner: true,
+    }) as DependenciesByOwner;
+    expect(invalidRenovateRules).toMatchInlineSnapshot(`
+      Array [
+        "Invalid renovate rule: 'testing-libs' declares package 'undefined-package', which is not found in package.json.",
+      ]
+    `);
+  });
+
   it('returns comprehensive ownership coverage, considering only enabled rules', () => {
     const result = identifyDependencyOwnership({});
     expect(result).toEqual({
       prodDependenciesByOwner: {
         '@elastic/elastic': ['lodash', 'react'],
         '@elastic/infra': ['lodash', 'react'],
         '@elastic/ui': [],
-        '@elastic/disabled-team': [],
+        '@elastic/resolved': [],
       },
       devDependenciesByOwner: {
         '@elastic/elastic': [],
         '@elastic/infra': [],
         '@elastic/ui': ['@testing-library/react'],
-        '@elastic/disabled-team': [],
+        '@elastic/resolved': [],
       },
-      uncoveredProdDependencies: [],
+      uncoveredProdDependencies: ['disabled-package'],
       uncoveredDevDependencies: ['jest'],
       coveredProdDependencies: ['lodash', 'react'],
       coveredDevDependencies: ['@testing-library/react'],
diff --git a/packages/kbn-dependency-ownership/src/dependency_ownership.ts b/packages/kbn-dependency-ownership/src/dependency_ownership.ts
@@ -21,6 +21,7 @@ interface GetDependencyOwnershipParams {
 export interface DependenciesByOwner {
   prodDependencies: string[];
   devDependencies: string[];
+  invalidRenovateRules?: string[];
 }
 
 interface DependenciesByOwners {
@@ -106,6 +107,31 @@ const getDependenciesByOwner = (): DependenciesByOwners => {
   return dependenciesByOwner;
 };
 
+const getInvalidRenovateRules = (): string[] => {
+  const { renovateRules, packageDependencies, packageDevDependencies, packageResolutions } =
+    parseConfig();
+  const declaredDependencies = new Set([...packageDependencies, ...packageDevDependencies]);
+
+  const errors: string[] = [];
+
+  renovateRules.forEach((rule) => {
+    const { matchPackageNames = [], matchDepNames = [] } = rule;
+    const allMatchedNames = [...matchPackageNames, ...matchDepNames];
+    allMatchedNames.forEach((name) => {
+      if (
+        !declaredDependencies.has(name) &&
+        !packageResolutions.some((resolution) => resolution.includes(name))
+      ) {
+        errors.push(
+          `Invalid renovate rule: '${rule.groupName}' declares package '${name}', which is not found in package.json.`
+        );
+      }
+    });
+  });
+
+  return errors;
+};
+
 const getDependenciesCoverage = (): DependenciesCoverage => {
   const { renovateRules, packageDependencies, packageDevDependencies } = parseConfig();
 
@@ -168,10 +194,13 @@ export const identifyDependencyOwnership = ({
     coveredProdDependencies,
   } = getDependenciesCoverage();
 
+  const invalidRenovateRules = getInvalidRenovateRules();
+
   if (missingOwner) {
     return {
       prodDependencies: uncoveredProdDependencies,
       devDependencies: uncoveredDevDependencies,
+      invalidRenovateRules,
     };
   }
 
diff --git a/packages/kbn-dependency-ownership/src/parse_config.ts b/packages/kbn-dependency-ownership/src/parse_config.ts
@@ -17,6 +17,7 @@ export const parseConfig = (() => {
     renovateRules: RenovatePackageRule[];
     packageDependencies: string[];
     packageDevDependencies: string[];
+    packageResolutions: string[];
   } | null = null;
 
   return () => {
@@ -37,8 +38,9 @@ export const parseConfig = (() => {
     const packageDevDependencies = Object.keys(packageConfig?.devDependencies || {}).filter(
       packageFilter
     );
+    const packageResolutions = Object.keys(packageConfig?.resolutions || {});
 
-    cache = { renovateRules, packageDependencies, packageDevDependencies };
+    cache = { renovateRules, packageDependencies, packageDevDependencies, packageResolutions };
     return cache;
   };
 })();
diff --git a/packages/kbn-dependency-ownership/src/rule.ts b/packages/kbn-dependency-ownership/src/rule.ts
@@ -13,6 +13,7 @@ export interface RenovatePackageRule {
   matchDepNames?: string[];
   matchPackagePatterns?: string[];
   matchDepPatterns?: string[];
+  matchManagers?: string[];
   excludePackageNames?: string[];
   excludePackagePatterns?: string[];
   enabled?: boolean;
@@ -26,11 +27,18 @@ export function ruleFilter(rule: RenovatePackageRule) {
     '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.
   ];
+  // Rules that use custom managers are not supported by this tool, and are ignored.
+  const rulesWithCustomManagers = ['chainguard', 'chainguard-fips'];
+
   return (
     // 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:'))
+    rule.reviewers?.some((reviewer) => reviewer.startsWith('team:')) &&
+    // Only include rules that use the default manager, or specify npm
+    (!rule.matchManagers || !rule.matchManagers.length || rule.matchManagers.includes('npm')) &&
+    // Exclude rules that use custom managers
+    !rulesWithCustomManagers.includes(rule.groupName)
   );
 }
 
diff --git a/renovate.json b/renovate.json
diff --git a/yarn.lock b/yarn.lock
