Skip to content

fix: authenticate quickstart data-plane API requests - #12

Open
KubeKyrie wants to merge 1 commit into
RLinf:mainfrom
KubeKyrie:codex/fix-quickstart-dp-auth
Open

KubeKyrie wants to merge 1 commit into
RLinf:mainfrom
KubeKyrie:codex/fix-quickstart-dp-auth

Conversation

@KubeKyrie

@KubeKyrie KubeKyrie commented Oct 21, 2026 •

Copy link
Copy Markdown

Description

  • Authenticate quickstart-dp.sh using the administrator password stored in the local quickstart Secret, or an existing administrator JWT supplied through RLARK_TOKEN.
  • Include the token in node checks, agent certificate requests, and node registration verification.
  • Add HTTP timeouts and stop on authentication failures or incomplete certificate responses.
  • Add script regression tests and update the English and Chinese quickstart documentation.

Motivation and Context

Gateway requires JWT authentication, but the data-plane quickstart script sends anonymous requests. A healthy Gateway returns HTTP 401, which the script incorrectly reports as “Control plane not reachable.”

Certificate requests and subsequent node queries also lack authentication, so bypassing the initial check does not resolve the deployment failure.

How has this been tested?

Tested locally on macOS using Bash, Python 3, and jq:

python3 -m unittest discover -s apps/rlark/docs/examples/tests -v
bash -n apps/rlark/docs/examples/quickstart-dp.sh
git diff --check

All four regression tests passed, covering:

  • Automatic login and authenticated API requests throughout the script.
  • Reuse of an existing token without reading credentials or logging in.
  • Credential, login, and initial node-query failures stopping execution before kind cluster mutations.
  • Failed or incomplete certificate responses stopping execution before Agent deployment.

Docker, kind, kubectl, curl, and sleep are mocked. No live cluster deployment or full repository test suite was run.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@KubeKyrie

Copy link
Copy Markdown
Author

@lkygithub @pigeonligh Hi, could you please review this PR when you have a chance? Thanks!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It modifies the authentication and cluster-mutating flow of the deployment onboarding script, which the author notes was not validated against a live cluster, so a human should give final sign-off.

No open issues 🎉

What changed in this PR

This PR fixes a bug in the data-plane quickstart script (quickstart-dp.sh) where it sent anonymous requests to a Gateway that requires JWT authentication. A healthy Gateway returns HTTP 401, which the old anonymous reachability check misreported as "Control plane not reachable." The fix authenticates the script against the Gateway before performing any cluster-mutating operations, and routes all subsequent API calls (reachability check, agent certificate request, and node-registration verification) through an authenticated helper.

Changes:

  • Authenticate using the admin password from the local rlark-ui-auth quickstart Secret (via /api/v1/auth/login) or a supplied RLARK_TOKEN, and add a gateway_curl helper that attaches the Bearer token to every Gateway request.
  • Harden the script with HTTP connect/max timeouts, a curl prerequisite check, fail-fast on auth/credential errors before touching kind clusters, safe JSON body encoding via jq -n, and validation that the certificate response is complete.
  • Add a self-contained Python regression test (Docker/kind/kubectl/curl/sleep mocked) and update the EN and CN quickstart docs.
File Description
apps/​rlark/​docs/​examples/​quickstart-dp.sh Adds authentication (login or RLARK_TOKEN), gateway_curl helper, timeouts, and cert-response validation; replaces the anonymous reachability check.
apps/​rlark/​docs/​examples/​tests/​test_quickstart_dp.py New regression tests validating login, token reuse, and fail-fast behavior before cluster mutations/agent deployment.
apps/​rlark/​docs/​quickstart.md Documents that the script auto-logs-in with quickstart admin credentials or RLARK_TOKEN, and that reruns recreate kind clusters.
apps/​rlark/​docs/​zh/​quickstart.md Chinese translation of the same guidance, kept in sync with the EN version.

I verified the script's integration points against the codebase: the secret name/namespace/context (rlark-ui-auth, default, root-shard) match quickstart-cp.sh; the login request/response shape (role/token) matches auth_handler.go; the cert response fields (ca_cert/agent_cert/agent_key) match cert_handler.go; and the admin-only /certificates/agent route plus Authorization: Bearer acceptance match router.go/auth_middleware.go. The jq validations and error handling are correct, and the test logic (including the 3 /nodes calls and special-character password encoding) is sound. I found no issues to flag.

Review effort: Balanced

@lkygithub

Copy link
Copy Markdown
Collaborator

Overall

