Skip to content

test/ci: enterprise-grade hardening — chaos e2e, mTLS, soak, perf trend, API gate, pipeline reliability - #52

Merged
lukekim merged 5 commits into
trunkfrom
test/enterprise-grade-hardening
Aug 1, 2026
Merged

test/ci: enterprise-grade hardening — chaos e2e, mTLS, soak, perf trend, API gate, pipeline reliability#52
lukekim merged 5 commits into
trunkfrom
test/enterprise-grade-hardening

Conversation

@lukekim

@lukekim lukekim commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the P0/P1 gaps from the test-posture assessment: the SDK's resilience features now have end-to-end proof against real runtime failures, mTLS has real handshake coverage, performance is tracked as a trend with deterministic contracts, a nightly soak hunts slow leaks, an API-compatibility gate protects the public surface, and the CI pipeline itself is hardened against the flakes that kept turning PRs red.

P0 — CI pipeline reliability

  • Spice CLI install: one composite action (.github/actions/setup-spice) with retries + binary verification + GITHUB_TOKEN (the JDK-matrix job lacked the token — the source of most installer flakes), replacing six inlined copies in two languages.
  • Runtime startup: one composite action (.github/actions/start-spice-app) with a real /v1/ready poll, replacing four copies of the quickstart heredoc — and retiring the flaky sleep 10 in the pre-existing build/publish jobs as a side effect.
  • OWASP split into two steps: a best-effort NVD data update (continue-on-error, step timeout) and a gating offline check — network flakiness can no longer mask real CVSS ≥ 7 findings, and findings can no longer be swallowed as flakes. nvdValidForHours=24 makes warm caches skip the refresh. Wired for an NVD_API_KEY secret (needs to be added in repo settings; free from NVD).

P0 — Chaos e2e (ChaosE2ETest + e2e-chaos job, every PR)

