-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Design documents for Quadlet API Endpoints #27240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
See https://issues.redhat.com/browse/RUN-3555 Signed-off-by: Jhon Honce <[email protected]> Helped-by: Nicola Sella <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jwhonce The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Jhon Honce <[email protected]>
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
Signed-off-by: Jhon Honce <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @jwhonce, I did a quick pass over, will take another look with fresh eyes as well.
@@ -0,0 +1,2955 @@ | |||
openapi: 3.1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we okay to use openapi version 3.x? We are using https://github.com/go-swagger/go-swagger which currently only supports version 2.x
"/{name}": | ||
parameters: | ||
- name: name | ||
in: query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in: query | |
in: path |
schema: | ||
$ref: "#/components/schemas/Error" | ||
components: | ||
parameters: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to use the $ref reference here like used elsewhere?:
parameters:
- $ref: "#/components/parameters/NameParam"
- $ref: "#/components/parameters/ForceParam"
|
||
### Challenges | ||
|
||
1. Where should the unit files be written? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already decided, podman quadlet install
has complete logic for this.
1. XDG_CONFIG_HOME is not currently always available | ||
2. Need to research if HOME is set when SocketActivated() | ||
3. What is the server environment for the new TLS endpoint? | ||
2. How to safely and securely invoke systemd reloads or other operations from API service. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really a problem? I don't think reloading systemd from an active unit is a problem.
|
||
## **Target Podman Release** | ||
|
||
5.Y+ (with no breaking changes we can release on any Y branch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6.0 seems safest
security: | ||
- main_auth: [] | ||
paths: | ||
"/": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The complete lack of comments in this file makes it very difficult to read, IMO
- quadlet | ||
summary: Get quadlet by name | ||
description: | | ||
Read the quadlet system unit files by name and render them as JSON. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why JSON? Why are we mutilating the unit file before transfer?
schema: | ||
$ref: "#/components/schemas/Error" | ||
|
||
"/{name}/build": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we doing separate endpoints for each unit type? I don't see a reason not to unify them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The underlying APIs are entirely unified, and for a good number of cases expect to install multiple Quadlets of varying types at the same time)
|
||
## **Short Summary** | ||
|
||
The aim is to offer a collection of API endpoints for managing quadlets on a remote Podman API server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we used "quadlets" or "Quadlets"? From the blog posts on line, it looks to be capitalized.
See https://issues.redhat.com/browse/RUN-3555
Signed-off-by: Jhon Honce [email protected]
Helped-by: Nicola Sella [email protected]