brent.edwards
b51df2ee0f
CI / docker (push) Blocked by required conditions
CI / status-check (push) Blocked by required conditions
CI / build (push) Successful in 20s
CI / helm (push) Successful in 23s
CI / lint (push) Successful in 3m19s
CI / typecheck (push) Successful in 3m57s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 4m5s
CI / integration_tests (push) Successful in 7m25s
CI / unit_tests (push) Failing after 13m18s
CI / quality (push) Failing after 13m19s
CI / e2e_tests (push) Failing after 18m16s
CI / coverage (push) Failing after 19m20s
CI / benchmark-publish (push) Failing after 28m16s
## Summary This PR adds Kubernetes Helm deployment support for the CleverAgents server. Closes #928 ### What this PR includes - Helm chart under `k8s/` with Deployment, Service, optional Ingress, ConfigMap, ServiceAccount, Secrets, NOTES, and optional Redis subchart configuration. - Multi-stage `Dockerfile.server` for server runtime deployment. - Deployment-focused docs in `k8s/README.md`. - Behave + Robot + benchmark coverage for chart/deployment wiring. ### Review fixes applied (cycle 11 — hurui200320 review #2687) **Critical fix:** 1. **CI SHA256 checksum verification fixed** — All 3 Helm install blocks (`unit_tests`, `integration_tests`, `helm` jobs) now save the tarball using its original filename (`helm-v3.16.4-linux-amd64.tar.gz`) instead of `helm.tgz`, so the `.sha256sum` file can correctly locate and verify it. This was causing all 3 CI Helm jobs to fail with "No such file or directory". **Major fixes (test coverage gaps):** 2. **405 Allow header now tested** — The existing "POST to known path returns method not allowed" scenario now asserts that the `Allow: GET` header is present. Added a second 405 scenario testing `POST /live` for broader `_KNOWN_PATHS` coverage (review item #11). 3. **Security-hardening headers now tested** — New scenario "HTTP responses include security-hardening headers" verifies `content-length`, `x-content-type-options: nosniff`, and `cache-control: no-store` headers are present on HTTP responses. 4. **Lifespan warning logging now tested** — New scenario "Unrecognised lifespan message type logs warning and continues" queues `lifespan.startup` → `lifespan.bogus` → `lifespan.shutdown` and verifies: (a) the app completes the lifespan cycle cleanly, (b) a warning is logged mentioning the unrecognised type. ### Review fixes applied (cycle 10) **Critical/Major fixes (from hurui200320's prior REQUEST_CHANGES):** 1. **Rebased branch onto master** — Removed merge commit per CONTRIBUTING.md rebase-only policy. Clean linear history restored. 2. **Fixed commit message body** — Replaced literal `\n` sequences with actual newlines. `ISSUES CLOSED: #928` footer is now on its own line after a blank separator. 3. **Moved uvicorn import to module top level** — `from uvicorn import run as uvicorn_run` is now at the top of `src/cleveragents/cli/commands/server.py` per Import Guidelines. Updated test mock target from `uvicorn.run` to `cleveragents.cli.commands.server.uvicorn_run`. 4. **Added SHA256 checksum verification** — All three Helm CLI install blocks in `.forgejo/workflows/ci.yml` now download and verify `helm.sha256sum` before extracting the binary. **Minor fixes:** 5. **Added `Dockerfile.server` build to CI** — New "Build Docker image (Server)" step in the `docker` job validates the server Dockerfile. 6. **ASGI 405 Method Not Allowed** — Known paths (`/`, `/live`, `/ready`, `/health`) now return 405 with `Allow: GET` header for non-GET methods, per RFC 9110 §15.5.6. Added `_KNOWN_PATHS` frozenset. 7. **WebSocket close protocol fix** — App now calls `await receive()` to consume the `websocket.connect` event before closing. Changed close code from 1000 (Normal Closure) to 1008 (Policy Violation). 8. **Lifespan handler logging** — Unrecognised lifespan message types are now logged as warnings instead of silently consumed. 9. **Security-hardening headers** — `_send_response` now includes `content-length`, `x-content-type-options: nosniff`, and `cache-control: no-store` on all HTTP responses. 10. **`.dockerignore` credential patterns** — Added `*.pem`, `*.key`, `*.p12`, `*.pfx`, `credentials*.json`. 11. **`--log-level` validation** — Constrained to `click.Choice(["critical", "error", "warning", "info", "debug", "trace"])` for clean CLI validation errors. 12. **Reverted unrelated semgrep pre-commit change** — `pass_filenames` and `entry` restored to original values per atomic commit hygiene. 13. **Removed unused `ReceiveCallable` type alias** and `Callable`/`Awaitable` imports from `asgi_app_steps.py`. 14. **Fixed redundant `shutil.which("helm")` check** — `_skip_if_helm_missing` now returns `bool` to eliminate the duplicate check in `_render_chart`. 15. **Improved test deque error handling** — Lifespan test receive mock now raises descriptive `AssertionError` instead of opaque `IndexError`. 16. **Scope type dispatch** — Changed `if/if/if` to `if/elif/elif` for mutually exclusive ASGI scope types. 17. **Dockerfile.server base image** — Standardised to `python:3.13-slim` (floating minor) consistent with CLI Dockerfile. 18. **Dockerfile layer caching** — Split `uv pip install build` and `python -m build` into separate `RUN` instructions. 19. **Removed extraneous double blank line** in Dockerfile.server. ### Deferred items (acknowledged, not in scope) - PodDisruptionBudget, HorizontalPodAutoscaler, NetworkPolicy — Follow-up for production hardening. - `appVersion: "1.0.0"` placeholder — Needs tracking issue for release versioning alignment. - Readiness probe with downstream dependency checks — Documented limitation. - Cross-system test for probe paths matching ASGI routes — Test enhancement. - Improved benchmarks (helm template timing vs PyYAML parsing) — Benchmark quality improvement. - CI DRY violation (Helm install 3×) — Code quality improvement, consider composite action. - File length limits exceeded (`k8s_helm_chart_steps.py` 551 lines, `helper_k8s_helm_chart.py` 678 lines) — Non-blocking, can be split in follow-up. - `runAsGroup: 1000` in pod security context — Defense-in-depth improvement. - HEAD method support on known paths — RFC compliance, does not affect K8s probes. - `click.Choice` log-level validation via CLI runner test — Test gap. ### Scope note: status-check CI gate The `status-check` job now includes `integration_tests`, `e2e_tests`, and `helm` in its `needs` list. The `helm` job is new in this PR. The `integration_tests` and `e2e_tests` additions fix previously-missing gate checks — included here since this PR modifies both of those jobs to install Helm. ### Quality gates - `nox -e lint` ✅ - `nox -e typecheck` ✅ - `nox -e unit_tests` ✅ (12,321 scenarios passed, 4 skipped) - `nox -e integration_tests` — 3 pre-existing failures in unrelated areas (plan correction, resource types) - `nox -e e2e_tests` — pre-existing failures (LLM API keys not available in local env) - `nox -e coverage_report` ✅ (**97.7%**) Reviewed-on: #1085 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
135 lines
7.3 KiB
Gherkin
135 lines
7.3 KiB
Gherkin
@phase2 @a2a @server-cli-coverage
|
|
Feature: Server CLI command coverage boost
|
|
As a developer
|
|
I want full test coverage for server.py CLI commands
|
|
So that the server_connect (rich format), server_status, and resolve_server_mode
|
|
error paths are all exercised
|
|
|
|
# -----------------------------------------------------------------------
|
|
# resolve_server_mode — exception paths (lines 58-59)
|
|
# -----------------------------------------------------------------------
|
|
|
|
Scenario: resolve_server_mode returns disabled when resolve raises ValueError
|
|
Given a mock config service that raises ValueError on resolve
|
|
When I call resolve_server_mode with the mock
|
|
Then the server mode should be "disabled"
|
|
|
|
Scenario: resolve_server_mode returns disabled when resolve raises KeyError
|
|
Given a mock config service that raises KeyError on resolve
|
|
When I call resolve_server_mode with the mock
|
|
Then the server mode should be "disabled"
|
|
|
|
# -----------------------------------------------------------------------
|
|
# server_connect — rich format output (lines 132-142)
|
|
# -----------------------------------------------------------------------
|
|
|
|
Scenario: server_connect renders Rich Panel when format is rich
|
|
Given a mock config service that accepts writes
|
|
When I call server_connect with url "https://rich.example.com" and rich format
|
|
Then the captured console output should contain "Server Connection (Stub)"
|
|
And the captured console output should contain "https://rich.example.com"
|
|
And the captured console output should contain "stubbed"
|
|
And the captured console output should contain "not yet implemented"
|
|
|
|
Scenario: server_connect rich output includes namespace and TLS verify
|
|
Given a mock config service that accepts writes
|
|
When I call server_connect with url "https://ns.example.com" namespace "my-team" tls_verify false and rich format
|
|
Then the captured console output should contain "my-team"
|
|
And the captured console output should contain "False"
|
|
|
|
# -----------------------------------------------------------------------
|
|
# server_status — full function, no URL configured (lines 157-204)
|
|
# -----------------------------------------------------------------------
|
|
|
|
Scenario: server_status with json format when no URL is configured
|
|
Given a mock config service that resolves with no server config
|
|
When I call server_status with format "json"
|
|
Then the echoed output should contain "disabled"
|
|
And the echoed output should contain "server_mode"
|
|
|
|
Scenario: server_status with plain format when no URL is configured
|
|
Given a mock config service that resolves with no server config
|
|
When I call server_status with format "plain"
|
|
Then the echoed output should contain "disabled"
|
|
|
|
# -----------------------------------------------------------------------
|
|
# server_status — URL is configured (stubbed mode)
|
|
# -----------------------------------------------------------------------
|
|
|
|
Scenario: server_status with json format when URL is configured
|
|
Given a mock config service that resolves with full server config
|
|
When I call server_status with format "json"
|
|
Then the echoed output should contain "stubbed"
|
|
And the echoed output should contain "https://configured.example.com"
|
|
And the echoed output should contain "production"
|
|
|
|
# -----------------------------------------------------------------------
|
|
# server_status — rich format rendering (lines 196-206)
|
|
# -----------------------------------------------------------------------
|
|
|
|
Scenario: server_status renders Rich Panel when format is rich and no URL
|
|
Given a mock config service that resolves with no server config
|
|
When I call server_status with format "rich"
|
|
Then the captured console output should contain "Server Status"
|
|
And the captured console output should contain "disabled"
|
|
And the captured console output should contain "(not configured)"
|
|
|
|
Scenario: server_status renders Rich Panel when format is rich and URL is configured
|
|
Given a mock config service that resolves with full server config
|
|
When I call server_status with format "rich"
|
|
Then the captured console output should contain "Server Status"
|
|
And the captured console output should contain "stubbed"
|
|
And the captured console output should contain "https://configured.example.com"
|
|
|
|
# -----------------------------------------------------------------------
|
|
# server_status — exception paths for namespace / tls-verify (lines 168-183)
|
|
# -----------------------------------------------------------------------
|
|
|
|
Scenario: server_status defaults namespace when resolve raises ValueError
|
|
Given a mock config service where namespace resolve raises ValueError
|
|
When I call server_status with format "json"
|
|
Then the echoed output should contain "default"
|
|
|
|
Scenario: server_status defaults tls_verify when resolve raises KeyError
|
|
Given a mock config service where tls-verify resolve raises KeyError
|
|
When I call server_status with format "json"
|
|
Then the echoed output should contain "true"
|
|
|
|
Scenario: server_status defaults all fields when all resolves raise exceptions
|
|
Given a mock config service where all resolves raise exceptions
|
|
When I call server_status with format "json"
|
|
Then the echoed output should contain "disabled"
|
|
And the echoed output should contain "null"
|
|
And the echoed output should contain "default"
|
|
|
|
# -----------------------------------------------------------------------
|
|
# server_serve — uvicorn.run argument mapping
|
|
# -----------------------------------------------------------------------
|
|
|
|
Scenario: server_serve maps explicit CLI arguments to uvicorn.run
|
|
Given uvicorn run is patched for server serve assertions
|
|
When I call server_serve with app "cleveragents.a2a.asgi:app" host "127.0.0.1" port 9001 workers 3 and log level "warning"
|
|
Then uvicorn run should be called with app "cleveragents.a2a.asgi:app" host "127.0.0.1" port 9001 workers 3 and log level "warning"
|
|
|
|
Scenario: server_serve uses documented defaults for uvicorn.run
|
|
Given uvicorn run is patched for server serve assertions
|
|
When I call server_serve with default arguments
|
|
Then uvicorn run should be called with app "cleveragents.a2a.asgi:app" host "0.0.0.0" port 8000 workers 1 and log level "info"
|
|
|
|
Scenario: server_serve exits cleanly when uvicorn is unavailable
|
|
Given uvicorn run is unavailable for server serve assertions
|
|
When I call server_serve with default arguments and capture failure
|
|
Then server_serve should exit with code 1
|
|
And the captured console output should contain "optional dependency 'uvicorn' is not installed"
|
|
|
|
Scenario: server_serve normalizes log level via Typer option parsing
|
|
Given uvicorn run is patched for server serve assertions
|
|
When I invoke server_serve through the CLI with log level "WARNING"
|
|
Then uvicorn run should be called with app "cleveragents.a2a.asgi:app" host "0.0.0.0" port 8000 workers 1 and log level "warning"
|
|
|
|
Scenario: server_serve rejects invalid log level via Typer option parsing
|
|
Given uvicorn run is patched for server serve assertions
|
|
When I invoke server_serve through the CLI with invalid log level "LOUD"
|
|
Then the CLI invocation should fail with exit code 2
|
|
And the server CLI invocation output should contain "Invalid value for '--log-level'"
|