[FEATURE] Expose get services registration Admin API

[petyos]

Is your feature request related to a problem? Please describe.
We would like to move features from Admin BB to Core BB. One of these is the get building-blocks API in the Admin BB which is called by the admin app. In Core BB the corresponding entity is ServiceReg. We would like to deprecate the API in the Admin BB and start using the one in the Core BB.

Describe the solution you'd like
Admin BB building-blocks gives the following structure:

[
    {
        "key": "rewards",
        "name": "Rewards",
        "version_url": "{apis_url}/version",
        "environments": [
            {
                "key": "dev",
                "health_status": "working",
                "version": "1.0.8",
                "apis_url": "https://api-dev.rokwire.illinois.edu/rewards",
                "web_url": null
            },
            {
                "key": "test",
                "health_status": "down",
                "version": "",
                "apis_url": "https://api-test.rokwire.illinois.edu/rewards",
                "web_url": null
            },
            {
                "key": "prod",
                "health_status": "down",
                "version": "",
                "apis_url": "https://api.rokwire.illinois.edu/rewards",
                "web_url": null
            }
        ]
    },
    ...
]

We would like to expose GET admin/organization/service-regs API which gives the services for the current organization. The missing fields have to be added so that the admin app to call the new created API from the Core BB.

[shurwit]

Hi @petyos, thanks for opening this issue! I have a couple of comments.

As I mentioned in #325, I believe the only change necessary from the current service registration system we have is the need to add a path to the version endpoint. I do not believe it is necessary to store the "health_status" of each service, as this would require an admin to manually update this status anyway. I believe simply including the "version" path would allow the admin client to make a request to this version API on each service to determine both the current version and status of the service in that environment (ie. if the request fails the status is "down" and if it succeeds it is "working").

I also do not believe it makes sense to try to keep the structure from the Admin BB listed above. If we want to retrieve the information for the services in a different environment, we should make a request to the Core BB in that environment since the environments should have no knowledge of each other. With this in mind, I would suggest that we use the existing structure for the GET /system/service-regs endpoint (with the additional version_path field), move this endpoint to /admin/service-regs, and remove the specific permission requirement for this API.

Let me know if you have any questions or feedback on these suggestions. Thanks!

[petyos]

Hi @shurwit ,
thanks for your comments.

As I remember the Admin BB populate the health status for every BB and environment automatically but not making the admin to set it manually. BTW, this is what the admin would like to find out instead of setting it. Your suggestion is removing the knowledge from the back-end about the BBs health status and putting it to the admin app(which is not an issue) but if we want to populate the entry screen(where we show all BBs with health status for every env) in the admin app then the client will need to make 17 services x 3 environments = 51 requests when displaying the screen. On other side we unload the back-end from logic which is good. Also having in mind that the admin app will not be used by thousands users we could accept having the status health of the services to happen in the admin app not loading the back-end with many requests as this screen would be shown rarely.

I am ok with your suggestion but we will also need to update the admin app to do the work for this screen.

I would suggest not removing GET /system/service-regs endpoint. It makes sense this API to exist and to give all service regs.
We will add another one in the admin section which will give only the services for the current organization. The admin of one organization must see only the services for that organization.

Let me know your thoughts. Thanks

cc @stefanvit

[shurwit]

Hi @petyos, thanks for following up!

As I remember the Admin BB populate the health status for every BB and environment automatically but not making the admin to set it manually. BTW, this is what the admin would like to find out instead of setting it. Your suggestion is removing the knowledge from the back-end about the BBs health status and putting it to the admin app(which is not an issue) but if we want to populate the entry screen(where we show all BBs with health status for every env) in the admin app then the client will need to make 17 services x 3 environments = 51 requests when displaying the screen. On other side we unload the back-end from logic which is good. Also having in mind that the admin app will not be used by thousands users we could accept having the status health of the services to happen in the admin app not loading the back-end with many requests as this screen would be shown rarely.

Thanks for pointing this out. I did not realize that the Admin BB was making these requests. It looks to me like the Admin BB is making these requests every time the building blocks are retrieved (https://github.com/rokwire/admin-building-block/blob/7c44b41cec35e14d7ab5fd3da18a42efde5b6f57/core/services.go#L60). This is going to be the exact same number of requests with the same amount of overhead, however it is also adding the burden of making these requests to our back-end resources. While there may be ways to cache this information to improve performance if we do keep it on the back end, I believe the version requests are simple and small enough that it is better to just let the client make them itself to ensure the information is as up-to-date as possible.

I am ok with your suggestion but we will also need to update the admin app to do the work for this screen.

Yes, we will need to make some additional changes in the admin app if we take this approach, but I believe some changes will be necessary either way.

I would suggest not removing GET /system/service-regs endpoint. It makes sense this API to exist and to give all service regs. We will add another one in the admin section which will give only the services for the current organization. The admin of one organization must see only the services for that organization.

I agree that it probably makes sense to keep the GET /system/service-regs as you suggested.

Now that I understand how the admin app is currently working a bit better, I have a couple more issues to bring up.

1. As I mentioned previously, the services in different environment should not have information about each other. However, I now know the admin app displays the status across environments all at once. I am not entirely sure that this makes sense (eg. what if an admin is only supposed to have access in the dev environment? Why should they be able to see data from prod?), but if we want to keep it this way, the new /admin/service-regs endpoint we discussed probably shouldn't have any form of authorization. This is because we do not want to login and manage tokens for all of the environments at the same time. The service registrations are meant to be public records and already are exposed publicly on the /bbs and /tps subrouters, so it is ok if this endpoint is not protected. If we decide to only show the status for the current environment, we can protect this endpoint with the admin user auth.
2. The second issue is that you mention loading only the services for the current organization. As far as I know, the services are currently associated with a specific app/org. The admin app app/org will probably need to have all of the services used by the organization, so we may be able to use that app/org for this purpose, but I wanted to see if that is what you had in mind. If we do decide to make the /admin/service-regs endpoint public as discussed in point 1 above, we will need to include app ID and org ID query parameters for this to be specified since we will not have a token to get these values from. If we decide to only show the status for the current environment, we can just get these values from the user's token.

Let me know if you have any thoughts or questions about any of the points above. Thanks again!

[petyos]

Hi @shurwit ,
Yes, I agree that displaying all environments health statuses into a single panel makes difficulties. I also agree that the admin could easily switch the environment and see the health statuses for another environment as you already mentioned the environments do not know each other and they are independent.

If @pmarkhennessy is ok to populate the Health Status Version table with the statuses only for the current environment(but not for all environments) then I would suggest to protect the /admin/service-regs endpoint with the admin token and giving the services only for the current organization. What I mean is:

• got the org id from the token
• find all application_organizations for that org id
• get all services ids for the the found application organizations
• give the services for the found services ids.

I mean this is what you describe in point two but to be sure we are on the same page.

[shurwit]

Hi @petyos, I spoke to @pmarkhennessy and @johnmpaul and they said we should keep all of the environments together since they typically use that to compare the versions across environment. I think with this in mind we will need to make this service registration endpoint public as discussed above.

Thank you for the description of the organization services. I think the approach you suggest sounds good.

[shurwit]

After further discussion, our conclusion for now is that we should just have public admin APIs that expose the list of all service registrations. We should also add the version_path field to the service registration records to specify the path off of the host where the client can find the public version endpoint of the service.