Skip to content

Instantly share code, notes, and snippets.

@mhoppa
Last active February 19, 2020 22:02
Show Gist options
  • Save mhoppa/6cfd7fcb2f9ec7d1feab85dcf45624bb to your computer and use it in GitHub Desktop.
Save mhoppa/6cfd7fcb2f9ec7d1feab85dcf45624bb to your computer and use it in GitHub Desktop.
List of rewritten routes to deprecate

Deprecation routes

Approach

  • Deprecated routes do not need to be rewritten from Perl to Go
  • Deprecated routes will still be available in 1.x but will not be available in 2.x
  • Deprecated routes will be marked as such in the documentation and will return a deprecation notice in the response

Deprecation Route Candidates

Routes that duplicate existing functionality

Route Verbs Existing Route support
asns/{id} GET Use asns route with query parameter id
cachegroups/trimmed Use cachegroups route as trimmed is just a subset of the data returned from that endpoint
cachegroups/{id}/unassigned_parameters Use cachegroupparameters route with query parameter cachegroup and diff that to parameters response
cachegroups/{id} GET Use cachegroups route with query parameter id
cdns/name/{name} GET Use cdns route with query parameter name
cdns/{id} GET Use cdns route with query parameter id
cdns/{name}/federations/{id} GET Use cdns/{name}/federations route with query parameter id
deliveryservice_matches Use deliveryservices_regexes route as deliveryservice_matches is just a subset of its return data
deliveryservices/hostname/{{host name}}/sskeys Use deliveryservices/xmlId/{{XMLID}}/sslkeys and filter the results via the hostname returned on array item
deliveryservices/{dsid}/regexes/{regexid} GET Use deliveryservices/{dsid}/regexes route with query parameter id
deliveryservices/{id}/unassigned_servers Use either deliveryservices/{id}/servers route and diff that with servers or even better use deliveryservices/{id}/servers/eligible/
deliveryservices/{id} GET Use deliveryservices route with query parameter id
divisions/{id} GET Use divisions route with query parameter id
keys/ping Use riak/ping route as it does the same thing and has the same backing logic
logs/{days}/days Use logs route with query parameter days
parameters/profile/{name} Use profiles/name/{name}/parameters route instead as same response is returned
parameters/{id} GET Use parameters route with query parameter id
phys_locations/trimmed Use phys_locations route as trimmed is just a subset of the data returned from that endpoint
phys_locations/{id} GET Use phys_locations route with query parameter id
profiles/trimmed Use profiles route as trimmed is just a subset of the data returned from that endpoint
profiles/{id}/unassigned_parameters Use profiles/{id}/parameters route and diff that with parameters
profiles/{id} GET Use profiles route with query parameter id
regions/name/{name} GET Use regions route with query parameter name
regions/{id} DELETE Use regions route with query parameter id
regions/{id} GET Use regions route with query parameter id
servers/hostname/{hostName}/details Use servers/details route with query parameter hostName
servers/status Use servers route and do the counting client side. NOTE need to look into TP work to support this
servers/{id} GET Use servers route with query parameter id
statuses/{id} GET Use statuses route with query parameter id
steering/{deliveryservice}/targets/{target} GET Use steering/{deliveryservice}/targets route with query parameter target
tenants/{id} GET Use tenants route with query parameter id
types/{id} GET Use types route with query parameter id
user/{id}/deliveryservices/available User delivery service associations provide no value in current system
users/{id}/deliveryservices User delivery service associations provide no value in current system

Routes to be deprecated and replaced

Work to do in replacement

Route(s) Replacement Path(s) Reasoning
cdns/name/{{name}}/dnsseckeys/delete cdns/name/{{name}}/dnsseckeys The HTTP verb should not be apart of the route and a DELETE should not be a GET request
cdns/{{ID}}/snapshot & snapshot/{{CDN}} snapshot The two routes do the same thing besides specifying the CDN by name vs id. This could be combined into using the same route and specifying the identifier via query parameters.
deliveryservice_server/{{DS ID}}/{{Server ID}} deliveryserviceserver/{{DS ID}}/{{Server ID}} We already used the base route deliveryserviceserver for deliveryservice server association, should change this to maintain consistentency.
deliveryservices/xmlId/{{XML ID}}/sslkeys/delete deliveryservices/xmlId/{{XML ID}}/sslkeys The HTTP verb should not be apart of the route and a DELETE should not be a GET request
riak/bucket/{{bucket}}/key/{{key}}/values vault/bucket/{{bucket}}/key/{{key}}/values Get rid of riak verbiage in our routing naming as it is an implementation detail of traffic vault
riak/ping vault/ping Get rid of riak verbiage in our routing naming it is an implementation detail of traffic vault
servers/checks servercheck We represent server checks on two different paths servercheck & servers/checks. This is just cleaning them up so they are on the same path.
to_extensions servercheck/extensions & plugins to_extensions used to support CONFIG_EXTENSIONS, STATISTIC_EXTENSION and CHECK_EXTENSIONS in the Perl implementation but since in Go TO config,stat extensions were superseded by the plugin system the rewrite pulled just check extensions and configured plugins. This can be cleaned up to be more clear to the users of the API what they are interacting with by splitting them up.

Routes to be deprecated with no direct replacement

Route Reasoning
/cdn/metric_types Currently returns 501 in 1.x and provides no value

If VERBS is left blank that means route will be deprecated in whole

@mitchell852
Copy link

mitchell852 commented Feb 14, 2020

/cachegroupparameters has quite a different response than cachegroups/{id}/parameters so imo if cachegroups/{id}/parameters is going to be deprecated, then /cachegroupparameters needs to return the full parameter object. or wait. maybe GET /parameters?cachegroup= would work...but i'm guessing the cachegroup query param is unsupported currently on that route.

