From b7b1bf442a636286f20d0ca27d54c1110fc6c12e Mon Sep 17 00:00:00 2001 From: bussyjd Date: Fri, 10 Jul 2026 15:05:29 +0400 Subject: [PATCH 1/2] feat(serviceoffer-controller): render model provider/mcp_servers/turns from Agent CR Additive AgentSpec fields (modelProvider, mcpServers, maxTurns, disabledToolsets) threaded through renderHermesConfig so the CR is the source of truth for the agent's inference provider and paid MCP wiring. Unset fields render byte-identical to the prior fixed template (no CRD version bump, no behavior change for existing agents). Claude-Session: https://claude.ai/code/session_01VquWN9UMaSHH7MHGcG8bw1 --- .../base/templates/agent-crd.yaml | 70 +++++++++++++ internal/monetizeapi/types.go | 47 ++++++++- internal/monetizeapi/zz_generated.deepcopy.go | 49 ++++++++++ .../serviceoffercontroller/agent_render.go | 85 ++++++++++++++-- .../agent_render_test.go | 97 ++++++++++++++++++- 5 files changed, 335 insertions(+), 13 deletions(-) diff --git a/internal/embed/infrastructure/base/templates/agent-crd.yaml b/internal/embed/infrastructure/base/templates/agent-crd.yaml index 8338c0d6..ad7b2217 100644 --- a/internal/embed/infrastructure/base/templates/agent-crd.yaml +++ b/internal/embed/infrastructure/base/templates/agent-crd.yaml @@ -64,12 +64,82 @@ spec: type: object spec: properties: + disabledToolsets: + description: |- + Hermes agent.disabled_toolsets. Nil = ["memory","web"] (historical + default). Explicit empty list disables none. + items: + maxLength: 64 + type: string + maxItems: 32 + type: array + maxTurns: + description: Hermes agent.max_turns. Nil = 30 (historical sub-agent + default). + maximum: 10000 + minimum: 1 + type: integer + mcpServers: + description: |- + MCP servers rendered into Hermes config.yaml under mcp_servers. + Empty/omitted: no mcp_servers block (byte-identical to pre-field config). + items: + description: |- + MCPServer is one Hermes MCP server entry (stdio transport). Env values + may use ${VAR} interpolation; Hermes resolves them in-pod at runtime — + the controller does not expand them. + properties: + args: + description: Arguments passed to Command. Omitted from YAML + when empty. + items: + type: string + maxItems: 64 + type: array + command: + description: Command to launch the MCP server process. + maxLength: 256 + minLength: 1 + type: string + env: + additionalProperties: + type: string + description: |- + Environment variables for the MCP process. Values may contain + ${VAR} placeholders resolved by Hermes in-pod (not by the controller). + maxProperties: 64 + type: object + name: + description: Server name used as the key under mcp_servers in + config.yaml. + maxLength: 64 + minLength: 1 + pattern: ^[a-zA-Z0-9][a-zA-Z0-9._-]*$ + type: string + timeout: + description: Optional process timeout in seconds. Omitted from + YAML when nil. + minimum: 1 + type: integer + required: + - command + - name + type: object + maxItems: 32 + type: array model: description: |- LiteLLM model name to pin. Empty = controller picks cluster top-of-rank on first deploy and writes status.pinnedModel. maxLength: 256 type: string + modelProvider: + description: |- + Hermes model provider. Empty or "custom" keeps the cluster LiteLLM + path (base_url + api_key). Other values (e.g. "xai-oauth") omit those + and resolve credentials in-pod. + maxLength: 64 + type: string objective: description: |- Operator-supplied objective text. Substituted into the SOUL.md diff --git a/internal/monetizeapi/types.go b/internal/monetizeapi/types.go index ce7968ed..afca57e8 100644 --- a/internal/monetizeapi/types.go +++ b/internal/monetizeapi/types.go @@ -901,6 +901,11 @@ type AgentSpec struct { // top-of-rank on first deploy and writes status.pinnedModel. // +kubebuilder:validation:MaxLength=256 Model string `json:"model,omitempty"` + // Hermes model provider. Empty or "custom" keeps the cluster LiteLLM + // path (base_url + api_key). Other values (e.g. "xai-oauth") omit those + // and resolve credentials in-pod. + // +kubebuilder:validation:MaxLength=64 + ModelProvider string `json:"modelProvider,omitempty"` // Allow-listed skills written to the per-agent skills dir on first // reconcile. Agent can edit afterwards; this is a seed, not a sandbox. // +kubebuilder:validation:MaxItems=64 @@ -910,8 +915,46 @@ type AgentSpec struct { // Operator-supplied objective text. Substituted into the SOUL.md // template by the seeder on first write. Agent owns SOUL.md after that. // +kubebuilder:validation:MaxLength=4096 - Objective string `json:"objective,omitempty"` - Wallet AgentWallet `json:"wallet,omitempty"` + Objective string `json:"objective,omitempty"` + // MCP servers rendered into Hermes config.yaml under mcp_servers. + // Empty/omitted: no mcp_servers block (byte-identical to pre-field config). + // +kubebuilder:validation:MaxItems=32 + MCPServers []MCPServer `json:"mcpServers,omitempty"` + // Hermes agent.max_turns. Nil = 30 (historical sub-agent default). + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=10000 + MaxTurns *int `json:"maxTurns,omitempty"` + // Hermes agent.disabled_toolsets. Nil = ["memory","web"] (historical + // default). Explicit empty list disables none. + // +kubebuilder:validation:MaxItems=32 + // +kubebuilder:validation:items:MaxLength=64 + DisabledToolsets []string `json:"disabledToolsets,omitempty"` + Wallet AgentWallet `json:"wallet,omitempty"` +} + +// MCPServer is one Hermes MCP server entry (stdio transport). Env values +// may use ${VAR} interpolation; Hermes resolves them in-pod at runtime — +// the controller does not expand them. +type MCPServer struct { + // Server name used as the key under mcp_servers in config.yaml. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=64 + // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9][a-zA-Z0-9._-]*$` + Name string `json:"name"` + // Command to launch the MCP server process. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=256 + Command string `json:"command"` + // Arguments passed to Command. Omitted from YAML when empty. + // +kubebuilder:validation:MaxItems=64 + Args []string `json:"args,omitempty"` + // Optional process timeout in seconds. Omitted from YAML when nil. + // +kubebuilder:validation:Minimum=1 + Timeout *int `json:"timeout,omitempty"` + // Environment variables for the MCP process. Values may contain + // ${VAR} placeholders resolved by Hermes in-pod (not by the controller). + // +kubebuilder:validation:MaxProperties=64 + Env map[string]string `json:"env,omitempty"` } type AgentWallet struct { diff --git a/internal/monetizeapi/zz_generated.deepcopy.go b/internal/monetizeapi/zz_generated.deepcopy.go index 3fb6ee76..7d087c4b 100644 --- a/internal/monetizeapi/zz_generated.deepcopy.go +++ b/internal/monetizeapi/zz_generated.deepcopy.go @@ -187,6 +187,23 @@ func (in *AgentSpec) DeepCopyInto(out *AgentSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.MCPServers != nil { + in, out := &in.MCPServers, &out.MCPServers + *out = make([]MCPServer, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.MaxTurns != nil { + in, out := &in.MaxTurns, &out.MaxTurns + *out = new(int) + **out = **in + } + if in.DisabledToolsets != nil { + in, out := &in.DisabledToolsets, &out.DisabledToolsets + *out = make([]string, len(*in)) + copy(*out, *in) + } out.Wallet = in.Wallet } @@ -253,6 +270,38 @@ func (in *Condition) DeepCopy() *Condition { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPServer) DeepCopyInto(out *MCPServer) { + *out = *in + if in.Args != nil { + in, out := &in.Args, &out.Args + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(int) + **out = **in + } + if in.Env != nil { + in, out := &in.Env, &out.Env + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPServer. +func (in *MCPServer) DeepCopy() *MCPServer { + if in == nil { + return nil + } + out := new(MCPServer) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PurchaseAutoRefill) DeepCopyInto(out *PurchaseAutoRefill) { *out = *in diff --git a/internal/serviceoffercontroller/agent_render.go b/internal/serviceoffercontroller/agent_render.go index 4e030929..1ab3dce6 100644 --- a/internal/serviceoffercontroller/agent_render.go +++ b/internal/serviceoffercontroller/agent_render.go @@ -5,6 +5,8 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "sort" + "strings" "github.com/ObolNetwork/obol-stack/internal/monetizeapi" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -71,7 +73,7 @@ func agentManifests(agent *monetizeapi.Agent, litellmKey, apiKey string) ([]*uns return nil, fmt.Errorf("agentManifests: agent has no resolved model") } - configYAML := renderHermesConfig(model, litellmKey) + configYAML := renderHermesConfig(agent, litellmKey) out := []*unstructured.Unstructured{ buildAgentNamespace(agent.Namespace), @@ -92,6 +94,10 @@ func agentManifests(agent *monetizeapi.Agent, litellmKey, apiKey string) ([]*uns // expects, matching the master agent's known-good shape from // internal/hermes.generateConfig. // +// Optional AgentSpec fields (ModelProvider, MaxTurns, DisabledToolsets, +// MCPServers) override the historical defaults when set. When all are +// unset, output is byte-identical to the pre-field template. +// // Sub-agent constraints: every Agent CR is a sub-agent-for-sale (the // master is deployed via `obol agent init`, not via ServiceOffer), so the // terminal/agent caps below apply unconditionally. The Cloudflare free @@ -117,30 +123,91 @@ func agentManifests(agent *monetizeapi.Agent, litellmKey, apiKey string) ([]*uns // agent-isolation NetworkPolicy (cluster-closed, cloud-IMDS blocked). Quote // "off" so the YAML parser keeps it the string "off" and never folds it to // the boolean false. -func renderHermesConfig(model, litellmKey string) string { - return fmt.Sprintf(`model: +func renderHermesConfig(agent *monetizeapi.Agent, litellmKey string) string { + model := agent.EffectiveModel() + var b strings.Builder + + // Model block: empty/"custom" => cluster LiteLLM path (historical default). + // Any other provider omits base_url/api_key (credentials resolve in-pod). + provider := agent.Spec.ModelProvider + if provider == "" || provider == "custom" { + fmt.Fprintf(&b, `model: default: %q provider: custom base_url: http://litellm.llm.svc.cluster.local:4000/v1 api_key: %q -terminal: +`, model, litellmKey) + } else { + fmt.Fprintf(&b, `model: + default: %q + provider: %s +`, model, provider) + } + + maxTurns := 30 + if agent.Spec.MaxTurns != nil { + maxTurns = *agent.Spec.MaxTurns + } + + disabled := agent.Spec.DisabledToolsets + if disabled == nil { + disabled = []string{"memory", "web"} + } + + fmt.Fprintf(&b, `terminal: backend: local cwd: /data/.hermes/workspace timeout: 80 lifetime_seconds: 90 docker_mount_cwd_to_workspace: false agent: - max_turns: 30 + max_turns: %d reasoning_effort: low disabled_toolsets: - - memory - - web -approvals: +`, maxTurns) + for _, ts := range disabled { + fmt.Fprintf(&b, " - %s\n", ts) + } + b.WriteString(`approvals: mode: "off" skills: external_dirs: - /data/.hermes/obol-skills -`, model, litellmKey) +`) + + // mcp_servers only when the operator listed servers; empty => omit entirely + // so existing agents stay byte-identical to the pre-field template. + if len(agent.Spec.MCPServers) > 0 { + b.WriteString("mcp_servers:\n") + for _, srv := range agent.Spec.MCPServers { + fmt.Fprintf(&b, " %s:\n", srv.Name) + fmt.Fprintf(&b, " command: %s\n", srv.Command) + if len(srv.Args) > 0 { + b.WriteString(" args:\n") + for _, arg := range srv.Args { + fmt.Fprintf(&b, " - %s\n", arg) + } + } + if srv.Timeout != nil { + fmt.Fprintf(&b, " timeout: %d\n", *srv.Timeout) + } + if len(srv.Env) > 0 { + b.WriteString(" env:\n") + keys := make([]string, 0, len(srv.Env)) + for k := range srv.Env { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + // Emit env values verbatim — do not expand ${VAR}; Hermes + // resolves interpolation in-pod at runtime. + fmt.Fprintf(&b, " %s: %s\n", k, srv.Env[k]) + } + } + } + } + + return b.String() } func buildAgentNamespace(ns string) *unstructured.Unstructured { diff --git a/internal/serviceoffercontroller/agent_render_test.go b/internal/serviceoffercontroller/agent_render_test.go index 91fce353..3786769f 100644 --- a/internal/serviceoffercontroller/agent_render_test.go +++ b/internal/serviceoffercontroller/agent_render_test.go @@ -344,8 +344,47 @@ func agentConfigChecksum(t *testing.T, agent *monetizeapi.Agent) string { return "" } +// preChangeHermesConfigGolden is the exact byte output of the historical +// renderHermesConfig(model, litellmKey) template for model=qwen3.5:9b and +// litellmKey=lit-key, before AgentSpec gained ModelProvider/MaxTurns/ +// DisabledToolsets/MCPServers. When those fields are unset, output MUST +// remain byte-identical to this golden string. +const preChangeHermesConfigGolden = `model: + default: "qwen3.5:9b" + provider: custom + base_url: http://litellm.llm.svc.cluster.local:4000/v1 + api_key: "lit-key" +terminal: + backend: local + cwd: /data/.hermes/workspace + timeout: 80 + lifetime_seconds: 90 + docker_mount_cwd_to_workspace: false +agent: + max_turns: 30 + reasoning_effort: low + disabled_toolsets: + - memory + - web +approvals: + mode: "off" +skills: + external_dirs: + - /data/.hermes/obol-skills +` + +func testAgentForHermesConfig(model string) *monetizeapi.Agent { + a := &monetizeapi.Agent{} + a.Name = "quant" + a.Namespace = "agent-quant" + a.Spec.Model = model + return a +} + +func ptrInt(n int) *int { return &n } + func TestRenderHermesConfig_HasModelAndSkillsDir(t *testing.T) { - cfg := renderHermesConfig("qwen3.5:9b", "lit-key") + cfg := renderHermesConfig(testAgentForHermesConfig("qwen3.5:9b"), "lit-key") for _, must := range []string{ `default: "qwen3.5:9b"`, `api_key: "lit-key"`, @@ -363,7 +402,7 @@ func TestRenderHermesConfig_HasModelAndSkillsDir(t *testing.T) { // knobs so a single sale stays inside the 100s Cloudflare free-tunnel // window. If any of these drift it should fail loudly. func TestRenderHermesConfig_SubAgentConstraints(t *testing.T) { - cfg := renderHermesConfig("qwen3.5:9b", "lit-key") + cfg := renderHermesConfig(testAgentForHermesConfig("qwen3.5:9b"), "lit-key") for _, must := range []string{ `timeout: 80`, `lifetime_seconds: 90`, @@ -395,6 +434,60 @@ func TestRenderHermesConfig_SubAgentConstraints(t *testing.T) { } } +// TestRenderHermesConfig_UnsetFieldsByteIdentical proves that with all new +// AgentSpec fields at zero value, rendered config matches the pre-change +// template exactly (no accidental whitespace/quoting/order drift). +func TestRenderHermesConfig_UnsetFieldsByteIdentical(t *testing.T) { + agent := testAgentForHermesConfig("qwen3.5:9b") + // Explicit zero values — defensive against accidental defaults. + agent.Spec.ModelProvider = "" + agent.Spec.MCPServers = nil + agent.Spec.MaxTurns = nil + agent.Spec.DisabledToolsets = nil + + got := renderHermesConfig(agent, "lit-key") + if got != preChangeHermesConfigGolden { + t.Errorf("unset-fields config not byte-identical to pre-change golden\n--- got ---\n%q\n--- want ---\n%q", got, preChangeHermesConfigGolden) + } +} + +// TestRenderHermesConfig_OptionalSpecOverrides covers non-default +// ModelProvider, MaxTurns, and MCPServers (incl. unexpanded ${VAR} env). +func TestRenderHermesConfig_OptionalSpecOverrides(t *testing.T) { + agent := testAgentForHermesConfig("grok-4") + agent.Spec.ModelProvider = "xai-oauth" + agent.Spec.MaxTurns = ptrInt(300) + agent.Spec.MCPServers = []monetizeapi.MCPServer{ + { + Name: "search", + Command: "npx", + Args: []string{"-y", "some-mcp"}, + Env: map[string]string{"API_KEY": "${SEARCH_API_KEY}"}, + }, + } + + got := renderHermesConfig(agent, "unused-litellm-key") + + for _, must := range []string{ + `provider: xai-oauth`, + `max_turns: 300`, + `mcp_servers:`, + `search:`, + `command: npx`, + `${SEARCH_API_KEY}`, + } { + if !strings.Contains(got, must) { + t.Errorf("config missing %q\n---\n%s", must, got) + } + } + if strings.Contains(got, "litellm") { + t.Errorf("xai-oauth provider must omit litellm base_url/api_key; got:\n%s", got) + } + if strings.Contains(got, "unused-litellm-key") { + t.Errorf("non-custom provider must not emit api_key; got:\n%s", got) + } +} + // parseTerminalInt extracts the integer value of a `key: ` line from the // rendered Hermes config. Fails the test if the key is absent or unparsable. func parseTerminalInt(t *testing.T, cfg, key string) int { From 9a347d42cb064289fd87736c22fcf0f999621adc Mon Sep 17 00:00:00 2001 From: bussyjd Date: Fri, 10 Jul 2026 15:14:55 +0400 Subject: [PATCH 2/2] fix(serviceoffer-controller): hash-skip hermes-config apply + ConfigDrift condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gate the per-agent hermes-config ConfigMap write on a content-hash annotation (obol.org/hermes-config-hash) stored on the live ConfigMap. Skip the apply when the freshly-rendered desired hash matches the stored annotation — so an unchanged config is not rewritten on every reconcile or controller restart. This ends the every-'obol stack up' re-provision that silently wiped operator config (now CR-driven after b7b1bf4). The skip decision is purely desiredHash == storedAnnotation, never desired-vs-live-content, so operator edits are not treated as drift to revert. Out-of-band ConfigMap edits are surfaced via a new ConfigDrift status condition instead of being clobbered. Annotation lives on the persistent ConfigMap so the skip survives controller-pod restarts. Claude-Session: https://claude.ai/code/session_01VquWN9UMaSHH7MHGcG8bw1 --- internal/serviceoffercontroller/agent.go | 87 ++++++ .../agent_confighash_test.go | 277 ++++++++++++++++++ .../serviceoffercontroller/agent_render.go | 6 + 3 files changed, 370 insertions(+) create mode 100644 internal/serviceoffercontroller/agent_confighash_test.go diff --git a/internal/serviceoffercontroller/agent.go b/internal/serviceoffercontroller/agent.go index 24ae79ca..ef8719e2 100644 --- a/internal/serviceoffercontroller/agent.go +++ b/internal/serviceoffercontroller/agent.go @@ -2,6 +2,7 @@ package serviceoffercontroller import ( "context" + "crypto/sha256" "encoding/base64" "fmt" "log" @@ -33,6 +34,19 @@ const ( // True only when both Validated and Provisioned are True. agentConditionReady = "Ready" + // agentConditionConfigDrift reports whether the live hermes-config + // ConfigMap was edited out-of-band relative to the hash the + // controller stamped on its last write. True only when reconcile + // skips the ConfigMap apply (desired hash still matches the stored + // annotation) but live data no longer hashes to that annotation. + agentConditionConfigDrift = "ConfigDrift" + + // hermesConfigHashAnnotation is stamped on hermes-config ConfigMaps + // with the sha256 hex of data["config.yaml"] at the controller's last + // write. Survives controller-pod restarts so provisionAgent can skip + // rewrites when the CR-rendered desired config is unchanged. + hermesConfigHashAnnotation = "obol.org/hermes-config-hash" + // agentFinalizer keeps the CR around until the controller has had a // chance to tear down per-agent resources (Deployments, PVCs, // Secrets, etc.) ahead of the namespace deletion that K8s' GC would @@ -295,6 +309,11 @@ func (c *Controller) updateAgentStatus(ctx context.Context, raw *unstructured.Un // shared cluster Secret so sub-agents can route inference. Resources // are server-side-applied with the controller's field manager so // repeated reconciles converge instead of fighting each other. +// +// hermes-config is special-cased: when the live ConfigMap already carries +// an annotation equal to the freshly-rendered desired hash, the apply is +// skipped so operator out-of-band edits are not clobbered every reconcile. +// Out-of-band drift is reported via the ConfigDrift condition instead. func (c *Controller) provisionAgent(ctx context.Context, agent *monetizeapi.Agent, status *monetizeapi.AgentStatus) error { apiKey, err := c.ensureAgentAPIKey(ctx, agent) if err != nil { @@ -315,6 +334,12 @@ func (c *Controller) provisionAgent(ctx context.Context, agent *monetizeapi.Agen } for _, m := range manifests { + if m.GetKind() == "ConfigMap" && m.GetName() == hermesConfigMap { + if err := c.applyOrSkipHermesConfig(ctx, agent, status, m); err != nil { + return err + } + continue + } if err := c.applyAgentObject(ctx, c.resourceFor(m), m); err != nil { return fmt.Errorf("apply %s/%s %s: %w", m.GetKind(), m.GetName(), m.GetNamespace(), err) } @@ -322,6 +347,68 @@ func (c *Controller) provisionAgent(ctx context.Context, agent *monetizeapi.Agen return nil } +// applyOrSkipHermesConfig gates the hermes-config ConfigMap write on the +// content hash stamped as hermesConfigHashAnnotation on the live object. +// When desired hash equals the stored annotation, the apply is skipped so +// controller restarts and no-op reconciles do not rewrite the ConfigMap. +// Drift (live data no longer matching the stored annotation) is surfaced +// on status rather than clobbered. +func (c *Controller) applyOrSkipHermesConfig(ctx context.Context, agent *monetizeapi.Agent, status *monetizeapi.AgentStatus, desired *unstructured.Unstructured) error { + configYAML, _, _ := unstructured.NestedString(desired.Object, "data", "config.yaml") + desiredHash := fmt.Sprintf("%x", sha256.Sum256([]byte(configYAML))) + + live, err := c.configMaps.Namespace(agent.Namespace).Get(ctx, hermesConfigMap, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + live = nil + err = nil + } + if err != nil { + return fmt.Errorf("get hermes-config: %w", err) + } + + skipApply, drift := hermesConfigDecision(live, desiredHash) + if skipApply { + if drift { + setAgentCondition(status, agentConditionConfigDrift, "True", "OutOfBandEdit", + "live hermes-config content differs from what the controller last wrote") + } else { + setAgentCondition(status, agentConditionConfigDrift, "False", "InSync", + "live hermes-config content matches controller-written hash") + } + return nil + } + + if err := c.applyAgentObject(ctx, c.resourceFor(desired), desired); err != nil { + return fmt.Errorf("apply %s/%s %s: %w", desired.GetKind(), desired.GetName(), desired.GetNamespace(), err) + } + setAgentCondition(status, agentConditionConfigDrift, "False", "InSync", + "live hermes-config content matches controller-written hash") + return nil +} + +// hermesConfigDecision decides whether provisionAgent should skip applying +// the hermes-config ConfigMap and whether live content has drifted out-of-band. +// +// live == nil means ConfigMap not found (Get returned apierrors.IsNotFound). +// skipApply is true only when desiredHash equals the hash annotation the +// controller stamped on its last write — never by comparing desired content +// against live content directly (that would treat deliberate operator edits +// as "drift to revert" and reintroduce every-reconcile wipes). +// drift is only meaningful when skipApply is true; when skipApply is false +// this function always returns drift=false. +func hermesConfigDecision(live *unstructured.Unstructured, desiredHash string) (skipApply, drift bool) { + if live == nil { + return false, false + } + stored, _, _ := unstructured.NestedString(live.Object, "metadata", "annotations", hermesConfigHashAnnotation) + if stored != desiredHash { + return false, false + } + liveData, _, _ := unstructured.NestedString(live.Object, "data", "config.yaml") + liveHash := fmt.Sprintf("%x", sha256.Sum256([]byte(liveData))) + return true, liveHash != desiredHash +} + // hasFinalizer reports whether the given raw CR carries the named // finalizer. Mirrors the slices.Contains pattern the offer reconciler // uses, but in unstructured-land where each finalizer is an `any`. diff --git a/internal/serviceoffercontroller/agent_confighash_test.go b/internal/serviceoffercontroller/agent_confighash_test.go new file mode 100644 index 00000000..62029243 --- /dev/null +++ b/internal/serviceoffercontroller/agent_confighash_test.go @@ -0,0 +1,277 @@ +package serviceoffercontroller + +import ( + "context" + "crypto/sha256" + "fmt" + "testing" + + "github.com/ObolNetwork/obol-stack/internal/monetizeapi" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestHermesConfigDecision(t *testing.T) { + desiredYAML := "model: qwen\n" + desiredHash := fmt.Sprintf("%x", sha256.Sum256([]byte(desiredYAML))) + otherYAML := "model: other\n" + otherHash := fmt.Sprintf("%x", sha256.Sum256([]byte(otherYAML))) + + cases := []struct { + name string + live *unstructured.Unstructured + wantSkip bool + wantDrift bool + }{ + { + name: "live nil (not found) -> first create", + live: nil, + wantSkip: false, + wantDrift: false, + }, + { + name: "annotation matches and data hashes to desired -> skip, no drift", + live: hermesConfigCM(t, "ns", desiredYAML, desiredHash, "1"), + wantSkip: true, + wantDrift: false, + }, + { + name: "annotation matches but data hand-edited -> skip with drift", + live: hermesConfigCM(t, "ns", otherYAML, desiredHash, "1"), + wantSkip: true, + wantDrift: true, + }, + { + name: "annotation does not match desired -> apply", + live: hermesConfigCM(t, "ns", desiredYAML, otherHash, "1"), + wantSkip: false, + wantDrift: false, + }, + { + name: "annotation absent (pre-feature migration) -> apply", + live: hermesConfigCM(t, "ns", desiredYAML, "", "1"), + wantSkip: false, + wantDrift: false, + }, + { + name: "annotation matches but data key missing -> skip with drift", + live: hermesConfigCMNoData(t, "ns", desiredHash, "1"), + wantSkip: true, + wantDrift: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + skip, drift := hermesConfigDecision(tc.live, desiredHash) + if skip != tc.wantSkip || drift != tc.wantDrift { + t.Fatalf("hermesConfigDecision = (%v, %v), want (%v, %v)", + skip, drift, tc.wantSkip, tc.wantDrift) + } + }) + } +} + +func TestReconcileAgent_HermesConfig_SkipWhenHashMatches(t *testing.T) { + agent := testAgentForConfigHash("quant", "agent-quant") + litellmKey := "test-master-key" + configYAML := renderHermesConfig(agent, litellmKey) + desiredHash := fmt.Sprintf("%x", sha256.Sum256([]byte(configYAML))) + + const seedRV = "42" + seeded := hermesConfigCM(t, agent.Namespace, configYAML, desiredHash, seedRV) + // Pre-label so ownership checks (if any) see controller management. + seeded.SetLabels(agentLabels(agent.Name)) + + c := newProvisioningTestController(t, agent, litellmSecretObject(t, litellmKey), seeded) + + if err := c.reconcileAgent(context.Background(), "agent-quant/quant"); err != nil { + t.Fatalf("reconcileAgent (finalizer): %v", err) + } + if err := c.reconcileAgent(context.Background(), "agent-quant/quant"); err != nil { + t.Fatalf("reconcileAgent (provision): %v", err) + } + + live, err := c.configMaps.Namespace(agent.Namespace).Get(context.Background(), hermesConfigMap, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get hermes-config: %v", err) + } + if got := live.GetResourceVersion(); got != seedRV { + t.Errorf("resourceVersion = %q, want %q (ConfigMap must not be rewritten)", got, seedRV) + } + liveYAML, _, _ := unstructured.NestedString(live.Object, "data", "config.yaml") + if liveYAML != configYAML { + t.Errorf("config.yaml rewritten; got %q", liveYAML) + } + + got := getAgent(t, c, agent.Namespace, agent.Name) + cond := agentCondition(t, got, agentConditionConfigDrift) + if cond.Status != "False" || cond.Reason != "InSync" { + t.Errorf("ConfigDrift = %+v, want False/InSync", cond) + } +} + +func TestReconcileAgent_HermesConfig_AppliesWhenAnnotationStaleOrAbsent(t *testing.T) { + for _, tc := range []struct { + name string + annotation string // empty = absent + }{ + {name: "stale annotation", annotation: "deadbeef"}, + {name: "annotation absent", annotation: ""}, + } { + t.Run(tc.name, func(t *testing.T) { + agent := testAgentForConfigHash("quant", "agent-quant") + litellmKey := "test-master-key" + configYAML := renderHermesConfig(agent, litellmKey) + desiredHash := fmt.Sprintf("%x", sha256.Sum256([]byte(configYAML))) + + // Seed outdated content so a successful apply is observable. + staleContent := "model: stale-placeholder\n" + seeded := hermesConfigCM(t, agent.Namespace, staleContent, tc.annotation, "7") + seeded.SetLabels(agentLabels(agent.Name)) + + c := newProvisioningTestController(t, agent, litellmSecretObject(t, litellmKey), seeded) + + if err := c.reconcileAgent(context.Background(), "agent-quant/quant"); err != nil { + t.Fatalf("reconcileAgent (finalizer): %v", err) + } + if err := c.reconcileAgent(context.Background(), "agent-quant/quant"); err != nil { + t.Fatalf("reconcileAgent (provision): %v", err) + } + + live, err := c.configMaps.Namespace(agent.Namespace).Get(context.Background(), hermesConfigMap, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get hermes-config: %v", err) + } + gotHash, _, _ := unstructured.NestedString(live.Object, "metadata", "annotations", hermesConfigHashAnnotation) + if gotHash != desiredHash { + t.Errorf("stamped hash = %q, want %q", gotHash, desiredHash) + } + liveYAML, _, _ := unstructured.NestedString(live.Object, "data", "config.yaml") + if liveYAML != configYAML { + t.Errorf("config.yaml not updated to desired render") + } + + got := getAgent(t, c, agent.Namespace, agent.Name) + cond := agentCondition(t, got, agentConditionConfigDrift) + if cond.Status != "False" || cond.Reason != "InSync" { + t.Errorf("ConfigDrift = %+v, want False/InSync after apply", cond) + } + }) + } +} + +func TestReconcileAgent_HermesConfig_DriftDoesNotClobber(t *testing.T) { + agent := testAgentForConfigHash("quant", "agent-quant") + litellmKey := "test-master-key" + configYAML := renderHermesConfig(agent, litellmKey) + desiredHash := fmt.Sprintf("%x", sha256.Sum256([]byte(configYAML))) + + handEdited := "# operator edit\n" + configYAML + "\nextra: true\n" + const seedRV = "99" + seeded := hermesConfigCM(t, agent.Namespace, handEdited, desiredHash, seedRV) + seeded.SetLabels(agentLabels(agent.Name)) + + c := newProvisioningTestController(t, agent, litellmSecretObject(t, litellmKey), seeded) + + if err := c.reconcileAgent(context.Background(), "agent-quant/quant"); err != nil { + t.Fatalf("reconcileAgent (finalizer): %v", err) + } + if err := c.reconcileAgent(context.Background(), "agent-quant/quant"); err != nil { + t.Fatalf("reconcileAgent (provision): %v", err) + } + + live, err := c.configMaps.Namespace(agent.Namespace).Get(context.Background(), hermesConfigMap, metav1.GetOptions{}) + if err != nil { + t.Fatalf("get hermes-config: %v", err) + } + if got := live.GetResourceVersion(); got != seedRV { + t.Errorf("resourceVersion = %q, want %q (must not overwrite out-of-band edit)", got, seedRV) + } + liveYAML, _, _ := unstructured.NestedString(live.Object, "data", "config.yaml") + if liveYAML != handEdited { + t.Errorf("config.yaml was overwritten; want hand-edited content preserved") + } + + got := getAgent(t, c, agent.Namespace, agent.Name) + cond := agentCondition(t, got, agentConditionConfigDrift) + if cond.Status != "True" || cond.Reason != "OutOfBandEdit" { + t.Errorf("ConfigDrift = %+v, want True/OutOfBandEdit", cond) + } +} + +func TestBuildAgentConfigMap_StampsHashAnnotation(t *testing.T) { + agent := testAgentForConfigHash("quant", "agent-quant") + configYAML := "model: test\n" + wantHash := fmt.Sprintf("%x", sha256.Sum256([]byte(configYAML))) + + cm := buildAgentConfigMap(agent, configYAML) + got, _, _ := unstructured.NestedString(cm.Object, "metadata", "annotations", hermesConfigHashAnnotation) + if got != wantHash { + t.Errorf("hash annotation = %q, want %q", got, wantHash) + } +} + +// ─── helpers ──────────────────────────────────────────────────────────────── + +func testAgentForConfigHash(name, namespace string) *monetizeapi.Agent { + return &monetizeapi.Agent{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "obol.org/v1alpha1", + Kind: "Agent", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Generation: 1, + }, + Spec: monetizeapi.AgentSpec{ + Runtime: "hermes", + Model: "qwen3.5:9b", + Skills: []string{"addresses"}, + }, + } +} + +func hermesConfigCM(t *testing.T, namespace, configYAML, hashAnnotation, resourceVersion string) *unstructured.Unstructured { + t.Helper() + meta := map[string]any{ + "name": hermesConfigMap, + "namespace": namespace, + "resourceVersion": resourceVersion, + } + if hashAnnotation != "" { + meta["annotations"] = map[string]any{ + hermesConfigHashAnnotation: hashAnnotation, + } + } + u := &unstructured.Unstructured{} + u.SetUnstructuredContent(map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": meta, + "data": map[string]any{ + "config.yaml": configYAML, + }, + }) + return u +} + +func hermesConfigCMNoData(t *testing.T, namespace, hashAnnotation, resourceVersion string) *unstructured.Unstructured { + t.Helper() + u := &unstructured.Unstructured{} + u.SetUnstructuredContent(map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": hermesConfigMap, + "namespace": namespace, + "resourceVersion": resourceVersion, + "annotations": map[string]any{ + hermesConfigHashAnnotation: hashAnnotation, + }, + }, + "data": map[string]any{}, + }) + return u +} diff --git a/internal/serviceoffercontroller/agent_render.go b/internal/serviceoffercontroller/agent_render.go index 1ab3dce6..31c9b4fa 100644 --- a/internal/serviceoffercontroller/agent_render.go +++ b/internal/serviceoffercontroller/agent_render.go @@ -262,6 +262,9 @@ func buildAgentDataPVC(agent *monetizeapi.Agent) *unstructured.Unstructured { } func buildAgentConfigMap(agent *monetizeapi.Agent, configYAML string) *unstructured.Unstructured { + // Stamp the same sha256 hex used for Deployment's checksum/hermes-config + // annotation so provisionAgent can skip rewrites when desired is unchanged. + configHash := fmt.Sprintf("%x", sha256.Sum256([]byte(configYAML))) u := &unstructured.Unstructured{} u.SetUnstructuredContent(map[string]any{ "apiVersion": "v1", @@ -270,6 +273,9 @@ func buildAgentConfigMap(agent *monetizeapi.Agent, configYAML string) *unstructu "name": hermesConfigMap, "namespace": agent.Namespace, "labels": asAnyMap(agentLabels(agent.Name)), + "annotations": map[string]any{ + hermesConfigHashAnnotation: configHash, + }, }, "data": map[string]any{"config.yaml": configYAML}, })