Deuxfleurs
/
garage
41
37
Fork 55
Code Issues 70 Pull Requests 6 Releases 29 Wiki Activity

OpenAPI specification of admin APIv1 #672

Merged
lx merged 15 commits from api-v1 into main 2023-11-29 15:42:47 +00:00
Conversation 10 Commits 15 Files Changed 7 +1471 -24
quentin commented 2023-11-22 08:00:38 +00:00
Owner
Copy Link

Changed endpoint:

  • GetNodes (GetClusterStatus)
    • new field garageFeatures
    • new field rustVersion
    • new field dbEngine
    • KnownNode is now a vec and not a map
    • KnownNodeResp has a new field id, prev the key of the map (NodeNetworkInfo)
    • switch to camel case
      • knownNodeResp structure (NodeNetworkInfo)
      • other nodes
  • GetClusterHealth
    • Newly implemented
  • GetLayout (GetClusterLayout)
    • Add NodeRoleChange
    • Fix doc to inform that capacity now represents bytes
  • AddLayout (UpdateClusterLayout)
    • Add NodeRoleChange
    • Inform that returns schemas/ClusterLayout
    • Fix doc to inform that capacity now represents bytes
  • ApplyLayout (ApplyClusterLayout)
    • Does not return 204 anymore but a special response containing the ClusterLayout object + a message
  • GetKey, SearchKey (GetKeyInfo)
    • add showSecretKey
  • UpdateKey
  • CreateKey
    • Set name as nullable
  • ImportKey
    • Set name as nullable

Regenerating SDK:

  • Golang
  • Python
  • Javascript

Testing SDK:

  • Golang
  • Python
  • Javascript

GetClusterHealth

  • Implement
  • Regenerate SDKs
  • Test & document it

Pre-release

  • Collect feedbacks from users
  • Ask a review from LX
  • Merge the SDKs PR

Update the doc:

  • Publish redoc on GarageHQ
  • Update the link in the markdown files to point to v1 and not v0 anymore (but keep both)

Fixes #671.

Changed endpoint: - [x] GetNodes (GetClusterStatus) - [X] new field garageFeatures - [X] new field rustVersion - [X] new field dbEngine - [x] KnownNode is now a vec and not a map - [x] KnownNodeResp has a new field `id`, prev the key of the map (NodeNetworkInfo) - [x] switch to camel case - [x] knownNodeResp structure (NodeNetworkInfo) - [x] other nodes - [X] GetClusterHealth - [X] Newly implemented - [x] GetLayout (GetClusterLayout) - [X] Add NodeRoleChange - [X] Fix doc to inform that capacity now represents bytes - [x] AddLayout (UpdateClusterLayout) - [X] Add NodeRoleChange - [X] Inform that returns schemas/ClusterLayout - [X] Fix doc to inform that capacity now represents bytes - [X] ApplyLayout (ApplyClusterLayout) - [X] Does not return 204 anymore but a special response containing the ClusterLayout object + a message - [x] GetKey, SearchKey (GetKeyInfo) - [x] add showSecretKey - [x] UpdateKey - [x] CreateKey - [x] Set name as nullable - [x] ImportKey - [x] Set name as nullable Regenerating SDK: - [x] Golang - [x] Python - [x] Javascript Testing SDK: - [x] Golang - [x] Python - [x] Javascript GetClusterHealth - [x] Implement - [x] Regenerate SDKs - [x] Test & document it Pre-release - [x] Collect feedbacks from users - [ ] Ask a review from LX - [x] Merge the SDKs PR Update the doc: - [x] Publish redoc on GarageHQ - [x] Update the link in the markdown files to point to v1 and not v0 anymore (but keep both) Fixes #671.
quentin added 1 commit 2023-11-22 08:00:39 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
c63b446989
skeleton for api v1
quentin added 1 commit 2023-11-22 08:29:21 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
d1d1940252
Health info message now advertises API v1
quentin added 1 commit 2023-11-22 13:25:18 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
9b24d7c402
Upgrade GetNodes
quentin added 1 commit 2023-11-22 14:24:51 +00:00
continuous-integration/drone/push Build is failing Details
continuous-integration/drone/pr Build is passing Details
e3cd6ed530
port GetLayout and AddLayout
quentin added 1 commit 2023-11-22 16:50:06 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
20b3afbde4
Port layout endpoints
quentin added 1 commit 2023-11-22 17:05:32 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
0d415f42ac
Port GetKeyInfo by adding showSecretKey query param
quentin added 1 commit 2023-11-22 17:14:53 +00:00
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details
3684c29ad0
handle key changes
quentin commented 2023-11-22 17:48:08 +00:00
Author
Owner
Copy Link

