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

[courier/rootSearchSource] stop loading the defaultIndexPattern #11012

Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Apr 3, 2017

Kibana uses a rootSearchSource to define the global search context (things like the time filter). This searchSource has historically been assigned the default index pattern, but as far as I can tell this behavior is not utilized or desired and this PR removes it. Now searchSources must define the index they are intended for in order to be fetch-able.

@spalger spalger added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc review labels Apr 3, 2017
@spalger spalger requested a review from epixa April 3, 2017 22:22
@w33ble
Copy link
Contributor

w33ble commented Apr 10, 2017

This appears to close #9028

EDIT: Maybe not actually... I can't get 9028 to happen locally at all, even in master, so I don't think this actually closes it. But it may already be closed.

@w33ble
Copy link
Contributor

w33ble commented Apr 10, 2017

@spalger I'm not sure what this is fixing exactly. I can't get an error state in master that this would seem to resolve. How should we be testing this fix? And does it close any open issues?

@spalger
Copy link
Contributor Author

spalger commented Apr 10, 2017

@w33ble @BigFunger This doesn't outright fix any issue, it just removes an "optimization" in favor of only loading IndexPatterns that we actually need. This was mentioned by a user who was pointing out that even when Kibana didn't use the "default" index pattern it tried to load it, which is slightly wasteful but in a corner case, when mixed with document-level access control and #9028 (and related issues), lock users out of Kibana unnecessarily.

Consider this scenario:

  1. Users 1 and 2 have access to Index A
  2. Default Kibana IndexPattern is Index A
  3. User 1 builds a bunch of dashboards to User 2 based on Index A
  4. User 1 introduces User 3, creates Index B, gives access to themselves and User 3
  5. While User 1 builds dashboards for User 3 they change the default IndexPattern to Index B
  6. User 2 can't use Kibana anymore because they don't have access to the IndexPattern document for Index B.

This doesn't fix the issue if the user tried to use the index pattern, or loaded up discover with a default index, but it does remove the useless loading of the default index pattern.

@BigFunger
Copy link
Contributor

BigFunger commented Apr 11, 2017

@spalger can you rebase/merge on master, please?

@BigFunger BigFunger self-requested a review April 11, 2017 16:04
Copy link
Contributor

@BigFunger BigFunger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@w33ble w33ble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v5.4.0 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants