[Cross Cluster Replication] NP Shim by jloleysens · Pull Request #60121 · elastic/kibana

jloleysens · 2020-03-13T16:47:33Z

Summary

Initial shimming of Cross Cluster Replication.

Future Work (for move out of legacy)

Proper handling of mounting the react application
Proper handling of license data and license route protection

Checklist

Fix unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

Updated the client code to properly encode requests to the server. Did first E2E test. Route tests are probably broken, need to fix them.

elasticmachine · 2020-03-13T16:47:35Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

alisonelizabeth

Thanks for working on this @jloleysens!

I left a couple minor comments in the code. I also found two bugs that I think need to be addressed.

I cannot edit a follower index and get the following error:

It looks like the Follower badge is missing in Index Management. I checked master, and it is working as expected, so I believe there was a regression in this PR.

alisonelizabeth · 2020-03-16T13:26:32Z

...plugins/cross_cluster_replication/public/np_ready/app/components/auto_follow_pattern_form.js

 import { extractQueryParams } from '../services/query_params';
 import { getRemoteClusterName } from '../services/get_remote_cluster_name';
-import { API_STATUS } from '../constants';
+import { API_STATUS, INDEX_ILLEGAL_CHARACTERS_VISIBLE } from '../constants';


I think you should be able to import INDEX_ILLEGAL_CHARACTERS_VISIBLE once #60186 is merged.

Yeah I saw this guy coming up! I'll resolve once it is merged.

alisonelizabeth · 2020-03-16T13:37:02Z

x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/services/breadcrumbs.ts

+import { ManagementAppMountParams } from '../../../../../../../../src/plugins/management/public';
+
+// @ts-ignore
+import { BASE_PATH } from '../../../../common/constants';


is this @ts-ignore needed?

Nope! Placeholder from when it was a JS file!

alisonelizabeth · 2020-03-16T13:38:39Z

...legacy/plugins/cross_cluster_replication/public/np_ready/app/services/documentation_links.ts

+  esBase = `${ELASTIC_WEBSITE_URL}guide/en/elasticsearch/reference/${DOC_LINK_VERSION}`;
+};
+
+export const autoFollowPatternUrl = () => `${esBase}/ccr-put-auto-follow-pattern.html`;


nit: now that these are functions, maybe consider renaming to use a verb, e.g., getAutoFollowPatternUrl?

Great point!

alisonelizabeth · 2020-03-16T13:43:10Z

...legacy/plugins/cross_cluster_replication/public/np_ready/app/services/documentation_links.ts

+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+let esBase: () => string;


Suggested change

let esBase: () => string;

let esBase: string;

alisonelizabeth · 2020-03-16T14:15:53Z

.../app/components/auto_follow_pattern_action_menu/auto_follow_pattern_action_menu.container.ts


 // @ts-ignore
-import { pauseAutoFollowPattern, resumeAutoFollowPattern } from '../../store/actions';
+import { pauseAutoFollowPattern, resumeAutoFollowPattern } from '../../store/actions/index';


is this change necessary?

And by 👍 I mean no of course 😄

Also migrated to the a more NP friendly version of index mgmt extension.

jloleysens · 2020-03-16T16:38:48Z

@elasticmachine merge upstream

jloleysens · 2020-03-16T16:47:36Z

@alisonelizabeth Thanks for taking a look!

I cannot edit a follower index and get the following error:

Hmm, curious, I remember seeing this on the create and then I changed the request body to be a string. Perhaps I hadn't pushed that change - I am able to create and edit auto follow patterns and create follower indices (but not edit them, it looks disallowed by the UI). Would you mind taking another look?

It looks like the Follower badge is missing in Index Management...

Very well caught! It looks like I introduced a logic regression for the data enricher on the server side! Just pushed a fix 👍

alisonelizabeth · 2020-03-16T17:10:10Z

@jloleysens thanks for fixing the issue with the badge, looks good now!

I am able to create and edit auto follow patterns and create follower indices (but not edit them, it looks disallowed by the UI).

