Kibana request headers by kobelb · Pull Request #79218 · elastic/kibana

kobelb · 2020-10-01T21:22:23Z

Elasticsearch would like a way to identify when a request is coming from Kibana. This would allow them to add an exception to their system-index deprecation logs, so we don't get a deprecation logged prior to Kibana switching to the new system index APIs.

Within this PR, I implemented the changes to hard-code a User-Agent: Kibana header which is specified whenever a request is made to Elasticsearch using either the legacy or new Elasticsearch client. However, there is a complication introduced by the existing elasticsearch.customHeaders and elasticsearch.requestHeadersWhitelist kibana.yml settings. These settings allow the user to either set a hard-coded User-Agent header on every request or forward the User-Agent from the incoming HTTP request to Kibana.

We could theoretically only specify the User-Agent header whenever it isn't specified within these settings and give the user's configured value priority; however, this would cause Elasticsearch to log deprecations. If we no longer allow the user to specify the User-Agent in either elasticsearch.customHeaders or elasticsearch.requestHeadersWhitelist, this is a breaking change

I can't think of any situations where I've seen end-users using these settings with the User-Agent header, but given our lack of telemetry on these settings, I'm hesitant to introduce this breaking change.

Instead of using the User-Agent header we could theoretically use a different header, X-Less-Likely-To-Have-Conflicts, to lessen the likelihood of this impacting end-users but in the strictest sense, this will still be treated as a breaking change.

@joshdover, can you think of any other option that I'm overlooking here?

@gwbrown, do we have other options here which don't introduce breaking changes? Can we just not classify the Kibana indices as system-indices before making the necessary changes?

We currently allow end-users to set whatever headers they'd like to be forwarded to Elasticsearch with `elasticsearch.customHeaders` and `elasticsearch.requestHeadersWhitelist`. This is potentially problematic with us always specifying `User-Agent: kibana` as it could interfere with what our end-users have done...

AthenaEryma · 2020-10-01T21:43:00Z

Kibana's indices have already been registered as System Indices since 7.8 (relevant PR: elastic/elasticsearch#54858). I'm not sure off the top of my head whether changing would cause issues, but either way it would be preferable to issue the deprecation warnings when users access .kibana directly, just like the other System Indices.

I'll think on this and report back if I come up with anything, but my first impression is that choosing a sufficiently obscure X-Less-Likely-To-Have-Conflicts header and allowing the user's requests to take priority is a decent option if it's not too difficult to implement - this would theoretically allow Kibana to trigger deprecation warnings, but only in a very edge-case-y configuration.

AthenaEryma · 2020-10-02T21:13:10Z

I haven't been able to come up with a noticeably better solution yet. It would also be a possibility, if we allowed user headers to take precedence and used a header along the lines of X-elastic-product-origin: kibana, to simply suppress the warning in Elasticsearch whenever the header is present, with any value. This way, the header would be usable for any other products which need similar functionality, and continue to work even if a different value is specified by the user and passed through Kibana to Elasticsearch.

kobelb · 2020-10-02T21:14:45Z

I like that idea @gwbrown. I'll get this PR changed to implement that approach. Even if we just use a X-elastic-product-origin header, the likelihood that we'll have a collision seems negligible.

…ader

T#

joshdover · 2020-10-05T22:20:24Z

src/core/server/elasticsearch/kibana_headers.ts

+
+export const KIBANA_HEADERS = {
+  'x-elastic-product-origin': 'kibana',
+};


nits:

Can we wrap this with deepFreeze?

Would you mind renaming this to DEFAULT_HEADERS? Anything named "kibana" means everything and nothing anymore 😄

Sure :) I hated the name of this variable, but since it was meant to designate the request as "coming from Kibana", it seemed appropriate at the point in time.

joshdover · 2020-10-05T22:23:33Z

