Skip to content
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

Index Patterns with Commas #17147

Closed
kobelb opened this issue Mar 14, 2018 · 10 comments
Closed

Index Patterns with Commas #17147

kobelb opened this issue Mar 14, 2018 · 10 comments
Labels
bug Fixes for quality problems that affect the customer experience

Comments

@kobelb
Copy link
Contributor

kobelb commented Mar 14, 2018

If I create the following two indices in Elasticsearch: this-is-my-index-2018.01, this-is-my-index-also-2018.01

Kibana 6.1.1

I am able to create the following index pattern that appears to match the proper indices:

screen shot 2018-03-14 at 8 06 55 am

When I go to Discover and try to use this Index Pattern, it's not returning any results. However, if I execute the following query against Elasticsearch directly it returns results:

POST this-is-my-index-*,-*also*/_search

Kibana 6.2.1

I'm unable to create the following index pattern:

screen shot 2018-03-14 at 8 15 13 am

@kobelb kobelb added bug Fixes for quality problems that affect the customer experience :Management triage_needed labels Mar 14, 2018
@chrisronline
Copy link
Contributor

I can comment on the index pattern creation situation.

We had numerous regression occur when we refactored index pattern creation to React, mainly due to some smart guy (me 😞 ) trying a new technique to prevent the double request we have to send each time the user searches for something. Unfortunately, that approach caused regressions with CCS, aliases, and other built-ins that ES provides if we use their api directly (which we were with our old approach). We decided to stop hitting our heads against a wall and reverted back to using two queries which should have fixed all regressions, including this one.

This is 6.2.3:
screen shot 2018-03-14 at 9 47 00 am
screen shot 2018-03-14 at 9 47 06 am

@chrisronline
Copy link
Contributor

For the other issue, this feels like an issue that always existed, but was somewhat masked.

@kobelb checked in 5.6 and verified that, for some reason, you could not create index patterns containing a comma. However, in the 6.1 refactor, we changed how we validate an index pattern query which appears to have started allowing commas in index patterns (since they are allowed in ES). But since this wasn't planned/intentional, we didn't do any due diligence around ensuring patterns containing commas worked in other parts of the app.

This code will create an array of indices, but if we consider the search query this generates with an index pattern containing a comma, it looks like:

{"index":["*o*,-*log*"],"ignore_unavailable":true,"preference":1521040611501}
...

But this query always returns 0 results in ES.

But if we change that code to result in either of these:

{"index":["*o*","-*log*"],"ignore_unavailable":true,"preference":1521040611501}
...
{"index":"*o*,-*log*","ignore_unavailable":true,"preference":1521040611501}
...

ES returns results (assuming the rest of the query should match some data).

So basically, we need to decide what we actually support here and update the code if necessary.

cc @skearns64

@chrisronline
Copy link
Contributor

cc @spalger for thoughts on fixing this

@spalger
Copy link
Contributor

spalger commented Mar 14, 2018

@chrisronline isn't the the code you linked resolving the list of indexes to query using the field_caps api, which would return resolved index names rather than using *o*,-*log* within the segmented fetch?

@chrisronline
Copy link
Contributor

I'm not seeing how the field_caps is invoked here (but happy to learn if I'm missing something). It looks like the request is just using the title field directly from the index pattern saved object. Because of that, the index is the raw index pattern, unless I'm missing something?

@spalger
Copy link
Contributor

spalger commented Mar 14, 2018

Oh, nm, we removed that. #12814

I really want to support , in index patterns, as it has come up as a workaround for issues a bunch of times in the past. Maybe we should flatten the index parameter in segmented search to ensure it is always a comma delimited string, doing something like params.index = _.pluck(indices, 'index').join(',');?

@chrisronline
Copy link
Contributor

Yea my thoughts exactly. I just did something similar: indices.map(({ index }) => index).join(',') and it worked just fine. I'm just not sure if there will be unexpected fallout with this change

@spalger
Copy link
Contributor

spalger commented Mar 14, 2018

The index field currently only accepts either an array of index names or a comma separated string, so for the current API this should be fine. The API could change in the future but I don't think we need to worry about that now. My main concern is other places that are using index patterns and might not be expecting comma separated patterns, ie timelion

@chrisronline
Copy link
Contributor

@skearns64 echoed the same sentiment and I think he pinged ML specifically to see if they were using any separate APIs that might not work. I'll see if I can do some spot checking to ensure it works in some other areas.

@loesak
Copy link

loesak commented May 24, 2018

@chrisronline I originally reported on this in the forums (https://discuss.elastic.co/t/index-pattern-not-returning-results/123629/6) so thank all you for looking into and fixing. I know that this issue is closed but I wanted to also point out a similar problem occurs when i attempt to create a role that has read privilege on an index using the pattern *,-.*. This also seems to prevent users assigned to this role to read any data from the cluster instead from all indexes except those starting with a period. In order to get around this i had to use the pattern /@&~(\.@)/. There also seems to be no way to provide two index entries such that allows read on * and deny all on -.* so seems that using regex patterns is the only way to achieve this. I'm not sure if this was covered in the fix, or if it should have been.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience
Projects
None yet
Development

No branches or pull requests

4 participants