feat(api): catch the HTTP API up with the CLI (secret get/set/create-tls, pvcSize) - #12
Merged
Conversation
…g to a real cluster
The HTTP API had drifted behind the CLI: `secret get` and `secret set` (v2.10.0)
had no endpoints, and `secret create-tls` never had one at all. Verified against
a running server — GET/PATCH /api/v1/secrets/{name} and POST
/api/v1/secrets/tls all answered 405.
- GET /api/v1/secrets/{name} lists the keys, withholding values unless
?showValues=true, and answers 404 rather than 500 for a missing Secret.
- PATCH /api/v1/secrets/{name} merges keys, the counterpart to POST, which
replaces the data and drops what it was not given. Its response carries the
note that running pods keep the old values until restarted.
- POST /api/v1/secrets/tls creates a BYO-cert Secret from inline PEM, the
parity gap for `secret create-tls`.
SecretInfo keeps its capitalised JSON field names: they are what openapi.yaml
already publishes, so renaming them to match the rest of the API would break
existing consumers. Noted in the schema description instead.
Two things this does NOT need: x-orcinus-env-from-secret already works over
HTTP (conversion runs in the server process), and GET /api/v1/secrets already
returned KeyNames.
Documents a limitation found while testing: compose keys that point at local
files — env_file, configs/secrets with file:, bind mounts — cannot work over
HTTP, because only the compose text is uploaded and there is nothing to resolve
paths against. The request fails naming the missing path.
Fixes a hazard in the api test harness. Config{Kubeconfig: ""} falls through to
$KUBECONFIG and then ~/.orcinus/kubeconfig, so a test that reached applier()
wrote to whatever cluster the machine pointed at — the new route tests created a
TLS Secret on a live cluster before this was caught. testServer now pins an
unresolvable kubeconfig so those routes fail with 503, with a guard test that
fails if isolation ever breaks. Existing tests never hit this because they only
exercised convert and a 404.
The raw-body deploy path builds its DeployRequest from query params and read every field except pvcSize, so `POST /api/v1/deploy?pvcSize=10Gi` with a YAML body silently produced a 1Gi claim. Silent is the bad part: a PVC cannot be resized after the fact unless the StorageClass allows expansion, so the only way out is deleting the claim and its data. openapi.yaml described the raw-body path as "options as query params" without listing any, which is why the gap went unnoticed. Both /convert and /deploy now enumerate them. The JSON body path was always correct; every other CLI deploy flag already had an equivalent on both paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The HTTP API had drifted behind the CLI. Probed against a running server:
secret getandsecret setshipped in v2.10.0 with no endpoints, andsecret create-tlsnever had one — only the generic and docker-registry writers existed.Change
GET /api/v1/secrets/{name}— keys listed, values only with?showValues=true; 404 for a missing Secret rather than a blanket 500.PATCH /api/v1/secrets/{name}— merges, the counterpart toPOSTwhich replaces and drops keys it was not given. Creates when absent. The response carries the restart caveat.POST /api/v1/secrets/tls— BYO cert from inline PEM (the CLI reads files; over HTTP the contents come inline).openapi.yamlanddocs/API.mdupdated with all three.Two things that needed nothing
x-orcinus-env-from-secretalready works over HTTP — conversion runs inside the server process. Confirmed via/convert: the rendered output contains theenvFrom/secretRef.GET /api/v1/secretsalready returnedKeyNames, since it serializes the shared type.Deliberately not changed
SecretInfokeeps its capitalised JSON fields (Name,Keys,KeyNames). They are whatopenapi.yamlalready publishes, so renaming to match the camelCase used elsewhere would break existing consumers. The inconsistency is now noted in the schema description; worth a deliberate breaking change later, not a drive-by one.Limitation documented, not fixed
Compose keys that point at local files cannot work over HTTP — only the compose text is uploaded, so there is nothing to resolve paths against:
Same for
configs:/secrets:withfile:and bind mounts. Now called out inAPI.md, pointing atenvironment:orx-orcinus-env-from-secretinstead. Fixing it properly means multi-file upload, which is a separate design question.Test harness hazard — please read
Config{Kubeconfig: ""}falls through to$KUBECONFIG, then~/.orcinus/kubeconfig. So an api test that reachedapplier()wrote to whatever cluster the machine points at. My new route tests did exactly that and created akubernetes.io/tlsSecret namedcon a live cluster (tls.crt: eA==— base64 of the literal"x"from the test body) before I caught it. It has been deleted.testServernow pins an unresolvable kubeconfig so those routes fail with 503, plusTestTestServerCannotReachAClusterfails if that isolation ever breaks. Existing tests never tripped this because they only exercisedconvertand a plugin 404.Verification
Live against a real 3-node k3s cluster, through a running
orcinus api: create → get (redacted) → get?showValues=true→ patch (merge; the untouched key survived) → create-tls → 404 on a missing name → delete. All test Secrets cleaned up; the cluster's own 7 secrets untouched.Offline: 151 tests pass, including 4 new api tests (routes wired, writes validate before touching a cluster, openapi covers the new surface, and the isolation guard).