Skip to content

Commit 01954f8

Browse files
authored
Test/concurrent make test (#300)
* test: run test packages concurrently again The -p 1 flag serialized all test packages as a workaround for shared state, at the cost of making every full suite run several times slower than it needed to be. The blockers are gone: the sqlite-backed test packages isolate their data directories per run through TestMain (SPIKE_NEXUS_DATA_DIR pointing at a temporary directory), no test binds a fixed network port, and environment variables are per-process, so package-level parallelism cannot leak state between packages. Remove the flag and document why concurrency is safe now. Verified with back-to-back full-suite runs under -race with zero failures. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * test(nexus): add the state-layer lifecycle integration suite The regressions this month (the boot-order deadlock, the policy get-by-name break, the restore flow) were caught by hand or by the live drill, never by tests: nothing exercised the seams between state, persist, and recovery. Slice A of specs/integration-tests.md closes that gap inside the normal suite. The new app/nexus/internal/state/integration package walks the state layer through its whole life against the real sqlite stack: initialize and verify the root key is cached; write a secret and a policy; prove a duplicate Initialize neither recomputes key material nor recreates the backend (the pre-existing secret stays readable); export operator recovery shards; zero the root key to simulate total crash; restore through the real RestoreBackingStoreFromPilotShards path using only a threshold-sized shard subset; verify the recovered key equals the original and the pre-crash data reads back; and exercise deletion and undeletion on the restored state. The restore path reaches for a SPIFFE source after initializing state; the test sets a malformed SPIFFE_ENDPOINT_SOCKET so that step fails at validation instead of dialing the missing agent forever, and recovers the expected log.FatalErr panic via SPIKE_STACK_TRACES_ON_LOG_FATAL. The missing SVID-acquisition timeout that makes the unbounded dial possible is tracked as its own Phase 5 task. Slice B (Pilot behavior against an uninitialized or unreachable Nexus) stays open on the spec's questions about build-tagged live tests. Spec: specs/integration-tests.md Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> --------- Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com>
1 parent 67c88f4 commit 01954f8

5 files changed

Lines changed: 330 additions & 8 deletions

File tree

.context/TASKS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ the name-based policy work.
4747
- [ ] Graduate generic internal helpers to spike-sdk-go (nonce/crypto, Shamir verify, permission (de)serialize, validation, trust/spiffeid, URL builders, `GCMNonceSize`, `Id()`, canonical permission set) → ideas/research-sdk-extraction.md #source:jira.xml #added:2026-07-14
4848

4949
### Phase 3: Testing `#priority:medium`
50-
- [ ] Make `make test` concurrent again (currently serialized by env setup) → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14
50+
- [x] Make `make test` concurrent again → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 #done:2026-07-17 (removed -p 1 once the data-dir isolation landed; nothing else shared state across packages — no fixed ports, no t.Parallel, env vars are per-process. Full -race suite: 29.7s serialized to 2.9s concurrent, roughly 10x; two consecutive concurrent runs clean)
5151
- [x] Move the sqlite state tests off the real ~/.spike/data/spike.db: make test deleted the live dev environment database mid-run (bit us three times this week). #added:2026-07-16 #done:2026-07-16 (fs.NexusDataFolder is sync.Once-memoized, so per-test t.Setenv cannot work; instead each affected package sets SPIKE_NEXUS_DATA_DIR to a per-run temp dir in TestMain before the first resolution — state/base, state/persist, and backend/sqlite/persist — verified: a full package run leaves ~/.spike untouched)
52-
- [ ] Add integration tests: root key cached/recovered/not-re-initialized; secret & policy CRUD; Pilot denies when Nexus uninitialized / warns when unreachable → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14
52+
- [ ] Add integration tests: root key cached/recovered/not-re-initialized; secret & policy CRUD; Pilot denies when Nexus uninitialized / warns when unreachable → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 #in-progress (2026-07-17: Slice A shipped — specs/integration-tests.md; app/nexus/internal/state/integration covers the root-key lifecycle, the not-re-initialized-twice invariant, CRUD, and an in-process shard-restore round trip inside the normal suite. Remaining: Slice B, the Pilot uninitialized/unreachable behaviors, gated on the spec open questions)
5353
- [ ] Raise CLI command coverage to 60%+ via unit + HTTP-mock tests; fix `t.Skip()`ed tests; DI-refactor `sendShardsToKeepers` → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14
5454
- [x] `start.sh` should exercise recovery/restore and encryption/decryption #source:jira.xml #added:2026-07-14 #done:2026-07-16 (encryption/decryption checks live in start.sh since the policy-validation rework; recovery/restore is exercised by make drill-recovery, kept as a separate second-terminal script deliberately so the crash simulation never runs inside the normal startup path)
5555
- [x] Scripted live recovery/restore drill: once `make start` completes cleanly, run `spike operator recover`, kill Nexus and the Keepers, restart Nexus alone, feed the shards back via `spike operator restore` (scriptable via stdin since fix/operator-restore), and verify a pre-crash secret reads back. Rationale: the 2026-07-16 code review found no live breakage (shard-index fidelity intact end to end; guards use exact SPIFFE role matching, unaffected by the policy-name migration), so only a drill can prove the Phase 1 "recovery/restore is broken" claim stale and close both tasks. Needs the recover/restore role entries (spire-server-entry-recover-register.sh / -restore-register.sh), which make start does not register by default. #added:2026-07-16 #done:2026-07-16 (implemented as hack/bare-metal/drill/recovery-drill.sh behind make drill-recovery; the drill first exposed the Nexus boot-order deadlock, then passed end to end once it was fixed)
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/
2+
// \\\\\ Copyright 2024-present SPIKE contributors.
3+
// \\\\\\\ SPDX-License-Identifier: Apache-2.0
4+
5+
// Package integration exercises the SPIKE Nexus state layer end to end
6+
// against the real sqlite persistence stack: the root-key lifecycle,
7+
// secret and policy operations, and the operator shard-restore round
8+
// trip (an in-process mirror of the live recovery drill under
9+
// hack/bare-metal/drill). The package contains no production code; it
10+
// exists so the seams between state, persist, and recovery stay covered
11+
// by the normal test suite. See specs/integration-tests.md, Slice A.
12+
package integration
Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
// \\ SPIKE: Secure your secrets with SPIFFE. — https://spike.ist/
2+
// \\\\\ Copyright 2024-present SPIKE contributors.
3+
// \\\\\\\ SPDX-License-Identifier: Apache-2.0
4+
5+
package integration
6+
7+
import (
8+
"fmt"
9+
"os"
10+
"testing"
11+
12+
"github.com/spiffe/spike-sdk-go/api/entity/data"
13+
"github.com/spiffe/spike-sdk-go/config/env"
14+
"github.com/spiffe/spike-sdk-go/crypto"
15+
"github.com/spiffe/spike-sdk-go/security/mem"
16+
17+
"github.com/spiffe/spike/app/nexus/internal/initialization/recovery"
18+
state "github.com/spiffe/spike/app/nexus/internal/state/base"
19+
)
20+
21+
// TestMain isolates the package run: the sqlite backend writes into a
22+
// per-run temporary directory (fs.NexusDataFolder memoizes its result,
23+
// so the override must precede the first resolution), and the backend
24+
// store type is pinned to sqlite explicitly since the lifecycle under
25+
// test only exists for persistent backends.
26+
func TestMain(m *testing.M) {
27+
dir, mkErr := os.MkdirTemp("", "spike-state-integration-test-*")
28+
if mkErr != nil {
29+
fmt.Fprintln(os.Stderr,
30+
"failed to create a temporary data directory:", mkErr)
31+
os.Exit(1)
32+
}
33+
34+
for key, value := range map[string]string{
35+
env.NexusDataDir: dir,
36+
env.NexusBackendStore: "sqlite",
37+
} {
38+
if setErr := os.Setenv(key, value); setErr != nil {
39+
_ = os.RemoveAll(dir)
40+
fmt.Fprintln(os.Stderr, "failed to set "+key+":", setErr)
41+
os.Exit(1)
42+
}
43+
}
44+
45+
code := m.Run()
46+
47+
_ = os.RemoveAll(dir)
48+
os.Exit(code)
49+
}
50+
51+
const (
52+
secretPath = "integration/db/creds"
53+
policyName = "integration-workload-can-read"
54+
)
55+
56+
// TestStateLifecycle walks the state layer through its whole life:
57+
// initialization, secret and policy writes, a duplicate initialization
58+
// (which must not recompute or corrupt anything), the export of
59+
// operator recovery shards, a simulated root-key loss, a shard-based
60+
// restore, and finally proof that the pre-crash data is readable again.
61+
// The stages depend on each other and run in order.
62+
func TestStateLifecycle(t *testing.T) {
63+
rootKey := &[crypto.AES256KeySize]byte{}
64+
for i := range rootKey {
65+
rootKey[i] = byte(i + 1)
66+
}
67+
68+
// Stage 1: initialize and verify the root key is cached.
69+
state.Initialize(rootKey)
70+
if state.RootKeyZero() {
71+
t.Fatal("root key is not cached after Initialize")
72+
return
73+
}
74+
75+
// Stage 2: write a secret and a policy through the real stack.
76+
secretValues := map[string]string{
77+
"username": "spike",
78+
"password": "integration-v1",
79+
}
80+
if upsertErr := state.UpsertSecret(secretPath, secretValues); upsertErr != nil {
81+
t.Fatalf("failed to upsert the secret: %v", upsertErr)
82+
return
83+
}
84+
85+
got, getErr := state.GetSecret(secretPath, 0)
86+
if getErr != nil {
87+
t.Fatalf("failed to read the secret back: %v", getErr)
88+
return
89+
}
90+
if got["password"] != secretValues["password"] {
91+
t.Fatalf("secret round trip mismatch: got %q", got["password"])
92+
return
93+
}
94+
95+
if _, policyErr := state.UpsertPolicy(data.Policy{
96+
Name: policyName,
97+
SPIFFEIDPattern: `^spiffe://spike\.ist/workload/.*$`,
98+
PathPattern: `^integration/.*$`,
99+
Permissions: []data.PolicyPermission{"read"},
100+
}); policyErr != nil {
101+
t.Fatalf("failed to upsert the policy: %v", policyErr)
102+
return
103+
}
104+
105+
policy, policyGetErr := state.GetPolicy(policyName)
106+
if policyGetErr != nil {
107+
t.Fatalf("failed to read the policy back: %v", policyGetErr)
108+
return
109+
}
110+
if policy.PathPattern != `^integration/.*$` {
111+
t.Fatalf("policy round trip mismatch: got %q", policy.PathPattern)
112+
return
113+
}
114+
115+
// Stage 3: a duplicate initialization must not recompute root key
116+
// material or recreate the backend. The observable invariant: the
117+
// secret written before the duplicate call stays readable, which
118+
// proves the backend (and the cipher derived from the original
119+
// key) survived intact.
120+
state.Initialize(rootKey)
121+
if state.RootKeyZero() {
122+
t.Fatal("root key lost after a duplicate Initialize")
123+
return
124+
}
125+
if _, rereadErr := state.GetSecret(secretPath, 0); rereadErr != nil {
126+
t.Fatalf("secret unreadable after duplicate Initialize: %v", rereadErr)
127+
return
128+
}
129+
130+
// Stage 4: export recovery shards while healthy, as the operator
131+
// recover flow does, and keep only a threshold-sized subset to
132+
// prove reconstruction does not need every share.
133+
shardMap := recovery.NewPilotRecoveryShards()
134+
threshold := env.ShamirThresholdVal()
135+
if len(shardMap) < threshold {
136+
t.Fatalf("expected at least %d shards, got %d",
137+
threshold, len(shardMap))
138+
return
139+
}
140+
141+
shards := make([]crypto.ShamirShard, 0, threshold)
142+
for idx, value := range shardMap {
143+
if len(shards) == threshold {
144+
break
145+
}
146+
shards = append(shards, crypto.ShamirShard{
147+
ID: uint64(idx),
148+
Value: value,
149+
})
150+
}
151+
152+
// Stage 5: simulate the crash by zeroing the cached root key, the
153+
// in-process equivalent of losing Nexus and every Keeper.
154+
state.LockRootKey()
155+
mem.ClearRawBytes(state.RootKeyNoLock())
156+
state.UnlockRootKey()
157+
if !state.RootKeyZero() {
158+
t.Fatal("root key still cached after the simulated crash")
159+
return
160+
}
161+
162+
// Stage 6: restore from the shard subset. The restore path
163+
// initializes the state first and only then reaches for a SPIFFE
164+
// source to hydrate the Keepers; that step fails via log.FatalErr,
165+
// which the panic mode converts into a recoverable panic. The panic
166+
// is therefore expected here, and it fires after the part under
167+
// test has completed.
168+
//
169+
// A malformed workload API address makes the source creation fail
170+
// at validation time. With no SPIFFE_ENDPOINT_SOCKET at all,
171+
// go-spiffe would instead dial the default socket with an
172+
// unbounded context and hang the test forever (the missing
173+
// SVID-acquisition timeout is tracked as its own task).
174+
t.Setenv("SPIFFE_ENDPOINT_SOCKET", "bogus://fail-fast")
175+
t.Setenv("SPIKE_STACK_TRACES_ON_LOG_FATAL", "true")
176+
func() {
177+
defer func() {
178+
if r := recover(); r == nil {
179+
t.Error("expected a panic at the SPIFFE-source boundary" +
180+
" after the state restore")
181+
}
182+
}()
183+
recovery.RestoreBackingStoreFromPilotShards(shards)
184+
}()
185+
186+
// Stage 7: the restore must have recomputed the original root key
187+
// and the pre-crash data must be readable again.
188+
if state.RootKeyZero() {
189+
t.Fatal("root key not restored from shards")
190+
return
191+
}
192+
193+
state.LockRootKey()
194+
restoredMatches := *state.RootKeyNoLock() == *rootKey
195+
state.UnlockRootKey()
196+
if !restoredMatches {
197+
t.Fatal("restored root key differs from the original")
198+
return
199+
}
200+
201+
restored, restoredErr := state.GetSecret(secretPath, 0)
202+
if restoredErr != nil {
203+
t.Fatalf("secret unreadable after the restore: %v", restoredErr)
204+
return
205+
}
206+
if restored["password"] != secretValues["password"] {
207+
t.Fatalf("secret mismatch after the restore: got %q",
208+
restored["password"])
209+
return
210+
}
211+
212+
if _, policyRereadErr := state.GetPolicy(policyName); policyRereadErr != nil {
213+
t.Fatalf("policy unreadable after the restore: %v", policyRereadErr)
214+
return
215+
}
216+
217+
// Stage 8: deletion and undeletion survive the restored state.
218+
if delErr := state.DeleteSecret(secretPath, []int{1}); delErr != nil {
219+
t.Fatalf("failed to delete the secret: %v", delErr)
220+
return
221+
}
222+
if _, deletedErr := state.GetSecret(secretPath, 1); deletedErr == nil {
223+
t.Error("expected an error reading a deleted secret version")
224+
}
225+
226+
if undelErr := state.UndeleteSecret(secretPath, []int{1}); undelErr != nil {
227+
t.Fatalf("failed to undelete the secret: %v", undelErr)
228+
return
229+
}
230+
if _, revivedErr := state.GetSecret(secretPath, 1); revivedErr != nil {
231+
t.Fatalf("secret unreadable after undelete: %v", revivedErr)
232+
return
233+
}
234+
}

makefiles/Test.mk

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,15 @@ test/cover:
2525
# Usage: make test
2626
# Executes all tests in the project with verbose output and race detection
2727
# Does not generate coverage reports (use test/cover for that)
28-
# Flags: -v (verbose), -race (race detection), -buildvcs (include VCS info),
29-
# -p 1 (sequential execution to avoid race conditions)
30-
# NOTE: Sequential execution is temporary workaround for concurrent environment
31-
# variable/database access
32-
# FIXME: Remove -p 1 flag once issue with concurrent test isolation is resolved
28+
# Flags: -v (verbose), -race (race detection), -buildvcs (include VCS info)
29+
# Packages run concurrently (Go's default -p). This is safe because the
30+
# sqlite-backed test packages isolate their data directories per run via
31+
# TestMain (SPIKE_NEXUS_DATA_DIR points at a temporary directory), no
32+
# test binds a fixed network port, and environment variables are
33+
# per-process, so package-level parallelism cannot leak state.
3334
.PHONY: test
3435
test:
35-
go test -v -race -buildvcs -p 1 ./...
36+
go test -v -race -buildvcs ./...
3637

3738
# Comprehensive code quality audit
3839
# Usage: make audit

specs/integration-tests.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# Spec: Integration Test Suite
2+
3+
## Status
4+
5+
Proposed (2026-07-17). Backs the TASKS.md Phase 3 item "Add integration
6+
tests" and draws on `ideas/research-cli-testing.md`.
7+
8+
## Problem
9+
10+
The behaviors that broke this month — the Nexus boot-order deadlock,
11+
the policy get-by-name regression, the restore flow — were all caught
12+
by hand or by the live drill, not by tests. The suite exercises units
13+
well but nothing verifies the seams: root-key lifecycle against a real
14+
backing store, CRUD through the real persistence stack, and the
15+
Pilot's behavior when Nexus is uninitialized or unreachable.
16+
17+
## Approach: three slices, in increasing coupling order
18+
19+
### Slice A: state-layer integration (no SPIRE, runs in `make test`)
20+
21+
A new test-only package exercising the real state + sqlite persistence
22+
stack in-process, using the per-run temp-dir isolation that already
23+
exists (`SPIKE_NEXUS_DATA_DIR` via `TestMain`):
24+
25+
- Root key: `Initialize` caches it; a second initialization does not
26+
regenerate or re-encrypt (the "not re-initialized twice" invariant);
27+
`RestoreBackingStoreFromPilotShards` recovers the same key from
28+
threshold shards and the data written before the "crash" reads back
29+
(an in-process mirror of the live drill).
30+
- Secret CRUD: put/get/delete/undelete/list through state + sqlite,
31+
including versioning metadata.
32+
- Policy CRUD: create/get/delete/list by name through state + sqlite,
33+
including the pattern-regex compilation invariants.
34+
35+
These run as part of the normal suite; the concurrent gate keeps them
36+
cheap.
37+
38+
### Slice B: live integration (`//go:build integration`, opt-in)
39+
40+
Build-tagged tests gated on `SPIKE_INTEGRATION_TEST=1`, assuming a
41+
healthy `make start` environment, in the spirit of the recovery drill:
42+
43+
- Pilot denies operations when Nexus is uninitialized.
44+
- Pilot warns (does not hang or panic) when Nexus is unreachable.
45+
- A CLI-level smoke pass: secret put/get/delete, policy create/get by
46+
name, cipher round trip; asserts on stdout now that data goes there.
47+
48+
Run manually or in a dedicated CI job:
49+
`SPIKE_INTEGRATION_TEST=1 go test -tags=integration ./...`.
50+
51+
### Slice C: HTTP-mock helpers for CLI coverage (separate task)
52+
53+
Mocking the SDK's mTLS transport feeds the "CLI coverage to 60%+"
54+
task, not this one; it is out of scope here and tracked separately.
55+
56+
## Open questions (decide before Slice B)
57+
58+
1. Should Slice B live in this repository now, or wait until there is
59+
a CI runner with a SPIRE environment? A tagged suite nobody runs
60+
rots quietly, which is how the original "CI integration test is
61+
broken" task was born.
62+
2. Does Slice B subsume the recovery drill, or stay complementary?
63+
Recommendation: complementary. The drill kills real processes; the
64+
tagged tests only observe a healthy environment.
65+
66+
## Acceptance Criteria
67+
68+
- [ ] Slice A package passes in the normal `make test` run, isolated
69+
from `~/.spike`, leaving no artifacts.
70+
- [ ] The "not re-initialized twice" invariant has an explicit test.
71+
- [ ] The shard-restore round trip has an in-process test mirroring
72+
the live drill's semantics.
73+
- [ ] Secret and policy CRUD paths are covered end to end at the state
74+
layer, including deletion and undeletion.
75+
- [ ] Slice B lands only after the open questions are answered.

0 commit comments

Comments
 (0)