What do you mean by "it looks disallowed"? Maybe we have something set up differently? It is possible on my end, and I am still seeing the same error that I mentioned above. As you said, create/edit auto-follow patterns works as expected.

jloleysens · 2020-03-16T18:06:58Z

@alisonelizabeth Ah ok I see, I was referring to the the greyed out text fields, but I see there is an advanced section that can be edited.

alisonelizabeth

Latest changes LGTM. Thanks @jloleysens!

jloleysens · 2020-03-17T07:02:18Z

@elasticmachine merge upstream

Folder called __tests__ with Jest tests was breaking mocha.

jloleysens · 2020-03-18T09:07:17Z

Waiting for #60186 to merge, will need to clean up duplication of ui/indices code

cjcenizal · 2020-03-18T20:12:17Z

@jloleysens I just merged #60186

…-cluster-replication * 'master' of github.com:elastic/kibana: (89 commits) Sort by name when fetching alerts and connectors (elastic#60506) Make slack param validation handle empty messages (elastic#60468) [Alerting] Adds navigation by consumer and alert type to alerting (elastic#58997) Introduce search interceptor (elastic#60523) [ML] Add functional tests for file data visualizer (elastic#60413) [APM] Optimize service map query (elastic#60412) [SIEM][Detection Engine] Adds lists feature flag and list values to the REST interfaces Enhancement/update esdocs datasource (elastic#59512) [junit] only include stdout in report for failures (elastic#60530) Update dependency nock to v12 (elastic#60422) upgrade execa to get stdout/stderr in error messages (elastic#60537) skip flaky suite (elastic#60471) [Ingest] Agent Config Details - Data sources list ui (elastic#60429) [SIEM] Create ML Rules (elastic#58053) skip flaky suite (elastic#60559) fix agent type (elastic#60554) Fixed default message for index threshold includes both threshold values (elastic#60545) [Ingest] Add support for `yaml` field types (elastic#60440) Solved the issue for a GROUP BY expression validation (elastic#60558) [Maps] Mark instance state as readonly (elastic#60557) ... # Conflicts: # x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/components/auto_follow_pattern_form.js # x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/components/follower_index_form/follower_index_form.js # x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/components/follower_index_form/follower_index_form.test.js # x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/services/auto_follow_pattern_validators.js # x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/services/input_validation.js

kibanamachine · 2020-03-19T14:40:57Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b4bd35f

History

💚 Build #33830 succeeded 3cbb917
💔 Build #33749 failed ca2cd89
💔 Build #33716 failed a38eeb4
💔 Build #33707 failed 50430e7
💔 Build #33542 failed 49631db

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Public in WiP state, removed all 'ui/' imports * First iteration of public shimmed and working * A whole lotta WIP server side * Server-side to using the NP router + client side changes Updated the client code to properly encode requests to the server. Did first E2E test. Route tests are probably broken, need to fix them. * Removed unused error wrapping code * Update client Jest tests * Add breadcrumbs service mock * Fix server side Jest tests * Add helper functions file for server side Jest tests * Fix API integration tests * Fixed boolean logic mistake in due to refactor in index mgmt ext. Also migrated to the a more NP friendly version of index mgmt extension. * Remove unused import * Clean up some cruft and refactor URL variable names * Fix stringification of body and fix boolean server logic * Fix mocha Folder called __tests__ with Jest tests was breaking mocha. * Refactor to Jest test * Fix types issues in jest test * Migrate to new config-schema API Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* Public in WiP state, removed all 'ui/' imports * First iteration of public shimmed and working * A whole lotta WIP server side * Server-side to using the NP router + client side changes Updated the client code to properly encode requests to the server. Did first E2E test. Route tests are probably broken, need to fix them. * Removed unused error wrapping code * Update client Jest tests * Add breadcrumbs service mock * Fix server side Jest tests * Add helper functions file for server side Jest tests * Fix API integration tests * Fixed boolean logic mistake in due to refactor in index mgmt ext. Also migrated to the a more NP friendly version of index mgmt extension. * Remove unused import * Clean up some cruft and refactor URL variable names * Fix stringification of body and fix boolean server logic * Fix mocha Folder called __tests__ with Jest tests was breaking mocha. * Refactor to Jest test * Fix types issues in jest test * Migrate to new config-schema API Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: [ML] Use a new ML endpoint to estimate a model memory (elastic#60376) [Logs UI] Correctly update the expanded log rate table rows (elastic#60306) fixes drag and drop flakiness (elastic#60625) Removing isEmptyState from embeddable input (elastic#60511) [Cross Cluster Replication] NP Shim (elastic#60121) Clear changes when canceling an edit to an alert (elastic#60518) Update workflow syntax (elastic#60626) Updating project assigner workflows to v2.0.0 of the action and back to default tokens (elastic#60577) migrate saved objects management edition view to react/typescript/eui (elastic#59490)

jloleysens added 5 commits March 12, 2020 18:34

Public in WiP state, removed all 'ui/' imports

a451347

First iteration of public shimmed and working

68577fe

A whole lotta WIP server side

0b1d330

Server-side to using the NP router + client side changes

891ffa2

Updated the client code to properly encode requests to the server. Did first E2E test. Route tests are probably broken, need to fix them.

Removed unused error wrapping code

7d8d0cc

jloleysens added Feature:CCR and Remote Clusters v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 13, 2020

jloleysens requested a review from alisonelizabeth March 13, 2020 16:47

jloleysens added 4 commits March 16, 2020 15:16

Update client Jest tests

7030891

Add breadcrumbs service mock

75f79c7

Fix server side Jest tests

430b491

Add helper functions file for server side Jest tests

be87bab

alisonelizabeth requested changes Mar 16, 2020

View reviewed changes

jloleysens added 3 commits March 16, 2020 17:08

Fix API integration tests

c602fba

Fixed boolean logic mistake in due to refactor in index mgmt ext.

37e4b8c

Also migrated to the a more NP friendly version of index mgmt extension.

Remove unused import

4bf83cf

elasticmachine and others added 2 commits March 16, 2020 12:38

Merge branch 'master' into np-shim/cross-cluster-replication

a77999c

Clean up some cruft and refactor URL variable names

390dc6b

Fix stringification of body and fix boolean server logic

49631db

alisonelizabeth approved these changes Mar 16, 2020

View reviewed changes

Merge branch 'master' into np-shim/cross-cluster-replication

50430e7

jloleysens added 3 commits March 17, 2020 09:34

Fix mocha

a38eeb4

Folder called __tests__ with Jest tests was breaking mocha.

Refactor to Jest test

ca2cd89

Fix types issues in jest test

3cbb917

jloleysens added 2 commits March 19, 2020 13:27

Migrate to new config-schema API

b4bd35f

jloleysens merged commit 254cf99 into elastic:master Mar 19, 2020

jloleysens deleted the np-shim/cross-cluster-replication branch March 19, 2020 14:42

jloleysens mentioned this pull request Mar 19, 2020

[7.x] [Cross Cluster Replication] NP Shim (#60121) #60635

Merged

Conversation

jloleysens commented Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Future Work (for move out of legacy)

Checklist

For maintainers

Uh oh!

elasticmachine commented Mar 13, 2020

Uh oh!

alisonelizabeth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jloleysens commented Mar 16, 2020

Uh oh!

jloleysens commented Mar 16, 2020

Uh oh!

alisonelizabeth commented Mar 16, 2020

Uh oh!

jloleysens commented Mar 16, 2020

Uh oh!

alisonelizabeth left a comment

Choose a reason for hiding this comment

Uh oh!

jloleysens commented Mar 17, 2020

Uh oh!

jloleysens commented Mar 18, 2020

Uh oh!

cjcenizal commented Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Mar 19, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jloleysens commented Mar 13, 2020 •

edited

Loading

cjcenizal commented Mar 18, 2020 •

edited

Loading