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

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

Open
lukeelmers opened this issue Jul 14, 2021 · 14 comments
Open

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

lukeelmers opened this issue Jul 14, 2021 · 14 comments
Assignees
Labels
enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@lukeelmers
Copy link
Member

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

  • In most cases we aren't logging warnings on the server when a deprecated route has been hit
  • For documented REST APIs, our only option is to use the docs to indicate that the 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)

@lukeelmers lukeelmers added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result labels Jul 14, 2021
@elasticmachine
Copy link
Contributor

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

@mshustov
Copy link
Contributor

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
Copy link
Contributor

kobelb commented Jul 15, 2021

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
Copy link
Member Author

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
Copy link
Contributor

I created #117241 to track the need for UA to surface Kibana deprecation logs to support this type of functionality.

@lukeelmers
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member

rjernst commented Feb 7, 2023

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
Copy link
Contributor

lcawl commented Mar 20, 2024

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
Copy link
Contributor

deprecations to be handled under the #179467

@pgayvallet
Copy link
Contributor

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.

@pgayvallet pgayvallet closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2024
@rudolf
Copy link
Contributor

rudolf commented Aug 27, 2024

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

@rudolf rudolf reopened this Nov 12, 2024
@TinaHeiligers
Copy link
Contributor

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 #179170
cc @lukeelmers

@TinaHeiligers
Copy link
Contributor

Tasks:

  • Match the header structure to the same spec that Elasticsearch is using for the header and response code
  • Add the header to the router as optional:
export interface HttpResponseOptions<T extends HttpResponsePayload | ResponseError = any> {
  /** HTTP message to send to the client */
  body?: T;
  /** HTTP Headers with additional information about response */
  headers?: ResponseHeaders;
  /** Bypass the default error formatting */
  bypassErrorFormat?: boolean;
}
  /**
   * Description of deprecations for this HTTP API.
   *
   * @remark This will assist Kibana HTTP API users when upgrading to new versions
   * of the Elastic stack (via Upgrade Assistant) and will be surfaced in documentation
   * created from HTTP API introspection (like OAS).
   *
   * Setting this object marks the route as deprecated.
   *
   * @remarks This may be surfaced in OAS documentation.
   * @public
   */
  deprecated?: RouteDeprecationInfo;

Potentially add a request/response lifecycle hook to add the header (TBD)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests