Move CCR out of legacy #62890

cjcenizal · 2020-04-08T00:10:52Z

Other changes

Implemented [CCR] Tell user when multiple auto-follow patterns try to replicate the same data #27783 (review), which suggested we render the timestamp of auto-follow pattern errors (screenshots below).
Localized mocks with client integration tests.
Refactored API unit tests to use inline mocks instead of external fixture files, to make it easier to read and understand the assertions being made. See f6adfaf for the diff.
Incorporated bugfix from Add body validation to update follower index API endpoint. #63653.

Reviewing

Reviewing each commit individually might be easier than reviewing the end result. I tried to make each one as atomic as possible, but there may be some slight overlap as I ended up catching and fixing some errors in the later commits.

Testing

Setup

You can run a local cluster and simulate a remote cluster within a single Kibana directory.

Run yarn es snapshot --license=trial and kill the process once the snapshot has been installed.
Duplicate the ES installation by running cp -aR .es/8.0.0 .es/8.0.0-2.
Start your "local" cluster by running .es/8.0.0/bin/elasticsearch and starting Kibana.
Start your "remote" cluster by running .es/8.0.0-2/bin/elasticsearch -E cluster.name=europe -E transport.port=9400.
Index a document into your remote cluster by running curl -X PUT http://elastic:changeme@localhost:9201/my-leader-index --data '{"settings":{"number_of_shards":1,"soft_deletes.enabled":true}}' --header "Content-Type: application/json". Note that these settings are required for testing auto-follow pattern conflicts errors (see below).

Now you can create follower indices and auto-follow patterns to replicate the my-leader-index index on the remote cluster that's available at 127.0.0.1:9400.

Settings

xpack.ccr.ui.enabled: false

With the above setting, verify that CCR isn't visible in the UI and you can't see a "Follower" badge next to follower indices in Index Management.

xpack.remote_clusters.ui.enabled

With the above setting, verify the same effect.

xpack.index_management.enabled

With the above setting, verify the same effect.

xpack.ccr.enabled: false

With the above setting, verify the same effect.

License

Verify that an expired license hides CCR in the navigation.
Verify that a basic license hides CCR in the navigation.

Permissions

Create a user with just a kibana_user role and verify that a permission error callout is shown (per #27273).

Security

Disable security by setting xpack.security.enabled: false in the elastiscearch.yml file of your local cluster and ensure you can still access CCR (per #35333).

Actions

Create, edit, pause, resume, view the detail panel, and delete a single auto-follow pattern.
Bulk pause, resume, and delete multiple auto-follow patterns.
Create, edit, pause, resume, view the detail panel, and unfollow a single follower index
Bulk pause, resume, and unfollow multiple follower indices.

Auto-follow pattern conflict errors

Create two auto-follow patterns with overlapping patterns (e.g. my* and my-*) that will both capture the my-leader-index index on your remote cluster.
Run the curl command to create my-leader-index2 on your remote cluster, since auto-follow patterns don't replicate existing indices.
Open the details flyout of one of the auto-follow patterns and verify there is a list of recent errors.

Partial errors

Create two follower indices.
Open Index Management in another tab and delete one of them.
In the first tab, try to perform a bulk action on both of the (pause, resume, or unfollow).
A danger toast should appear notifying you there was an error with the one you deleted.

Missing entities

Navigate to a nonexistent auto-follow pattern and verify the UI shows an error callout.
Navigate to a nonexistent follower index and verify the UI shows an error callout.
Open a detail flyout of an auto-follow pattern and then edit the URL so that it loads one that doesn't exist. Verify it shows a "not found" message (screenshot below).
Open a detail flyout of a follower index and then edit the URL so that it loads one that doesn't exist. Verify it shows a "not found" message (screenshot below).

elasticmachine · 2020-04-08T00:11:10Z

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

- Remove deserializeAutoFollowPattern behavior that returned an empty object if the pattern was undefined.

- Localize mocks with the component integration tests.

- Remove security dependency. - Render timestamp for autofollow errors. - Fix license check so that CCR won't render if the license is invalid. - Fix server security check to be more precise by checking if ES has security disabled.

cjcenizal · 2020-04-20T00:19:21Z

@elasticmachine merge upstream

sebelga · 2020-04-20T09:06:53Z

Just a note: I don't think the test plan in Testrail has all the usecases mentioned in this PR description.
It might be a good idea to add a link inside the Testrail CCR plan description that points to this PR.

Back to our conversation from last week, we should have a matrix of what can and what cannot be automated. The "Security" part will have to be manual for sure. But almost all the rest could/should be automated.

e.g.

Component integration test: Test that the expected request URL and method is called
Component integration test: Fake the HTTP response with a 404 status and have a test that verifies that there is a callout in the UI
API integration test: call the same URL + method and expect a 404

We just automated the "Missing entities" part of the testing.
This also implies that we have a single source of truth to declare our API endpoints. See cjcenizal#24

cc @cuff-links

jloleysens

Great work @cjcenizal , thanks for cleaning up the server side code!

Most of my comments do not need to be blocked on, however;

I would highly recommend make the setup functions synchronous however, this is something we will have to do in the future at any rate.
Would you mind answering my q's below before I approve?

Running through your tests I noted the following:

Auto-follow patterns

Open the details flyout of one of the auto-follow patterns and verify there is a list of recent errors.

I was not able to see the recent errors you mentioned here 👆🏻. Probably a user error on my part. I did create a follower index before testing this, would that have affected it? Would appreciate you confirming this

Partial errors

...danger toast should appear notifying you there was an error with the one you deleted.

I saw a success notification after performing the bulk update here (not the error notification I expected)

x-pack/plugins/cross_cluster_replication/public/app/app.tsx

jloleysens · 2020-04-20T11:12:00Z

x-pack/plugins/cross_cluster_replication/public/app/index.tsx

+  DOC_LINK_VERSION: string;
+}): Promise<UnmountCallback> {
+  // Initialize additional services.
+  initBreadcrumbs(setBreadcrumbs);


For consistency across our plugins, could we move these init calls to the plugin.setup function?

I moved them here based on guidance in #62487 (comment). The gist that I got was that we should try to exclude as many imports from the main bundle as possible, to reduce its size. I'll add a comment to the code to clarify this.

Hm, I see what you're saying. I think the merit (in KB) of importing here vs. plugin is very small. But we can leave as is for now

Agreed! It's negligible. But it's also a negligible effort on my part, and the difference in readability isn't worth a debate IMO. :)

jloleysens · 2020-04-20T11:17:00Z

x-pack/plugins/cross_cluster_replication/public/plugin.ts

+          initUiMetric(usageCollection);
+          initNotification(toasts, fatalErrors);
+
+          management.sections.getSection('elasticsearch')!.registerApp({


We should keep all registration functionality synchronous. See x-pack/plugins/watcher/public/plugin.ts for an instance of using license but keeping registration sync.

jloleysens · 2020-04-20T11:20:05Z

x-pack/plugins/cross_cluster_replication/server/plugin.ts

+      },
+    });
+
+    // The UI is also dependent upon the Remote Clusters UI.


Not sure I follow this comment, I'm guessing we needed to add Remote Clusters UI config setting?

The remote_clusters.ui.enabled config setting is what drives the value remoteClusters.isUiEnabled. It sounds like that's not clear? My intention for this comment was to explain that we care about this setting's value because the actual CCR UI cross-links to the Remote Clusters UI, so it won't function without it. I will add these details to the comment but let me know if there's anything else that isn't clear.

Ah, sorry, I understand now - user error :)

jloleysens · 2020-04-20T11:21:06Z

x-pack/plugins/cross_cluster_replication/server/plugin.ts

+    { licensing, indexManagement, remoteClusters }: Dependencies
+  ): Promise<void> {
+    const router = http.createRouter();
+    const config = await this.config$.pipe(first()).toPromise();


We can update this to this.config$.pipe(first()).toPromise().then(() => /* do the things */) to keep setup synchronous

jloleysens · 2020-04-20T11:27:56Z

...cross_cluster_replication/server/routes/api/auto_follow_pattern/register_pause_route.test.ts

+    const mockRouteContext = ({
+      crossClusterReplication: {
+        client: {
+          callAsCurrentUser: jest


Nit; this seems like a pattern that would benefit from being slightly more terse by creating a helper for creating the route context.

jloleysens · 2020-04-20T11:39:02Z

.../plugins/cross_cluster_replication/server/routes/api/follower_index/register_update_route.ts

+      },
+    },
+    license.guardApiRoute(async (context, request, response) => {
+      const { id } = request.params as typeof paramsSchema.type;


nit; the license guardApiRoute is missing the <P, Q, B> type annotation. See x-pack/plugins/painless_lab/server/services/license.ts. Then we can remove all of the as typeof declarations.

nit 2: I personally think it is easier to read like this. I agree there are a few more chars (as typeof), but the overall handler is easier to parse for me at least.

Hm, I think there are a few issues with using the as keyword (verbosity is one of them, another being taking on the work of the type system - which can lead to unintended side effects). I would really recommend not using unless necessary.

Thank you both for your input here. I think the connection between request.params and paramsSchema is not too hard to make, considering we define params: paramsSchema in the validation config a few lines above. So I'm going to go ahead and apply JL's suggestion.

jloleysens · 2020-04-20T11:39:58Z

x-pack/plugins/cross_cluster_replication/server/types.ts

+export interface RouteDependencies {
+  router: IRouter;
+  license: License;
+  lib: {


cchaos

I don't see the index.scss being imported anywhere. Maybe the styles aren't all that necessary? Or you need to import it inside plugin.ts

cjcenizal · 2020-04-21T17:49:28Z

@elasticmachine merge upstream

- Import and use History type. - Make public and server plugin setup functions synchronous. - Add comments to explain role of remote_clusters.ui.enabled setting and purpose for initializing services inside of the app mount function.

cjcenizal · 2020-04-21T20:44:36Z

@cchaos @jloleysens I've addressed your feedback. Could you please take another look? Caroline, I ended up removing the styles.

cjcenizal · 2020-04-21T20:50:14Z

@jloleysens

Auto-follow patterns

Sorry, I should have clarified in the instructions that you need to index the document in the remote cluster after setting up the auto-follow patterns, since they won't capture existing documents. I just tested locally and verified. I also updated the instructions.

Partial errors

Can you confirm that you were attempting to delete a non-existing follower index? I.e., one that you had already deleted via Index Management or Console? Also, there is a 30 second refresh interval which you might be racing against. If this interval triggers a refresh, the list will refresh and it might appear as if there was no error.

cchaos

👍 Awesome. Deletions are always good :)

kibanamachine · 2020-04-22T01:51:43Z

💚 Build Succeeded

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

History

💚 Build #42281 succeeded 4baad16
💚 Build #42229 succeeded 5aa2947
💚 Build #41771 succeeded 640b712
💚 Build #41756 succeeded 934a7b2
💔 Build #41753 failed af76d67

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

jloleysens

Thanks for the updates to the PR description (as Seb mentioned, definitely a good source of testing information when we review our current automated vs. manual testing).

I trust you have tested the two points I had questions about, and am happy this contribution to be merged.

jloleysens · 2020-04-22T07:55:13Z

x-pack/plugins/cross_cluster_replication/public/app/index.tsx

+  DOC_LINK_VERSION: string;
+}): Promise<UnmountCallback> {
+  // Initialize additional services.
+  initBreadcrumbs(setBreadcrumbs);


Hm, I see what you're saying. I think the merit (in KB) of importing here vs. plugin is very small. But we can leave as is for now

jloleysens · 2020-04-22T07:57:23Z

x-pack/plugins/cross_cluster_replication/server/plugin.ts

+      },
+    });
+
+    // The UI is also dependent upon the Remote Clusters UI.


Ah, sorry, I understand now - user error :)

cjcenizal · 2020-04-22T14:13:18Z

as Seb mentioned, definitely a good source of testing information when we review our current automated vs. manual testing

Totally! I've shared this PR with @cuff-links and he'll update TestRail with the testing information.

* Convert common/services and server/lib to TypeScript. Update Jest tests. - Remove deserializeAutoFollowPattern behavior that returned an empty object if the pattern was undefined. * Localize mocks with the component integration tests. * Update API unit tests to use NP mocks. - Break up test files. - Use inline mocked ES response instead of fixture files. - Move remaining fixture files into client integration tests directory. * Make API route validation more strict. * Publish isUiDisabled as part of Remote Clusters contract. * Default trackUiMetric service to be a no-op. * Remove security dependency. - Fix license check so that CCR won't render if the license is invalid. - Fix server security check to be more precise by checking if ES has security disabled. * Render timestamp for autofollow errors.

cjcenizal added chore Feature:CCR and Remote Clusters Feature:New Platform Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.8.0 v8.0.0 labels Apr 8, 2020

cjcenizal added the release_note:skip Skip the PR/issue when compiling release notes label Apr 8, 2020

elastic deleted a comment from kibanamachine Apr 10, 2020

cjcenizal force-pushed the np/ccr-out-of-legacy branch from 46405cb to 7beacfb Compare April 10, 2020 02:14

cjcenizal changed the title ~~Move CCR out of legacy~~ [skip-ci] Move CCR out of legacy Apr 10, 2020

cjcenizal force-pushed the np/ccr-out-of-legacy branch 5 times, most recently from 6313a9c to 12a3bb5 Compare April 14, 2020 05:01

mattkime mentioned this pull request Apr 14, 2020

[New Platform Migration]: Management - Shim apps #47431

Closed

19 tasks

cjcenizal force-pushed the np/ccr-out-of-legacy branch 2 times, most recently from be75026 to 77ffce5 Compare April 15, 2020 21:01

cjcenizal mentioned this pull request Apr 16, 2020

Add body validation to update follower index API endpoint. #63653

Merged

cjcenizal force-pushed the np/ccr-out-of-legacy branch 3 times, most recently from c0f8311 to e53aa4f Compare April 16, 2020 04:24

cjcenizal added Feature:NP Migration and removed Feature:New Platform labels Apr 16, 2020

cjcenizal added 5 commits April 17, 2020 14:02

Move CCR to plugins directory.

b9adddf

Migrate server to NP.

043496d

Update plugin paths in sasslint and eslint config files.

ce4693b

Convert common/services and server/lib to TypeScript. Update Jest tests.

ed6673f

- Remove deserializeAutoFollowPattern behavior that returned an empty object if the pattern was undefined.

Migrate public to NP.

343f9c0

- Localize mocks with the component integration tests.

Clean up permissions, security, and stats logic.

1daf9ad

- Remove security dependency. - Render timestamp for autofollow errors. - Fix license check so that CCR won't render if the license is invalid. - Fix server security check to be more precise by checking if ES has security disabled.

cjcenizal force-pushed the np/ccr-out-of-legacy branch from aebaae9 to 1daf9ad Compare April 19, 2020 06:11

cjcenizal added 3 commits April 19, 2020 08:24

Fix API integration tests.

af76d67

Fix i18n.

8654d34

Fix index management API integration tests.

934a7b2

Merge branch 'master' into np/ccr-out-of-legacy

640b712

sebelga mentioned this pull request Apr 20, 2020

Bug/update follower index validation cr cjcenizal/kibana#24

Open

jloleysens reviewed Apr 20, 2020

View reviewed changes

cchaos suggested changes Apr 20, 2020

View reviewed changes

elasticmachine and others added 5 commits April 21, 2020 13:49

Merge branch 'master' into np/ccr-out-of-legacy

5aa2947

Address JL's feedback:

032b844

- Import and use History type. - Make public and server plugin setup functions synchronous. - Add comments to explain role of remote_clusters.ui.enabled setting and purpose for initializing services inside of the app mount function.

Remove use of 'as' in route handler definitions.

f63f03c

Extract mockRouteContext into a helper.

11e117b

Remove unnecessary styles.

4baad16

cchaos approved these changes Apr 21, 2020

View reviewed changes

Migrate kibana/server dependencies to src/core/server.

a661666

jloleysens approved these changes Apr 22, 2020

View reviewed changes

cjcenizal merged commit 91f7911 into elastic:master Apr 22, 2020

cjcenizal deleted the np/ccr-out-of-legacy branch April 22, 2020 14:15

cjcenizal mentioned this pull request Apr 22, 2020

[7.x] Move CCR out of legacy (#62890) #64191

Merged

cjcenizal mentioned this pull request May 26, 2020

Add steps for quickly testing to the READMEs of ILM, Rollup, and CCR. #67213

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move CCR out of legacy #62890

Move CCR out of legacy #62890

cjcenizal commented Apr 8, 2020 •

edited

Loading

elasticmachine commented Apr 8, 2020

cjcenizal commented Apr 20, 2020

sebelga commented Apr 20, 2020 •

edited

Loading

jloleysens left a comment

jloleysens Apr 20, 2020

cjcenizal Apr 21, 2020 •

edited

Loading

jloleysens Apr 22, 2020 •

edited

Loading

cjcenizal Apr 22, 2020

jloleysens Apr 20, 2020

jloleysens Apr 20, 2020

cjcenizal Apr 21, 2020

jloleysens Apr 22, 2020

jloleysens Apr 20, 2020

jloleysens Apr 20, 2020

jloleysens Apr 20, 2020

sebelga Apr 20, 2020

jloleysens Apr 21, 2020

cjcenizal Apr 21, 2020

jloleysens Apr 20, 2020

cchaos left a comment

cjcenizal commented Apr 21, 2020

cjcenizal commented Apr 21, 2020

cjcenizal commented Apr 21, 2020 •

edited

Loading

cchaos left a comment

kibanamachine commented Apr 22, 2020

jloleysens left a comment

jloleysens Apr 22, 2020 •

edited

Loading

jloleysens Apr 22, 2020

cjcenizal commented Apr 22, 2020

Move CCR out of legacy #62890

Move CCR out of legacy #62890

Conversation

cjcenizal commented Apr 8, 2020 • edited Loading

Other changes

Reviewing

Testing

Setup

Settings

License

Permissions

Security

Actions

Auto-follow pattern conflict errors

Partial errors

Missing entities

elasticmachine commented Apr 8, 2020

cjcenizal commented Apr 20, 2020

sebelga commented Apr 20, 2020 • edited Loading

jloleysens left a comment

Choose a reason for hiding this comment

Auto-follow patterns

Partial errors

Choose a reason for hiding this comment

cjcenizal Apr 21, 2020 • edited Loading

Choose a reason for hiding this comment

jloleysens Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

cjcenizal commented Apr 21, 2020

cjcenizal commented Apr 21, 2020

cjcenizal commented Apr 21, 2020 • edited Loading

Auto-follow patterns

Partial errors

cchaos left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 22, 2020

💚 Build Succeeded

History

jloleysens left a comment

Choose a reason for hiding this comment

jloleysens Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal commented Apr 22, 2020

cjcenizal commented Apr 8, 2020 •

edited

Loading

sebelga commented Apr 20, 2020 •

edited

Loading

cjcenizal Apr 21, 2020 •

edited

Loading

jloleysens Apr 22, 2020 •

edited

Loading

cjcenizal commented Apr 21, 2020 •

edited

Loading

jloleysens Apr 22, 2020 •

edited

Loading