Thanks for tracking this down — the diagnosis is correct and I was able to reproduce it locally.

On a healthy control plane (with a valid jwt-signing-key patched in manually), running the two
versions of the data-plane script side by side gives:

  • main: ✗ Control plane not reachable at http://localhost:9000. Run quickstart-cp.sh first.
  • this PR: ✓ Control plane is reachable

So the fix works and the direction is right. A few things I particularly like:

  • gateway_curl() is a clean abstraction and keeps the call sites readable.
  • Honouring a pre-existing RLARK_TOKEN instead of always logging in.
  • The jq -e 'all(.ca_cert, .agent_cert, .agent_key; ...)' completeness check on the issued
    certificate — that would have caught a silent truncation.
  • Distinct, actionable error messages for "cannot read credentials" vs. "login failed".
  • Shipping tests/test_quickstart_dp.py alongside the change (python3 -m unittest discover -s apps/rlark/docs/examples/tests → 4 tests, OK on my machine).

My main concern is scope: the PR fixes quickstart-dp.sh, but the quickstart is still 100%
unusable end to end, because it breaks earlier — in the control-plane stage — and because the
merged quickstart.sh has the exact same bug this PR is fixing. As it stands, a user following
the docs never reaches the code path this PR repairs.

Would it be possible to fold the four items below into this PR? Items 1 and 2 in particular are
what stand between this change and a quickstart that actually completes.


1. rlark-ui-auth is created without jwt-signing-key, so the Gateway exits on startup

  • apps/rlark/docs/examples/quickstart-cp.sh:218-222
  • apps/rlark/docs/examples/quickstart.sh:217-220

The secret is created with only admin-password and user-password:

kubectl ... create secret generic rlark-ui-auth -n default \
  --from-literal="admin-password=$ADMIN_PASSWORD" \
  --from-literal="user-password=$USER_PASSWORD" \
  --validate=false

But loadJWTSigningKey (apps/rlark/pkg/gateway/auth_handler.go:103-114) requires a
jwt-signing-key of at least 32 bytes, and it is called before the HTTP server starts
(apps/rlark/pkg/gateway/gateway.go:103-105), so the failure is fatal.

Reproduced locally on main:

[12:13:56] ✗ Gateway failed to become ready

$ docker logs rlark-gateway
Error: load JWT signing key: jwt-signing-key must contain at least 32 bytes

$ kubectl ... get secret rlark-ui-auth -o jsonpath='{.data}'
{"admin-password":"...","user-password":"..."}

Suggested fix — add a third literal in both scripts:

  --from-literal="jwt-signing-key=$(openssl rand -hex 32)" \

This is a hard blocker: without it the control plane never comes up, so the data-plane fix in
this PR can never be exercised.

2. The control-plane readiness probe still uses the anonymous-200 pattern this PR is fixing

  • apps/rlark/docs/examples/quickstart-cp.sh:262
  • apps/rlark/docs/examples/quickstart.sh:261
if curl -s -o /dev/null -w "%{http_code}" "http://localhost:9000/api/v1/rlinf.io/v1alpha1/nodes" 2>/dev/null | grep -q "200"; then

GET /nodes is behind requireJWT() (apps/rlark/pkg/gateway/router.go:12 and :28), and
requireJWT() returns 401 for anonymous requests
(apps/rlark/pkg/gateway/auth_middleware.go:16-47). I verified this directly against a working
gateway: anonymous GET /nodes → 401, same request with a bearer token → 200.

This is byte-for-byte the same bug the PR fixes in quickstart-dp.sh:73-76, just in the
control-plane script, so the readiness loop can only ever time out. It would be good to apply the
same treatment here.

3. quickstart.sh (the combined script) is untouched and has the same problem at three call sites

  • apps/rlark/docs/examples/quickstart.sh:321 — POST /api/v1/certificates/agent
  • apps/rlark/docs/examples/quickstart.sh:374 — GET /api/v1/rlinf.io/v1alpha1/nodes
  • apps/rlark/docs/examples/quickstart.sh:383 — GET /api/v1/rlinf.io/v1alpha1/nodes

All three are anonymous. POST /certificates/agent additionally requires the admin role
(apps/rlark/pkg/gateway/router.go:14 and :96-100), so it returns 401 for an unauthenticated
caller and 403 for a non-admin one.

Since quickstart.sh is the all-in-one variant of quickstart-cp.sh + quickstart-dp.sh, it
will immediately drift from the scripts fixed here. Could the login + gateway_curl() block be
factored into a shared snippet (e.g. a small sourced helper next to the scripts) and reused by
all three? If that is too large for this PR, even copying the block over would keep the three
scripts consistent.

