Skip to content

Commit 4cc7bb1

Browse files
authored
Merge pull request #470 from huang195/fix/placeholder-swap-followups
fix(authbridge): post-#464 follow-ups — abctl pipeline panic + echo demo preflight
2 parents 191b2cb + b830120 commit 4cc7bb1

8 files changed

Lines changed: 181 additions & 10 deletions

File tree

authbridge/cmd/abctl/tui/app.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,11 @@ type model struct {
216216
catalogTbl table.Model
217217
detailVp viewport.Model
218218
detailEvent *pipeline.SessionEvent
219-
detailPlugin *apiclient.PipelinePlugin
219+
// detailInvocation is the plugin invocation selected in the events pane
220+
// when the detail view was opened. Used to re-scope the event to that
221+
// plugin on resize/re-render. nil means "whole event" (no invocation).
222+
detailInvocation *pipeline.Invocation
223+
detailPlugin *apiclient.PipelinePlugin
220224
filterInput textinput.Model
221225

222226
// visibleRows holds the invocationRow spec for each rendered row in

authbridge/cmd/abctl/tui/detail_pane.go

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,43 @@ import (
99
"github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline"
1010
)
1111

12+
// eventScopedToPlugin returns a shallow copy of e whose Invocations and
13+
// per-plugin Plugins map are limited to the given plugin. Event-level context
14+
// (protocol slot, identity) is preserved; filterForDetail still trims those by
15+
// phase. Returns e unchanged when plugin is empty.
16+
func eventScopedToPlugin(e *pipeline.SessionEvent, plugin string) *pipeline.SessionEvent {
17+
if e == nil || plugin == "" {
18+
return e
19+
}
20+
scoped := *e // shallow copy
21+
if e.Invocations != nil {
22+
scoped.Invocations = &pipeline.Invocations{
23+
Inbound: filterInvocationsByPlugin(e.Invocations.Inbound, plugin),
24+
Outbound: filterInvocationsByPlugin(e.Invocations.Outbound, plugin),
25+
}
26+
}
27+
if e.Plugins != nil {
28+
if raw, ok := e.Plugins[plugin]; ok {
29+
scoped.Plugins = map[string]json.RawMessage{plugin: raw}
30+
} else {
31+
scoped.Plugins = nil
32+
}
33+
}
34+
return &scoped
35+
}
36+
37+
// filterInvocationsByPlugin returns the invocations in invs whose Plugin
38+
// matches plugin, preserving order. Returns nil when none match.
39+
func filterInvocationsByPlugin(invs []pipeline.Invocation, plugin string) []pipeline.Invocation {
40+
var out []pipeline.Invocation
41+
for _, iv := range invs {
42+
if iv.Plugin == plugin {
43+
out = append(out, iv)
44+
}
45+
}
46+
return out
47+
}
48+
1249
// showDetail loads e into the detail viewport as colorized JSON and
1350
// remembers the focused event so yank (y) can find it.
1451
//
@@ -21,9 +58,18 @@ import (
2158
// When the event arrived over TLS (SessionEvent.TLS non-nil), a small
2259
// header block is prepended to the JSON so operators can see the
2360
// connection-level identity at a glance. Absent for plaintext events.
24-
func (m *model) showDetail(e *pipeline.SessionEvent) {
61+
//
62+
// The events list is one row per plugin invocation, so showDetail takes the
63+
// selected invocation and renders a copy of the event scoped to that plugin
64+
// (see eventScopedToPlugin). A nil invocation renders the whole event.
65+
func (m *model) showDetail(e *pipeline.SessionEvent, inv *pipeline.Invocation) {
2566
m.detailEvent = e
26-
data, err := json.Marshal(e)
67+
m.detailInvocation = inv
68+
ev := e
69+
if inv != nil {
70+
ev = eventScopedToPlugin(e, inv.Plugin)
71+
}
72+
data, err := json.Marshal(ev)
2773
if err != nil {
2874
m.detailVp.SetContent("error marshaling event: " + err.Error())
2975
return

authbridge/cmd/abctl/tui/detail_pane_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package tui
22

33
import (
4+
"encoding/json"
45
"strings"
56
"testing"
67

@@ -65,3 +66,62 @@ func TestTLSHeader_PeerOnly(t *testing.T) {
6566
t.Errorf("tlsHeader unexpectedly included version/cipher on peer-only state\ngot:\n%s", got)
6667
}
6768
}
69+
70+
// eventScopedToPlugin must restrict both the Invocations slices and the
71+
// per-plugin Plugins map to the selected plugin, and must NOT mutate the
72+
// original event (the shallow copy aliases the slices/map otherwise).
73+
func TestEventScopedToPlugin_FiltersToSelectedPlugin(t *testing.T) {
74+
ev := &pipeline.SessionEvent{
75+
Invocations: &pipeline.Invocations{
76+
Inbound: []pipeline.Invocation{
77+
{Plugin: "jwt-validation", Action: pipeline.ActionAllow},
78+
{Plugin: "a2a-parser", Action: pipeline.ActionObserve},
79+
},
80+
},
81+
Plugins: map[string]json.RawMessage{
82+
"jwt-validation": json.RawMessage("{}"),
83+
"a2a-parser": json.RawMessage("{}"),
84+
},
85+
}
86+
87+
scoped := eventScopedToPlugin(ev, "jwt-validation")
88+
89+
if got := len(scoped.Invocations.Inbound); got != 1 {
90+
t.Fatalf("scoped inbound invocations = %d, want 1", got)
91+
}
92+
if got := scoped.Invocations.Inbound[0].Plugin; got != "jwt-validation" {
93+
t.Errorf("scoped invocation plugin = %q, want %q", got, "jwt-validation")
94+
}
95+
if got := len(scoped.Plugins); got != 1 {
96+
t.Fatalf("scoped Plugins entries = %d, want 1", got)
97+
}
98+
if _, ok := scoped.Plugins["jwt-validation"]; !ok {
99+
t.Errorf("scoped Plugins missing jwt-validation key: %v", scoped.Plugins)
100+
}
101+
if _, ok := scoped.Plugins["a2a-parser"]; ok {
102+
t.Errorf("scoped Plugins unexpectedly retained a2a-parser")
103+
}
104+
105+
// Original event must be untouched (no aliasing of slice/map).
106+
if got := len(ev.Invocations.Inbound); got != 2 {
107+
t.Errorf("original inbound invocations mutated: = %d, want 2", got)
108+
}
109+
if got := len(ev.Plugins); got != 2 {
110+
t.Errorf("original Plugins map mutated: = %d, want 2", got)
111+
}
112+
}
113+
114+
// An empty plugin string means "no specific invocation" — the helper returns
115+
// the event unchanged (same pointer) so old whole-event behavior is preserved.
116+
func TestEventScopedToPlugin_EmptyPluginReturnsUnchanged(t *testing.T) {
117+
ev := &pipeline.SessionEvent{
118+
Invocations: &pipeline.Invocations{
119+
Inbound: []pipeline.Invocation{
120+
{Plugin: "jwt-validation", Action: pipeline.ActionAllow},
121+
},
122+
},
123+
}
124+
if got := eventScopedToPlugin(ev, ""); got != ev {
125+
t.Errorf("eventScopedToPlugin(ev, \"\") = %p, want original %p", got, ev)
126+
}
127+
}

authbridge/cmd/abctl/tui/events_pane.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,19 @@ func (m *model) selectedEvent() *pipeline.SessionEvent {
184184
return m.visibleRows[cur].event
185185
}
186186

187+
// selectedInvocation returns the plugin invocation for the highlighted events
188+
// row, or nil (pseudo-row for an event with no invocations, or no rows).
189+
func (m *model) selectedInvocation() *pipeline.Invocation {
190+
if len(m.visibleRows) == 0 {
191+
return nil
192+
}
193+
cur := m.eventsTbl.Cursor()
194+
if cur < 0 || cur >= len(m.visibleRows) {
195+
return nil
196+
}
197+
return m.visibleRows[cur].inv
198+
}
199+
187200
// invocationRow is one table row — the cartesian product of SessionEvent
188201
// × Invocation. An event with N plugin invocations produces N rows; an
189202
// event with no invocations produces one row with an empty invocation.

authbridge/cmd/abctl/tui/keys.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd {
207207
if ev == nil {
208208
return nil
209209
}
210-
m.showDetail(ev)
210+
m.showDetail(ev, m.selectedInvocation())
211211
m.pane = paneDetail
212212
return nil
213213
case panePipeline:
@@ -476,7 +476,7 @@ func (m *model) layout() {
476476
// Re-wrap the detail viewport to the new width so long JSON values
477477
// continue to fit after a terminal resize.
478478
if m.detailEvent != nil {
479-
m.showDetail(m.detailEvent)
479+
m.showDetail(m.detailEvent, m.detailInvocation)
480480
}
481481
}
482482

authbridge/cmd/abctl/tui/pipeline_pane.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,11 @@ func (m *model) rebuildPipelineTable() {
4040
for _, p := range m.pipeline.Inbound {
4141
rows = append(rows, pipelineRow(p, counts[p.Name], m.pipeline.Inbound))
4242
}
43-
// Divider between inbound and outbound.
44-
rows = append(rows, table.Row{"", "", "── (app) ──", "", "", "", ""})
43+
// Divider between inbound and outbound. Cell count MUST match the 6
44+
// columns defined in newPipelineTable (#, DIRECTION, PLUGIN, DEPS, BODY,
45+
// EVENTS) — bubbles' table.renderRow indexes columns by cell position and
46+
// panics on a mismatch.
47+
rows = append(rows, table.Row{"", "", "── (app) ──", "", "", ""})
4548
for _, p := range m.pipeline.Outbound {
4649
rows = append(rows, pipelineRow(p, counts[p.Name], m.pipeline.Outbound))
4750
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package tui
2+
3+
import (
4+
"testing"
5+
6+
"github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/apiclient"
7+
)
8+
9+
// TestRebuildPipelineTable_AllRowsMatchColumnCount is a regression guard:
10+
// every row added to the pipeline table — including the inbound/outbound
11+
// "(app)" divider — must have exactly as many cells as the table has columns.
12+
// The divider previously carried 7 cells against 6 columns, which made
13+
// bubbles' table.renderRow panic ("index out of range [6] with length 6")
14+
// the moment abctl rendered any pipeline.
15+
func TestRebuildPipelineTable_AllRowsMatchColumnCount(t *testing.T) {
16+
m := &model{
17+
pipeline: &apiclient.PipelineView{
18+
Inbound: []apiclient.PipelinePlugin{{Name: "jwt-validation", Direction: "inbound", Position: 1}},
19+
Outbound: []apiclient.PipelinePlugin{{Name: "token-exchange", Direction: "outbound", Position: 1}},
20+
},
21+
pipelineTbl: newPipelineTable(),
22+
}
23+
24+
// Must not panic: the buggy 7-cell divider panicked here via
25+
// SetRows → UpdateViewport → renderRow.
26+
m.rebuildPipelineTable()
27+
28+
rows := m.pipelineTbl.Rows()
29+
if len(rows) != 3 { // inbound plugin + divider + outbound plugin
30+
t.Fatalf("rows = %d, want 3 (inbound + divider + outbound)", len(rows))
31+
}
32+
const wantCells = 6 // #, DIRECTION, PLUGIN, DEPS, BODY, EVENTS
33+
for i, r := range rows {
34+
if len(r) != wantCells {
35+
t.Errorf("row %d has %d cells, want %d (must match column count)", i, len(r), wantCells)
36+
}
37+
}
38+
}

authbridge/demos/echo/Makefile

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
# Prerequisites:
1515
# - kagenti installed on a kind cluster (this is the operator-flow demo)
1616
# - kubectl, kind, podman/docker, python3 + PyYAML
17-
# - python-keycloak (for setup-keycloak; pip install python-keycloak)
17+
# - python-keycloak (for setup-keycloak; pip3 install --user python-keycloak)
1818

1919
.PHONY: help preflight build-sidecar load-sidecar override-sidecar-image \
2020
build-images load-images deploy wait-pods setup-keycloak \
@@ -94,6 +94,13 @@ preflight: ## Verify kagenti is installed + tools are available
9494
echo " Install: pip3 install --user pyyaml"; \
9595
exit 1; \
9696
}
97+
@python3 -c 'import keycloak' 2>/dev/null || { \
98+
echo "ERROR: python-keycloak is required (used later by setup-keycloak)."; \
99+
echo " Install: pip3 install --user python-keycloak"; \
100+
echo " Checked here in preflight so demo-echo fails before the"; \
101+
echo " build/deploy steps rather than after them."; \
102+
exit 1; \
103+
}
97104

98105
# ---------- Sidecar (authbridge proxy) ----------
99106

@@ -225,11 +232,11 @@ wait-pods: ## Wait for pods + operator-rendered authbridge ConfigMap
225232
# can't reach that, port-forward in another terminal:
226233
# kubectl port-forward service/keycloak-service -n keycloak 8080:8080
227234
# and set KEYCLOAK_URL=http://localhost:8080.
228-
# Requires the python-keycloak package: pip install python-keycloak
235+
# Requires the python-keycloak package: pip3 install --user python-keycloak
229236
setup-keycloak: ## Configure Keycloak (echo-upstream client, scopes, demo users)
230237
@python3 -c 'import keycloak' 2>/dev/null || { \
231238
echo "ERROR: python-keycloak is required for setup-keycloak."; \
232-
echo " Install: pip install python-keycloak"; \
239+
echo " Install: pip3 install --user python-keycloak"; \
233240
exit 1; \
234241
}
235242
python3 scripts/setup_keycloak.py --namespace $(NAMESPACE) --service-account $(AGENT_NAME)

0 commit comments

Comments
 (0)