The Python OpenAPI generator generates malformed URL on the SearchKey endpoint:

2023-11-22T17:45:32.456141Z  INFO garage_api::generic_server: 127.0.0.1:56118 GET /v1/key?search=openapi?showSecretKey=True

Expected: ?search=openapi&showSecretKey=True
Got: ?search=openapi?showSecretKey=True (the 2nd question marks must be an ampersand)

Edit: fixed in multiple iterations:

  • We don't template anymore query parameters
  • Boolean in query parameters were replaced by string enum (as Garage does not support True with an uppercase T)
The Python OpenAPI generator generates malformed URL on the SearchKey endpoint: ``` 2023-11-22T17:45:32.456141Z INFO garage_api::generic_server: 127.0.0.1:56118 GET /v1/key?search=openapi?showSecretKey=True ``` Expected: `?search=openapi&showSecretKey=True` Got: `?search=openapi?showSecretKey=True` (the 2nd question marks must be an ampersand) Edit: fixed in multiple iterations: - We don't template anymore query parameters - Boolean in query parameters were replaced by string enum (as Garage does not support True with an uppercase T)
quentin added 1 commit 2023-11-22 19:40:01 +00:00
continuous-integration/drone/pr Build is failing Details
continuous-integration/drone/push Build is passing Details
4f473f43c9
Change how query parameters are handled
quentin added 1 commit 2023-11-22 20:05:47 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
2d37e7fa39
convert showsecretkey from bool to enum
quentin commented 2023-11-22 20:36:53 +00:00
Author
Owner
Copy Link

As I created a new file instead of patching v0, it's not possible to see from Gitea interface what changed. This URL shows the difference: diff open api spec

As I created a new file instead of patching v0, it's not possible to see from Gitea interface what changed. This URL shows the difference: [diff open api spec](https://git.deuxfleurs.fr/Deuxfleurs/garage/compare/d1d1940252d2a55e3b56386d6eaefdf5c09af8be...api-v1)
unrob reviewed 2023-11-23 01:51:15 +00:00
doc/api/garage-admin-v1.yml Outdated
@ -0,0 +410,4 @@
Delete a key from the cluster. Its access will be removed from all the buckets. Buckets are not automatically deleted and can be dangling. You should manually delete them before.
parameters:
- name: access_key
in: path
unrob commented 2023-11-23 01:51:15 +00:00
Contributor
Copy Link

Thanks for the super fast response on #671! Updated my fork of the terraform provider but can't delete a key (with the golang sdk).

It issues a request to DELETE /v1/key without an id query string parameter (which i think the endpoint consumes), and here it says it needs to go on the path, but there's no {access_key} in there. Should this be name: id, in: query?

Thanks for the super fast response on #671! Updated [my fork](https://git.rob.mx/nidito/terraform-provider-garage/pulls/1) of the terraform provider but can't delete a key (with the golang sdk). It issues a request to `DELETE /v1/key` without an `id` query string parameter (which i think the [endpoint](https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/api/admin/router_v1.rs#L114) consumes), and here it says it needs to go on the path, but there's no `{access_key}` in there. Should this be `name: id, in: query`?
👍 1
quentin commented 2023-11-23 07:57:50 +00:00
Author
Owner
Copy Link

Indeed you're right.

Should be fixed now.

Indeed you're right. Should be fixed now.
quentin marked this conversation as resolved
quentin added 1 commit 2023-11-23 07:50:26 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
814b3e11d4
fix query parameters for keys
quentin added 1 commit 2023-11-23 09:02:51 +00:00
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details
1caa6e29e5
capacity is int64
quentin commented 2023-11-23 09:03:37 +00:00
Author
Owner
Copy Link

Discovered a new issue: capacity must be tagged as int64. Fixing the SDK...

Discovered a new issue: capacity must be tagged as int64. Fixing the SDK...
quentin force-pushed api-v1 from 12ee23f75a to 9f1043586c 2023-11-23 09:16:24 +00:00 Compare
quentin added 1 commit 2023-11-23 09:20:46 +00:00
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details
68d23cccdf
disable int64 finally for now
quentin commented 2023-11-23 09:51:16 +00:00
Author
Owner
Copy Link

