From 98e5844cc7a898249f92f6b647f3e18d768eb75d Mon Sep 17 00:00:00 2001 From: Jared Jakacky Date: Mon, 8 Jun 2026 21:16:50 -0500 Subject: [PATCH] Polish Opskit contract metadata and readiness helpers --- README.md | 5 +- component.go | 26 ++++++-- component_test.go | 17 ++++- docs/api.md | 42 +++++++++--- docs/composition.md | 11 +++- docs/design.md | 33 ++++++---- docs/usage.md | 4 ++ readiness.go | 68 +++++++++++++++++++ readiness_test.go | 137 +++++++++++++++++++++++++++++++++++++++ registry.go | 10 +-- registry_helpers.go | 30 ++++++--- registry_helpers_test.go | 38 ++++++++++- registry_test.go | 84 ++++++++++++++++++++---- 13 files changed, 439 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 7dc09cd..30dfdf1 100644 --- a/README.md +++ b/README.md @@ -267,7 +267,7 @@ type Component interface { } ``` -`ComponentInfo` gives the component a stable operational identity. Names must be unique within a registry and safe for use in path-oriented operations surfaces. Kinds and attribute keys should be stable, low-cardinality safe tokens, but Opskit leaves stricter validation to presentation and telemetry layers. +`ComponentInfo` gives the component a stable operational identity. Names must be unique within a registry and safe for use in path-oriented operations surfaces. Kinds, labels, and attribute keys should be stable, low-cardinality safe tokens, but Opskit leaves stricter validation to presentation and telemetry layers. Labels are stable identity metadata for passive inventory and filtering; attributes on status, inspection, checks, and commands carry runtime or result-specific metadata. `Status` reports the current component state. It should normally be a fast cached or local snapshot. Expensive work such as dependency pings, reloads, active checks, command dispatch, remote calls, or state transitions belongs in explicit check, command, worker, or application execution paths. @@ -346,6 +346,9 @@ type CommandDescriber interface { } ``` +The handler method is intentionally named `HandleCommand` rather than `Command` +because it is the active operation on a handler, distinct from command metadata. + `CommandDescriber` lets admin surfaces, CLIs, worker runtimes, and docs generators list supported commands without invoking them. diff --git a/component.go b/component.go index 2811287..e02dd96 100644 --- a/component.go +++ b/component.go @@ -6,14 +6,21 @@ import "context" // // Name must be stable and unique within a Registry. Names must be safe single // path segments containing only ASCII letters, ASCII digits, dots, underscores, -// and hyphens. Kind should be a low-cardinality category such as "config", -// "worker_runtime", "clients", "dependencies", or "state". Kind is not -// validated by Opskit; prefer stable, safe tokens because presentation and +// and hyphens. Use ValidateComponentName or IsValidComponentName to check names +// before registration. Kind should be a low-cardinality category such as +// "config", "worker_runtime", "clients", "dependencies", or "state". Kind is +// not validated by Opskit; prefer stable, safe tokens because presentation and // telemetry layers may use it in filters, labels, dashboards, or routes. +// +// Labels are stable identity metadata for passive inventory, routing, +// filtering, dashboards, and admin presentation. Labels must be safe to expose +// anywhere ComponentInfo appears. Do not use labels for secrets, user data, +// request IDs, dynamic health details, or high-cardinality values. type ComponentInfo struct { - Name string `json:"name"` - Kind string `json:"kind,omitempty"` - Description string `json:"description,omitempty"` + Name string `json:"name"` + Kind string `json:"kind,omitempty"` + Description string `json:"description,omitempty"` + Labels []Attribute `json:"labels,omitempty"` } // Component is the minimum operational contract for something that can be @@ -103,7 +110,7 @@ type ComponentFunc struct { // ComponentInfo returns the component identity. func (c ComponentFunc) ComponentInfo() ComponentInfo { - return c.Info + return cloneComponentInfo(c.Info) } // Status returns the component status. @@ -116,3 +123,8 @@ func (c ComponentFunc) Status(ctx context.Context) Status { return c.Fn(ctx) } + +func cloneComponentInfo(info ComponentInfo) ComponentInfo { + info.Labels = cloneAttributes(info.Labels) + return info +} diff --git a/component_test.go b/component_test.go index 9d28545..381297a 100644 --- a/component_test.go +++ b/component_test.go @@ -17,9 +17,12 @@ func TestComponentInfoJSONIncludesAllFields(t *testing.T) { Name: "cache", Kind: "dependency", Description: "primary cache", + Labels: []Attribute{ + Attr("tier", "critical"), + }, } - requireJSON(t, info, `{"name":"cache","kind":"dependency","description":"primary cache"}`) + requireJSON(t, info, `{"name":"cache","kind":"dependency","description":"primary cache","labels":[{"key":"tier","value":"critical"}]}`) } func TestReadinessPolicyValues(t *testing.T) { @@ -73,6 +76,9 @@ func TestComponentEntryJSON(t *testing.T) { Component: ComponentInfo{ Name: "cache", Kind: "dependency", + Labels: []Attribute{ + Attr("tier", "critical"), + }, }, Registration: ComponentRegistration{ ReadinessPolicy: ReadinessOptional, @@ -82,7 +88,7 @@ func TestComponentEntryJSON(t *testing.T) { }, } - requireJSON(t, entry, `{"component":{"name":"cache","kind":"dependency"},"registration":{"readiness_policy":"optional"},"capabilities":{"checker":true}}`) + requireJSON(t, entry, `{"component":{"name":"cache","kind":"dependency","labels":[{"key":"tier","value":"critical"}]},"registration":{"readiness_policy":"optional"},"capabilities":{"checker":true}}`) } func TestComponentSnapshotJSONOmitsPointerViews(t *testing.T) { @@ -204,16 +210,23 @@ func TestComponentFuncComponentInfo(t *testing.T) { Info: ComponentInfo{ Name: "cache", Kind: "dependency", + Labels: []Attribute{ + Attr("tier", "critical"), + }, }, } info := component.ComponentInfo() + info.Labels[0] = Attr("tier", "mutated") if info.Name != "cache" { t.Fatalf("Name = %q, want cache", info.Name) } if info.Kind != "dependency" { t.Fatalf("Kind = %q, want dependency", info.Kind) } + if component.Info.Labels[0] != Attr("tier", "critical") { + t.Fatalf("ComponentInfo returned mutable labels, Labels = %+v", component.Info.Labels) + } } func TestComponentFuncStatus(t *testing.T) { diff --git a/docs/api.md b/docs/api.md index 21a4419..8c0dc76 100644 --- a/docs/api.md +++ b/docs/api.md @@ -111,16 +111,22 @@ operational concern. ```go type ComponentInfo struct { - Name string `json:"name"` - Kind string `json:"kind,omitempty"` - Description string `json:"description,omitempty"` + Name string `json:"name"` + Kind string `json:"kind,omitempty"` + Description string `json:"description,omitempty"` + Labels []Attribute `json:"labels,omitempty"` } + +func ValidateComponentName(name string) error +func IsValidComponentName(name string) bool ``` `Name` must be unique within a registry. It must be one safe path segment using ASCII letters, ASCII digits, dots, underscores, and hyphens. Empty names, spaces, slashes, `.` and `..`, colons, and other path-hostile characters are rejected at -registration time. +registration time. Use `ValidateComponentName` when callers need the same +sentinel errors returned by registration. Use `IsValidComponentName` when only a +boolean predicate is needed. `Kind` should be low-cardinality, such as `config`, `worker_runtime`, `dependencies`, `clients`, `state`, or `build`. Opskit does not validate @@ -129,12 +135,13 @@ underscores, and hyphens because presentation and telemetry layers may use kinds in filters, labels, dashboards, or routes. `Description` is optional human context for admin surfaces. -`ComponentInfo` does not currently include labels. Stable operational metadata -belongs on `Attribute` fields in the relevant read model, such as status, -readiness, inspection, check descriptors or results, command descriptors, -command requests or results, and future event records if Opskit later defines -an event envelope. Opskit may add identity-level labels later if sibling kits -demonstrate a concrete need before those read models are available. +`Labels` are stable identity metadata for passive inventory, routing, filtering, +dashboards, and admin presentation. Labels must be safe to expose anywhere +`ComponentInfo` appears. Do not use labels for secrets, user data, request IDs, +dynamic health details, or high-cardinality values. + +Use `Attribute` fields on status, inspection, checks, commands, and future event +records for runtime or result-specific metadata. ### `ComponentFunc` @@ -250,6 +257,7 @@ Helpers: func ReadyReadiness(reason string, components ...ReadinessItem) Readiness func NotReadyReadiness(reason string, components ...ReadinessItem) Readiness func ReadinessFromItems(reason string, items ...ReadinessItem) Readiness +func ReadinessFromPolicyItems(reason string, items ...ReadinessItem) Readiness func ReadinessFromStatus(ComponentInfo, Status) Readiness func ReadinessItemFromStatus(ComponentInfo, Status) ReadinessItem ``` @@ -258,6 +266,10 @@ The helpers defensively copy component slices. `ReadyReadiness` and `NotReadyReadiness` create explicit aggregate readiness results. `ReadinessFromItems` derives aggregate readiness from child items and is the safer helper when every child item must be ready for the aggregate to be ready. +`ReadinessFromPolicyItems` derives aggregate readiness from required child +items. Optional and informational child items are included in the result but do +not block the aggregate. Missing or unknown child item policy is treated as +`required`. `ReadinessFromStatus` produces a single-item readiness result derived from `Status.Ready`, `Status.State`, and `Status.Message`. @@ -384,6 +396,13 @@ readiness is not ready with reason `"no required readiness components registered"`, even when optional components are ready. This prevents a service from accidentally becoming ready with no required readiness contract. +Registry-level readiness policy and child item policy are separate layers. +Registration policy controls whether the registered component blocks service +readiness. `ReadinessItem.Policy` is for contributor-owned child aggregation, +such as dependency groups where some children are required and others are +optional. Contributors that need child policy should return readiness built with +`ReadinessFromPolicyItems`. + `Snapshot` returns the combined view of one component: ```go @@ -642,6 +661,9 @@ type CommandDescriber interface { type CommandHandlerFunc func(context.Context, CommandRequest) CommandResult ``` +The handler method is intentionally named `HandleCommand` rather than `Command` +because it is the active operation on a handler, distinct from command metadata. + Nil `CommandHandlerFunc` values return an unknown, rejected result instead of panicking. Nil contexts are normalized. diff --git a/docs/composition.md b/docs/composition.md index 8b1edef..e0e8dd1 100644 --- a/docs/composition.md +++ b/docs/composition.md @@ -147,9 +147,14 @@ Register components where the service is assembled. Keep component names stable. Operational consumers may put names in paths, logs, alerts, dashboards, and tests. -Keep component kinds and attribute keys stable and low-cardinality. Opskit does -not validate them because each presentation or telemetry layer may have its own -field, label, or route constraints. +Use `opskit.ValidateComponentName` or `opskit.IsValidComponentName` when sibling +kits or applications need to check component names before registration or route +exposure. + +Keep component kinds, labels, and attribute keys stable and low-cardinality. +Opskit does not validate them because each presentation or telemetry layer may +have its own field, label, or route constraints. Use labels for stable +identity-level metadata and attributes for runtime or result-specific metadata. Use required readiness sparingly but deliberately. If a component blocks serving traffic, make it required. If it is useful but non-critical, make it optional. If diff --git a/docs/design.md b/docs/design.md index 07340dc..92f0705 100644 --- a/docs/design.md +++ b/docs/design.md @@ -165,17 +165,19 @@ type Component interface { } ``` -`ComponentInfo` gives the component a stable operational identity. Identity is -intentionally limited to name, kind, and description for now. Stable operational -metadata should be represented with `Attribute` values on status, readiness, -inspection, check descriptors or results, command descriptors, command requests -or results, and future event records if Opskit later defines an event envelope. - -Opskit may add `ComponentInfo` labels later if sibling kits demonstrate a -concrete need for stable identity-level metadata before active status, -inspection, check, command, or event data is available. Until then, labels in -the broader Kit Series architecture are safe operational attributes attached to -the relevant read model, not part of the `ComponentInfo` compatibility contract. +`ComponentInfo` gives the component a stable operational identity. Identity +includes name, kind, description, and optional labels. + +Labels are stable identity metadata for passive inventory, routing, filtering, +dashboards, and admin presentation. Labels must be safe to expose anywhere +`ComponentInfo` appears. They are not runtime state. Do not use labels for +secrets, user data, request IDs, dynamic health details, or high-cardinality +values. + +Runtime or result-specific metadata should be represented with `Attribute` +values on status, inspection, check descriptors or results, command descriptors, +command requests or results, and future event records if Opskit later defines +an event envelope. `Status` reports the component's current local state. Status should be cheap, descriptive, and safe to expose. @@ -383,7 +385,7 @@ sequenceDiagram end Registry->>Registry: fill missing child item policy from registration policy - Registry->>Registry: required items decide aggregate Ready + Registry->>Registry: registration policy decides whether component blocks aggregate Ready end end @@ -398,6 +400,13 @@ When a readiness contributor returns child readiness items, Opskit preserves any explicit child item policy. If a child item omits policy, Opskit fills it from the component's registration policy. +Contributor-owned child item policy is separate from registry-level registration +policy. Registration policy decides whether a registered component blocks +service readiness. Child item policy helps a contributor compute its own +aggregate readiness when it represents a group, such as clients or dependencies +where some children are required and others are optional. Use +`ReadinessFromPolicyItems` for that contributor-side aggregation. + ## Inspection Is Safe Diagnostic Detail Status and readiness should stay compact. Inspection is for richer operational diff --git a/docs/usage.md b/docs/usage.md index 5e400a2..356e8fa 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -74,6 +74,10 @@ names include `config`, `worker_runtime`, `cache.primary`, and `WorkerA`. Invalid names include empty strings, names with spaces, names with slashes, `.`, `..`, and path-hostile punctuation. +Use `opskit.ValidateComponentName` when code needs the same sentinel errors +returned by registration. Use `opskit.IsValidComponentName` when only a boolean +predicate is needed. + Component kinds and attribute keys should also be stable, low-cardinality safe tokens, but Opskit does not validate them. Presentation and telemetry layers may apply their own field, label, route, or dashboard constraints. diff --git a/readiness.go b/readiness.go index da887cc..8fe2322 100644 --- a/readiness.go +++ b/readiness.go @@ -87,6 +87,60 @@ func ReadinessFromItems(reason string, items ...ReadinessItem) Readiness { } } +// ReadinessFromPolicyItems builds a readiness result whose aggregate readiness +// is derived from required readiness items. +func ReadinessFromPolicyItems(reason string, items ...ReadinessItem) Readiness { + components := normalizePolicyReadinessItems(items) + + if len(components) == 0 { + if reason == "" { + reason = "no readiness items" + } + return Readiness{ + Ready: false, + Reason: reason, + } + } + + required := 0 + ready := true + for _, item := range components { + if !blocksReadiness(item.Policy) { + continue + } + + required++ + if !item.Ready { + ready = false + } + } + + if required == 0 { + if reason == "" { + reason = "no required readiness items" + } + return Readiness{ + Ready: false, + Reason: reason, + Components: components, + } + } + + if reason == "" { + if ready { + reason = "all required readiness items ready" + } else { + reason = "one or more required readiness items are not ready" + } + } + + return Readiness{ + Ready: ready, + Reason: reason, + Components: components, + } +} + // ReadinessFromStatus builds a readiness result from component status. func ReadinessFromStatus(info ComponentInfo, status Status) Readiness { reason := "component ready" @@ -136,3 +190,17 @@ func normalizeReadinessItems(items []ReadinessItem) []ReadinessItem { } return cloned } + +func normalizePolicyReadinessItems(items []ReadinessItem) []ReadinessItem { + if len(items) == 0 { + return nil + } + + cloned := make([]ReadinessItem, len(items)) + for i, item := range items { + item.Policy = normalizeReadinessPolicy(item.Policy) + item.State = normalizeReadinessItemState(item.Ready, item.State) + cloned[i] = item + } + return cloned +} diff --git a/readiness_test.go b/readiness_test.go index ce18787..8e7f84c 100644 --- a/readiness_test.go +++ b/readiness_test.go @@ -123,6 +123,143 @@ func TestReadinessFromItemsPreservesReason(t *testing.T) { } } +func TestReadinessFromPolicyItemsWithNoItems(t *testing.T) { + readiness := ReadinessFromPolicyItems("") + + if readiness.Ready { + t.Fatal("Ready = true, want false") + } + if readiness.Reason != "no readiness items" { + t.Fatalf("Reason = %q, want no readiness items", readiness.Reason) + } + if readiness.Components != nil { + t.Fatalf("Components = %+v, want nil", readiness.Components) + } +} + +func TestReadinessFromPolicyItemsWithNoItemsPreservesReason(t *testing.T) { + readiness := ReadinessFromPolicyItems("custom reason") + + if readiness.Ready { + t.Fatal("Ready = true, want false") + } + if readiness.Reason != "custom reason" { + t.Fatalf("Reason = %q, want custom reason", readiness.Reason) + } +} + +func TestReadinessFromPolicyItemsAllRequiredReady(t *testing.T) { + items := []ReadinessItem{ + {Name: "database", Policy: ReadinessRequired, Ready: true}, + {Name: "cache", Ready: true, State: StateDegraded}, + } + + readiness := ReadinessFromPolicyItems("", items...) + items[0].Name = "mutated" + + if !readiness.Ready { + t.Fatal("Ready = false, want true") + } + if readiness.Reason != "all required readiness items ready" { + t.Fatalf("Reason = %q, want all required readiness items ready", readiness.Reason) + } + if readiness.Components[0].Name != "database" { + t.Fatalf("Components[0].Name = %q, want database", readiness.Components[0].Name) + } + if readiness.Components[0].State != StateReady { + t.Fatalf("Components[0].State = %q, want %q", readiness.Components[0].State, StateReady) + } + if readiness.Components[1].Policy != ReadinessRequired { + t.Fatalf("Components[1].Policy = %q, want %q", readiness.Components[1].Policy, ReadinessRequired) + } + if readiness.Components[1].State != StateDegraded { + t.Fatalf("Components[1].State = %q, want %q", readiness.Components[1].State, StateDegraded) + } +} + +func TestReadinessFromPolicyItemsRequiredNotReadyBlocks(t *testing.T) { + readiness := ReadinessFromPolicyItems("", ReadinessItem{ + Name: "database", + Policy: ReadinessRequired, + Ready: false, + }) + + if readiness.Ready { + t.Fatal("Ready = true, want false") + } + if readiness.Reason != "one or more required readiness items are not ready" { + t.Fatalf("Reason = %q, want one or more required readiness items are not ready", readiness.Reason) + } + if readiness.Components[0].State != StateNotReady { + t.Fatalf("Components[0].State = %q, want %q", readiness.Components[0].State, StateNotReady) + } +} + +func TestReadinessFromPolicyItemsOptionalAndInformationalDoNotBlock(t *testing.T) { + readiness := ReadinessFromPolicyItems("", + ReadinessItem{Name: "database", Policy: ReadinessRequired, Ready: true}, + ReadinessItem{Name: "cache", Policy: ReadinessOptional, Ready: false}, + ReadinessItem{Name: "build", Policy: ReadinessInformational, Ready: false}, + ) + + if !readiness.Ready { + t.Fatal("Ready = false, want true") + } + if len(readiness.Components) != 3 { + t.Fatalf("Components length = %d, want 3", len(readiness.Components)) + } + if readiness.Components[1].Policy != ReadinessOptional { + t.Fatalf("Components[1].Policy = %q, want %q", readiness.Components[1].Policy, ReadinessOptional) + } + if readiness.Components[2].Policy != ReadinessInformational { + t.Fatalf("Components[2].Policy = %q, want %q", readiness.Components[2].Policy, ReadinessInformational) + } +} + +func TestReadinessFromPolicyItemsWithoutRequiredItems(t *testing.T) { + readiness := ReadinessFromPolicyItems("", + ReadinessItem{Name: "cache", Policy: ReadinessOptional, Ready: true}, + ReadinessItem{Name: "build", Policy: ReadinessInformational, Ready: true}, + ) + + if readiness.Ready { + t.Fatal("Ready = true, want false") + } + if readiness.Reason != "no required readiness items" { + t.Fatalf("Reason = %q, want no required readiness items", readiness.Reason) + } + if len(readiness.Components) != 2 { + t.Fatalf("Components length = %d, want 2", len(readiness.Components)) + } +} + +func TestReadinessFromPolicyItemsDefaultsUnknownPolicyToRequired(t *testing.T) { + readiness := ReadinessFromPolicyItems("", ReadinessItem{ + Name: "database", + Policy: ReadinessPolicy("unknown"), + Ready: true, + }) + + if !readiness.Ready { + t.Fatal("Ready = false, want true") + } + if readiness.Components[0].Policy != ReadinessRequired { + t.Fatalf("Components[0].Policy = %q, want %q", readiness.Components[0].Policy, ReadinessRequired) + } +} + +func TestReadinessFromPolicyItemsPreservesReason(t *testing.T) { + ready := ReadinessFromPolicyItems("custom ready", ReadinessItem{Name: "database", Ready: true}) + if ready.Reason != "custom ready" { + t.Fatalf("ready.Reason = %q, want custom ready", ready.Reason) + } + + notReady := ReadinessFromPolicyItems("custom not ready", ReadinessItem{Name: "database", Ready: false}) + if notReady.Reason != "custom not ready" { + t.Fatalf("notReady.Reason = %q, want custom not ready", notReady.Reason) + } +} + func TestReadinessFromStatusReady(t *testing.T) { readiness := ReadinessFromStatus( ComponentInfo{Name: "component", Kind: "test"}, diff --git a/registry.go b/registry.go index 4e3825a..1235167 100644 --- a/registry.go +++ b/registry.go @@ -2,7 +2,6 @@ package opskit import ( "context" - "strings" "sync" ) @@ -81,12 +80,9 @@ func (r *Registry) Register(component Component, opts ...RegisterOption) error { return ErrNilComponent } - info := component.ComponentInfo() - if strings.TrimSpace(info.Name) == "" { - return ErrEmptyComponentName - } - if !isValidComponentName(info.Name) { - return ErrInvalidComponentName + info := cloneComponentInfo(component.ComponentInfo()) + if err := ValidateComponentName(info.Name); err != nil { + return err } reg := registration{ diff --git a/registry_helpers.go b/registry_helpers.go index dca00ec..c9838c5 100644 --- a/registry_helpers.go +++ b/registry_helpers.go @@ -16,6 +16,7 @@ func (r *Registry) registration(name string) (registration, bool) { defer r.mu.RUnlock() reg, ok := r.registrations[name] + reg = cloneRegistration(reg) return reg, ok } @@ -25,29 +26,36 @@ func (r *Registry) snapshot() []registration { registrations := make([]registration, 0, len(r.order)) for _, name := range r.order { - registrations = append(registrations, r.registrations[name]) + registrations = append(registrations, cloneRegistration(r.registrations[name])) } return registrations } +func cloneRegistration(reg registration) registration { + reg.info = cloneComponentInfo(reg.info) + return reg +} + func (r *Registry) ensureInitializedLocked() { if r.registrations == nil { r.registrations = make(map[string]registration) } } -func isValidComponentName(name string) bool { - if name == "" { - return false +// ValidateComponentName verifies that name is a stable, path-safe component +// name. +func ValidateComponentName(name string) error { + if strings.TrimSpace(name) == "" { + return ErrEmptyComponentName } if name != strings.TrimSpace(name) { - return false + return ErrInvalidComponentName } if name == "." || name == ".." { - return false + return ErrInvalidComponentName } for _, ch := range name { @@ -57,11 +65,17 @@ func isValidComponentName(name string) bool { case ch >= '0' && ch <= '9': case ch == '.', ch == '_', ch == '-': default: - return false + return ErrInvalidComponentName } } - return true + return nil +} + +// IsValidComponentName reports whether name is a stable, path-safe component +// name. +func IsValidComponentName(name string) bool { + return ValidateComponentName(name) == nil } func capabilitiesOf(component Component) ComponentCapabilities { diff --git a/registry_helpers_test.go b/registry_helpers_test.go index 91bb69a..96938d6 100644 --- a/registry_helpers_test.go +++ b/registry_helpers_test.go @@ -96,8 +96,42 @@ func TestIsValidComponentName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := isValidComponentName(tt.name); got != tt.want { - t.Fatalf("isValidComponentName(%q) = %t, want %t", tt.name, got, tt.want) + if got := IsValidComponentName(tt.name); got != tt.want { + t.Fatalf("IsValidComponentName(%q) = %t, want %t", tt.name, got, tt.want) + } + }) + } +} + +func TestValidateComponentName(t *testing.T) { + tests := []struct { + name string + want error + }{ + {name: "", want: ErrEmptyComponentName}, + {name: " ", want: ErrEmptyComponentName}, + {name: "component", want: nil}, + {name: "Component_1.2-alpha", want: nil}, + {name: "cache.primary_1-alpha", want: nil}, + {name: " component", want: ErrInvalidComponentName}, + {name: "component ", want: ErrInvalidComponentName}, + {name: "component\n", want: ErrInvalidComponentName}, + {name: "component\tname", want: ErrInvalidComponentName}, + {name: ".", want: ErrInvalidComponentName}, + {name: "..", want: ErrInvalidComponentName}, + {name: "component/name", want: ErrInvalidComponentName}, + {name: "component name", want: ErrInvalidComponentName}, + {name: "component:name", want: ErrInvalidComponentName}, + {name: "component@name", want: ErrInvalidComponentName}, + {name: "component\x00name", want: ErrInvalidComponentName}, + {name: "\u2003component", want: ErrInvalidComponentName}, + {name: "café", want: ErrInvalidComponentName}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := ValidateComponentName(tt.name); err != tt.want { + t.Fatalf("ValidateComponentName(%q) error = %v, want %v", tt.name, err, tt.want) } }) } diff --git a/registry_test.go b/registry_test.go index f5ae6ef..9c4b8ba 100644 --- a/registry_test.go +++ b/registry_test.go @@ -60,11 +60,23 @@ func TestRegistryEntries(t *testing.T) { var registry Registry first := testComponent{ - info: ComponentInfo{Name: "first", Kind: "test"}, + info: ComponentInfo{ + Name: "first", + Kind: "test", + Labels: []Attribute{ + Attr("tier", "critical"), + }, + }, status: ReadyStatus("first ready"), } second := &testOperationalComponent{ - info: ComponentInfo{Name: "second", Kind: "operational"}, + info: ComponentInfo{ + Name: "second", + Kind: "operational", + Labels: []Attribute{ + Attr("scope", "admin"), + }, + }, status: ReadyStatus("second ready"), } @@ -80,18 +92,14 @@ func TestRegistryEntries(t *testing.T) { if len(entries) != 2 { t.Fatalf("Entries length = %d, want 2", len(entries)) } - if entries[0].Component != first.info { - t.Fatalf("Entries[0].Component = %+v, want %+v", entries[0].Component, first.info) - } + requireComponentInfo(t, entries[0].Component, first.info) if entries[0].Registration.ReadinessPolicy != ReadinessRequired { t.Fatalf("Entries[0].Registration.ReadinessPolicy = %q, want %q", entries[0].Registration.ReadinessPolicy, ReadinessRequired) } if entries[0].Capabilities != (ComponentCapabilities{}) { t.Fatalf("Entries[0].Capabilities = %+v, want none", entries[0].Capabilities) } - if entries[1].Component != second.info { - t.Fatalf("Entries[1].Component = %+v, want %+v", entries[1].Component, second.info) - } + requireComponentInfo(t, entries[1].Component, second.info) if entries[1].Registration.ReadinessPolicy != ReadinessOptional { t.Fatalf("Entries[1].Registration.ReadinessPolicy = %q, want %q", entries[1].Registration.ReadinessPolicy, ReadinessOptional) } @@ -106,10 +114,14 @@ func TestRegistryEntries(t *testing.T) { } entries[0].Component.Name = "mutated" + entries[0].Component.Labels[0] = Attr("tier", "mutated") entries = registry.Entries() if entries[0].Component.Name != "first" { t.Fatalf("Entries returned mutable registry slice, first name = %q", entries[0].Component.Name) } + if entries[0].Component.Labels[0] != Attr("tier", "critical") { + t.Fatalf("Entries returned mutable component labels, labels = %+v", entries[0].Component.Labels) + } } func TestRegistryEntriesEmpty(t *testing.T) { @@ -1206,6 +1218,9 @@ func TestRegistryReadModelsUseRegisteredComponentInfo(t *testing.T) { Name: "component", Kind: "test", Description: "registered description", + Labels: []Attribute{ + Attr("tier", "critical"), + }, }, status: ReadyStatus("ready"), } @@ -1219,15 +1234,26 @@ func TestRegistryReadModelsUseRegisteredComponentInfo(t *testing.T) { Name: "mutated component", Kind: "mutated", Description: "mutated description", + Labels: []Attribute{ + Attr("tier", "mutated"), + }, + } + + wantInfo := ComponentInfo{ + Name: "component", + Kind: "test", + Description: "registered description", + Labels: []Attribute{ + Attr("tier", "critical"), + }, } status := registry.Status(ctx) if len(status.Components) != 1 { t.Fatalf("Status.Components length = %d, want 1", len(status.Components)) } - if got := status.Components[0].Component; got != (ComponentInfo{Name: "component", Kind: "test", Description: "registered description"}) { - t.Fatalf("Status.Component = %+v, want registered component info", got) - } + requireComponentInfo(t, status.Components[0].Component, wantInfo) + status.Components[0].Component.Labels[0] = Attr("tier", "mutated") readiness := registry.Readiness(ctx) if len(readiness.Components) != 1 { @@ -1244,9 +1270,8 @@ func TestRegistryReadModelsUseRegisteredComponentInfo(t *testing.T) { if err != nil { t.Fatalf("Snapshot error = %v", err) } - if got := snapshot.Component; got != (ComponentInfo{Name: "component", Kind: "test", Description: "registered description"}) { - t.Fatalf("Snapshot.Component = %+v, want registered component info", got) - } + requireComponentInfo(t, snapshot.Component, wantInfo) + snapshot.Component.Labels[0] = Attr("tier", "mutated") if snapshot.Readiness == nil { t.Fatal("Snapshot.Readiness is nil, want readiness") } @@ -1256,6 +1281,37 @@ func TestRegistryReadModelsUseRegisteredComponentInfo(t *testing.T) { if got := snapshot.Readiness.Components[0].Kind; got != "test" { t.Fatalf("Snapshot.Readiness.Components[0].Kind = %q, want test", got) } + + nextStatus := registry.Status(ctx) + requireComponentInfo(t, nextStatus.Components[0].Component, wantInfo) + + nextSnapshot, err := registry.Snapshot(ctx, "component") + if err != nil { + t.Fatalf("Snapshot after mutation error = %v", err) + } + requireComponentInfo(t, nextSnapshot.Component, wantInfo) +} + +func requireComponentInfo(t *testing.T, got, want ComponentInfo) { + t.Helper() + + if got.Name != want.Name { + t.Fatalf("ComponentInfo.Name = %q, want %q", got.Name, want.Name) + } + if got.Kind != want.Kind { + t.Fatalf("ComponentInfo.Kind = %q, want %q", got.Kind, want.Kind) + } + if got.Description != want.Description { + t.Fatalf("ComponentInfo.Description = %q, want %q", got.Description, want.Description) + } + if len(got.Labels) != len(want.Labels) { + t.Fatalf("ComponentInfo.Labels length = %d, want %d", len(got.Labels), len(want.Labels)) + } + for i := range want.Labels { + if got.Labels[i] != want.Labels[i] { + t.Fatalf("ComponentInfo.Labels[%d] = %+v, want %+v", i, got.Labels[i], want.Labels[i]) + } + } } type testComponent struct {