4. The docs touched by this PR still show an unauthenticated curl

  • apps/rlark/docs/quickstart.md:235
  • apps/rlark/docs/zh/quickstart.md:236
curl -s "http://localhost:9000/api/v1/rlinf.io/v1alpha1/nodes" | \
  jq '.items[] | {name: .metadata.name, cluster: .metadata.labels["rlark.io/cluster-id"]}'

This PR already adds the RLARK_TOKEN note a few lines above (around
apps/rlark/docs/quickstart.md:186-191), so it would be a shame to leave the verification
snippet returning 401 for anyone who copy-pastes it. Suggested shape:

RLARK_TOKEN=$(curl -fsS -H 'Content-Type: application/json' \
  -d '{"username":"admin","password":"<admin-password>"}' \
  http://localhost:9000/api/v1/auth/login | jq -er '.token')

curl -fsS -H "Authorization: Bearer $RLARK_TOKEN" \
  "http://localhost:9000/api/v1/rlinf.io/v1alpha1/nodes" | \
  jq '.items[] | {name: .metadata.name, cluster: .metadata.labels["rlark.io/cluster-id"]}'

Please keep the EN and ZH pages in sync.


Verification notes: reproduced on macOS with kind + docker compose, control plane via
quickstart-cp.sh --no-ui. After manually patching jwt-signing-key into the secret I confirmed
anonymous GET /nodes → 401 and POST /certificates/agent → 401, and both → 200 with an admin
bearer token. mkdocs build --strict passes on this branch.

@lkygithub

Copy link
Copy Markdown
Collaborator

Hi @KubeKyrie , thanks for the PR and sorry for the slow turnaround.

Heads-up: we just merged a large internal iteration into main (#17), so this
PR is now based on an outdated baseline. The good news is that we re-checked
your findings against the new main and the issue is still fully present:

  • apps/rlark/docs/examples/quickstart-dp.sh still calls the Gateway
    anonymously at the reachability probe, the agent-certificate request, and both
    node queries
  • apps/rlark/pkg/gateway/router.go:17 still wraps /api/v1 in requireJWT(),
    and /api/v1/certificates/** still sits behind requireAdmin()
    (router.go:112-117), so those calls can only ever return 401
  • the login response still returns role and token in the body
    (apps/rlark/pkg/gateway/auth_handler.go:59-62), so the token-extraction
    logic in your patch remains valid as-is

So the fix is still needed. Could you please:

  1. Rebase onto the latest main — we don't expect conflicts, since feature: runtime-neutral Task execution, Cluster resource, tenant-enforced APIs, and unified contracts #17 did not
    touch any of the files this PR changes.

  2. Re-run the script end-to-end on the new baseline to confirm it still passes.

  3. While you're rebasing, please also pick up the four items from the earlier
    review — they're all still reproducible on the new main, only the line
    numbers moved. Without items 1 and 2 the quickstart still can't complete, so
    the code path this PR repairs is never reached:

    • (1) rlark-ui-auth is created without jwt-signing-key, so the Gateway
      exits on startup
      — quickstart-cp.sh:224-226 and quickstart.sh:222-224
      only set admin-password / user-password, but
      loadJWTSigningKey (auth_handler.go:67, :74) requires at least 32
      bytes and is called before the HTTP server starts (gateway.go:114).
      Adding --from-literal="jwt-signing-key=$(openssl rand -hex 32)" to both
      scripts fixes it.
    • (2) The control-plane readiness probe uses the same anonymous-200 pattern
      this PR fixes
      — quickstart-cp.sh:267 and quickstart.sh:266.
    • (3) quickstart.sh (the all-in-one variant) is untouched and has the
      same problem at three call sites
      — quickstart.sh:326
      (POST /certificates/agent, which also needs the admin role), :379
      and :388 (GET /nodes). Factoring the login + gateway_curl() block
      into a shared sourced helper would keep all three scripts from drifting;
      copying the block over is fine too if that's too much for one PR.
    • (4) The docs touched by this PR still show an unauthenticated curl —
      docs/quickstart.md:230 and docs/zh/quickstart.md:232. Please keep the
      EN and ZH pages in sync.
  4. Push the rebased branch so CI re-runs, and we'll take another look.

If you'd rather keep this PR narrow, splitting items 3 and 4 into a follow-up is
fine — but items 1 and 2 really need to land together with this change,
otherwise the quickstart still fails before it gets here.

Thanks again — this one is a real blocker for anyone following the quickstart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants