elasticsearch client: allow calling API as a user so that deprecation warnings are surfaced

[rudolf]

Upgrade assistant will hide deprecation warnings for requests made with x-elastic-product-origin: kibana. However, deprecations from some API calls should explicitly be surfaced, e.g. api calls from the console app.

In order to do this, plugins will need to unset the x-elastic-product-origin: kibana header.

One way to achieve that would be to change the custom Kibana transport for the Elasticsearch-js client so that when a api request is made with headers: {'x-elastic-product-origin': null} we remove the x-elastic-product-origin header from the request altogether.

Related: #120043

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[afharo]

how about an isExternal flag? It can be used for every plugin that allow custom requests. This way we hide the implementation details (which headers we actually rely on), and we can even differentiate between Kibana-internal requests, and user-generated ones (for telemetry and deprecation purposes).

What do you think?

[pgayvallet]

how about an isExternal flag? It can be used for every plugin that allow custom requests.

We're exposing vanilla instances of Client from @elastic/elasticsearch, not sure how/where you're suggested adding this flag?

[afharo]

That is a very strong point! I guess it's not worth overcomplicating this then.

So the scope of this issue is to allow the header "reset" and document that this is possible?

[rudolf]

yeah, given the timelines and our architecture that seems like the most pragmatic.

[pgayvallet]

to change the custom Kibana transport for the Elasticsearch-js client so that when a api request is made with headers: {'x-elastic-product-origin': null} we remove the x-elastic-product-origin header from the request altogether.

Hum, I thought we were the ones setting the x-elastic-product-origin in our custom transport, but TIL this is actually done from deeper in the client or transport. If using headers: {'x-elastic-product-origin': null} when calling an API of the client with current implementation doesn't remove the header, I'm unsure how we can adapt our transport to get to this behavior?

@delvedor maybe you can help us here?

[mshustov]

In order to do this, plugins will need to unset the x-elastic-product-origin: kibana header.

x-elastic-product-origin is added to restrict access to system indices. from #81536

Additionally, Kibana can continue accessing these indices using the standard ES APIs
while specifying a special HTTP request header and value pair.
It's my understanding that the ES implementation will require the 'x-elastic-product-origin': 'kibana'
header to be specified to denote these requests as originating from Kibana.

It means that plugin setting headers: {'x-elastic-product-origin': null} won't have access to system indices. Is that okay?

Upgrade assistant will hide deprecation warnings for requests made with x-elastic-product-origin: kibana

IMO we are blurring the responsibility of this header. x-elastic-product-origin single responsibility is to indicate that a request is initiated by Kibana.
Maybe we need another header to distinguish between a required triggered by a user or Kibana itself?
We already use kbn-system-request to differentiate requests sent from the Kibana browser app

kibana/src/core/public/http/fetch.ts

Line 139 in e527c3f

fetchOptions.headers['kbn-system-request'] = 'true';

I'd suggest to use API identical to the client-side to configure system requests:

client.seatch({...}, { asSystemRequest: true})

In theory we can extend TransportRequestOptions interface provided by elasticsearch-js to support a custom setting.

Another problem that we will need to solve is to enforce kbn-system-request usage when necessary. We didn't solve this problem for the client-side logic #79889 (comment)

However, deprecations from some API calls should explicitly be surfaced, e.g. api calls from the console app.

Hm. AFAIK Console app doesn't set x-elastic-product-origin: kibana header. #90123

[mshustov]

@delvedor suggested deleting the x-elastic-product-origin header on the Transport level. I don't think it's possible (see https://github.com/elastic/elasticsearch-js/blob/7.16/lib/Transport.js#L382) because prepareRequest is called inside of Transport.request method.

Unfortunatelly, we can't work around it by setting the header as undefined.

client.get(..., { headers: { 'x-opaque-id': undefined } })

since the ES client throws TypeError: Invalid value "undefined" for header "x-elastic-product-origin"
But we can set the header conditionally in Transport

  class KibanaTransport extends Transport {
    request(params: TransportRequestParams, options?: TransportRequestOptions) {
      const opts = options || {};
      const opaqueId = getExecutionContext();
      if (opaqueId && !opts.opaqueId) {
        // rewrites headers['x-opaque-id'] if it presents
        opts.opaqueId = opaqueId;
      }
      if (!opts.headers || !('x-elastic-product-origin' in opts.headers)) {
        opts.headers = opts.headers || {};
        opts.headers['x-elastic-product-origin'] = 'kibana';
      }
      return super.request(params, opts);
    }

In theory we can extend TransportRequestOptions interface provided by elasticsearch-js to support a custom setting.

After thinking a bit more, I realized it's not the best option because we will have to add this global d.ts file to every TS project. Other alternatives aren't good either: to declare a custom TS client interface for the ES client or allow headers: { 'x-opaque-id': ... } }) implementation detail to leak into the plugin code.

Hm. AFAIK Console app doesn't set x-elastic-product-origin: kibana header

UPD: it doesn't set the header. #90123
it doesn't use elasticseach-js client either https://github.com/elastic/kibana/blob/ff929b8845cf3423d59312bdee637e90ff3751ee/src/plugins/console/server/lib/proxy_request.ts

[lukeelmers]

cc @thesmallestduck @arisonl for input on priority for this

[pgayvallet]

@lukeelmers @rudolf is that something that may come back for 9.0? Or should we close the issue?

[rudolf]

As far as I know there are no other use cases where Kibana should not add it's header, so it's only console. Everywhere else Kibana controls which APIs it calls so it's our responsibility to stop depending on the deprecated APIs before 9.

I double checked and what @mshustov wrote is still true, so the console app doesn't need any workarounds, it already doesn't include this header.

UPD: it doesn't set the header. #90123
it doesn't use elasticseach-js client either https://github.com/elastic/kibana/blob/ff929b8845cf3423d59312bdee637e90ff3751ee/src/plugins/console/server/lib/proxy_request.ts

So I'll close this.