@zrhoffman
Copy link

zrhoffman commented Feb 14, 2020

Consider adding POST deliveryservices/request to this list, as it duplicates POST deliveryservice_requests.

Edit: deliveryservices/request is actually for requesting that a delivery service be created and is not redundant.

@mitchell852
Copy link

users/{id}/deliveryservices actually does provide value but it needs to be "fixed" - apache/trafficcontrol#3754

and actually maybe GET /deliveryservices?user= would make the most sense to fetch "all the delivery services accessible by the user"

@zrhoffman
Copy link

zrhoffman commented Feb 14, 2020

Sort routes, please?

You can just copy this sorted list (click to expand).
Route | Verbs | Deprecation reason
--- | --- | ---
`asns/{id}` | GET | Can use `/asns` route with query parameter id
`cachegroups/{id}` | GET | Can use `/cachegroups` route with query parameter id
`cachegroups/{id}/parameters` | | Can use `/cachegroupparameters` route with query parameter cachegroup
`cachegroups/{id}/unassigned_parameters` | | Can use `/cachegroupparameters` route with query parameter cachegroup and diff that to `/parameters`
`cachegroups/trimmed` | | Can get the same information from `/cachegroups` route.
`cdns/{id}` | GET | Use `/cdns` route with query parameter id
`cdns/{name}/federations/{id}` | GET | Use `/cdns/{name}/federations` route with query parameter id
`cdns/name/{name}` | GET | Use `/cdns` route with query parameter name
`deliveryservices/{dsid}/regexes/{regexid}` | GET | Use `/cdns/{name}/federations` route with query parameter id
`deliveryservices/{id}` | GET | Use `/deliveryservices` route with query parameter id
`deliveryservices/{id}/unassigned_servers` | | Use either `deliveryservices/{id}/servers` route and diff that with `servers` or even better use `deliveryservices/{id}/servers/eligible/`
`divisions/{id}` | GET | Use `/divisions` route with query parameter id
`logs/{days}/days` | | Use `/logs` route with query parameter days
`parameters/{id}` | GET | Use `/parameters` route with query parameter id
`parameters/profile/{name}` | | Use `/profiles/name/{name}/parameters` route instead
`phys_locations/{id}` | GET | Use `/phys_locations` route with query parameter id
`phys_locations/trimmed` | | Can get the same information from `/phys_locations` route.
`profiles/{id}` | GET | Use `/profiles` route with query parameter id
`profiles/{id}/unassigned_parameters` | | Use `/profiles/{id}/parameters` route and diff that with `parameters`
`profiles/trimmed` | | Can get the same information from `/profiles` route.
`regions/{id}` | DELETE | Use `/regions` route with query parameter id
`regions/{id}` | GET | Use `/regions` route with query parameter id
`regions/name/{name}` | GET | Use `/regions` route with query parameter name
`servers/hostname/{hostName}/details` |  | Use `servers/details` route with query parameter hostName
`servers/{id}` | GET | Use `/servers` route with query parameter id
`statuses/{id}` | GET | Use `/statuses` route with query parameter id
`steering/{deliveryservice}/targets/{target}` | GET | Use `/steering/{deliveryservice}/targets` route with query parameter target
`tenants/{id}` | GET | Use `/tenants` route with query parameter id
`types/{id}` | GET | Use `/types` route with query parameter id
`user/{id}/deliveryservices/available` | | User delivery service associations provide no value in current system 
`users/{id}/deliveryservices` | | User delivery service associations provide no value in current system 
`users/{id}` | GET | Use `/users` route with query parameter id

@ericholguin
Copy link

What about deliveryservice_user and deliveryservice_user/{{dsID}}/{{userID}}?

@mhoppa
Copy link
Author

mhoppa commented Feb 18, 2020

@ericholguin
Copy link

I don't see deliveryservices/{{xml_id}}/servers on there. It duplicates deliveryserviceserver functionality and I don't think it is actually used in TP

@mhoppa
Copy link
Author

mhoppa commented Feb 18, 2020

Well they are actually slightly different, deliveryserviceserver supports both GET & POST and deliveryservices/{{xml_id}}/servers supports just POST.

That and deliveryserviceserver takes servers by ids while deliveryservices/{{xml_id}}/servers takes them by names

@ericholguin
Copy link

deliveryservices/{{xml_id}}/servers supports just GET.
That and deliveryserviceserver takes servers by ids while deliveryservices/{{xml_id}}/servers takes them by names

I think you meant POST, but you're right they do have different implementations:

deliveryserviceserver uses query parameters, i.e. dsId=1&replace=true&servers=12
and
deliveryservices/{{xml_id}}/servers uses a json body i.e. { "serverNames": [ "edge" ] }

But they still do the same thing and I don't see where deliveryservices/{{xml_id}}/servers is actually used.

I'll also add theres another endpoint that is not documented that does the same thing as these ones: servers/{id}/deliveryservices
It's also implemented differently and I'm not sure where it is used.

@mhoppa
Copy link
Author

mhoppa commented Feb 18, 2020

The approach we tried to carry is if there is a direct replacement than we could deprecate but if the endpoint has slightly different implementations we kept those around. Also just because the endpoint is not used by TP should we rely on other clients not using it out there.

@ericholguin
Copy link

Okay, that makes sense. Thanks.

@mitchell852
Copy link

users/{id}/deliveryservices should not be deprecated until there is an alternate way to fetch ALL delivery services accessible to a tenant. apache/trafficcontrol#4402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment