Add uri decode to es_ui_shared and fix navigation issues with special characters #80835

yuliacech · 2020-10-16T14:45:04Z

Summary

Fixes #80661
This PR fixes several issues when navigating to Index Management pages with special characters in names:

viewing data stream in detail flyout by clicking on its name in the table
navigating to backing indices of a data stream (opens indices tab with data stream filter activated)
editing and saving an index template
navigating to Index Management from ILM filtering indices managed by a policy

Code changes

Double uri encoding (encodeURI + encodeURIComponent) for all links containing special characters (especially with dynamic resource names)
All components getting match params from react router need attemptToURIDecode to safely decode values.
When using filter in tables, its value needs to be enclosed in double quotes to be safely used with special characters (data_stream="%my_data_stream")
Encoding data stream name when sending ES requests

How to test

Data streams

Dev Tools commands

PUT /_ilm/policy/my-data-stream-policy
{
  "policy": {
    "phases": {
      "hot": {
        "actions": {
          "rollover": {
            "max_size": "25GB"
          }
        }
      },
      "delete": {
        "min_age": "30d",
        "actions": {
          "delete": {}
        }
      }
    }
  }
}

PUT /_index_template/my-data-stream-template
{
  "index_patterns": [ "%my-data-stream*" ],
  "data_stream": { },
  "priority": 200,
  "template": {
    "settings": {
      "index.lifecycle.name": "my-data-stream-policy"
    }
  }
}

POST /%25my-data-stream/_doc/
{
  "@timestamp": "2020-12-06T11:04:05.000Z",
  "user": {
    "id": "vlb44hny"
  },
  "message": "Login attempt failed"
}

Index template and index lifecycle policy with a special character can be created via UI

Release Note

Fixes issues in Index Management and ILM when resources have special characters in names

yuliacech · 2020-10-19T13:05:52Z

@elasticmachine merge upstream

…s for filters

yuliacech · 2020-10-20T13:45:11Z

@elasticmachine merge upstream

elasticmachine · 2020-10-20T16:07:07Z

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

yuliacech · 2020-10-21T13:49:34Z

@elasticmachine merge upstream

yuliacech · 2020-10-21T13:53:11Z

@elasticmachine merge upstream

sebelga

Great job @yuliacech ! Thanks for looking into this, I've tested all the scenario and it all works correctly except in one scenario.

I followed the steps from #80661

When I am in the indices list table, viewing the backing index

When I click on the data stream, the app crashes

sebelga · 2020-10-22T10:57:55Z

