elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.69k stars 8.24k forks source link

[core.http] Support `Warning` headers for deprecated http routes #105692

Open lukeelmers opened 3 years ago

lukeelmers commented 3 years ago

Currently there isn't a great way for clients to understand that an http route has been deprecated:

Elasticsearch solves this problem with a Warning header that includes a 299 miscellaneous warn code and follows the RFC7234 spec for warning headers.

Example:

Warning: 299 Elasticsearch-8.0.0-###"Your license will expire in [N] days. Contact your administrator or update your license for continued use of features"

It would be useful if Kibana's http routes could be registered with some text that gets interpolated into a warning header, or perhaps a simple isDeprecated flag that would automatically produce a warning header. While deprecations is the obvious use case that comes to mind, there may be others as well.

As for the structure, I'd propose that we match the header structure to the same spec that Elasticsearch is using.

(cc @kobelb)

elasticmachine commented 3 years ago

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

mshustov commented 3 years ago

To clarify in the functional requirements:

It would be useful if Kibana's http routes could be registered with some text that gets interpolated into a warning header,

Should it be integrated with Upgrade Assistant? Telemetry?

Warning: 299 Elasticsearch-8.0.0-###"Your license will expire in [N] days. Contact your administrator or update your license for continued use of features"

Should the message be i18n?

an http route has been deprecated:

In addition to the depreciation, we should document the URL versioning policy. Kibana used to rely on URL versioning (../v1/...), however, most URL do not rely on this pattern.

kobelb commented 3 years ago

In addition to the depreciation, we should document the URL versioning policy. Kibana used to rely on URL versioning (../v1/...), however, most URL do not rely on this pattern.

A long-time ago, the decision was made that we shouldn't be versioning our HTTP APIs using URLs, and they should be versioned along with the Stack version. I'm all for reopening this discussion, as this approach is rather cumbersome in some situations, especially when we only want to change the HTTP response format. However, I think we need to have this discussion separately and include the Elasticsearch team, so we have consistency.

lukeelmers commented 3 years ago

Should it be integrated with Upgrade Assistant? Telemetry?

Yes, I could see a possible integration with upgrade assistant and/or telemetry. It would be cool to surface warnings in upgrade assistant if we've detected any external (non-Elastic) uses of deprecated routes. And for internal usage of deprecated routes, perhaps we could log a warning to the browser console while in dev mode or automate warnings to the server logs (though it could get a bit chatty)

Should the message be i18n?

IMHO I would treat this the same way as we treat our server logs and not handle i18n. If apps want to take the error and display it to users, they could still handle the i18n on the client.

I think we need to have this discussion separately and include the Elasticsearch team, so we have consistency.

Yeah agreed - I intentionally left versioning out of this as I think it's a different (albeit related) issue.

cjcenizal commented 3 years ago

I created https://github.com/elastic/kibana/issues/117241 to track the need for UA to surface Kibana deprecation logs to support this type of functionality.

lukeelmers commented 2 years ago

Looks like the Warning header is now deprecated and not recommended for use in new projects: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Warning

@elastic/es-core-infra Does ES plan to continue using Warning headers for the foreseeable future, or have there been any discussions about replacing it with something else?

TinaHeiligers commented 1 year ago

Such a shame the header's not an option anymore. We really need to come up with a standard way to warn about deprecated HTTP routes.

rjernst commented 1 year ago

Here's a little more info about the deprecation: https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-cache-06#section-5.5

The "Warning" header field was used to carry additional information about the status or transformation of a message that might not be reflected in the status code. This specification obsoletes it, as it is not widely generated or surfaced to users. The information it carried can be gleaned from examining other header fields, such as Age.

Sorry we missed the ping. We were not aware of this deprecation, nor do we have any current plans regarding it.

lcawl commented 8 months ago

With respect to how this is ultimately represented in the generated openAPI specification, we'd want to use the deprecated property, for example set at the operation level here: https://spec.openapis.org/oas/latest#operation-object

TinaHeiligers commented 7 months ago

deprecations to be handled under the https://github.com/elastic/kibana/issues/179467

pgayvallet commented 4 months ago

Things have evolved quite a bit since that issue was opened, especially given the work we've been performing on OAS generation. We may want to add a header surfacing this information later, but it will be in a different form, and when we'll want it, this should naturallu resurface in our discussions, so I'll go ahead and close this one.

rudolf commented 3 months ago

To future proof this we might want to adopt the draft deprecation header https://datatracker.ietf.org/doc/html/draft-ietf-httpapi-deprecation-header

While we typically deprecate a whole API path, it's possible that a single resource in an API is deprecated. E.g. the feature privileges API might not be deprecated, but it's possible to access a deprecated feature id (resource).

This remains a lower priority item. At this time we believe deprecation notices in upgrade assistant are more likely to be picked up and fixed by users than headers returned to clients. But if possible it would be nice to release this in 8.last

TinaHeiligers commented 2 weeks ago

As discussed offline, Core will support two response headers for deprecated http routes: warning and sunsetting. These will be introduced during the 9.x series as part of Maturing our HTTP API layer. (issue to be created)

We'll also be adding headers to http stability https://github.com/elastic/kibana/issues/179170 cc @lukeelmers