Add attested-tls-proxy to buildernet/mkosi recipe

[canercidam]

No description provided.

[claude]

Claude finished @canercidam's task in 2m 12s —— View job

---

PR Review: Add attested-tls-proxy to buildernet/mkosi recipe

• Gather context and read changed files
• Review playground.yaml changes
• Review TLS certificate and key files
• Post review feedback

---

Summary

This PR adds an attested-tls-proxy service to the BuilderNet mkosi custom recipe, along with a self-signed TLS certificate and private key for localhost.

Issues Found

1. Private key committed to repository (inline comment)

server.key contains an RSA private key checked into the repo. Even for local dev, this is not ideal. Consider generating the cert/key pair at runtime (e.g., via an init step with openssl) rather than committing static credentials. If they must be static, add documentation clarifying they are throwaway dev-only credentials.

2. No depends_on for upstream service (inline comment)

The proxy forwards to 127.0.0.1:8000 but there's no depends_on or ready_check configured, so the proxy may start before whatever service is expected on port 8000 is ready.

3. Attestation disabled without documentation (inline comment)

Both attestation types are set to none. A YAML comment explaining this is intentional for local dev would help future readers.

4. Certificate expiry

The certificate is valid from 2026-02-12 to 2027-02-12 (1 year). This is fine for dev but will silently expire. Runtime generation would avoid this concern entirely.

5. Missing PR description

No description was provided. Adding context about why this proxy is needed and what service it fronts would help reviewers.

[claude]

Security concern: Private key committed to repository.

Even if this is a self-signed key for local development/testing, committing private keys to a public repository is a bad practice:

1. Credential hygiene - It normalizes committing secrets to version control. Anyone cloning this repo gets this key, and it could be mistakenly used beyond local dev.
2. Key rotation - If this key ever needs to be rotated, it remains in git history forever.

Consider:

• Generating the key/cert pair at runtime (e.g., in an init step or as an artifact) instead of checking them in. The attested-tls-proxy or a simple openssl command in an init script could generate ephemeral certs.
• If they must be static files, add a clear comment in the YAML or a README explaining they are intentionally throwaway dev-only credentials.

[claude]

Both --server-attestation-type none and --allowed-remote-attestation-type none disable attestation verification. This is expected for local dev, but worth a comment in the YAML for clarity so future readers understand this is intentional and must not be used in production.

[claude]

The last positional argument 127.0.0.1:8000 is the backend/upstream address being proxied to. What service is expected to be listening on port 8000? There's no depends_on configured for this service, so if the upstream isn't ready when the proxy starts, connections will fail. Consider adding a depends_on or a ready_check if the upstream service is defined elsewhere in the recipe.

[claude]

Claude finished @fkondej's task in 2m 9s —— View job

---

PR Review (re-review after sync): Add attested-tls-proxy to buildernet/mkosi recipe

• Gather context and read changed files
• Review changes since last review
• Post review feedback

---

Changes Since Last Review

The second commit (931c9f0) addresses some of the prior feedback:

• depends_on added — The proxy now depends on builder-hub-proxy:healthy, resolving the startup ordering concern.
• Backend target corrected — Changed from 127.0.0.1:8000 to builder-hub-proxy:8888, which correctly points to the builder-hub-proxy service (defined in components.go:979-991).
• Listen address updated — Changed from 127.0.0.1:7000 to 0.0.0.0:7000 so the proxy is reachable from outside the container.
• Port exposed — Added ports: http: 7000.
• Test script added — test-atls-proxy.sh spins up a client proxy, curls through it, and tears it down.

Remaining Issues

1. Private key still committed to repository

server.key and server.crt are still checked into the repo. The prior review's suggestion to generate these at runtime (e.g., via an init step) was not addressed. This is a self-signed localhost cert so the risk is low, but it sets a poor precedent and the cert will expire on 2027-02-12.

2. Missing newline at EOF in test script (inline comment)

test-atls-proxy.sh is missing a trailing newline.

3. No PR description

Still no description explaining the purpose of the proxy, what traffic flow it enables, or why it's needed in the mkosi recipe. This context would help reviewers and future readers.

[claude]

Missing newline at end of file.

Suggested change

	curl -s -w "\n--- %{http_code}\n" "http://127.0.0.1:${PORT}${PATH_}"
	curl -s -w "\n--- %{http_code}\n" "http://127.0.0.1:${PORT}${PATH_}"

[claude]

The $NAME variable in the trap should be quoted to prevent word splitting. Also, the trap uses single quotes which means $NAME won't be expanded at trap definition time — but since it's a constant assigned on line 7, this works correctly. Just noting for clarity.

Suggested change

	trap 'docker rm -f $NAME >/dev/null 2>&1 || true' EXIT
	trap 'docker rm -f "$NAME" >/dev/null 2>&1 || true' EXIT

Wait — single quotes prevent variable expansion entirely. The $NAME won't expand inside single quotes. This actually works because bash expands $NAME at trap execution time (not definition time) when using single quotes, but only if NAME is still in scope. Since this is the same script, it works. Disregard the suggestion above.