diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index a95293ea8..fb2bcef56 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -2085,17 +2085,36 @@ func (d *Daemon) checkViperblockReady() bool { return d.natsConn.IsConnected() } -// checkPredastoreReady checks if predastore is reachable via TCP. +// predastoreReadinessClient is hoisted to package scope so the 2s readiness +// poll loop doesn't build a fresh transport every call. Verification is off +// because the probe authenticates nothing and reads no data. +var predastoreReadinessClient = &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // readiness probe only, no data exchanged + }, +} + +// checkPredastoreReady reports whether predastore is serving HTTPS, not just +// listening. Any response counts as ready, including the 401/403 an S3 +// endpoint returns for this deliberately unsigned request. func (d *Daemon) checkPredastoreReady() bool { host := admin.DialTarget(d.config.Predastore.Host) if host == "" { return true // no predastore configured, skip check } - conn, err := net.DialTimeout("tcp", host, 3*time.Second) + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://"+host+"/", nil) if err != nil { return false } - _ = conn.Close() + resp, err := predastoreReadinessClient.Do(req) + if err != nil { + return false + } + defer resp.Body.Close() return true } diff --git a/spinifex/daemon/predastore_ready_test.go b/spinifex/daemon/predastore_ready_test.go new file mode 100644 index 000000000..2a6c9cb38 --- /dev/null +++ b/spinifex/daemon/predastore_ready_test.go @@ -0,0 +1,52 @@ +package daemon + +import ( + "net" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/mulgadc/spinifex/spinifex/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCheckPredastoreReady_HTTPSServing asserts that any HTTP response from +// predastore's endpoint counts as ready, including 403 — the response an S3 +// endpoint gives this deliberately unsigned probe request. The prior TCP-dial +// probe treated a bare listening socket as ready and raced predastore's TLS +// handshake; this proves the replacement actually waits for HTTP. +func TestCheckPredastoreReady_HTTPSServing(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + t.Cleanup(srv.Close) + + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + d := &Daemon{config: &config.Config{Predastore: config.PredastoreConfig{Host: u.Host}}} + assert.True(t, d.checkPredastoreReady()) +} + +// TestCheckPredastoreReady_ClosedPort asserts a port nothing is listening on +// is reported not-ready, rather than panicking or failing open. +func TestCheckPredastoreReady_ClosedPort(t *testing.T) { + // Bind then immediately close to get a real, currently-unused port. + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := l.Addr().String() + require.NoError(t, l.Close()) + + d := &Daemon{config: &config.Config{Predastore: config.PredastoreConfig{Host: addr}}} + assert.False(t, d.checkPredastoreReady()) +} + +// TestCheckPredastoreReady_NoPredastoreConfigured asserts the short-circuit: +// an empty Predastore.Host means no predastore runs on this node, and the +// check must return ready without dialling anything. +func TestCheckPredastoreReady_NoPredastoreConfigured(t *testing.T) { + d := &Daemon{config: &config.Config{}} + assert.True(t, d.checkPredastoreReady()) +}