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/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..4e03483 --- /dev/null +++ b/api/v1alpha1/hyperfleetconfig_types.go @@ -0,0 +1,286 @@ +/* +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). +// +// 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, 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 +// 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 index 81d883a..9b5a943 100644 --- a/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml +++ b/config/crd/bases/hyperfleet.redhat.com_hyperfleetconfigs.yaml @@ -11,13 +11,30 @@ spec: kind: HyperFleetConfig listKind: HyperFleetConfigList plural: hyperfleetconfigs + shortNames: + - hfc singular: hyperfleetconfig - scope: Namespaced + scope: Cluster versions: - - name: v1alpha + - 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. + 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: |- @@ -37,17 +54,218 @@ spec: metadata: type: object spec: - description: HyperFleetConfigSpec defines the desired state of HyperFleetConfig. + description: |- + HyperFleetConfigSpec defines the desired state of HyperFleetConfig. It captures + partner intent only; internal machinery (broker, adapters, sentinel) is never + expressed here. properties: - foo: - description: Foo is an example field of HyperFleetConfig. Edit hyperfleetconfig_types.go - to remove/update + 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, 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])?)*$ + 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, 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])?)*$ + 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. + 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: 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/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 d4b495d..af61c39 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..9b2e579 100644 --- a/internal/controller/hyperfleetconfig_controller_test.go +++ b/internal/controller/hyperfleetconfig_controller_test.go @@ -25,47 +25,40 @@ import ( "k8s.io/apimachinery/pkg/types" "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{ - ObjectMeta: metav1.ObjectMeta{ - Name: resourceName, - Namespace: "default", - }, - // TODO(user): Specify other spec details if needed. - } + if errors.IsNotFound(err) { + resource := validHyperFleetConfig() 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()) } + // 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() { - // 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()) - }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource") controllerReconciler := &HyperFleetConfigReconciler{ @@ -77,8 +70,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..672e6e9 --- /dev/null +++ b/internal/controller/hyperfleetconfig_types_test.go @@ -0,0 +1,427 @@ +/* +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" +) + +// 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. + +// 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: testDBSecretName}, + }, + Auth: hyperfleetv1alpha1.AuthSpec{ + Enabled: ptr.To(true), + Issuer: testIssuerURL, + Audience: testAudience, + }, + }, + }, + } +} + +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. 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() { + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + }) + } + + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + }) + } + + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + + 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]any)["api"].(map[string]any)["auth"].(map[string]any) + 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 = 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")) + }) + + 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]any)["api"].(map[string]any)["auth"] = + map[string]any{ + "enabled": true, + "issuer": testIssuerURL, + "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()) + DeferCleanup(deleteSingletonAndWait, ctx) + }) + }) + + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + }) + + 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 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{ + 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]any), "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]any), "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]any)["api"].(map[string]any) + 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]any)["api"].(map[string]any) + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + + 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()) + DeferCleanup(deleteSingletonAndWait, ctx) + + 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]any{ + "bundle": string(hyperfleetv1alpha1.BundleCloudCAPI), + "api": map[string]any{ + "database": map[string]any{ + "secretRef": map[string]any{"name": testDBSecretName}, + }, + "auth": map[string]any{ + // enabled omitted -> defaults to true + "issuer": testIssuerURL, + "audience": testAudience, + }, + // profile omitted -> defaults to "small" + }, + } + return u +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 308cc40..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" @@ -32,7 +35,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 +62,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 @@ -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