-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sustainable Kibana Architecture: Move modules owned by @elastic/kibana-security
#202748
base: main
Are you sure you want to change the base?
Sustainable Kibana Architecture: Move modules owned by @elastic/kibana-security
#202748
Conversation
A documentation preview will be available soon. Request a new doc build by commenting
If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here. |
Pinging @elastic/kibana-core (Team:Core) |
Pinging @elastic/kibana-security (Team:Security) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SCSS files only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResponseOps changes LGTM. Was just 5 files, and the only changes were in comments. Which looked ok to me.
I'm slightly awed we were able to move a bunch of these plugins we use within ResponseOps without any other changes! That change to reference other plugins via @kbn/XXX-plugin
was a huge win, for changes like this!
1eb4b47
to
33582ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core code changes LGTM.
FYI the failed jest test is a known issue, skipped in main, then re-enabled again (#202289), only to be flaky once more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM for the Threat Hunting Investigations!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsoldevila I noticed some links to specific commits that shouldn't be changed. I will push a commit to revert those.
The only other thing I noticed were discrepancies with some of the jest.config.js
files. It looks like git may have gotten confused about which of these files were moved and where. However, the end result appears to be accurate, and I don't necessarily think it is an issue to have a strange file history on these particular files, but I wanted to bring it up.
legacy_rfcs/text/0016_ols_phase_1.md
Outdated
@@ -103,7 +103,7 @@ export interface SavedObject<T = unknown> { | |||
|
|||
### 3.1.3 Saved Objects Client: Security wrapper | |||
|
|||
The [security wrapper](https://github.com/elastic/kibana/blob/701697cc4a34d07c0508c3bdf01dca6f9d40a636/x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts) authorizes and audits operations against saved objects. | |||
The [security wrapper](https://github.com/elastic/kibana/blob/701697cc4a34d07c0508c3bdf01dca6f9d40a636/x-pack/platform/plugins/shared/security/server/saved_objects/secure_saved_objects_client_wrapper.ts) authorizes and audits operations against saved objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SO wrapper no longer exists (was replaced with the extension a long while back), and IMO we don't need to update any of these legacy RFC docs.
@@ -19,7 +19,7 @@ export interface SavedObjectTypeIdTuple { | |||
type: string; | |||
} | |||
|
|||
// NOTE: moved from x-pack/plugins/spaces/common/types.ts for use by SO security ext | |||
// NOTE: moved from x-pack/platform/plugins/shared/spaces/common/types.ts for use by SO security ext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just delete this comment now. It was useful 2 years ago, but is no longer relevant.
// NOTE: moved from x-pack/platform/plugins/shared/spaces/common/types.ts for use by SO security ext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is off here - the source and destination do not match. There is a jest.config.js file in the new x-pack/platform/packages/private/security/role_management_model
folder, which looks like a new added file. It should have been moved and adjusted from the old location.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this file got mixed up with role_management_model/jest.config.js
. They both should have been moved accordingly, but instead this one was deleted, the origin of role_management_model/jest.config.js
was moved erroneously, and x-pack/platform/packages/private/security/role_management_model/jest.config.js
was created new.
@@ -345,7 +345,7 @@ functions and will be impacted: | |||
6. [code](https://github.com/elastic/kibana/blob/5049b460b47d4ae3432e1d9219263bb4be441392/x-pack/legacy/plugins/code/server/plugin.ts#L129-L149) | |||
7. [spaces](https://github.com/elastic/kibana/blob/096c7ee51136327f778845c636d7c4f1188e5db2/x-pack/legacy/plugins/spaces/server/new_platform/plugin.ts#L95) | |||
8. [licensing](https://github.com/elastic/kibana/blob/4667c46caef26f8f47714504879197708debae32/x-pack/plugins/licensing/server/plugin.ts) | |||
9. [security](https://github.com/elastic/kibana/blob/0f2324e44566ce2cf083d89082841e57d2db6ef6/x-pack/plugins/security/server/plugin.ts#L96) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to update these old legacy RFCs
@@ -343,7 +343,7 @@ functions and will be impacted: | |||
3. [vis_type_table](https://github.com/elastic/kibana/blob/6039709929caf0090a4130b8235f3a53bd04ed84/src/legacy/core_plugins/vis_type_table/public/plugin.ts#L61) | |||
4. [vis_type_vega](https://github.com/elastic/kibana/blob/6039709929caf0090a4130b8235f3a53bd04ed84/src/legacy/core_plugins/vis_type_vega/public/plugin.ts#L59) | |||
6. [code](https://github.com/elastic/kibana/blob/5049b460b47d4ae3432e1d9219263bb4be441392/x-pack/legacy/plugins/code/server/plugin.ts#L129-L149) | |||
7. [spaces](https://github.com/elastic/kibana/blob/096c7ee51136327f778845c636d7c4f1188e5db2/x-pack/legacy/plugins/spaces/server/new_platform/plugin.ts#L95) | |||
7. [spaces](https://github.com/elastic/kibana/blob/096c7ee51136327f778845c636d7c4f1188e5db2/x-pack/legacy/platform/plugins/shared/spaces/server/new_platform/plugin.ts#L95) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: No need to update the legacy RFCs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few of the jest.config.js
files are in this state, where they show up as having been moved from one domain to another (security/ui_components to spaces), and corrected. I don't think this is really consequential if the end result is correct.
@@ -51,7 +51,7 @@ export const getWarningHeader = ( | |||
|
|||
/** | |||
* Taken from | |||
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 | |||
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/platform/plugins/shared/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link should remain as it was, since it points to a specific commit.
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/platform/plugins/shared/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 | |
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 |
@@ -8,7 +8,7 @@ import type { Headers } from '@kbn/core/server'; | |||
|
|||
/** | |||
* Taken from | |||
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 | |||
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/platform/plugins/shared/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original link should remain, as it references a specific commit.
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/platform/plugins/shared/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 | |
* https://github.com/elastic/kibana/blob/ec30f2aeeb10fb64b507935e558832d3ef5abfaa/x-pack/plugins/spaces/server/usage_stats/usage_stats_client.ts#L113-L118 |
⏳ Build in-progress, with failures
Failed CI Steps
Test Failures
History
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gsoldevila I'm not exactly sure why @elastic/obs-ux-management-team is being requested on this review as I don't see it touching any files that belong to us, but because we're listed here, I'll add a cursory approval (I've not looked at this PR at all beyond checking the "File filter" in the files tab to see if any files belong to our team, and none do)
@jeramysoucy I'm re-running the relocation logic against this PR, it'll pick up manual changes. |
Summary
This PR aims at relocating some of the Kibana modules (plugins and packages) into a new folder structure, according to the Sustainable Kibana Architecture initiative.
Important
packages[\/\\]
andplugins[\/\\]
patterns in the source code (Babel and Eslint config files), and update them appropriately..buildkite/scripts/pipelines/pull_request/pipeline.ts
to ensure that any CI pipeline customizations continue to be correctly applied after the changed path names.ts
and.js
files listed in the sections below, as some of them contain relative paths that have been updated.Note
This PR has been auto-generated.
Do not attempt to push any changes unless you know what you are doing.
Please use #sustainable_kibana_architecture Slack channel for feedback.
4 plugin(s) are going to be relocated:
@kbn/encrypted-saved-objects-plugin
x-pack/platform/plugins/shared/encrypted_saved_objects
@kbn/interactive-setup-plugin
src/platform/plugins/private/interactive_setup
@kbn/security-plugin
x-pack/platform/plugins/shared/security
@kbn/spaces-plugin
x-pack/platform/plugins/shared/spaces
14 package(s) are going to be relocated:
@kbn/crypto
src/platform/packages/shared/kbn-crypto
@kbn/handlebars
src/platform/packages/private/kbn-handlebars
@kbn/safer-lodash-set
src/platform/packages/shared/kbn-safer-lodash-set
@kbn/security-api-key-management
x-pack/platform/packages/shared/security/api_key_management
@kbn/security-authorization-core
x-pack/platform/packages/private/security/authorization_core
@kbn/security-authorization-core-common
x-pack/platform/packages/private/security/authorization_core_common
@kbn/security-form-components
x-pack/platform/packages/shared/security/form_components
@kbn/security-hardening
src/platform/packages/shared/kbn-security-hardening
@kbn/security-plugin-types-common
x-pack/platform/packages/shared/security/plugin_types_common
@kbn/security-plugin-types-public
x-pack/platform/packages/shared/security/plugin_types_public
@kbn/security-plugin-types-server
x-pack/platform/packages/shared/security/plugin_types_server
@kbn/security-role-management-model
x-pack/platform/packages/private/security/role_management_model
@kbn/security-ui-components
x-pack/platform/packages/private/security/ui_components
@kbn/user-profile-components
src/platform/packages/shared/kbn-user-profile-components
Updated references
Updated relative paths
Script errors