Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions authbridge/authlib/plugins/lineage/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,30 @@ type Config struct {
// not ready and skips every exchange, polling the file in background (see
// Init), and /readyz names it meanwhile. Ignored when SelfID is set.
SelfIDFile string `json:"self_id_file" description:"Read when self_id is empty; until it is readable the plugin is not ready and emits nothing. Refused at start only when self_id is also empty." default:"/shared/client-id.txt"`

// Namespace is the Kubernetes namespace this workload runs in, emitted as
// the lineage.self.namespace fact on every span. Required: lineage.self.id
// alone is not an identity — the same workload name in two namespaces is
// two workloads, and the consumer keys entity identity on the
// (namespace, self.id) pair (wire contract §7). The attach kit writes its
// NAMESPACE here. It is never derived from the SPIFFE ID's path — that
// layout is a registrar convention, and the kit path has no SPIFFE ID at
// all. Empty, blank, or not an RFC 1123 DNS label (the only shape a
// namespace can have) refuses at start (see resolveNamespace); when empty,
// NamespaceFile is consulted instead.
Namespace string `json:"namespace" required:"true" description:"This workload's Kubernetes namespace (an RFC 1123 DNS label), emitted as lineage.self.namespace on every span; refused at start when empty or not a label. Alternatively namespace_file."`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitnamespace is the only required:"true" field in this Config, and the tag carries no "one-of" notion, so abctl's renderer (edit/templates.go:110, collectRequiredPaths) will emit # Required: namespace plus a [REQUIRED] annotation — contradicting the README's platform recipe, which sets only namespace_file.

I checked that this can't actually break that path: pipeline/schema.go:64-66 is explicit that Required is metadata and "boot semantics are the plugin's own concern." And the tag is defensible on its own terms — this is the one key with no default, unlike self_id, which falls back to self_id_file's default. The description does name the alternative. It's purely that an operator generating a template from the schema will be told they can't omit a key the docs tell them to omit.


// NamespaceFile is a file carrying the namespace, read once at Init when
// Namespace is empty — meant for the path the kubelet projects from the
// pod's own metadata into every container that mounts the service-account
// volume, /var/run/secrets/kubernetes.io/serviceaccount/namespace. That
// is the one source that is correct in every copy of a ConfigMap shared
// across namespaces (the platform's per-namespace authbridge-runtime-config
// is rendered from one template and copied), where an inline literal would
// be confidently wrong in every namespace but one. No default and no
// poller: an absent file is a wrong path or a missing mount, and refuses
// at start like an empty value. Ignored when Namespace is set.
NamespaceFile string `json:"namespace_file" description:"Read when namespace is empty, once at start (e.g. /var/run/secrets/kubernetes.io/serviceaccount/namespace); absent, blank or not a DNS label refuses at start."`
}

