-
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
[NP] Migrate logstash server side code to NP #63135
[NP] Migrate logstash server side code to NP #63135
Conversation
Pinging @elastic/kibana-platform (Team:Platform) |
I'm surprised there is no code-owner even for the LP part, so I tagged @ycombinator directly. |
Hey @restrry, looks like some tests are still failing on this PR. Did you want to fix them first before I start reviewing or should I go ahead? |
@ycombinator as I can see, those failed tests aren't related to the changes. I'm going to take a look at them tomorrow when I'm back. It doesn't block a review I believe |
ack: will review today |
"showLinks": false, | ||
"showLogin": false, | ||
"showRoleMappingsManagement": false, | ||
"allowLogin": true, |
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 added default values to the getFeature
mock led to these changes.
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.
Security changes LGTM.
x-pack/plugins/licensing/server/licensing_checker_route_handler_wrapper.test.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/licensing/server/license_checker_route_handler_wrapper.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/licensing/server/licensing_checker_route_handler_wrapper.test.ts
Outdated
Show resolved
Hide resolved
I've tested the basic functionality of the Logstash Central Management UI with this PR and compared it with running I'm now going to test the slightly more advanced setup that requires the Logstash Central Management UI reading data from the Stack Monitoring plugin. |
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've tested the slightly more advanced setup where the Logstash Central Management plugin reads data from the Stack Monitoring plugin and it also works as expected 👍.
Functionally, this PR LGTM. Thanks for migrating this plugin to NP, @restrry!
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.
LGTM
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* convert api_integration test into TS * create logstash NP plugin and move models * move common/constants to NP * type fetch all from scroll * move route declaration to NP * add licence checker wrapper * register logstash route handlers in NP * track logstash NP i18n * address shaunak comment * fix validation * udpdate security tests since for new mock defaults * address Pierres comments * rename upgrade file route
* convert api_integration test into TS * create logstash NP plugin and move models * move common/constants to NP * type fetch all from scroll * move route declaration to NP * add licence checker wrapper * register logstash route handlers in NP * track logstash NP i18n * address shaunak comment * fix validation * udpdate security tests since for new mock defaults * address Pierres comments * rename upgrade file route
* master: (29 commits) Add test:jest_integration npm script (elastic#62938) [data.search.aggs] Remove service getters from agg types (AggConfig part) (elastic#62548) [Discover] Fix broken setting of bucketInterval (elastic#62939) Disable adding conditions when in alert management context. (elastic#63514) [Alerting] fixes to allow pre-configured actions to be executed (elastic#63432) adding useMemo (elastic#63504) [Maps] fix double fetch when filter pill is added (elastic#63024) [Lens] Fix missing formatting bug in "break down by" (elastic#63288) [SIEM] [Cases] Removed double pasted line (elastic#63507) [Reporting] Improve functional test steps (elastic#63259) [SIEM][CASE] Tests for server's configuration API (elastic#63099) [SIEM] [Cases] Case container unit tests (elastic#63376) [ML] Improving parsing of large uploaded files (elastic#62970) [ML] Listing global calendars on the job management page (elastic#63124) [Ingest][Endpoint] Add Ingest rest api response types for use in Endpoint (elastic#63373) Add help text to form fields (elastic#63165) [ML] Converts utils Mocha tests to Jest (elastic#63132) [Metrics UI] Refactor With* containers to hooks (elastic#59503) [NP] Migrate logstash server side code to NP (elastic#63135) Clicking cancel in saved query save modal doesn't close it (elastic#62774) ...
* alerting/alert-services-mock: (107 commits) removed unused import added alert services mock and use it in siem [Metrics UI] Refactor With* containers to hooks (elastic#59503) [NP] Migrate logstash server side code to NP (elastic#63135) Clicking cancel in saved query save modal doesn't close it (elastic#62774) [Lens] Migration from 7.7 (elastic#62879) [Lens] Fix bug where suggestions didn't use filters (elastic#63293) Task/linux events (elastic#63400) [Remote clusters] guard against usageCollection plugin if unav… (elastic#63284) [Uptime] Remove pings graphql (elastic#59392) Index Pattern Field class - factor out copy_field code for future typescripting (elastic#63083) [EPM] add/remove package in package settings page (elastic#63389) Adjust API authorization logging (elastic#63350) Revert FTR: add chromium-based Edge browser support (elastic#61684) (elastic#63448) [Event Log] Adds namespace into save objects (elastic#62974) document code splitting for client code (elastic#62593) Escape single quotes surrounded by double quotes (elastic#63229) [Endpoint] Update cli mapping to match endpoint package (elastic#63372) update in-app links to metricbeat configuration docs (elastic#63295) investigation notes field (documentation / metadata) (elastic#63386) ...
* convert api_integration test into TS * create logstash NP plugin and move models * move common/constants to NP * type fetch all from scroll * move route declaration to NP * add licence checker wrapper * register logstash route handlers in NP * track logstash NP i18n * address shaunak comment * fix validation * udpdate security tests since for new mock defaults * address Pierres comments * rename upgrade file route
Summary
Part of #60937
Checklist
For maintainers