diff --git a/api/hypershift/v1beta1/hosted_controlplane.go b/api/hypershift/v1beta1/hosted_controlplane.go index 3024c9b8166..487fc3a8ab8 100644 --- a/api/hypershift/v1beta1/hosted_controlplane.go +++ b/api/hypershift/v1beta1/hosted_controlplane.go @@ -205,6 +205,7 @@ type HostedControlPlaneSpec struct { // This field is optional and once set cannot be changed. // +optional // +openshift:enable:FeatureGate=DisableClusterCapabilities + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="Capabilities is immutable. Changes might result in unpredictable and disruptive behavior." Capabilities *Capabilities `json:"capabilities,omitempty"` } diff --git a/api/hypershift/v1beta1/hostedcluster_types.go b/api/hypershift/v1beta1/hostedcluster_types.go index 8ae8ca9343a..a97e558719f 100644 --- a/api/hypershift/v1beta1/hostedcluster_types.go +++ b/api/hypershift/v1beta1/hostedcluster_types.go @@ -363,7 +363,6 @@ const ImageRegistryCapability OptionalCapability = OptionalCapability(configv1.C // capabilities allows disabling optional components at install time. // Once set, it cannot be changed. -// +kubebuilder:validation:XValidation:rule="!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)", message="disabledCapabilities is required once set" type Capabilities struct { // disabledCapabilities when specified, sets the cluster version baselineCapabilitySet to None // and sets all additionalEnabledCapabilities BUT the ones supplied in disabledCapabilities. @@ -375,7 +374,6 @@ type Capabilities struct { // Once set, this field cannot be changed. // // +listType=atomic - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="disabledCapabilities is immutable" // +optional DisabledCapabilities []OptionalCapability `json:"disabledCapabilities,omitempty"` } @@ -388,7 +386,6 @@ type Capabilities struct { // +kubebuilder:validation:XValidation:rule=`self.platform.type == "Azure" ? self.services.exists(s, s.service == "Konnectivity" && s.servicePublishingStrategy.type == "Route" && s.servicePublishingStrategy.route.hostname != "") : true`,message="Azure platform requires Konnectivity Route service with a hostname to be defined" // +kubebuilder:validation:XValidation:rule=`self.platform.type == "Azure" ? self.services.exists(s, s.service == "Ignition" && s.servicePublishingStrategy.type == "Route" && s.servicePublishingStrategy.route.hostname != "") : true`,message="Azure platform requires Ignition Route service with a hostname to be defined" // +kubebuilder:validation:XValidation:rule=`has(self.issuerURL) || !has(self.serviceAccountSigningKey)`,message="If serviceAccountSigningKey is set, issuerURL must be set" - type HostedClusterSpec struct { // release specifies the desired OCP release payload for all the hosted cluster components. // This includes those components running management side like the Kube API Server and the CVO but also the operands which land in the hosted cluster data plane like the ingress controller, ovn agents, etc. @@ -665,6 +662,7 @@ type HostedClusterSpec struct { // This field is optional and once set cannot be changed. // +optional // +openshift:enable:FeatureGate=DisableClusterCapabilities + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="Capabilities is immutable. Changes might result in unpredictable and disruptive behavior." Capabilities *Capabilities `json:"capabilities,omitempty"` } diff --git a/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/DisableClusterCapabilities.yaml b/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/DisableClusterCapabilities.yaml index 6b0e35505bf..5bf2240ef90 100644 --- a/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/DisableClusterCapabilities.yaml +++ b/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedclusters.hypershift.openshift.io/DisableClusterCapabilities.yaml @@ -170,13 +170,11 @@ spec: type: string type: array x-kubernetes-list-type: atomic - x-kubernetes-validations: - - message: disabledCapabilities is immutable - rule: self == oldSelf type: object x-kubernetes-validations: - - message: disabledCapabilities is required once set - rule: '!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)' + - message: Capabilities is immutable. Changes might result in unpredictable + and disruptive behavior. + rule: self == oldSelf channel: description: |- channel is an identifier for explicitly requesting that a non-default set of updates be applied to this cluster. diff --git a/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/DisableClusterCapabilities.yaml b/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/DisableClusterCapabilities.yaml index 5580a82685d..ec5f756db77 100644 --- a/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/DisableClusterCapabilities.yaml +++ b/api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/DisableClusterCapabilities.yaml @@ -138,13 +138,11 @@ spec: type: string type: array x-kubernetes-list-type: atomic - x-kubernetes-validations: - - message: disabledCapabilities is immutable - rule: self == oldSelf type: object x-kubernetes-validations: - - message: disabledCapabilities is required once set - rule: '!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)' + - message: Capabilities is immutable. Changes might result in unpredictable + and disruptive behavior. + rule: self == oldSelf channel: description: |- channel is an identifier for explicitly requesting that a non-default diff --git a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-CustomNoUpgrade.crd.yaml b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-CustomNoUpgrade.crd.yaml index b43cfa3630f..c6835ffa578 100644 --- a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-CustomNoUpgrade.crd.yaml +++ b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-CustomNoUpgrade.crd.yaml @@ -218,13 +218,11 @@ spec: type: string type: array x-kubernetes-list-type: atomic - x-kubernetes-validations: - - message: disabledCapabilities is immutable - rule: self == oldSelf type: object x-kubernetes-validations: - - message: disabledCapabilities is required once set - rule: '!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)' + - message: Capabilities is immutable. Changes might result in unpredictable + and disruptive behavior. + rule: self == oldSelf channel: description: |- channel is an identifier for explicitly requesting that a non-default set of updates be applied to this cluster. diff --git a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-TechPreviewNoUpgrade.crd.yaml b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-TechPreviewNoUpgrade.crd.yaml index 57c0b182f0d..e5f2e81a6e2 100644 --- a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-TechPreviewNoUpgrade.crd.yaml +++ b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedclusters-TechPreviewNoUpgrade.crd.yaml @@ -218,13 +218,11 @@ spec: type: string type: array x-kubernetes-list-type: atomic - x-kubernetes-validations: - - message: disabledCapabilities is immutable - rule: self == oldSelf type: object x-kubernetes-validations: - - message: disabledCapabilities is required once set - rule: '!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)' + - message: Capabilities is immutable. Changes might result in unpredictable + and disruptive behavior. + rule: self == oldSelf channel: description: |- channel is an identifier for explicitly requesting that a non-default set of updates be applied to this cluster. diff --git a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-CustomNoUpgrade.crd.yaml b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-CustomNoUpgrade.crd.yaml index ffc89caf6ca..a0a95cf9037 100644 --- a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-CustomNoUpgrade.crd.yaml +++ b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-CustomNoUpgrade.crd.yaml @@ -186,13 +186,11 @@ spec: type: string type: array x-kubernetes-list-type: atomic - x-kubernetes-validations: - - message: disabledCapabilities is immutable - rule: self == oldSelf type: object x-kubernetes-validations: - - message: disabledCapabilities is required once set - rule: '!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)' + - message: Capabilities is immutable. Changes might result in unpredictable + and disruptive behavior. + rule: self == oldSelf channel: description: |- channel is an identifier for explicitly requesting that a non-default diff --git a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-TechPreviewNoUpgrade.crd.yaml b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-TechPreviewNoUpgrade.crd.yaml index 68df88f23fd..f6b951842e4 100644 --- a/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-TechPreviewNoUpgrade.crd.yaml +++ b/cmd/install/assets/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-TechPreviewNoUpgrade.crd.yaml @@ -186,13 +186,11 @@ spec: type: string type: array x-kubernetes-list-type: atomic - x-kubernetes-validations: - - message: disabledCapabilities is immutable - rule: self == oldSelf type: object x-kubernetes-validations: - - message: disabledCapabilities is required once set - rule: '!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)' + - message: Capabilities is immutable. Changes might result in unpredictable + and disruptive behavior. + rule: self == oldSelf channel: description: |- channel is an identifier for explicitly requesting that a non-default diff --git a/control-plane-operator/controllers/hostedcontrolplane/cvo/reconcile.go b/control-plane-operator/controllers/hostedcontrolplane/cvo/reconcile.go index 630f8cf1073..d890b3c6fb7 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/cvo/reconcile.go +++ b/control-plane-operator/controllers/hostedcontrolplane/cvo/reconcile.go @@ -1,6 +1,7 @@ package cvo import ( + "encoding/json" "fmt" "path" "strings" @@ -10,6 +11,7 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/controlplaneoperator" "github.com/openshift/hypershift/support/api" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/certs" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/metrics" @@ -118,7 +120,7 @@ func cvoLabels() map[string]string { var port int32 = 8443 -func ReconcileDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef, deploymentConfig config.DeploymentConfig, controlPlaneReleaseImage, dataPlaneReleaseImage, cliImage, availabilityProberImage, clusterID string, updateService configv1.URL, platformType hyperv1.PlatformType, oauthEnabled, enableCVOManagementClusterMetricsAccess bool, featureSet configv1.FeatureSet) error { +func ReconcileDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef, deploymentConfig config.DeploymentConfig, controlPlaneReleaseImage, dataPlaneReleaseImage, cliImage, availabilityProberImage, clusterID string, updateService configv1.URL, platformType hyperv1.PlatformType, oauthEnabled, enableCVOManagementClusterMetricsAccess bool, featureSet configv1.FeatureSet, caps *hyperv1.Capabilities) error { ownerRef.ApplyTo(deployment) // preserve existing resource requirements for main CVO container @@ -132,6 +134,35 @@ func ReconcileDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef MatchLabels: cvoLabels(), } } + + // the ClusterVersion resource is created by the CVO bootstrap container. + // we marshal it to json as a means to validate its formatting, which protects + // us against easily preventable mistakes, such as typos. + cv := &configv1.ClusterVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterVersion", + APIVersion: "config.openshift.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(clusterID), + }, + } + + if !capabilities.IsImageRegistryCapabilityEnabled(caps) { + cv.Spec.Capabilities = &configv1.ClusterVersionCapabilitiesSpec{ + BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetNone, + AdditionalEnabledCapabilities: capabilities.CalculateEnabledCapabilities(caps), + } + } + + clusterVersionJSON, err := json.Marshal(cv) + if err != nil { + return err + } + deployment.Spec = appsv1.DeploymentSpec{ Selector: selector, Template: corev1.PodTemplateSpec{ @@ -142,7 +173,7 @@ func ReconcileDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef AutomountServiceAccountToken: ptr.To(false), InitContainers: []corev1.Container{ util.BuildContainer(cvoContainerPrepPayload(), buildCVOContainerPrepPayload(dataPlaneReleaseImage, platformType, oauthEnabled, featureSet)), - util.BuildContainer(cvoContainerBootstrap(), buildCVOContainerBootstrap(cliImage, clusterID)), + util.BuildContainer(cvoContainerBootstrap(), buildCVOContainerBootstrap(cliImage, clusterVersionJSON)), }, Containers: []corev1.Container{ util.BuildContainer(cvoContainerMain(), buildCVOContainerMain(controlPlaneReleaseImage, dataPlaneReleaseImage, deployment.Namespace, updateService, enableCVOManagementClusterMetricsAccess)), @@ -203,13 +234,13 @@ func buildCVOContainerPrepPayload(image string, platformType hyperv1.PlatformTyp } } -func buildCVOContainerBootstrap(image, clusterID string) func(*corev1.Container) { +func buildCVOContainerBootstrap(image string, clusterVersionJSON []byte) func(*corev1.Container) { return func(c *corev1.Container) { c.Image = image c.Command = []string{"/bin/bash"} c.Args = []string{ "-c", - cvoBootrapScript(clusterID), + cvoBootstrapScript(clusterVersionJSON), } c.Resources.Requests = corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("10m"), @@ -338,31 +369,28 @@ func preparePayloadScript(platformType hyperv1.PlatformType, oauthEnabled bool, return strings.Join(stmts, "\n") } -func cvoBootrapScript(clusterID string) string { +func cvoBootstrapScript(clusterVersionJSON []byte) string { payloadDir := volumeMounts.Path(cvoContainerBootstrap().Name, cvoVolumePayload().Name) var scriptTemplate = `#!/bin/bash set -euo pipefail -cat > /tmp/clusterversion.yaml < /tmp/clusterversion.json < /dev/null || oc create ns openshift-config oc get ns openshift-config-managed &> /dev/null || oc create ns openshift-config-managed +oc apply -f ${MANIFEST_DIR}/0000_00_cluster-version-operator_01_clusterversions* +oc apply -f /tmp/clusterversion.json while true; do - echo "Applying CVO bootstrap manifests" - if oc apply -f %s/manifests; then + echo "Applying CVO bootstrap manifests..." + if oc apply -f ${MANIFEST_DIR}; then echo "Bootstrap manifests applied successfully." break fi sleep 1 done -oc get clusterversion/version &> /dev/null || oc create -f /tmp/clusterversion.yaml ` - return fmt.Sprintf(scriptTemplate, clusterID, payloadDir) + return fmt.Sprintf(scriptTemplate, payloadDir, string(clusterVersionJSON)) } func buildCVOContainerMain(controlPlaneReleaseImage, dataPlaneReleaseImage, namespace string, updateService configv1.URL, enableCVOManagementClusterMetricsAccess bool) func(c *corev1.Container) { diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 4afeee7f48d..2ae95df24f7 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1331,11 +1331,12 @@ func (r *HostedControlPlaneReconciler) reconcile(ctx context.Context, hostedCont if !r.IsCPOV2 { // Reconcile image registry operator - r.Log.Info("Reconciling Image Registry Operator") - if err := r.reconcileImageRegistryOperator(ctx, hostedControlPlane, releaseImageProvider, userReleaseImageProvider, createOrUpdate); err != nil { - return fmt.Errorf("failed to reconcile image registry operator: %w", err) + if capabilities.IsImageRegistryCapabilityEnabled(hostedControlPlane.Spec.Capabilities) { + r.Log.Info("Reconciling Image Registry Operator") + if err := r.reconcileImageRegistryOperator(ctx, hostedControlPlane, releaseImageProvider, userReleaseImageProvider, createOrUpdate); err != nil { + return fmt.Errorf("failed to reconcile image registry operator: %w", err) + } } - if IsStorageAndCSIManaged(hostedControlPlane) { // Reconcile cluster storage operator r.Log.Info("Reconciling cluster storage operator") @@ -2544,12 +2545,14 @@ func (r *HostedControlPlaneReconciler) reconcilePKI(ctx context.Context, hcp *hy return fmt.Errorf("failed to reconcile olm operator serving cert: %w", err) } - // Image Registry Operator Serving Cert - imageRegistryOperatorServingCert := manifests.ImageRegistryOperatorServingCert(hcp.Namespace) - if _, err := createOrUpdate(ctx, r, imageRegistryOperatorServingCert, func() error { - return pki.ReconcileRegistryOperatorServingCert(imageRegistryOperatorServingCert, rootCASecret, p.OwnerRef) - }); err != nil { - return fmt.Errorf("failed to reconcile image registry operator serving cert: %w", err) + if capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + // Image Registry Operator Serving Cert + imageRegistryOperatorServingCert := manifests.ImageRegistryOperatorServingCert(hcp.Namespace) + if _, err := createOrUpdate(ctx, r, imageRegistryOperatorServingCert, func() error { + return pki.ReconcileRegistryOperatorServingCert(imageRegistryOperatorServingCert, rootCASecret, p.OwnerRef) + }); err != nil { + return fmt.Errorf("failed to reconcile image registry operator serving cert: %w", err) + } } kcmServerSecret := manifests.KCMServerCertSecret(hcp.Namespace) @@ -3013,7 +3016,7 @@ func (r *HostedControlPlaneReconciler) reconcileKubeAPIServer(ctx context.Contex kubeAPIServerConfig := manifests.KASConfig(hcp.Namespace) if _, err := createOrUpdate(ctx, r, kubeAPIServerConfig, func() error { - return kas.ReconcileConfig(kubeAPIServerConfig, p.OwnerRef, p.ConfigParams()) + return kas.ReconcileConfig(kubeAPIServerConfig, p.OwnerRef, p.ConfigParams(), hcp.Spec.Capabilities) }); err != nil { return fmt.Errorf("failed to reconcile api server config: %w", err) } @@ -3349,7 +3352,7 @@ func (r *HostedControlPlaneReconciler) reconcileOpenShiftAPIServer(ctx context.C p := oapi.NewOpenShiftAPIServerParams(hcp, observedConfig, releaseImageProvider, r.SetDefaultSecurityContext) oapicfg := manifests.OpenShiftAPIServerConfig(hcp.Namespace) if _, err := createOrUpdate(ctx, r, oapicfg, func() error { - return oapi.ReconcileConfig(oapicfg, p.AuditWebhookRef, p.OwnerRef, p.EtcdURL, p.IngressDomain(), p.MinTLSVersion(), p.CipherSuites(), p.Image, p.Project) + return oapi.ReconcileConfig(oapicfg, p.AuditWebhookRef, p.OwnerRef, p.EtcdURL, p.IngressDomain(), p.MinTLSVersion(), p.CipherSuites(), p.Image, p.Project, hcp.Spec.Capabilities) }); err != nil { return fmt.Errorf("failed to reconcile openshift apiserver config: %w", err) } @@ -3537,7 +3540,10 @@ func (r *HostedControlPlaneReconciler) reconcileOpenShiftControllerManager(ctx c p := ocm.NewOpenShiftControllerManagerParams(hcp, observedConfig, releaseImageProvider, r.SetDefaultSecurityContext) config := manifests.OpenShiftControllerManagerConfig(hcp.Namespace) if _, err := createOrUpdate(ctx, r, config, func() error { - return ocm.ReconcileOpenShiftControllerManagerConfig(config, p.OwnerRef, p.DeployerImage, p.DockerBuilderImage, p.MinTLSVersion(), p.CipherSuites(), p.Image, p.Build, p.Network) + return ocm.ReconcileOpenShiftControllerManagerConfig(config, + p.OwnerRef, p.DeployerImage, p.DockerBuilderImage, + p.MinTLSVersion(), p.CipherSuites(), p.Image, p.Build, + p.Network, hcp.Spec.Capabilities) }); err != nil { return fmt.Errorf("failed to reconcile openshift controller manager config: %w", err) } @@ -3694,7 +3700,7 @@ func (r *HostedControlPlaneReconciler) reconcileClusterVersionOperator(ctx conte deployment := manifests.ClusterVersionOperatorDeployment(hcp.Namespace) if _, err := createOrUpdate(ctx, r, deployment, func() error { - return cvo.ReconcileDeployment(deployment, p.OwnerRef, p.DeploymentConfig, controlPlaneReleaseImage, dataPlaneReleaseImage, p.CLIImage, p.AvailabilityProberImage, p.ClusterID, hcp.Spec.UpdateService, p.PlatformType, util.HCPOAuthEnabled(hcp), r.EnableCVOManagementClusterMetricsAccess, p.FeatureSet) + return cvo.ReconcileDeployment(deployment, p.OwnerRef, p.DeploymentConfig, controlPlaneReleaseImage, dataPlaneReleaseImage, p.CLIImage, p.AvailabilityProberImage, p.ClusterID, hcp.Spec.UpdateService, p.PlatformType, util.HCPOAuthEnabled(hcp), r.EnableCVOManagementClusterMetricsAccess, p.FeatureSet, hcp.Spec.Capabilities) }); err != nil { return fmt.Errorf("failed to reconcile cluster version operator deployment: %w", err) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/config.go b/control-plane-operator/controllers/hostedcontrolplane/kas/config.go index 5eff3accdd5..c57ddf97fad 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/config.go @@ -13,6 +13,7 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/cloud/openstack" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/common" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/pki" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/certs" hcpconfig "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/globalconfig" @@ -37,12 +38,17 @@ const ( DefaultEtcdPort = 2379 ) -func ReconcileConfig(config *corev1.ConfigMap, ownerRef hcpconfig.OwnerRef, p KubeAPIServerConfigParams) error { +func ReconcileConfig(config *corev1.ConfigMap, ownerRef hcpconfig.OwnerRef, p KubeAPIServerConfigParams, caps *hyperv1.Capabilities) error { ownerRef.ApplyTo(config) if config.Data == nil { config.Data = map[string]string{} } kasConfig := generateConfig(p) + + if !capabilities.IsImageRegistryCapabilityEnabled(caps) { + kasConfig.ImagePolicyConfig.InternalRegistryHostname = "" + } + serializedConfig, err := json.Marshal(kasConfig) if err != nil { return fmt.Errorf("failed to serialize kube apiserver config: %w", err) diff --git a/control-plane-operator/controllers/hostedcontrolplane/oapi/config.go b/control-plane-operator/controllers/hostedcontrolplane/oapi/config.go index ab21922fb64..a2773619ad8 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/oapi/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/oapi/config.go @@ -9,6 +9,7 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/kas" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/pki" "github.com/openshift/hypershift/support/api" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/certs" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/util" @@ -26,7 +27,7 @@ const ( defaultInternalRegistryHostname = "image-registry.openshift-image-registry.svc:5000" ) -func ReconcileConfig(cm *corev1.ConfigMap, auditWebhookRef *corev1.LocalObjectReference, ownerRef config.OwnerRef, etcdURL, ingressDomain, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, projectConfig *configv1.Project) error { +func ReconcileConfig(cm *corev1.ConfigMap, auditWebhookRef *corev1.LocalObjectReference, ownerRef config.OwnerRef, etcdURL, ingressDomain, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, projectConfig *configv1.Project, caps *hyperv1.Capabilities) error { ownerRef.ApplyTo(cm) if cm.Data == nil { cm.Data = map[string]string{} @@ -37,7 +38,7 @@ func ReconcileConfig(cm *corev1.ConfigMap, auditWebhookRef *corev1.LocalObjectRe return fmt.Errorf("failed to read existing config: %w", err) } } - reconcileConfigObject(openshiftAPIServerConfig, auditWebhookRef, etcdURL, ingressDomain, minTLSVersion, cipherSuites, imageConfig, projectConfig) + reconcileConfigObject(openshiftAPIServerConfig, auditWebhookRef, etcdURL, ingressDomain, minTLSVersion, cipherSuites, imageConfig, projectConfig, caps) serializedConfig, err := util.SerializeResource(openshiftAPIServerConfig, api.Scheme) if err != nil { return fmt.Errorf("failed to serialize openshift apiserver config: %w", err) @@ -46,7 +47,7 @@ func ReconcileConfig(cm *corev1.ConfigMap, auditWebhookRef *corev1.LocalObjectRe return nil } -func reconcileConfigObject(cfg *openshiftcpv1.OpenShiftAPIServerConfig, auditWebhookRef *corev1.LocalObjectReference, etcdURL, ingressDomain, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, projectConfig *configv1.Project) { +func reconcileConfigObject(cfg *openshiftcpv1.OpenShiftAPIServerConfig, auditWebhookRef *corev1.LocalObjectReference, etcdURL, ingressDomain, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, projectConfig *configv1.Project, caps *hyperv1.Capabilities) { cfg.TypeMeta = metav1.TypeMeta{ Kind: "OpenShiftAPIServerConfig", APIVersion: openshiftcpv1.GroupVersion.String(), @@ -83,7 +84,9 @@ func reconcileConfigObject(cfg *openshiftcpv1.OpenShiftAPIServerConfig, auditWeb } // Image policy config - cfg.ImagePolicyConfig.InternalRegistryHostname = defaultInternalRegistryHostname + if capabilities.IsImageRegistryCapabilityEnabled(caps) { + cfg.ImagePolicyConfig.InternalRegistryHostname = defaultInternalRegistryHostname + } if imageConfig != nil { cfg.ImagePolicyConfig.ExternalRegistryHostnames = imageConfig.ExternalRegistryHostnames var allowedRegistries openshiftcpv1.AllowedRegistries diff --git a/control-plane-operator/controllers/hostedcontrolplane/ocm/config.go b/control-plane-operator/controllers/hostedcontrolplane/ocm/config.go index fdf4c7cb158..03624d36e80 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ocm/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ocm/config.go @@ -4,9 +4,11 @@ import ( "fmt" "path" + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/common" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/kas" "github.com/openshift/hypershift/support/api" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/certs" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/util" @@ -23,7 +25,7 @@ const ( ConfigKey = "config.yaml" ) -func ReconcileOpenShiftControllerManagerConfig(cm *corev1.ConfigMap, ownerRef config.OwnerRef, deployerImage, dockerBuilderImage, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, buildConfig *configv1.Build, networkConfig *configv1.NetworkSpec) error { +func ReconcileOpenShiftControllerManagerConfig(cm *corev1.ConfigMap, ownerRef config.OwnerRef, deployerImage, dockerBuilderImage, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, buildConfig *configv1.Build, networkConfig *configv1.NetworkSpec, caps *hyperv1.Capabilities) error { ownerRef.ApplyTo(cm) if cm.Data == nil { @@ -36,7 +38,8 @@ func ReconcileOpenShiftControllerManagerConfig(cm *corev1.ConfigMap, ownerRef co return fmt.Errorf("unable to decode existing openshift controller manager configuration: %w", err) } } - if err := reconcileConfig(config, deployerImage, dockerBuilderImage, minTLSVersion, cipherSuites, imageConfig, buildConfig, networkConfig); err != nil { + if err := reconcileConfig(config, deployerImage, dockerBuilderImage, minTLSVersion, + cipherSuites, imageConfig, buildConfig, networkConfig, caps); err != nil { return err } configStr, err := util.SerializeResource(config, api.Scheme) @@ -47,7 +50,7 @@ func ReconcileOpenShiftControllerManagerConfig(cm *corev1.ConfigMap, ownerRef co return nil } -func reconcileConfig(cfg *openshiftcpv1.OpenShiftControllerManagerConfig, deployerImage, dockerBuilderImage, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, buildConfig *configv1.Build, networkConfig *configv1.NetworkSpec) error { +func reconcileConfig(cfg *openshiftcpv1.OpenShiftControllerManagerConfig, deployerImage, dockerBuilderImage, minTLSVersion string, cipherSuites []string, imageConfig *configv1.ImageSpec, buildConfig *configv1.Build, networkConfig *configv1.NetworkSpec, caps *hyperv1.Capabilities) error { cpath := func(volume, file string) string { dir := volumeMounts.Path(ocmContainerMain().Name, volume) return path.Join(dir, file) @@ -57,16 +60,18 @@ func reconcileConfig(cfg *openshiftcpv1.OpenShiftControllerManagerConfig, deploy APIVersion: openshiftcpv1.GroupVersion.String(), } - // Do not modify cfg.Controllers! - // This field is currently owned by the HCCO. - // When we add Capabilities support, we will set Controllers here - // but we have to remove setting it in the HCCO at the same time. + if !capabilities.IsImageRegistryCapabilityEnabled(caps) { + cfg.Controllers = []string{"*", fmt.Sprintf("-%s", openshiftcpv1.OpenShiftServiceAccountPullSecretsController)} + } cfg.Build.ImageTemplateFormat.Format = dockerBuilderImage cfg.Deployer.ImageTemplateFormat.Format = deployerImage // registry config - cfg.DockerPullSecret.InternalRegistryHostname = config.DefaultImageRegistryHostname + if capabilities.IsImageRegistryCapabilityEnabled(caps) { + cfg.DockerPullSecret.InternalRegistryHostname = config.DefaultImageRegistryHostname + } + if imageConfig != nil { cfg.DockerPullSecret.RegistryURLs = imageConfig.ExternalRegistryHostnames } diff --git a/control-plane-operator/controllers/hostedcontrolplane/ocm/config_test.go b/control-plane-operator/controllers/hostedcontrolplane/ocm/config_test.go index 1cb0f06e6ad..af766ef56e2 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/ocm/config_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/ocm/config_test.go @@ -67,7 +67,7 @@ func TestReconcileOpenShiftControllerManagerConfig(t *testing.T) { params := NewOpenShiftControllerManagerParams(hcp, observedConfig, imageProvider, true) configMap := manifests.OpenShiftControllerManagerConfig(hcp.Namespace) - if err := ReconcileOpenShiftControllerManagerConfig(configMap, config2.OwnerRefFrom(hcp), params.DeployerImage, params.DockerBuilderImage, params.MinTLSVersion(), params.CipherSuites(), imageConfig, buildConfig, networkConfig); err != nil { + if err := ReconcileOpenShiftControllerManagerConfig(configMap, config2.OwnerRefFrom(hcp), params.DeployerImage, params.DockerBuilderImage, params.MinTLSVersion(), params.CipherSuites(), imageConfig, buildConfig, networkConfig, nil); err != nil { t.Fatalf("unexpected error: %v", err) } configMapYaml, err := util.SerializeResource(configMap, api.Scheme) diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents.yaml index bafeea9dbbd..a779f2a246e 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents.yaml @@ -303,31 +303,28 @@ spec: - | #!/bin/bash set -euo pipefail - cat > /tmp/clusterversion.yaml < /tmp/clusterversion.json < /dev/null || oc create ns openshift-config oc get ns openshift-config-managed &> /dev/null || oc create ns openshift-config-managed + oc apply -f /var/payload/manifests/0000_00_cluster-version-operator_01_clusterversions* + oc apply -f /tmp/clusterversion.json while true; do - echo "Applying CVO bootstrap manifests" + echo "Applying CVO bootstrap manifests..." if oc apply -f /var/payload/manifests; then echo "Bootstrap manifests applied successfully." break fi sleep 1 done - oc get clusterversion/version &> /dev/null || oc create -f /tmp/clusterversion.yaml command: - /bin/bash env: - name: KUBECONFIG value: /etc/kubernetes/kubeconfig - - name: CLUSTER_ID + - name: CLUSTER_VERSION_JSON + value: '{"kind":"ClusterVersion","apiVersion":"config.openshift.io/v1","metadata":{"name":"version","creationTimestamp":null},"spec":{"clusterID":"","signatureStores":null},"status":{"desired":{"version":"","image":""},"observedGeneration":0,"versionHash":"","capabilities":{},"availableUpdates":null}}' image: cli imagePullPolicy: IfNotPresent name: bootstrap diff --git a/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents_TechPreviewNoUpgrade.yaml b/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents_TechPreviewNoUpgrade.yaml index ba070a66099..482b7265ba5 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents_TechPreviewNoUpgrade.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/testdata/cluster-version-operator/zz_fixture_TestControlPlaneComponents_TechPreviewNoUpgrade.yaml @@ -303,31 +303,28 @@ spec: - | #!/bin/bash set -euo pipefail - cat > /tmp/clusterversion.yaml < /tmp/clusterversion.json < /dev/null || oc create ns openshift-config oc get ns openshift-config-managed &> /dev/null || oc create ns openshift-config-managed + oc apply -f /var/payload/manifests/0000_00_cluster-version-operator_01_clusterversions* + oc apply -f /tmp/clusterversion.json while true; do - echo "Applying CVO bootstrap manifests" + echo "Applying CVO bootstrap manifests..." if oc apply -f /var/payload/manifests; then echo "Bootstrap manifests applied successfully." break fi sleep 1 done - oc get clusterversion/version &> /dev/null || oc create -f /tmp/clusterversion.yaml command: - /bin/bash env: - name: KUBECONFIG value: /etc/kubernetes/kubeconfig - - name: CLUSTER_ID + - name: CLUSTER_VERSION_JSON + value: '{"kind":"ClusterVersion","apiVersion":"config.openshift.io/v1","metadata":{"name":"version","creationTimestamp":null},"spec":{"clusterID":"","signatureStores":null},"status":{"desired":{"version":"","image":""},"observedGeneration":0,"versionHash":"","capabilities":{},"availableUpdates":null}}' image: cli imagePullPolicy: IfNotPresent name: bootstrap diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/assets/cluster-version-operator/deployment.yaml b/control-plane-operator/controllers/hostedcontrolplane/v2/assets/cluster-version-operator/deployment.yaml index 9be66607e18..f43b002ccda 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/assets/cluster-version-operator/deployment.yaml +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/assets/cluster-version-operator/deployment.yaml @@ -88,25 +88,21 @@ spec: - | #!/bin/bash set -euo pipefail - cat > /tmp/clusterversion.yaml < /tmp/clusterversion.json < /dev/null || oc create ns openshift-config oc get ns openshift-config-managed &> /dev/null || oc create ns openshift-config-managed + oc apply -f /var/payload/manifests/0000_00_cluster-version-operator_01_clusterversions* + oc apply -f /tmp/clusterversion.json while true; do - echo "Applying CVO bootstrap manifests" + echo "Applying CVO bootstrap manifests..." if oc apply -f /var/payload/manifests; then echo "Bootstrap manifests applied successfully." break fi sleep 1 done - oc get clusterversion/version &> /dev/null || oc create -f /tmp/clusterversion.yaml command: - /bin/bash env: diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go b/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go index b142bd9d942..c6a5217c5a2 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go @@ -1,6 +1,7 @@ package cvo import ( + "encoding/json" "fmt" "path" "strings" @@ -8,6 +9,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/common" hyperapi "github.com/openshift/hypershift/support/api" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/config" component "github.com/openshift/hypershift/support/controlplane-component" "github.com/openshift/hypershift/support/util" @@ -51,10 +53,36 @@ func (cvo *clusterVersionOperator) adaptDeployment(cpContext component.WorkloadC } c.Image = controlPlaneReleaseImage }) + + // the ClusterVersion resource is created by the CVO bootstrap container. + // we marshal it to json as a means to validate its formatting, which protects + // us against easily preventable mistakes, such as typos. + cv := &configv1.ClusterVersion{ + TypeMeta: metav1.TypeMeta{ + Kind: "ClusterVersion", + APIVersion: "config.openshift.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(cpContext.HCP.Spec.ClusterID), + }, + } + if !capabilities.IsImageRegistryCapabilityEnabled(cpContext.HCP.Spec.Capabilities) { + cv.Spec.Capabilities = &configv1.ClusterVersionCapabilitiesSpec{ + BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetNone, + AdditionalEnabledCapabilities: capabilities.CalculateEnabledCapabilities(cpContext.HCP.Spec.Capabilities), + } + } + clusterVersionJSON, err := json.Marshal(cv) + if err != nil { + return err + } util.UpdateContainer("bootstrap", deployment.Spec.Template.Spec.InitContainers, func(c *corev1.Container) { c.Env = append(c.Env, corev1.EnvVar{ - Name: "CLUSTER_ID", - Value: cpContext.HCP.Spec.ClusterID, + Name: "CLUSTER_VERSION_JSON", + Value: string(clusterVersionJSON), }) }) diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/params.go b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/params.go index e8d455f6653..f52a22edc98 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/params.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/params.go @@ -7,6 +7,7 @@ import ( "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/cloud/aws" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/cloud/azure" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/globalconfig" "github.com/openshift/hypershift/support/util" @@ -64,7 +65,6 @@ func NewConfigParams(hcp *hyperv1.HostedControlPlane) KubeAPIServerConfigParams KASPodPort: util.KASPodPort(hcp), TLSSecurityProfile: tlsSecurityProfile(hcp.Spec.Configuration), AdditionalCORSAllowedOrigins: additionalCORSAllowedOrigins(hcp.Spec.Configuration), - InternalRegistryHostName: config.DefaultImageRegistryHostname, ExternalRegistryHostNames: externalRegistryHostNames(hcp.Spec.Configuration), DefaultNodeSelector: defaultNodeSelector(hcp.Spec.Configuration), AdvertiseAddress: util.GetAdvertiseAddress(hcp, config.DefaultAdvertiseIPv4Address, config.DefaultAdvertiseIPv6Address), @@ -118,6 +118,10 @@ func NewConfigParams(hcp *hyperv1.HostedControlPlane) KubeAPIServerConfigParams kasConfig.MaxMutatingRequestsInflight = mutatingReqInflight } + if capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + kasConfig.InternalRegistryHostName = config.DefaultImageRegistryHostname + } + return kasConfig } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/oapi/config.go b/control-plane-operator/controllers/hostedcontrolplane/v2/oapi/config.go index c9ea118237f..ae1538ec1c5 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/oapi/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/oapi/config.go @@ -6,6 +6,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/support/api" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/config" component "github.com/openshift/hypershift/support/controlplane-component" "github.com/openshift/hypershift/support/globalconfig" @@ -68,6 +69,10 @@ func adaptConfig(cfg *openshiftcpv1.OpenShiftAPIServerConfig, hcp *hyperv1.Hoste cfg.ImagePolicyConfig.AllowedRegistriesForImport = allowedRegistries } + if !capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + cfg.ImagePolicyConfig.InternalRegistryHostname = "" + } + // Routing config cfg.RoutingConfig.Subdomain = globalconfig.IngressDomain(hcp) diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go index bf5a9502080..359480696e3 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go @@ -6,6 +6,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/imageprovider" "github.com/openshift/hypershift/support/api" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/config" component "github.com/openshift/hypershift/support/controlplane-component" "github.com/openshift/hypershift/support/globalconfig" @@ -38,7 +39,7 @@ func adaptConfigMap(cpContext component.WorkloadContext, cm *corev1.ConfigMap) e return fmt.Errorf("failed to read observed global config: %w", err) } - adaptConfig(config, cpContext.HCP.Spec.Configuration, cpContext.ReleaseImageProvider, observedConfig.Build) + adaptConfig(config, cpContext.HCP.Spec.Configuration, cpContext.ReleaseImageProvider, observedConfig.Build, cpContext.HCP.Spec.Capabilities) configStr, err := util.SerializeResource(config, api.Scheme) if err != nil { return fmt.Errorf("failed to serialize openshift controller manager configuration: %w", err) @@ -47,15 +48,15 @@ func adaptConfigMap(cpContext component.WorkloadContext, cm *corev1.ConfigMap) e return nil } -func adaptConfig(cfg *openshiftcpv1.OpenShiftControllerManagerConfig, configuration *hyperv1.ClusterConfiguration, releaseImageProvider imageprovider.ReleaseImageProvider, buildConfig *configv1.Build) { - // Do not modify cfg.Controllers! - // This field is currently owned by the HCCO. - // When we add Capabilities support, we will set Controllers here - // but we have to remove setting it in the HCCO at the same time. - +func adaptConfig(cfg *openshiftcpv1.OpenShiftControllerManagerConfig, configuration *hyperv1.ClusterConfiguration, releaseImageProvider imageprovider.ReleaseImageProvider, buildConfig *configv1.Build, caps *hyperv1.Capabilities) { cfg.Build.ImageTemplateFormat.Format = releaseImageProvider.GetImage("docker-builder") cfg.Deployer.ImageTemplateFormat.Format = releaseImageProvider.GetImage("deployer") + if !capabilities.IsImageRegistryCapabilityEnabled(caps) { + cfg.Controllers = []string{"*", fmt.Sprintf("-%s", openshiftcpv1.OpenShiftServiceAccountPullSecretsController)} + cfg.DockerPullSecret.InternalRegistryHostname = "" + } + if configuration != nil && configuration.Image != nil { cfg.DockerPullSecret.RegistryURLs = configuration.Image.ExternalRegistryHostnames } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config_test.go b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config_test.go index 3d188f1beaf..23b28bce0a2 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config_test.go @@ -18,66 +18,78 @@ import ( ) func TestReconcileOpenShiftControllerManagerConfig(t *testing.T) { - hcp := &hyperv1.HostedControlPlane{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "test-namespace", - }, - Spec: hyperv1.HostedControlPlaneSpec{ - ReleaseImage: "quay.io/ocp-dev/test-release-image:latest", - Platform: hyperv1.PlatformSpec{ - Type: hyperv1.AWSPlatform, - }, - IssuerURL: "https://www.example.com", - Configuration: &hyperv1.ClusterConfiguration{ - Image: &v1.ImageSpec{}, - Network: &v1.NetworkSpec{ - ExternalIP: &v1.ExternalIPConfig{ - AutoAssignCIDRs: []string{"99.1.0.0/24"}, + testFunc := func(capabilities *hyperv1.Capabilities) func(*testing.T) { + return func(t *testing.T) { + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test-namespace", + }, + Spec: hyperv1.HostedControlPlaneSpec{ + ReleaseImage: "quay.io/ocp-dev/test-release-image:latest", + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + }, + IssuerURL: "https://www.example.com", + Configuration: &hyperv1.ClusterConfiguration{ + Image: &v1.ImageSpec{}, + Network: &v1.NetworkSpec{ + ExternalIP: &v1.ExternalIPConfig{ + AutoAssignCIDRs: []string{"99.1.0.0/24"}, + }, + }, }, + Capabilities: capabilities, }, - }, - }, - } - images := map[string]string{ - "openshift-controller-manager": "quay.io/test/openshift-controller-manager", - "docker-builder": "quay.io/test/docker-builder", - "deployer": "quay.io/test/deployer", - } - imageProvider := imageprovider.NewFromImages(images) + } + images := map[string]string{ + "openshift-controller-manager": "quay.io/test/openshift-controller-manager", + "docker-builder": "quay.io/test/docker-builder", + "deployer": "quay.io/test/deployer", + } + imageProvider := imageprovider.NewFromImages(images) - buildConfig := &v1.Build{ - Spec: v1.BuildSpec{ - BuildDefaults: v1.BuildDefaults{ - Env: []corev1.EnvVar{ - { - Name: "TEST_VAR", - Value: "TEST_VALUE", + buildConfig := &v1.Build{ + Spec: v1.BuildSpec{ + BuildDefaults: v1.BuildDefaults{ + Env: []corev1.EnvVar{ + { + Name: "TEST_VAR", + Value: "TEST_VALUE", + }, + }, }, }, - }, - }, - } + } - configMap := &corev1.ConfigMap{} - controlplanecomponent.LoadManifestInto(ComponentName, "config.yaml", configMap) + configMap := &corev1.ConfigMap{} + controlplanecomponent.LoadManifestInto(ComponentName, "config.yaml", configMap) - config := &openshiftcpv1.OpenShiftControllerManagerConfig{} - err := util.DeserializeResource(configMap.Data[configKey], config, api.Scheme) - if err != nil { - t.Fatalf("unable to decode existing openshift controller manager configuration: %v", err) - } + config := &openshiftcpv1.OpenShiftControllerManagerConfig{} + err := util.DeserializeResource(configMap.Data[configKey], config, api.Scheme) + if err != nil { + t.Fatalf("unable to decode existing openshift controller manager configuration: %v", err) + } - adaptConfig(config, hcp.Spec.Configuration, imageProvider, buildConfig) - configStr, err := util.SerializeResource(config, api.Scheme) - if err != nil { - t.Fatalf("failed to serialize openshift controller manager configuration: %v", err) + adaptConfig(config, hcp.Spec.Configuration, imageProvider, buildConfig, hcp.Spec.Capabilities) + configStr, err := util.SerializeResource(config, api.Scheme) + if err != nil { + t.Fatalf("failed to serialize openshift controller manager configuration: %v", err) + } + configMap.Data[configKey] = configStr + + configMapYaml, err := util.SerializeResource(configMap, api.Scheme) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testutil.CompareWithFixture(t, configMapYaml) + } } - configMap.Data[configKey] = configStr - configMapYaml, err := util.SerializeResource(configMap, api.Scheme) - if err != nil { - t.Fatalf("unexpected error: %v", err) + caps := &hyperv1.Capabilities{ + DisabledCapabilities: []hyperv1.OptionalCapability{hyperv1.ImageRegistryCapability}, } - testutil.CompareWithFixture(t, configMapYaml) + + t.Run("WithAllCapabilitiesEnabled", testFunc(nil)) + t.Run("WithImageRegistryCapabilityDisabled", testFunc(caps)) } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/testdata/zz_fixture_TestReconcileOpenShiftControllerManagerConfig.yaml b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/testdata/zz_fixture_TestReconcileOpenShiftControllerManagerConfig_WithAllCapabilitiesEnabled.yaml similarity index 100% rename from control-plane-operator/controllers/hostedcontrolplane/v2/ocm/testdata/zz_fixture_TestReconcileOpenShiftControllerManagerConfig.yaml rename to control-plane-operator/controllers/hostedcontrolplane/v2/ocm/testdata/zz_fixture_TestReconcileOpenShiftControllerManagerConfig_WithAllCapabilitiesEnabled.yaml diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/testdata/zz_fixture_TestReconcileOpenShiftControllerManagerConfig_WithImageRegistryCapabilityDisabled.yaml b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/testdata/zz_fixture_TestReconcileOpenShiftControllerManagerConfig_WithImageRegistryCapabilityDisabled.yaml new file mode 100644 index 00000000000..43edc6387db --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/testdata/zz_fixture_TestReconcileOpenShiftControllerManagerConfig_WithImageRegistryCapabilityDisabled.yaml @@ -0,0 +1,81 @@ +apiVersion: v1 +data: + config.yaml: | + apiVersion: openshiftcontrolplane.config.openshift.io/v1 + build: + additionalTrustedCA: "" + buildDefaults: + env: + - name: TEST_VAR + value: TEST_VALUE + resources: {} + buildOverrides: null + imageTemplateFormat: + format: quay.io/test/docker-builder + latest: false + controllers: + - '*' + - -openshift.io/serviceaccount-pull-secrets + deployer: + imageTemplateFormat: + format: quay.io/test/deployer + latest: false + dockerPullSecret: + internalRegistryHostname: "" + registryURLs: null + featureGates: null + imageImport: + disableScheduledImport: false + maxScheduledImageImportsPerMinute: 0 + scheduledImageImportMinimumIntervalSeconds: 0 + ingress: + ingressIPNetworkCIDR: 99.1.0.0/24 + kind: OpenShiftControllerManagerConfig + kubeClientConfig: + connectionOverrides: + acceptContentTypes: "" + burst: 0 + contentType: "" + qps: 0 + kubeConfig: /etc/kubernetes/secrets/svc-kubeconfig/kubeconfig + leaderElection: + leaseDuration: 0s + renewDeadline: 0s + retryPeriod: 0s + network: + clusterNetworks: null + networkPluginName: "" + serviceNetworkCIDR: "" + vxlanPort: 0 + resourceQuota: + concurrentSyncs: 0 + minResyncPeriod: 0s + syncPeriod: 0s + securityAllocator: + mcsAllocatorRange: "" + mcsLabelsPerProject: 0 + uidAllocatorRange: "" + serviceAccount: + managedNames: null + serviceServingCert: + signer: null + servingInfo: + bindAddress: 0.0.0.0:8443 + bindNetwork: "" + certFile: /etc/kubernetes/certs/tls.crt + cipherSuites: + - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 + - TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 + - TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 + - TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 + - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 + - TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 + clientCA: /etc/kubernetes/client-ca/ca.crt + keyFile: /etc/kubernetes/certs/tls.key + maxRequestsInFlight: 0 + minTLSVersion: VersionTLS12 + requestTimeoutSeconds: 0 +kind: ConfigMap +metadata: + creationTimestamp: null + name: openshift-controller-manager-config diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/registryoperator/component.go b/control-plane-operator/controllers/hostedcontrolplane/v2/registryoperator/component.go index 12dfa3008b8..6fdcf899104 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/registryoperator/component.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/registryoperator/component.go @@ -3,6 +3,7 @@ package registryoperator import ( oapiv2 "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/oapi" "github.com/openshift/hypershift/support/azureutil" + "github.com/openshift/hypershift/support/capabilities" component "github.com/openshift/hypershift/support/controlplane-component" ) @@ -33,6 +34,7 @@ func (r *imageRegistryOperator) NeedsManagementKASAccess() bool { func NewComponent() component.ControlPlaneComponent { return component.NewDeploymentComponent(ComponentName, &imageRegistryOperator{}). WithAdaptFunction(adaptDeployment). + WithPredicate(isImageRegistryCapabilityEnabled). WithManifestAdapter( "podmonitor.yaml", component.WithAdaptFunction(adaptPodMonitor), @@ -46,6 +48,12 @@ func NewComponent() component.ControlPlaneComponent { Build() } +func isImageRegistryCapabilityEnabled(cpContext component.WorkloadContext) (bool, error) { + return capabilities.IsImageRegistryCapabilityEnabled( + cpContext.HCP.Spec.Capabilities, + ), nil +} + func isAroHCP(cpContext component.WorkloadContext) bool { return azureutil.IsAroHCP() } diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go index c9c1eb6d883..320927e623f 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go @@ -39,6 +39,7 @@ import ( "github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/operator" "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool" "github.com/openshift/hypershift/support/azureutil" + "github.com/openshift/hypershift/support/capabilities" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/globalconfig" "github.com/openshift/hypershift/support/releaseinfo" @@ -374,41 +375,44 @@ func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result // in CIRO before the registry config is created. For now, this is the case for the OpenStack platform. // If the object exist, we reconcile the registry config for other fields as it should be fine since the PVC would // exist at this point. - if imageRegistryPlatformWithPVC(hcp.Spec.Platform.Type) && (!registryConfigExists || registryConfig == nil) { - log.Info("skipping registry config to let CIRO bootstrap") - } else { - log.Info("reconciling registry config") - if _, err := r.CreateOrUpdate(ctx, r.client, registryConfig, func() error { - registry.ReconcileRegistryConfig(registryConfig, r.platformType, hcp.Spec.InfrastructureAvailabilityPolicy) - return nil - }); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile imageregistry config: %w", err)) - } - if registryConfig.Spec.ManagementState == operatorv1.Removed && r.platformType != hyperv1.IBMCloudPlatform { - log.Info("imageregistry operator managementstate is removed, disabling openshift-controller-manager controllers and cleaning up resources") - ocmConfigMap := cpomanifests.OpenShiftControllerManagerConfig(r.hcpNamespace) - if _, err := r.CreateOrUpdate(ctx, r.cpClient, ocmConfigMap, func() error { - if ocmConfigMap.Data == nil { - // CPO has not created the configmap yet, wait for create - // This should not happen as we are started by the CPO after the configmap should be created - return nil - } - config := &openshiftcpv1.OpenShiftControllerManagerConfig{} - if configStr, exists := ocmConfigMap.Data[ocm.ConfigKey]; exists && len(configStr) > 0 { - err := util.DeserializeResource(configStr, config, api.Scheme) + if capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + if imageRegistryPlatformWithPVC(hcp.Spec.Platform.Type) && (!registryConfigExists || registryConfig == nil) { + log.Info("skipping registry config to let CIRO bootstrap") + } else { + log.Info("reconciling registry config") + if _, err := r.CreateOrUpdate(ctx, r.client, registryConfig, func() error { + registry.ReconcileRegistryConfig(registryConfig, r.platformType, hcp.Spec.InfrastructureAvailabilityPolicy) + return nil + }); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile imageregistry config: %w", err)) + } + // TODO(fmissi): remove this when Hypershift Capabilities becomes GA + if registryConfig.Spec.ManagementState == operatorv1.Removed && r.platformType != hyperv1.IBMCloudPlatform { + log.Info("imageregistry operator managementstate is removed, disabling openshift-controller-manager controllers and cleaning up resources") + ocmConfigMap := cpomanifests.OpenShiftControllerManagerConfig(r.hcpNamespace) + if _, err := r.CreateOrUpdate(ctx, r.cpClient, ocmConfigMap, func() error { + if ocmConfigMap.Data == nil { + // CPO has not created the configmap yet, wait for create + // This should not happen as we are started by the CPO after the configmap should be created + return nil + } + config := &openshiftcpv1.OpenShiftControllerManagerConfig{} + if configStr, exists := ocmConfigMap.Data[ocm.ConfigKey]; exists && len(configStr) > 0 { + err := util.DeserializeResource(configStr, config, api.Scheme) + if err != nil { + return fmt.Errorf("unable to decode existing openshift controller manager configuration: %w", err) + } + } + config.Controllers = []string{"*", fmt.Sprintf("-%s", openshiftcpv1.OpenShiftServiceAccountPullSecretsController)} + configStr, err := util.SerializeResource(config, api.Scheme) if err != nil { - return fmt.Errorf("unable to decode existing openshift controller manager configuration: %w", err) + return fmt.Errorf("failed to serialize openshift controller manager configuration: %w", err) } + ocmConfigMap.Data[ocm.ConfigKey] = configStr + return nil + }); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile openshift-controller-manager config: %w", err)) } - config.Controllers = []string{"*", fmt.Sprintf("-%s", openshiftcpv1.OpenShiftServiceAccountPullSecretsController)} - configStr, err := util.SerializeResource(config, api.Scheme) - if err != nil { - return fmt.Errorf("failed to serialize openshift controller manager configuration: %w", err) - } - ocmConfigMap.Data[ocm.ConfigKey] = configStr - return nil - }); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile openshift-controller-manager config: %w", err)) } } } @@ -1230,6 +1234,12 @@ func (r *reconciler) reconcileClusterVersion(ctx context.Context, hcp *hyperv1.H if _, err := r.CreateOrUpdate(ctx, r.client, clusterVersion, func() error { clusterVersion.Spec.ClusterID = configv1.ClusterID(hcp.Spec.ClusterID) clusterVersion.Spec.Capabilities = nil + if !capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + clusterVersion.Spec.Capabilities = &configv1.ClusterVersionCapabilitiesSpec{ + BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetNone, + AdditionalEnabledCapabilities: capabilities.CalculateEnabledCapabilities(hcp.Spec.Capabilities), + } + } clusterVersion.Spec.Upstream = hcp.Spec.UpdateService clusterVersion.Spec.Channel = hcp.Spec.Channel clusterVersion.Spec.DesiredUpdate = nil @@ -1483,15 +1493,23 @@ func (r *reconciler) reconcileCloudCredentialSecrets(ctx context.Context, hcp *h return nil } for arn, secret := range map[string]*corev1.Secret{ - hcp.Spec.Platform.AWS.RolesRef.IngressARN: manifests.AWSIngressCloudCredsSecret(), - hcp.Spec.Platform.AWS.RolesRef.StorageARN: manifests.AWSStorageCloudCredsSecret(), - hcp.Spec.Platform.AWS.RolesRef.ImageRegistryARN: manifests.AWSImageRegistryCloudCredsSecret(), + hcp.Spec.Platform.AWS.RolesRef.IngressARN: manifests.AWSIngressCloudCredsSecret(), + hcp.Spec.Platform.AWS.RolesRef.StorageARN: manifests.AWSStorageCloudCredsSecret(), } { err := syncSecret(secret, arn) if err != nil { errs = append(errs, err) } } + if capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + err := syncSecret( + manifests.AWSImageRegistryCloudCredsSecret(), + hcp.Spec.Platform.AWS.RolesRef.ImageRegistryARN, + ) + if err != nil { + errs = append(errs, err) + } + } case hyperv1.AzurePlatform: secretData := map[string][]byte{ "azure_federated_token_file": []byte("/var/run/secrets/openshift/serviceaccount/token"), @@ -1524,13 +1542,15 @@ func (r *reconciler) reconcileCloudCredentialSecrets(ctx context.Context, hcp *h errs = append(errs, fmt.Errorf("failed to reconcile guest cluster CSI secret: %w", err)) } - imageRegistrySecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "openshift-image-registry", Name: "installer-cloud-credentials"}} - if _, err := r.CreateOrUpdate(ctx, r.client, imageRegistrySecret, func() error { - secretData["azure_client_id"] = []byte(hcp.Spec.Platform.Azure.ManagedIdentities.DataPlane.ImageRegistryMSIClientID) - imageRegistrySecret.Data = secretData - return nil - }); err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile guest cluster image-registry secret: %w", err)) + if capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + imageRegistrySecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "openshift-image-registry", Name: "installer-cloud-credentials"}} + if _, err := r.CreateOrUpdate(ctx, r.client, imageRegistrySecret, func() error { + secretData["azure_client_id"] = []byte(hcp.Spec.Platform.Azure.ManagedIdentities.DataPlane.ImageRegistryMSIClientID) + imageRegistrySecret.Data = secretData + return nil + }); err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile guest cluster image-registry secret: %w", err)) + } } azureFileCSISecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "openshift-cluster-csi-drivers", Name: "azure-file-credentials"}} @@ -1606,22 +1626,24 @@ func (r *reconciler) reconcileCloudCredentialSecrets(ctx context.Context, hcp *h errs = append(errs, fmt.Errorf("failed to reconcile powervs storage cloud credentials secret %w", err)) } - var imageRegistryCredentials corev1.Secret - err = r.cpClient.Get(ctx, client.ObjectKey{Namespace: hcp.Namespace, Name: hcp.Spec.Platform.PowerVS.ImageRegistryOperatorCloudCreds.Name}, &imageRegistryCredentials) - if err != nil { - errs = append(errs, fmt.Errorf("failed to get image registry operator cloud credentials secret %s from hcp namespace : %w", hcp.Spec.Platform.PowerVS.ImageRegistryOperatorCloudCreds.Name, err)) - return errs - } + if capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + var imageRegistryCredentials corev1.Secret + err = r.cpClient.Get(ctx, client.ObjectKey{Namespace: hcp.Namespace, Name: hcp.Spec.Platform.PowerVS.ImageRegistryOperatorCloudCreds.Name}, &imageRegistryCredentials) + if err != nil { + errs = append(errs, fmt.Errorf("failed to get image registry operator cloud credentials secret %s from hcp namespace : %w", hcp.Spec.Platform.PowerVS.ImageRegistryOperatorCloudCreds.Name, err)) + return errs + } - imageRegistryInstallerCloudCredentials := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "openshift-image-registry", - Name: "installer-cloud-credentials", - }, - } - err = createPowerVSSecret(&imageRegistryCredentials, imageRegistryInstallerCloudCredentials) - if err != nil { - errs = append(errs, fmt.Errorf("failed to reconcile powervs image registry cloud credentials secret %w", err)) + imageRegistryInstallerCloudCredentials := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "openshift-image-registry", + Name: "installer-cloud-credentials", + }, + } + err = createPowerVSSecret(&imageRegistryCredentials, imageRegistryInstallerCloudCredentials) + if err != nil { + errs = append(errs, fmt.Errorf("failed to reconcile powervs image registry cloud credentials secret %w", err)) + } } } return errs @@ -2023,18 +2045,20 @@ func (r *reconciler) ensureCloudResourcesDestroyed(ctx context.Context, hcp *hyp return remaining, err } var errs []error - log.Info("Ensuring image registry storage is removed") - removed, err := r.ensureImageRegistryStorageRemoved(ctx) - if err != nil { - errs = append(errs, err) - } - if !removed { - remaining.Insert("image-registry") - } else { - log.Info("Image registry is removed") + if capabilities.IsImageRegistryCapabilityEnabled(hcp.Spec.Capabilities) { + log.Info("Ensuring image registry storage is removed") + removed, err := r.ensureImageRegistryStorageRemoved(ctx) + if err != nil { + errs = append(errs, err) + } + if !removed { + remaining.Insert("image-registry") + } else { + log.Info("Image registry is removed") + } } log.Info("Ensuring ingress controllers are removed") - removed, err = r.ensureIngressControllersRemoved(ctx, hcp) + removed, err := r.ensureIngressControllersRemoved(ctx, hcp) if err != nil { errs = append(errs, err) } diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go index 80a8f513a61..4e9a80a3c71 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go @@ -955,6 +955,84 @@ func TestReconcileClusterVersion(t *testing.T) { g.Expect(clusterVersion.Spec.Channel).To(BeEmpty()) } +func TestReconcileClusterVersionWithDisabledCapabilities(t *testing.T) { + hcp := &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + ClusterID: "test-cluster-id", + Capabilities: &hyperv1.Capabilities{ + DisabledCapabilities: []hyperv1.OptionalCapability{ + hyperv1.ImageRegistryCapability, + }, + }, + }, + } + testOverrides := []configv1.ComponentOverride{ + { + Kind: "Pod", + Group: "", + Name: "test", + Namespace: "default", + Unmanaged: true, + }, + } + clusterVersion := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: "some-other-id", + Capabilities: &configv1.ClusterVersionCapabilitiesSpec{ + AdditionalEnabledCapabilities: []configv1.ClusterVersionCapability{ + "foo", + "bar", + }, + }, + Channel: "fast", + DesiredUpdate: &configv1.Update{ + Version: "4.12.5", + Image: "example.com/imagens/image:latest", + Force: true, + }, + Upstream: configv1.URL("https://upstream.example.com"), + Overrides: testOverrides, + }, + } + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(clusterVersion).Build() + g := NewWithT(t) + r := &reconciler{ + client: fakeClient, + CreateOrUpdateProvider: &simpleCreateOrUpdater{}, + } + err := r.reconcileClusterVersion(context.Background(), hcp) + g.Expect(err).ToNot(HaveOccurred()) + err = fakeClient.Get(context.Background(), client.ObjectKeyFromObject(clusterVersion), clusterVersion) + g.Expect(err).ToNot(HaveOccurred()) + + expectedCapabilities := &configv1.ClusterVersionCapabilitiesSpec{ + BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetNone, + AdditionalEnabledCapabilities: []configv1.ClusterVersionCapability{ + configv1.ClusterVersionCapabilityBuild, + configv1.ClusterVersionCapabilityCSISnapshot, + configv1.ClusterVersionCapabilityCloudControllerManager, + configv1.ClusterVersionCapabilityCloudCredential, + configv1.ClusterVersionCapabilityConsole, + configv1.ClusterVersionCapabilityDeploymentConfig, + // configv1.ClusterVersionCapabilityImageRegistry, + configv1.ClusterVersionCapabilityIngress, + configv1.ClusterVersionCapabilityInsights, + configv1.ClusterVersionCapabilityMachineAPI, + configv1.ClusterVersionCapabilityNodeTuning, + configv1.ClusterVersionCapabilityOperatorLifecycleManager, + configv1.ClusterVersionCapabilityOperatorLifecycleManagerV1, + configv1.ClusterVersionCapabilityStorage, + configv1.ClusterVersionCapabilityBaremetal, + configv1.ClusterVersionCapabilityMarketplace, + configv1.ClusterVersionCapabilityOpenShiftSamples, + }, + } + g.Expect(clusterVersion.Spec.Capabilities).To(Equal(expectedCapabilities)) +} + func TestReconcileImageContentPolicyType(t *testing.T) { testCases := []struct { name string diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 2f2ac94d79b..82129a9db72 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -2146,6 +2146,8 @@ func reconcileHostedControlPlane(hcp *hyperv1.HostedControlPlane, hcluster *hype hcp.Spec.Configuration = nil } + hcp.Spec.Capabilities = hcluster.Spec.Capabilities + return nil } diff --git a/support/capabilities/hosted_control_plane_capabilities.go b/support/capabilities/hosted_control_plane_capabilities.go new file mode 100644 index 00000000000..f32cd20f173 --- /dev/null +++ b/support/capabilities/hosted_control_plane_capabilities.go @@ -0,0 +1,54 @@ +package capabilities + +import ( + "slices" + "strings" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + + configv1 "github.com/openshift/api/config/v1" + + "k8s.io/apimachinery/pkg/util/sets" +) + +// IsImageRegistryCapabilityEnabled returns true if the Image Registry +// capability is enabled, or false if disabled. +// +// The Image Registry capability is enabled by default. +func IsImageRegistryCapabilityEnabled(capabilities *hyperv1.Capabilities) bool { + if capabilities == nil { + return true + } + enabled := true + for _, disabledCap := range capabilities.DisabledCapabilities { + if disabledCap == hyperv1.ImageRegistryCapability { + enabled = false + } + } + return enabled +} + +// CalculateEnabledCapabilities returns the difference between the default set +// of enabled capabilities (vCurrent) and the given set of capabilities to +// disable, in alphabetical order. +func CalculateEnabledCapabilities(capabilities *hyperv1.Capabilities) []configv1.ClusterVersionCapability { + vCurrent := configv1.ClusterVersionCapabilitySets[configv1.ClusterVersionCapabilitySetCurrent] + enabledCaps := sets.New[configv1.ClusterVersionCapability](vCurrent...) + + if capabilities != nil && len(capabilities.DisabledCapabilities) > 0 { + disabledCaps := make([]configv1.ClusterVersionCapability, len(capabilities.DisabledCapabilities)) + for i, dc := range capabilities.DisabledCapabilities { + disabledCaps[i] = configv1.ClusterVersionCapability(dc) + } + enabledCaps = enabledCaps.Delete(disabledCaps...) + } + + return sortedCapabilities(enabledCaps.UnsortedList()) +} + +func sortedCapabilities(caps []configv1.ClusterVersionCapability) []configv1.ClusterVersionCapability { + slices.SortFunc(caps, func(a, b configv1.ClusterVersionCapability) int { + return strings.Compare(string(a), string(b)) + }) + return caps +} diff --git a/support/capabilities/hosted_control_plane_capabilities_test.go b/support/capabilities/hosted_control_plane_capabilities_test.go new file mode 100644 index 00000000000..876c71d41d7 --- /dev/null +++ b/support/capabilities/hosted_control_plane_capabilities_test.go @@ -0,0 +1,113 @@ +package capabilities + +import ( + "reflect" + "testing" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + + configv1 "github.com/openshift/api/config/v1" +) + +func TestIsImageRegistryCapabilityEnabled(t *testing.T) { + tests := []struct { + name string + disabledCapabilities []hyperv1.OptionalCapability + expectImageRegistryEnabled bool + }{ + { + name: "returns false when image registry capability is disabled", + disabledCapabilities: []hyperv1.OptionalCapability{hyperv1.ImageRegistryCapability}, + expectImageRegistryEnabled: false, + }, + { + name: "returns true when image registry capability is enabled", + disabledCapabilities: nil, + expectImageRegistryEnabled: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + caps := &hyperv1.Capabilities{ + DisabledCapabilities: test.disabledCapabilities, + } + enabled := IsImageRegistryCapabilityEnabled(caps) + if test.expectImageRegistryEnabled && !enabled { + t.Fatal("expected the registry to be enabled, but it wasn't") + } + if !test.expectImageRegistryEnabled && enabled { + t.Fatal("expected the registry to not be enabled, but it was") + } + }) + } +} + +func TestCalculateEnabledCapabilities(t *testing.T) { + tests := []struct { + name string + disabledCapabilities []hyperv1.OptionalCapability + expectedCapabilities []configv1.ClusterVersionCapability + }{ + { + name: "returns default capability set when disabledCapabilities is nil", + disabledCapabilities: nil, + expectedCapabilities: []configv1.ClusterVersionCapability{ + configv1.ClusterVersionCapabilityBuild, + configv1.ClusterVersionCapabilityCSISnapshot, + configv1.ClusterVersionCapabilityCloudControllerManager, + configv1.ClusterVersionCapabilityCloudCredential, + configv1.ClusterVersionCapabilityConsole, + configv1.ClusterVersionCapabilityDeploymentConfig, + configv1.ClusterVersionCapabilityImageRegistry, + configv1.ClusterVersionCapabilityIngress, + configv1.ClusterVersionCapabilityInsights, + configv1.ClusterVersionCapabilityMachineAPI, + configv1.ClusterVersionCapabilityNodeTuning, + configv1.ClusterVersionCapabilityOperatorLifecycleManager, + configv1.ClusterVersionCapabilityOperatorLifecycleManagerV1, + configv1.ClusterVersionCapabilityStorage, + configv1.ClusterVersionCapabilityBaremetal, + configv1.ClusterVersionCapabilityMarketplace, + configv1.ClusterVersionCapabilityOpenShiftSamples, + }, + }, + { + name: "returns default set minus image registry capability when ImageRegistry capability is Disabled", + disabledCapabilities: []hyperv1.OptionalCapability{hyperv1.ImageRegistryCapability}, + expectedCapabilities: []configv1.ClusterVersionCapability{ + configv1.ClusterVersionCapabilityBuild, + configv1.ClusterVersionCapabilityCSISnapshot, + configv1.ClusterVersionCapabilityCloudControllerManager, + configv1.ClusterVersionCapabilityCloudCredential, + configv1.ClusterVersionCapabilityConsole, + configv1.ClusterVersionCapabilityDeploymentConfig, + // configv1.ClusterVersionCapabilityImageRegistry, + configv1.ClusterVersionCapabilityIngress, + configv1.ClusterVersionCapabilityInsights, + configv1.ClusterVersionCapabilityMachineAPI, + configv1.ClusterVersionCapabilityNodeTuning, + configv1.ClusterVersionCapabilityOperatorLifecycleManager, + configv1.ClusterVersionCapabilityOperatorLifecycleManagerV1, + configv1.ClusterVersionCapabilityStorage, + configv1.ClusterVersionCapabilityBaremetal, + configv1.ClusterVersionCapabilityMarketplace, + configv1.ClusterVersionCapabilityOpenShiftSamples, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + caps := &hyperv1.Capabilities{ + DisabledCapabilities: test.disabledCapabilities, + } + enabledCapabilities := CalculateEnabledCapabilities(caps) + if !reflect.DeepEqual(test.expectedCapabilities, enabledCapabilities) { + t.Logf("expected enabled capabilities: %v", test.expectedCapabilities) + t.Logf("calculated enabled capabilities: %v", enabledCapabilities) + t.Fatalf("expected enabled capabilities differed from calculated enabled capabilities") + } + }) + } +} diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go index 3024c9b8166..487fc3a8ab8 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.go @@ -205,6 +205,7 @@ type HostedControlPlaneSpec struct { // This field is optional and once set cannot be changed. // +optional // +openshift:enable:FeatureGate=DisableClusterCapabilities + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="Capabilities is immutable. Changes might result in unpredictable and disruptive behavior." Capabilities *Capabilities `json:"capabilities,omitempty"` } diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go index 8ae8ca9343a..a97e558719f 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_types.go @@ -363,7 +363,6 @@ const ImageRegistryCapability OptionalCapability = OptionalCapability(configv1.C // capabilities allows disabling optional components at install time. // Once set, it cannot be changed. -// +kubebuilder:validation:XValidation:rule="!has(oldSelf.disabledCapabilities) || has(self.disabledCapabilities)", message="disabledCapabilities is required once set" type Capabilities struct { // disabledCapabilities when specified, sets the cluster version baselineCapabilitySet to None // and sets all additionalEnabledCapabilities BUT the ones supplied in disabledCapabilities. @@ -375,7 +374,6 @@ type Capabilities struct { // Once set, this field cannot be changed. // // +listType=atomic - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="disabledCapabilities is immutable" // +optional DisabledCapabilities []OptionalCapability `json:"disabledCapabilities,omitempty"` } @@ -388,7 +386,6 @@ type Capabilities struct { // +kubebuilder:validation:XValidation:rule=`self.platform.type == "Azure" ? self.services.exists(s, s.service == "Konnectivity" && s.servicePublishingStrategy.type == "Route" && s.servicePublishingStrategy.route.hostname != "") : true`,message="Azure platform requires Konnectivity Route service with a hostname to be defined" // +kubebuilder:validation:XValidation:rule=`self.platform.type == "Azure" ? self.services.exists(s, s.service == "Ignition" && s.servicePublishingStrategy.type == "Route" && s.servicePublishingStrategy.route.hostname != "") : true`,message="Azure platform requires Ignition Route service with a hostname to be defined" // +kubebuilder:validation:XValidation:rule=`has(self.issuerURL) || !has(self.serviceAccountSigningKey)`,message="If serviceAccountSigningKey is set, issuerURL must be set" - type HostedClusterSpec struct { // release specifies the desired OCP release payload for all the hosted cluster components. // This includes those components running management side like the Kube API Server and the CVO but also the operands which land in the hosted cluster data plane like the ingress controller, ovn agents, etc. @@ -665,6 +662,7 @@ type HostedClusterSpec struct { // This field is optional and once set cannot be changed. // +optional // +openshift:enable:FeatureGate=DisableClusterCapabilities + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="Capabilities is immutable. Changes might result in unpredictable and disruptive behavior." Capabilities *Capabilities `json:"capabilities,omitempty"` }