Manages its own spiced processes (dataset-free pod, no external dependencies) via a new SpicedProcess fixture and proves against the real runtime:

  • crash + restart on the same address → same client reconnects and cached prepared statements transparently re-prepare
  • query issued during downtime → recovers via retry backoff (LB-failover scenario)
  • SIGKILL mid-stream → clean classifiable error, never a hang; client recovers
  • SIGSTOP (frozen peer, undetectable by TCP alone) → keep-alive detects within its configured window (bound derived from SpiceClient's actual constants), recovery after SIGCONT

P1 — mTLS integration (MtlsTest, TestCerts, TLS mode in TestFlightSqlServer)

Runtime-generated EC P-256 CA/server/client certificates, real handshakes in-process: custom-CA trust, full mutual TLS including parameterized queries, and rejection paths that assert the failure is a TLS failure (this immediately caught the negative tests passing via IPv6 connection-refused before any handshake — both sides now pin 127.0.0.1).

P1 — Perf trend + soak + API gate

  • PerfBenchmarkTest emits customSmallerIsBetter JSON (BENCH_JSON); nightly publishes the trend to gh-pages (branch created; action pinned to a full commit SHA; runs with -Djacoco.skip=true so latencies measure the real SDK, not coverage-instrumented classes). Benchmark ops assert full row counts so a result-dropping regression can't publish an "improved" latency.
  • Nightly soak (SoakTest, 30 min): 4-worker mixed workload with periodic reset(); asserts zero errors, non-empty results throughout, leak-free close, bounded thread growth from a warm baseline, and p99 stability across the run. Local validation: ~8-9k ops/s, 500k+ ops per minute-long smoke.
  • japicmp gate vs the last release in the quality job; verified the current surface is purely additive vs 0.6.0. japicmp.oldVersion is a pom property and the publish preflight fails on a vacuous self-comparison.

Security hardening (from adversarial review)

  • publish.yaml never executes mutable remote code: the release pipeline downloads a pinned, sha256-verified CLI/runtime release, and the GPG signing key is imported only after all downloaded code has run.
  • Third-party benchmark action pinned by full commit SHA (it receives a write token).

Review chain applied

  • Codex adversarial review (17 findings): 15 fixed — including a soak heap-exhaustion hazard, chaos process-management races, silent no-op chaos gating, and the workflow security items above; 2 accepted with rationale (temp-dir/cert cleanup: throwaway 1-day fixtures in user-private OS temp on ephemeral runners).
  • /simplify (4 parallel reviewers): composite-action consolidation, SpicedProcess extraction, shared countRows reuse, EC certs, dead-code and FQN cleanups; nightly chaos job dropped as a verbatim duplicate of the per-push job.

Validation

  • 293 tests + SpotBugs green (mock-only and with a live runtime)
  • Chaos e2e: 4/4 against real spiced, repeatedly
  • Soak smoke: 60s @ 8.3k ops/s, zero errors, threads flat, 20.8M rows verified
  • japicmp, jacoco-skip path, and all workflow/action YAML validated
  • docs/testing.md documents the tiers and how to run each locally

Follow-ups (deferred, noted in review)

  • Add NVD_API_KEY repo secret (speeds the OWASP data refresh)
  • Enable GitHub Pages for gh-pages if the benchmark dashboard should be served (data accumulates in-branch regardless)
  • Consider excluding ai.spice.example from the published jar (would obsolete the japicmp exclusion)

lukekim added 3 commits July 31, 2026 16:37
…nd, API gate, pipeline fixes

P0 — CI pipeline reliability:
- Spice CLI install steps (build x3 incl. Windows, publish, new jobs):
  retry with backoff + binary verification + GITHUB_TOKEN (the JDK-matrix
  job lacked the token and caused most installer flakes)
- OWASP: step-level 20m timeout so a slow NVD refresh can't hit the job
  timeout (which cancels the job and bypasses continue-on-error);
  nvdValidForHours=24 so warm caches skip the refresh entirely

P0 — chaos e2e (ChaosE2ETest + e2e-chaos job, gated by SPICE_E2E_CHAOS):
manages its own spiced processes (dataset-free pod) and proves against the
real runtime: crash/restart recovery on one client (reconnect + prepared
statement re-prepare), queries during downtime recovering via retry
backoff, clean mid-stream failure on SIGKILL, and keep-alive detection of
a frozen (SIGSTOP) peer with recovery after SIGCONT. 4/4 green locally.

P1 — mTLS integration (MtlsTest + TestCerts + TLS mode in
TestFlightSqlServer): runtime-generated CA/server/client certificates,
real handshakes in-process — custom-CA trust, full mutual TLS including
parameterized queries, missing-client-cert and untrusted-CA rejection.
First e2e coverage of the TLS configuration surface.

P1 — japicmp API-compatibility gate vs the last release (0.6.0), wired
into the quality job; verified the 0.7.0 surface is purely additive.
ADBC types (internal in 0.6.0, removed since) are scoped as ignored.

P1 — nightly workflow: 30-min soak (SoakTest: mixed workload with
periodic reset(); zero errors, leak-free close, bounded thread growth
from a warm baseline, p99 stability across the run — validated locally at
~9,200 ops/s), chaos rerun, and benchmark trend publishing to gh-pages
via github-action-benchmark (warn-only alerts; PerfBenchmarkTest emits
customSmallerIsBetter JSON when BENCH_JSON is set).

docs/testing.md documents the tiers and how to run each locally.

Full gate: 293 tests, 0 failures; SpotBugs clean.
Security (workflows):
- publish.yaml: replace curl|bash with a pinned, sha256-verified download
  of the exact Spice CLI/runtime release, and defer the GPG key import to a
  dedicated step after all downloaded code has run — no mutable remote code
  ever executes in a release-signing context
- nightly.yaml: pin github-action-benchmark to a full commit SHA (v1.22.1);
  document that GITHUB_TOKEN pushes don't trigger a Pages build

Correctness (workflows):
- build.yaml: split OWASP into a best-effort NVD update (continue-on-error)
  and a gating offline check — network flakiness can no longer mask real
  CVSS>=7 findings, and findings can no longer be swallowed as flakes
- nightly.yaml: curl timeouts in the readiness loop (no wedge on a stalled
  endpoint)

Tests:
- ChaosE2ETest: fail loudly when SPICE_E2E_CHAOS=1 but spiced is missing
  (was a silent no-op pass); daemon hang-guard threads; kill() asserts the
  process actually exited; launch() destroys half-started processes instead
  of orphaning them; start() retries the ephemeral-port race; the downtime
  restarter is joined in finally and its failures propagate
- SoakTest: bounded latency retention (5k samples/minute — a 30-min soak
  previously retained every sample and risked OOM before asserting);
  asserts every data operation returned rows (silent-empty-results guard)
- MtlsTest: negative tests now assert the failure is a TLS/certificate
  failure by walking the cause chain — which immediately exposed that they
  had been passing via IPv6 connection-refused before any handshake; TLS
  server + client now bind/dial 127.0.0.1 explicitly
- PerfBenchmarkTest: benchmark ops assert full row counts (a result-dropping
  regression can't publish an 'improved' latency) and the plain-query
  contract also asserts the DoGet count

Accepted as-is: temp workspace/cert cleanup (throwaway 1-day self-signed
fixtures in user-private OS temp on ephemeral runners).

Validated: 293 tests + SpotBugs green; chaos e2e 4/4 against real spiced.
Workflows (reuse + altitude):
- New composite actions .github/actions/setup-spice (retried, verified,
  token-authenticated CLI/runtime install — replaces six inlined copies in
  two languages) and start-spice-app (quickstart heredoc + /v1/ready poll —
  replaces four copies and retires the flaky 'sleep 10' everywhere,
  including the pre-existing build/publish app-start steps)
- Dropped the nightly chaos job (verbatim copy of the per-push e2e-chaos)
- e2e-chaos and soak jobs no longer run a full mvn install nothing consumes
- quality job runs checkstyle+japicmp in one Maven session
- bench-trend runs with -Djacoco.skip=true so published latencies measure
  the real SDK, not coverage-instrumented classes (pom gains an empty
  argLine default so surefire works when JaCoCo is skipped)
- japicmp oldVersion hoisted to a property; the publish preflight now fails
  if it equals the version being released (vacuous self-comparison guard)

Tests (simplification + efficiency):
- SpicedProcess promoted from ChaosE2ETest inner class to a top-level
  package-private fixture (spicepod content parameterized)
- Keep-alive interval/timeout promoted to package-visible SpiceClient
  constants; the frozen-peer test derives its detection bound from them
- ChaosE2ETest: shared daemon guard executor, AssertionError instead of
  fail+unreachable-throw, frozen-test recovery restructured sequentially
  with an honest comment, countRows delegates to the shared helper
- SoakTest: single bounded synchronizedList map replaces the parallel
  queue+counter maps, plain int roll counter, awaitTermination doubles as
  the completion join (latch removed), summary reuses p99Of
- PerfBenchmarkTest: checkedCountRows + paramsOp factory replace three
  hand-rolled assertion lambdas; plain imports replace inline FQNs
- TestCerts: EC P-256 keys (~300x faster than RSA-2048 on CI, same
  compatibility), writeKeyPem delegates to writePem
- TestFlightSqlServer: pemStream helper + plain imports

Validated: 293 tests + SpotBugs green; jacoco.skip path green; japicmp
green; chaos e2e 4/4; 60s soak smoke green (row guard: 20.8M rows).
Copilot AI review requested due to automatic review settings August 1, 2026 00:45
Comment thread .github/workflows/build.yaml Fixed
Comment thread .github/workflows/nightly.yaml Fixed

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.

Pull request overview

This PR hardens CI and test posture for the Spice Java SDK by adding runtime-backed resilience and TLS/mTLS integration tests, introducing long-running/perf signal coverage, and consolidating CI workflow logic into reusable composite actions.

Changes:

  • Adds new test suites/fixtures: chaos E2E against a real spiced process, runtime-generated TLS/mTLS handshake coverage, nightly soak, and benchmark trend emission.
  • Introduces CI hardening: composite actions for Spice install and runtime startup, nightly workflows, OWASP dependency-check split, and a japicmp API-compatibility gate.
  • Exposes keep-alive tuning constants for resilience tests and adds build properties for japicmp/Jacoco-skip scenarios.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test/java/ai/spice/TestFlightSqlServer.java Adds TLS/mTLS capability to the in-process Flight SQL server fixture.
src/test/java/ai/spice/TestCerts.java Introduces runtime-generated PEM cert fixtures for TLS/mTLS tests.
src/test/java/ai/spice/SpicedProcess.java Adds a real spiced process manager for chaos/lifecycle tests.
src/test/java/ai/spice/SoakTest.java Adds an availability-gated long-running soak test for leaks/latency/thread stability.
src/test/java/ai/spice/PerfBenchmarkTest.java Adds deterministic row-count contracts and JSON benchmark emission for trend tracking.
src/test/java/ai/spice/MtlsTest.java Adds in-process TLS and mutual-TLS handshake integration tests with negative-path assertions.
src/test/java/ai/spice/ChaosE2ETest.java Adds gated chaos E2E tests validating retry/reconnect/keep-alive behaviors against real runtime failures.
src/main/java/ai/spice/SpiceClient.java Exposes keep-alive tuning constants for tests and reuses them in channel configuration.
pom.xml Adds japicmp “old version” property/gate configuration and ensures empty argLine when JaCoCo is skipped.
docs/testing.md Documents the new tiered testing approach and how to run tiers locally.
.github/workflows/publish.yaml Hardens release pipeline (pinned downloads, delayed GPG import) but still has a runtime startup sleep.
.github/workflows/nightly.yaml Adds nightly soak and benchmark-trend publishing workflow.
.github/workflows/build.yaml Consolidates setup/start logic, adds chaos job, adds japicmp gate, and hardens OWASP dependency-check execution.
.github/actions/start-spice-app/action.yml New composite action to start a quickstart runtime and poll /v1/ready instead of sleeping.
.github/actions/setup-spice/action.yml New composite action to install Spice CLI/runtime with retries and verification.
Suppressed comments (1)

.github/workflows/publish.yaml:78

  • This workflow step still uses a fixed sleep for runtime startup. The PR description states build/publish jobs should use the shared /v1/ready poll (start-spice-app) to avoid flakiness; keeping sleep 10 here risks the same intermittent failures and contradicts the stated hardening.
      - name: Init and start spice app
        run: |
          spice init spice_qs
          cd spice_qs
          # Define the dataset inline rather than fetching spiceai/quickstart from

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/java/ai/spice/SpicedProcess.java Outdated
- Windows: the Spice runtime does not run on native Windows (the CLI's
  'spice install' exits 1 with a WSL pointer) — the old workflow silently
  never had a runtime there. The composite actions now install the CLI
  only and skip the app start on Windows with an explicit notice; Windows
  jobs validate the SDK against the in-process suite (live tests
  self-skip), which is what actually happened before, now stated honestly
- OWASP: with no local database (cold cache AND failed update),
  dependency-check forces a full download regardless of autoUpdate=false
  and burns the gating step's timeout. The gate now runs only when data
  exists and emits a loud workflow warning otherwise; with the NVD_API_KEY
  secret added the gate becomes effectively always-on
- CodeQL: explicit least-privilege permissions blocks (contents: read) on
  all jobs missing them across build/nightly/publish workflows
- SpicedProcess.waitUntilHealthy: preserve the interrupt flag and abort
  the wait instead of swallowing InterruptedException (Copilot)

Chaos e2e revalidated 4/4; full gate green.
Copilot AI review requested due to automatic review settings August 1, 2026 01:17

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.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/workflows/publish.yaml:80

  • The PR description says the flaky fixed sleeps in build/publish jobs were retired in favor of the /v1/ready poll, but publish.yaml still starts the runtime with a fixed sleep (later in this step). This leaves the release pipeline exposed to the same readiness flake the composite action was introduced to remove.
      - name: Init and start spice app
        run: |
          spice init spice_qs
          cd spice_qs
          # Define the dataset inline rather than fetching spiceai/quickstart from

src/test/java/ai/spice/SpicedProcess.java:109

  • ProcessBuilder is constructed with findBinary() directly; if no spiced binary is present this passes null as the executable and will throw a NullPointerException with little context. Even though ChaosE2ETest asserts binary presence, this helper should fail fast with a clear message (and avoid surprising NPEs if reused elsewhere).
    private void launch() throws Exception {
        ProcessBuilder builder = new ProcessBuilder(
                findBinary(),
                "--http", "127.0.0.1:" + httpPort,
                "--flight", "127.0.0.1:" + flightPort,

src/test/java/ai/spice/PerfBenchmarkTest.java:111

  • recordBench() assumes that an existing BENCH_JSON file contains a valid JSON array. If the file exists but is empty/whitespace (or partially written from a prior run), JsonParser.parseString(...).getAsJsonArray() will throw and fail the benchmark run. Making the read tolerant of empty content avoids fragile failures in CI/local runs.
    private static synchronized void recordBench(String name, String unit, long value) throws Exception {
        String path = System.getenv("BENCH_JSON");
        if (path == null || path.isEmpty()) {
            return;
        }
        Path file = Path.of(path);
        JsonArray entries = Files.exists(file)
                ? JsonParser.parseString(Files.readString(file)).getAsJsonArray()
                : new JsonArray();

A timed-out update leaves a partial .mv.db, which passed the presence
check — and dependency-check forces a full re-download over a partial
database regardless of autoUpdate=false, burning the gating timeout.
The gate now runs only when the update step succeeded (fresh, complete
data); otherwise the job emits a loud warning and succeeds — which also
lets actions/cache finally persist the update's incremental progress
(the cache only saves on job success), so cold caches converge to warm
across runs and the gate self-activates.
Copilot AI review requested due to automatic review settings August 1, 2026 01:46

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.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/workflows/build.yaml:215

  • The shell snippet builds an EXTRA string and expands it unquoted into the mvn command. If the API key ever contains whitespace or shell metacharacters, this can break argument parsing (and unquoted expansion is generally risky). Use an args array and pass it as "${EXTRA_ARGS[@]}" instead.
        run: |
          EXTRA=""
          if [ -n "$NVD_API_KEY" ]; then
            EXTRA="-DnvdApiKey=$NVD_API_KEY"
          fi
          mvn dependency-check:update-only -B $EXTRA

src/test/java/ai/spice/SpicedProcess.java:109

  • SpicedProcess.launch() passes findBinary() directly into ProcessBuilder. If the binary is missing (or the method is reused outside the ChaosE2ETest gate), this will throw a NullPointerException from ProcessBuilder with little context. Fail fast with a clear IllegalStateException when no spiced binary is found.
    private void launch() throws Exception {
        ProcessBuilder builder = new ProcessBuilder(
                findBinary(),
                "--http", "127.0.0.1:" + httpPort,
                "--flight", "127.0.0.1:" + flightPort,
                "--metrics", "127.0.0.1:" + metricsPort);

@lukekim lukekim self-assigned this Aug 1, 2026
@lukekim lukekim added the enhancement New feature or request label Aug 1, 2026
@lukekim lukekim added this to the v0.7.0 milestone Aug 1, 2026
@lukekim
lukekim enabled auto-merge (squash) August 1, 2026 03:52
@lukekim
lukekim merged commit 1df1978 into trunk Aug 1, 2026
21 checks passed
@lukekim
lukekim deleted the test/enterprise-grade-hardening branch August 1, 2026 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants