[API] Clean up the cluster-service API

weaveworks / weave-gitops-enterprise

This repo provides the enterprise level features for the weave-gitops product, including CAPI cluster creation and team workspaces.

https://docs.gitops.weave.works/

Apache License 2.0

160 stars 29 forks source link

Open foot opened 1 year ago

foot commented 1 year ago

Motivation: customers want to use our APIs! Lets tidy them up before giving users access

[ ] Make sure all methods have docstrings
[ ] Make sure all ambiguous param names have examples
[ ] Tags are a mix of TitleCase and lowercase
[x] For most resources we want to have
- [x] "single pane of glass" endpoint, e.g. GET /api/v1/helm-releases, that returns all helm-releases on all cluster on all namespace that the user can see.
- [x] Then for accessing a specific resource you have to use the very long path: /v1/namespaces/{clusterNamespace}/clusters/{clusternname}/namespace/{resNamespace}/helm-releases/{resName}
[x] Some endpoints have legacy naming:
- [x] POST /api/clusters to create a PR from a template, this should be POST /v1/templates/pull-request
[x] Make sure all message fields are snake_case (@simon double check this std. w/ google etc.)
[x] Check docstrings all make sense, and omit the name of message (function) in the proto file.
[x] kebab-case the resource names in the api endpoints, e.g. /helm-releases/ not /helm_releases/ (this is an OSS resource)

Tasks here:

foot commented 1 year ago

foot commented 1 year ago

Review all the places the UI is calling the static Service object and make sure we are typing it properly and not using any.

foot commented 1 year ago

Review all the places the UI is calling the static Service object and make sure we are typing it properly and not using any.

Templates, which was the main concern, seems to be all typed now which is great!

Explorer is also typed, think we are all good with this one.

foot commented 1 year ago

Oops, chatting w/ @sarataha , we realised clusters are namespaced... so this URL doesn't have enough info in it:

Options:

/api/v1/namespaces/{clusterNamespace}/clusters/{clusterName}/namespaces/{namespace}/helm_releases/{name}
/api/v1/clusters/{clusterNamespace}/{clusterName}/namespaces/{namespace}/helm_releases/{name}
/api/v1/clusters/{clusterNameThatIncludeNamespace}/namespaces/{namespace}/helm_releases/{name}
/api/v1/helmreleases # all helmrelease on all the clusters, can be filtered
/api/v1/helmreleases/clusters/{clusterName}/namespaces/{namespace}/{helmReleaseName}
/api/v1/helmreleases/clusters/{clusterName}
/api/v1/helmreleases/namespace/{namespace}

clusterNameThatIncludeNamespace is usually default%3dmy-cluster