diff --git a/authbridge/authlib/pipeline/schema.go b/authbridge/authlib/pipeline/schema.go new file mode 100644 index 000000000..b00218970 --- /dev/null +++ b/authbridge/authlib/pipeline/schema.go @@ -0,0 +1,199 @@ +// Schema introspection for plugin config structs. +// +// Plugins decode their config from JSON via Configurable.Configure +// (see configurable.go). The framework has long known JSON field +// names (`json:"foo_bar"` tags) but nothing else about each field. +// Tools that want to render config UIs, generate templates, or +// publish JSON Schema have had to read source. +// +// SchemaOf walks a config struct via reflection and emits one +// FieldSchema per JSON-tagged field. Plugin authors annotate fields +// with three additional tags: +// +// required:"true" // boot fails if empty/zero +// description:"prose, one line" // shown in templates / hover +// default:"5000" // documented default (cosmetic) +// enum:"a,b,c" // allowed values +// +// All four tags are optional. Absence of any tag means "no +// metadata" — the field appears in the schema but with empty fields. +// +// This package owns the shape; presentation (template YAML, hover +// formatting, JSON Schema) lives in the consumer (abctl, kagenti UI, +// future generators). + +package pipeline + +import ( + "reflect" + "strings" +) + +// SchemaProvider is implemented by plugins whose config field +// metadata should appear in the catalog and downstream tooling +// (abctl edit templates, future kagenti-UI forms, JSON Schema +// generators). Plugins without configs (a2a-parser, +// inference-parser today) can omit this interface. +// +// The convention is a one-line method that delegates to SchemaOf: +// +// func (p *MyPlugin) ConfigSchema() []FieldSchema { +// return SchemaOf(myPluginConfig{}) +// } +// +// The framework's catalog adapter type-asserts against this +// interface; absence is silently treated as "no field metadata." +type SchemaProvider interface { + ConfigSchema() []FieldSchema +} + +// FieldSchema describes one config field's metadata for tooling. +type FieldSchema struct { + // Name is the JSON key (snake_case) — what operators type in YAML. + Name string `json:"name"` + + // Type is a coarse-grained category sufficient to render templates + // and pick value placeholders. One of: + // "string", "int", "bool", "[]string", "object", "unknown". + // "object" indicates a nested struct whose fields populate Fields. + // "unknown" covers shapes the helper hasn't been taught (maps, + // slice-of-struct, etc.); the field still renders but without a + // type-specific placeholder. + Type string `json:"type"` + + // Required reports the `required:"true"` tag. Boot semantics are + // the plugin's own concern — this field is just metadata. + Required bool `json:"required,omitempty"` + + // Description is the `description:"..."` tag verbatim. Single-line. + Description string `json:"description,omitempty"` + + // Default is the `default:"..."` tag verbatim. Cosmetic — the + // authoritative default is whatever applyDefaults sets at runtime. + Default string `json:"default,omitempty"` + + // Enum is the `enum:"a,b,c"` tag split on commas. Empty when the + // field is not enum-shaped. + Enum []string `json:"enum,omitempty"` + + // Fields is populated when Type is "object" (nested struct). The + // outer field's Description applies to the struct as a whole; the + // nested fields each carry their own metadata. + Fields []FieldSchema `json:"fields,omitempty"` +} + +// SchemaOf walks the given struct value and returns its field schemas. +// Pass a zero value (e.g. `SchemaOf(ibacConfig{})`) — the value is +// inspected for type only, not for runtime field values. +// +// Returns nil if the argument isn't a struct (or pointer to struct). +// Fields without a `json:` tag are skipped (including untagged +// anonymous/embedded structs — unlike encoding/json, which promotes +// them). Explicit JSON tagging is the existing wire convention, and +// untagged fields don't appear in the operator-facing YAML, so they +// have nothing to surface in the schema either. No current plugin +// uses untagged embedding; if one ever needs that, this helper would +// need to grow flattening support. +func SchemaOf(configType any) []FieldSchema { + t := reflect.TypeOf(configType) + if t == nil { + return nil + } + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + if t.Kind() != reflect.Struct { + return nil + } + return schemaOfType(t, 0) +} + +// maxSchemaDepth caps recursion in schemaOfType. Plugin configs in +// practice nest one or two levels (tokenexchange's identity + routes +// blocks are the deepest today); the cap is defensive against a +// future config struct with an inadvertent self-referential field +// (e.g. a *Self pointer) which would otherwise stack-overflow. +const maxSchemaDepth = 10 + +func schemaOfType(t reflect.Type, depth int) []FieldSchema { + if depth > maxSchemaDepth { + return nil + } + var out []FieldSchema + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !f.IsExported() { + continue + } + jsonTag := f.Tag.Get("json") + if jsonTag == "" || jsonTag == "-" { + continue + } + name := strings.SplitN(jsonTag, ",", 2)[0] + if name == "" || name == "-" { + continue + } + schema := FieldSchema{ + Name: name, + Type: kindOf(f.Type), + Required: f.Tag.Get("required") == "true", + Description: f.Tag.Get("description"), + Default: f.Tag.Get("default"), + } + if enumTag := f.Tag.Get("enum"); enumTag != "" { + parts := strings.Split(enumTag, ",") + schema.Enum = make([]string, 0, len(parts)) + for _, p := range parts { + if v := strings.TrimSpace(p); v != "" { + schema.Enum = append(schema.Enum, v) + } + } + } + if schema.Type == "object" { + // Recurse with a depth bound (see maxSchemaDepth). Plugin + // configs nest one or two levels in practice; the cap + // guards against an inadvertent self-referential field + // (e.g. *Self) silently blowing the stack. + schema.Fields = schemaOfType(unwrap(f.Type), depth+1) + } + out = append(out, schema) + } + return out +} + +// kindOf returns the coarse-grained Type tag for a field. +func kindOf(t reflect.Type) string { + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + switch t.Kind() { + case reflect.String: + return "string" + case reflect.Bool: + return "bool" + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return "int" + case reflect.Slice: + // Only []string gets a typed tag; other slices are "unknown" + // (slice-of-struct, slice-of-map, etc. are rare in plugin + // configs and don't render to a single YAML scalar template). + if t.Elem().Kind() == reflect.String { + return "[]string" + } + return "unknown" + case reflect.Struct: + return "object" + default: + return "unknown" + } +} + +// unwrap returns the underlying struct type for a struct or *struct. +// Returns t unchanged if it's not a struct shape. +func unwrap(t reflect.Type) reflect.Type { + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + return t +} diff --git a/authbridge/authlib/pipeline/schema_test.go b/authbridge/authlib/pipeline/schema_test.go new file mode 100644 index 000000000..c58576672 --- /dev/null +++ b/authbridge/authlib/pipeline/schema_test.go @@ -0,0 +1,169 @@ +package pipeline + +import ( + "reflect" + "testing" +) + +// Tests below use representative shapes covering every Type the +// helper recognizes plus the no-tag and pointer-to-struct cases. +// They lock in the contract that consumers (abctl template +// renderer, JSON-Schema generator) build on. + +type primitives struct { + Hostname string `json:"hostname" required:"true" description:"Target hostname."` + Port int `json:"port" description:"TCP port." default:"8080"` + Verbose bool `json:"verbose"` + Mode string `json:"mode" enum:"strict, lenient, off"` +} + +func TestSchemaOf_Primitives(t *testing.T) { + got := SchemaOf(primitives{}) + want := []FieldSchema{ + {Name: "hostname", Type: "string", Required: true, Description: "Target hostname."}, + {Name: "port", Type: "int", Description: "TCP port.", Default: "8080"}, + {Name: "verbose", Type: "bool"}, + {Name: "mode", Type: "string", Enum: []string{"strict", "lenient", "off"}}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("SchemaOf primitives:\n got: %+v\nwant: %+v", got, want) + } +} + +type withList struct { + Allowed []string `json:"allowed" description:"Allowlisted hosts."` +} + +func TestSchemaOf_StringSlice(t *testing.T) { + got := SchemaOf(withList{}) + if len(got) != 1 || got[0].Type != "[]string" { + t.Errorf("expected one []string field, got %+v", got) + } +} + +type nestedInner struct { + Path string `json:"path" required:"true" description:"Routes file path."` + Inner bool `json:"inline"` +} + +type nestedOuter struct { + Name string `json:"name" required:"true"` + Inner nestedInner `json:"inner" description:"Nested config block."` +} + +func TestSchemaOf_NestedStruct(t *testing.T) { + got := SchemaOf(nestedOuter{}) + if len(got) != 2 { + t.Fatalf("expected 2 top-level fields, got %d (%+v)", len(got), got) + } + if got[1].Name != "inner" || got[1].Type != "object" { + t.Errorf("nested field shape wrong: %+v", got[1]) + } + if len(got[1].Fields) != 2 { + t.Fatalf("nested fields not populated: %+v", got[1].Fields) + } + if got[1].Fields[0].Name != "path" || !got[1].Fields[0].Required { + t.Errorf("nested[0] shape wrong: %+v", got[1].Fields[0]) + } +} + +type withSkippable struct { + JSONOmitted string `json:"-"` + NoTag string + Counted int `json:"counted"` +} + +func TestSchemaOf_SkipsUntaggedAndDashed(t *testing.T) { + // Skipping unexported fields is also part of the contract; + // we don't unit-test that case here because adding an + // unexported field with a json tag triggers `go vet`'s + // structtag check and pollutes the test file. The + // IsExported() guard in schemaOfType is straightforward. + got := SchemaOf(withSkippable{}) + if len(got) != 1 || got[0].Name != "counted" { + t.Errorf("expected only the `counted` field, got %+v", got) + } +} + +type withPointer struct { + Optional *int `json:"optional" description:"Optional override."` +} + +func TestSchemaOf_PointerField(t *testing.T) { + got := SchemaOf(withPointer{}) + if len(got) != 1 || got[0].Type != "int" { + t.Errorf("expected pointer-to-int to render as int, got %+v", got) + } +} + +func TestSchemaOf_PointerToStruct(t *testing.T) { + got := SchemaOf(&primitives{}) + if len(got) != 4 { + t.Errorf("expected SchemaOf to handle *struct, got %d fields", len(got)) + } +} + +func TestSchemaOf_NotAStruct(t *testing.T) { + if got := SchemaOf("not-a-struct"); got != nil { + t.Errorf("expected nil for non-struct, got %+v", got) + } + if got := SchemaOf(42); got != nil { + t.Errorf("expected nil for non-struct, got %+v", got) + } + if got := SchemaOf(nil); got != nil { + t.Errorf("expected nil for nil, got %+v", got) + } +} + +type withSliceOfStruct struct { + Routes []nestedInner `json:"routes" description:"List of routes."` +} + +func TestSchemaOf_SliceOfStructIsUnknown(t *testing.T) { + // We deliberately don't recurse into slice-of-struct — those + // shapes don't fit the per-line YAML-scalar template model + // abctl uses. Documented in kindOf comment. + got := SchemaOf(withSliceOfStruct{}) + if len(got) != 1 || got[0].Type != "unknown" { + t.Errorf("slice-of-struct should be \"unknown\", got %+v", got) + } +} + +// Empty-enum tag should produce nil, not [""]: the parser trims and +// drops empty parts. +func TestSchemaOf_EnumWhitespaceAndEmpty(t *testing.T) { + type withMessyEnum struct { + Mode string `json:"mode" enum:" on , , off "` + } + got := SchemaOf(withMessyEnum{}) + if len(got) != 1 || !reflect.DeepEqual(got[0].Enum, []string{"on", "off"}) { + t.Errorf("expected trimmed [on, off], got %+v", got) + } +} + +// Self-referential pointer fields should not stack-overflow — the +// depth bound in schemaOfType caps recursion. A linked-list-style +// node (Next *node) is the canonical case. +func TestSchemaOf_SelfReferentialIsBounded(t *testing.T) { + type node struct { + Name string `json:"name"` + Next *node `json:"next"` + } + // Should return without panicking and emit a finite schema. + got := SchemaOf(node{}) + if len(got) != 2 { + t.Fatalf("expected 2 top-level fields, got %d", len(got)) + } + // The recursion eventually returns nil at depth > maxSchemaDepth, + // so somewhere in the nested chain Fields is nil. Walk down and + // confirm we don't loop forever (the test itself would hang). + depth := 0 + cur := got[1] // "next" + for cur.Type == "object" && len(cur.Fields) >= 2 { + depth++ + if depth > maxSchemaDepth+5 { + t.Fatalf("recursion not bounded; reached depth %d", depth) + } + cur = cur.Fields[1] + } +} diff --git a/authbridge/authlib/plugins/ibac/plugin.go b/authbridge/authlib/plugins/ibac/plugin.go index b93109427..2a011620f 100644 --- a/authbridge/authlib/plugins/ibac/plugin.go +++ b/authbridge/authlib/plugins/ibac/plugin.go @@ -44,47 +44,29 @@ import ( // ibacConfig is the plugin's local config schema. See // authbridge/docs/plugin-reference.md for the decode → applyDefaults → // validate convention shared with jwt-validation and token-exchange. +// +// Field tags drive both runtime decoding (json) and operator-facing +// schema introspection (description / required / default / enum). +// See pipeline/schema.go for the consumer contract; descriptions are +// kept single-line — full prose lives in docs/ibac-plugin.md. type ibacConfig struct { - // JudgeEndpoint is the base URL of the LLM-judge service. The - // plugin POSTs OpenAI-compatible chat-completion requests to - // JudgeEndpoint+"/v1/chat/completions". - JudgeEndpoint string `json:"judge_endpoint"` - - // JudgeModel names the model to use for verdicts (e.g. - // "llama3.2:3b" for ollama, "gpt-4o-mini" for OpenAI). - JudgeModel string `json:"judge_model"` - - // JudgeBearer is an optional bearer token for the judge endpoint. - // Leave empty for unauthenticated local LLMs (ollama). - JudgeBearer string `json:"judge_bearer"` - - // SystemPrompt overrides the default judge system prompt. Empty - // means "use the default" (see judge.go:defaultSystemPrompt). - SystemPrompt string `json:"system_prompt"` - - // TimeoutMs bounds each judge call. Defaults to 5000. - TimeoutMs int `json:"timeout_ms"` - - // JudgeInference, when true, also judges outbound traffic where - // pctx.Extensions.Inference is populated (the agent's own LLM - // reasoning loop). Default false — judging the agent's prompts - // is high-cost low-value for typical deployments. - JudgeInference bool `json:"judge_inference"` - - // AgentLLMHost is a convenience: the host of the agent's own LLM - // endpoint. When set, it's added to the bypass-host list so the - // agent's reasoning traffic is never judged regardless of - // JudgeInference. - AgentLLMHost string `json:"agent_llm_host"` - - // BypassHosts are host globs (path.Match syntax) skipped without - // judging. Defaults include common infrastructure hostnames so - // the judge isn't called on Keycloak / OTel / agent-card hops. - BypassHosts []string `json:"bypass_hosts"` - - // BypassPaths are URL path globs skipped without judging. - // Defaults to bypass.DefaultPatterns (.well-known, healthz, etc). - BypassPaths []string `json:"bypass_paths"` + JudgeEndpoint string `json:"judge_endpoint" required:"true" description:"Base URL of the LLM-judge service. The plugin POSTs to {endpoint}/v1/chat/completions."` + + JudgeModel string `json:"judge_model" required:"true" description:"Model name passed to the judge, e.g. \"llama3.2:3b\" or \"gpt-4o-mini\"."` + + JudgeBearer string `json:"judge_bearer" description:"Optional bearer token for the judge endpoint. Empty for unauthenticated local LLMs."` + + SystemPrompt string `json:"system_prompt" description:"Override the default judge system prompt. Empty means use the built-in default."` + + TimeoutMs int `json:"timeout_ms" description:"Per-call judge timeout. Validation rejects values below 100." default:"5000"` + + JudgeInference bool `json:"judge_inference" description:"When true, also judge outbound LLM-reasoning traffic. High-cost / low-value default off." default:"false"` + + AgentLLMHost string `json:"agent_llm_host" description:"Convenience: agent's own LLM host. Added to bypass_hosts so reasoning traffic is never judged."` + + BypassHosts []string `json:"bypass_hosts" description:"Host globs (path.Match) skipped without judging. Defaults include keycloak / spire / otel."` + + BypassPaths []string `json:"bypass_paths" description:"URL path globs skipped without judging. Defaults: /.well-known/* /healthz /readyz /livez."` // NoIntentPolicy controls behavior when a request reaches step 6 // without a recorded user intent — either because Session is nil @@ -110,7 +92,7 @@ type ibacConfig struct { // Deployments that mix user-driven and self-driven traffic get // the right behavior automatically; deployments that want hard // fail-closed semantics opt in via "deny". - NoIntentPolicy string `json:"no_intent_policy"` + NoIntentPolicy string `json:"no_intent_policy" description:"Behavior when an action lacks recorded user intent. allow=skip; deny=403." default:"allow" enum:"allow,deny"` // UnclassifiedPolicy controls behavior at step 4 (the // classification gate) when no protocol parser populated any @@ -142,7 +124,15 @@ type ibacConfig struct { // arbitrary HTTP traffic isn't paid for by most operators. // The IBAC demo opts into "judge" to keep its plain-HTTP exfil // scenario operational. - UnclassifiedPolicy string `json:"unclassified_policy"` + UnclassifiedPolicy string `json:"unclassified_policy" description:"Behavior when no parser claimed the request. passthrough=skip; judge=fall through to judge." default:"passthrough" enum:"passthrough,judge"` +} + +// ConfigSchema exposes the ibacConfig fields for schema-aware tooling +// (abctl edit templates, future kagenti-UI forms, etc.). Implements +// pipeline.SchemaProvider; absence would simply make IBAC opaque to +// such tooling without affecting runtime. +func (p *IBAC) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(ibacConfig{}) } // no_intent_policy values. diff --git a/authbridge/authlib/plugins/jwtvalidation/plugin.go b/authbridge/authlib/plugins/jwtvalidation/plugin.go index f3f263de7..72c147879 100644 --- a/authbridge/authlib/plugins/jwtvalidation/plugin.go +++ b/authbridge/authlib/plugins/jwtvalidation/plugin.go @@ -22,12 +22,17 @@ import ( // jwtValidationConfig is the plugin's local config schema. See // authbridge/docs/plugin-reference.md for the decode → applyDefaults → // validate pattern. +// Field tags drive both runtime decoding (json) and operator-facing +// schema introspection (description / required / default / enum). +// Inline doc comments retain the long-form rationale; struct tags +// carry single-line summaries for templating. See pipeline/schema.go +// for the consumer contract. type jwtValidationConfig struct { // Issuer is the JWT `iss` claim expected on inbound tokens. // In split-horizon deployments this is the PUBLIC Keycloak URL // (whatever Keycloak stamps into the `iss` claim) — it only needs // to match bit-for-bit, not be reachable from inside the pod. - Issuer string `json:"issuer"` + Issuer string `json:"issuer" required:"true" description:"Expected JWT iss claim. Public Keycloak URL in split-horizon deployments."` // JWKSURL points at the JWKS endpoint used to verify signatures. // The sidecar actually GETs this URL from inside the cluster, so @@ -40,7 +45,7 @@ type jwtValidationConfig struct { // internal URL, when the operator supplies it) // 2. Issuer (fallback for single-horizon deployments where the // issuer hostname is reachable from inside the cluster) - JWKSURL string `json:"jwks_url"` + JWKSURL string `json:"jwks_url" description:"JWKS endpoint URL (internal). Derived from KeycloakURL+Realm or Issuer when empty."` // KeycloakURL and KeycloakRealm are a convenience for deriving // JWKSURL from the internal Keycloak service URL, symmetric with @@ -52,13 +57,13 @@ type jwtValidationConfig struct { // via a cross-plugin pass; per-plugin configs don't share state, // so each plugin now carries its own copy of the "where is // Keycloak internally" hint. - KeycloakURL string `json:"keycloak_url"` - KeycloakRealm string `json:"keycloak_realm"` + KeycloakURL string `json:"keycloak_url" description:"Internal Keycloak base URL. Used to derive jwks_url when omitted."` + KeycloakRealm string `json:"keycloak_realm" description:"Keycloak realm name. Pairs with keycloak_url for jwks_url derivation."` // Audience is the literal audience value expected on inbound // tokens. One of {Audience, AudienceFile, AudienceMode:"per-host"} // is required. - Audience string `json:"audience"` + Audience string `json:"audience" description:"Expected aud claim value. One of audience / audience_file / audience_mode=per-host is required."` // AudienceFile reads the expected audience from a file. Used // together with client-registration's /shared/client-id.txt. The @@ -69,12 +74,12 @@ type jwtValidationConfig struct { // will fill in /shared/client-id.txt. To opt out of any file poll, // supply an explicit Audience instead; the file default only kicks // in when both Audience and AudienceFile are empty. - AudienceFile string `json:"audience_file"` + AudienceFile string `json:"audience_file" description:"Read expected audience from this file. Default: /shared/client-id.txt when both audience fields are empty."` // AudienceMode chooses how the expected audience is resolved: // "static" (default) uses Audience/AudienceFile; "per-host" derives // it from pctx.Host via routing.ServiceNameFromHost (waypoint mode). - AudienceMode string `json:"audience_mode"` + AudienceMode string `json:"audience_mode" description:"How to resolve expected audience: static or per-host." default:"static" enum:"static,per-host"` // AllowedAudiences lists extra audience strings the sidecar accepts // for inbound JWT validation (OR semantics: the token's aud claim @@ -89,11 +94,11 @@ type jwtValidationConfig struct { // multiple audiences (RFC 7519 string or array) and the agent must // accept more than the workload client ID alone — prefer aligning // IdP audience policy long-term. See plugin-reference.md. - AllowedAudiences []string `json:"allowed_audiences"` + AllowedAudiences []string `json:"allowed_audiences" description:"Extra audience strings accepted (OR semantics)."` // BypassPaths are URL path globs (see authlib/bypass) that skip // validation entirely. - BypassPaths []string `json:"bypass_paths"` + BypassPaths []string `json:"bypass_paths" description:"Path globs (path.Match) skipped without JWT validation. Defaults: /healthz /readyz /livez /.well-known/*."` } func (c *jwtValidationConfig) applyDefaults() { @@ -215,6 +220,12 @@ func (p *JWTValidation) Capabilities() pipeline.PluginCapabilities { } } +// ConfigSchema implements pipeline.SchemaProvider; surfaces field +// metadata to abctl edit templates and other config-aware tooling. +func (p *JWTValidation) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(jwtValidationConfig{}) +} + // Configure decodes the plugin's config subtree, applies defaults, // validates, and constructs the internal auth handler. If AudienceFile // is set but the file isn't yet readable (client-registration still diff --git a/authbridge/authlib/plugins/registry.go b/authbridge/authlib/plugins/registry.go index 61532fdd0..d9421aad6 100644 --- a/authbridge/authlib/plugins/registry.go +++ b/authbridge/authlib/plugins/registry.go @@ -81,12 +81,20 @@ func RegisteredPlugins() []string { } // CatalogEntry pairs a registered plugin's name with the capabilities -// it advertises. Surfaces in `abctl`'s catalog pane and in the -// /v1/plugins endpoint so operators can see what plugins exist and -// what each one needs without reading source. +// it advertises and the field-level schema of its config (if it +// implements pipeline.SchemaProvider). Surfaces in `abctl`'s catalog +// pane, in the /v1/plugins endpoint, and in `abctl edit`'s template +// renderer so operators can see what plugins exist, what each one +// needs, and what each field means without reading source. +// +// Fields is nil for plugins without a config (or that don't +// implement SchemaProvider) — the wire format omits the field via +// `omitempty`, so existing consumers that don't know about field +// schemas keep working. type CatalogEntry struct { Name string Capabilities pipeline.PluginCapabilities + Fields []pipeline.FieldSchema } // catalogCache memoizes the Catalog() result on first call. Constructors @@ -139,10 +147,21 @@ func Catalog() []CatalogEntry { defer registryMu.RUnlock() out := make([]CatalogEntry, 0, len(registry)) for name, factory := range registry { - out = append(out, CatalogEntry{ + instance := factory() + entry := CatalogEntry{ Name: name, - Capabilities: factory().Capabilities().Normalize(), - }) + Capabilities: instance.Capabilities().Normalize(), + } + // Plugins that implement SchemaProvider expose per-field + // metadata for templating. Constraints carry over from the + // constructor contract above: the throwaway instance is never + // Shutdown'd, so SchemaProvider implementations must be + // instance-state-independent and side-effect free (typically + // just `return pipeline.SchemaOf(myConfig{})`). + if sp, ok := instance.(pipeline.SchemaProvider); ok { + entry.Fields = sp.ConfigSchema() + } + out = append(out, entry) } sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) catalogCacheVal = out @@ -175,11 +194,29 @@ func cloneCatalog(in []CatalogEntry) []CatalogEntry { After: append([]string(nil), caps.After...), Claims: append([]string(nil), caps.Claims...), }, + Fields: cloneFieldSchemas(in[i].Fields), } } return out } +// cloneFieldSchemas deep-copies the per-field schema slice. Fields +// itself is a slice, but each FieldSchema also contains slices +// (Enum, Fields for nested structs) that need their own backing +// arrays to avoid aliasing the cached snapshot. +func cloneFieldSchemas(in []pipeline.FieldSchema) []pipeline.FieldSchema { + if len(in) == 0 { + return nil + } + out := make([]pipeline.FieldSchema, len(in)) + for i, f := range in { + out[i] = f + out[i].Enum = append([]string(nil), f.Enum...) + out[i].Fields = cloneFieldSchemas(f.Fields) // recurse for nested struct fields + } + return out +} + // resetCatalogCache clears the memoized Catalog result. Intended for // tests that register/unregister plugins and need a fresh view. func resetCatalogCache() { diff --git a/authbridge/authlib/plugins/tokenbroker/plugin.go b/authbridge/authlib/plugins/tokenbroker/plugin.go index 345092b88..9a613151d 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin.go @@ -20,33 +20,37 @@ import ( ) // tokenBrokerConfig is the plugin's local config schema. +// +// Field tags drive both runtime decoding (json) and operator-facing +// schema introspection (description / required / default / enum). +// See pipeline/schema.go for the consumer contract. type tokenBrokerConfig struct { // BrokerURL is the base URL of the token broker service. - BrokerURL string `json:"broker_url"` + BrokerURL string `json:"broker_url" required:"true" description:"Base URL of the token broker service."` // DefaultPolicy is applied when a request's host matches no route: // "passthrough" (default) forwards the request unchanged; // "broker" attempts to acquire a token from the broker. - DefaultPolicy string `json:"default_policy"` + DefaultPolicy string `json:"default_policy" description:"Behavior when host matches no route." default:"passthrough" enum:"passthrough,broker"` // Routes drives host-to-broker matching. A host that matches no // route falls through to DefaultPolicy. - Routes tokenBrokerRoutes `json:"routes"` + Routes tokenBrokerRoutes `json:"routes" description:"Host-to-broker routing rules; non-matching hosts fall through to default_policy."` } type tokenBrokerRoutes struct { // File is an optional path to a routes.yaml file. - File string `json:"file"` + File string `json:"file" description:"Path to a routes.yaml file. Inline rules below are merged with file-loaded rules."` // Rules are inline route entries; combined with routes loaded from File. - Rules []tokenBrokerRoute `json:"rules"` + Rules []tokenBrokerRoute `json:"rules" description:"Inline route entries. Combined with rules loaded from file."` } type tokenBrokerRoute struct { - Host string `json:"host"` - Action string `json:"action"` // "broker" or "passthrough"; defaults to "broker" - AuthorizationEndpoint string `json:"authorization_endpoint,omitempty"` - TokenEndpoint string `json:"token_endpoint,omitempty"` + Host string `json:"host" description:"Host glob pattern."` + Action string `json:"action" description:"broker or passthrough." default:"broker" enum:"broker,passthrough"` + AuthorizationEndpoint string `json:"authorization_endpoint,omitempty" description:"Per-route OAuth authorization endpoint override."` + TokenEndpoint string `json:"token_endpoint,omitempty" description:"Per-route OAuth token endpoint override."` } func (c *tokenBrokerConfig) applyDefaults() { @@ -159,6 +163,12 @@ func (p *TokenBroker) Capabilities() pipeline.PluginCapabilities { } } +// ConfigSchema implements pipeline.SchemaProvider; surfaces field +// metadata to abctl edit templates and other config-aware tooling. +func (p *TokenBroker) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(tokenBrokerConfig{}) +} + func (p *TokenBroker) Configure(raw json.RawMessage) error { var c tokenBrokerConfig diff --git a/authbridge/authlib/plugins/tokenexchange/plugin.go b/authbridge/authlib/plugins/tokenexchange/plugin.go index 6183554df..f992ce615 100644 --- a/authbridge/authlib/plugins/tokenexchange/plugin.go +++ b/authbridge/authlib/plugins/tokenexchange/plugin.go @@ -23,24 +23,28 @@ import ( // tokenExchangeConfig is the plugin's local config schema. See // authbridge/docs/plugin-reference.md for the pattern. +// Field tags drive both runtime decoding (json) and operator-facing +// schema introspection (description / required / default / enum). +// Inline doc comments retain long-form rationale; struct tags carry +// single-line summaries for templating. See pipeline/schema.go. type tokenExchangeConfig struct { // TokenURL is the OAuth token endpoint. Explicit value wins; else // derived from KeycloakURL + KeycloakRealm using Keycloak's // convention. - TokenURL string `json:"token_url"` + TokenURL string `json:"token_url" description:"OAuth token endpoint URL. Required unless keycloak_url + keycloak_realm are both set (the plugin derives token_url from the pair)."` // KeycloakURL and KeycloakRealm are a convenience for deriving // TokenURL when the operator prefers to supply Keycloak base + realm // rather than the full token endpoint. - KeycloakURL string `json:"keycloak_url"` - KeycloakRealm string `json:"keycloak_realm"` + KeycloakURL string `json:"keycloak_url" description:"Internal Keycloak base URL. Required (with keycloak_realm) when token_url is empty."` + KeycloakRealm string `json:"keycloak_realm" description:"Keycloak realm name. Required (with keycloak_url) when token_url is empty."` // DefaultPolicy is applied when a request's host matches no route: // "passthrough" (default) forwards the request unchanged; // "exchange" attempts a client-credentials exchange with an empty // audience (usually fails — kept for rare use cases where the IdP // allows it). - DefaultPolicy string `json:"default_policy"` + DefaultPolicy string `json:"default_policy" description:"Behavior when host matches no route: passthrough forwards unchanged, exchange attempts empty-audience client-credentials." default:"passthrough" enum:"passthrough,exchange"` // NoTokenPolicy controls how the plugin handles outbound requests // that arrive without a bearer token: "client-credentials" does an @@ -48,41 +52,41 @@ type tokenExchangeConfig struct { // unchanged; "deny" rejects. Default: "deny" in all modes. // Operators who need a different behavior (e.g. waypoint's historic // "allow" default) must set it explicitly per plugin entry. - NoTokenPolicy string `json:"no_token_policy"` + NoTokenPolicy string `json:"no_token_policy" description:"Behavior when outbound has no bearer token: client-credentials, allow, or deny." default:"deny" enum:"client-credentials,allow,deny"` // Identity carries client credentials used for token exchange. - Identity tokenExchangeIdentity `json:"identity"` + Identity tokenExchangeIdentity `json:"identity" description:"Client credentials used for token exchange (spiffe or client-secret)."` // Routes drives host-to-audience matching. A host that matches no // route falls through to DefaultPolicy. - Routes tokenExchangeRoutes `json:"routes"` + Routes tokenExchangeRoutes `json:"routes" description:"Host-to-audience routing rules; non-matching hosts fall through to default_policy."` // AudienceFromHost — when true, requests with no matching route use // routing.ServiceNameFromHost(host) as the target audience. Used in // waypoint mode. - AudienceFromHost bool `json:"audience_from_host"` + AudienceFromHost bool `json:"audience_from_host" description:"When true, derive audience from host for unrouted requests (waypoint mode)." default:"false"` } type tokenExchangeIdentity struct { // Type is one of "spiffe" or "client-secret". - Type string `json:"type"` + Type string `json:"type" required:"true" description:"Identity scheme: spiffe (JWT-SVID assertion) or client-secret." enum:"spiffe,client-secret"` // ClientID identifies the client in Keycloak. Explicit value wins; // else read from ClientIDFile at Configure time (or by Init if the // file isn't yet available). - ClientID string `json:"client_id"` - ClientIDFile string `json:"client_id_file"` + ClientID string `json:"client_id" description:"Inline Keycloak client ID. One of client_id or client_id_file is required."` + ClientIDFile string `json:"client_id_file" description:"Read client ID from this file. Default: /shared/client-id.txt."` // ClientSecret / ClientSecretFile are the client-secret credentials // (type=client-secret). - ClientSecret string `json:"client_secret"` - ClientSecretFile string `json:"client_secret_file"` + ClientSecret string `json:"client_secret" description:"Inline Keycloak client secret (type=client-secret)."` + ClientSecretFile string `json:"client_secret_file" description:"Read client secret from file. Default: /shared/client-secret.txt."` // JWTAudience is the audience claim minted on the JWT-SVID used as // the RFC 8693 client assertion. Required when Type=="spiffe"; // ignored otherwise. Lives on the plugin (not the framework spiffe // block) because only the spiffe identity path consumes it. - JWTAudience string `json:"jwt_audience"` + JWTAudience string `json:"jwt_audience" description:"Audience claim minted on the JWT-SVID assertion. REQUIRED when type=spiffe; ignored otherwise."` // jwt_svid_path was historically a per-plugin path to the JWT-SVID // file written by spiffe-helper. Removed in favor of injection via @@ -94,11 +98,11 @@ type tokenExchangeIdentity struct { type tokenExchangeRoutes struct { // File is an optional path to a routes.yaml file (see // authlib/routing.LoadRoutes). - File string `json:"file"` + File string `json:"file" description:"Path to a routes.yaml file. Default: /etc/authproxy/routes.yaml."` // Rules are inline route entries; combined with routes loaded from // File. - Rules []tokenExchangeRoute `json:"rules"` + Rules []tokenExchangeRoute `json:"rules" description:"Inline route entries. Combined with rules loaded from file."` } type tokenExchangeRoute struct { @@ -278,6 +282,12 @@ func (p *TokenExchange) Capabilities() pipeline.PluginCapabilities { } } +// ConfigSchema implements pipeline.SchemaProvider; surfaces field +// metadata to abctl edit templates and other config-aware tooling. +func (p *TokenExchange) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(tokenExchangeConfig{}) +} + func (p *TokenExchange) Configure(raw json.RawMessage) error { var c tokenExchangeConfig if len(raw) > 0 { diff --git a/authbridge/authlib/sessionapi/catalog_adapter.go b/authbridge/authlib/sessionapi/catalog_adapter.go index dfb678e65..17f4d6845 100644 --- a/authbridge/authlib/sessionapi/catalog_adapter.go +++ b/authbridge/authlib/sessionapi/catalog_adapter.go @@ -1,6 +1,9 @@ package sessionapi -import "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" +import ( + "github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline" + "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" +) // PluginsCatalog adapts plugins.Catalog() into the wire-shaped // CatalogEntry slice WithCatalog expects. Three binaries (authbridge- @@ -11,6 +14,11 @@ import "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins" // most plugins can be configured into either chain (parsers especially). // abctl renders direction only for the active pipeline, where the // answer is positional, not type-level. +// +// Fields is populated for plugins that implement +// pipeline.SchemaProvider (most config-bearing plugins). Plugins +// without configs emit a nil Fields slice and the wire format +// elides it via `omitempty`. func PluginsCatalog() []CatalogEntry { src := plugins.Catalog() out := make([]CatalogEntry, len(src)) @@ -26,6 +34,30 @@ func PluginsCatalog() []CatalogEntry { After: n.After, Claims: n.Claims, Description: n.Description, + Fields: convertFieldSchemas(e.Fields), + } + } + return out +} + +// convertFieldSchemas maps the framework-level FieldSchema slice to +// the wire-level FieldSchemaEntry slice. Recurses into nested struct +// schemas. Returns nil for empty input so the JSON marshaller can +// elide the field via `omitempty`. +func convertFieldSchemas(in []pipeline.FieldSchema) []FieldSchemaEntry { + if len(in) == 0 { + return nil + } + out := make([]FieldSchemaEntry, len(in)) + for i, f := range in { + out[i] = FieldSchemaEntry{ + Name: f.Name, + Type: f.Type, + Required: f.Required, + Description: f.Description, + Default: f.Default, + Enum: append([]string(nil), f.Enum...), + Fields: convertFieldSchemas(f.Fields), } } return out diff --git a/authbridge/authlib/sessionapi/server.go b/authbridge/authlib/sessionapi/server.go index db6a2fe8f..6d18844a0 100644 --- a/authbridge/authlib/sessionapi/server.go +++ b/authbridge/authlib/sessionapi/server.go @@ -52,16 +52,31 @@ type Server struct { // that documents bodyAccess as deprecated, so there's no compat cost to // emit the right name from day one. type CatalogEntry struct { - Name string `json:"name"` - Direction string `json:"direction,omitempty"` - ReadsBody bool `json:"readsBody,omitempty"` - Writes []string `json:"writes,omitempty"` - Reads []string `json:"reads,omitempty"` - Requires []string `json:"requires,omitempty"` - RequiresAny []string `json:"requiresAny,omitempty"` - After []string `json:"after,omitempty"` - Claims []string `json:"claims,omitempty"` - Description string `json:"description,omitempty"` + Name string `json:"name"` + Direction string `json:"direction,omitempty"` + ReadsBody bool `json:"readsBody,omitempty"` + Writes []string `json:"writes,omitempty"` + Reads []string `json:"reads,omitempty"` + Requires []string `json:"requires,omitempty"` + RequiresAny []string `json:"requiresAny,omitempty"` + After []string `json:"after,omitempty"` + Claims []string `json:"claims,omitempty"` + Description string `json:"description,omitempty"` + Fields []FieldSchemaEntry `json:"fields,omitempty"` +} + +// FieldSchemaEntry is the wire shape for one config field's schema +// metadata. Mirrors pipeline.FieldSchema; lives in the sessionapi +// package so consumers (abctl apiclient, future kagenti-UI clients) +// don't have to import authlib/pipeline transitively. +type FieldSchemaEntry struct { + Name string `json:"name"` + Type string `json:"type"` + Required bool `json:"required,omitempty"` + Description string `json:"description,omitempty"` + Default string `json:"default,omitempty"` + Enum []string `json:"enum,omitempty"` + Fields []FieldSchemaEntry `json:"fields,omitempty"` } // CatalogProvider is the function the binary supplies to expose diff --git a/authbridge/authlib/sessionapi/server_test.go b/authbridge/authlib/sessionapi/server_test.go index f2ca67985..35ba20b83 100644 --- a/authbridge/authlib/sessionapi/server_test.go +++ b/authbridge/authlib/sessionapi/server_test.go @@ -718,6 +718,63 @@ func TestHandlePluginCatalog_ListsRegisteredPlugins(t *testing.T) { } } +// TestHandlePluginCatalog_IncludesFieldSchemas confirms field-level +// schemas attached by the catalog provider make it onto the wire +// (and that omitempty hides them when absent). +func TestHandlePluginCatalog_IncludesFieldSchemas(t *testing.T) { + stub := func() []CatalogEntry { + return []CatalogEntry{ + { + Name: "alpha", + Description: "Has fields", + Fields: []FieldSchemaEntry{ + {Name: "endpoint", Type: "string", Required: true, Description: "API URL."}, + {Name: "policy", Type: "string", Default: "allow", Enum: []string{"allow", "deny"}}, + }, + }, + {Name: "beta", Description: "No fields"}, + } + } + store := session.New(5*time.Minute, 100, 0) + defer store.Close() + srv := New(":0", store, WithCatalog(stub)) + ts := httptest.NewServer(srv.server.Handler) + defer ts.Close() + + resp, err := http.Get(ts.URL + "/v1/plugins") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + var body struct { + Plugins []CatalogEntry `json:"plugins"` + } + if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if len(body.Plugins) != 2 { + t.Fatalf("plugins = %d, want 2", len(body.Plugins)) + } + // alpha: fields populated, including required/enum/default. + if got := len(body.Plugins[0].Fields); got != 2 { + t.Fatalf("alpha.Fields = %d, want 2", got) + } + if !body.Plugins[0].Fields[0].Required { + t.Errorf("alpha.endpoint should be required") + } + if body.Plugins[0].Fields[1].Default != "allow" { + t.Errorf("alpha.policy.Default = %q", body.Plugins[0].Fields[1].Default) + } + if got := len(body.Plugins[0].Fields[1].Enum); got != 2 { + t.Errorf("alpha.policy.Enum length = %d", got) + } + // beta: no fields → wire format omits the field; decoded slice is nil/empty. + if len(body.Plugins[1].Fields) != 0 { + t.Errorf("beta.Fields should be empty, got %+v", body.Plugins[1].Fields) + } +} + func TestHandlePluginCatalog_NoProvider404(t *testing.T) { store := session.New(5*time.Minute, 100, 0) defer store.Close() diff --git a/authbridge/cmd/abctl/apiclient/client.go b/authbridge/cmd/abctl/apiclient/client.go index 1fe4ac168..06b8eab0c 100644 --- a/authbridge/cmd/abctl/apiclient/client.go +++ b/authbridge/cmd/abctl/apiclient/client.go @@ -119,16 +119,30 @@ type PluginCatalog struct { // post-Normalize); the older bodyAccess alias is intentionally NOT // emitted on this new wire shape. type PluginCatalogEntry struct { - Name string `json:"name"` - Direction string `json:"direction,omitempty"` - ReadsBody bool `json:"readsBody,omitempty"` - Writes []string `json:"writes,omitempty"` - Reads []string `json:"reads,omitempty"` - Requires []string `json:"requires,omitempty"` - RequiresAny []string `json:"requiresAny,omitempty"` - After []string `json:"after,omitempty"` - Claims []string `json:"claims,omitempty"` - Description string `json:"description,omitempty"` + Name string `json:"name"` + Direction string `json:"direction,omitempty"` + ReadsBody bool `json:"readsBody,omitempty"` + Writes []string `json:"writes,omitempty"` + Reads []string `json:"reads,omitempty"` + Requires []string `json:"requires,omitempty"` + RequiresAny []string `json:"requiresAny,omitempty"` + After []string `json:"after,omitempty"` + Claims []string `json:"claims,omitempty"` + Description string `json:"description,omitempty"` + Fields []PluginFieldEntry `json:"fields,omitempty"` +} + +// PluginFieldEntry mirrors sessionapi.FieldSchemaEntry — per-field +// schema metadata for a plugin's config. Used by abctl edit's +// templates renderer; nil for plugins without configs. +type PluginFieldEntry struct { + Name string `json:"name"` + Type string `json:"type"` + Required bool `json:"required,omitempty"` + Description string `json:"description,omitempty"` + Default string `json:"default,omitempty"` + Enum []string `json:"enum,omitempty"` + Fields []PluginFieldEntry `json:"fields,omitempty"` } // GetPluginCatalog fetches /v1/plugins. Returns ErrNotFound when the diff --git a/authbridge/cmd/abctl/apiclient/client_test.go b/authbridge/cmd/abctl/apiclient/client_test.go index 428e489e1..76d78dc70 100644 --- a/authbridge/cmd/abctl/apiclient/client_test.go +++ b/authbridge/cmd/abctl/apiclient/client_test.go @@ -179,6 +179,81 @@ func TestGetPluginCatalog(t *testing.T) { } } +// TestGetPluginCatalog_DecodesFieldSchemas guards against tag drift +// between server-side FieldSchemaEntry (authlib/sessionapi/server.go) +// and client-side PluginFieldEntry (here in apiclient). Every JSON +// key the server emits must decode into the matching Go field, or the +// abctl edit templates renderer silently loses metadata. Covers +// nested fields too — token-exchange's identity sub-schema is the +// real-world use case. +func TestGetPluginCatalog_DecodesFieldSchemas(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/plugins" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "plugins": [ + { + "name": "te", + "description": "Token exchange.", + "fields": [ + {"name": "token_url", "type": "string", "description": "Endpoint."}, + { + "name": "identity", + "type": "object", + "description": "Client credentials.", + "fields": [ + {"name": "type", "type": "string", "required": true, + "description": "Scheme.", "enum": ["spiffe", "client-secret"]}, + {"name": "timeout_ms", "type": "int", "default": "5000"} + ] + } + ] + } + ] + }`)) + })) + defer ts.Close() + + c := New(ts.URL) + cat, err := c.GetPluginCatalog(context.Background()) + if err != nil { + t.Fatalf("GetPluginCatalog: %v", err) + } + if len(cat.Plugins) != 1 || len(cat.Plugins[0].Fields) != 2 { + t.Fatalf("unexpected catalog shape: %+v", cat) + } + te := cat.Plugins[0] + + tokenURL := te.Fields[0] + if tokenURL.Name != "token_url" || tokenURL.Type != "string" || tokenURL.Description != "Endpoint." { + t.Errorf("token_url decode mismatch: %+v", tokenURL) + } + + id := te.Fields[1] + if id.Name != "identity" || id.Type != "object" || id.Description != "Client credentials." { + t.Errorf("identity decode mismatch: %+v", id) + } + if len(id.Fields) != 2 { + t.Fatalf("identity.Fields = %d, want 2", len(id.Fields)) + } + + idType := id.Fields[0] + if idType.Name != "type" || !idType.Required || idType.Description != "Scheme." { + t.Errorf("identity.type decode mismatch: %+v", idType) + } + if len(idType.Enum) != 2 || idType.Enum[0] != "spiffe" || idType.Enum[1] != "client-secret" { + t.Errorf("identity.type.enum = %v", idType.Enum) + } + + timeout := id.Fields[1] + if timeout.Name != "timeout_ms" || timeout.Type != "int" || timeout.Default != "5000" { + t.Errorf("identity.timeout_ms decode mismatch: %+v", timeout) + } +} + // TestPipelinePluginDecodesCapabilityMetadata verifies the new // metadata fields decode correctly through the apiclient. func TestPipelinePluginDecodesCapabilityMetadata(t *testing.T) { diff --git a/authbridge/cmd/abctl/edit/edit.go b/authbridge/cmd/abctl/edit/edit.go index 794d6b536..db26fc502 100644 --- a/authbridge/cmd/abctl/edit/edit.go +++ b/authbridge/cmd/abctl/edit/edit.go @@ -7,6 +7,8 @@ import ( "time" tea "github.com/charmbracelet/bubbletea" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/apiclient" ) // tempFileMaxAge is how long a stale edit tempfile is allowed to sit @@ -42,9 +44,14 @@ func SweepStaleTempfiles() int { // FetchedMsg is the result of FetchCmd. On success: Fetched and TempPath // are both set, Err is nil. On failure: Err is populated, others are zero. +// +// Catalog carries the catalog used to render templates, so the TUI can +// cache it into m.catalog after a fresh fetch. Nil when the caller +// supplied a cached catalog or no client. type FetchedMsg struct { Fetched *FetchedPipeline TempPath string // path to the tempfile holding just the pipeline subtree + Catalog *apiclient.PluginCatalog Err error } @@ -54,7 +61,20 @@ type FetchedMsg struct { // $EDITOR), and emits FetchedMsg. The tempfile lives in $TMPDIR; abctl // leaves it in place on every exit path (success, error, abort) so users // can recover an in-progress edit. -func FetchCmd(ctx context.Context, run Runner, namespace, pod string) tea.Cmd { +// +// cachedCatalog is the catalog the TUI has already fetched (e.g. via +// the catalog pane). When non-nil it's used as-is to render templates. +// When nil and client is non-nil, FetchCmd fetches it inline so the +// edit experience works on first 'e' press without requiring the +// operator to open the catalog pane first. Both nil → no templates +// (used by tests and degraded server paths). +func FetchCmd( + ctx context.Context, + run Runner, + client *apiclient.Client, + namespace, pod string, + cachedCatalog []apiclient.PluginCatalogEntry, +) tea.Cmd { return func() tea.Msg { agent, err := ResolveAgentName(ctx, run, namespace, pod) if err != nil { @@ -64,6 +84,19 @@ func FetchCmd(ctx context.Context, run Runner, namespace, pod string) tea.Cmd { if err != nil { return FetchedMsg{Err: err} } + + // Resolve the catalog: prefer cached, otherwise fetch inline. + // Catalog-fetch failure is non-fatal — the edit still opens + // without templates, mirroring the older "no catalog" path. + catalog := cachedCatalog + var freshCatalog *apiclient.PluginCatalog + if catalog == nil && client != nil { + if c, err := client.GetPluginCatalog(ctx); err == nil && c != nil { + freshCatalog = c + catalog = c.Plugins + } + } + tmp, err := os.CreateTemp("", "abctl-pipeline-*.yaml") if err != nil { return FetchedMsg{Err: err} @@ -73,11 +106,17 @@ func FetchCmd(ctx context.Context, run Runner, namespace, pod string) tea.Cmd { tmp.Close() return FetchedMsg{Err: err} } + if templates := RenderTemplates(catalog); len(templates) > 0 { + if _, err := tmp.Write(templates); err != nil { + tmp.Close() + return FetchedMsg{Err: err} + } + } path := tmp.Name() if err := tmp.Close(); err != nil { return FetchedMsg{Err: err} } - return FetchedMsg{Fetched: fp, TempPath: path} + return FetchedMsg{Fetched: fp, TempPath: path, Catalog: freshCatalog} } } diff --git a/authbridge/cmd/abctl/edit/edit_test.go b/authbridge/cmd/abctl/edit/edit_test.go index 173a9c996..c21901581 100644 --- a/authbridge/cmd/abctl/edit/edit_test.go +++ b/authbridge/cmd/abctl/edit/edit_test.go @@ -14,7 +14,7 @@ func TestFetchCmd_Success(t *testing.T) { stub := func(ctx context.Context, args ...string) ([]byte, error) { return []byte(fixtureCMYAML), nil } - cmd := FetchCmd(context.Background(), stub, "team1", "email-agent") + cmd := FetchCmd(context.Background(), stub, nil, "team1", "email-agent", nil) msg := cmd().(FetchedMsg) if msg.Err != nil { t.Fatalf("FetchedMsg.Err = %v", msg.Err) @@ -31,7 +31,7 @@ func TestFetchCmd_Error(t *testing.T) { stub := func(ctx context.Context, args ...string) ([]byte, error) { return nil, fmt.Errorf("forbidden") } - cmd := FetchCmd(context.Background(), stub, "team1", "email-agent") + cmd := FetchCmd(context.Background(), stub, nil, "team1", "email-agent", nil) msg := cmd().(FetchedMsg) if msg.Err == nil { t.Fatal("expected error") diff --git a/authbridge/cmd/abctl/edit/templates.go b/authbridge/cmd/abctl/edit/templates.go new file mode 100644 index 000000000..32bca5c27 --- /dev/null +++ b/authbridge/cmd/abctl/edit/templates.go @@ -0,0 +1,383 @@ +package edit + +import ( + "bytes" + "fmt" + "strings" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/apiclient" +) + +// FenceMarker delimits the active pipeline subtree (above) from the +// commented templates reference (below) inside the abctl edit tempfile. +// The save path strips everything from this line onward before applying. +// +// The exact bytes matter: detection is a literal line match. Keep them +// in sync with templates_test.go and configmap.go's fence-stripping. +const FenceMarker = "# === ABCTL TEMPLATES BELOW (stripped on save) ===" + +// templatesBanner is the prose shown immediately below FenceMarker, +// telling the operator how to use the reference. The leading and +// trailing rule lines bracket the fence visually so the boundary +// scans at-a-glance even after a long pipeline subtree. +// +// All lines are below the fence and get stripped on save. The visual +// padding ABOVE the fence is supplied by RenderTemplates as blank +// lines; StripTemplates trims trailing blank lines so the round-trip +// stays byte-identical. +// +// ASCII-only by design: a previous Unicode-bordered banner triggered +// editor-plugin failures on some operator setups. ASCII works +// everywhere without sacrificing visual hierarchy. +const templatesBanner = `# ==================================================================== +# +# Reference: every plugin in the catalog. +# +# To use a template, copy a plugin block from below, paste it above the +# fence into your inbound: or outbound: chain, strip the leading "# " +# from each line, then adjust indentation (the templates use a 6-space +# "- name:" indent -- match whatever your existing plugins use). +# +# This entire section -- from the fence line down -- is stripped before +# the edited buffer is written back to the ConfigMap. +# +# ====================================================================` + +// RenderTemplates returns a fence marker followed by a commented YAML +// template block per plugin in the catalog. Returns nil for an empty +// catalog so callers can append unconditionally. +// +// Every emitted line starts with "#" — the templates section is pure +// comments. If the operator deletes the fence marker by accident, the +// templates still parse as comment-only YAML (no semantic effect), +// which is the safe-fallback the plan committed to. +// +// Output starts with two blank lines for visual separation from the +// active pipeline subtree above, then the fence marker, then the +// banner block. The blank lines are safe because StripTemplates trims +// trailing whitespace-only lines after truncating at the fence — so a +// render-then-strip round-trip is byte-identical even with the +// padding. A render that emitted CONTENT before the fence would +// survive strip and surface as a spurious `+` line in the no-changes +// diff, which is why all decoration lives below the fence. +func RenderTemplates(plugins []apiclient.PluginCatalogEntry) []byte { + if len(plugins) == 0 { + return nil + } + var b strings.Builder + b.WriteString("\n\n") + b.WriteString(FenceMarker) + b.WriteString("\n") + b.WriteString(templatesBanner) + b.WriteString("\n") + for _, p := range plugins { + renderPluginTemplate(&b, p) + } + return []byte(b.String()) +} + +func renderPluginTemplate(b *strings.Builder, p apiclient.PluginCatalogEntry) { + b.WriteString("\n# --- ") + b.WriteString(p.Name) + b.WriteString(" ---\n") + if p.Description != "" { + // Description is single-line (Go struct tags can't carry newlines) + // so a one-line "# " comment is sufficient. + b.WriteString("# ") + b.WriteString(p.Description) + b.WriteString("\n") + } + + // Split top-level fields into required vs optional for ordering + // (required render first inside the config: block). Object fields + // with required descendants (e.g. identity wrapping identity.type) + // are treated as effectively required so the parent renders at the + // top of the block, matching the [REQUIRED] annotation it'll get + // from renderField. + var required, optional []apiclient.PluginFieldEntry + for _, f := range p.Fields { + if f.Required || hasRequiredDescendant(f) { + required = append(required, f) + } else { + optional = append(optional, f) + } + } + // Header lists every required field anywhere in the schema — + // including nested sub-fields like identity.type — so operators + // see the full set of must-fill slots without having to skim + // the body looking for [REQUIRED] markers inside object blocks. + if len(p.Fields) > 0 { + b.WriteString("# Required: ") + if reqPaths := collectRequiredPaths("", p.Fields); len(reqPaths) == 0 { + b.WriteString("(none — every field is optional)") + } else { + b.WriteString(strings.Join(reqPaths, ", ")) + } + b.WriteString("\n") + } + + b.WriteString("# - name: ") + b.WriteString(p.Name) + b.WriteString("\n") + if len(p.Fields) == 0 { + b.WriteString("# # (no configurable fields)\n") + return + } + b.WriteString("# config:\n") + for _, f := range required { + renderField(b, f, " ") + } + for _, f := range optional { + renderField(b, f, " ") + } +} + +// hasRequiredDescendant reports whether f or any field beneath it +// carries Required=true. An object field whose own Required tag is +// false but which contains a required leaf (e.g. tokenexchange's +// identity wrapping a required identity.type) is "effectively +// required" — the operator can't legally omit the block, so the +// renderer should show it as [REQUIRED] rather than [optional]. +func hasRequiredDescendant(f apiclient.PluginFieldEntry) bool { + for _, sf := range f.Fields { + if sf.Required || hasRequiredDescendant(sf) { + return true + } + } + return false +} + +// collectRequiredPaths walks the schema tree and returns dotted paths +// for every required field. Top-level required fields appear as +// `name`; nested ones as `parent.child` (e.g. `identity.type`). Used +// by the per-plugin "Required:" header so operators see the full +// must-fill set including ones buried inside object sub-trees. +func collectRequiredPaths(prefix string, fields []apiclient.PluginFieldEntry) []string { + var out []string + for _, f := range fields { + path := f.Name + if prefix != "" { + path = prefix + "." + f.Name + } + if f.Required { + out = append(out, path) + } + if f.Type == "object" && len(f.Fields) > 0 { + out = append(out, collectRequiredPaths(path, f.Fields)...) + } + } + return out +} + +// renderField emits two lines per field for readability: +// +// # # [REQUIRED] +// # : +// +// or, for optional fields: +// +// # # [optional, default=X, enum=a|b] +// # : +// +// For nested struct fields with their own sub-fields, the placeholder +// is replaced by a recursive block so operators see required nested +// fields (e.g. identity.type) instead of a misleading "identity: {}". +// +// The bracket prefix scans at the left margin so the operator can tell +// required from optional without parsing inline notes. +// +// indent is the column-prefix for the value line (counted after the +// leading "#"). The default value column is " " (matching +// " config:"); recursive calls deepen by two spaces per level. +func renderField(b *strings.Builder, f apiclient.PluginFieldEntry, indent string) { + // Effective required-ness: a field is required either because its + // own tag says so, or because it's an object containing a required + // descendant (omitting the parent block makes the child unsettable). + effectivelyRequired := f.Required || hasRequiredDescendant(f) + + // Annotation line. + b.WriteString("#") + b.WriteString(indent) + b.WriteString("# ") + if effectivelyRequired { + b.WriteString("[REQUIRED]") + } else { + var attrs []string + attrs = append(attrs, "optional") + if f.Default != "" { + attrs = append(attrs, fmt.Sprintf("default=%s", f.Default)) + } + if len(f.Enum) > 0 { + attrs = append(attrs, "enum="+strings.Join(f.Enum, "|")) + } + b.WriteString("[") + b.WriteString(strings.Join(attrs, ", ")) + b.WriteString("]") + } + if f.Description != "" { + b.WriteString(" ") + b.WriteString(f.Description) + } + b.WriteString("\n") + + // Required field with an enum: still surface the choices, since + // they're constraint information, not a default note. + if f.Required && len(f.Enum) > 0 { + b.WriteString("#") + b.WriteString(indent) + b.WriteString("# choices: ") + b.WriteString(strings.Join(f.Enum, " | ")) + b.WriteString("\n") + } + + // Object field with sub-fields: render the sub-tree inline so + // nested required fields (e.g. identity.type) are visible. + if f.Type == "object" && len(f.Fields) > 0 { + b.WriteString("#") + b.WriteString(indent) + b.WriteString(f.Name) + b.WriteString(":\n") + // Reorder sub-fields so required render first within the + // nested block, matching top-level convention. + var req, opt []apiclient.PluginFieldEntry + for _, sf := range f.Fields { + if sf.Required { + req = append(req, sf) + } else { + opt = append(opt, sf) + } + } + nestedIndent := indent + " " + for _, sf := range req { + renderField(b, sf, nestedIndent) + } + for _, sf := range opt { + renderField(b, sf, nestedIndent) + } + return + } + + // Value line. + b.WriteString("#") + b.WriteString(indent) + b.WriteString(f.Name) + b.WriteString(": ") + b.WriteString(placeholderFor(f)) + b.WriteString("\n") +} + +// StripTemplates returns edited with the fence marker and everything +// after it removed. If the fence marker is absent, edited is returned +// unchanged — the safe-fallback contract from the plan: a missing +// fence marker means the operator either deleted it deliberately or +// the catalog wasn't available at fetch time, and either way the +// remaining buffer is what gets applied. +// +// Detection is a strict line match: a line whose first non-whitespace +// content is exactly FenceMarker. Whitespace before the marker is +// tolerated (some editors auto-indent comment lines), but the marker +// itself must be intact. Truncation occurs at the start of that line +// so the trailing newline of the previous line — the active pipeline +// subtree's terminator — survives. +// +// After truncating, any trailing whitespace-only lines are also +// removed. RenderTemplates inserts blank lines BEFORE the fence as +// visual padding; without this trim those blank lines would survive +// strip and surface as spurious `+` diff lines on save-without-changes. +// The previous real line's terminating \n is preserved. +func StripTemplates(edited []byte) []byte { + target := FenceMarker + // Walk lines manually rather than splitting; preserves the byte + // position needed for the truncation cut. + i := 0 + for i < len(edited) { + // Find the next line's end. + end := i + for end < len(edited) && edited[end] != '\n' { + end++ + } + // Strip leading whitespace for the comparison. + start := i + for start < end && (edited[start] == ' ' || edited[start] == '\t') { + start++ + } + // Exact line match: tolerate a trailing CR (CRLF endings) but + // reject any other extra content. A prefix match would let + // "# === ABCTL TEMPLATES BELOW (stripped on save) ===extra" + // trigger truncation, silently dropping operator edits made + // on a line that happens to begin with the marker. + lineEnd := end + if lineEnd > start && edited[lineEnd-1] == '\r' { + lineEnd-- + } + if lineEnd-start == len(target) && string(edited[start:lineEnd]) == target { + // Truncate at i — the start of this line — discarding the + // fence and everything after, then trim trailing blank + // lines so the renderer can prepend visual padding. + return trimTrailingBlankLines(edited[:i]) + } + // Advance past the newline. + i = end + 1 + } + return edited +} + +// trimTrailingBlankLines drops trailing whitespace-only lines from +// out, preserving the terminating newline of the last non-blank line. +// "out" is expected to end with \n (the typical content shape) but +// the function is safe on any input. +func trimTrailingBlankLines(out []byte) []byte { + for len(out) > 0 && out[len(out)-1] == '\n' { + // Find the start of the line that ends at len(out)-1. + prevNL := bytes.LastIndexByte(out[:len(out)-1], '\n') + lineStart := prevNL + 1 // 0 when no earlier \n + line := out[lineStart : len(out)-1] + if !isBlankLine(line) { + break + } + // Drop the blank line; the previous real line's terminating + // \n (at position prevNL) is at out[lineStart-1] and stays. + out = out[:lineStart] + } + return out +} + +// isBlankLine reports whether b contains only horizontal whitespace. +// '\r' counts too: an editor that saves with CRLF line endings would +// otherwise leave a stray "\r\n" line surviving the trim and break +// the byte-identical round-trip on save-without-changes. +func isBlankLine(b []byte) bool { + for _, c := range b { + if c != ' ' && c != '\t' && c != '\r' { + return false + } + } + return true +} + +// placeholderFor picks the YAML placeholder that goes after the field +// name. Documented defaults take priority for primitive types so the +// operator sees the actual fallback inline; otherwise an empty value +// matching the field's type. +func placeholderFor(f apiclient.PluginFieldEntry) string { + if f.Default != "" && f.Type != "object" { + // Quote strings so the line is valid YAML when uncommented. + if f.Type == "string" { + return fmt.Sprintf("%q", f.Default) + } + return f.Default + } + switch f.Type { + case "string": + return `""` + case "int": + return "0" + case "bool": + return "false" + case "[]string": + return "[]" + case "object": + return "{}" + } + return `""` +} diff --git a/authbridge/cmd/abctl/edit/templates_test.go b/authbridge/cmd/abctl/edit/templates_test.go new file mode 100644 index 000000000..4fb435367 --- /dev/null +++ b/authbridge/cmd/abctl/edit/templates_test.go @@ -0,0 +1,434 @@ +package edit + +import ( + "context" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/apiclient" +) + +func TestRenderTemplates_EmptyCatalog(t *testing.T) { + if got := RenderTemplates(nil); got != nil { + t.Fatalf("nil catalog should produce nil output, got %q", string(got)) + } + if got := RenderTemplates([]apiclient.PluginCatalogEntry{}); got != nil { + t.Fatalf("empty catalog should produce nil output, got %q", string(got)) + } +} + +func TestRenderTemplates_FenceMarkerPresent(t *testing.T) { + out := RenderTemplates([]apiclient.PluginCatalogEntry{{Name: "noop"}}) + if !strings.Contains(string(out), FenceMarker) { + t.Fatalf("output missing fence marker:\n%s", string(out)) + } +} + +func TestRenderTemplates_PluginWithFields(t *testing.T) { + cat := []apiclient.PluginCatalogEntry{ + { + Name: "ibac", + Description: "Intent-based access control via LLM judge.", + Fields: []apiclient.PluginFieldEntry{ + {Name: "judge_endpoint", Type: "string", Required: true, + Description: "Base URL of the LLM judge."}, + {Name: "judge_model", Type: "string", Required: true, + Description: "Model name."}, + {Name: "timeout_ms", Type: "int", Default: "5000", + Description: "Per-call timeout."}, + {Name: "unclassified_policy", Type: "string", Default: "passthrough", + Enum: []string{"passthrough", "judge"}, + Description: "Behavior when no parser claimed the request."}, + }, + }, + } + out := string(RenderTemplates(cat)) + + for _, want := range []string{ + "# --- ibac ---", + "Intent-based access control via LLM judge.", + "# Required: judge_endpoint, judge_model", + "# - name: ibac", + "# config:", + "# [REQUIRED] Base URL of the LLM judge.", + `# judge_endpoint: ""`, + "# [optional, default=5000] Per-call timeout.", + "# timeout_ms: 5000", + "[optional, default=passthrough, enum=passthrough|judge]", + } { + if !strings.Contains(out, want) { + t.Errorf("output missing %q\n----\n%s", want, out) + } + } + + // Required fields should appear before optional ones. + reqIdx := strings.Index(out, "judge_endpoint:") + optIdx := strings.Index(out, "timeout_ms:") + if reqIdx < 0 || optIdx < 0 { + t.Fatalf("could not locate fields in output:\n%s", out) + } + if reqIdx > optIdx { + t.Errorf("required fields should render before optional ones; got required at %d, optional at %d", reqIdx, optIdx) + } +} + +func TestRenderTemplates_PluginNoFields(t *testing.T) { + cat := []apiclient.PluginCatalogEntry{ + {Name: "a2a-parser", Description: "A2A protocol parser."}, + } + out := string(RenderTemplates(cat)) + if !strings.Contains(out, "# (no configurable fields)") { + t.Fatalf("expected no-fields hint:\n%s", out) + } + if strings.Contains(out, "config:") { + t.Errorf("plugin without fields shouldn't emit config: line:\n%s", out) + } +} + +func TestRenderTemplates_AllOptionalShowsRequiredNoneHeader(t *testing.T) { + cat := []apiclient.PluginCatalogEntry{ + { + Name: "all-opt", + Fields: []apiclient.PluginFieldEntry{ + {Name: "x", Type: "string"}, + }, + }, + } + out := string(RenderTemplates(cat)) + if !strings.Contains(out, "# Required: (none — every field is optional)") { + t.Fatalf("plugin with no required fields should explicitly say so:\n%s", out) + } +} + +func TestRenderTemplates_HeaderListsNestedRequiredPaths(t *testing.T) { + cat := []apiclient.PluginCatalogEntry{ + { + Name: "te", + Fields: []apiclient.PluginFieldEntry{ + {Name: "token_url", Type: "string"}, + { + Name: "identity", Type: "object", + Fields: []apiclient.PluginFieldEntry{ + {Name: "type", Type: "string", Required: true}, + }, + }, + }, + }, + } + out := string(RenderTemplates(cat)) + if !strings.Contains(out, "# Required: identity.type") { + t.Fatalf("nested required field should appear in header as identity.type:\n%s", out) + } +} + +func TestRenderTemplates_NestedObjectRecurses(t *testing.T) { + cat := []apiclient.PluginCatalogEntry{ + { + Name: "te", + Fields: []apiclient.PluginFieldEntry{ + { + Name: "identity", + Type: "object", + Description: "Client credentials.", + Fields: []apiclient.PluginFieldEntry{ + {Name: "type", Type: "string", Required: true, + Enum: []string{"spiffe", "client-secret"}, + Description: "Identity scheme."}, + {Name: "client_id", Type: "string", + Description: "Inline client id."}, + }, + }, + }, + }, + } + out := string(RenderTemplates(cat)) + if strings.Contains(out, "identity: {}") { + t.Errorf("nested object should NOT collapse to {}; got:\n%s", out) + } + if !strings.Contains(out, "identity:\n") { + t.Errorf("nested object should render `identity:` then sub-fields, got:\n%s", out) + } + if !strings.Contains(out, "[REQUIRED] Identity scheme.") { + t.Errorf("nested required field annotation missing:\n%s", out) + } + // Object parent of a required leaf should itself read [REQUIRED] + // even though its own tag isn't required:"true" — operators can't + // legally omit the block, so [optional] would mislead. + if !strings.Contains(out, "[REQUIRED] Client credentials.") { + t.Errorf("parent of required leaf should annotate as [REQUIRED]:\n%s", out) + } + // Nested fields should be indented further than the top-level + // config fields. Top-level uses "# " (11 spaces); + // nested adds 2 more. + if !strings.Contains(out, "# type:") { + t.Errorf("nested field should indent by 2 spaces beyond parent:\n%s", out) + } +} + +func TestRenderTemplates_RequiredEnumShowsChoices(t *testing.T) { + cat := []apiclient.PluginCatalogEntry{ + { + Name: "p", + Fields: []apiclient.PluginFieldEntry{ + {Name: "mode", Type: "string", Required: true, + Enum: []string{"a", "b", "c"}, Description: "Pick one."}, + }, + }, + } + out := string(RenderTemplates(cat)) + if !strings.Contains(out, "# [REQUIRED] Pick one.") { + t.Errorf("required field annotation missing:\n%s", out) + } + if !strings.Contains(out, "# choices: a | b | c") { + t.Errorf("required+enum field should surface choices line:\n%s", out) + } +} + +func TestRenderTemplates_PlaceholderTypes(t *testing.T) { + cat := []apiclient.PluginCatalogEntry{ + { + Name: "broker", + Fields: []apiclient.PluginFieldEntry{ + {Name: "name", Type: "string"}, + {Name: "count", Type: "int"}, + {Name: "flag", Type: "bool"}, + {Name: "items", Type: "[]string"}, + {Name: "nested", Type: "object"}, + {Name: "with_default", Type: "string", Default: "abc"}, + }, + }, + } + out := string(RenderTemplates(cat)) + for _, want := range []string{ + `name: ""`, + "count: 0", + "flag: false", + "items: []", + "nested: {}", + `with_default: "abc"`, // string default should be quoted + } { + if !strings.Contains(out, want) { + t.Errorf("output missing %q\n----\n%s", want, out) + } + } +} + +func TestFetchCmd_AppendsTemplatesWhenCatalogProvided(t *testing.T) { + r := func(_ context.Context, _ ...string) ([]byte, error) { + return []byte(fixtureCMYAML), nil + } + cat := []apiclient.PluginCatalogEntry{ + {Name: "ibac", Description: "test plugin"}, + } + cmd := FetchCmd(context.Background(), r, nil, "team1", "email-agent", cat) + msg := cmd().(FetchedMsg) + if msg.Err != nil { + t.Fatalf("FetchCmd err: %v", msg.Err) + } + body, err := os.ReadFile(msg.TempPath) + if err != nil { + t.Fatalf("read tempfile: %v", err) + } + if !strings.Contains(string(body), FenceMarker) { + t.Fatalf("tempfile missing fence marker; templates not appended:\n%s", string(body)) + } + if !strings.Contains(string(body), "# --- ibac ---") { + t.Fatalf("tempfile missing plugin block:\n%s", string(body)) + } +} + +func TestStripTemplates_RemovesFenceAndBelow(t *testing.T) { + original := "pipeline:\n inbound:\n plugins: []\n" + edited := []byte(original + "\n" + FenceMarker + "\n# --- ibac ---\n# stuff\n") + got := string(StripTemplates(edited)) + // Trailing blank line(s) before the fence are also stripped — the + // renderer prepends them as visual padding, so preserving them + // would create drift on save-without-changes. + if got != original { + t.Fatalf("StripTemplates mismatch\nwant: %q\ngot: %q", original, got) + } +} + +func TestStripTemplates_TrimsMultipleBlankLinesBeforeFence(t *testing.T) { + original := "pipeline: {}\n" + edited := []byte(original + "\n\n\n" + FenceMarker + "\nbanner\n") + got := string(StripTemplates(edited)) + if got != original { + t.Fatalf("multiple blank lines before fence should all be stripped\nwant: %q\ngot: %q", original, got) + } +} + +func TestStripTemplates_TrimsWhitespaceOnlyLineBeforeFence(t *testing.T) { + // A line with just spaces / tabs is also "blank" for trim purposes. + original := "pipeline: {}\n" + edited := []byte(original + " \t \n" + FenceMarker + "\n") + got := string(StripTemplates(edited)) + if got != original { + t.Fatalf("whitespace-only line before fence should be stripped\nwant: %q\ngot: %q", original, got) + } +} + +func TestStripTemplates_TrimsCRLFBlankLineBeforeFence(t *testing.T) { + // An editor that normalizes line endings to CRLF on save would + // leave "\r\n" blank lines above the fence. Without the \r-aware + // blank check, those would survive the trim and break the + // byte-identical round-trip the PR is built around. + original := "pipeline: {}\n" + edited := []byte(original + "\r\n\r\n" + FenceMarker + "\n") + got := string(StripTemplates(edited)) + if got != original { + t.Fatalf("CRLF blank line before fence should be stripped\nwant: %q\ngot: %q", original, got) + } +} + +func TestStripTemplates_NoFenceReturnsUnchanged(t *testing.T) { + in := []byte("pipeline:\n inbound:\n plugins: []\n") + got := StripTemplates(in) + if string(got) != string(in) { + t.Fatalf("input without fence should be unchanged\nwant: %q\ngot: %q", string(in), string(got)) + } +} + +func TestStripTemplates_ToleratesLeadingWhitespace(t *testing.T) { + original := "pipeline: {}\n" + edited := []byte(original + " " + FenceMarker + "\n# stuff\n") + got := string(StripTemplates(edited)) + if got != original { + t.Fatalf("leading-whitespace fence not stripped\nwant: %q\ngot: %q", original, got) + } +} + +func TestStripTemplates_FenceAtEOF(t *testing.T) { + original := "pipeline: {}\n" + edited := []byte(original + FenceMarker) + got := string(StripTemplates(edited)) + if got != original { + t.Fatalf("fence-at-eof case\nwant: %q\ngot: %q", original, got) + } +} + +func TestStripTemplates_NotFooledBySimilarLine(t *testing.T) { + // A YAML comment that mentions the marker as part of prose, not on its own line. + in := []byte("pipeline:\n # see " + FenceMarker + " for details\n inbound: {}\n") + got := StripTemplates(in) + if string(got) != string(in) { + t.Fatalf("inline mention should not match; input should be unchanged") + } +} + +func TestStripTemplates_RequiresExactFenceMatch(t *testing.T) { + // A line that starts with the marker but has trailing extra content + // must NOT match — otherwise an operator's edits on that line could + // be silently truncated. + original := "pipeline: {}\n" + in := []byte(original + FenceMarker + " trailing comment\nstill active\n") + got := StripTemplates(in) + if string(got) != string(in) { + t.Fatalf("fence with trailing content should not trigger truncation\nwant: %q\ngot: %q", string(in), string(got)) + } +} + +func TestStripTemplates_AcceptsCRLFFenceLine(t *testing.T) { + // CRLF line endings on the fence line itself must still match — + // a trailing CR is the one tolerated suffix. + original := "pipeline: {}\n" + in := []byte(original + FenceMarker + "\r\nbanner\r\n") + got := StripTemplates(in) + if string(got) != original { + t.Fatalf("CRLF on fence line should still match\nwant: %q\ngot: %q", original, string(got)) + } +} + +func TestStripTemplates_IntegratesWithRender(t *testing.T) { + // Round-trip: render templates after a real subtree, strip them, + // and get the subtree back byte-for-byte. This is the no-changes- + // diff case: opening + saving without edits must produce the same + // bytes the operator started with — otherwise the diff prompt + // misleadingly shows a `+` line and asks to apply. + subtree := []byte("pipeline:\n inbound:\n plugins:\n - name: ibac\n") + templates := RenderTemplates([]apiclient.PluginCatalogEntry{ + {Name: "ibac", Description: "test"}, + }) + combined := append([]byte{}, subtree...) + combined = append(combined, templates...) + stripped := StripTemplates(combined) + if string(stripped) != string(subtree) { + t.Fatalf("round-trip mismatch (would surface as spurious diff)\nwant: %q\ngot: %q", + string(subtree), string(stripped)) + } +} + +func TestFetchCmd_FetchesCatalogInlineWhenCacheNil(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/plugins" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"plugins":[{"name":"fetched-from-stub"}]}`)) + })) + defer srv.Close() + client := apiclient.New(srv.URL) + r := func(_ context.Context, _ ...string) ([]byte, error) { + return []byte(fixtureCMYAML), nil + } + cmd := FetchCmd(context.Background(), r, client, "team1", "email-agent", nil) + msg := cmd().(FetchedMsg) + if msg.Err != nil { + t.Fatalf("FetchCmd err: %v", msg.Err) + } + body, err := os.ReadFile(msg.TempPath) + if err != nil { + t.Fatalf("read tempfile: %v", err) + } + if !strings.Contains(string(body), "fetched-from-stub") { + t.Fatalf("inline-fetched catalog not rendered:\n%s", string(body)) + } + if msg.Catalog == nil { + t.Fatal("FetchedMsg.Catalog should be set when fetched inline (so the TUI can cache it)") + } +} + +func TestFetchCmd_FetcherErrorIsNonFatal(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + client := apiclient.New(srv.URL) + r := func(_ context.Context, _ ...string) ([]byte, error) { + return []byte(fixtureCMYAML), nil + } + cmd := FetchCmd(context.Background(), r, client, "team1", "email-agent", nil) + msg := cmd().(FetchedMsg) + if msg.Err != nil { + t.Fatalf("catalog-fetch failure should not break edit: %v", msg.Err) + } + body, err := os.ReadFile(msg.TempPath) + if err != nil { + t.Fatalf("read tempfile: %v", err) + } + if strings.Contains(string(body), FenceMarker) { + t.Fatalf("templates should be absent when fetcher errored:\n%s", string(body)) + } +} + +func TestFetchCmd_NoTemplatesWhenCatalogNil(t *testing.T) { + r := func(_ context.Context, _ ...string) ([]byte, error) { + return []byte(fixtureCMYAML), nil + } + cmd := FetchCmd(context.Background(), r, nil, "team1", "email-agent", nil) + msg := cmd().(FetchedMsg) + if msg.Err != nil { + t.Fatalf("FetchCmd err: %v", msg.Err) + } + body, err := os.ReadFile(msg.TempPath) + if err != nil { + t.Fatalf("read tempfile: %v", err) + } + if strings.Contains(string(body), FenceMarker) { + t.Fatalf("tempfile should not contain fence marker when catalog is nil:\n%s", string(body)) + } +} diff --git a/authbridge/cmd/abctl/tui/app.go b/authbridge/cmd/abctl/tui/app.go index 6c4fe6534..928e8f255 100644 --- a/authbridge/cmd/abctl/tui/app.go +++ b/authbridge/cmd/abctl/tui/app.go @@ -633,6 +633,13 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.editState.fetched = msg.Fetched m.editState.tempPath = msg.TempPath m.editState.phase = editPhaseEditing + // FetchCmd may have fetched the catalog inline so it could + // render the templates section. Cache it so the catalog pane + // (P) and the post-save validator both reuse it without a + // second round-trip. + if msg.Catalog != nil { + m.catalog = msg.Catalog + } return m, openEditorCmd(m.editState.generation, msg.TempPath) case editorExitedMsg: @@ -650,6 +657,12 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.editState.err = "read edited file: " + err.Error() return m, nil } + // Strip the templates reference section before any downstream + // processing — the empty-check, YAML parse, splice preview, and + // validation all expect to see only the active pipeline subtree. + // No-op when the catalog wasn't included at fetch time (no fence + // marker present in the buffer). + edited = edit.StripTemplates(edited) // Fix 2: reject empty input before it can silently wipe the pipeline. if len(bytes.TrimSpace(edited)) == 0 { m.editState.phase = editPhaseError diff --git a/authbridge/cmd/abctl/tui/keys.go b/authbridge/cmd/abctl/tui/keys.go index 798eb0c9e..b55f6a13f 100644 --- a/authbridge/cmd/abctl/tui/keys.go +++ b/authbridge/cmd/abctl/tui/keys.go @@ -6,9 +6,22 @@ import ( tea "github.com/charmbracelet/bubbletea" + "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/apiclient" "github.com/kagenti/kagenti-extensions/authbridge/cmd/abctl/edit" ) +// catalogPlugins extracts the plugin slice from a (possibly nil) +// catalog snapshot so FetchCmd can render templates inline. Nil-safe: +// if the catalog hasn't loaded yet (--endpoint mode without +// /v1/plugins, or first-edit-before-poll), the edit just opens +// without templates rather than blocking. +func catalogPlugins(c *apiclient.PluginCatalog) []apiclient.PluginCatalogEntry { + if c == nil { + return nil + } + return c.Plugins +} + // handleKey processes every key press. The filter-input overlay takes // precedence; otherwise keys are dispatched based on the active pane. func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { @@ -247,7 +260,7 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd { } gen := m.editState.generation + 1 m.editState = editState{phase: editPhaseFetching, generation: gen} - return withGen(gen, edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod)) + return withGen(gen, edit.FetchCmd(m.ctx, m.editRunner, m.client, m.selectedNamespace, m.selectedPod, catalogPlugins(m.catalog))) case "g": m.goTop() @@ -504,7 +517,7 @@ func (m *model) handleEditKey(msg tea.KeyMsg) tea.Cmd { if m.editState.tempPath == "" { gen := m.editState.generation + 1 m.editState = editState{phase: editPhaseFetching, generation: gen} - return withGen(gen, edit.FetchCmd(m.ctx, m.editRunner, m.selectedNamespace, m.selectedPod)) + return withGen(gen, edit.FetchCmd(m.ctx, m.editRunner, m.client, m.selectedNamespace, m.selectedPod, catalogPlugins(m.catalog))) } m.editState.phase = editPhaseEditing return openEditorCmd(m.editState.generation, m.editState.tempPath)