search

EasyDynamics / oscal-rest

An initial OpenAPI definition of an OSCAL REST API.
Creative Commons Attribution Share Alike 4.0 International
38 stars 13 forks source link

Add GET operation to API base path #67

Closed folksgl closed 2 months ago

folksgl commented 2 years ago

Any request to the base path of the api currently returns an error message, which seems a bit unfriendly to me. There are certainly use-cases where I simply want to know whether the API at a given endpoint is the API that I expect, or even what version of the API is being used. This change introduces an extremely simple GET operation on the base bath of the API that currently returns just the version of the API. It can be augmented to return additional data, but version seems like a good starting point. This endpoint may make a great candidate for an initial health-check of the running API.

folksgl commented 2 years ago

I thought about this a little before deciding to open the PR. I believe that no matter what, the base URL should return something. In my personal experience, the first thing I do when interacting with a new API endpoint is check out what the base path gives me. In lieu of the base url being used for anything right now, API version seems to be a very safe bet.

That being said, I don't intend this to be a replacement for a healthcheck or management API in the future. For example, Prometheus and many other popular OSS tools have dedicated endpoints for determining node health, version, status, etc, and don't have anything returned at the base path (they 404). I'm not sure yet what the "correct" thing for the basepath to do is just yet.

laurelmay commented 2 years ago

In lieu of the base url being used for anything right now, API version seems to be a very safe bet

Yeah, so I see where you're going with this. But I could also see GET / being a request for either all support root object types, all documents the user has access too (probably requires #55 to be technically feasible), or (as done in this PR) metadata about the server implementation itself.

If we do this, I think I'd like to make sure we've considered what future expansions to this object look like and whether we'd consider that a breaking change. I don't think there are any consumers or implementations of this API today besides EasyDynamics/oscal-rest-service and EasyDynamics/oscal-react-library but I still want to make sure we're aware of when we're introducing a breaking change (like #55 will be).

I also think we should talk about whether metadata should be returned as part of many responses objects (I've really enjoyed APIs that do this; whether it's API version, server-side repsonse latency, read/write consumption used, etc) and maybe we should make this more generic than just / and have a consistent object structure that we use? But I wouldn't want to use metadata as the key because that means something in OSCAL (but like $metadata might work).

I am going to ponder this a bit and maybe try to find some time with a whiteboard. Or maybe I just need to relax my ethics on breaking API changes (because again, #55 is staring at us) when we're pre-0.1 (though I'd like to release that in the semi-near future).

brian-comply0 commented 2 months ago

Closing as it is out of date with the revised spec.