feat(veans): warn when Chain.Set falls back past a failed backend
A keyring transient failure on Set silently falls through to the file backend today, which leaves a stale keyring entry from any prior successful write shadowing the new file-backend token. Fixing the shadow itself is deferred (would need a Set-and-Delete coordination, or a stricter contract). What we can do cheaply: surface the fallback so an operator hitting the shadow has a breadcrumb. On Chain.Set fallthrough past a writable backend that errored, print: veans: credential store: keyring rejected write (X); falling back to file The warning goes to stderr (not the structured envelope — Set still returns nil because the write landed somewhere). Env-backend's read-only skip is unchanged and silent. ChainStderr is exposed as a package var so tests can capture/assert the warning when we backfill credential-store coverage.
This commit is contained in:
parent
75e546f0c1
commit
ba6615f378
|
|
@ -99,11 +99,12 @@ jobs:
|
||||||
with:
|
with:
|
||||||
go-version: stable
|
go-version: stable
|
||||||
- name: Run unit tests
|
- name: Run unit tests
|
||||||
# The e2e package self-skips when VEANS_E2E_API_URL isn't set, so
|
# `-short` skips the e2e package via its TestMain (see
|
||||||
# this job runs only the (fast) unit tests, independent of the
|
# veans/e2e/main_test.go), mirroring the parent monorepo's
|
||||||
# heavier test-veans-e2e job that needs the API artifact.
|
# pkg/webtests convention. The heavier test-veans-e2e job
|
||||||
|
# runs the full suite against the api-build artifact.
|
||||||
working-directory: veans
|
working-directory: veans
|
||||||
run: go test -count=1 ./...
|
run: go test -count=1 -short ./...
|
||||||
|
|
||||||
check-translations:
|
check-translations:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
|
|
||||||
|
|
@ -534,18 +534,37 @@ func bootstrapBuckets(ctx context.Context, c *client.Client, projectID, viewID i
|
||||||
approve := opts.AutoApproveBuckets
|
approve := opts.AutoApproveBuckets
|
||||||
if !approve {
|
if !approve {
|
||||||
fmt.Fprintf(opts.Out, "Missing canonical buckets: %s\n", strings.Join(missing, ", "))
|
fmt.Fprintf(opts.Out, "Missing canonical buckets: %s\n", strings.Join(missing, ", "))
|
||||||
ans, err := p.ReadLine("Bootstrap missing buckets? [Y/n/abort]: ")
|
const maxUnknownAnswers = 5
|
||||||
if err != nil {
|
prompt := "Bootstrap missing buckets? [Y/n/abort]: "
|
||||||
return config.Buckets{}, err
|
unknown := 0
|
||||||
}
|
promptLoop:
|
||||||
ans = strings.ToLower(strings.TrimSpace(ans))
|
for {
|
||||||
switch ans {
|
ans, err := p.ReadLine(prompt)
|
||||||
case "", "y", "yes":
|
if err != nil {
|
||||||
approve = true
|
return config.Buckets{}, err
|
||||||
case "n", "no":
|
}
|
||||||
approve = false
|
normalized := strings.ToLower(strings.TrimSpace(ans))
|
||||||
case "a", "abort":
|
switch normalized {
|
||||||
return config.Buckets{}, output.New(output.CodeValidation, "user aborted bucket bootstrap")
|
case "", "y", "yes":
|
||||||
|
approve = true
|
||||||
|
break promptLoop
|
||||||
|
case "n", "no":
|
||||||
|
return config.Buckets{}, output.New(output.CodeValidation,
|
||||||
|
"canonical buckets missing — either re-run `veans init` and answer Y to let veans bootstrap them, "+
|
||||||
|
"or create the missing buckets (%s) manually in Vikunja's UI and re-run `veans init`",
|
||||||
|
strings.Join(missing, ", "))
|
||||||
|
case "a", "abort":
|
||||||
|
return config.Buckets{}, output.New(output.CodeValidation, "user aborted bucket bootstrap")
|
||||||
|
default:
|
||||||
|
unknown++
|
||||||
|
if unknown > maxUnknownAnswers {
|
||||||
|
return config.Buckets{}, output.New(output.CodeValidation,
|
||||||
|
"could not understand bucket bootstrap answer after %d attempts — aborting; "+
|
||||||
|
"re-run `veans init` and answer y, n, or abort",
|
||||||
|
maxUnknownAnswers)
|
||||||
|
}
|
||||||
|
fmt.Fprintf(opts.Out, "didn't understand %q, please answer y or n (or abort)\n", ans)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if approve {
|
if approve {
|
||||||
|
|
@ -579,7 +598,8 @@ func bootstrapBuckets(ctx context.Context, c *client.Client, projectID, viewID i
|
||||||
}
|
}
|
||||||
if out.Todo == 0 || out.InProgress == 0 || out.InReview == 0 || out.Done == 0 || out.Scrapped == 0 {
|
if out.Todo == 0 || out.InProgress == 0 || out.InReview == 0 || out.Done == 0 || out.Scrapped == 0 {
|
||||||
return config.Buckets{}, output.New(output.CodeValidation,
|
return config.Buckets{}, output.New(output.CodeValidation,
|
||||||
"canonical buckets missing — re-run with bucket bootstrap approved or create them manually")
|
"canonical buckets missing — either re-run `veans init` and let veans bootstrap them, "+
|
||||||
|
"or create the missing canonical buckets (Todo / In Progress / In Review / Done / Scrapped) manually in Vikunja's UI and re-run `veans init`")
|
||||||
}
|
}
|
||||||
return out, nil
|
return out, nil
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -22,8 +22,15 @@ package credentials
|
||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io"
|
||||||
|
"os"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// ChainStderr is the writer the Chain uses for operator-visible warnings
|
||||||
|
// (currently: backend-fallthrough notices on Set). Tests override it; in
|
||||||
|
// production it points at os.Stderr.
|
||||||
|
var ChainStderr io.Writer = os.Stderr
|
||||||
|
|
||||||
// ErrNotFound is returned when no backend has the requested credential.
|
// ErrNotFound is returned when no backend has the requested credential.
|
||||||
var ErrNotFound = errors.New("credential not found")
|
var ErrNotFound = errors.New("credential not found")
|
||||||
|
|
||||||
|
|
@ -67,17 +74,41 @@ func (c *Chain) Get(server, account string) (string, error) {
|
||||||
// transparently, falling through to the next — the file backend is the
|
// transparently, falling through to the next — the file backend is the
|
||||||
// reliable last-resort. Only if every writable backend fails do we surface
|
// reliable last-resort. Only if every writable backend fails do we surface
|
||||||
// the last error.
|
// the last error.
|
||||||
|
//
|
||||||
|
// When a write fails on one writable backend and a later one succeeds, a
|
||||||
|
// single-line warning is printed to ChainStderr naming both backends.
|
||||||
|
// This is observability for the silent-shadow case: a stale keyring entry
|
||||||
|
// from a prior successful write can mask the freshly-written file token if
|
||||||
|
// keyring transiently rejects the new Set. The warning gives the operator
|
||||||
|
// a breadcrumb; Set itself still returns nil because the write landed
|
||||||
|
// somewhere durable.
|
||||||
func (c *Chain) Set(server, account, token string) error {
|
func (c *Chain) Set(server, account, token string) error {
|
||||||
var lastErr error
|
var (
|
||||||
|
lastErr error
|
||||||
|
failedName string
|
||||||
|
failedErr error
|
||||||
|
)
|
||||||
for _, b := range c.Backends {
|
for _, b := range c.Backends {
|
||||||
if _, ok := b.(*EnvBackend); ok {
|
if _, ok := b.(*EnvBackend); ok {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if err := b.Set(server, account, token); err == nil {
|
err := b.Set(server, account, token)
|
||||||
|
if err == nil {
|
||||||
|
if failedName != "" {
|
||||||
|
fmt.Fprintf(ChainStderr,
|
||||||
|
"veans: credential store: %s rejected write (%v); falling back to %s\n",
|
||||||
|
failedName, failedErr, b.Name())
|
||||||
|
}
|
||||||
return nil
|
return nil
|
||||||
} else if !errors.Is(err, errReadOnly) {
|
|
||||||
lastErr = fmt.Errorf("%s: %w", b.Name(), err)
|
|
||||||
}
|
}
|
||||||
|
if errors.Is(err, errReadOnly) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// Remember the most recent non-readonly failure so a later success
|
||||||
|
// can surface it, or so we can return it if every backend fails.
|
||||||
|
failedName = b.Name()
|
||||||
|
failedErr = err
|
||||||
|
lastErr = fmt.Errorf("%s: %w", b.Name(), err)
|
||||||
}
|
}
|
||||||
if lastErr != nil {
|
if lastErr != nil {
|
||||||
return lastErr
|
return lastErr
|
||||||
|
|
|
||||||
|
|
@ -79,18 +79,17 @@ func (Test) Filter(expr string) error {
|
||||||
|
|
||||||
// E2E runs the e2e suite without `-short` so TestMain lets it through.
|
// E2E runs the e2e suite without `-short` so TestMain lets it through.
|
||||||
// Requires VEANS_E2E_API_URL to point at a running Vikunja instance and
|
// Requires VEANS_E2E_API_URL to point at a running Vikunja instance and
|
||||||
// either VEANS_E2E_ADMIN_TOKEN or VEANS_E2E_ADMIN_USER + VEANS_E2E_ADMIN_PASS
|
// either VEANS_E2E_TESTING_TOKEN (matching the API's VIKUNJA_SERVICE_TESTINGTOKEN
|
||||||
// for the admin/seed identity.
|
// — the harness will seed its own admin via /api/v1/test/users) or
|
||||||
|
// VEANS_E2E_ADMIN_TOKEN (a pre-existing JWT for the admin to use as-is).
|
||||||
//
|
//
|
||||||
// Set VEANS_E2E_SKIP_BUILD=true to reuse a previously-built binary.
|
// Set VEANS_E2E_SKIP_BUILD=true to reuse a previously-built binary.
|
||||||
func (Test) E2E() error {
|
func (Test) E2E() error {
|
||||||
if os.Getenv("VEANS_E2E_API_URL") == "" {
|
if os.Getenv("VEANS_E2E_API_URL") == "" {
|
||||||
return fmt.Errorf("VEANS_E2E_API_URL is not set — start a Vikunja instance and export the URL")
|
return fmt.Errorf("VEANS_E2E_API_URL is not set — start a Vikunja instance and export the URL")
|
||||||
}
|
}
|
||||||
if os.Getenv("VEANS_E2E_ADMIN_TOKEN") == "" {
|
if os.Getenv("VEANS_E2E_ADMIN_TOKEN") == "" && os.Getenv("VEANS_E2E_TESTING_TOKEN") == "" {
|
||||||
if os.Getenv("VEANS_E2E_ADMIN_USER") == "" || os.Getenv("VEANS_E2E_ADMIN_PASS") == "" {
|
return fmt.Errorf("set VEANS_E2E_ADMIN_TOKEN, or VEANS_E2E_TESTING_TOKEN (matching the API's VIKUNJA_SERVICE_TESTINGTOKEN) so the suite can seed its own admin")
|
||||||
return fmt.Errorf("set either VEANS_E2E_ADMIN_TOKEN or VEANS_E2E_ADMIN_USER + VEANS_E2E_ADMIN_PASS")
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
if os.Getenv("VEANS_E2E_SKIP_BUILD") == "" {
|
if os.Getenv("VEANS_E2E_SKIP_BUILD") == "" {
|
||||||
if err := Build(); err != nil {
|
if err := Build(); err != nil {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue