Skip to content

Commit 67c88f4

Browse files
authored
Fix/pilot stdout and test isolation (#299)
* fix(pilot): route normal CLI output to stdout Cobra's Print family writes to OutOrStderr, and the root command never set an output writer, so every piece of normal output, secret values included, went to stderr. Shell redirection silently misbehaved: `spike secret get db/creds > creds.txt` produced an empty file while the secret splashed onto the terminal, and piping data into another tool required merging the streams first. Set the root command's output writer to stdout; subcommands inherit it. The PrintErr family keeps writing to stderr, so errors remain separable from data, which is what shell users and scripts expect. The bare-metal harness scripts already capture both streams (2>&1), so their behavior does not change. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * test(nexus): isolate sqlite state tests from the real data directory The sqlite-backed tests in state/base, state/persist, and backend/sqlite/persist operated on the real ~/.spike/data/spike.db and deleted it as part of their setup, which destroyed the database out from under a live bare-metal dev environment whenever `make test` ran alongside one. It bit three gate runs this week. fs.NexusDataFolder memoizes its result with sync.Once, so a per-test t.Setenv cannot redirect it once the first test has resolved the path. Instead, each affected package now sets SPIKE_NEXUS_DATA_DIR to a per-run temporary directory in TestMain, before anything resolves the folder, and removes the directory afterward. A full run of the three packages leaves ~/.spike untouched, and the temp-dir removal also cleans up the spike_test_*.db files the suite used to leave behind. TASKS.md: closes this task and the Pilot stdout task delivered by the previous commit. Spec: TBD Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> --------- Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com>
1 parent 6c36860 commit 67c88f4

5 files changed

Lines changed: 110 additions & 2 deletions

File tree

.context/TASKS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ the name-based policy work.
4141
- [x] Retry sqlite operations with exponential backoff on transient locks → ideas/research-db-resilience.md #source:jira.xml #added:2026-07-14 #done:2026-07-16 (withSerializableTx retries SQLITE_BUSY/SQLITE_LOCKED with exponential backoff at the single choke point every write flows through; reads rely on WAL plus the busy_timeout DSN parameter and honor the operation deadline; note the DB ops live under state/backend/sqlite/persist these days, not state/persist)
4242
- [x] Bound the Bootstrap keeper-wait loop with a configurable timeout/max-attempts instead of looping forever → ideas/research-db-resilience.md #source:jira.xml #added:2026-07-14 #done:2026-07-16 (stale: superseded by the SDK retry migration; broadcastToKeeper bounds each keeper with retry.WithMaxAttempts, a per-keeper context timeout, and configurable backoff intervals — app/bootstrap/internal/net/dispatch.go — and broadcast.go bounds init verification with WithMaxElapsedTime)
4343
- [ ] Make `env` accessors return sentinel errors instead of calling `log.FatalLn` (removes env→log circular dep, makes them testable) → ideas/research-env-error-handling.md #source:jira.xml #added:2026-07-14 (2026-07-16: the offending accessors now live in spike-sdk-go config/env, so the sentinel-error refactor is an upstream SDK change; the in-repo share is adapting callers once the SDK ships it)
44-
- [ ] Fix Pilot printing normal output to stderr: cobra Print* writes to OutOrStderr and the root command never calls SetOut, so data output (secrets included) lands on stderr and `spike secret get x > file.txt` yields an empty file; every harness script compensates with 2>&1. Set the root command output writer to stdout and audit scripts/docs for reliance on the old behavior. #added:2026-07-16
44+
- [x] Fix Pilot printing normal output to stderr: cobra Print* writes to OutOrStderr and the root command never calls SetOut, so data output (secrets included) lands on stderr and `spike secret get x > file.txt` yields an empty file; every harness script compensates with 2>&1. #added:2026-07-16 #done:2026-07-16 (rootCmd.SetOut(os.Stdout) in cmd.Initialize; PrintErr still goes to stderr, and the harness scripts that merge streams keep working)
4545

4646
### Phase 2: SDK Extraction `#priority:medium`
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`
5050
- [ ] Make `make test` concurrent again (currently serialized by env setup) → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14
51-
- [ ] Move the sqlite state tests off the real ~/.spike/data/spike.db to t.TempDir(): make test deletes the live dev environment database mid-run (bit us twice on 2026-07-15 and 2026-07-16), and shared global state is part of why tests are serialized with -p 1. #added:2026-07-16
51+
- [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)
5252
- [ ] 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
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)
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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 persist
6+
7+
import (
8+
"fmt"
9+
"os"
10+
"testing"
11+
12+
"github.com/spiffe/spike-sdk-go/config/env"
13+
)
14+
15+
// TestMain points SPIKE_NEXUS_DATA_DIR at a per-run temporary directory
16+
// before any test resolves the Nexus data folder. fs.NexusDataFolder
17+
// memoizes its result with sync.Once, so the override must happen before
18+
// the first call; doing it here isolates the whole package run from the
19+
// real ~/.spike/data directory, whose database these tests used to
20+
// delete out from under a live dev environment.
21+
func TestMain(m *testing.M) {
22+
dir, mkErr := os.MkdirTemp("", "spike-sqlite-persist-test-*")
23+
if mkErr != nil {
24+
fmt.Fprintln(os.Stderr,
25+
"failed to create a temporary data directory:", mkErr)
26+
os.Exit(1)
27+
}
28+
29+
if setErr := os.Setenv(env.NexusDataDir, dir); setErr != nil {
30+
_ = os.RemoveAll(dir)
31+
fmt.Fprintln(os.Stderr, "failed to set "+env.NexusDataDir+":", setErr)
32+
os.Exit(1)
33+
}
34+
35+
code := m.Run()
36+
37+
_ = os.RemoveAll(dir)
38+
os.Exit(code)
39+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
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 base
6+
7+
import (
8+
"fmt"
9+
"os"
10+
"testing"
11+
12+
"github.com/spiffe/spike-sdk-go/config/env"
13+
)
14+
15+
// TestMain points SPIKE_NEXUS_DATA_DIR at a per-run temporary directory
16+
// before any test resolves the Nexus data folder. fs.NexusDataFolder
17+
// memoizes its result with sync.Once, so the override must happen before
18+
// the first call; doing it here isolates the whole package run from the
19+
// real ~/.spike/data directory, whose database these tests used to
20+
// delete out from under a live dev environment.
21+
func TestMain(m *testing.M) {
22+
dir, mkErr := os.MkdirTemp("", "spike-state-base-test-*")
23+
if mkErr != nil {
24+
fmt.Fprintln(os.Stderr,
25+
"failed to create a temporary data directory:", mkErr)
26+
os.Exit(1)
27+
}
28+
29+
if setErr := os.Setenv(env.NexusDataDir, dir); setErr != nil {
30+
_ = os.RemoveAll(dir)
31+
fmt.Fprintln(os.Stderr, "failed to set "+env.NexusDataDir+":", setErr)
32+
os.Exit(1)
33+
}
34+
35+
code := m.Run()
36+
37+
_ = os.RemoveAll(dir)
38+
os.Exit(code)
39+
}

app/nexus/internal/state/persist/init_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
package persist
66

77
import (
8+
"fmt"
89
"os"
910
"testing"
1011

12+
"github.com/spiffe/spike-sdk-go/config/env"
1113
"github.com/spiffe/spike-sdk-go/crypto"
1214

1315
"github.com/spiffe/spike/app/nexus/internal/state/backend/memory"
@@ -311,12 +313,32 @@ func BenchmarkBackend_Access(b *testing.B) {
311313

312314
// Helper to clean environment between tests
313315
func TestMain(m *testing.M) {
316+
// Point SPIKE_NEXUS_DATA_DIR at a per-run temporary directory before
317+
// any test resolves the Nexus data folder. fs.NexusDataFolder
318+
// memoizes its result with sync.Once, so the override must happen
319+
// before the first call; doing it here isolates the whole package
320+
// run from the real ~/.spike/data directory, whose database these
321+
// tests used to delete out from under a live dev environment.
322+
dir, mkErr := os.MkdirTemp("", "spike-state-persist-test-*")
323+
if mkErr != nil {
324+
fmt.Fprintln(os.Stderr,
325+
"failed to create a temporary data directory:", mkErr)
326+
os.Exit(1)
327+
}
328+
329+
if setErr := os.Setenv(env.NexusDataDir, dir); setErr != nil {
330+
_ = os.RemoveAll(dir)
331+
fmt.Fprintln(os.Stderr, "failed to set "+env.NexusDataDir+":", setErr)
332+
os.Exit(1)
333+
}
334+
314335
// Run tests
315336
code := m.Run()
316337

317338
// Cleanup - reset to memory backend to avoid affecting other tests
318339
_ = os.Setenv("SPIKE_NEXUS_BACKEND_STORE", "memory")
319340
InitializeBackend(nil)
320341

342+
_ = os.RemoveAll(dir)
321343
os.Exit(code)
322344
}

app/spike/internal/cmd/cmd.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ import (
4444
// Initialize(source, "spiffe://example.org/pilot")
4545
// Execute()
4646
func Initialize(source *workloadapi.X509Source, SPIFFEID string) {
47+
// Cobra's Print family writes to OutOrStderr, so without an explicit
48+
// output writer every piece of normal output (secret values included)
49+
// lands on stderr, and shell redirection such as
50+
// `spike secret get db/creds > creds.txt` yields an empty file.
51+
// Route data output to stdout; the PrintErr family keeps writing to
52+
// stderr, so errors stay separable. Subcommands inherit this writer.
53+
rootCmd.SetOut(os.Stdout)
54+
4755
rootCmd.AddCommand(policy.NewCommand(source, SPIFFEID))
4856
rootCmd.AddCommand(secret.NewCommand(source, SPIFFEID))
4957
rootCmd.AddCommand(cipher.NewCommand(source, SPIFFEID))

0 commit comments

Comments
 (0)