From cf97b10a154e85c94f0a30031c939973929a79b7 Mon Sep 17 00:00:00 2001 From: tithakka Date: Mon, 17 Aug 2026 16:21:01 -0500 Subject: [PATCH 1/4] HYPERFLEET-1406 - feat: Define the HyperFleetConfig CRD (v1alpha1) --- Makefile | 15 +- PROJECT | 6 +- api/v1alpha/hyperfleetconfig_types.go | 64 --- .../groupversion_info.go | 6 +- api/v1alpha1/hyperfleetconfig_types.go | 272 ++++++++++++ .../zz_generated.deepcopy.go | 104 ++++- cmd/main.go | 4 +- ...perfleet.redhat.com_hyperfleetconfigs.yaml | 270 ++++++++++++ config/rbac/role.yaml | 33 +- .../hyperfleet_v1alpha1_hyperfleetconfig.yaml | 27 ++ .../hyperfleet_v1alpha_hyperfleetconfig.yaml | 9 - config/samples/kustomization.yaml | 2 +- go.mod | 2 +- .../controller/hyperfleetconfig_controller.go | 4 +- .../hyperfleetconfig_controller_test.go | 59 ++- .../controller/hyperfleetconfig_types_test.go | 406 ++++++++++++++++++ internal/controller/suite_test.go | 4 +- 17 files changed, 1172 insertions(+), 115 deletions(-) delete mode 100644 api/v1alpha/hyperfleetconfig_types.go rename api/{v1alpha => v1alpha1}/groupversion_info.go (89%) create mode 100644 api/v1alpha1/hyperfleetconfig_types.go rename api/{v1alpha => v1alpha1}/zz_generated.deepcopy.go (55%) create mode 100644 config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml create mode 100644 config/samples/hyperfleet_v1alpha1_hyperfleetconfig.yaml delete mode 100644 config/samples/hyperfleet_v1alpha_hyperfleetconfig.yaml create mode 100644 internal/controller/hyperfleetconfig_types_test.go diff --git a/Makefile b/Makefile index fd8d299..e3d5168 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,12 @@ +# GOTOOLCHAIN pins the Go toolchain for all build/generate/test targets. +# The tooling module (tools/go.mod) requires Go >= 1.26, while the root module +# keeps its language level at `go 1.24` so the pinned golangci-lint (built with +# go1.24) still accepts go.mod. Forcing one toolchain here — rather than a +# go.mod `toolchain` directive — avoids GOTOOLCHAIN=auto switching mid-build, +# which breaks coverage ("go: no such tool covdata"). go1.26.5 is the first +# release carrying the fixes for CVE-2026-39822 and CVE-2026-42505. +export GOTOOLCHAIN ?= go1.26.5 + # VERSION defines the project version for the bundle. # Update this value when you upgrade the version of your project. # To re-generate a bundle for another specific version without changing the standard setup, you can: @@ -131,7 +140,11 @@ KIND_CLUSTER ?= hyperfleet-operator-test-e2e .PHONY: test test: manifests generate fmt vet ## Run tests. - KUBEBUILDER_ASSETS="$(shell $(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out + # Resolve envtest assets in the recipe shell (via $$(...)) rather than make's + # $(shell ...): the latter runs at parse time and does not inherit the + # exported GOTOOLCHAIN, so setup-envtest (a tools/go.mod tool needing Go 1.26) + # would run under the wrong toolchain and yield an empty KUBEBUILDER_ASSETS. + KUBEBUILDER_ASSETS="$$($(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out .PHONY: setup-test-e2e setup-test-e2e: ## Set up a Kind cluster for e2e tests if it does not exist diff --git a/PROJECT b/PROJECT index 53331ff..92f52cd 100644 --- a/PROJECT +++ b/PROJECT @@ -13,11 +13,11 @@ repo: github.com/openshift-hyperfleet/hyperfleet-operator resources: - api: crdVersion: v1 - namespaced: true + namespaced: false controller: true domain: redhat.com group: hyperfleet kind: HyperFleetConfig - path: github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha - version: v1alpha + path: github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1 + version: v1alpha1 version: "3" diff --git a/api/v1alpha/hyperfleetconfig_types.go b/api/v1alpha/hyperfleetconfig_types.go deleted file mode 100644 index 4fb6780..0000000 --- a/api/v1alpha/hyperfleetconfig_types.go +++ /dev/null @@ -1,64 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1alpha - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! -// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. - -// HyperFleetConfigSpec defines the desired state of HyperFleetConfig. -type HyperFleetConfigSpec struct { - // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster - // Important: Run "make" to regenerate code after modifying this file - - // Foo is an example field of HyperFleetConfig. Edit hyperfleetconfig_types.go to remove/update - Foo string `json:"foo,omitempty"` -} - -// HyperFleetConfigStatus defines the observed state of HyperFleetConfig. -type HyperFleetConfigStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster - // Important: Run "make" to regenerate code after modifying this file -} - -// +kubebuilder:object:root=true -// +kubebuilder:subresource:status - -// HyperFleetConfig is the Schema for the hyperfleetconfigs API. -type HyperFleetConfig struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec HyperFleetConfigSpec `json:"spec,omitempty"` - Status HyperFleetConfigStatus `json:"status,omitempty"` -} - -// +kubebuilder:object:root=true - -// HyperFleetConfigList contains a list of HyperFleetConfig. -type HyperFleetConfigList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` - Items []HyperFleetConfig `json:"items"` -} - -func init() { - SchemeBuilder.Register(&HyperFleetConfig{}, &HyperFleetConfigList{}) -} diff --git a/api/v1alpha/groupversion_info.go b/api/v1alpha1/groupversion_info.go similarity index 89% rename from api/v1alpha/groupversion_info.go rename to api/v1alpha1/groupversion_info.go index 7b2a8b8..6bd5d79 100644 --- a/api/v1alpha/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -14,10 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package v1alpha contains API Schema definitions for the hyperfleet v1alpha API group. +// Package v1alpha1 contains API Schema definitions for the hyperfleet v1alpha1 API group. // +kubebuilder:object:generate=true // +groupName=hyperfleet.redhat.com -package v1alpha +package v1alpha1 import ( "k8s.io/apimachinery/pkg/runtime/schema" @@ -26,7 +26,7 @@ import ( var ( // GroupVersion is group version used to register these objects. - GroupVersion = schema.GroupVersion{Group: "hyperfleet.redhat.com", Version: "v1alpha"} + GroupVersion = schema.GroupVersion{Group: "hyperfleet.redhat.com", Version: "v1alpha1"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme. SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} diff --git a/api/v1alpha1/hyperfleetconfig_types.go b/api/v1alpha1/hyperfleetconfig_types.go new file mode 100644 index 0000000..b863f70 --- /dev/null +++ b/api/v1alpha1/hyperfleetconfig_types.go @@ -0,0 +1,272 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NOTE: json tags are required. Any new fields you add must have json tags for +// the fields to be serialized. Run "make generate manifests" after editing. + +// SingletonName is the only permitted name for a HyperFleetConfig. The resource +// is a cluster-scoped singleton: a CEL validation rule pins the name to +// "cluster", and cluster-scoped name uniqueness then guarantees at most one +// instance (a second create is rejected by the API server as AlreadyExists, not +// by CEL, since CEL cannot see other objects). No admission webhooks are used +// (see architecture ADR-0019). +const SingletonName = "cluster" + +// BundleType selects one of the operator-internal bundle definitions. It is a +// selector only: the CR carries the choice of a bundle, never its contents. The +// bundle controller (HYPERFLEET-1407) resolves the selected bundle into a +// concrete component set. +// +// These constants are the single source of truth for the valid bundle values +// and MUST stay in lockstep with the bundle definitions shipped inside the +// operator. Adding a bundle means adding a constant here and the matching enum +// value in the +kubebuilder:validation:Enum marker below. +// +// +kubebuilder:validation:Enum=cloud-capi;onprem-agent +type BundleType string + +const ( + // BundleCloudCAPI is the cloud, CAPI-based provisioning deployment + // (managed OpenShift on a public cloud). + BundleCloudCAPI BundleType = "cloud-capi" + // BundleOnPremAgent is the on-premise, air-gapped (agent-based) deployment. + BundleOnPremAgent BundleType = "onprem-agent" +) + +// AllBundleTypes is the Go-level source of truth for the valid BundleType +// values. The +kubebuilder:validation:Enum marker on BundleType must list +// exactly these values; the lockstep guard test creates a HyperFleetConfig with +// each entry and fails if any is not accepted by the CRD, catching drift between +// the constants and the enum marker. +var AllBundleTypes = []BundleType{BundleCloudCAPI, BundleOnPremAgent} + +// SizingProfile expresses sizing intent, not replica engineering. The operator +// maps each profile to concrete replicas, resource requests/limits and HPA/PDB +// defaults for the operand. +// +// +kubebuilder:validation:Enum=small;medium;large +type SizingProfile string + +const ( + // SizingProfileSmall is the default, lowest-footprint sizing profile. + SizingProfileSmall SizingProfile = "small" + // SizingProfileMedium is a mid-range sizing profile. + SizingProfileMedium SizingProfile = "medium" + // SizingProfileLarge is the highest-footprint sizing profile. + SizingProfileLarge SizingProfile = "large" +) + +// AllSizingProfiles is the Go-level source of truth for the valid SizingProfile +// values; it must match the +kubebuilder:validation:Enum marker on +// SizingProfile (asserted by the lockstep guard test). +var AllSizingProfiles = []SizingProfile{SizingProfileSmall, SizingProfileMedium, SizingProfileLarge} + +// Condition types reported on HyperFleetConfig status. This is deliberate +// operator-layer vocabulary describing installation health, and is distinct from +// the HyperFleet API's own resource-condition vocabulary (Available/Ready/ +// Reconciled/LastKnownReconciled/per-adapter; see architecture ADR-0007 and +// ADR-0008). The bundle controller (HYPERFLEET-1409) populates these; this story +// defines the schema only. +const ( + // ConditionAvailable is True when the installed operand (the API) is + // deployed and healthy. + ConditionAvailable = "Available" + // ConditionProgressing is True while the operator is actively rolling out a + // change to the operand. + ConditionProgressing = "Progressing" + // ConditionDegraded is True when the operator cannot reach or maintain the + // desired state. + ConditionDegraded = "Degraded" +) + +// SecretReference references a Secret by name. Referenced Secrets must live in +// the operator's own namespace: because HyperFleetConfig is cluster-scoped, no +// namespace field is exposed (name-only + operator-namespace convention, decided +// in the HYPERFLEET-1406 API review). +type SecretReference struct { + // name is the name of the Secret in the operator's namespace. It must be a + // valid DNS-1123 subdomain (the same constraint the API server places on + // Secret names) so an unresolvable reference is rejected at admission rather + // than failing opaquely when the reference is later resolved. + // + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` + Name string `json:"name"` +} + +// DatabaseSpec configures the HyperFleet API's connection to its external +// PostgreSQL database. The database is partner-provided; the operator never +// provisions it. +type DatabaseSpec struct { + // secretRef references a Secret holding the database connection credentials. + // The Secret must provide the keys db.host, db.port, db.name, db.user and + // db.password. + // + // +kubebuilder:validation:Required + SecretRef SecretReference `json:"secretRef"` +} + +// AuthSpec configures partner-facing JWT authentication intent for the API. +// Machinery details (JWKS rotation, public-path allowlist) remain +// operator-internal defaults and are not exposed here. +// +// +kubebuilder:validation:XValidation:rule="!self.enabled || (has(self.issuer) && has(self.audience))",message="issuer and audience are required when auth is enabled" +type AuthSpec struct { + // enabled turns JWT authentication on for the API endpoint. It defaults to + // true, so a config that omits it gets authentication ON. It is a pointer to + // distinguish "unset" (apply the default, true) from an explicit false + // (disable auth), which a non-pointer bool cannot express: with omitempty a + // plain false is dropped and re-defaulted to true, so auth could never be + // turned off via the typed client; without omitempty an unset field serializes + // as false and suppresses the default. Only *bool avoids both traps. + // + // +kubebuilder:default=true + // +optional + Enabled *bool `json:"enabled,omitempty"` + + // issuer is the OIDC issuer URL that mints accepted tokens. Required when + // enabled is true. Whenever it is set (regardless of enabled) it must be a + // valid https URL with a host, so a malformed issuer is rejected at admission + // rather than surfacing later at token-validation time. + // + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=2048 + // +kubebuilder:validation:XValidation:rule="isURL(self) && url(self).getScheme() == 'https' && url(self).getHostname() != ''",message="issuer must be a valid https URL" + // +optional + Issuer string `json:"issuer,omitempty"` + + // audience is the token audience the API requires. Required and non-empty + // when enabled is true. + // + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +optional + Audience string `json:"audience,omitempty"` +} + +// TLSSpec configures TLS for the API endpoint. The certificate material is +// referenced, not described. +type TLSSpec struct { + // secretRef references a kubernetes.io/tls Secret (providing tls.crt and + // tls.key) used to serve the API endpoint. + // + // +kubebuilder:validation:Required + SecretRef SecretReference `json:"secretRef"` +} + +// APISpec is the partner-facing configuration for the HyperFleet API component, +// which lives in the shared tier of every bundle. +type APISpec struct { + // database configures the external PostgreSQL connection. + // + // +kubebuilder:validation:Required + Database DatabaseSpec `json:"database"` + + // auth configures partner-facing JWT authentication intent. + // + // +kubebuilder:validation:Required + Auth AuthSpec `json:"auth"` + + // tls optionally configures TLS for the API endpoint. When omitted, the + // operator applies its default serving configuration. + // + // +optional + TLS *TLSSpec `json:"tls,omitempty"` + + // profile selects a sizing profile for the API. Defaults to "small". + // + // +kubebuilder:default=small + // +optional + Profile SizingProfile `json:"profile,omitempty"` +} + +// HyperFleetConfigSpec defines the desired state of HyperFleetConfig. It captures +// partner intent only; internal machinery (broker, adapters, Sentinel) is never +// expressed here. +type HyperFleetConfigSpec struct { + // bundle selects one of the operator-internal bundle definitions. It is + // immutable after creation: switching deployments requires recreating the + // resource. + // + // +kubebuilder:validation:Required + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="bundle is immutable" + Bundle BundleType `json:"bundle"` + + // api is the partner-facing configuration for the HyperFleet API component. + // + // +kubebuilder:validation:Required + API APISpec `json:"api"` +} + +// HyperFleetConfigStatus defines the observed state of HyperFleetConfig. It is +// populated by the bundle controller in later stories; this story defines the +// schema only. +type HyperFleetConfigStatus struct { + // observedGeneration is the .metadata.generation the operator last acted on. + // + // +kubebuilder:validation:Minimum=0 + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // conditions represent the current installation health of the operand. + // Recognized types are Available, Progressing and Degraded. + // + // +listType=map + // +listMapKey=type + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:resource:scope=Cluster,shortName=hfc +// +kubebuilder:validation:XValidation:rule="self.metadata.name == 'cluster'",message="the only permitted name is 'cluster'; HyperFleetConfig is a cluster-scoped singleton" +// +kubebuilder:printcolumn:name="Bundle",type=string,JSONPath=`.spec.bundle` +// +kubebuilder:printcolumn:name="Profile",type=string,JSONPath=`.spec.api.profile` +// +kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.conditions[?(@.type=="Available")].status` +// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` + +// HyperFleetConfig is the Schema for the hyperfleetconfigs API. It is a +// cluster-scoped singleton: exactly one instance, named "cluster", is permitted. +type HyperFleetConfig struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // +kubebuilder:validation:Required + Spec HyperFleetConfigSpec `json:"spec"` + Status HyperFleetConfigStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// HyperFleetConfigList contains a list of HyperFleetConfig. +type HyperFleetConfigList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []HyperFleetConfig `json:"items"` +} + +func init() { + SchemeBuilder.Register(&HyperFleetConfig{}, &HyperFleetConfigList{}) +} diff --git a/api/v1alpha/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go similarity index 55% rename from api/v1alpha/zz_generated.deepcopy.go rename to api/v1alpha1/zz_generated.deepcopy.go index 3ea7438..0c15ec4 100644 --- a/api/v1alpha/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -18,19 +18,78 @@ limitations under the License. // Code generated by controller-gen. DO NOT EDIT. -package v1alpha +package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *APISpec) DeepCopyInto(out *APISpec) { + *out = *in + out.Database = in.Database + in.Auth.DeepCopyInto(&out.Auth) + if in.TLS != nil { + in, out := &in.TLS, &out.TLS + *out = new(TLSSpec) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APISpec. +func (in *APISpec) DeepCopy() *APISpec { + if in == nil { + return nil + } + out := new(APISpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AuthSpec) DeepCopyInto(out *AuthSpec) { + *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthSpec. +func (in *AuthSpec) DeepCopy() *AuthSpec { + if in == nil { + return nil + } + out := new(AuthSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DatabaseSpec) DeepCopyInto(out *DatabaseSpec) { + *out = *in + out.SecretRef = in.SecretRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DatabaseSpec. +func (in *DatabaseSpec) DeepCopy() *DatabaseSpec { + if in == nil { + return nil + } + out := new(DatabaseSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HyperFleetConfig) DeepCopyInto(out *HyperFleetConfig) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec - out.Status = in.Status + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HyperFleetConfig. @@ -86,6 +145,7 @@ func (in *HyperFleetConfigList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HyperFleetConfigSpec) DeepCopyInto(out *HyperFleetConfigSpec) { *out = *in + in.API.DeepCopyInto(&out.API) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HyperFleetConfigSpec. @@ -101,6 +161,13 @@ func (in *HyperFleetConfigSpec) DeepCopy() *HyperFleetConfigSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HyperFleetConfigStatus) DeepCopyInto(out *HyperFleetConfigStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HyperFleetConfigStatus. @@ -112,3 +179,34 @@ func (in *HyperFleetConfigStatus) DeepCopy() *HyperFleetConfigStatus { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretReference) DeepCopyInto(out *SecretReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretReference. +func (in *SecretReference) DeepCopy() *SecretReference { + if in == nil { + return nil + } + out := new(SecretReference) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { + *out = *in + out.SecretRef = in.SecretRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSSpec. +func (in *TLSSpec) DeepCopy() *TLSSpec { + if in == nil { + return nil + } + out := new(TLSSpec) + in.DeepCopyInto(out) + return out +} diff --git a/cmd/main.go b/cmd/main.go index b0b1437..b3eb955 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -37,7 +37,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" - hyperfleetv1alpha "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha" + hyperfleetv1alpha1 "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1" "github.com/openshift-hyperfleet/hyperfleet-operator/internal/controller" // +kubebuilder:scaffold:imports ) @@ -50,7 +50,7 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(hyperfleetv1alpha.AddToScheme(scheme)) + utilruntime.Must(hyperfleetv1alpha1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } diff --git a/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml b/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml new file mode 100644 index 0000000..2b0930d --- /dev/null +++ b/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml @@ -0,0 +1,270 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.21.0 + name: hyperfleetconfigs.hyperfleet.redhat.com +spec: + group: hyperfleet.redhat.com + names: + kind: HyperFleetConfig + listKind: HyperFleetConfigList + plural: hyperfleetconfigs + shortNames: + - hfc + singular: hyperfleetconfig + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .spec.bundle + name: Bundle + type: string + - jsonPath: .spec.api.profile + name: Profile + type: string + - jsonPath: .status.conditions[?(@.type=="Available")].status + name: Available + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + HyperFleetConfig is the Schema for the hyperfleetconfigs API. It is a + cluster-scoped singleton: exactly one instance, named "cluster", is permitted. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: |- + HyperFleetConfigSpec defines the desired state of HyperFleetConfig. It captures + partner intent only; internal machinery (broker, adapters, Sentinel) is never + expressed here. + properties: + api: + description: api is the partner-facing configuration for the HyperFleet + API component. + properties: + auth: + description: auth configures partner-facing JWT authentication + intent. + properties: + audience: + description: |- + audience is the token audience the API requires. Required and non-empty + when enabled is true. + maxLength: 253 + minLength: 1 + type: string + enabled: + default: true + description: |- + enabled turns JWT authentication on for the API endpoint. It defaults to + true, so a config that omits it gets authentication ON. It is a pointer to + distinguish "unset" (apply the default, true) from an explicit false + (disable auth), which a non-pointer bool cannot express: with omitempty a + plain false is dropped and re-defaulted to true, so auth could never be + turned off via the typed client; without omitempty an unset field serializes + as false and suppresses the default. Only *bool avoids both traps. + type: boolean + issuer: + description: |- + issuer is the OIDC issuer URL that mints accepted tokens. Required when + enabled is true. Whenever it is set (regardless of enabled) it must be a + valid https URL with a host, so a malformed issuer is rejected at admission + rather than surfacing later at token-validation time. + maxLength: 2048 + minLength: 1 + type: string + x-kubernetes-validations: + - message: issuer must be a valid https URL + rule: isURL(self) && url(self).getScheme() == 'https' && + url(self).getHostname() != '' + type: object + x-kubernetes-validations: + - message: issuer and audience are required when auth is enabled + rule: '!self.enabled || (has(self.issuer) && has(self.audience))' + database: + description: database configures the external PostgreSQL connection. + properties: + secretRef: + description: |- + secretRef references a Secret holding the database connection credentials. + The Secret must provide the keys db.host, db.port, db.name, db.user and + db.password. + properties: + name: + description: |- + name is the name of the Secret in the operator's namespace. It must be a + valid DNS-1123 subdomain (the same constraint the API server places on + Secret names) so an unresolvable reference is rejected at admission rather + than failing opaquely when the reference is later resolved. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - name + type: object + required: + - secretRef + type: object + profile: + default: small + description: profile selects a sizing profile for the API. Defaults + to "small". + enum: + - small + - medium + - large + type: string + tls: + description: |- + tls optionally configures TLS for the API endpoint. When omitted, the + operator applies its default serving configuration. + properties: + secretRef: + description: |- + secretRef references a kubernetes.io/tls Secret (providing tls.crt and + tls.key) used to serve the API endpoint. + properties: + name: + description: |- + name is the name of the Secret in the operator's namespace. It must be a + valid DNS-1123 subdomain (the same constraint the API server places on + Secret names) so an unresolvable reference is rejected at admission rather + than failing opaquely when the reference is later resolved. + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - name + type: object + required: + - secretRef + type: object + required: + - auth + - database + type: object + bundle: + description: |- + bundle selects one of the operator-internal bundle definitions. It is + immutable after creation: switching deployments requires recreating the + resource. + enum: + - cloud-capi + - onprem-agent + type: string + x-kubernetes-validations: + - message: bundle is immutable + rule: self == oldSelf + required: + - api + - bundle + type: object + status: + description: |- + HyperFleetConfigStatus defines the observed state of HyperFleetConfig. It is + populated by the bundle controller in later stories; this story defines the + schema only. + properties: + conditions: + description: |- + conditions represent the current installation health of the operand. + Recognized types are Available, Progressing and Degraded. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + observedGeneration: + description: observedGeneration is the .metadata.generation the operator + last acted on. + format: int64 + minimum: 0 + type: integer + type: object + required: + - spec + type: object + x-kubernetes-validations: + - message: the only permitted name is 'cluster'; HyperFleetConfig is a cluster-scoped + singleton + rule: self.metadata.name == 'cluster' + served: true + storage: true + subresources: + status: {} diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index db7eabe..e3e10d9 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -1,11 +1,32 @@ +--- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - labels: - app.kubernetes.io/name: hyperfleet-operator - app.kubernetes.io/managed-by: kustomize name: manager-role rules: -- apiGroups: [""] - resources: ["pods"] - verbs: ["get", "list", "watch"] +- apiGroups: + - hyperfleet.redhat.com + resources: + - hyperfleetconfigs + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - hyperfleet.redhat.com + resources: + - hyperfleetconfigs/finalizers + verbs: + - update +- apiGroups: + - hyperfleet.redhat.com + resources: + - hyperfleetconfigs/status + verbs: + - get + - patch + - update diff --git a/config/samples/hyperfleet_v1alpha1_hyperfleetconfig.yaml b/config/samples/hyperfleet_v1alpha1_hyperfleetconfig.yaml new file mode 100644 index 0000000..06907e0 --- /dev/null +++ b/config/samples/hyperfleet_v1alpha1_hyperfleetconfig.yaml @@ -0,0 +1,27 @@ +apiVersion: hyperfleet.redhat.com/v1alpha1 +kind: HyperFleetConfig +metadata: + labels: + app.kubernetes.io/name: hyperfleet-operator + app.kubernetes.io/managed-by: kustomize + # HyperFleetConfig is a cluster-scoped singleton; the only permitted name is + # "cluster". It has no namespace. + name: cluster +spec: + # bundle selects the deployment shape. It is immutable after creation. + # One of: cloud-capi, onprem-agent. + bundle: cloud-capi + api: + # database.secretRef points at a Secret in the operator's namespace holding + # keys: db.host, db.port, db.name, db.user, db.password. + database: + secretRef: + name: hyperfleet-db + # auth configures partner-facing JWT authentication intent. issuer and + # audience are required when enabled is true (the default). + auth: + enabled: true + issuer: https://issuer.example.com + audience: hyperfleet-api + # profile is a sizing hint. One of: small (default), medium, large. + profile: small diff --git a/config/samples/hyperfleet_v1alpha_hyperfleetconfig.yaml b/config/samples/hyperfleet_v1alpha_hyperfleetconfig.yaml deleted file mode 100644 index 38000b6..0000000 --- a/config/samples/hyperfleet_v1alpha_hyperfleetconfig.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: hyperfleet.redhat.com/v1alpha -kind: HyperFleetConfig -metadata: - labels: - app.kubernetes.io/name: hyperfleet-operator - app.kubernetes.io/managed-by: kustomize - name: hyperfleetconfig-sample -spec: - # TODO(user): Add fields here diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index 7f70873..2806bf3 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -1,4 +1,4 @@ ## Append samples of your project ## resources: -- hyperfleet_v1alpha_hyperfleetconfig.yaml +- hyperfleet_v1alpha1_hyperfleetconfig.yaml # +kubebuilder:scaffold:manifestskustomizesamples diff --git a/go.mod b/go.mod index baa5e32..bcd00ea 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/onsi/gomega v1.36.1 k8s.io/apimachinery v0.33.0 k8s.io/client-go v0.33.0 + k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 sigs.k8s.io/controller-runtime v0.21.0 ) @@ -88,7 +89,6 @@ require ( k8s.io/component-base v0.33.0 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff // indirect - k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 // indirect sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect sigs.k8s.io/randfill v1.0.0 // indirect diff --git a/internal/controller/hyperfleetconfig_controller.go b/internal/controller/hyperfleetconfig_controller.go index 40ff9c6..b96de15 100644 --- a/internal/controller/hyperfleetconfig_controller.go +++ b/internal/controller/hyperfleetconfig_controller.go @@ -24,7 +24,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" - hyperfleetv1alpha "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha" + hyperfleetv1alpha1 "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1" ) // HyperFleetConfigReconciler reconciles a HyperFleetConfig object @@ -57,7 +57,7 @@ func (r *HyperFleetConfigReconciler) Reconcile(ctx context.Context, req ctrl.Req // SetupWithManager sets up the controller with the Manager. func (r *HyperFleetConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&hyperfleetv1alpha.HyperFleetConfig{}). + For(&hyperfleetv1alpha1.HyperFleetConfig{}). Named("hyperfleetconfig"). Complete(r) } diff --git a/internal/controller/hyperfleetconfig_controller_test.go b/internal/controller/hyperfleetconfig_controller_test.go index d6951b0..67e7aed 100644 --- a/internal/controller/hyperfleetconfig_controller_test.go +++ b/internal/controller/hyperfleetconfig_controller_test.go @@ -23,48 +23,73 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/reconcile" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - hyperfleetv1alpha "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha" + hyperfleetv1alpha1 "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1" ) var _ = Describe("HyperFleetConfig Controller", func() { Context("When reconciling a resource", func() { - const resourceName = "test-resource" + // HyperFleetConfig is a cluster-scoped singleton: the only permitted name + // is "cluster" and there is no namespace. + const resourceName = hyperfleetv1alpha1.SingletonName ctx := context.Background() typeNamespacedName := types.NamespacedName{ - Name: resourceName, - Namespace: "default", // TODO(user):Modify as needed + Name: resourceName, } - hyperfleetconfig := &hyperfleetv1alpha.HyperFleetConfig{} + hyperfleetconfig := &hyperfleetv1alpha1.HyperFleetConfig{} BeforeEach(func() { By("creating the custom resource for the Kind HyperFleetConfig") err := k8sClient.Get(ctx, typeNamespacedName, hyperfleetconfig) - if err != nil && errors.IsNotFound(err) { - resource := &hyperfleetv1alpha.HyperFleetConfig{ + if errors.IsNotFound(err) { + resource := &hyperfleetv1alpha1.HyperFleetConfig{ ObjectMeta: metav1.ObjectMeta{ - Name: resourceName, - Namespace: "default", + Name: resourceName, + }, + Spec: hyperfleetv1alpha1.HyperFleetConfigSpec{ + Bundle: hyperfleetv1alpha1.BundleCloudCAPI, + API: hyperfleetv1alpha1.APISpec{ + Database: hyperfleetv1alpha1.DatabaseSpec{ + SecretRef: hyperfleetv1alpha1.SecretReference{Name: "hyperfleet-db"}, + }, + Auth: hyperfleetv1alpha1.AuthSpec{ + Enabled: ptr.To(true), + Issuer: "https://issuer.example.com", + Audience: "hyperfleet-api", + }, + }, }, - // TODO(user): Specify other spec details if needed. } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } else { + // A Get error other than NotFound means the fixture state is unknown; + // fail loudly instead of silently skipping the Create and letting the + // reconcile spec pass without its required resource. + Expect(err).NotTo(HaveOccurred()) } }) AfterEach(func() { - // TODO(user): Cleanup logic after each test, like removing the resource instance. - resource := &hyperfleetv1alpha.HyperFleetConfig{} - err := k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) - By("Cleanup the specific resource instance HyperFleetConfig") - Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + resource := &hyperfleetv1alpha1.HyperFleetConfig{ + ObjectMeta: metav1.ObjectMeta{Name: resourceName}, + } + // NotFound means cleanup already happened; any other delete error + // must fail the test loudly instead of being masked as an Eventually + // timeout below. + if err := k8sClient.Delete(ctx, resource); err != nil && !errors.IsNotFound(err) { + Expect(err).NotTo(HaveOccurred()) + } + Eventually(func() bool { + err := k8sClient.Get(ctx, typeNamespacedName, &hyperfleetv1alpha1.HyperFleetConfig{}) + return errors.IsNotFound(err) + }).Should(BeTrue()) }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource") @@ -77,8 +102,6 @@ var _ = Describe("HyperFleetConfig Controller", func() { NamespacedName: typeNamespacedName, }) Expect(err).NotTo(HaveOccurred()) - // TODO(user): Add more specific assertions depending on your controller's reconciliation logic. - // Example: If you expect a certain status condition after reconciliation, verify it here. }) }) }) diff --git a/internal/controller/hyperfleetconfig_types_test.go b/internal/controller/hyperfleetconfig_types_test.go new file mode 100644 index 0000000..9018015 --- /dev/null +++ b/internal/controller/hyperfleetconfig_types_test.go @@ -0,0 +1,406 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + + hyperfleetv1alpha1 "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1" +) + +// These specs exercise the CRD's declarative validation and defaulting through a +// real API server (envtest). They assert the schema contract for HYPERFLEET-1406; +// no reconciler behavior is involved. + +// validHyperFleetConfig returns a well-formed, minimally-complete singleton that +// the API server must accept. Individual specs mutate a copy to drive negative +// cases. +func validHyperFleetConfig() *hyperfleetv1alpha1.HyperFleetConfig { + return &hyperfleetv1alpha1.HyperFleetConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: hyperfleetv1alpha1.SingletonName, + }, + Spec: hyperfleetv1alpha1.HyperFleetConfigSpec{ + Bundle: hyperfleetv1alpha1.BundleCloudCAPI, + API: hyperfleetv1alpha1.APISpec{ + Database: hyperfleetv1alpha1.DatabaseSpec{ + SecretRef: hyperfleetv1alpha1.SecretReference{Name: "hyperfleet-db"}, + }, + Auth: hyperfleetv1alpha1.AuthSpec{ + Enabled: ptr.To(true), + Issuer: "https://issuer.example.com", + Audience: "hyperfleet-api", + }, + }, + }, + } +} + +var _ = Describe("HyperFleetConfig CRD validation", func() { + ctx := context.Background() + + // Every valid object is forced to the same name ("cluster"), so specs collide + // unless the singleton is torn down between them. + AfterEach(func() { + obj := &hyperfleetv1alpha1.HyperFleetConfig{ + ObjectMeta: metav1.ObjectMeta{Name: hyperfleetv1alpha1.SingletonName}, + } + // A negative spec may have prevented creation, so NotFound is a valid + // "already clean" outcome; any other delete error must fail the test + // loudly rather than be masked as an Eventually timeout below. + if err := k8sClient.Delete(ctx, obj); err != nil && !errors.IsNotFound(err) { + Expect(err).NotTo(HaveOccurred()) + } + Eventually(func() bool { + err := k8sClient.Get(ctx, + types.NamespacedName{Name: hyperfleetv1alpha1.SingletonName}, + &hyperfleetv1alpha1.HyperFleetConfig{}) + return errors.IsNotFound(err) + }).Should(BeTrue()) + }) + + Context("singleton enforcement", func() { + It("accepts the resource when named 'cluster'", func() { + Expect(k8sClient.Create(ctx, validHyperFleetConfig())).To(Succeed()) + }) + + It("rejects any name other than 'cluster'", func() { + obj := validHyperFleetConfig() + obj.Name = "not-cluster" + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("only permitted name is 'cluster'")) + }) + + It("rejects a second instance as AlreadyExists", func() { + // The name-pin CEL rule forces every instance to the name "cluster"; + // cluster-scoped name uniqueness then makes the second create collide. + Expect(k8sClient.Create(ctx, validHyperFleetConfig())).To(Succeed()) + err := k8sClient.Create(ctx, validHyperFleetConfig()) + Expect(err).To(HaveOccurred()) + Expect(errors.IsAlreadyExists(err)).To(BeTrue()) + }) + }) + + Context("enum validation", func() { + // Positive coverage: every constant declared in the Go source must be + // accepted by the generated enum. Generating one spec per value guards + // against the constants and the +kubebuilder:validation:Enum marker drifting + // apart. + for _, bundle := range hyperfleetv1alpha1.AllBundleTypes { + It("accepts declared bundle value "+string(bundle), func() { + obj := validHyperFleetConfig() + obj.Spec.Bundle = bundle + Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + }) + } + + for _, profile := range hyperfleetv1alpha1.AllSizingProfiles { + It("accepts declared sizing profile "+string(profile), func() { + obj := validHyperFleetConfig() + obj.Spec.API.Profile = profile + Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + }) + } + + It("rejects an unknown bundle", func() { + obj := validHyperFleetConfig() + obj.Spec.Bundle = "bogus-bundle" + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Unsupported value")) + }) + + It("rejects an unknown sizing profile", func() { + obj := validHyperFleetConfig() + obj.Spec.API.Profile = "huge" + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Unsupported value")) + }) + }) + + Context("bundle immutability", func() { + It("rejects changing the bundle after creation", func() { + obj := validHyperFleetConfig() + Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + + obj.Spec.Bundle = hyperfleetv1alpha1.BundleOnPremAgent + err := k8sClient.Update(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("bundle is immutable")) + }) + + It("allows changing a mutable field (profile) after creation", func() { + // The immutability rule is scoped to bundle; other fields must remain + // updatable. This proves self==oldSelf was not applied too broadly. + obj := validHyperFleetConfig() + Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + + obj.Spec.API.Profile = hyperfleetv1alpha1.SizingProfileLarge + Expect(k8sClient.Update(ctx, obj)).To(Succeed()) + + got := &hyperfleetv1alpha1.HyperFleetConfig{} + Expect(k8sClient.Get(ctx, + types.NamespacedName{Name: hyperfleetv1alpha1.SingletonName}, got)).To(Succeed()) + Expect(got.Spec.API.Profile).To(Equal(hyperfleetv1alpha1.SizingProfileLarge)) + }) + }) + + Context("auth validation", func() { + It("rejects enabled auth without issuer and audience", func() { + obj := validHyperFleetConfig() + obj.Spec.API.Auth = hyperfleetv1alpha1.AuthSpec{Enabled: ptr.To(true)} + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("issuer and audience are required when auth is enabled")) + }) + + It("rejects a non-https issuer", func() { + obj := validHyperFleetConfig() + obj.Spec.API.Auth.Issuer = "http://issuer.example.com" + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("issuer must be a valid https URL")) + }) + + It("rejects a scheme-only issuer with no host", func() { + // "https://" satisfies a naive startsWith('https://') check but has no + // host; the URL-library rule must reject it. + obj := validHyperFleetConfig() + obj.Spec.API.Auth.Issuer = "https://" + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("issuer must be a valid https URL")) + }) + + It("rejects a present but empty issuer", func() { + // An explicit empty issuer is "present" to CEL, so MinLength (not the + // https XValidation alone) must reject it. Only unstructured can carry an + // explicit empty scalar; the typed client omits it via omitempty. + u := minimalUnstructured() + auth := u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{})["auth"].(map[string]interface{}) + auth["issuer"] = "" + err := k8sClient.Create(ctx, u) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("should be at least 1 chars long")) + }) + + It("rejects an issuer longer than the 2048-character maximum", func() { + // A long-but-valid https URL isolates MaxLength: isURL, scheme and host + // all pass, so only the length bound can reject it. + obj := validHyperFleetConfig() + obj.Spec.API.Auth.Issuer = "https://issuer.example.com/" + strings.Repeat("a", 2048) + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("may not be more than 2048 bytes")) + }) + + It("rejects an empty audience when auth is enabled", func() { + // An explicit empty string is "present" to CEL has(), so the object-level + // rule alone would let it through; MinLength must reject it. Only an + // unstructured object can carry an explicit empty scalar (the typed client + // omits it via omitempty). + u := minimalUnstructured() + u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{})["auth"] = + map[string]interface{}{ + "enabled": true, + "issuer": "https://issuer.example.com", + "audience": "", + } + err := k8sClient.Create(ctx, u) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("should be at least 1 chars long")) + }) + + It("rejects an audience longer than the 253-character maximum", func() { + obj := validHyperFleetConfig() + obj.Spec.API.Auth.Audience = strings.Repeat("a", 254) + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("may not be more than 253 bytes")) + }) + + It("accepts disabled auth without issuer and audience", func() { + obj := validHyperFleetConfig() + obj.Spec.API.Auth = hyperfleetv1alpha1.AuthSpec{Enabled: ptr.To(false)} + Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + }) + }) + + Context("required fields", func() { + It("rejects an empty database secret name", func() { + obj := validHyperFleetConfig() + obj.Spec.API.Database.SecretRef.Name = "" + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + // Assert the MinLength message specifically: an empty name also trips the + // DNS-1123 Pattern, so checking only the field path would keep passing if + // MinLength=1 were dropped. This isolates the MinLength constraint. + Expect(err.Error()).To(ContainSubstring("should be at least 1 chars long")) + }) + + It("rejects a database secret name that is not a DNS-1123 subdomain", func() { + obj := validHyperFleetConfig() + obj.Spec.API.Database.SecretRef.Name = "Invalid_Name" + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("database.secretRef.name")) + }) + + It("rejects a database secret name longer than the 253-character maximum", func() { + // A 254-char all-lowercase name satisfies the DNS-1123 Pattern, so only + // MaxLength can reject it. + obj := validHyperFleetConfig() + obj.Spec.API.Database.SecretRef.Name = strings.Repeat("a", 254) + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("may not be more than 253 bytes")) + }) + }) + + Context("optional TLS", func() { + It("accepts a well-formed TLS secret reference", func() { + obj := validHyperFleetConfig() + obj.Spec.API.TLS = &hyperfleetv1alpha1.TLSSpec{ + SecretRef: hyperfleetv1alpha1.SecretReference{Name: "hyperfleet-tls"}, + } + Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + }) + + It("rejects a TLS block with an empty secret name", func() { + obj := validHyperFleetConfig() + obj.Spec.API.TLS = &hyperfleetv1alpha1.TLSSpec{ + SecretRef: hyperfleetv1alpha1.SecretReference{Name: ""}, + } + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("tls.secretRef.name")) + }) + + It("rejects a TLS secret name longer than the 253-character maximum", func() { + obj := validHyperFleetConfig() + obj.Spec.API.TLS = &hyperfleetv1alpha1.TLSSpec{ + SecretRef: hyperfleetv1alpha1.SecretReference{Name: strings.Repeat("a", 254)}, + } + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("may not be more than 253 bytes")) + }) + }) + + Context("required object fields", func() { + // Structural-schema Required markers must reject an object that omits a + // mandatory field entirely. Only an unstructured object can omit fields the + // typed client always sends. + It("rejects a config with no bundle", func() { + u := minimalUnstructured() + delete(u.Object["spec"].(map[string]interface{}), "bundle") + err := k8sClient.Create(ctx, u) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("spec.bundle")) + }) + + It("rejects a config with no api", func() { + u := minimalUnstructured() + delete(u.Object["spec"].(map[string]interface{}), "api") + err := k8sClient.Create(ctx, u) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("spec.api")) + }) + + It("rejects an api with no database", func() { + u := minimalUnstructured() + api := u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{}) + delete(api, "database") + err := k8sClient.Create(ctx, u) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("database")) + }) + + It("rejects an api with no auth", func() { + u := minimalUnstructured() + api := u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{}) + delete(api, "auth") + err := k8sClient.Create(ctx, u) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("auth")) + }) + }) + + Context("defaulting", func() { + // Server-side defaults only apply to fields genuinely absent from the + // submitted object. An unstructured object is used to omit them entirely. + It("populates profile and auth.enabled defaults on a minimal object", func() { + u := minimalUnstructured() + Expect(k8sClient.Create(ctx, u)).To(Succeed()) + + got := &hyperfleetv1alpha1.HyperFleetConfig{} + Expect(k8sClient.Get(ctx, + types.NamespacedName{Name: hyperfleetv1alpha1.SingletonName}, got)).To(Succeed()) + Expect(got.Spec.API.Profile).To(Equal(hyperfleetv1alpha1.SizingProfileSmall)) + Expect(got.Spec.API.Auth.Enabled).To(HaveValue(BeTrue())) + }) + + It("defaults auth.enabled to true on the typed-client path when omitted", func() { + // Regression guard: with a non-pointer bool the typed client serializes + // enabled:false when the field is left zero, silently defeating the + // default=true. A *bool with omitempty must round-trip to true here. + obj := validHyperFleetConfig() + obj.Spec.API.Auth.Enabled = nil + Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + + got := &hyperfleetv1alpha1.HyperFleetConfig{} + Expect(k8sClient.Get(ctx, + types.NamespacedName{Name: hyperfleetv1alpha1.SingletonName}, got)).To(Succeed()) + Expect(got.Spec.API.Auth.Enabled).To(HaveValue(BeTrue())) + }) + }) +}) + +// minimalUnstructured builds the smallest valid HyperFleetConfig as an +// unstructured object, omitting every field that carries a server-side default +// (auth.enabled, api.profile) so those defaults are exercised. +func minimalUnstructured() *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(hyperfleetv1alpha1.GroupVersion.WithKind("HyperFleetConfig")) + u.SetName(hyperfleetv1alpha1.SingletonName) + u.Object["spec"] = map[string]interface{}{ + "bundle": string(hyperfleetv1alpha1.BundleCloudCAPI), + "api": map[string]interface{}{ + "database": map[string]interface{}{ + "secretRef": map[string]interface{}{"name": "hyperfleet-db"}, + }, + "auth": map[string]interface{}{ + // enabled omitted -> defaults to true + "issuer": "https://issuer.example.com", + "audience": "hyperfleet-api", + }, + // profile omitted -> defaults to "small" + }, + } + return u +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 308cc40..dcac0ce 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -32,7 +32,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - hyperfleetv1alpha "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha" + hyperfleetv1alpha1 "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1" // +kubebuilder:scaffold:imports ) @@ -59,7 +59,7 @@ var _ = BeforeSuite(func() { ctx, cancel = context.WithCancel(context.TODO()) var err error - err = hyperfleetv1alpha.AddToScheme(scheme.Scheme) + err = hyperfleetv1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) // +kubebuilder:scaffold:scheme From bda67ecd21d740873a02b1f23d5c7428c748f24c Mon Sep 17 00:00:00 2001 From: tithakka Date: Mon, 17 Aug 2026 19:38:05 -0500 Subject: [PATCH 2/4] HYPERFLEET-1406 - feat: fix pipeline and coderabbit suggestions --- .github/workflows/lint.yml | 2 +- .gitignore | 1 + Makefile | 38 ++++++++++++------- go.mod | 2 +- .../hyperfleetconfig_controller_test.go | 6 +-- .../controller/hyperfleetconfig_types_test.go | 25 ++++++++---- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 86e3845..f4b9ce6 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -20,4 +20,4 @@ jobs: - name: Run linter uses: golangci/golangci-lint-action@v8 with: - version: v2.1.0 + version: v2.12.2 diff --git a/.gitignore b/.gitignore index 9f0f3a1..3513d3f 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ *.so *.dylib bin/* +_output/ Dockerfile.cross # Test binary, built with `go test -c` diff --git a/Makefile b/Makefile index e3d5168..2e3b505 100644 --- a/Makefile +++ b/Makefile @@ -1,12 +1,3 @@ -# GOTOOLCHAIN pins the Go toolchain for all build/generate/test targets. -# The tooling module (tools/go.mod) requires Go >= 1.26, while the root module -# keeps its language level at `go 1.24` so the pinned golangci-lint (built with -# go1.24) still accepts go.mod. Forcing one toolchain here — rather than a -# go.mod `toolchain` directive — avoids GOTOOLCHAIN=auto switching mid-build, -# which breaks coverage ("go: no such tool covdata"). go1.26.5 is the first -# release carrying the fixes for CVE-2026-39822 and CVE-2026-42505. -export GOTOOLCHAIN ?= go1.26.5 - # VERSION defines the project version for the bundle. # Update this value when you upgrade the version of your project. # To re-generate a bundle for another specific version without changing the standard setup, you can: @@ -132,6 +123,15 @@ vet: ## Run go vet against code. ENVTEST_K8S_VERSION ?= $(shell go list -m -f "{{ .Version }}" k8s.io/api | awk -F'[v.]' '{printf "1.%d", $$3}') KIND_CLUSTER ?= hyperfleet-operator-test-e2e +# On Linux, setup-envtest stores its downloaded control-plane binaries under +# $XDG_DATA_HOME, falling back to $HOME/.local/share (store/helpers.go). OpenShift +# CI's unit step runs with HOME=/ (unwritable) and no XDG_DATA_HOME set, so the +# store resolves to /.local/share and setup-envtest dies with "mkdir /.local: +# permission denied" — leaving KUBEBUILDER_ASSETS empty and the suite unable to +# find etcd. Point XDG_DATA_HOME at a writable path inside the repo so the store +# is always creatable. `?=` lets a developer or CI override it. +export XDG_DATA_HOME ?= $(ROOT_DIR)_output/.local/share + # TODO(user): To use a different vendor for e2e tests, modify the setup under 'tests/e2e'. # The default setup assumes Kind is pre-installed and builds/loads the Manager Docker image locally. @@ -140,11 +140,21 @@ KIND_CLUSTER ?= hyperfleet-operator-test-e2e .PHONY: test test: manifests generate fmt vet ## Run tests. - # Resolve envtest assets in the recipe shell (via $$(...)) rather than make's - # $(shell ...): the latter runs at parse time and does not inherit the - # exported GOTOOLCHAIN, so setup-envtest (a tools/go.mod tool needing Go 1.26) - # would run under the wrong toolchain and yield an empty KUBEBUILDER_ASSETS. - KUBEBUILDER_ASSETS="$$($(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out + # Honor a KUBEBUILDER_ASSETS supplied by the environment (e.g. an OpenShift CI + # pod that pre-bakes the envtest control-plane binaries); fall back to + # setup-envtest only when it is unset. Resolve that fallback in the recipe + # shell (via $$(...)) rather than make's $(shell ...): $(shell ...) runs at + # parse time on every make invocation (even unrelated targets) and cannot see + # a KUBEBUILDER_ASSETS exported into the recipe environment. + # + # Assign on its own line so that, under .SHELLFLAGS -e, a setup-envtest failure + # aborts here with its real error instead of being masked by go test's exit + # status (a bare `VAR="$$(cmd)" go test` reports go test's status, not cmd's). + # Then require a non-empty path so a silent empty resolve can't launch the + # suite with no control-plane binaries ("etcd: executable file not found"). + assets="$${KUBEBUILDER_ASSETS:-$$($(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)}"; \ + [ -n "$$assets" ] || { echo "error: KUBEBUILDER_ASSETS is empty; setup-envtest did not resolve envtest binaries" >&2; exit 1; }; \ + KUBEBUILDER_ASSETS="$$assets" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out .PHONY: setup-test-e2e setup-test-e2e: ## Set up a Kind cluster for e2e tests if it does not exist diff --git a/go.mod b/go.mod index bcd00ea..af61c39 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/openshift-hyperfleet/hyperfleet-operator -go 1.24.0 +go 1.26.0 require ( github.com/onsi/ginkgo/v2 v2.22.0 diff --git a/internal/controller/hyperfleetconfig_controller_test.go b/internal/controller/hyperfleetconfig_controller_test.go index 67e7aed..b8584a2 100644 --- a/internal/controller/hyperfleetconfig_controller_test.go +++ b/internal/controller/hyperfleetconfig_controller_test.go @@ -56,12 +56,12 @@ var _ = Describe("HyperFleetConfig Controller", func() { Bundle: hyperfleetv1alpha1.BundleCloudCAPI, API: hyperfleetv1alpha1.APISpec{ Database: hyperfleetv1alpha1.DatabaseSpec{ - SecretRef: hyperfleetv1alpha1.SecretReference{Name: "hyperfleet-db"}, + SecretRef: hyperfleetv1alpha1.SecretReference{Name: testDBSecretName}, }, Auth: hyperfleetv1alpha1.AuthSpec{ Enabled: ptr.To(true), - Issuer: "https://issuer.example.com", - Audience: "hyperfleet-api", + Issuer: testIssuerURL, + Audience: testAudience, }, }, }, diff --git a/internal/controller/hyperfleetconfig_types_test.go b/internal/controller/hyperfleetconfig_types_test.go index 9018015..b016b19 100644 --- a/internal/controller/hyperfleetconfig_types_test.go +++ b/internal/controller/hyperfleetconfig_types_test.go @@ -31,6 +31,15 @@ import ( hyperfleetv1alpha1 "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1" ) +// Repeated fixture values are declared once as constants: goconst flags any +// string literal that recurs three or more times in a package, and a single +// definition keeps the specs in sync. +const ( + testDBSecretName = "hyperfleet-db" + testIssuerURL = "https://issuer.example.com" + testAudience = "hyperfleet-api" +) + // These specs exercise the CRD's declarative validation and defaulting through a // real API server (envtest). They assert the schema contract for HYPERFLEET-1406; // no reconciler behavior is involved. @@ -47,12 +56,12 @@ func validHyperFleetConfig() *hyperfleetv1alpha1.HyperFleetConfig { Bundle: hyperfleetv1alpha1.BundleCloudCAPI, API: hyperfleetv1alpha1.APISpec{ Database: hyperfleetv1alpha1.DatabaseSpec{ - SecretRef: hyperfleetv1alpha1.SecretReference{Name: "hyperfleet-db"}, + SecretRef: hyperfleetv1alpha1.SecretReference{Name: testDBSecretName}, }, Auth: hyperfleetv1alpha1.AuthSpec{ Enabled: ptr.To(true), - Issuer: "https://issuer.example.com", - Audience: "hyperfleet-api", + Issuer: testIssuerURL, + Audience: testAudience, }, }, }, @@ -213,7 +222,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { // A long-but-valid https URL isolates MaxLength: isURL, scheme and host // all pass, so only the length bound can reject it. obj := validHyperFleetConfig() - obj.Spec.API.Auth.Issuer = "https://issuer.example.com/" + strings.Repeat("a", 2048) + obj.Spec.API.Auth.Issuer = testIssuerURL + "/" + strings.Repeat("a", 2048) err := k8sClient.Create(ctx, obj) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("may not be more than 2048 bytes")) @@ -228,7 +237,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{})["auth"] = map[string]interface{}{ "enabled": true, - "issuer": "https://issuer.example.com", + "issuer": testIssuerURL, "audience": "", } err := k8sClient.Create(ctx, u) @@ -392,12 +401,12 @@ func minimalUnstructured() *unstructured.Unstructured { "bundle": string(hyperfleetv1alpha1.BundleCloudCAPI), "api": map[string]interface{}{ "database": map[string]interface{}{ - "secretRef": map[string]interface{}{"name": "hyperfleet-db"}, + "secretRef": map[string]interface{}{"name": testDBSecretName}, }, "auth": map[string]interface{}{ // enabled omitted -> defaults to true - "issuer": "https://issuer.example.com", - "audience": "hyperfleet-api", + "issuer": testIssuerURL, + "audience": testAudience, }, // profile omitted -> defaults to "small" }, From 119b648d301b2a1bda984482904188acc8870bbb Mon Sep 17 00:00:00 2001 From: tithakka Date: Tue, 18 Aug 2026 14:05:58 -0500 Subject: [PATCH 3/4] HYPERFLEET-1406 - feat: fix regex suggestions and test fixes --- .github/workflows/lint.yml | 2 +- Makefile | 24 ++++++---- api/v1alpha1/hyperfleetconfig_types.go | 22 +++++++-- ...perfleet.redhat.com_hyperfleetconfigs.yaml | 16 ++++--- config/default/kustomization.yaml | 15 ++++-- .../hyperfleetconfig_controller_test.go | 42 ++-------------- .../controller/hyperfleetconfig_types_test.go | 48 ++++++++++++------- internal/controller/suite_test.go | 28 +++++++++++ 8 files changed, 117 insertions(+), 80 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f4b9ce6..86e3845 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -20,4 +20,4 @@ jobs: - name: Run linter uses: golangci/golangci-lint-action@v8 with: - version: v2.12.2 + version: v2.1.0 diff --git a/Makefile b/Makefile index 2e3b505..da3247e 100644 --- a/Makefile +++ b/Makefile @@ -140,20 +140,28 @@ export XDG_DATA_HOME ?= $(ROOT_DIR)_output/.local/share .PHONY: test test: manifests generate fmt vet ## Run tests. - # Honor a KUBEBUILDER_ASSETS supplied by the environment (e.g. an OpenShift CI - # pod that pre-bakes the envtest control-plane binaries); fall back to - # setup-envtest only when it is unset. Resolve that fallback in the recipe - # shell (via $$(...)) rather than make's $(shell ...): $(shell ...) runs at - # parse time on every make invocation (even unrelated targets) and cannot see - # a KUBEBUILDER_ASSETS exported into the recipe environment. + # Resolve the envtest control-plane binaries. We deliberately do NOT honor the + # ambient KUBEBUILDER_ASSETS here: it is the standard variable envtest tooling + # exports, so a stale value in a developer's shell (or leftover from another + # repo/branch) would silently bypass the ENVTEST_K8S_VERSION pin and run the + # suite against the wrong control-plane version. Instead, an explicit + # CI_KUBEBUILDER_ASSETS opt-in lets an environment that pre-bakes the binaries + # supply its own path without that footgun; otherwise we fall back to + # setup-envtest at the pinned version. (Neither our GitHub Actions nor the + # OpenShift CI unit step pre-bakes today, so in practice the fallback is what + # executes everywhere.) + # + # Resolve that fallback in the recipe shell (via $$(...)) rather than make's + # $(shell ...): $(shell ...) runs at parse time on every make invocation (even + # unrelated targets) and cannot see a value exported into the recipe environment. # # Assign on its own line so that, under .SHELLFLAGS -e, a setup-envtest failure # aborts here with its real error instead of being masked by go test's exit # status (a bare `VAR="$$(cmd)" go test` reports go test's status, not cmd's). # Then require a non-empty path so a silent empty resolve can't launch the # suite with no control-plane binaries ("etcd: executable file not found"). - assets="$${KUBEBUILDER_ASSETS:-$$($(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)}"; \ - [ -n "$$assets" ] || { echo "error: KUBEBUILDER_ASSETS is empty; setup-envtest did not resolve envtest binaries" >&2; exit 1; }; \ + assets="$${CI_KUBEBUILDER_ASSETS:-$$($(SETUP_ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)}"; \ + [ -n "$$assets" ] || { echo "error: envtest assets path is empty; setup-envtest did not resolve envtest binaries (set CI_KUBEBUILDER_ASSETS to override)" >&2; exit 1; }; \ KUBEBUILDER_ASSETS="$$assets" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out .PHONY: setup-test-e2e diff --git a/api/v1alpha1/hyperfleetconfig_types.go b/api/v1alpha1/hyperfleetconfig_types.go index b863f70..4e03483 100644 --- a/api/v1alpha1/hyperfleetconfig_types.go +++ b/api/v1alpha1/hyperfleetconfig_types.go @@ -102,17 +102,31 @@ const ( // the operator's own namespace: because HyperFleetConfig is cluster-scoped, no // namespace field is exposed (name-only + operator-namespace convention, decided // in the HYPERFLEET-1406 API review). +// +// TODO(HYPERFLEET-1512): the operator-namespace constraint is convention-only +// today — the schema cannot enforce it (CEL sees no cross-object/namespace +// state; ADR-0019 rules out webhooks). The reconciler must enforce it (resolve +// the Secret in the operator's own namespace and surface a Degraded condition +// when it is missing) once it lands. type SecretReference struct { // name is the name of the Secret in the operator's namespace. It must be a - // valid DNS-1123 subdomain (the same constraint the API server places on - // Secret names) so an unresolvable reference is rejected at admission rather - // than failing opaquely when the reference is later resolved. + // valid DNS-1123 subdomain, matching what k8s.io/apimachinery/pkg/util/validation + // enforces for Secret names (IsDNS1123Subdomain, max length 253), so an + // unresolvable reference is rejected at admission rather than failing opaquely + // when the reference is later resolved. // // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=253 // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` Name string `json:"name"` + + // Maintainer note (free-standing so it stays out of the generated CRD + // description): the MaxLength and Pattern markers on name are literal copies of + // apimachinery's DNS1123SubdomainMaxLength (253) and dns1123SubdomainFmt. They + // can't reference those symbols — controller-gen markers accept only literal + // values, and the format constant is unexported — so keep them in sync by hand + // if k8s.io/apimachinery/pkg/util/validation ever changes. } // DatabaseSpec configures the HyperFleet API's connection to its external @@ -202,7 +216,7 @@ type APISpec struct { } // HyperFleetConfigSpec defines the desired state of HyperFleetConfig. It captures -// partner intent only; internal machinery (broker, adapters, Sentinel) is never +// partner intent only; internal machinery (broker, adapters, sentinel) is never // expressed here. type HyperFleetConfigSpec struct { // bundle selects one of the operator-internal bundle definitions. It is diff --git a/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml b/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml index 2b0930d..9b5a943 100644 --- a/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml +++ b/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml @@ -56,7 +56,7 @@ spec: spec: description: |- HyperFleetConfigSpec defines the desired state of HyperFleetConfig. It captures - partner intent only; internal machinery (broker, adapters, Sentinel) is never + partner intent only; internal machinery (broker, adapters, sentinel) is never expressed here. properties: api: @@ -114,9 +114,10 @@ spec: name: description: |- name is the name of the Secret in the operator's namespace. It must be a - valid DNS-1123 subdomain (the same constraint the API server places on - Secret names) so an unresolvable reference is rejected at admission rather - than failing opaquely when the reference is later resolved. + valid DNS-1123 subdomain, matching what k8s.io/apimachinery/pkg/util/validation + enforces for Secret names (IsDNS1123Subdomain, max length 253), so an + unresolvable reference is rejected at admission rather than failing opaquely + when the reference is later resolved. maxLength: 253 minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ @@ -149,9 +150,10 @@ spec: name: description: |- name is the name of the Secret in the operator's namespace. It must be a - valid DNS-1123 subdomain (the same constraint the API server places on - Secret names) so an unresolvable reference is rejected at admission rather - than failing opaquely when the reference is later resolved. + valid DNS-1123 subdomain, matching what k8s.io/apimachinery/pkg/util/validation + enforces for Secret names (IsDNS1123Subdomain, max length 253), so an + unresolvable reference is rejected at admission rather than failing opaquely + when the reference is later resolved. maxLength: 253 minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 5da69aa..8687cac 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -8,11 +8,16 @@ namespace: hyperfleet-operator-system # field above. namePrefix: hyperfleet-operator- -# Labels to add to all resources and selectors. -#labels: -#- includeSelectors: true -# pairs: -# someName: someValue +# Labels to add to all resources. includeSelectors is false so these land only in +# metadata.labels (for `kubectl get -l app.kubernetes.io/name=...` discovery) and are +# NOT injected into Deployment/Service selectors, which are immutable on in-place +# updates. This also backfills the labels onto controller-gen-managed files +# (rbac/role.yaml, metrics roles) and the CRD, which cannot carry them inline. +labels: +- includeSelectors: false + pairs: + app.kubernetes.io/name: hyperfleet-operator + app.kubernetes.io/managed-by: kustomize resources: - ../crd diff --git a/internal/controller/hyperfleetconfig_controller_test.go b/internal/controller/hyperfleetconfig_controller_test.go index b8584a2..9b2e579 100644 --- a/internal/controller/hyperfleetconfig_controller_test.go +++ b/internal/controller/hyperfleetconfig_controller_test.go @@ -23,11 +23,8 @@ import ( . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/reconcile" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - hyperfleetv1alpha1 "github.com/openshift-hyperfleet/hyperfleet-operator/api/v1alpha1" ) @@ -48,24 +45,7 @@ var _ = Describe("HyperFleetConfig Controller", func() { By("creating the custom resource for the Kind HyperFleetConfig") err := k8sClient.Get(ctx, typeNamespacedName, hyperfleetconfig) if errors.IsNotFound(err) { - resource := &hyperfleetv1alpha1.HyperFleetConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: resourceName, - }, - Spec: hyperfleetv1alpha1.HyperFleetConfigSpec{ - Bundle: hyperfleetv1alpha1.BundleCloudCAPI, - API: hyperfleetv1alpha1.APISpec{ - Database: hyperfleetv1alpha1.DatabaseSpec{ - SecretRef: hyperfleetv1alpha1.SecretReference{Name: testDBSecretName}, - }, - Auth: hyperfleetv1alpha1.AuthSpec{ - Enabled: ptr.To(true), - Issuer: testIssuerURL, - Audience: testAudience, - }, - }, - }, - } + resource := validHyperFleetConfig() Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } else { // A Get error other than NotFound means the fixture state is unknown; @@ -73,24 +53,12 @@ var _ = Describe("HyperFleetConfig Controller", func() { // reconcile spec pass without its required resource. Expect(err).NotTo(HaveOccurred()) } + // The singleton now exists (created just above or already present), so + // schedule its teardown. DeferCleanup runs after the spec and replaces a + // blanket AfterEach; deleteSingletonAndWait lives in suite_test.go. + DeferCleanup(deleteSingletonAndWait, ctx) }) - AfterEach(func() { - By("Cleanup the specific resource instance HyperFleetConfig") - resource := &hyperfleetv1alpha1.HyperFleetConfig{ - ObjectMeta: metav1.ObjectMeta{Name: resourceName}, - } - // NotFound means cleanup already happened; any other delete error - // must fail the test loudly instead of being masked as an Eventually - // timeout below. - if err := k8sClient.Delete(ctx, resource); err != nil && !errors.IsNotFound(err) { - Expect(err).NotTo(HaveOccurred()) - } - Eventually(func() bool { - err := k8sClient.Get(ctx, typeNamespacedName, &hyperfleetv1alpha1.HyperFleetConfig{}) - return errors.IsNotFound(err) - }).Should(BeTrue()) - }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource") controllerReconciler := &HyperFleetConfigReconciler{ diff --git a/internal/controller/hyperfleetconfig_types_test.go b/internal/controller/hyperfleetconfig_types_test.go index b016b19..9df6f39 100644 --- a/internal/controller/hyperfleetconfig_types_test.go +++ b/internal/controller/hyperfleetconfig_types_test.go @@ -72,28 +72,17 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { ctx := context.Background() // Every valid object is forced to the same name ("cluster"), so specs collide - // unless the singleton is torn down between them. - AfterEach(func() { - obj := &hyperfleetv1alpha1.HyperFleetConfig{ - ObjectMeta: metav1.ObjectMeta{Name: hyperfleetv1alpha1.SingletonName}, - } - // A negative spec may have prevented creation, so NotFound is a valid - // "already clean" outcome; any other delete error must fail the test - // loudly rather than be masked as an Eventually timeout below. - if err := k8sClient.Delete(ctx, obj); err != nil && !errors.IsNotFound(err) { - Expect(err).NotTo(HaveOccurred()) - } - Eventually(func() bool { - err := k8sClient.Get(ctx, - types.NamespacedName{Name: hyperfleetv1alpha1.SingletonName}, - &hyperfleetv1alpha1.HyperFleetConfig{}) - return errors.IsNotFound(err) - }).Should(BeTrue()) - }) + // unless the singleton is torn down between them. Rather than a blanket + // AfterEach, each spec that actually creates the singleton schedules its own + // teardown via DeferCleanup(deleteSingletonAndWait, ctx) right after a + // successful Create: the ~16 negative specs never create, so they register no + // cleanup and pay nothing. deleteSingletonAndWait lives in suite_test.go and + // is shared with the reconciler specs. Context("singleton enforcement", func() { It("accepts the resource when named 'cluster'", func() { Expect(k8sClient.Create(ctx, validHyperFleetConfig())).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) }) It("rejects any name other than 'cluster'", func() { @@ -108,6 +97,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { // The name-pin CEL rule forces every instance to the name "cluster"; // cluster-scoped name uniqueness then makes the second create collide. Expect(k8sClient.Create(ctx, validHyperFleetConfig())).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) err := k8sClient.Create(ctx, validHyperFleetConfig()) Expect(err).To(HaveOccurred()) Expect(errors.IsAlreadyExists(err)).To(BeTrue()) @@ -124,6 +114,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { obj := validHyperFleetConfig() obj.Spec.Bundle = bundle Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) }) } @@ -132,6 +123,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { obj := validHyperFleetConfig() obj.Spec.API.Profile = profile Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) }) } @@ -156,6 +148,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { It("rejects changing the bundle after creation", func() { obj := validHyperFleetConfig() Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) obj.Spec.Bundle = hyperfleetv1alpha1.BundleOnPremAgent err := k8sClient.Update(ctx, obj) @@ -168,6 +161,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { // updatable. This proves self==oldSelf was not applied too broadly. obj := validHyperFleetConfig() Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) obj.Spec.API.Profile = hyperfleetv1alpha1.SizingProfileLarge Expect(k8sClient.Update(ctx, obj)).To(Succeed()) @@ -257,6 +251,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { obj := validHyperFleetConfig() obj.Spec.API.Auth = hyperfleetv1alpha1.AuthSpec{Enabled: ptr.To(false)} Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) }) }) @@ -298,6 +293,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { SecretRef: hyperfleetv1alpha1.SecretReference{Name: "hyperfleet-tls"}, } Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) }) It("rejects a TLS block with an empty secret name", func() { @@ -310,6 +306,20 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { Expect(err.Error()).To(ContainSubstring("tls.secretRef.name")) }) + It("rejects a TLS secret name that is not a DNS-1123 subdomain", func() { + // SecretReference.Name's Pattern marker is shared with the database path, + // but the generated CRD inlines an independent copy at each field; assert + // it is enforced at the tls.secretRef.name path too, mirroring the + // database case in the "required fields" context. + obj := validHyperFleetConfig() + obj.Spec.API.TLS = &hyperfleetv1alpha1.TLSSpec{ + SecretRef: hyperfleetv1alpha1.SecretReference{Name: "Invalid_Name"}, + } + err := k8sClient.Create(ctx, obj) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("tls.secretRef.name")) + }) + It("rejects a TLS secret name longer than the 253-character maximum", func() { obj := validHyperFleetConfig() obj.Spec.API.TLS = &hyperfleetv1alpha1.TLSSpec{ @@ -366,6 +376,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { It("populates profile and auth.enabled defaults on a minimal object", func() { u := minimalUnstructured() Expect(k8sClient.Create(ctx, u)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) got := &hyperfleetv1alpha1.HyperFleetConfig{} Expect(k8sClient.Get(ctx, @@ -381,6 +392,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { obj := validHyperFleetConfig() obj.Spec.API.Auth.Enabled = nil Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + DeferCleanup(deleteSingletonAndWait, ctx) got := &hyperfleetv1alpha1.HyperFleetConfig{} Expect(k8sClient.Get(ctx, diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index dcac0ce..88edf44 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -25,6 +25,9 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -92,6 +95,31 @@ var _ = AfterSuite(func() { Expect(err).NotTo(HaveOccurred()) }) +// deleteSingletonAndWait removes the "cluster" singleton and blocks until the +// API server confirms it is gone, so each spec starts from a clean slate. Specs +// schedule it via DeferCleanup immediately after a successful Create (both the +// CRD-validation and the reconciler specs force the same singleton name, so they +// would otherwise collide); the negative specs never create and so never +// schedule it. The cleanup is still idempotent by design: NotFound on delete is +// a valid "already clean" outcome, while any other delete error fails the +// calling spec loudly rather than being masked as an Eventually timeout below. +// GinkgoHelper attributes any failure here to the spec that scheduled it. +func deleteSingletonAndWait(ctx context.Context) { + GinkgoHelper() + obj := &hyperfleetv1alpha1.HyperFleetConfig{ + ObjectMeta: metav1.ObjectMeta{Name: hyperfleetv1alpha1.SingletonName}, + } + if err := k8sClient.Delete(ctx, obj); err != nil && !errors.IsNotFound(err) { + Expect(err).NotTo(HaveOccurred()) + } + Eventually(func() bool { + err := k8sClient.Get(ctx, + types.NamespacedName{Name: hyperfleetv1alpha1.SingletonName}, + &hyperfleetv1alpha1.HyperFleetConfig{}) + return errors.IsNotFound(err) + }).Should(BeTrue()) +} + // getFirstFoundEnvTestBinaryDir locates the first binary in the specified path. // ENVTEST-based tests depend on specific binaries, usually located in paths set by // controller-runtime. When running tests directly (e.g., via an IDE) without using From d495406190ed5730e58d547df8a080070c0f2827 Mon Sep 17 00:00:00 2001 From: tithakka Date: Wed, 19 Aug 2026 08:38:45 -0500 Subject: [PATCH 4/4] HYPERFLEET-1406 - feat: fix linter --- .../controller/hyperfleetconfig_types_test.go | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/controller/hyperfleetconfig_types_test.go b/internal/controller/hyperfleetconfig_types_test.go index 9df6f39..672e6e9 100644 --- a/internal/controller/hyperfleetconfig_types_test.go +++ b/internal/controller/hyperfleetconfig_types_test.go @@ -205,7 +205,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { // https XValidation alone) must reject it. Only unstructured can carry an // explicit empty scalar; the typed client omits it via omitempty. u := minimalUnstructured() - auth := u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{})["auth"].(map[string]interface{}) + auth := u.Object["spec"].(map[string]any)["api"].(map[string]any)["auth"].(map[string]any) auth["issuer"] = "" err := k8sClient.Create(ctx, u) Expect(err).To(HaveOccurred()) @@ -228,8 +228,8 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { // unstructured object can carry an explicit empty scalar (the typed client // omits it via omitempty). u := minimalUnstructured() - u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{})["auth"] = - map[string]interface{}{ + u.Object["spec"].(map[string]any)["api"].(map[string]any)["auth"] = + map[string]any{ "enabled": true, "issuer": testIssuerURL, "audience": "", @@ -337,7 +337,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { // typed client always sends. It("rejects a config with no bundle", func() { u := minimalUnstructured() - delete(u.Object["spec"].(map[string]interface{}), "bundle") + delete(u.Object["spec"].(map[string]any), "bundle") err := k8sClient.Create(ctx, u) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("spec.bundle")) @@ -345,7 +345,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { It("rejects a config with no api", func() { u := minimalUnstructured() - delete(u.Object["spec"].(map[string]interface{}), "api") + delete(u.Object["spec"].(map[string]any), "api") err := k8sClient.Create(ctx, u) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("spec.api")) @@ -353,7 +353,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { It("rejects an api with no database", func() { u := minimalUnstructured() - api := u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{}) + api := u.Object["spec"].(map[string]any)["api"].(map[string]any) delete(api, "database") err := k8sClient.Create(ctx, u) Expect(err).To(HaveOccurred()) @@ -362,7 +362,7 @@ var _ = Describe("HyperFleetConfig CRD validation", func() { It("rejects an api with no auth", func() { u := minimalUnstructured() - api := u.Object["spec"].(map[string]interface{})["api"].(map[string]interface{}) + api := u.Object["spec"].(map[string]any)["api"].(map[string]any) delete(api, "auth") err := k8sClient.Create(ctx, u) Expect(err).To(HaveOccurred()) @@ -409,13 +409,13 @@ func minimalUnstructured() *unstructured.Unstructured { u := &unstructured.Unstructured{} u.SetGroupVersionKind(hyperfleetv1alpha1.GroupVersion.WithKind("HyperFleetConfig")) u.SetName(hyperfleetv1alpha1.SingletonName) - u.Object["spec"] = map[string]interface{}{ + u.Object["spec"] = map[string]any{ "bundle": string(hyperfleetv1alpha1.BundleCloudCAPI), - "api": map[string]interface{}{ - "database": map[string]interface{}{ - "secretRef": map[string]interface{}{"name": testDBSecretName}, + "api": map[string]any{ + "database": map[string]any{ + "secretRef": map[string]any{"name": testDBSecretName}, }, - "auth": map[string]interface{}{ + "auth": map[string]any{ // enabled omitted -> defaults to true "issuer": testIssuerURL, "audience": testAudience,