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

Migrate from path based versioning to header based versioning #159181

Open
jloleysens opened this issue Jun 7, 2023 · 10 comments
Open

Migrate from path based versioning to header based versioning #159181

jloleysens opened this issue Jun 7, 2023 · 10 comments
Labels
chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:EnterpriseSearch Team:Observability Team label for Observability Team (for things that are handled across all of observability) Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@jloleysens
Copy link
Contributor

jloleysens commented Jun 7, 2023

6/6 for the public APIs we will have to schedule the breaking change; for the internal APIs we don't need to do this for a while (basically this is tech debt)

There are endpoints in Kibana that use path-based versioning. We need to decide at which point to migrate these to use header-based versioning (i.e. onboard to the versioned router).

The plan

For each endpoint

  1. Determine whether it is internal or public
  2. Follow the appropriate breaking change process to remove versioning from the path, i.e. remove v. params

List of endpoints by team

Do not consider this list exhaustive. It may get out-of-date.

Security

get /api/security/v1/logout
get /api/security/v1/me

Core (#159839)

post /api/telemetry/v2/optIn
get /api/telemetry/v2/config
post /api/telemetry/v2/clusters/_stats
post /api/telemetry/v2/clusters/_opt_in_stats
put /api/telemetry/v2/userHasSeenNotice
get /api/telemetry/v2/last_reported
put /api/telemetry/v2/last_reported

SharedUX

post /api/reporting/v1/generate/immediate/csv_searchsource
...other reporting endpoints?

Profiling

delete /api/profiling/v1/cache/executables
delete /api/profiling/v1/cache/stackframes
get /api/profiling/v1/flamechart
get /api/profiling/v1/topn/functions
get /api/profiling/v1/topn/containers
get /api/profiling/v1/topn/deployments
get /api/profiling/v1/topn/hosts
get /api/profiling/v1/topn/traces
get /api/profiling/v1/topn/threads
get /api/profiling/v1/setup/es_resources
post /api/profiling/v1/setup/es_resources
get /api/profiling/v1/setup/instructions

Observability

post /api/monitoring/v1/alert/{clusterUuid}/status
post /api/monitoring/v1/alerts/enable
post /api/monitoring/v1/clusters/{clusterUuid}/apm/{apmUuid}
post /api/monitoring/v1/clusters/{clusterUuid}/apm/instances
post /api/monitoring/v1/clusters/{clusterUuid}/apm
post /api/monitoring/v1/clusters/{clusterUuid}/beats/beat/{beatUuid}
post /api/monitoring/v1/clusters/{clusterUuid}/beats/beats
post /api/monitoring/v1/clusters/{clusterUuid}/beats
get /api/monitoring/v1/check_access
post /api/monitoring/v1/clusters/{clusterUuid}
post /api/monitoring/v1/clusters
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch/indices/{id}
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch/indices
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch/nodes/{nodeUuid}
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch/nodes
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch/ml_jobs
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch/ccr
post /api/monitoring/v1/clusters/{clusterUuid}/elasticsearch/ccr/{index}/shard/{shardId}
get /api/monitoring/v1/elasticsearch_settings/check/cluster
post /api/monitoring/v1/elasticsearch_settings/check/internal_monitoring
get /api/monitoring/v1/elasticsearch_settings/check/nodes
put /api/monitoring/v1/elasticsearch_settings/set/collection_enabled
put /api/monitoring/v1/elasticsearch_settings/set/collection_interval
post /api/monitoring/v1/clusters/{clusterUuid}/enterprise_search
get /api/monitoring/v1/_health
post /api/monitoring/v1/clusters/{clusterUuid}/logstash/pipeline_ids
post /api/monitoring/v1/clusters/{clusterUuid}/logstash/pipelines
post /api/monitoring/v1/clusters/{clusterUuid}/logstash/node/{logstashUuid}/pipelines
post /api/monitoring/v1/clusters/{clusterUuid}/logstash/node/{logstashUuid}
post /api/monitoring/v1/clusters/{clusterUuid}/logstash/nodes
post /api/monitoring/v1/clusters/{clusterUuid}/logstash
post /api/monitoring/v1/clusters/{clusterUuid}/logstash/pipeline/{pipelineId}/{pipelineHash?}
post /api/monitoring/v1/setup/collection/cluster/{clusterUuid?}
post /api/monitoring/v1/setup/collection/{clusterUuid}/disable_internal_collection
post /api/monitoring/v1/setup/collection/node/{nodeUuid}
post /api/monitoring/v1/clusters/{clusterUuid}/kibana/{kibanaUuid}
post /api/monitoring/v1/clusters/{clusterUuid}/kibana/instances
post /api/monitoring/v1/clusters/{clusterUuid}/kibana

Enterprise search

post /internal/app_search/search-ui/api/as/v1/engines/{engineName}/search.json

@jloleysens jloleysens added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Team:Observability Team label for Observability Team (for things that are handled across all of observability) Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Team:EnterpriseSearch labels Jun 7, 2023
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@elasticmachine
Copy link
Contributor

Pinging @elastic/unified-observability (Team:Observability)

@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@petrklapka
Copy link
Member

We need to reconcile moving to BWCA with not wanting to create a breaking change. Removing V1 would cause a breaking change. We are proposing that we add a new endpoint without the V1 as a redirect and to satisfy BWCA with that, while retaining the V1 endpoint to avoid the breaking change. What is the call on breaking changes in Serverless vs. Stateful? Will serverless coincide with 9.0 and will it be okay to introduce breaking changes?

@lukeelmers
Copy link
Member

Will serverless coincide with 9.0 and will it be okay to introduce breaking changes?

@petrklapka Serverless will not coincide with a 9.0 release, so while it will be okay to introduce breaking changes in the initial serverless offering only, we must maintain BWC for self-managed until we have a 9.0 release.

We are proposing that we add a new endpoint without the V1 as a redirect and to satisfy BWCA with that, while retaining the V1 endpoint to avoid the breaking change

I would add the new endpoint without the v1 in both self-managed in serverless, and skip registering the v1 path in serverless. So self-managed has both to preserve BWC, and serverless has the new non-v1 path only. This should be pretty easy to maintain as you can simply reuse the same route handler and register it for different paths.

The only catch would be determining the best way to prevent the old v1 path from being registered in serverless. It isn't clear yet whether a mechanism for this will be provided by core, so you might need to have some configuration to control this. This is being actively discussed in #157266.

Also just to clarify on add a new endpoint without the V1 as a redirect, I would not do a 301/302 redirect from old v1 > new on self-managed. This would work seamlessly for browsers, but could still break any users who aren't automatically following redirects.

@petrklapka
Copy link
Member

petrklapka commented Jun 14, 2023

Also just to clarify on add a new endpoint without the V1 as a redirect, I would not do a 301/302 redirect from old v1 > new on self-managed. This would work seamlessly for browsers, but could still break any users who aren't automatically following redirects.

Yup. I used redirect as a shorthand. I believe Vadim's term was a "fake redirect" ;)

@tsullivan - I think this gets us where we need to be to move ahead, at least to the point where we implement the header based version to comply with BWCA and continue the discussion on what to do with the old routes and how the BCC will fit into the picture and when. Any ideas on how to not register the old route on serverless, in case we do decide to diverge?

@rudolf
Copy link
Contributor

rudolf commented Jun 16, 2023

The principle of make-it-minor / the breaking changes committee is: reducing the pain of breaking changes for our users.

There's been an ongoing discussion around how that principle applies to serverless and the new API versioning specification. We're working on making sure we have alignment with senior leadership, but until then we'd recommend that teams hold off on any breaking changes to public APIs.

@afharo
Copy link
Member

afharo commented Aug 7, 2023

Before going on leave, I had this draft PR that needed just some final cleanup to get a green CI: #159839.

All green now and ready for a review 🟢

@legrego legrego added the chore label Oct 25, 2023
@tsullivan
Copy link
Member

Thanks @jloleysens @lukeelmers and @petrklapka for bringing this up. From the SharedUX/Reporting side, I think the work has been done as of #162288 and we can mark this as closed in the AppEx-SharedUX team board. That was done awhile ago, but I'm just going through our backlog now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:EnterpriseSearch Team:Observability Team label for Observability Team (for things that are handled across all of observability) Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

No branches or pull requests

8 participants