Current commits to test:

go get git.deuxfleurs.fr/garage-sdk/garage-admin-sdk-golang@ffd9578e975e886d9075a4f2e710117893de8ac7

npm install git+https://git.deuxfleurs.fr/garage-sdk/garage-admin-sdk-js.git#f7b4e20e4292883f1eaa5cd6cd5287aa550ff605

pip install git+https://git.deuxfleurs.fr/garage-sdk/garage-admin-sdk-python@8c3a7daa8a6d305dc0ee9082ce1f28682951e7b8

Still missing:

  • Maybe have full tests for Golang+Javascript would be great, currently only the Python SDK has tests for most of its endpoints
  • Cluster Health
Current commits to test: ``` go get git.deuxfleurs.fr/garage-sdk/garage-admin-sdk-golang@ffd9578e975e886d9075a4f2e710117893de8ac7 npm install git+https://git.deuxfleurs.fr/garage-sdk/garage-admin-sdk-js.git#f7b4e20e4292883f1eaa5cd6cd5287aa550ff605 pip install git+https://git.deuxfleurs.fr/garage-sdk/garage-admin-sdk-python@8c3a7daa8a6d305dc0ee9082ce1f28682951e7b8 ``` Still missing: - [ ] Maybe have full tests for Golang+Javascript would be great, currently only the Python SDK has tests for most of its endpoints - [ ] Cluster Health
lx commented 2023-11-27 09:32:31 +00:00
Owner
Copy Link

Should we merge now, or are we waiting for something ?

Should we merge now, or are we waiting for something ?
fivebats commented 2023-11-27 23:38:50 +00:00
Contributor
Copy Link

Have you seen #671 (comment) ?

Discovered while testing the go sdk.

Have you seen https://git.deuxfleurs.fr/Deuxfleurs/garage/issues/671#issuecomment-7374 ? Discovered while testing the go sdk.
quentin added 1 commit 2023-11-28 08:34:16 +00:00
continuous-integration/drone/pr Build is passing Details
continuous-integration/drone/push Build is passing Details
3908619eac
add ClusterHealthReport endpoint to the API
lx commented 2023-11-28 11:57:00 +00:00
Owner
Copy Link

Just saw this that might be interesting for automatically generating the OpenApi document from the code: https://github.com/juhaku/utoipa

I'm reluctant now to make further changes to the admin api given how much work it is to fix all the generated sdk frameworks, but if we have something like this maybe it can be made easier.

Just saw this that might be interesting for automatically generating the OpenApi document from the code: https://github.com/juhaku/utoipa I'm reluctant now to make further changes to the admin api given how much work it is to fix all the generated sdk frameworks, but if we have something like this maybe it can be made easier.
quentin commented 2023-11-28 14:17:51 +00:00
Author
Owner
Copy Link

I have a new issue with the SDK: https://github.com/OpenAPITools/openapi-generator/issues/17208

I have a new issue with the SDK: https://github.com/OpenAPITools/openapi-generator/issues/17208
quentin added 1 commit 2023-11-28 15:18:35 +00:00
continuous-integration/drone/push Build is passing Details
continuous-integration/drone/pr Build is passing Details
8088690650
fix the doc
quentin commented 2023-11-28 15:32:13 +00:00
Author
Owner
Copy Link

I did not test the golang SDK as extensively as I would like, but it's way better now (see example/golang/main.go for example).

Concerning the generation of the OpenAPI spec, my opinion on the subject is that there is no shortcut: we will have to describe the API in a formalized language anyways, we will have to document it and publish it anyways, and we will have to manually fix / find workaround for impedance mismatches between languages (like this golang one or this python one). I think it will simply move the work of specifying the API from me to Alex in the end basically, which could be (or not be) a desired outcome.

More globally, there is in general 2 approaches to OpenAPI: a code-centric and a protocol-centric. In the code centric, what matters is the code and the REST API spec is a byproduct of it you can generate. In the protocol-centric, you define the API and then implement your code following its specification, often by generating the server as well as the client.

Personally I don't like code generation and I prefer protocol-centric approaches: we define the API specification and then we implement it by using as few code generation as possible. By following this path, we acknowledge that the API, the contract, the interface is more important than the code, the algorithm behind, and that it will survive it.

But to be 100% fair, today we still have a code-centric approach as we did not discuss the API design before its implementation.

