Add includes of route, domain, process for show app endpoint

[svkrieger]

Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:

• A short explanation of the proposed change: Add includes for route, domain and process to the /v3/apps/:guid endpoint.

• An explanation of the use cases your change solves The V2 app summary endpoint (/v2/apps/:guid/summary) is no longer available in V3. For a detailed problem description see https://github.com/cloudfoundry/cloud_controller_ng/issues/1990

This PR implements the first step mentioned in the above issue and makes it easier to retrieve domains, routes and processes associated with an app.

• [x] I have reviewed the contributing guide

• [x] I have viewed, signed, and submitted the Contributor License Agreement

• [x] I have made this pull request to the main branch

• [x] I have run all the unit tests using bundle exec rake

• [ ] I have run CF Acceptance Tests

[svkrieger]

One topic to discuss is how to handle large result sets of included resources. E.g if the app has 1000 routes.

An idea would be to set a limit for included resources e.g. if the app has up to 50 associated routes, the request will be processed and returns the routes. This could be either hardcoded or configurable.

If the app has more resources than allowed, the request would be rejected with a 403 - Forbidden with an according message that the result set is too large and that e.g. the /v3/apps/:guid/routes endpoint shall be used to retrieve the routes of the app, which supports pagination.

I understand the purpose of includes in general and also this PR to get an overview of an app's associated resources with a single API call. If we would add pagination to included resources, the client would have to make several calls, which is not different from using the already existing endpoints. So IMHO limiting the retrievable result sets in the includes seems appropriate. Also this should be fine for most apps and use cases.

I'm happy for comments and other suggestions.

[sethboyles]

Curious to know if the use case for an app with 1000 routes is a real one you all have encountered. If so, can you tell us a little about how apps use 1000 routes?

I agree that pagination on include statements would not be ideal. Perhaps we could limit the number of returned routes and then provide a link to routes to get the rest of the records, already prepopulated with the pagination parameters. I'm not sure a 403 is meant for such cases, and in general returning an error on a valid request like that seems a bit unorthodox.

@Gerg do you have any thoughts here?

@svkrieger I know you all were also thinking about an 'includes' for process stats--have you given any more thought as to how that might work?

[Gerg]

Re-posting this from https://github.com/cloudfoundry/cloud_controller_ng/issues/2879

---

We have been thinking about including *-to-many relationships for a while, but we never came up with a design we were super happy with. I think the closest we got was this proposal.

[Gerg]

Also, the thinking has generally been that include paths should correspond to relationship names on resources.

So, you wouldn't do something like GET /v3/apps?include=organization, you would do GET /v3/apps?include=space.organization, since apps don't have an organization relationship. This is important since resources can have multiple relationship paths to the same type of resource. For example, service instances (and soon routes) have an "owning" space, but can also be shared with other spaces.

For the routes/domains case here, destinations will probably complicate things. I wish I had a good idea on how to solve that, but I don't 😄.

Docs:

• https://github.com/cloudfoundry/cloud_controller_ng/blob/main/docs/v3_style_guide.md#including-related-resources
• https://v3-apidocs.cloudfoundry.org/version/release-candidate/#include

[svkrieger]

@sethboyles

• On our biggest landscapes we can see apps with 300+ routes. This is due to multitenancy.
• As the hard limit for routes per app seems to be 1000 and the max_page_size is hardcoded to 5000, we could consider to not implement any limit or pagination mechanism here.
• If we decide to implement a limit, your suggestion to show the first e.g. 50 and provide a link to the paginated result sounds reasonable.

@Gerg

• I think we don't want to go down the road of implementing *-to-many relationships. The requirement from users we have right now is to provide an endpoint which enables them to retrieve a single app and it's associated resources. Currently we also do not plan to implement this in the list endpoint, as this requires more thoughts on performance.
• Thanks for pointing me to the style guide regarding includes! Though that guideline seems to be breached already, as you can also retrieve the org of an app via include=org. But it makes sense to stick to route.domain instead of domain. The question would be, do we want to remove the include=org? Also at the moment when including space.organization both the space and the organization will be included. Is this on purpose? Or should it rather be like space includes the space and space.organization includes the org only and if you want both you'd need to specify include=space,space.organization?
• Destinations is a good point. I didn't check whether they are filtered for visibility yet. I guess a user should only be able to see the destinations, which the user has permissions for.
• Would include=route,route.domain work for you? I know that in the background the connection between app and route is made through destinations (DB table route_mappings), but not sure whether we need to add those here.

[Gerg]

I think we don't want to go down the road of implementing *-to-many relationships. The requirement from users we have right now is to provide an endpoint which enables them to retrieve a single app and it's associated resources. Currently we also do not plan to implement this in the list endpoint, as this requires more thoughts on performance.

Processes and Routes (and, by extension, Domains) are one-to-many relationships for Apps. The patterns we define for this endpoint should apply to other endpoints as well.

...Though that guideline seems to be breached already, as you can also retrieve the org of an app via include=org.... The question would be, do we want to remove the include=org?

GET /v3/apps/:guid?include=org was a previous implementation that we continue to support for backwards-compatibility reasons. It is not documented in the API docs, and it is not recommended for use.

Also at the moment when including space.organization both the space and the organization will be included. Is this on purpose? Or should it rather be like space includes the space and space.organization includes the org only and if you want both you'd need to specify include=space,space.organization?

We intentionally include space so that the included resources are "fully linked" to the root resource. This is primarily useful for list endpoints. Otherwise, you wouldn't be able to tell which apps are in which organizations. See the relevant section of this proposal.

Would include=route,route.domain work for you? I know that in the background the connection between app and route is made through destinations (DB table route_mappings), but not sure whether we need to add those here.

Thinking about the list case (GET /v3/apps?include=route), how would clients associate routes back to the root apps? I suppose the app guids in the route's destinations would be serviceable, though maybe not ideal. See the relevant section of this proposal.

[svkrieger]

We tried to come up with a concept which would work for show and list endpoints as well. We liked the idea to use the already present pagination for included resources like when including spaces for orgs, the pagination link would look like /v3/spaces?organization_guids=<list of org guids>&per_page=50&page=2. Though, this can lead to long URLs when querying for a big amount of orgs. These lead to URI too long errors, which we could bypass to a certain amount with enlarging the buffer size in the nginx config.

Nevertheless we would drop the idea of implementing *-to-many relationships for now. We spoke to one of the biggest consumers of the API on one of our landscapes and came to a conclusion that additional calls to the API for fetching app-related data is ok. What they would like to see included in the /v3/apps:guid endpoint is the ssh_enabled feature flag as well as environment variables. These are viable options for us. When we got a draft PR ready it will be linked to https://github.com/cloudfoundry/cloud_controller_ng/issues/1990