func defaultConfig() Config {
Expand Down
89 changes: 79 additions & 10 deletions authbridge/authlib/plugins/lineage/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import (
"net"
"os"
"path"
"regexp"
"strings"
"sync/atomic"
"time"
Expand Down Expand Up @@ -205,7 +206,7 @@ func (p *LineageTelemetry) Capabilities() pipeline.PluginCapabilities {
// The contract is cited major.minor only, deliberately: patch
// revisions (v1.5.x) clarify prose and never change span semantics,
// so a patch bump must not imply a producer change.
Description: "Emits two facts-only lineage spans per HTTP exchange (wire contract v1.6).",
Description: "Emits two facts-only lineage spans per HTTP exchange (wire contract v1.7).",
}
}

Expand Down Expand Up @@ -237,7 +238,8 @@ func (p *LineageTelemetry) Init(ctx context.Context) error {
// degrades honestly (abandoned / NULL / parent.source=wire or none).
// Identity is the fact's subject — it has no degraded form, and a shared
// placeholder would collapse every unidentified pod onto one entity row
// (entity id = uuid5("{kind}:{self.id}"), and entities is upsert-only).
// (entity id = uuid5("{kind}:{namespace}/{self.id}"), and entities is
// upsert-only).
//
// Failing closed is scoped to the span, not the process. An Init error
// fails Pipeline.Start and the binary exits, every plugin in the chain
Expand All @@ -254,13 +256,29 @@ func (p *LineageTelemetry) Init(ctx context.Context) error {
// self_id, which no amount of waiting fixes; both return before the gRPC
// client and batch-span-processor goroutine below exist, so a refused
// start leaks nothing.
//
// The namespace — the other half of identity (config.go, Namespace) —
// is resolved FIRST. It is cheap, unconditional and needs no waiting
// (an inline value is known when the config is rendered; a file value
// sits where the kubelet projected it before this container started),
// so a refusal happens before the identity switch can log a poller it
// would never start, and before any goroutine or connection exists.
ns, err := resolveNamespace(p.cfg)
if err != nil {
return err
}
p.cfg.Namespace = ns

var pending string // self_id_file left for the poller to resolve
switch {
case p.cfg.SelfID != "":
// Same reading as the file path: a blank value carries no identity
// and would key an entity on whitespace at the consumer.
// and would key an entity on whitespace at the consumer — and so
// does a value made only of separators ("/"), which serviceLabel
// returns as-is: a subject with no name is no subject (the #761
// round-6 question, answered here).
p.selfID = strings.TrimSpace(p.cfg.SelfID)
if p.selfID == "" {
if !hasIdentity(p.selfID) {
return fmt.Errorf("lineage-telemetry: self_id %q carries no identity", p.cfg.SelfID)
}
case p.cfg.SelfIDFile != "":
Expand Down Expand Up @@ -356,11 +374,11 @@ func (p *LineageTelemetry) Init(ctx context.Context) error {
bgCtx, cancel := context.WithCancel(context.Background())
p.bgCancel.Store(&cancel)
go p.awaitIdentity(bgCtx, pending, identityPollInterval)
slog.Info("lineage-telemetry: initialized, not ready until self_id_file resolves", "endpoint", endpoint, "self_id_file", pending)
slog.Info("lineage-telemetry: initialized, not ready until self_id_file resolves", "endpoint", endpoint, "self_id_file", pending, "namespace", p.cfg.Namespace)
return nil
}
p.ready.Store(true)
slog.Info("lineage-telemetry: initialized", "endpoint", endpoint, "self_id", p.selfID)
slog.Info("lineage-telemetry: initialized", "endpoint", endpoint, "self_id", p.selfID, "namespace", p.cfg.Namespace)
return nil
}

Expand Down Expand Up @@ -400,7 +418,7 @@ func (p *LineageTelemetry) awaitIdentity(ctx context.Context, path string, every
p.ready.Store(false)
return
}
slog.Info("lineage-telemetry: identity loaded from self_id_file; recording spans", "path", path, "self_id", id)
slog.Info("lineage-telemetry: identity loaded from self_id_file; recording spans", "path", path, "self_id", id, "namespace", p.cfg.Namespace)
return
}
if attempts++; logExportFailure(attempts) {
Expand All @@ -419,12 +437,17 @@ func readIdentityFile(path string) (string, error) {
if err != nil {
return "", err
}
if id := strings.TrimSpace(raw); id != "" {
if id := strings.TrimSpace(raw); hasIdentity(id) {
return id, nil
}
return "", fmt.Errorf("file %s carries no identity", path)
}

// hasIdentity is the one rule behind both identity sources: an identity is
// a string with at least one non-empty "/"-segment, so serviceLabel has a
// name to emit. Blank, and separator-only values such as "/", carry none.
func hasIdentity(id string) bool { return strings.Trim(id, "/") != "" }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nithasIdentity still admits whitespace-only segments. I ran it: " / / " → TrimSpace → "/ /"true, and "// //" → true. Since serviceLabel returns non-SPIFFE input as-is, those emit a whitespace self.id — the exact "entity keyed on whitespace" the Init comment above says this prevents.

The motivating / case is caught, so this is cosmetic completeness rather than a live hole:

func hasIdentity(id string) bool { return strings.Trim(id, "/ \t\n\r") != "" }


// newTracerProvider builds the provider Init installs. AlwaysSample is
// explicit and deliberate: lineage is an audit record, and under the SDK
// default ParentBased sampler a caller sending a valid traceparent with the
Expand Down Expand Up @@ -900,7 +923,14 @@ func spanKindFor(dir pipeline.Direction) trace.SpanKind {
func (p *LineageTelemetry) baseAttrs(pctx *pipeline.Context, self, protocol string) []attribute.KeyValue {
attrs := []attribute.KeyValue{
attribute.String("lineage.direction", pctx.Direction.String()),
p.capped("lineage.self.id", self),
// The two identity facts are not capped: an identity that reached the
// wire truncated would key the pod, at the consumer, on a name that is
// not its own. max_attr_bytes exists for caller-controlled values;
// both of these are operator configuration (self_id / self_id_file,
// namespace / namespace_file), and the namespace is bounded to a DNS
// label by Init besides.
attribute.String("lineage.self.id", self),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion — dropping p.capped() here changes behavior on a pre-existing fact, and nothing tests it. TestAttrBytesCapsCallerControlledValues only asserts url.path / lineage.peer.host / mcp.tool / span name, so there was no stale assertion to update — but there's also no new one, and lineage.self.namespace got TestNamespace_NeverCapped while lineage.self.id got nothing.

A self_id-flavored twin of that test (long self_id, MaxAttrBytes = 4, assert the whole value survives) would pin the half of v1.7.0 that is currently unguarded — and it's the half a future refactor is likelier to silently re-cap, since capped() is the default idiom everywhere else in baseAttrs.

attribute.String("lineage.self.namespace", p.cfg.Namespace),
attribute.String("lineage.protocol", protocol),
}
if pctx.Host != "" {
Expand Down Expand Up @@ -1062,6 +1092,44 @@ func mcpTool(pctx *pipeline.Context) string {
return ""
}

// dnsLabel is the RFC 1123 label shape a Kubernetes namespace must have —
// the same check the attach kit applies to NAMESPACE before rendering it.
var dnsLabel = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?$`)

// resolveNamespace yields the namespace fact from the config: the inline
// key first, else namespace_file — read once, synchronously. There is no
// poller, unlike self_id_file: the file this knob exists for is the one the
// kubelet projects from the pod's own metadata before any container starts,
// so an absent file is a wrong path or a missing mount, not a race.
//
// The value is trimmed of surrounding whitespace and must be an RFC 1123
// DNS label — the only shape a namespace can have. The consumer composes the
// entity key as "{kind}:{namespace}/{self.id}", so a "/" would make the key
// ambiguous, and a value longer than a label could reach the wire capped;
// both are refused here rather than emitted. Nothing is guessed: no source
// at all, an empty value, or a value of the wrong shape all refuse to start.
func resolveNamespace(cfg Config) (string, error) {
ns, source := strings.TrimSpace(cfg.Namespace), "namespace"
if ns == "" && cfg.NamespaceFile != "" {
// ReadCredentialFile already trims; absent or zero-length is its error.
raw, err := config.ReadCredentialFile(cfg.NamespaceFile)
if err != nil {
return "", fmt.Errorf("lineage-telemetry: namespace_file %q: %w", cfg.NamespaceFile, err)
}
if raw == "" {
return "", fmt.Errorf("lineage-telemetry: namespace_file %q carries no namespace", cfg.NamespaceFile)
}
ns, source = raw, "namespace_file "+cfg.NamespaceFile
}
if ns == "" {
return "", errors.New("lineage-telemetry: namespace is required (this workload's Kubernetes namespace): set namespace, or namespace_file to a file the kubelet projects")
}
if !dnsLabel.MatchString(ns) {
return "", fmt.Errorf("lineage-telemetry: %s: %q is not a DNS label (lowercase letters, digits and '-', 1-63 chars)", source, ns)
}
return ns, nil
}

// serviceLabel reduces a SPIFFE ID to its last non-empty path segment, or
// returns selfID as-is if it is not a SPIFFE URI. Used for the lineage.self.id
// fact and span names. The reduction is normative (contract §4): the consumer
Expand All @@ -1071,7 +1139,8 @@ func mcpTool(pctx *pipeline.Context) string {
// "spiffe://trust-domain/ns/team1/sa/weather-service" → "weather-service"
// "weather-service" → "weather-service"
// "spiffe://trust-domain/ns/team1/sa/agent/" → "agent" (trailing separator skipped)
// "/" → "/" (no non-empty segment: the input is returned unchanged)
// "/" → "/" (no non-empty segment: the input is returned unchanged;
// Init never admits such a value — see hasIdentity)
//
// selfID is never empty at the only call site: OnRequest runs only once ready,
// and readiness is stored only after an identity resolved (Init or its
Expand Down
Loading
Loading