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 5bbe083b..6847b02d 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 da3c167b..90671d58 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..2913da25 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,12 @@ 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() + // 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 04c60d35..2df28365 100644 --- a/pkg/registry/descriptor.go +++ b/pkg/registry/descriptor.go @@ -8,15 +8,39 @@ 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 - RequireSpecSchema bool // panic at startup if SpecSchemaName missing from spec + // 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"` + // panic at startup if SpecSchemaName missing from spec + RequireSpecSchema bool `mapstructure:"require_spec_schema" json:"require_spec_schema,omitempty"` } diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index e609fb15..877da002 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -18,6 +18,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 +76,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 +106,30 @@ 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, + )) + } + } } } diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go index feffbc34..c97606a0 100644 --- a/pkg/registry/registry_test.go +++ b/pkg/registry/registry_test.go @@ -318,3 +318,159 @@ 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 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 9ef1b00a..54d82624 100644 --- a/pkg/validators/schema_validator.go +++ b/pkg/validators/schema_validator.go @@ -124,12 +124,9 @@ func (v *SchemaValidator) ValidateNodePoolSpec(spec map[string]interface{}) erro func (v *SchemaValidator) validateSpec( spec map[string]interface{}, schemaRef *openapi3.SchemaRef, specTypeName string, ) error { - // Cast spec to interface{} for VisitJSON var specData interface{} = spec - // Validate against schema if err := schemaRef.Value.VisitJSON(specData); err != nil { - // Convert validation error to our error format with details validationDetails := convertValidationError(err, "spec") return errors.ValidationWithDetails( fmt.Sprintf("Invalid %s", specTypeName), @@ -146,36 +143,23 @@ func convertValidationError(err error, prefix string) []errors.ValidationDetail switch e := err.(type) { case openapi3.MultiError: - // Recursively process each sub-error for _, subErr := range e { subDetails := convertValidationError(subErr, prefix) details = append(details, subDetails...) } case *openapi3.SchemaError: - // Extract field path from SchemaError field := prefix - - // Use JSONPointer which contains the actual data path - // JSONPointer returns the path like ["platform", "gcp", "diskSize"] if len(e.JSONPointer()) > 0 { jsonPath := strings.Join(e.JSONPointer(), ".") if jsonPath != "" { field = prefix + "." + jsonPath } } - - // Use the error message (Reason) which already contains field information - // Examples: - // - "property 'region' is missing" - // - "property 'unknownField' is unsupported" - // - "number must be at least 10" details = append(details, errors.ValidationDetail{ Field: field, Message: e.Reason, }) default: - // Fallback for unknown error types - // Error message already contains the full description details = append(details, errors.ValidationDetail{ Field: prefix, Message: err.Error(), diff --git a/pkg/validators/schema_validator_test.go b/pkg/validators/schema_validator_test.go index 0126bdf7..5ec69844 100644 --- a/pkg/validators/schema_validator_test.go +++ b/pkg/validators/schema_validator_test.go @@ -80,13 +80,11 @@ func TestNewSchemaValidator(t *testing.T) { registerRequiredSpecValidationEntities() - // Create temporary schema file tmpDir := t.TempDir() schemaPath := filepath.Join(tmpDir, "test-schema.yaml") err := os.WriteFile(schemaPath, []byte(testSchema), 0600) Expect(err).To(BeNil()) - // Test successful schema loading validator, err := NewSchemaValidator(schemaPath) Expect(err).To(BeNil()) Expect(validator).ToNot(BeNil()) @@ -104,7 +102,6 @@ func TestNewSchemaValidator(t *testing.T) { func TestNewSchemaValidator_InvalidPath(t *testing.T) { RegisterTestingT(t) - // Test with non-existent file _, err := NewSchemaValidator("/nonexistent/path/schema.yaml") Expect(err).ToNot(BeNil()) Expect(err.Error()).To(ContainSubstring("failed to load OpenAPI schema")) @@ -128,7 +125,6 @@ func TestNewSchemaValidator_MissingSchemas(t *testing.T) { registerRequiredSpecValidationEntities() - // Schema without required components invalidSchema := ` openapi: 3.0.0 info: @@ -148,7 +144,7 @@ components: _, 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")) } func TestNewSchemaValidator_RegisteredEntityMissingSchema_Panics(t *testing.T) { @@ -289,7 +285,8 @@ 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_WifConfigSpec_Valid(t *testing.T) { @@ -345,52 +342,36 @@ func TestValidate_ChannelSpec_MissingRequiredField(t *testing.T) { func TestValidateClusterSpec_Valid(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test valid cluster spec - validSpec := map[string]interface{}{ + err := validator.ValidateClusterSpec(map[string]interface{}{ "region": "us-central1", "provider": "gcp", - } - - err := validator.ValidateClusterSpec(validSpec) + }) Expect(err).To(BeNil()) } func TestValidateClusterSpec_ValidWithOptionalFields(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test valid cluster spec with optional network field - validSpec := map[string]interface{}{ + err := validator.ValidateClusterSpec(map[string]interface{}{ "region": "europe-west1", "provider": "aws", - "network": map[string]interface{}{ - "vpc_id": "vpc-12345", - }, - } - - err := validator.ValidateClusterSpec(validSpec) + "network": map[string]interface{}{"vpc_id": "vpc-12345"}, + }) Expect(err).To(BeNil()) } func TestValidateClusterSpec_MissingRequiredField(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec missing required field - invalidSpec := map[string]interface{}{ + err := validator.ValidateClusterSpec(map[string]interface{}{ "region": "us-central1", - // missing "provider" - } - - err := validator.ValidateClusterSpec(invalidSpec) + }) Expect(err).ToNot(BeNil()) - // Check that error is a ServiceError with details serviceErr := getServiceError(err) Expect(serviceErr).ToNot(BeNil()) Expect(serviceErr.Details).ToNot(BeEmpty()) @@ -398,41 +379,29 @@ func TestValidateClusterSpec_MissingRequiredField(t *testing.T) { func TestValidateClusterSpec_InvalidEnum(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec with invalid enum value - invalidSpec := map[string]interface{}{ - "region": "asia-southeast1", // not in enum + err := validator.ValidateClusterSpec(map[string]interface{}{ + "region": "asia-southeast1", "provider": "gcp", - } - - err := validator.ValidateClusterSpec(invalidSpec) + }) Expect(err).ToNot(BeNil()) serviceErr := getServiceError(err) Expect(serviceErr).ToNot(BeNil()) Expect(serviceErr.Details).ToNot(BeEmpty()) - - // Verify we get validation details (field path extraction is tested separately) Expect(serviceErr.Details[0].Message).ToNot(BeEmpty()) } func TestValidateClusterSpec_InvalidNestedField(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec with invalid nested field (empty vpc_id) - invalidSpec := map[string]interface{}{ + err := validator.ValidateClusterSpec(map[string]interface{}{ "region": "us-central1", "provider": "gcp", - "network": map[string]interface{}{ - "vpc_id": "", // violates minLength: 1 - }, - } - - err := validator.ValidateClusterSpec(invalidSpec) + "network": map[string]interface{}{"vpc_id": ""}, + }) Expect(err).ToNot(BeNil()) serviceErr := getServiceError(err) @@ -441,47 +410,34 @@ func TestValidateClusterSpec_InvalidNestedField(t *testing.T) { func TestValidateNodePoolSpec_Valid(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test valid nodepool spec - validSpec := map[string]interface{}{ + err := validator.ValidateNodePoolSpec(map[string]interface{}{ "machine_type": "n1-standard-4", "replicas": 3, - } - - err := validator.ValidateNodePoolSpec(validSpec) + }) Expect(err).To(BeNil()) } func TestValidateNodePoolSpec_ValidWithOptional(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test valid nodepool spec with optional autoscaling - validSpec := map[string]interface{}{ + err := validator.ValidateNodePoolSpec(map[string]interface{}{ "machine_type": "n1-standard-4", "replicas": 5, "autoscaling": true, - } - - err := validator.ValidateNodePoolSpec(validSpec) + }) Expect(err).To(BeNil()) } func TestValidateNodePoolSpec_MissingRequiredField(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec missing required field - invalidSpec := map[string]interface{}{ + err := validator.ValidateNodePoolSpec(map[string]interface{}{ "machine_type": "n1-standard-4", - // missing "replicas" - } - - err := validator.ValidateNodePoolSpec(invalidSpec) + }) Expect(err).ToNot(BeNil()) serviceErr := getServiceError(err) @@ -491,67 +447,47 @@ func TestValidateNodePoolSpec_MissingRequiredField(t *testing.T) { func TestValidateNodePoolSpec_InvalidType(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec with wrong type (replicas should be integer) - invalidSpec := map[string]interface{}{ + err := validator.ValidateNodePoolSpec(map[string]interface{}{ "machine_type": "n1-standard-4", - "replicas": "three", // should be integer - } - - err := validator.ValidateNodePoolSpec(invalidSpec) + "replicas": "three", + }) Expect(err).ToNot(BeNil()) - - serviceErr := getServiceError(err) - Expect(serviceErr).ToNot(BeNil()) + Expect(getServiceError(err)).ToNot(BeNil()) } func TestValidateNodePoolSpec_OutOfRange(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec with value out of range - invalidSpec := map[string]interface{}{ + err := validator.ValidateNodePoolSpec(map[string]interface{}{ "machine_type": "n1-standard-4", - "replicas": 150, // exceeds maximum: 100 - } - - err := validator.ValidateNodePoolSpec(invalidSpec) + "replicas": 150, + }) Expect(err).ToNot(BeNil()) - - serviceErr := getServiceError(err) - Expect(serviceErr).ToNot(BeNil()) + Expect(getServiceError(err)).ToNot(BeNil()) } func TestValidateNodePoolSpec_BelowMinimum(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec with value below minimum - invalidSpec := map[string]interface{}{ + err := validator.ValidateNodePoolSpec(map[string]interface{}{ "machine_type": "n1-standard-4", - "replicas": 0, // below minimum: 1 - } - - err := validator.ValidateNodePoolSpec(invalidSpec) + "replicas": 0, + }) Expect(err).ToNot(BeNil()) } func TestValidateNodePoolSpec_EmptyMachineType(t *testing.T) { RegisterTestingT(t) - validator := setupTestValidator(t) - // Test spec with empty machine_type (violates minLength) - invalidSpec := map[string]interface{}{ + err := validator.ValidateNodePoolSpec(map[string]interface{}{ "machine_type": "", "replicas": 3, - } - - err := validator.ValidateNodePoolSpec(invalidSpec) + }) Expect(err).ToNot(BeNil()) } @@ -631,12 +567,8 @@ func getServiceError(err error) *errors.ServiceError { if err == nil { return nil } - - // Import errors package at the top - // This is a type assertion to check if err is a *errors.ServiceError if serviceErr, ok := err.(*errors.ServiceError); ok { return serviceErr } - return nil } 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..eadc3fde 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,12 @@ func NewHelper(t *testing.T) *Helper { env := environments.Environment() env.Config = cfg + registry.LoadDescriptors(cfg.Entities) + if len(cfg.Entities) == 0 { + loadDefaultTestEntities() + } + registry.Validate() + err = env.SetEnvironmentDefaults(pflag.CommandLine) if err != nil { logger.WithError(ctx, err).Error("Unable to set environment defaults") @@ -742,3 +751,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"