From 73a10fafc7bcd380554e7bdff6d6da53abf22e44 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 20 Oct 2026 00:30:10 +0200 Subject: [PATCH 1/2] Pull keycloak's moved-path harness into movedHarness, and test it reads the grant file (issue 1332) Adds the failing unit tests for keycloak's provider on the moved path: a grant-file consumer not moved should still be made and never held for retirement, and the moved harness should read MESH_RECEIVES as the grant path does. All three fail on an assertion against today's code, which wants nothing from the grant file on this path. --- .../cmd/keycloak-provider/grant_moved_test.go | 143 ++++++++++++++++++ .../keycloak/cmd/keycloak-provider/main.go | 20 ++- .../cmd/keycloak-provider/main_test.go | 19 +++ 3 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 modules/keycloak/cmd/keycloak-provider/grant_moved_test.go create mode 100644 modules/keycloak/cmd/keycloak-provider/main_test.go diff --git a/modules/keycloak/cmd/keycloak-provider/grant_moved_test.go b/modules/keycloak/cmd/keycloak-provider/grant_moved_test.go new file mode 100644 index 00000000..20843dcd --- /dev/null +++ b/modules/keycloak/cmd/keycloak-provider/grant_moved_test.go @@ -0,0 +1,143 @@ +package main + +// Keycloak runs the moved path with the grant file still open for its consumers not moved (issue 937), fixed for +// keycloak by issue 1332: mesh-mail-web, grafana and car-hunter are not moved, so without this fix keycloak never +// made their clients and held them for retirement. These tests drive the SDK's harness.Moved exactly as main.go +// wires it, against a fake Keycloak and an in-memory bus — no real backend. + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/provider" + "git.novox.be/novox/mesh-sdk/go/provider/harness" +) + +// grantGives writes a grant file (MESH_RECEIVES) naming consumers the plans do not move (issue 937): each one's +// secret file, and the json the grant path reads. +func grantGives(t *testing.T, given ...map[string]any) string { + t.Helper() + dir := t.TempDir() + for _, g := range given { + as := g["as"].(string) + secret := filepath.Join(dir, as+".secret") + if err := os.WriteFile(secret, []byte("pw-"+as+"\n"), 0o600); err != nil { + t.Fatal(err) + } + g["secret"] = secret + } + raw, _ := json.Marshal(map[string]any{"requirement": ProvisionName, "given": given}) + at := filepath.Join(dir, "mesh.json") + if err := os.WriteFile(at, raw, 0o600); err != nil { + t.Fatal(err) + } + return at +} + +// noOpPlan puts one assignment the bus can read that binds nothing of keycloak's, so the moved path has plans it +// could read (m.asked answers "read", issue 241) even when nothing is moved. +func noOpPlan(bus *bucketBus, node, module string) { + raw, _ := json.Marshal(map[string]any{"node": node, "module": module}) + _ = bus.Put(ctx, provider.AssignmentsBucket, node+"."+module, raw) +} + +// waitUntil polls cond until it is true or the timeout passes. +func waitUntil(t *testing.T, timeout time.Duration, cond func() bool) { + t.Helper() + deadline := time.Now().Add(timeout) + for { + if cond() { + return + } + if time.Now().After(deadline) { + t.Fatalf("condition never held within %s", timeout) + } + time.Sleep(2 * time.Millisecond) + } +} + +// movedWorld builds keycloak's moved path, as main.go wires it, against a fake Keycloak and a bucket bus. Nothing +// runs until start is called, so a test can seed the backend first; it is stopped when the test ends. +func movedWorld(t *testing.T, h *harness.Harness) (f *fakeKeycloak, start func()) { + t.Helper() + var o OidcClients + f, o = oidcWorld(t) + h.Adapter = provisioner{clients: o, announce: func(string, map[string]any) {}, log: func(string, ...any) {}} + if h.Every == 0 { + h.Every = 5 * time.Millisecond + } + consumersFile := filepath.Join(t.TempDir(), "consumers.json") + if err := os.WriteFile(consumersFile, []byte("[]"), 0o600); err != nil { + t.Fatal(err) + } + run, _, _, err := moved(consumersFile, h, testIssuer) + if err != nil { + t.Fatal(err) + } + bus := &bucketBus{buckets: map[string]map[string][]byte{}} + noOpPlan(bus, "elsewhere", "unrelated") + p := provider.New(provider.Options{Node: h.Node, Module: "keycloak", Bus: bus, + Vault: provider.ConsumersFile{Paths: []string{consumersFile}}, + Offers: map[string]provider.Offer{ProvisionName: {Credential: provider.PerConsumer, Serves: servedValues(testIssuer)}}, + Make: run.Make, Gone: run.Gone, Every: h.Every, Logf: func(string, ...any) {}}) + return f, func() { + runCtx, cancel := context.WithTimeout(ctx, 2*time.Second) + t.Cleanup(cancel) + go func() { _ = run.Run(runCtx, p, bus) }() + } +} + +// 1332/AC1: a consumer only the grant file gives (oidc-client for novox.mesh-mail-web, not moved) is wanted and +// Adapter.Create is asked for its client, though keycloak runs on the moved path. +func TestAGrantFileConsumerNotMovedIsMadeOnTheMovedPath(t *testing.T) { + mailWeb := "mesh_novox_mail_web" + receives := grantGives(t, map[string]any{"as": mailWeb, "node": "novox", "values": map[string]any{ + "label": "mail-web", "endpoint": "web", "callback": "/login/generic_oauth", "name": "mail.example.org"}}) + h := &harness.Harness{Resource: ProvisionName, Receives: receives, Node: "novox"} + f, start := movedWorld(t, h) + start() + waitUntil(t, time.Second, func() bool { + found := false + f.set(func() { + for _, c := range f.clients { + if c["clientId"] == mailWeb { + found = true + } + } + }) + return found + }) +} + +// 1332/AC2: a grant-file consumer whose client already exists in the backend (mesh_ace_grafana) counts as asked and +// is not held for retirement on the moved path. +func TestAGrantFileConsumerAlreadyInKeycloakIsNotRetiredOnTheMovedPath(t *testing.T) { + grafana := "mesh_ace_grafana" + receives := grantGives(t, map[string]any{"as": grafana, "node": "ace", "values": map[string]any{ + "label": "grafana", "endpoint": "web", "callback": "/login/generic_oauth", "name": "grafana.example.org"}}) + h := &harness.Harness{Resource: ProvisionName, Receives: receives, Node: "ace", + StablePasses: 1, StableFor: time.Millisecond} + f, start := movedWorld(t, h) + // Seed the backend with the client the mesh made earlier, exactly as grafana's already is — before anything + // runs, so the harness's one-time seed (issue 1332) sees it. + f.set(func() { + f.clients["existing-grafana"] = Rep{"id": "existing-grafana", "clientId": grafana, "enabled": true, + "attributes": map[string]any{Mark: "true"}} + }) + start() + // Many passes, well past StablePasses and StableFor if the login were ever counted unasked. + time.Sleep(200 * time.Millisecond) + f.set(func() { + c := f.clients["existing-grafana"] + if c["enabled"] == false { + t.Fatalf("retired %v though its grant still gives it (issue 1332)", c) + } + if _, retired := attributes(c)[MarkRetired]; retired { + t.Fatalf("marked retired %v though its grant still gives it (issue 1332)", c) + } + }) +} diff --git a/modules/keycloak/cmd/keycloak-provider/main.go b/modules/keycloak/cmd/keycloak-provider/main.go index 97633a51..f17d7c89 100644 --- a/modules/keycloak/cmd/keycloak-provider/main.go +++ b/modules/keycloak/cmd/keycloak-provider/main.go @@ -53,13 +53,7 @@ func main() { // Handed to the provisioner (novox/hq ADR 0373): the provider helper answers the consumers its // machine's client fills the file with; the grant directory is not read. Retirement is the // harness's, never run, so its guards and its tools stand (issue 805). - h = &harness.Harness{ - Resource: ProvisionName, - Adapter: provisioner{clients: OidcClients{KC: kc, Realm: realm}, guard: guard, announce: announce, log: logStderr}, - Log: logStderr, - Announce: announce, - Node: os.Getenv("MESH_NODE"), - } + h = movedHarness(os.Getenv, provisioner{clients: OidcClients{KC: kc, Realm: realm}, guard: guard, announce: announce, log: logStderr}) run, p, bus, err := moved(consumers, h, issuer) if err != nil { say("%v", err) @@ -101,6 +95,18 @@ func main() { } } +// movedHarness is the harness the moved path shares with the SDK's helper: what main wires together, pulled out so +// it can be built and inspected without starting the process (novox/hq issue 1332). +func movedHarness(getenv func(string) string, adapter harness.Adapter) *harness.Harness { + return &harness.Harness{ + Resource: ProvisionName, + Adapter: adapter, + Log: logStderr, + Announce: announce, + Node: getenv("MESH_NODE"), + } +} + // handedConsumersFile is the consumers' file when keycloak is handed to the provisioner: the file MESH_CONSUMERS_FILE // names, which the controller places only where it hands the module. Empty, where it is not set or the file is not // there, and keycloak then reads the grant directory exactly as it always did (issue 805). diff --git a/modules/keycloak/cmd/keycloak-provider/main_test.go b/modules/keycloak/cmd/keycloak-provider/main_test.go new file mode 100644 index 00000000..617873d6 --- /dev/null +++ b/modules/keycloak/cmd/keycloak-provider/main_test.go @@ -0,0 +1,19 @@ +package main + +// What main wires for the moved path (novox/hq issue 1332): the harness it builds from the environment, the same +// way the grant path reads its own settings. + +import "testing" + +// 1332/AC3: the moved-path harness, built from the environment, reads MESH_RECEIVES the way the grant path does +// (main.go's other branch, line 67), as postgres's moved harness does (postgres-provider/main.go:43). +func TestTheMovedPathHarnessReadsMESH_RECEIVESAsTheGrantPathDoes(t *testing.T) { + env := map[string]string{"MESH_RECEIVES": "/grants/mesh.json", "MESH_NODE": "novox"} + getenv := func(k string) string { return env[k] } + h := movedHarness(getenv, provisioner{}) + if h.Receives != env["MESH_RECEIVES"] { + t.Fatalf("Receives = %q, want %q — the grant file still gives every consumer not moved (issue 937), "+ + "read exactly as MESH_RECEIVES on the grant path, as postgres's moved harness does "+ + "(postgres-provider/main.go:43)", h.Receives, env["MESH_RECEIVES"]) + } +} From b9104454f9a5a3461c6a969ae199d921a6829955 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 20 Oct 2026 00:30:45 +0200 Subject: [PATCH 2/2] Read the grant file on keycloak's moved path, so a consumer not moved is still made (issue 1332) mesh-mail-web, grafana and car-hunter are not moved, so keycloak wanted nothing for them: it never made mesh_novox_mail_web, and held mesh_ace_grafana and mesh_ace_carhunt for retirement. Keycloak's moved spec now asks for the grant file's consumers too (WithGrants: true), and the moved harness reads MESH_RECEIVES exactly as the grant path does and as postgres's moved harness already did. --- modules/keycloak/cmd/keycloak-provider/main.go | 8 +++++--- modules/keycloak/cmd/keycloak-provider/moved.go | 8 +++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/modules/keycloak/cmd/keycloak-provider/main.go b/modules/keycloak/cmd/keycloak-provider/main.go index f17d7c89..81b274bc 100644 --- a/modules/keycloak/cmd/keycloak-provider/main.go +++ b/modules/keycloak/cmd/keycloak-provider/main.go @@ -51,8 +51,8 @@ func main() { say("%v — the provisioner cannot run without it", err) } else if consumers := handedConsumersFile(os.Getenv, os.Stat); consumers != "" { // Handed to the provisioner (novox/hq ADR 0373): the provider helper answers the consumers its - // machine's client fills the file with; the grant directory is not read. Retirement is the - // harness's, never run, so its guards and its tools stand (issue 805). + // machine's client fills the file with, and the grant file still gives the consumers not moved + // (issue 937, 1332). Retirement is the harness's, never run, so its guards and its tools stand (issue 805). h = movedHarness(os.Getenv, provisioner{clients: OidcClients{KC: kc, Realm: realm}, guard: guard, announce: announce, log: logStderr}) run, p, bus, err := moved(consumers, h, issuer) if err != nil { @@ -96,10 +96,12 @@ func main() { } // movedHarness is the harness the moved path shares with the SDK's helper: what main wires together, pulled out so -// it can be built and inspected without starting the process (novox/hq issue 1332). +// it can be built and inspected without starting the process. Receives is MESH_RECEIVES, read exactly as the grant +// path reads it (novox/hq issue 937), so the grant file still gives every consumer not moved (novox/hq issue 1332). func movedHarness(getenv func(string) string, adapter harness.Adapter) *harness.Harness { return &harness.Harness{ Resource: ProvisionName, + Receives: getenv("MESH_RECEIVES"), Adapter: adapter, Log: logStderr, Announce: announce, diff --git a/modules/keycloak/cmd/keycloak-provider/moved.go b/modules/keycloak/cmd/keycloak-provider/moved.go index b3b6d1b1..12fec0a7 100644 --- a/modules/keycloak/cmd/keycloak-provider/moved.go +++ b/modules/keycloak/cmd/keycloak-provider/moved.go @@ -43,10 +43,16 @@ func valuesOf(e provider.Entry) map[string]any { // moved is keycloak handed to the provisioner: the SDK's moved path (harness.Moved) answering its consumers, and the // harness (never run) whose retirement and tools it shares. What differs from postgres is only this declaration // (issue 849). +// +// **A consumer not moved is still the grant file's** (issue 937, ADR 0349 clause 17, fixed here by issue 1332): +// only a moved consumer's plan names keycloak as its provider, so the controller still gives every other consumer +// through keycloak's grant file (MESH_RECEIVES) until R1 (issue 682) — mesh-mail-web, grafana and car-hunter among +// them. Each client it gives is made and checked by the harness's grant pass exactly as on the grant path; a client +// a plan names is the helper's alone. func moved(consumersFile string, h *harness.Harness, issuer string) (*harness.Moved, *provider.Provider, provider.Bus, error) { return harness.NewMoved(consumersFile, h, harness.MovedSpec{Module: "keycloak", Provision: ProvisionName, Offer: provider.Offer{Credential: provider.PerConsumer, Serves: servedValues(issuer)}, Login: func(e provider.Entry) string { return e.As }, Values: valuesOf, - Noun: "client id", Made: "client", Backend: "Keycloak", WithGrants: false}) + Noun: "client id", Made: "client", Backend: "Keycloak", WithGrants: true}) }