In this vein, I really like the work / the idea of Amazon on the subject, with Smithy and the idea they present in their talks. But currently, Smithy was a too high step for me, that's why I chose OpenAPI with which I'm more confortable.

But I must admit that my time is also limited, and if someone has the time and the will to maintain better Garage SDKs than me, that are shipped faster, I am very open to a maintainer transition, and it could be the opportunity to change how we handle the SDK generation.

I did not test the golang SDK as extensively as I would like, but it's way better now (see [example/golang/main.go](https://git.deuxfleurs.fr/garage-sdk/garage-admin-sdk-generator/src/commit/6a07252d5618645f0aaa2160f7e90b44f65c92b9/example/golang/main.go) for example). Concerning the generation of the OpenAPI spec, my opinion on the subject is that there is no shortcut: we will have to describe the API in a formalized language anyways, we will have to document it and publish it anyways, and we will have to manually fix / find workaround for impedance mismatches between languages (like [this golang one](https://github.com/OpenAPITools/openapi-generator/issues/17208) or [this python one](https://github.com/OpenAPITools/openapi-generator/issues/1260)). I think it will simply move the work of specifying the API from me to Alex in the end basically, which could be (or not be) a desired outcome. More globally, there is in general 2 approaches to OpenAPI: a code-centric and a protocol-centric. In the code centric, what matters is the code and the REST API spec is a byproduct of it you can generate. In the protocol-centric, you define the API and then implement your code following its specification, often by generating the server as well as the client. Personally I don't like code generation and I prefer protocol-centric approaches: we define the API specification and then we implement it by using as few code generation as possible. By following this path, we acknowledge that the API, the contract, the interface is more important than the code, the algorithm behind, and that it will survive it. But to be 100% fair, today we still have a code-centric approach as we did not discuss the API design before its implementation. In this vein, I really like the work / the idea of Amazon on the subject, with [Smithy](https://smithy.io/2.0/index.html) and the idea [they present in their talks](https://www.youtube.com/watch?v=3GpZzu4guTE). But currently, Smithy was a too high step for me, that's why I chose OpenAPI with which I'm more confortable. But I must admit that my time is also limited, and if someone has the time and the will to maintain better Garage SDKs than me, that are shipped faster, I am very open to a maintainer transition, and it could be the opportunity to change how we handle the SDK generation.
quentin changed title from WIP: Implement APIv1 to Implement APIv1 2023-11-28 15:32:17 +00:00
quentin changed title from Implement APIv1 to OpenAPI specification of APIv1 2023-11-28 15:32:41 +00:00
quentin changed title from OpenAPI specification of APIv1 to OpenAPI specification of admin APIv1 2023-11-28 15:32:52 +00:00
quentin commented 2023-11-28 15:43:22 +00:00
Author
Owner
Copy Link

I have merged all my PRs in the garage-sdk organization. I let you synchronize this PR with main, merge it as you like (feel free to rebase/squash the PR). Feel free to push modifications directly on the branch if required.

If we find issues in the SDK later, I will handle them as bugs and fix them as they arrive.

I have merged all my PRs in the garage-sdk organization. I let you synchronize this PR with main, merge it as you like (feel free to rebase/squash the PR). Feel free to push modifications directly on the branch if required. If we find issues in the SDK later, I will handle them as bugs and fix them as they arrive.
lx merged commit a8b0e01f88 into main 2023-11-29 15:42:47 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
AdminAPI

Issues concerning the administration API

  
Bug

   
Check AWS

The behaviour should be checked against AWS S3

  
CI

Issues with Nix, Drone, Rust Compiler, etc.

  
Correctness

Issues of theoretical correctness that may impact edge-case scenarios

  
Critical

   
Documentation

   
Ideas

For abstract ideas/proposal to make Garage better, to better communicate our not yet achieved aims

  
Improvement

   
Low priority

For issues that we don't want to spend time on until made absolutely necessary

  
Newcomer

A bug that should be newcomer friendly.

  
Performance

   
S3 Compatibility

   
Testing

   
Usability

Propositions to make Garage easier to operate, by improving error reporting, CLI, etc.

No Label
AdminAPI
Bug
Check AWS
CI
Correctness
Critical
Documentation
Ideas
Improvement
Low priority
Newcomer
Performance
S3 Compatibility
Testing
Usability
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Deuxfleurs/garage#672
No description provided.
Delete Branch "api-v1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?