From 92bcc7e648025f1b2b3563382abf2f2e50ffdd89 Mon Sep 17 00:00:00 2001 From: tithakka Date: Tue, 30 Jun 2026 22:27:00 -0500 Subject: [PATCH] HYPERFLEET-1158 - feat: config-driven entity registration via LoadDescriptors --- CLAUDE.md | 11 +- charts/CLAUDE.md | 6 + charts/README.md | 3 +- charts/templates/configmap.yaml | 44 +++++ charts/values.schema.json | 67 +++++++ charts/values.yaml | 19 ++ cmd/hyperfleet-api/main.go | 4 +- cmd/hyperfleet-api/servecmd/cmd.go | 8 + configs/config.yaml.example | 22 +++ pkg/config/config.go | 15 +- pkg/config/dump.go | 14 ++ pkg/config/loader.go | 3 + pkg/registry/descriptor.go | 39 +++- pkg/registry/registry.go | 69 ++++++- pkg/registry/registry_test.go | 245 ++++++++++++++++++++++++ pkg/validators/schema_validator.go | 25 +-- pkg/validators/schema_validator_test.go | 54 ++---- plugins/CLAUDE.md | 34 ++-- plugins/channels/plugin.go | 44 ----- plugins/entities/plugin.go | 58 ++++++ plugins/entities/plugin_test.go | 88 +++++++++ plugins/versions/plugin.go | 47 ----- plugins/wifconfigs/plugin.go | 44 ----- test/helper.go | 37 ++++ test/integration/versions_test.go | 3 - test/integration/wifconfigs_test.go | 2 - 26 files changed, 772 insertions(+), 233 deletions(-) delete mode 100644 plugins/channels/plugin.go create mode 100644 plugins/entities/plugin.go create mode 100644 plugins/entities/plugin_test.go delete mode 100644 plugins/versions/plugin.go delete mode 100644 plugins/wifconfigs/plugin.go diff --git a/CLAUDE.md b/CLAUDE.md index d0a9cb46..ab94f7e7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,11 @@ Interface + `sql*Dao` implementation using SessionFactory: - On write errors: call `db.MarkForRollback(ctx, err)` ### Plugin Registration -Each resource registers via `init()` function: +Generic entity types (Channel, Version, WifConfig) are config-driven — declared in `config.yaml` under `entities:`, +registered at startup via `registry.LoadDescriptors()`, routes auto-generated by `plugins/entities/plugin.go`. +No per-entity Go code needed. See `plugins/CLAUDE.md` for details. + +Cluster and NodePool use legacy typed plugins with `init()` registration: - Reference: `plugins/clusters/plugin.go` - `registry.RegisterService()`, `server.RegisterRoutes()`, `presenters.RegisterPath()`, `presenters.RegisterKind()` @@ -102,7 +106,8 @@ Each resource registers via `init()` function: - Read requests (GET) skip transaction creation for performance - OpenAPI spec and code generation: see [openapi/README.md](openapi/README.md) — run `make generate` before building; generated files in `pkg/api/openapi/` are **never edited** - Status aggregation: Service layer synthesizes `Available`, `Reconciled`, and `LastKnownReconciled` conditions from adapter reports -- Plugin-based: each resource type registers routes/services in `plugins/*/plugin.go` +- Generic entities are config-driven (`config.yaml` → `registry.LoadDescriptors` → auto-generated routes) +- Cluster/NodePool use legacy typed plugins in `plugins/*/plugin.go` ## Boundaries @@ -122,7 +127,7 @@ Subdirectories contain context-specific guidance that loads when you work in tho - `pkg/dao/CLAUDE.md` — DAO interface, session access, and rollback patterns - `pkg/db/CLAUDE.md` — SessionFactory and transaction middleware - `pkg/errors/CLAUDE.md` — Error constructors, codes, and RFC 9457 details -- `plugins/CLAUDE.md` — Plugin registration (init-based) +- `plugins/CLAUDE.md` — Plugin registration (config-driven entities + legacy init-based) - `test/CLAUDE.md` — Test conventions, factories, and environment variables - `charts/CLAUDE.md` — Helm chart testing and configuration - `openapi/README.md` — OpenAPI schema import, code generation, schema validation, and oapi-codegen config diff --git a/charts/CLAUDE.md b/charts/CLAUDE.md index 6eadc27e..730c7ab1 100644 --- a/charts/CLAUDE.md +++ b/charts/CLAUDE.md @@ -14,6 +14,12 @@ Adapter arrays are required — templates fail without them: --set 'adapters.nodepool=["validation"]' ``` +## Entity Registration + +Entity descriptors are configured in `config.entities`. Default values include Channel, Version, and WifConfig. +Override with `--set-json 'config.entities=[...]'` for custom entity sets. An empty list disables all +generic entity routes. + ## Database Modes Two database configurations supported: diff --git a/charts/README.md b/charts/README.md index 0567b239..24782236 100644 --- a/charts/README.md +++ b/charts/README.md @@ -44,7 +44,7 @@ helm install hyperfleet-api oci://REGISTRY/hyperfleet-api \ | ports.api | int | `8000` | API server port | | ports.health | int | `8080` | Health check endpoint port | | ports.metrics | int | `9090` | Prometheus metrics endpoint port | -| config | object | `{"adapters":{"required":{"cluster":[],"nodepool":[]}},"database":{"debug":false,"dialect":"postgres","host":"","name":"hyperfleet","pool":{"conn_max_idle_time":"1m","conn_max_lifetime":"5m","conn_retry_attempts":10,"conn_retry_interval":"3s","max_connections":50,"max_idle_connections":10,"request_timeout":"30s"},"port":5432,"ssl":{"mode":"disable","root_cert_file":""}},"existingConfigMap":"","health":{"db_ping_timeout":"2s","host":"0.0.0.0","port":8080,"shutdown_timeout":"20s","tls":{"enabled":false}},"logging":{"format":"json","level":"info","masking":{"enabled":true,"fields":["password","secret","token","api_key","access_token","refresh_token","client_secret"],"headers":["Authorization","X-API-Key","Cookie","X-Auth-Token","X-Forwarded-Authorization","X-HyperFleet-Identity"]},"otel":{"enabled":false},"output":"stdout"},"metrics":{"host":"0.0.0.0","label_metrics_inclusion_duration":"168h","port":9090,"reconciliation_stuck_threshold":"10m","tls":{"enabled":false}},"server":{"host":"0.0.0.0","hostname":"","identity_header":"","jwk":{"cert_file":"","cert_url":""},"jwt":{"audience":"","enabled":false,"identity_claim":"email","issuer_url":""},"port":8000,"timeouts":{"read":"5s","write":"30s"},"tls":{"cert_file":"","enabled":false,"key_file":""}}}` | Application configuration. All settings in this section generate the ConfigMap consumed by the API server. Set `config.existingConfigMap` to use a pre-existing ConfigMap instead. | +| config | object | `{"adapters":{"required":{"cluster":[],"nodepool":[]}},"database":{"debug":false,"dialect":"postgres","host":"","name":"hyperfleet","pool":{"conn_max_idle_time":"1m","conn_max_lifetime":"5m","conn_retry_attempts":10,"conn_retry_interval":"3s","max_connections":50,"max_idle_connections":10,"request_timeout":"30s"},"port":5432,"ssl":{"mode":"disable","root_cert_file":""}},"entities":[{"kind":"Channel","plural":"channels","search_disallowed_fields":["spec"],"spec_schema_name":"ChannelSpec"},{"kind":"Version","on_parent_delete":"restrict","parent_kind":"Channel","plural":"versions","search_disallowed_fields":["spec"],"spec_schema_name":"VersionSpec"},{"kind":"WifConfig","plural":"wifconfigs","search_disallowed_fields":["spec"],"spec_schema_name":"WifConfigSpec"}],"existingConfigMap":"","health":{"db_ping_timeout":"2s","host":"0.0.0.0","port":8080,"shutdown_timeout":"20s","tls":{"enabled":false}},"logging":{"format":"json","level":"info","masking":{"enabled":true,"fields":["password","secret","token","api_key","access_token","refresh_token","client_secret"],"headers":["Authorization","X-API-Key","Cookie","X-Auth-Token","X-Forwarded-Authorization","X-HyperFleet-Identity"]},"otel":{"enabled":false},"output":"stdout"},"metrics":{"host":"0.0.0.0","label_metrics_inclusion_duration":"168h","port":9090,"reconciliation_stuck_threshold":"10m","tls":{"enabled":false}},"server":{"host":"0.0.0.0","hostname":"","identity_header":"","jwk":{"cert_file":"","cert_url":""},"jwt":{"audience":"","enabled":false,"identity_claim":"email","issuer_url":""},"port":8000,"timeouts":{"read":"5s","write":"30s"},"tls":{"cert_file":"","enabled":false,"key_file":""}}}` | Application configuration. All settings in this section generate the ConfigMap consumed by the API server. Set `config.existingConfigMap` to use a pre-existing ConfigMap instead. | | config.existingConfigMap | string | `""` | Use an existing ConfigMap instead of generating one. When set, all other `config.*` values are ignored. | | config.server | object | `{"host":"0.0.0.0","hostname":"","identity_header":"","jwk":{"cert_file":"","cert_url":""},"jwt":{"audience":"","enabled":false,"identity_claim":"email","issuer_url":""},"port":8000,"timeouts":{"read":"5s","write":"30s"},"tls":{"cert_file":"","enabled":false,"key_file":""}}` | HTTP server settings | | config.server.hostname | string | `""` | Public hostname advertised by the API (leave empty for auto-detect) | @@ -111,6 +111,7 @@ helm install hyperfleet-api oci://REGISTRY/hyperfleet-api \ | config.adapters.required | object | `{"cluster":[],"nodepool":[]}` | Adapters required for cluster resources | | config.adapters.required.cluster | list | `[]` | Required cluster adapters (e.g. `["validation", "dns", "pullsecret", "hypershift"]`) | | config.adapters.required.nodepool | list | `[]` | Required nodepool adapters (e.g. `["validation", "hypershift"]`) | +| config.entities | list | `[{"kind":"Channel","plural":"channels","search_disallowed_fields":["spec"],"spec_schema_name":"ChannelSpec"},{"kind":"Version","on_parent_delete":"restrict","parent_kind":"Channel","plural":"versions","search_disallowed_fields":["spec"],"spec_schema_name":"VersionSpec"},{"kind":"WifConfig","plural":"wifconfigs","search_disallowed_fields":["spec"],"spec_schema_name":"WifConfigSpec"}]` | Entity descriptors registered at startup. Each entry auto-generates REST endpoints, spec validation, and delete policies. Cluster and NodePool use legacy typed handlers and are NOT listed here. | | serviceAccount | object | `{"annotations":{},"create":true,"name":""}` | ServiceAccount configuration | | serviceAccount.create | bool | `true` | Create a ServiceAccount for the API server | | serviceAccount.annotations | object | `{}` | Annotations added to the ServiceAccount (e.g. for Workload Identity) | diff --git a/charts/templates/configmap.yaml b/charts/templates/configmap.yaml index 7b18ce13..0b326666 100644 --- a/charts/templates/configmap.yaml +++ b/charts/templates/configmap.yaml @@ -136,4 +136,48 @@ data: {{- else }} [] {{- end }} + + # NOTE: When EntityDescriptor gains many more fields, consider replacing + # the explicit range below with: {{ .Values.config.entities | toYaml | indent 6 }} +{{- if .Values.config.entities }} + entities: +{{- range .Values.config.entities }} + - kind: {{ .kind }} + plural: {{ .plural }} +{{- if .parent_kind }} + parent_kind: {{ .parent_kind }} +{{- end }} +{{- if .on_parent_delete }} + on_parent_delete: {{ .on_parent_delete }} +{{- end }} + spec_schema_name: {{ .spec_schema_name }} +{{- if .search_disallowed_fields }} + search_disallowed_fields: +{{- range .search_disallowed_fields }} + - {{ . }} +{{- end }} +{{- end }} +{{- if .required_adapters }} + required_adapters: +{{- range .required_adapters }} + - {{ . }} +{{- end }} +{{- end }} +{{- if .references }} + references: +{{- range .references }} + - ref_type: {{ .ref_type }} + target_kind: {{ .target_kind }} +{{- if .min }} + min: {{ .min }} +{{- end }} +{{- if .max }} + max: {{ .max }} +{{- end }} +{{- end }} +{{- end }} +{{- end }} +{{- else }} + entities: [] +{{- end }} {{- end }} diff --git a/charts/values.schema.json b/charts/values.schema.json index e5c0c1b1..1e8fbe35 100644 --- a/charts/values.schema.json +++ b/charts/values.schema.json @@ -414,6 +414,73 @@ } } } + }, + "entities": { + "type": "array", + "description": "Entity descriptors registered at startup. Each entry auto-generates REST endpoints, spec validation, and delete policies.", + "items": { + "type": "object", + "required": ["kind", "plural"], + "properties": { + "kind": { + "type": "string", + "description": "Discriminator value stored in Resource.Kind (e.g. Channel, Version)" + }, + "plural": { + "type": "string", + "description": "URL path segment for this entity (e.g. channels, versions)" + }, + "parent_kind": { + "type": "string", + "description": "Kind of the parent entity (empty for top-level entities)" + }, + "on_parent_delete": { + "type": "string", + "enum": ["restrict", "cascade"], + "description": "Child behavior when parent is deleted (restrict or cascade)" + }, + "spec_schema_name": { + "type": "string", + "description": "OpenAPI component name for spec field validation (e.g. ChannelSpec)" + }, + "search_disallowed_fields": { + "type": "array", + "items": { "type": "string" }, + "description": "Fields blocked from TSL search queries" + }, + "required_adapters": { + "type": "array", + "items": { "type": "string" }, + "description": "Adapters that must finalize before hard-delete" + }, + "references": { + "type": "array", + "description": "Non-ownership associations to other entity types (HYPERFLEET-1156)", + "items": { + "type": "object", + "required": ["ref_type", "target_kind"], + "properties": { + "ref_type": { + "type": "string", + "description": "Key in the references map (e.g. wif_config)" + }, + "target_kind": { + "type": "string", + "description": "Kind of the referenced entity (e.g. WifConfig)" + }, + "min": { + "type": "integer", + "description": "Minimum references of this type (0 = optional)" + }, + "max": { + "type": "integer", + "description": "Maximum references of this type (0 = unlimited)" + } + } + } + } + } + } } } }, diff --git a/charts/values.yaml b/charts/values.yaml index 45b53c95..64cf1697 100644 --- a/charts/values.yaml +++ b/charts/values.yaml @@ -208,6 +208,25 @@ config: # -- Required nodepool adapters (e.g. `["validation", "hypershift"]`) nodepool: [] + # -- Entity descriptors registered at startup. Each entry auto-generates + # REST endpoints, spec validation, and delete policies. + # Cluster and NodePool use legacy typed handlers and are NOT listed here. + entities: + - kind: Channel + plural: channels + spec_schema_name: ChannelSpec + search_disallowed_fields: [spec] + - kind: Version + plural: versions + parent_kind: Channel + on_parent_delete: restrict + spec_schema_name: VersionSpec + search_disallowed_fields: [spec] + - kind: WifConfig + plural: wifconfigs + spec_schema_name: WifConfigSpec + search_disallowed_fields: [spec] + # -- ServiceAccount configuration serviceAccount: # -- Create a ServiceAccount for the API server diff --git a/cmd/hyperfleet-api/main.go b/cmd/hyperfleet-api/main.go index 1b342d99..16b24792 100755 --- a/cmd/hyperfleet-api/main.go +++ b/cmd/hyperfleet-api/main.go @@ -17,13 +17,11 @@ import ( // Import plugins to trigger their init() functions // _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/events" // REMOVED: Events plugin no longer exists _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/adapterStatus" - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/channels" _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/clusters" + _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/entities" _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/generic" _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/nodePools" _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/versions" - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/wifconfigs" ) // nolint diff --git a/cmd/hyperfleet-api/servecmd/cmd.go b/cmd/hyperfleet-api/servecmd/cmd.go index 6f8df5c6..7eacc74e 100755 --- a/cmd/hyperfleet-api/servecmd/cmd.go +++ b/cmd/hyperfleet-api/servecmd/cmd.go @@ -19,6 +19,7 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/health" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/metrics" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/telemetry" ) @@ -55,6 +56,13 @@ func runServe(cmd *cobra.Command, args []string) { // and ensure SessionFactory, clients, services, handlers all use the correct config environments.Environment().Config = cfg + // Load entity descriptors from config before services and routes are built. + // Descriptors must be registered before Initialize() because services call + // registry.MustGet() at construction time. + registry.LoadDescriptors(cfg.Entities) + registry.Validate() + registry.ValidateSchemas(cfg.Server.OpenAPISchemaPath) + // Initialize environment (applies overrides, creates SessionFactory, loads clients, services, handlers) err = environments.Environment().Initialize() if err != nil { diff --git a/configs/config.yaml.example b/configs/config.yaml.example index a1266703..7d5f80c6 100644 --- a/configs/config.yaml.example +++ b/configs/config.yaml.example @@ -115,6 +115,28 @@ adapters: - validation - hypershift +# Entity Registration +# Generic resource types registered at startup. Each entry auto-generates +# REST endpoints, spec validation, and delete policies. +# Cluster and NodePool use legacy typed handlers and are NOT listed here. +entities: + - kind: Channel + plural: channels + spec_schema_name: ChannelSpec + search_disallowed_fields: [spec] + + - kind: Version + plural: versions + parent_kind: Channel + on_parent_delete: restrict + spec_schema_name: VersionSpec + search_disallowed_fields: [spec] + + - kind: WifConfig + plural: wifconfigs + spec_schema_name: WifConfigSpec + search_disallowed_fields: [spec] + # ---------------------------------------------------------------------------- # Configuration Priority (highest to lowest): # 1. Command-line flags (e.g., --server-host=0.0.0.0 --server-port=8000) diff --git a/pkg/config/config.go b/pkg/config/config.go index 357a97d4..9e9f1485 100755 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,14 +1,17 @@ package config +import "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" + // ApplicationConfig holds all application configuration // Follows HyperFleet Configuration Standard with validation and structured marshaling type ApplicationConfig struct { - Server *ServerConfig `mapstructure:"server" json:"server" validate:"required"` - Metrics *MetricsConfig `mapstructure:"metrics" json:"metrics" validate:"required"` - Health *HealthConfig `mapstructure:"health" json:"health" validate:"required"` - Database *DatabaseConfig `mapstructure:"database" json:"database" validate:"required"` - Logging *LoggingConfig `mapstructure:"logging" json:"logging" validate:"required"` - Adapters *AdapterRequirementsConfig `mapstructure:"adapters" json:"adapters" validate:"required"` + Server *ServerConfig `mapstructure:"server" json:"server" validate:"required"` + Metrics *MetricsConfig `mapstructure:"metrics" json:"metrics" validate:"required"` + Health *HealthConfig `mapstructure:"health" json:"health" validate:"required"` + Database *DatabaseConfig `mapstructure:"database" json:"database" validate:"required"` + Logging *LoggingConfig `mapstructure:"logging" json:"logging" validate:"required"` + Adapters *AdapterRequirementsConfig `mapstructure:"adapters" json:"adapters" validate:"required"` + Entities []registry.EntityDescriptor `mapstructure:"entities" json:"entities"` } // NewApplicationConfig returns default ApplicationConfig with all sub-configs initialized diff --git a/pkg/config/dump.go b/pkg/config/dump.go index 51676518..07e225b0 100644 --- a/pkg/config/dump.go +++ b/pkg/config/dump.go @@ -2,6 +2,8 @@ package config import ( "fmt" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" ) // DumpConfig returns a human-readable string representation of configuration @@ -35,6 +37,7 @@ func DumpConfig(config *ApplicationConfig) string { Adapters: ClusterAdapters: %v NodePoolAdapters: %v + Entities: %d registered (kinds: %v) `, config.Server.BindAddress(), config.Server.TLS.Enabled, @@ -53,9 +56,20 @@ func DumpConfig(config *ApplicationConfig) string { config.Health.BindAddress(), safeAdapterList(config.Adapters, true), safeAdapterList(config.Adapters, false), + len(config.Entities), + entityKindNames(config.Entities), ) } +// entityKindNames extracts Kind strings from entity descriptors for logging. +func entityKindNames(entities []registry.EntityDescriptor) []string { + kinds := make([]string, len(entities)) + for i, e := range entities { + kinds[i] = e.Kind + } + return kinds +} + // safeAdapterList safely extracts adapter list, handling nil config func safeAdapterList(adapters *AdapterRequirementsConfig, cluster bool) []string { if adapters == nil { diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 74990077..96a43393 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -365,6 +365,9 @@ func (l *ConfigLoader) bindAllEnvVars() { // Adapters config l.bindEnv("adapters.required.cluster") l.bindEnv("adapters.required.nodepool") + + // Entities: config-file-only (complex list-of-struct type). + // No env var or CLI flag bindings — loaded exclusively via YAML config. } // bindFlags binds command-line flags to their corresponding Viper config keys diff --git a/pkg/registry/descriptor.go b/pkg/registry/descriptor.go index 9f398b59..13ebff0a 100644 --- a/pkg/registry/descriptor.go +++ b/pkg/registry/descriptor.go @@ -8,14 +8,37 @@ const ( OnParentDeleteCascade OnParentDeletePolicy = "cascade" ) +// ReferenceDescriptor declares a non-ownership association from one entity type to another. +// See HYPERFLEET-1156 for the full resource references implementation. +type ReferenceDescriptor struct { + // key in the references map on the Resource API type, e.g. "wif_config" + RefType string `mapstructure:"ref_type" json:"ref_type"` + // Kind of the referenced entity, e.g. "WifConfig" + TargetKind string `mapstructure:"target_kind" json:"target_kind"` + // minimum references of this type (0 = optional) + Min int `mapstructure:"min" json:"min,omitempty"` + // maximum references of this type (0 = unlimited) + Max int `mapstructure:"max" json:"max,omitempty"` +} + // EntityDescriptor defines everything specific to a HyperFleet entity type. -// Descriptors are registered at startup via Register() in plugin init() functions. +// Descriptors are loaded from the application config YAML at startup via LoadDescriptors. +// Cluster and NodePool use legacy typed plugins and are not registered here. type EntityDescriptor struct { - Kind string // discriminator value stored in Resource.Kind - Plural string // URL path segment, e.g. "channels" - ParentKind string // "" for top-level entities - SpecSchemaName string // OpenAPI component name for spec validation - OnParentDelete OnParentDeletePolicy // only meaningful when ParentKind != "" - SearchDisallowedFields []string // fields blocked from TSL search - RequiredAdapters []string // adapters that must finalize before hard-delete + // discriminator value stored in Resource.Kind + Kind string `mapstructure:"kind" json:"kind"` + // URL path segment, e.g. "channels" + Plural string `mapstructure:"plural" json:"plural"` + // "" for top-level entities + ParentKind string `mapstructure:"parent_kind" json:"parent_kind,omitempty"` + // OpenAPI component name for spec validation + SpecSchemaName string `mapstructure:"spec_schema_name" json:"spec_schema_name,omitempty"` + // only meaningful when ParentKind != "" + OnParentDelete OnParentDeletePolicy `mapstructure:"on_parent_delete" json:"on_parent_delete,omitempty"` + // adapters that must finalize before hard-delete + RequiredAdapters []string `mapstructure:"required_adapters" json:"required_adapters,omitempty"` + // fields blocked from TSL search + SearchDisallowedFields []string `mapstructure:"search_disallowed_fields" json:"search_disallowed_fields,omitempty"` //nolint:lll + // non-ownership associations to other entity types (HYPERFLEET-1156) + References []ReferenceDescriptor `mapstructure:"references" json:"references,omitempty"` } diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index 95da534f..101677bf 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -1,6 +1,10 @@ package registry -import "fmt" +import ( + "fmt" + + "github.com/getkin/kin-openapi/openapi3" +) var descriptors = make(map[string]EntityDescriptor) @@ -18,6 +22,14 @@ func Register(d EntityDescriptor) { descriptors[d.Kind] = d } +// LoadDescriptors registers entity descriptors loaded from the application config. +// Called during startup after config is parsed and before services/routes are built. +func LoadDescriptors(descriptors []EntityDescriptor) { + for _, d := range descriptors { + Register(d) + } +} + // Get returns a descriptor by Kind, or (zero, false) if not found. func Get(entityKind string) (EntityDescriptor, bool) { d, ok := descriptors[entityKind] @@ -68,6 +80,9 @@ func ChildrenOf(parentKind string) []EntityDescriptor { // - empty Kind or Plural on any descriptor // - any ParentKind that references an unregistered kind // - duplicate Plural values across descriptors +// - ReferenceDescriptor with TargetKind that doesn't resolve +// - duplicate RefType within a single entity's References +// - Max < Min (when Max > 0) func Validate() { plurals := make(map[string]string, len(descriptors)) @@ -95,6 +110,58 @@ func Validate() { )) } plurals[d.Plural] = d.Kind + + // Track seen RefType values to detect duplicates within this entity's References + refTypes := make(map[string]bool, len(d.References)) + for _, ref := range d.References { + if _, ok := descriptors[ref.TargetKind]; !ok { + panic(fmt.Sprintf( + "entity %q: reference %q targets unregistered kind %q", + d.Kind, ref.RefType, ref.TargetKind, + )) + } + if refTypes[ref.RefType] { + panic(fmt.Sprintf( + "entity %q: duplicate ref_type %q in references", + d.Kind, ref.RefType, + )) + } + refTypes[ref.RefType] = true + if ref.Max > 0 && ref.Max < ref.Min { + panic(fmt.Sprintf( + "entity %q: reference %q has max (%d) < min (%d)", + d.Kind, ref.RefType, ref.Max, ref.Min, + )) + } + } + } +} + +// ValidateSchemas checks that every registered entity's SpecSchemaName resolves +// to an existing component in the OpenAPI spec at the given path. +// Panics with a descriptive message if any schema is missing. +// Called at startup after Validate() and before the server accepts requests. +func ValidateSchemas(schemaPath string) { + if schemaPath == "" { + return + } + + loader := openapi3.NewLoader() + doc, err := loader.LoadFromFile(schemaPath) + if err != nil { + panic(fmt.Sprintf("failed to load OpenAPI schema from %s: %s", schemaPath, err)) + } + + for _, d := range descriptors { + if d.SpecSchemaName == "" { + continue + } + if doc.Components == nil || doc.Components.Schemas[d.SpecSchemaName] == nil { + panic(fmt.Sprintf( + "entity %q: spec_schema_name %q not found in OpenAPI spec at %s", + d.Kind, d.SpecSchemaName, schemaPath, + )) + } } } diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go index 05a7050c..b03e594d 100644 --- a/pkg/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -1,6 +1,7 @@ package registry import ( + "os" "testing" . "github.com/onsi/gomega" @@ -203,3 +204,247 @@ func TestDescriptorFields(t *testing.T) { Expect(d.SpecSchemaName).To(Equal("VersionSpec")) Expect(d.SearchDisallowedFields).To(ConsistOf("spec")) } + +func TestLoadDescriptors_RegistersAll(t *testing.T) { + RegisterTestingT(t) + Reset() + + descriptors := []EntityDescriptor{ + {Kind: "Channel", Plural: "channels", SpecSchemaName: "ChannelSpec"}, + {Kind: "Version", Plural: "versions", ParentKind: "Channel", SpecSchemaName: "VersionSpec"}, + } + + LoadDescriptors(descriptors) + + ch, ok := Get("Channel") + Expect(ok).To(BeTrue()) + Expect(ch.Plural).To(Equal("channels")) + Expect(ch.SpecSchemaName).To(Equal("ChannelSpec")) + + ver, ok := Get("Version") + Expect(ok).To(BeTrue()) + Expect(ver.Plural).To(Equal("versions")) + Expect(ver.ParentKind).To(Equal("Channel")) +} + +func TestLoadDescriptors_EmptySlice(t *testing.T) { + RegisterTestingT(t) + Reset() + + LoadDescriptors(nil) + Expect(All()).To(BeEmpty()) + + LoadDescriptors([]EntityDescriptor{}) + Expect(All()).To(BeEmpty()) +} + +func TestLoadDescriptors_DuplicateKind_Panics(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "Channel", Plural: "channels"}) + + Expect(func() { + LoadDescriptors([]EntityDescriptor{ + {Kind: "Channel", Plural: "ch"}, + }) + }).To(PanicWith(ContainSubstring("already registered"))) +} + +func TestLoadDescriptors_AllFields(t *testing.T) { + RegisterTestingT(t) + Reset() + + LoadDescriptors([]EntityDescriptor{ + { + Kind: "Cluster", + Plural: "clusters", + SpecSchemaName: "ClusterSpec", + SearchDisallowedFields: []string{"spec"}, + RequiredAdapters: []string{"provisioner"}, + }, + { + Kind: "NodePool", + Plural: "nodepools", + ParentKind: "Cluster", + OnParentDelete: OnParentDeleteCascade, + SpecSchemaName: "NodePoolSpec", + SearchDisallowedFields: []string{"spec"}, + RequiredAdapters: []string{"provisioner", "lifecycle"}, + }, + }) + + cluster := MustGet("Cluster") + Expect(cluster.SpecSchemaName).To(Equal("ClusterSpec")) + Expect(cluster.SearchDisallowedFields).To(ConsistOf("spec")) + Expect(cluster.RequiredAdapters).To(ConsistOf("provisioner")) + Expect(cluster.ParentKind).To(BeEmpty()) + + np := MustGet("NodePool") + Expect(np.ParentKind).To(Equal("Cluster")) + Expect(np.OnParentDelete).To(Equal(OnParentDeleteCascade)) + Expect(np.SpecSchemaName).To(Equal("NodePoolSpec")) + Expect(np.RequiredAdapters).To(ConsistOf("provisioner", "lifecycle")) +} + +func TestValidate_ReferenceTargetKindUnregistered_Panics(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{ + Kind: "Cluster", + Plural: "clusters", + References: []ReferenceDescriptor{ + {RefType: "wif_config", TargetKind: "WifConfig"}, + }, + }) + + Expect(func() { + Validate() + }).To(PanicWith(ContainSubstring("targets unregistered kind"))) +} + +func TestValidate_DuplicateRefType_Panics(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "WifConfig", Plural: "wifconfigs"}) + Register(EntityDescriptor{ + Kind: "Cluster", + Plural: "clusters", + References: []ReferenceDescriptor{ + {RefType: "wif_config", TargetKind: "WifConfig"}, + {RefType: "wif_config", TargetKind: "WifConfig"}, + }, + }) + + Expect(func() { + Validate() + }).To(PanicWith(ContainSubstring("duplicate ref_type"))) +} + +func TestValidate_ReferenceMaxLessThanMin_Panics(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "WifConfig", Plural: "wifconfigs"}) + Register(EntityDescriptor{ + Kind: "Cluster", + Plural: "clusters", + References: []ReferenceDescriptor{ + {RefType: "wif_config", TargetKind: "WifConfig", Min: 2, Max: 1}, + }, + }) + + Expect(func() { + Validate() + }).To(PanicWith(ContainSubstring("max (1) < min (2)"))) +} + +func TestValidateSchemas_EmptyPath_NoOp(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "Channel", Plural: "channels", SpecSchemaName: "NonExistent"}) + + Expect(func() { + ValidateSchemas("") + }).ToNot(Panic()) +} + +func TestValidateSchemas_MissingSchema_Panics(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "Channel", Plural: "channels", SpecSchemaName: "NonExistentSpec"}) + + schemaContent := `openapi: "3.0.0" +info: + title: Test + version: "1.0" +paths: {} +components: + schemas: + SomeOtherSpec: + type: object +` + tmpDir := t.TempDir() + schemaPath := tmpDir + "/test-schema.yaml" + err := os.WriteFile(schemaPath, []byte(schemaContent), 0600) + Expect(err).To(BeNil()) + + Expect(func() { + ValidateSchemas(schemaPath) + }).To(PanicWith(ContainSubstring("NonExistentSpec"))) +} + +func TestValidateSchemas_AllSchemasPresent_Success(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "Channel", Plural: "channels", SpecSchemaName: "ChannelSpec"}) + + schemaContent := `openapi: "3.0.0" +info: + title: Test + version: "1.0" +paths: {} +components: + schemas: + ChannelSpec: + type: object + properties: + name: + type: string +` + tmpDir := t.TempDir() + schemaPath := tmpDir + "/test-schema.yaml" + err := os.WriteFile(schemaPath, []byte(schemaContent), 0600) + Expect(err).To(BeNil()) + + Expect(func() { + ValidateSchemas(schemaPath) + }).ToNot(Panic()) +} + +func TestValidateSchemas_EmptySpecSchemaName_Skipped(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "Channel", Plural: "channels"}) + + schemaContent := `openapi: "3.0.0" +info: + title: Test + version: "1.0" +paths: {} +` + tmpDir := t.TempDir() + schemaPath := tmpDir + "/test-schema.yaml" + err := os.WriteFile(schemaPath, []byte(schemaContent), 0600) + Expect(err).To(BeNil()) + + Expect(func() { + ValidateSchemas(schemaPath) + }).ToNot(Panic()) +} + +func TestValidate_ValidReferences_Success(t *testing.T) { + RegisterTestingT(t) + Reset() + + Register(EntityDescriptor{Kind: "WifConfig", Plural: "wifconfigs"}) + Register(EntityDescriptor{Kind: "Network", Plural: "networks"}) + Register(EntityDescriptor{ + Kind: "Cluster", + Plural: "clusters", + References: []ReferenceDescriptor{ + {RefType: "wif_config", TargetKind: "WifConfig", Min: 1, Max: 1}, + {RefType: "network", TargetKind: "Network", Min: 0, Max: 0}, + }, + }) + + Expect(func() { + Validate() + }).ToNot(Panic()) +} diff --git a/pkg/validators/schema_validator.go b/pkg/validators/schema_validator.go index f6402f1e..86eebd34 100644 --- a/pkg/validators/schema_validator.go +++ b/pkg/validators/schema_validator.go @@ -7,7 +7,6 @@ import ( "github.com/getkin/kin-openapi/openapi3" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" ) @@ -27,9 +26,8 @@ type SchemaValidator struct { } // NewSchemaValidator creates a new schema validator by loading an OpenAPI spec from the given path. -// Cluster and NodePool must be registered with SpecSchemaName and have matching OpenAPI components. -// Other registered entities with SpecSchemaName are validated only when their component exists; -// missing components are skipped with a warning at startup. +// Every registered entity with a SpecSchemaName must have a matching component in the OpenAPI spec; +// missing components cause a startup error. func NewSchemaValidator(schemaPath string) (*SchemaValidator, error) { loader := openapi3.NewLoader() doc, err := loader.LoadFromFile(schemaPath) @@ -53,27 +51,16 @@ func NewSchemaValidator(schemaPath string) (*SchemaValidator, error) { } func buildSchemasMap(doc *openapi3.T) (map[string]*ResourceSchema, error) { - ctx := context.Background() schemas := make(map[string]*ResourceSchema) // registeredKinds := make(map[string]bool, len(requiredSpecValidationKinds)) for _, d := range registry.WithSpecSchema() { schemaRef := doc.Components.Schemas[d.SpecSchemaName] if schemaRef == nil { - // TODO : HYPERFLEET-1159 - Uncomment this once Cluster and NodePool are registered - // if isRequiredSpecValidationKind(d.Kind) { - // return nil, fmt.Errorf( - // "%s schema not found in OpenAPI spec (required for entity kind %q)", - // d.SpecSchemaName, d.Kind, - // ) - // } - - logger.With(ctx, - "schema_name", d.SpecSchemaName, - "kind", d.Kind, - "plural", d.Plural, - ).Warn("OpenAPI spec schema not found, skipping validation for entity") - continue + return nil, fmt.Errorf( + "entity %q: spec_schema_name %q not found in OpenAPI spec", + d.Kind, d.SpecSchemaName, + ) } schemas[d.Plural] = &ResourceSchema{ diff --git a/pkg/validators/schema_validator_test.go b/pkg/validators/schema_validator_test.go index cd2708cc..4886419d 100644 --- a/pkg/validators/schema_validator_test.go +++ b/pkg/validators/schema_validator_test.go @@ -1,15 +1,12 @@ package validators import ( - "bytes" - "log/slog" "os" "path/filepath" "testing" . "github.com/onsi/gomega" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/errors" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" ) @@ -149,10 +146,9 @@ components: err := os.WriteFile(schemaPath, []byte(invalidSchema), 0600) Expect(err).To(BeNil()) - // Should fail because ClusterSpec is missing _, err = NewSchemaValidator(schemaPath) Expect(err).ToNot(BeNil()) - Expect(err.Error()).To(ContainSubstring("ClusterSpec schema not found")) + Expect(err.Error()).To(ContainSubstring("not found in OpenAPI spec")) } // TODO : HYPERFLEET-1159 - Uncomment this once Cluster and NodePool are registered @@ -176,17 +172,9 @@ components: // Expect(err.Error()).To(ContainSubstring(`entity kind "NodePool" with SpecSchemaName must be registered`)) // } -func TestNewSchemaValidator_OptionalEntityMissingOpenAPISchema_SkipsWithWarning(t *testing.T) { +func TestNewSchemaValidator_RegisteredEntityMissingOpenAPISchema_Errors(t *testing.T) { RegisterTestingT(t) - var logBuf bytes.Buffer - logger.ReconfigureGlobalLogger(&logger.LogConfig{ - Level: slog.LevelWarn, - Format: logger.FormatText, - Output: &logBuf, - Component: "validators-test", - }) - registerRequiredSpecValidationEntities() registry.Register(registry.EntityDescriptor{ Kind: "WifConfig", @@ -219,16 +207,11 @@ components: err := os.WriteFile(schemaPath, []byte(schemaWithoutWifConfig), 0600) Expect(err).To(BeNil()) - validator, err := NewSchemaValidator(schemaPath) - Expect(err).To(BeNil()) - Expect(validator.HasSchema("clusters")).To(BeTrue()) - Expect(validator.HasSchema("nodepools")).To(BeTrue()) - Expect(validator.HasSchema("wifconfigs")).To(BeFalse()) - - logOutput := logBuf.String() - Expect(logOutput).To(ContainSubstring("skipping validation for entity")) - Expect(logOutput).To(ContainSubstring("WifConfigSpec")) - Expect(logOutput).To(ContainSubstring("WifConfig")) + _, err = NewSchemaValidator(schemaPath) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("WifConfig")) + Expect(err.Error()).To(ContainSubstring("WifConfigSpec")) + Expect(err.Error()).To(ContainSubstring("not found in OpenAPI spec")) } func TestNewSchemaValidator_RequiredEntityMissingOpenAPISchema_Fails(t *testing.T) { @@ -258,20 +241,13 @@ components: _, err = NewSchemaValidator(schemaPath) Expect(err).ToNot(BeNil()) - Expect(err.Error()).To(ContainSubstring("NodePoolSpec schema not found")) + Expect(err.Error()).To(ContainSubstring("NodePoolSpec")) + Expect(err.Error()).To(ContainSubstring("not found")) } -func TestValidate_SkipsWhenOptionalEntitySchemaNotLoaded(t *testing.T) { +func TestValidate_ErrorsWhenRegisteredEntitySchemaNotLoaded(t *testing.T) { RegisterTestingT(t) - var logBuf bytes.Buffer - logger.ReconfigureGlobalLogger(&logger.LogConfig{ - Level: slog.LevelWarn, - Format: logger.FormatText, - Output: &logBuf, - Component: "validators-test", - }) - registerRequiredSpecValidationEntities() registry.Register(registry.EntityDescriptor{ Kind: "WifConfig", @@ -304,12 +280,10 @@ components: err := os.WriteFile(schemaPath, []byte(schemaWithoutWifConfig), 0600) Expect(err).To(BeNil()) - validator, err := NewSchemaValidator(schemaPath) - Expect(err).To(BeNil()) - - // Invalid spec would fail validation if WifConfigSpec were loaded. - err = validator.Validate("wifconfigs", map[string]interface{}{}) - Expect(err).To(BeNil()) + _, err = NewSchemaValidator(schemaPath) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("WifConfig")) + Expect(err.Error()).To(ContainSubstring("WifConfigSpec")) } func TestValidate_WifConfigSpec_Valid(t *testing.T) { diff --git a/plugins/CLAUDE.md b/plugins/CLAUDE.md index 877546c9..e5598a19 100644 --- a/plugins/CLAUDE.md +++ b/plugins/CLAUDE.md @@ -1,10 +1,28 @@ # Claude Code Guidelines for Plugins -## Registration Pattern +## Two Registration Models -Each resource plugin registers itself in an `init()` function. Reference: `clusters/plugin.go` +### Config-driven entities (Channel, Version, WifConfig, future types) -Four registrations per plugin: +Entity types are declared in `config.yaml` under `entities:` and registered at startup via +`registry.LoadDescriptors()`. Routes are auto-generated by `plugins/entities/plugin.go`. +No per-entity plugin, DAO, service, or handler code is needed. + +To add a new generic entity type: +1. Add an entry to the `entities:` section in `config.yaml` (and `charts/values.yaml` for Helm) +2. Add the spec schema to the provider OpenAPI spec (`hyperfleet-api-spec`) +3. Redeploy — routes, spec validation, and delete policies are generated automatically + +Reference: `plugins/entities/plugin.go`, `pkg/registry/load.go` + +### Legacy typed plugins (Cluster, NodePool) + +Cluster and NodePool use dedicated DAO, service, handler, and plugin packages with +custom business logic. They register via `init()` functions. + +Reference: `clusters/plugin.go` + +Four registrations per legacy plugin: 1. **Service**: `registry.RegisterService("Clusters", func(env interface{}) interface{} { ... })` 2. **Routes**: `server.RegisterRoutes("clusters", func(router, services, ...) { ... })` @@ -13,7 +31,7 @@ Four registrations per plugin: ## ServiceLocator -Each plugin defines a `ServiceLocator` type (func returning the service) and a `Service()` helper that looks up the service from the environment's service registry. +Each legacy plugin defines a `ServiceLocator` type (func returning the service) and a `Service()` helper that looks up the service from the environment's service registry. ``` type ServiceLocator func() services.ClusterService @@ -28,14 +46,6 @@ Inside `RegisterRoutes`, use gorilla/mux router methods: - `router.HandleFunc("/clusters/{id}", handler.Get).Methods("GET")` - Nested resources: `/clusters/{id}/nodepools`, `/clusters/{id}/statuses` -## Adding a New Resource - -1. Create the DAO interface + implementation in `pkg/dao/` -2. Create the service interface + implementation in `pkg/services/` -3. Create the handler in `pkg/handlers/` -4. Create the plugin in `plugins//plugin.go` with all 4 registrations -5. Run `make generate-mocks` to generate service mocks - ## Related CLAUDE.md Files - `pkg/handlers/CLAUDE.md` — Handler pipeline diff --git a/plugins/channels/plugin.go b/plugins/channels/plugin.go deleted file mode 100644 index 681f2e84..00000000 --- a/plugins/channels/plugin.go +++ /dev/null @@ -1,44 +0,0 @@ -package channels - -import ( - "net/http" - - "github.com/gorilla/mux" - - "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" - "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" - "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" -) - -const ( - kindChannel = "Channel" - pluralChannels = "channels" - specSchemaChannel = "ChannelSpec" -) - -func init() { - registry.Register(registry.EntityDescriptor{ - Kind: kindChannel, - Plural: pluralChannels, - SpecSchemaName: specSchemaChannel, - SearchDisallowedFields: []string{"spec"}, - }) - - server.RegisterRoutes(pluralChannels, func(apiV1Router *mux.Router, svc server.ServicesInterface) { - envServices := svc.(*environments.Services) - descriptor := registry.MustGet(kindChannel) - h := handlers.NewResourceHandler( - descriptor, - resources.Service(envServices), - ) - - r := apiV1Router.PathPrefix("/" + descriptor.Plural).Subrouter() - r.HandleFunc("", h.List).Methods(http.MethodGet) - r.HandleFunc("", h.Create).Methods(http.MethodPost) - r.HandleFunc("/{id}", h.Get).Methods(http.MethodGet) - r.HandleFunc("/{id}", h.Patch).Methods(http.MethodPatch) - r.HandleFunc("/{id}", h.Delete).Methods(http.MethodDelete) - }) -} diff --git a/plugins/entities/plugin.go b/plugins/entities/plugin.go new file mode 100644 index 00000000..575682a6 --- /dev/null +++ b/plugins/entities/plugin.go @@ -0,0 +1,58 @@ +package entities + +import ( + "net/http" + "sort" + + "github.com/gorilla/mux" + + "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" + "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" + "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" +) + +func init() { + server.RegisterRoutes("entities", func(apiV1Router *mux.Router, svc server.ServicesInterface) { + envServices := svc.(*environments.Services) + resourceService := resources.Service(envServices) + + RegisterEntityRoutes(apiV1Router, resourceService) + }) +} + +// RegisterEntityRoutes creates handlers and registers routes for every entity +// descriptor in the registry. Called at startup after config-driven descriptors +// have been loaded via registry.LoadDescriptors. +// +// Top-level entities get routes at /{plural}. Child entities (ParentKind != "") +// get nested routes only, under /{parent_plural}/{parent_id}/{plural}. +func RegisterEntityRoutes(apiV1Router *mux.Router, resourceService services.ResourceService) { + descriptors := registry.All() + sort.Slice(descriptors, func(i, j int) bool { + return descriptors[i].Kind < descriptors[j].Kind + }) + + for _, descriptor := range descriptors { + h := handlers.NewResourceHandler(descriptor, resourceService) + + if descriptor.ParentKind == "" { + r := apiV1Router.PathPrefix("/" + descriptor.Plural).Subrouter() + r.HandleFunc("", h.List).Methods(http.MethodGet) + r.HandleFunc("", h.Create).Methods(http.MethodPost) + r.HandleFunc("/{id}", h.Get).Methods(http.MethodGet) + r.HandleFunc("/{id}", h.Patch).Methods(http.MethodPatch) + r.HandleFunc("/{id}", h.Delete).Methods(http.MethodDelete) + } else { + parent := registry.MustGet(descriptor.ParentKind) + pr := apiV1Router.PathPrefix("/" + parent.Plural + "/{parent_id}/" + descriptor.Plural).Subrouter() + pr.HandleFunc("", h.ListByOwner).Methods(http.MethodGet) + pr.HandleFunc("", h.CreateWithOwner).Methods(http.MethodPost) + pr.HandleFunc("/{id}", h.GetByOwner).Methods(http.MethodGet) + pr.HandleFunc("/{id}", h.PatchByOwner).Methods(http.MethodPatch) + pr.HandleFunc("/{id}", h.DeleteByOwner).Methods(http.MethodDelete) + } + } +} diff --git a/plugins/entities/plugin_test.go b/plugins/entities/plugin_test.go new file mode 100644 index 00000000..189f0591 --- /dev/null +++ b/plugins/entities/plugin_test.go @@ -0,0 +1,88 @@ +package entities + +import ( + "net/http/httptest" + "testing" + + "github.com/gorilla/mux" + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" +) + +func TestRegisterEntityRoutes_TopLevelEntity(t *testing.T) { + RegisterTestingT(t) + registry.Reset() + registry.Register(registry.EntityDescriptor{ + Kind: "Channel", + Plural: "channels", + SpecSchemaName: "ChannelSpec", + }) + + router := mux.NewRouter() + apiV1 := router.PathPrefix("/api/hyperfleet/v1").Subrouter() + RegisterEntityRoutes(apiV1, nil) + + assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/channels") + assertRouteMatches(t, router, "POST", "/api/hyperfleet/v1/channels") + assertRouteMatches(t, router, "GET", "/api/hyperfleet/v1/channels/00000000-0000-0000-0000-000000000001") + assertRouteMatches(t, router, "PATCH", "/api/hyperfleet/v1/channels/00000000-0000-0000-0000-000000000001") + assertRouteMatches(t, router, "DELETE", "/api/hyperfleet/v1/channels/00000000-0000-0000-0000-000000000001") +} + +func TestRegisterEntityRoutes_ChildEntity_NestedOnly(t *testing.T) { + RegisterTestingT(t) + registry.Reset() + registry.Register(registry.EntityDescriptor{Kind: "Channel", Plural: "channels"}) + registry.Register(registry.EntityDescriptor{ + Kind: "Version", + Plural: "versions", + ParentKind: "Channel", + }) + + router := mux.NewRouter() + apiV1 := router.PathPrefix("/api/hyperfleet/v1").Subrouter() + RegisterEntityRoutes(apiV1, nil) + + parentID := "00000000-0000-0000-0000-000000000001" + childID := "00000000-0000-0000-0000-000000000002" + base := "/api/hyperfleet/v1/channels/" + parentID + "/versions" + + assertRouteMatches(t, router, "GET", base) + assertRouteMatches(t, router, "POST", base) + assertRouteMatches(t, router, "GET", base+"/"+childID) + assertRouteMatches(t, router, "PATCH", base+"/"+childID) + assertRouteMatches(t, router, "DELETE", base+"/"+childID) + + assertRouteNotFound(t, router, "GET", "/api/hyperfleet/v1/versions") + assertRouteNotFound(t, router, "POST", "/api/hyperfleet/v1/versions") +} + +func TestRegisterEntityRoutes_EmptyRegistry(t *testing.T) { + RegisterTestingT(t) + registry.Reset() + + router := mux.NewRouter() + apiV1 := router.PathPrefix("/api/hyperfleet/v1").Subrouter() + + Expect(func() { + RegisterEntityRoutes(apiV1, nil) + }).ToNot(Panic()) +} + +func assertRouteMatches(t *testing.T, router *mux.Router, method, path string) { + t.Helper() + req := httptest.NewRequest(method, path, nil) + match := mux.RouteMatch{} + Expect(router.Match(req, &match)).To(BeTrue(), "expected route to match: %s %s", method, path) +} + +func assertRouteNotFound(t *testing.T, router *mux.Router, method, path string) { + t.Helper() + req := httptest.NewRequest(method, path, nil) + match := mux.RouteMatch{} + matched := router.Match(req, &match) + if matched && match.MatchErr == nil && match.Handler != nil { + t.Errorf("expected no route match for %s %s but one was found", method, path) + } +} diff --git a/plugins/versions/plugin.go b/plugins/versions/plugin.go deleted file mode 100644 index c3852ee7..00000000 --- a/plugins/versions/plugin.go +++ /dev/null @@ -1,47 +0,0 @@ -package versions - -import ( - "net/http" - - "github.com/gorilla/mux" - - "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" - "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" - "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" -) - -const ( - kindVersion = "Version" - pluralVersions = "versions" - specSchemaVersion = "VersionSpec" -) - -func init() { - registry.Register(registry.EntityDescriptor{ - Kind: kindVersion, - Plural: pluralVersions, - ParentKind: "Channel", - SpecSchemaName: specSchemaVersion, - OnParentDelete: registry.OnParentDeleteRestrict, - SearchDisallowedFields: []string{"spec"}, - }) - - server.RegisterRoutes(pluralVersions, func(apiV1Router *mux.Router, svc server.ServicesInterface) { - envServices := svc.(*environments.Services) - channelDescriptor := registry.MustGet("Channel") - descriptor := registry.MustGet(kindVersion) - h := handlers.NewResourceHandler( - descriptor, - resources.Service(envServices), - ) - - r := apiV1Router.PathPrefix("/" + channelDescriptor.Plural + "/{parent_id}/" + descriptor.Plural).Subrouter() - r.HandleFunc("", h.ListByOwner).Methods(http.MethodGet) - r.HandleFunc("", h.CreateWithOwner).Methods(http.MethodPost) - r.HandleFunc("/{id}", h.GetByOwner).Methods(http.MethodGet) - r.HandleFunc("/{id}", h.PatchByOwner).Methods(http.MethodPatch) - r.HandleFunc("/{id}", h.DeleteByOwner).Methods(http.MethodDelete) - }) -} diff --git a/plugins/wifconfigs/plugin.go b/plugins/wifconfigs/plugin.go deleted file mode 100644 index 8a46d740..00000000 --- a/plugins/wifconfigs/plugin.go +++ /dev/null @@ -1,44 +0,0 @@ -package wifconfigs - -import ( - "net/http" - - "github.com/gorilla/mux" - - "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/environments" - "github.com/openshift-hyperfleet/hyperfleet-api/cmd/hyperfleet-api/server" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/handlers" - "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" - "github.com/openshift-hyperfleet/hyperfleet-api/plugins/resources" -) - -const ( - kindWifConfig = "WifConfig" - pluralWifConfigs = "wifconfigs" - specSchemaWifConfig = "WifConfigSpec" -) - -func init() { - registry.Register(registry.EntityDescriptor{ - Kind: kindWifConfig, - Plural: pluralWifConfigs, - SpecSchemaName: specSchemaWifConfig, - SearchDisallowedFields: []string{"spec"}, - }) - - server.RegisterRoutes(pluralWifConfigs, func(apiV1Router *mux.Router, svc server.ServicesInterface) { - envServices := svc.(*environments.Services) - descriptor := registry.MustGet(kindWifConfig) - h := handlers.NewResourceHandler( - descriptor, - resources.Service(envServices), - ) - - r := apiV1Router.PathPrefix("/" + descriptor.Plural).Subrouter() - r.HandleFunc("", h.List).Methods(http.MethodGet) - r.HandleFunc("", h.Create).Methods(http.MethodPost) - r.HandleFunc("/{id}", h.Get).Methods(http.MethodGet) - r.HandleFunc("/{id}", h.Patch).Methods(http.MethodPatch) - r.HandleFunc("/{id}", h.Delete).Methods(http.MethodDelete) - }) -} diff --git a/test/helper.go b/test/helper.go index 144fe618..bdd33e4c 100755 --- a/test/helper.go +++ b/test/helper.go @@ -27,8 +27,11 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/config" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/logger" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/registry" "github.com/openshift-hyperfleet/hyperfleet-api/test/factories" "github.com/openshift-hyperfleet/hyperfleet-api/test/mocks" + + _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/entities" ) const ( @@ -90,6 +93,13 @@ func NewHelper(t *testing.T) *Helper { env := environments.Environment() env.Config = cfg + registry.LoadDescriptors(cfg.Entities) + if len(cfg.Entities) == 0 { + loadDefaultTestEntities() + } + registry.Validate() + registry.ValidateSchemas(cfg.Server.OpenAPISchemaPath) + err = env.SetEnvironmentDefaults(pflag.CommandLine) if err != nil { logger.WithError(ctx, err).Error("Unable to set environment defaults") @@ -742,3 +752,30 @@ func initTestLogger() { } logger.InitGlobalLogger(cfg) } + +// loadDefaultTestEntities registers the standard entity descriptors that +// integration tests need when no config file provides them. +func loadDefaultTestEntities() { + registry.LoadDescriptors([]registry.EntityDescriptor{ + { + Kind: "Channel", + Plural: "channels", + SpecSchemaName: "ChannelSpec", + SearchDisallowedFields: []string{"spec"}, + }, + { + Kind: "Version", + Plural: "versions", + ParentKind: "Channel", + OnParentDelete: registry.OnParentDeleteRestrict, + SpecSchemaName: "VersionSpec", + SearchDisallowedFields: []string{"spec"}, + }, + { + Kind: "WifConfig", + Plural: "wifconfigs", + SpecSchemaName: "WifConfigSpec", + SearchDisallowedFields: []string{"spec"}, + }, + }) +} diff --git a/test/integration/versions_test.go b/test/integration/versions_test.go index cc3c5ce4..b0a81a9b 100644 --- a/test/integration/versions_test.go +++ b/test/integration/versions_test.go @@ -11,9 +11,6 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/services" - - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/channels" - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/versions" ) func createTestChannel(t *testing.T, svc services.ResourceService) *api.Resource { diff --git a/test/integration/wifconfigs_test.go b/test/integration/wifconfigs_test.go index 89d8f716..f2b495da 100644 --- a/test/integration/wifconfigs_test.go +++ b/test/integration/wifconfigs_test.go @@ -13,8 +13,6 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api/openapi" "github.com/openshift-hyperfleet/hyperfleet-api/test" - - _ "github.com/openshift-hyperfleet/hyperfleet-api/plugins/wifconfigs" ) const wifConfigsPath = "/wifconfigs"