Add ESLINT constraints to detect inter-group dependencies #194810
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gsoldevila
added
Team:Core
💔 Build Failed
Failed CI StepsHistory
To update your PR or re-run it, just comment with: |
This was referenced Oct 8, 2024
gsoldevila
force-pushed
the
kbn-team-1066-eslint-restrictions
branch
from
667aa3b
to
d6ac48d
Compare
|
Pinging @elastic/kibana-core (Team:Core) |
gsoldevila
changed the title
💚 Build Succeeded
Metrics [docs]History
|
Ikuni17
reviewed
Oct 15, 2024
jloleysens
approved these changes
Oct 16, 2024
There was a problem hiding this comment.
Overall this looks great @gsoldevila , I tested locally focussing mainly on the end DX and the changes LGTM! I tested by setting this in cases kibana.jsonc and importing from elsewhere:
diff --git a/x-pack/plugins/cases/kibana.jsonc b/x-pack/plugins/cases/kibana.jsonc
index 300b1ee4c2c..0b47b80f625 100644
--- a/x-pack/plugins/cases/kibana.jsonc
+++ b/x-pack/plugins/cases/kibana.jsonc
@@ -3,6 +3,8 @@
"id": "@kbn/cases-plugin",
"owner": "@elastic/response-ops",
"description": "The Case management system in Kibana",
+ "group": "observability",
+ "visibility": "public",
"plugin": {
"id": "cases",
"server": true,And after restarting extensions I got:
What do you think about introducing the new group: ModuleGroup and visibility: ModuleVisibility value to the packages/kbn-kibana-manifest-schema/src/kibana_json_v2_schema.ts so that VS code can autocomplete it?
| sourcePath: relativePath, | ||
| 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'.`, |
There was a problem hiding this comment.
Reading this warning: if you're importing from something that is private I'm not sure moving "this" module will solve the import restriction? Unless we move it to the private module?
There was a problem hiding this comment.
Yes, I was thinking moving this module to the same group as the one it depends on, as one possible option, but perhaps there's better wording for that.
There was a problem hiding this comment.
Not quite sure I understand how to test this one. Mind providing an example?
There was a problem hiding this comment.
Basically, if you plugin manifest has an illegal dependency, your export function plugin() as well as your setup() and start() hooks will complain.
That sounds like a very good idea, was not knowledgeable about this part! |
gsoldevila
force-pushed
the
kbn-team-1066-eslint-restrictions
branch
from
4870c55
to
79c2ef1
Compare
Ikuni17
approved these changes
Oct 21, 2024
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/11459301334 |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Unknown metric groupsAPI count
History
|
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
kibanamachine
added
the
backport missing
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
|
Friendly reminder: Looks like this PR hasn’t been backported yet. |
gsoldevila
added a commit
to gsoldevila/kibana
that referenced
this pull request
Oct 24, 2024
…4810) ## Summary Addresses elastic/kibana-team#1175 As part of the **Sustainable Kibana Architecture** initiative, this PR sets the foundation to start classifying plugins in isolated groups, matching our current solutions / project types: * It adds support for the following fields in the packages' manifests (kibana.jsonc): * `group?: 'search' | 'security' | 'observability' | 'platform' | 'common'` * `visibility?: 'private' | 'shared'` * It proposes a folder structure to automatically infer groups: ```javascript 'src/platform/plugins/shared': { group: 'platform', visibility: 'shared', }, 'src/platform/plugins/internal': { group: 'platform', visibility: 'private', }, 'x-pack/platform/plugins/shared': { group: 'platform', visibility: 'shared', }, 'x-pack/platform/plugins/internal': { group: 'platform', visibility: 'private', }, 'x-pack/solutions/observability/plugins': { group: 'observability', visibility: 'private', }, 'x-pack/solutions/security/plugins': { group: 'security', visibility: 'private', }, 'x-pack/solutions/search/plugins': { group: 'search', visibility: 'private', }, ``` * If a plugin is moved to one of the specific locations above, the group and visibility in the manifest (if specified) must match those inferred from the path. * Plugins that are not relocated are considered: `group: 'common', visibility: 'shared'` by default. As soon as we specify a custom `group`, the ESLINT rules will check violations against dependencies / dependants. The ESLINT rules are pretty simple: * Plugins can only depend on: * Plugins in the same group * OR plugins with `'shared'` visibility * Plugins in `'observability', 'security', 'search'` groups are mandatorily `'private'`. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 2a085e1) # Conflicts: # .github/CODEOWNERS # packages/kbn-eslint-config/.eslintrc.js
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
gsoldevila
added a commit
that referenced
this pull request
Oct 24, 2024
) (#197670) # Backport This will backport the following commits from `main` to `8.x`: - [Add ESLINT constraints to detect inter-group dependencies (#194810)](#194810) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Gerard Soldevila","email":"gerard.soldevila@elastic.co"},"sourceCommit":{"committedDate":"2024-10-22T11:34:19Z","message":"Add ESLINT constraints to detect inter-group dependencies (#194810)\n\n## Summary\r\n\r\nAddresses https://github.com/elastic/kibana-team/issues/1175\r\n\r\nAs part of the **Sustainable Kibana Architecture** initiative, this PR\r\nsets the foundation to start classifying plugins in isolated groups,\r\nmatching our current solutions / project types:\r\n\r\n* It adds support for the following fields in the packages' manifests\r\n(kibana.jsonc):\r\n* `group?: 'search' | 'security' | 'observability' | 'platform' |\r\n'common'`\r\n * `visibility?: 'private' | 'shared'`\r\n\r\n* It proposes a folder structure to automatically infer groups:\r\n```javascript\r\n 'src/platform/plugins/shared': {\r\n group: 'platform',\r\n visibility: 'shared',\r\n },\r\n 'src/platform/plugins/internal': {\r\n group: 'platform',\r\n visibility: 'private',\r\n },\r\n 'x-pack/platform/plugins/shared': {\r\n group: 'platform',\r\n visibility: 'shared',\r\n },\r\n 'x-pack/platform/plugins/internal': {\r\n group: 'platform',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/observability/plugins': {\r\n group: 'observability',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/security/plugins': {\r\n group: 'security',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/search/plugins': {\r\n group: 'search',\r\n visibility: 'private',\r\n },\r\n```\r\n\r\n* If a plugin is moved to one of the specific locations above, the group\r\nand visibility in the manifest (if specified) must match those inferred\r\nfrom the path.\r\n* Plugins that are not relocated are considered: `group: 'common',\r\nvisibility: 'shared'` by default. As soon as we specify a custom\r\n`group`, the ESLINT rules will check violations against dependencies /\r\ndependants.\r\n\r\nThe ESLINT rules are pretty simple:\r\n* Plugins can only depend on:\r\n * Plugins in the same group\r\n * OR plugins with `'shared'` visibility\r\n* Plugins in `'observability', 'security', 'search'` groups are\r\nmandatorily `'private'`.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"2a085e103afe8c7bdfb626d0dc683fc8be0e6c05","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","backport missing","v9.0.0","release_note:feature","backport:prev-minor"],"number":194810,"url":"https://github.com/elastic/kibana/pull/194810","mergeCommit":{"message":"Add ESLINT constraints to detect inter-group dependencies (#194810)\n\n## Summary\r\n\r\nAddresses https://github.com/elastic/kibana-team/issues/1175\r\n\r\nAs part of the **Sustainable Kibana Architecture** initiative, this PR\r\nsets the foundation to start classifying plugins in isolated groups,\r\nmatching our current solutions / project types:\r\n\r\n* It adds support for the following fields in the packages' manifests\r\n(kibana.jsonc):\r\n* `group?: 'search' | 'security' | 'observability' | 'platform' |\r\n'common'`\r\n * `visibility?: 'private' | 'shared'`\r\n\r\n* It proposes a folder structure to automatically infer groups:\r\n```javascript\r\n 'src/platform/plugins/shared': {\r\n group: 'platform',\r\n visibility: 'shared',\r\n },\r\n 'src/platform/plugins/internal': {\r\n group: 'platform',\r\n visibility: 'private',\r\n },\r\n 'x-pack/platform/plugins/shared': {\r\n group: 'platform',\r\n visibility: 'shared',\r\n },\r\n 'x-pack/platform/plugins/internal': {\r\n group: 'platform',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/observability/plugins': {\r\n group: 'observability',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/security/plugins': {\r\n group: 'security',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/search/plugins': {\r\n group: 'search',\r\n visibility: 'private',\r\n },\r\n```\r\n\r\n* If a plugin is moved to one of the specific locations above, the group\r\nand visibility in the manifest (if specified) must match those inferred\r\nfrom the path.\r\n* Plugins that are not relocated are considered: `group: 'common',\r\nvisibility: 'shared'` by default. As soon as we specify a custom\r\n`group`, the ESLINT rules will check violations against dependencies /\r\ndependants.\r\n\r\nThe ESLINT rules are pretty simple:\r\n* Plugins can only depend on:\r\n * Plugins in the same group\r\n * OR plugins with `'shared'` visibility\r\n* Plugins in `'observability', 'security', 'search'` groups are\r\nmandatorily `'private'`.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"2a085e103afe8c7bdfb626d0dc683fc8be0e6c05"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194810","number":194810,"mergeCommit":{"message":"Add ESLINT constraints to detect inter-group dependencies (#194810)\n\n## Summary\r\n\r\nAddresses https://github.com/elastic/kibana-team/issues/1175\r\n\r\nAs part of the **Sustainable Kibana Architecture** initiative, this PR\r\nsets the foundation to start classifying plugins in isolated groups,\r\nmatching our current solutions / project types:\r\n\r\n* It adds support for the following fields in the packages' manifests\r\n(kibana.jsonc):\r\n* `group?: 'search' | 'security' | 'observability' | 'platform' |\r\n'common'`\r\n * `visibility?: 'private' | 'shared'`\r\n\r\n* It proposes a folder structure to automatically infer groups:\r\n```javascript\r\n 'src/platform/plugins/shared': {\r\n group: 'platform',\r\n visibility: 'shared',\r\n },\r\n 'src/platform/plugins/internal': {\r\n group: 'platform',\r\n visibility: 'private',\r\n },\r\n 'x-pack/platform/plugins/shared': {\r\n group: 'platform',\r\n visibility: 'shared',\r\n },\r\n 'x-pack/platform/plugins/internal': {\r\n group: 'platform',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/observability/plugins': {\r\n group: 'observability',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/security/plugins': {\r\n group: 'security',\r\n visibility: 'private',\r\n },\r\n 'x-pack/solutions/search/plugins': {\r\n group: 'search',\r\n visibility: 'private',\r\n },\r\n```\r\n\r\n* If a plugin is moved to one of the specific locations above, the group\r\nand visibility in the manifest (if specified) must match those inferred\r\nfrom the path.\r\n* Plugins that are not relocated are considered: `group: 'common',\r\nvisibility: 'shared'` by default. As soon as we specify a custom\r\n`group`, the ESLINT rules will check violations against dependencies /\r\ndependants.\r\n\r\nThe ESLINT rules are pretty simple:\r\n* Plugins can only depend on:\r\n * Plugins in the same group\r\n * OR plugins with `'shared'` visibility\r\n* Plugins in `'observability', 'security', 'search'` groups are\r\nmandatorily `'private'`.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"2a085e103afe8c7bdfb626d0dc683fc8be0e6c05"}}]}] BACKPORT-->
kibanamachine
added
v8.17.0
and removed
backport missing
gsoldevila
added a commit
that referenced
this pull request
Oct 25, 2024
## Summary Follow-up of #195367 As part of the Sustainable Kibana Architecture initiative, this PR leverages the mechanisms and concepts introduced in #194810, updating plugins that were considered to be solution-specific in Luke's [PoC](#179710). This might trigger linting rule violations in CI, and help uncover conflicts related to forbidden dependencies. As soon as they are resolved, we can proceed to classify solutions' plugins.
gsoldevila
added a commit
to gsoldevila/kibana
that referenced
this pull request
Nov 7, 2024
…95375) ## Summary Follow-up of elastic#195367 As part of the Sustainable Kibana Architecture initiative, this PR leverages the mechanisms and concepts introduced in elastic#194810, updating plugins that were considered to be solution-specific in Luke's [PoC](elastic#179710). This might trigger linting rule violations in CI, and help uncover conflicts related to forbidden dependencies. As soon as they are resolved, we can proceed to classify solutions' plugins. (cherry picked from commit a5517d9) # Conflicts: # src/plugins/maps_ems/kibana.jsonc # x-pack/plugins/file_upload/kibana.jsonc # x-pack/plugins/maps/kibana.jsonc
gsoldevila
added a commit
to gsoldevila/kibana
that referenced
this pull request
Nov 7, 2024
…95375) ## Summary Follow-up of elastic#195367 As part of the Sustainable Kibana Architecture initiative, this PR leverages the mechanisms and concepts introduced in elastic#194810, updating plugins that were considered to be solution-specific in Luke's [PoC](elastic#179710). This might trigger linting rule violations in CI, and help uncover conflicts related to forbidden dependencies. As soon as they are resolved, we can proceed to classify solutions' plugins. (cherry picked from commit a5517d9) # Conflicts: # src/plugins/maps_ems/kibana.jsonc # x-pack/plugins/file_upload/kibana.jsonc # x-pack/plugins/maps/kibana.jsonc
gsoldevila
added a commit
that referenced
this pull request
Nov 8, 2024
…5375) (#199268) # Backport This will backport the following commits from `main` to `8.x`: - [[Sustainable Kibana Architecture] Update plugins (wave #1) (#195375)](#195375) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Gerard Soldevila","email":"gerard.soldevila@elastic.co"},"sourceCommit":{"committedDate":"2024-10-25T14:05:27Z","message":"[Sustainable Kibana Architecture] Update plugins (wave #1) (#195375)\n\n## Summary\r\n\r\nFollow-up of https://github.com/elastic/kibana/pull/195367\r\nAs part of the Sustainable Kibana Architecture initiative, this PR\r\nleverages the mechanisms and concepts introduced in\r\nhttps://github.com//pull/194810, updating plugins that\r\nwere considered to be solution-specific in Luke's\r\n[PoC](https://github.com/elastic/kibana/pull/179710).\r\n\r\nThis might trigger linting rule violations in CI, and help uncover\r\nconflicts related to forbidden dependencies.\r\nAs soon as they are resolved, we can proceed to classify solutions'\r\nplugins.","sha":"a5517d9d2cf38369fc46ea9622ce36c768436aad","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","Feature:Embedding","Feature:ExpressionLanguage","release_note:skip","Feature:Drilldowns","Team:Fleet","v9.0.0","backport:prev-minor","Team:Obs AI Assistant","ci:project-deploy-observability","Team:obs-ux-infra_services","Team:obs-ux-management"],"number":195375,"url":"https://github.com/elastic/kibana/pull/195375","mergeCommit":{"message":"[Sustainable Kibana Architecture] Update plugins (wave #1) (#195375)\n\n## Summary\r\n\r\nFollow-up of https://github.com/elastic/kibana/pull/195367\r\nAs part of the Sustainable Kibana Architecture initiative, this PR\r\nleverages the mechanisms and concepts introduced in\r\nhttps://github.com//pull/194810, updating plugins that\r\nwere considered to be solution-specific in Luke's\r\n[PoC](https://github.com/elastic/kibana/pull/179710).\r\n\r\nThis might trigger linting rule violations in CI, and help uncover\r\nconflicts related to forbidden dependencies.\r\nAs soon as they are resolved, we can proceed to classify solutions'\r\nplugins.","sha":"a5517d9d2cf38369fc46ea9622ce36c768436aad"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195375","number":195375,"mergeCommit":{"message":"[Sustainable Kibana Architecture] Update plugins (wave #1) (#195375)\n\n## Summary\r\n\r\nFollow-up of https://github.com/elastic/kibana/pull/195367\r\nAs part of the Sustainable Kibana Architecture initiative, this PR\r\nleverages the mechanisms and concepts introduced in\r\nhttps://github.com//pull/194810, updating plugins that\r\nwere considered to be solution-specific in Luke's\r\n[PoC](https://github.com/elastic/kibana/pull/179710).\r\n\r\nThis might trigger linting rule violations in CI, and help uncover\r\nconflicts related to forbidden dependencies.\r\nAs soon as they are resolved, we can proceed to classify solutions'\r\nplugins.","sha":"a5517d9d2cf38369fc46ea9622ce36c768436aad"}}]}] BACKPORT-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Addresses https://github.com/elastic/kibana-team/issues/1175
As part of the Sustainable Kibana Architecture initiative, this PR sets the foundation to start classifying plugins in isolated groups, matching our current solutions / project types:
It adds support for the following fields in the packages' manifests (kibana.jsonc):
group?: 'search' | 'security' | 'observability' | 'platform' | 'common'visibility?: 'private' | 'shared'It proposes a folder structure to automatically infer groups:
group: 'common', visibility: 'shared'by default. As soon as we specify a customgroup, the ESLINT rules will check violations against dependencies / dependants.The ESLINT rules are pretty simple:
'shared'visibility'observability', 'security', 'search'groups are mandatorily'private'.