src/core/server/elasticsearch/client/cluster_client.ts

    }

    return {
+      ...KIBANA_HEADERS,


I wonder if we should also automatically forward this header from the incoming request. For instance, if elastic-agent makes a request to Kibana, what should the x-elastic-product-origin header be when it arrives at ES?

May be unnecessary for this initial change, but it could be useful for telemetry purposes in ES in the future.

At the moment, this is only being consumed to determine when to suppress deprecation logs when the Kibana system-indices are queried by Kibana. If the usage of this grows, I could see us making this change, but it doesn't seem necessary at this point-in-time.

cjcenizal · 2020-10-05T22:52:14Z

src/core/server/elasticsearch/default_headers.ts

+import { deepFreeze } from '@kbn/std';
+
+export const DEFAULT_HEADERS = deepFreeze({
+  'x-elastic-product-origin': 'kibana',


Think someone reading this code would benefit from a comment that explains the intention? E.g.

Elasticsearch uses this to identify when a request is coming from Kibana, to allow Kibana deprecated access to system indices logging a warning. We'll migrate to use new system index APIs in the future, which will allow us to remove this header.

Good call, adding that!

kibanamachine · 2020-10-06T00:25:50Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4982ccf

Metrics [docs]

distributable file count

id	before	after	diff
`default`	47110	47111	+1
`oss`	28597	28598	+1

History

💚 Build #79790 succeeded 8f10124
💔 Build #78957 failed 882b4ea

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

* Adding X-Kibana header for the legacy client requests * Specifying `X-Kibana: true` header for all requests coming from Kibana * Dev Tools now override the X-Kibana header to false * DevTools doesn't need to do anything, it's not using the ES client * Switching from `X-Kibana: true` to `User-Agent: Kibana` * Switching from `X-Kibana: true` to `User-Agent: Kibana` * Adding a constant * Starting to add unit tests... We currently allow end-users to set whatever headers they'd like to be forwarded to Elasticsearch with `elasticsearch.customHeaders` and `elasticsearch.requestHeadersWhitelist`. This is potentially problematic with us always specifying `User-Agent: kibana` as it could interfere with what our end-users have done... * Switching from user-agent to X-elastic-product-origin * Adding some tests * Adding and updating the legacy client unit tests T# * /s/KIBANA_HEADERS/DEFAULT_HEADERS and a deepFreeze * Adding comment for why `x-elastic-product-origin` exists

…into feature/task_manager_429 * 'feature/task_manager_429' of github.com:elastic/kibana: (158 commits) Add license check to direct package upload handler. (#79653) [Ingest Manager] Rename API /api/ingest_manager => /api/fleet (#79193) [Security Solution][Resolver] Simplify CopyableField styling and add comments (#79594) Fine-tunes ML related text on Metrics UI (#79425) [ML] DF Analytics creation wizard: ensure job creation possible when model memory lower than estimate (#79229) Add new "Add Data" tutorials (#77237) Update APM telemetry docs (#79583) Revert "Add support for runtime field types to mappings editor. (#77420)" (#79611) Kibana request headers (#79218) ensure missing indexPattern error is bubbled up to error callout (#79378) Missing space fix (#79585) remove duplicate tab states (#79501) [data.ui] Lazy load UI components in data plugin. (#78889) Add generic type params to search dependency. (#79608) [Ingest Manager] Internal action for policy reassign (#78493) [ILM] Add index_codec to forcemerge action in hot and warm phases (#78175) [Ingest Manager] Update open API spec and add condition to agent upgrade endpoint (#79579) [ML] Hide Data Grid column options when histogram charts are enabled. (#79459) [Telemetry] Synchronous `setup` and `start` methods (#79457) [Observability] Persist time range across apps (#79258) ...

kobelb added 8 commits September 30, 2020 14:58

Adding X-Kibana header for the legacy client requests

812fead

Specifying X-Kibana: true header for all requests coming from Kibana

bf47c13

Dev Tools now override the X-Kibana header to false

ccaa60c

DevTools doesn't need to do anything, it's not using the ES client

4366d25

Switching from X-Kibana: true to User-Agent: Kibana

39079a9

Switching from X-Kibana: true to User-Agent: Kibana

5a5dadd

Adding a constant

40e9f7c

AthenaEryma mentioned this pull request Oct 2, 2020

Deprecate REST access to System Indices elastic/elasticsearch#60945

Merged

kobelb added 4 commits October 5, 2020 09:27

Merge remote-tracking branch 'upstream/master' into kibana-request-he…

a187183

…ader

Switching from user-agent to X-elastic-product-origin

37bd0a0

Adding some tests

51244dd

Adding and updating the legacy client unit tests

8f10124

T#

kobelb marked this pull request as ready for review October 5, 2020 21:06

kobelb requested a review from a team as a code owner October 5, 2020 21:06

kobelb added v8.0.0 v7.10.0 release_note:skip Skip the PR/issue when compiling release notes labels Oct 5, 2020

joshdover approved these changes Oct 5, 2020

View reviewed changes

/s/KIBANA_HEADERS/DEFAULT_HEADERS and a deepFreeze

b11e5ce

cjcenizal reviewed Oct 5, 2020

View reviewed changes

Adding comment for why x-elastic-product-origin exists

4982ccf

kobelb merged commit 1ad68fd into elastic:master Oct 6, 2020

kobelb deleted the kibana-request-header branch October 6, 2020 13:40

kobelb mentioned this pull request Oct 6, 2020

[7.x] Kibana request headers (#79218) #79665

Merged

kobelb mentioned this pull request Jan 29, 2021

Kibana's system indices #81536

Closed

5 tasks

pgayvallet mentioned this pull request Feb 3, 2021

[console] exclude the x-elastic-product-origin header for requests performed from the dev console #90123

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kibana request headers#79218

Kibana request headers#79218
kobelb merged 14 commits intoelastic:masterfrom
kobelb:kibana-request-header

kobelb commented Oct 1, 2020 •

edited

Loading

Uh oh!

AthenaEryma commented Oct 1, 2020

Uh oh!

AthenaEryma commented Oct 2, 2020

Uh oh!

kobelb commented Oct 2, 2020 •

edited

Loading

Uh oh!

joshdover Oct 5, 2020

Uh oh!

kobelb Oct 5, 2020

Uh oh!

joshdover Oct 5, 2020

Uh oh!

kobelb Oct 5, 2020

Uh oh!

cjcenizal Oct 5, 2020

Uh oh!

kobelb Oct 5, 2020

Uh oh!

kibanamachine commented Oct 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kobelb commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AthenaEryma commented Oct 1, 2020

Uh oh!

AthenaEryma commented Oct 2, 2020

Uh oh!

kobelb commented Oct 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshdover Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

kobelb Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

joshdover Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

kobelb Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

cjcenizal Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

kobelb Oct 5, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Oct 6, 2020

💚 Build Succeeded

Metrics [docs]

distributable file count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kobelb commented Oct 1, 2020 •

edited

Loading

kobelb commented Oct 2, 2020 •

edited

Loading