Skip to content

Commit 62d6560

Browse files
committed
engine-manager: test the fail-closed setting read
Covers the two paths the earlier tests stubbed around: a relay error (broker unreachable) and an undecodable broker reply both deny the non-loopback bind. Also exercises the live relay path end to end with a true setting value. Signed-off-by: Mallikh Kalkere <mkalkere@users.noreply.github.com>
1 parent 818c190 commit 62d6560

1 file changed

Lines changed: 96 additions & 0 deletions

File tree

‎services/nvpair-engine-manager/bind_validation_test.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@ import (
77
"bytes"
88
"context"
99
"encoding/json"
10+
"errors"
1011
"io"
1112
"strings"
1213
"testing"
14+
15+
enginesettings "nvpair-shared/enginesettings"
1316
)
1417

1518
// codecCapture is an io.ReadWriter that discards reads and accumulates writes
@@ -95,3 +98,96 @@ func TestRunOpAllowsNonLoopbackBindWhenSettingOn(t *testing.T) {
9598
t.Errorf("invalid bind with setting on: response %q (reachedExec=%v), want invalid-address rejection", out, reachedExec)
9699
}
97100
}
101+
102+
// runOpWithRelaySend drives Manager.runOp like runOpError, but leaves the
103+
// lanBindForTest stub unset so lanBindAllowed takes the real relay path, with
104+
// send standing in for the broker round-trip.
105+
func runOpWithRelaySend(t *testing.T, params string, send func(string, any) error) (out string, reachedExec bool) {
106+
t.Helper()
107+
cap := &codecCapture{}
108+
m := NewManager(NewCodec(cap), &Executor{}, nil)
109+
// Same fixture deal as runOpError: nil the executor out so a
110+
// validation-passing call panics, which the recovery below reports.
111+
m.exec = nil
112+
m.settingsRelay.send = send
113+
msg := &Message{Method: "engine:start", Params: json.RawMessage(params)}
114+
id := json.RawMessage(`"test-1"`)
115+
msg.ID = &id
116+
defer func() {
117+
if recover() != nil {
118+
reachedExec = true
119+
}
120+
}()
121+
m.runOp(context.Background(), msg)
122+
return cap.buf.String(), false
123+
}
124+
125+
// replyWith returns a relay send func that answers the settings read with the
126+
// given raw result, as the broker would.
127+
func replyWith(m *Manager, result json.RawMessage) func(string, any) error {
128+
return func(_ string, value any) error {
129+
rel, ok := value.(enginesettings.Relay)
130+
if !ok {
131+
return errors.New("unexpected relay payload")
132+
}
133+
m.settingsRelay.mu.Lock()
134+
ch := m.settingsRelay.pending[rel.ID]
135+
m.settingsRelay.mu.Unlock()
136+
ch <- settingsReply{Result: result}
137+
return nil
138+
}
139+
}
140+
141+
// TestRunOpFailsClosedWhenSettingUnreadable: the node-setting read must fail
142+
// closed. If the relay to the broker errors, or the broker answers something
143+
// that is not a setting value, a non-loopback bind is rejected instead of
144+
// being let through.
145+
func TestRunOpFailsClosedWhenSettingUnreadable(t *testing.T) {
146+
// Broker unreachable: the relay send fails.
147+
out, reachedExec := runOpWithRelaySend(t, `{"engine":"ollama","bind":"192.168.1.5"}`,
148+
func(string, any) error { return errors.New("broker down") })
149+
if reachedExec || !strings.Contains(out, "bind must be a loopback address") {
150+
t.Errorf("relay error: response %q (reachedExec=%v), want loopback rejection", out, reachedExec)
151+
}
152+
153+
// Broker answers garbage: the setting value cannot be decoded.
154+
cap := &codecCapture{}
155+
m := NewManager(NewCodec(cap), &Executor{}, nil)
156+
m.exec = nil
157+
m.settingsRelay.send = replyWith(m, json.RawMessage(`not-json`))
158+
msg := &Message{Method: "engine:start", Params: json.RawMessage(`{"engine":"ollama","bind":"192.168.1.5"}`)}
159+
id := json.RawMessage(`"test-1"`)
160+
msg.ID = &id
161+
func() {
162+
defer func() { _ = recover() }()
163+
m.runOp(context.Background(), msg)
164+
}()
165+
if out := cap.buf.String(); !strings.Contains(out, "bind must be a loopback address") {
166+
t.Errorf("garbage reply: response %q, want loopback rejection", out)
167+
}
168+
}
169+
170+
// TestRunOpReadsLiveSettingThroughRelay: the real relay path (no test stub)
171+
// honors the broker's answer. A true value lets the bind through; this is the
172+
// path production takes on every start.
173+
func TestRunOpReadsLiveSettingThroughRelay(t *testing.T) {
174+
cap := &codecCapture{}
175+
m := NewManager(NewCodec(cap), &Executor{}, nil)
176+
m.exec = nil
177+
m.settingsRelay.send = replyWith(m, json.RawMessage(`{"value": true}`))
178+
msg := &Message{Method: "engine:start", Params: json.RawMessage(`{"engine":"ollama","bind":"192.168.1.5"}`)}
179+
id := json.RawMessage(`"test-1"`)
180+
msg.ID = &id
181+
reachedExec := false
182+
func() {
183+
defer func() {
184+
if recover() != nil {
185+
reachedExec = true
186+
}
187+
}()
188+
m.runOp(context.Background(), msg)
189+
}()
190+
if out := cap.buf.String(); !reachedExec || strings.Contains(out, "bind must be") {
191+
t.Errorf("live setting true: response %q (reachedExec=%v), want validation to pass", out, reachedExec)
192+
}
193+
}

0 commit comments

Comments
 (0)