[8.x] Add ESLINT constraints to detect inter-group dependencies (#194810

) (#197670) # Backport This will backport the following commits from `main` to `8.x`: - [Add ESLINT constraints to detect inter-group dependencies (#194810)](#194810)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)
elastic · Oct 24, 2024 · 7b82013 · 7b82013
1 parent 9d54a62
commit 7b82013
Show file tree

Hide file tree

Showing 36 changed files with 1,277 additions and 48 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -596,6 +596,7 @@ packages/kbn-management/settings/types @elastic/kibana-management
 packages/kbn-management/settings/utilities @elastic/kibana-management
 packages/kbn-management/storybook/config @elastic/kibana-management
 test/plugin_functional/plugins/management_test_plugin @elastic/kibana-management
+packages/kbn-manifest @elastic/kibana-core
 packages/kbn-mapbox-gl @elastic/kibana-gis
 x-pack/examples/third_party_maps_source_example @elastic/kibana-gis
 src/plugins/maps_ems @elastic/kibana-gis
@@ -930,9 +931,9 @@ packages/kbn-test-eui-helpers @elastic/kibana-visualizations
 x-pack/test/licensing_plugin/plugins/test_feature_usage @elastic/kibana-security
 packages/kbn-test-jest-helpers @elastic/kibana-operations @elastic/appex-qa
 packages/kbn-test-subj-selector @elastic/kibana-operations @elastic/appex-qa
-x-pack/test_serverless 
-test 
-x-pack/test 
+x-pack/test_serverless
+test
+x-pack/test
 x-pack/performance @elastic/appex-qa
 x-pack/examples/testing_embedded_lens @elastic/kibana-visualizations
 x-pack/examples/third_party_lens_navigation_prompt @elastic/kibana-visualizations

diff --git a/package.json b/package.json
@@ -633,6 +633,7 @@
     "@kbn/management-settings-types": "link:packages/kbn-management/settings/types",
     "@kbn/management-settings-utilities": "link:packages/kbn-management/settings/utilities",
     "@kbn/management-test-plugin": "link:test/plugin_functional/plugins/management_test_plugin",
+    "@kbn/manifest": "link:packages/kbn-manifest",
     "@kbn/mapbox-gl": "link:packages/kbn-mapbox-gl",
     "@kbn/maps-custom-raster-source-plugin": "link:x-pack/examples/third_party_maps_source_example",
     "@kbn/maps-ems-plugin": "link:src/plugins/maps_ems",

diff --git a/packages/kbn-dev-utils/src/plugin_list/run_plugin_list_cli.ts b/packages/kbn-dev-utils/src/plugin_list/run_plugin_list_cli.ts
@@ -20,14 +20,39 @@ const OUTPUT_PATH = Path.resolve(REPO_ROOT, 'docs/developer/plugin-list.asciidoc
 export function runPluginListCli() {
   run(async ({ log }) => {
     log.info('looking for oss plugins');
-    const ossPlugins = discoverPlugins('src/plugins');
-    log.success(`found ${ossPlugins.length} plugins`);
+    const ossLegacyPlugins = discoverPlugins('src/plugins');
+    const ossPlatformPlugins = discoverPlugins('src/platform/plugins');
+    log.success(`found ${ossLegacyPlugins.length + ossPlatformPlugins.length} plugins`);
 
     log.info('looking for x-pack plugins');
-    const xpackPlugins = discoverPlugins('x-pack/plugins');
-    log.success(`found ${xpackPlugins.length} plugins`);
+    const xpackLegacyPlugins = discoverPlugins('x-pack/plugins');
+    const xpackPlatformPlugins = discoverPlugins('x-pack/platform/plugins');
+    const xpackSearchPlugins = discoverPlugins('x-pack/solutions/search/plugins');
+    const xpackSecurityPlugins = discoverPlugins('x-pack/solutions/security/plugins');
+    const xpackObservabilityPlugins = discoverPlugins('x-pack/solutions/observability/plugins');
+    log.success(
+      `found ${
+        xpackLegacyPlugins.length +
+        xpackPlatformPlugins.length +
+        xpackSearchPlugins.length +
+        xpackSecurityPlugins.length +
+        xpackObservabilityPlugins.length
+      } plugins`
+    );
 
     log.info('writing plugin list to', OUTPUT_PATH);
-    Fs.writeFileSync(OUTPUT_PATH, generatePluginList(ossPlugins, xpackPlugins));
+    Fs.writeFileSync(
+      OUTPUT_PATH,
+      generatePluginList(
+        [...ossLegacyPlugins, ...ossPlatformPlugins],
+        [
+          ...xpackLegacyPlugins,
+          ...xpackPlatformPlugins,
+          ...xpackSearchPlugins,
+          ...xpackSecurityPlugins,
+          ...xpackObservabilityPlugins,
+        ]
+      )
+    );
   });
 }
diff --git a/packages/kbn-eslint-config/.eslintrc.js b/packages/kbn-eslint-config/.eslintrc.js
@@ -326,7 +326,8 @@ module.exports = {
     '@kbn/imports/uniform_imports': 'error',
     '@kbn/imports/no_unused_imports': 'error',
     '@kbn/imports/no_boundary_crossing': 'error',
-
+    '@kbn/imports/no_group_crossing_manifests': 'error',
+    '@kbn/imports/no_group_crossing_imports': 'error',
     'no-new-func': 'error',
     'no-implied-eval': 'error',
     'no-prototype-builtins': 'error',

diff --git a/packages/kbn-eslint-plugin-disable/src/helpers/protected_rules.ts b/packages/kbn-eslint-plugin-disable/src/helpers/protected_rules.ts
@@ -12,4 +12,6 @@ export const PROTECTED_RULES = new Set([
   '@kbn/disable/no_protected_eslint_disable',
   '@kbn/disable/no_naked_eslint_disable',
   '@kbn/imports/no_unused_imports',
+  '@kbn/imports/no_group_crossing_imports',
+  '@kbn/imports/no_group_crossing_manifests',
 ]);
diff --git a/packages/kbn-eslint-plugin-imports/index.ts b/packages/kbn-eslint-plugin-imports/index.ts
@@ -13,6 +13,8 @@ import { UniformImportsRule } from './src/rules/uniform_imports';
 import { ExportsMovedPackagesRule } from './src/rules/exports_moved_packages';
 import { NoUnusedImportsRule } from './src/rules/no_unused_imports';
 import { NoBoundaryCrossingRule } from './src/rules/no_boundary_crossing';
+import { NoGroupCrossingImportsRule } from './src/rules/no_group_crossing_imports';
+import { NoGroupCrossingManifestsRule } from './src/rules/no_group_crossing_manifests';
 import { RequireImportRule } from './src/rules/require_import';
 
 /**
@@ -25,5 +27,7 @@ export const rules = {
   exports_moved_packages: ExportsMovedPackagesRule,
   no_unused_imports: NoUnusedImportsRule,
   no_boundary_crossing: NoBoundaryCrossingRule,
+  no_group_crossing_imports: NoGroupCrossingImportsRule,
+  no_group_crossing_manifests: NoGroupCrossingManifestsRule,
   require_import: RequireImportRule,
 };
diff --git a/packages/kbn-eslint-plugin-imports/src/helpers/groups.ts b/packages/kbn-eslint-plugin-imports/src/helpers/groups.ts
@@ -0,0 +1,25 @@
+/*
+ * 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 type { ModuleGroup, ModuleVisibility } from '@kbn/repo-info/types';
+
+/**
+ * Checks whether a given ModuleGroup can import from another one
+ * @param importerGroup The group of the module that we are checking
+ * @param importedGroup The group of the imported module
+ * @param importedVisibility The visibility of the imported module
+ * @returns true if importerGroup is allowed to import from importedGroup/Visibiliy
+ */
+export function isImportableFrom(
+  importerGroup: ModuleGroup,
+  importedGroup: ModuleGroup,
+  importedVisibility: ModuleVisibility
+): boolean {
+  return importerGroup === importedGroup || importedVisibility === 'shared';
+}
diff --git a/packages/kbn-eslint-plugin-imports/src/helpers/report.ts b/packages/kbn-eslint-plugin-imports/src/helpers/report.ts
@@ -30,3 +30,19 @@ export function report(context: Rule.RuleContext, options: ReportOptions) {
       : null,
   });
 }
+
+export const toList = (strings: string[]) => {
+  const items = strings.map((s) => `"${s}"`);
+  const list = items.slice(0, -1).join(', ');
+  const last = items.at(-1);
+  return !list.length ? last ?? '' : `${list} or ${last}`;
+};
+
+export const formatSuggestions = (suggestions: string[]) => {
+  const s = suggestions.map((l) => l.trim()).filter(Boolean);
+  if (!s.length) {
+    return '';
+  }
+
+  return ` \nSuggestions:\n - ${s.join('\n - ')}\n\n`;
+};
diff --git a/packages/kbn-eslint-plugin-imports/src/rules/no_boundary_crossing.test.ts b/packages/kbn-eslint-plugin-imports/src/rules/no_boundary_crossing.test.ts
@@ -9,8 +9,9 @@
 
 import { RuleTester } from 'eslint';
 import { NoBoundaryCrossingRule } from './no_boundary_crossing';
-import { ModuleType } from '@kbn/repo-source-classifier';
+import type { ModuleType } from '@kbn/repo-source-classifier';
 import dedent from 'dedent';
+import { formatSuggestions } from '../helpers/report';
 
 const make = (from: ModuleType, to: ModuleType, imp = 'import') => ({
   filename: `${from}.ts`,
@@ -107,13 +108,12 @@ for (const [name, tester] of [tsTester, babelTester]) {
               data: {
                 importedType: 'server package',
                 ownType: 'common package',
-                suggestion: ` ${dedent`
-                  Suggestions:
-                   - Remove the import statement.
-                   - Limit your imports to "common package" or "static" code.
-                   - Covert to a type-only import.
-                   - Reach out to #kibana-operations for help.
-                `}`,
+                suggestion: formatSuggestions([
+                  'Remove the import statement.',
+                  'Limit your imports to "common package" or "static" code.',
+                  'Covert to a type-only import.',
+                  'Reach out to #kibana-operations for help.',
+                ]),
               },
             },
           ],

diff --git a/packages/kbn-eslint-plugin-imports/src/rules/no_boundary_crossing.ts b/packages/kbn-eslint-plugin-imports/src/rules/no_boundary_crossing.ts
@@ -12,13 +12,14 @@ import Path from 'path';
 import { TSESTree } from '@typescript-eslint/typescript-estree';
 import * as Bt from '@babel/types';
 import type { Rule } from 'eslint';
-import ESTree from 'estree';
-import { ModuleType } from '@kbn/repo-source-classifier';
+import type { Node } from 'estree';
+import type { ModuleType } from '@kbn/repo-source-classifier';
 
 import { visitAllImportStatements, Importer } from '../helpers/visit_all_import_statements';
 import { getSourcePath } from '../helpers/source';
 import { getRepoSourceClassifier } from '../helpers/repo_source_classifier';
 import { getImportResolver } from '../get_import_resolver';
+import { formatSuggestions, toList } from '../helpers/report';
 
 const ANY = Symbol();
 
@@ -33,22 +34,6 @@ const IMPORTABLE_FROM: Record<ModuleType, ModuleType[] | typeof ANY> = {
   tooling: ANY,
 };
 
-const toList = (strings: string[]) => {
-  const items = strings.map((s) => `"${s}"`);
-  const list = items.slice(0, -1).join(', ');
-  const last = items.at(-1);
-  return !list.length ? last ?? '' : `${list} or ${last}`;
-};
-
-const formatSuggestions = (suggestions: string[]) => {
-  const s = suggestions.map((l) => l.trim()).filter(Boolean);
-  if (!s.length) {
-    return '';
-  }
-
-  return ` Suggestions:\n - ${s.join('\n - ')}`;
-};
-
 const isTypeOnlyImport = (importer: Importer) => {
   // handle babel nodes
   if (Bt.isImportDeclaration(importer)) {
@@ -125,7 +110,7 @@ export const NoBoundaryCrossingRule: Rule.RuleModule = {
 
       if (!importable.includes(imported.type)) {
         context.report({
-          node: node as ESTree.Node,
+          node: node as Node,
           messageId: 'TYPE_MISMATCH',
           data: {
             ownType: self.type,

diff --git a/packages/kbn-eslint-plugin-imports/src/rules/no_group_crossing_imports.test.ts b/packages/kbn-eslint-plugin-imports/src/rules/no_group_crossing_imports.test.ts
@@ -0,0 +1,155 @@
+/*
+ * 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 { RuleTester } from 'eslint';
+import dedent from 'dedent';
+import { NoGroupCrossingImportsRule } from './no_group_crossing_imports';
+import { formatSuggestions } from '../helpers/report';
+import { ModuleGroup, ModuleVisibility } from '@kbn/repo-info/types';
+
+const make = (
+  fromGroup: ModuleGroup,
+  fromVisibility: ModuleVisibility,
+  toGroup: ModuleGroup,
+  toVisibility: ModuleVisibility,
+  imp = 'import'
+) => ({
+  filename: `${fromGroup}.${fromVisibility}.ts`,
+  code: dedent`
+    ${imp} '${toGroup}.${toVisibility}'
+  `,
+});
+
+jest.mock('../get_import_resolver', () => {
+  return {
+    getImportResolver() {
+      return {
+        resolve(req: string) {
+          return {
+            type: 'file',
+            absolute: req.split('.'),
+          };
+        },
+      };
+    },
+  };
+});
+
+jest.mock('../helpers/repo_source_classifier', () => {
+  return {
+    getRepoSourceClassifier() {
+      return {
+        classify(r: string | [string, string]) {
+          const [group, visibility] =
+            typeof r === 'string' ? (r.endsWith('.ts') ? r.slice(0, -3) : r).split('.') : r;
+          return {
+            pkgInfo: {
+              pkgId: 'aPackage',
+            },
+            group,
+            visibility,
+          };
+        },
+      };
+    },
+  };
+});
+
+const tsTester = [
+  '@typescript-eslint/parser',
+  new RuleTester({
+    parser: require.resolve('@typescript-eslint/parser'),
+    parserOptions: {
+      sourceType: 'module',
+      ecmaVersion: 2018,
+      ecmaFeatures: {
+        jsx: true,
+      },
+    },
+  }),
+] as const;
+
+const babelTester = [
+  '@babel/eslint-parser',
+  new RuleTester({
+    parser: require.resolve('@babel/eslint-parser'),
+    parserOptions: {
+      sourceType: 'module',
+      ecmaVersion: 2018,
+      requireConfigFile: false,
+      babelOptions: {
+        presets: ['@kbn/babel-preset/node_preset'],
+      },
+    },
+  }),
+] as const;
+
+for (const [name, tester] of [tsTester, babelTester]) {
+  describe(name, () => {
+    tester.run('@kbn/imports/no_group_crossing_imports', NoGroupCrossingImportsRule, {
+      valid: [
+        make('observability', 'private', 'observability', 'private'),
+        make('security', 'private', 'security', 'private'),
+        make('search', 'private', 'search', 'private'),
+        make('observability', 'private', 'platform', 'shared'),
+        make('security', 'private', 'common', 'shared'),
+        make('platform', 'shared', 'platform', 'shared'),
+        make('platform', 'shared', 'platform', 'private'),
+        make('common', 'shared', 'common', 'shared'),
+      ],
+
+      invalid: [
+        {
+          ...make('observability', 'private', 'security', 'private'),
+          errors: [
+            {
+              line: 1,
+              messageId: 'ILLEGAL_IMPORT',
+              data: {
+                importerPackage: 'aPackage',
+                importerGroup: 'observability',
+                importedPackage: 'aPackage',
+                importedGroup: 'security',
+                importedVisibility: 'private',
+                sourcePath: 'observability.private.ts',
+                suggestion: formatSuggestions([
+                  `Please review the dependencies in your module's manifest (kibana.jsonc).`,
+                  `Relocate this module to a different group, and/or make sure it has the right 'visibility'.`,
+                  `Address the conflicting dependencies by refactoring the code`,
+                ]),
+              },
+            },
+          ],
+        },
+        {
+          ...make('security', 'private', 'platform', 'private'),
+          errors: [
+            {
+              line: 1,
+              messageId: 'ILLEGAL_IMPORT',
+              data: {
+                importerPackage: 'aPackage',
+                importerGroup: 'security',
+                importedPackage: 'aPackage',
+                importedGroup: 'platform',
+                importedVisibility: 'private',
+                sourcePath: 'security.private.ts',
+                suggestion: formatSuggestions([
+                  `Please review the dependencies in your module's manifest (kibana.jsonc).`,
+                  `Relocate this module to a different group, and/or make sure it has the right 'visibility'.`,
+                  `Address the conflicting dependencies by refactoring the code`,
+                ]),
+              },
+            },
+          ],
+        },
+      ],
+    });
+  });
+}