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

GET api/{version}/users/:userid/deliveryservices returns incorrect results #3754

Closed
mitchell852 opened this issue Jul 24, 2019 · 4 comments
Closed
Labels
bug something isn't working as intended medium impact impacts a significant portion of a CDN, or has the potential to do so Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1

Comments

@mitchell852
Copy link
Member

mitchell852 commented Jul 24, 2019

With the advent of tenancy, a few things have changed and one of the more notable things is that users now have access to ALL delivery services contained within the scope of their tenancy and not just the delivery services that are explicitly assigned to the user.

However GET api/{version}/users/:userid/deliveryservices returns delivery services that are explicitly assigned to the user in the deliveryservice_tmuser table. This table is no longer relevant and should be removed but that's another issue (#2910).

GET api/{version}/users/:userid/deliveryservices should take into account the specified user's tenant and return all delivery services based on that tenant.

This bug manifests itself in TP at https://tp.domain.com/#!/users/95/delivery-services. Once the api is fixed, TP will display the correct results.

@mitchell852 mitchell852 added bug something isn't working as intended Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 labels Jul 24, 2019
@mitchell852 mitchell852 changed the title GET api/{version}/users/:userid/deliveryservices returns inaccurate results GET api/{version}/users/:userid/deliveryservices returns incorrect results Jul 24, 2019
@ocket8888
Copy link
Contributor

I don't think this a bug. The docs for that endpoint say:

"Retrieves all Delivery Services assigned to the user."

which is what it does - although those are filtered by tenancy. Changing that behavior would be a breaking change, although personally I don't have a problem with that. Just that it may need to be restricted to the 1.4 API.

And for the record, I do think the behavior of the endpoint as-is are no longer useful, but using this endpoint to return Delivery Services may not be intuitive, since it has user and userId in the path. What if instead the tenant query parameter of a request to /deliveryservice was expanded to include sub-tenants? Or if that's undesirable - which I could see - maybe add a parameter accessibleTo to name a tenant and return all Delivery Services accessible to that tenant?

@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops and removed Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 labels Aug 28, 2019
@mitchell852
Copy link
Member Author

mitchell852 commented Sep 27, 2019

Honestly, I think the docs simply need to be changed to:

"Retrieves all Delivery Services assigned to the user via tenancy."

I really don't think this falls under the category of an api breaking change. It's just an overall change of behavior in the system.

It used to be that a user's delivery services were determined by the contents of the deliveryservice_tmuser table but now it's determined by the user's tenancy.

By using GET /users/:id/deliveryservices the api consumer is just looking for a list of delivery services accessible by the user. how that is determined (deliveryservice_tmuser or tenancy) should not be a concern of the api consumer.

@mitchell852
Copy link
Member Author

mitchell852 commented Sep 27, 2019

however, if we're not going to "fix" GET /users/:id/deliveryservices i think:

  1. it should be deprecated and
  2. it's probably a good idea to support something like `GET /deliveryservices? accessibleTo=tenant1 as @ocket8888 suggested

@mitchell852 mitchell852 added the Traffic Portal v1 related to Traffic Portal version 1 label Sep 27, 2019
@mitchell852 mitchell852 added the medium impact impacts a significant portion of a CDN, or has the potential to do so label Nov 5, 2019
@mitchell852
Copy link
Member Author

closing this in favor of #4402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working as intended medium impact impacts a significant portion of a CDN, or has the potential to do so Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1
Projects
None yet
Development

No branches or pull requests

2 participants