x-pack/plugins/index_management/server/routes/api/data_streams/register_get_route.ts

          ctx.core.elasticsearch.legacy.client.callAsCurrentUser('transport.request', {
-            path: `/_data_stream/${name}/_stats`,
+            path: `/_data_stream/${encodeURIComponent(name)}`,


I am surprised that we need to use transport.request here and not in other places (e.g. "cross_cluster_replication" plugin which I do think also allow the same special chars.).
Not sure how you tested here but could you do the same test with a follower index or auto_follow pattern?

You are right, @sebelga ! I reverted the getDataStream route back to using custom client. As I understand it: the custom client adds encoding automatically when provided with a parameter. When using request (like here for stats), we need to encode manually.

… route

yuliacech · 2020-10-22T12:58:05Z

Thanks a lot for your review, @sebelga! I fixed the navigation from indices back to data streams, nice catch :D
Also, tested cross cluster replication and it's a bit broken when used with special characters, I'll create an issue to track that.

yuliacech · 2020-10-26T09:27:19Z

@elasticmachine merge upstream

yuliacech · 2020-10-26T13:03:41Z

@elasticmachine merge upstream

cjcenizal · 2020-10-26T15:37:16Z

src/plugins/es_ui_shared/public/url/attempt_to_uri_decode.ts

+ * under the License.
+ */
+
+export const attemptToURIDecode = (value: string) => {


Can we add some unit tests for this function? I'd like to see cases that clarify the nuance behind result being overwritten once on line 23 and then immediately overwritten again on line 24. I'd also expect the test cases to communicate the use cases for this function to someone who's wondering when and how they should use it.

Thanks for review, @cjcenizal ! I added a comment when to use this function and tests with examples. I deleted decodeURI since I found out that it's already done in react router.

yuliacech · 2020-10-27T13:16:05Z

@elasticmachine merge upstream

yuliacech · 2020-10-27T14:53:25Z

@elasticmachine merge upstream

elastic#81664

kibanamachine · 2020-10-27T17:07:34Z

💚 Build Succeeded

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

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`esUiShared`	207	208	+1
`indexManagement`	509	508	-1
`ingestPipelines`	493	491	-2
total			-2

async chunks size

id	before	after	diff
`indexLifecycleManagement`	312.1KB	312.1KB	+5.0B
`indexManagement`	1.6MB	1.6MB	-324.0B
`ingestPipelines`	813.1KB	812.9KB	-136.0B
total			-455.0B

page load bundle size

id	before	after	diff
`esUiShared`	303.2KB	303.5KB	+241.0B
`indexManagement`	122.8KB	122.4KB	-356.0B
`ingestPipelines`	41.9KB	42.0KB	+153.0B
total			+38.0B

History

💚 Build #83963 succeeded bdec175
💔 Build #83930 failed 2126029
💚 Build #83635 succeeded 85414ec
💛 Build #83344 was flaky 6f58544
💚 Build #83341 succeeded eff3b40

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

sebelga

Thanks for making the changes @yuliacech ! Tested locally and works as expected 👍

… characters (elastic#80835) * Add uri decode to es_ui_shared and fix data stream issue with % name * Add a test for data streams tab opened for name with a dollar sign * Import uri decode function from es_ui_shared and fix navigation issues for filters * Add tests for data streams with special characters in name * Revert react router navigate changes (is done in a separate PR) * Reverting changes to dataManagement es client and get data stream api route * Fix data stream name filter when activated from a url parameter * Clean up for better consistency and fixes after elastic#81664 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

… characters (#80835) (#81897) * Add uri decode to es_ui_shared and fix data stream issue with % name * Add a test for data streams tab opened for name with a dollar sign * Import uri decode function from es_ui_shared and fix navigation issues for filters * Add tests for data streams with special characters in name * Revert react router navigate changes (is done in a separate PR) * Reverting changes to dataManagement es client and get data stream api route * Fix data stream name filter when activated from a url parameter * Clean up for better consistency and fixes after #81664 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…kibana into alerts/convert-to-tm-intervals * 'alerts/convert-to-tm-intervals' of github.com:gmmorris/kibana: (88 commits) fixed jest APM Experiments settings (elastic#81554) [Resolver] Enable resolver test plugin tests (elastic#81339) Add TS project references for inspector (elastic#81792) Add uri decode to es_ui_shared and fix navigation issues with special characters (elastic#80835) [Fleet] Rename ingestManager translations fleet (elastic#81837) [Logs UI] Transmit and render array field values in log entries (elastic#81385) Audit Logging: use the original url (elastic#81282) [User experience] Fix JS error rate (elastic#81512) [UX] Add median/percentile info in titles (elastic#79824) Support export for SO with circular refs (elastic#81582) Get rid of global types (elastic#81739) [APM] Fix precommit script (elastic#81594) skips overview tests (elastic#81877) [Security Solution][Case] Fix connector's labeling (elastic#81824) Added simple test, which only covers successful case when edit happened right after task was complete previous execution [Maps] Fix EMS test (elastic#81856) [Security Solutions][Detections] - Fix bug, last response not showing for disabled rules (elastic#81783) skip flaky suite (elastic#81853) Fixed type checks and unit tests ...

yuliacech added 2 commits October 16, 2020 16:42

Add uri decode to es_ui_shared and fix data stream issue with % name

9a193ae

Add a test for data streams tab opened for name with a dollar sign

99eb3cc

kibanamachine and others added 3 commits October 19, 2020 09:05

Merge branch 'master' into url_decode

7000277

Import uri decode function from es_ui_shared and fix navigation issue…

3ecb849

…s for filters

Add tests for data streams with special characters in name

9eb0a71

yuliacech changed the title ~~Add uri decode to es_ui_shared and fix data stream issue with % name~~ Add uri decode to es_ui_shared and fix navigation issues with special characters Oct 20, 2020

Revert react router navigate changes (is done in a separate PR)

a994579

Merge branch 'master' into url_decode

92c39b0

yuliacech added Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.11.0 v8.0.0 Feature:ILM release_note:fix labels Oct 20, 2020

yuliacech marked this pull request as ready for review October 20, 2020 16:07

yuliacech requested a review from a team as a code owner October 20, 2020 16:07

yuliacech requested a review from sebelga October 21, 2020 12:04

Merge branch 'master' into url_decode

4155636

sebelga reviewed Oct 22, 2020

View reviewed changes

yuliacech added 2 commits October 22, 2020 14:48

Reverting changes to dataManagement es client and get data stream api…

eff3b40

… route

Fix data stream name filter when activated from a url parameter

6f58544

Merge branch 'master' into url_decode

85414ec

cjcenizal reviewed Oct 26, 2020

View reviewed changes

Merge branch 'master' into url_decode

2126029

kibanamachine and others added 3 commits October 27, 2020 10:53

Merge branch 'master' into url_decode

bdec175

Clean up for better consistency and fixes after

f4fa3f5

elastic#81664

Merge remote-tracking branch 'origin/url_decode' into url_decode

ec73aec

sebelga approved these changes Oct 28, 2020

View reviewed changes

yuliacech merged commit 2a94139 into elastic:master Oct 28, 2020

yuliacech mentioned this pull request Oct 28, 2020

[7.x] Add uri decode to es_ui_shared and fix navigation issues with special characters (#80835) #81897

Merged

yuliacech deleted the url_decode branch December 3, 2020 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add uri decode to es_ui_shared and fix navigation issues with special characters #80835

Add uri decode to es_ui_shared and fix navigation issues with special characters #80835

yuliacech commented Oct 16, 2020 •

edited

Loading

yuliacech commented Oct 19, 2020

yuliacech commented Oct 20, 2020

elasticmachine commented Oct 20, 2020

yuliacech commented Oct 21, 2020

yuliacech commented Oct 21, 2020

sebelga left a comment

sebelga Oct 22, 2020

yuliacech Oct 22, 2020

yuliacech commented Oct 22, 2020

yuliacech commented Oct 26, 2020

yuliacech commented Oct 26, 2020

cjcenizal Oct 26, 2020

yuliacech Oct 27, 2020

yuliacech commented Oct 27, 2020

yuliacech commented Oct 27, 2020

kibanamachine commented Oct 27, 2020

sebelga left a comment

Add uri decode to es_ui_shared and fix navigation issues with special characters #80835

Add uri decode to es_ui_shared and fix navigation issues with special characters #80835

Conversation

yuliacech commented Oct 16, 2020 • edited Loading

Summary

Code changes

How to test

Release Note

yuliacech commented Oct 19, 2020

yuliacech commented Oct 20, 2020

elasticmachine commented Oct 20, 2020

yuliacech commented Oct 21, 2020

yuliacech commented Oct 21, 2020

sebelga left a comment

Choose a reason for hiding this comment

sebelga Oct 22, 2020

Choose a reason for hiding this comment

yuliacech Oct 22, 2020

Choose a reason for hiding this comment

yuliacech commented Oct 22, 2020

yuliacech commented Oct 26, 2020

yuliacech commented Oct 26, 2020

cjcenizal Oct 26, 2020

Choose a reason for hiding this comment

yuliacech Oct 27, 2020

Choose a reason for hiding this comment

yuliacech commented Oct 27, 2020

yuliacech commented Oct 27, 2020

kibanamachine commented Oct 27, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

sebelga left a comment

Choose a reason for hiding this comment

yuliacech commented Oct 16, 2020 •

edited

Loading