diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go index ed7848a1632a..45ca22550432 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config.go @@ -17,6 +17,8 @@ import ( openshiftcpv1 "github.com/openshift/api/openshiftcontrolplane/v1" corev1 "k8s.io/api/core/v1" + + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -34,6 +36,20 @@ func adaptConfigMap(cpContext component.WorkloadContext, cm *corev1.ConfigMap) e return fmt.Errorf("unable to decode existing openshift controller manager configuration: %w", err) } + // Fetch the existing ConfigMap from the cluster to preserve HCCO modifications + // (e.g., Controllers field when registry is disabled via managementState: Removed) + var existingControllers []string + existingCM := &corev1.ConfigMap{} + err = cpContext.Client.Get(cpContext.Context, client.ObjectKeyFromObject(cm), existingCM) + if err == nil && existingCM.Data != nil { + if existingConfigStr, exists := existingCM.Data[configKey]; exists && len(existingConfigStr) > 0 { + existingConfig := &openshiftcpv1.OpenShiftControllerManagerConfig{} + if err := util.DeserializeResource(existingConfigStr, existingConfig, api.Scheme); err == nil { + existingControllers = existingConfig.Controllers + } + } + } + observedConfig := &globalconfig.ObservedConfig{} if err := globalconfig.ReadObservedConfig(cpContext, cpContext.Client, observedConfig, cpContext.HCP.Namespace); err != nil { return fmt.Errorf("failed to read observed global config: %w", err) @@ -43,7 +59,7 @@ func adaptConfigMap(cpContext component.WorkloadContext, cm *corev1.ConfigMap) e if err != nil { return err } - adaptConfig(ocmConfig, cpContext.HCP.Spec.Configuration, cpContext.ReleaseImageProvider, observedConfig.Build, cpContext.HCP.Spec.Capabilities, featureGates) + adaptConfig(ocmConfig, cpContext.HCP.Spec.Configuration, cpContext.ReleaseImageProvider, observedConfig.Build, cpContext.HCP.Spec.Capabilities, featureGates, existingControllers) configStr, err := util.SerializeResource(ocmConfig, api.Scheme) if err != nil { return fmt.Errorf("failed to serialize openshift controller manager configuration: %w", err) @@ -52,13 +68,19 @@ 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, caps *hyperv1.Capabilities, featureGates []string) { +func adaptConfig(cfg *openshiftcpv1.OpenShiftControllerManagerConfig, configuration *hyperv1.ClusterConfiguration, releaseImageProvider imageprovider.ReleaseImageProvider, buildConfig *configv1.Build, caps *hyperv1.Capabilities, featureGates []string, existingControllers []string) { cfg.Build.ImageTemplateFormat.Format = releaseImageProvider.GetImage("docker-builder") cfg.Deployer.ImageTemplateFormat.Format = releaseImageProvider.GetImage("deployer") + // Preserve any existing Controllers configuration (e.g., modifications by HCCO) + // Only override if the ImageRegistry capability is explicitly disabled if !capabilities.IsImageRegistryCapabilityEnabled(caps) { cfg.Controllers = []string{"*", fmt.Sprintf("-%s", openshiftcpv1.OpenShiftServiceAccountPullSecretsController)} cfg.DockerPullSecret.InternalRegistryHostname = "" + } else if len(existingControllers) > 0 { + // Preserve existing Controllers field from the cluster to maintain any HCCO modifications + // (e.g., when registry is disabled via managementState: Removed instead of capability) + cfg.Controllers = existingControllers } if configuration != nil && configuration.Image != nil { 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 c79f6941a03f..2439ab07ac9f 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/ocm/config_test.go @@ -74,7 +74,7 @@ func TestReconcileOpenShiftControllerManagerConfig(t *testing.T) { t.Fatalf("unable to decode existing openshift controller manager configuration: %v", err) } - adaptConfig(config, hcp.Spec.Configuration, imageProvider, buildConfig, hcp.Spec.Capabilities, []string{"foo=true", "bar=false"}) + adaptConfig(config, hcp.Spec.Configuration, imageProvider, buildConfig, hcp.Spec.Capabilities, []string{"foo=true", "bar=false"}, nil) configStr, err := util.SerializeResource(config, api.Scheme) if err != nil { t.Fatalf("failed to serialize openshift controller manager configuration: %v", err) @@ -97,3 +97,89 @@ func TestReconcileOpenShiftControllerManagerConfig(t *testing.T) { t.Run("WithAllCapabilitiesEnabled", testFunc(nil)) t.Run("WithCapabilitiesEnabledAndDisabled", testFunc(caps)) } + +func TestAdaptConfig_PreservesExistingControllers(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{}, + }, + // ImageRegistry capability is enabled (default) + }, + } + images := map[string]string{ + "docker-builder": "quay.io/test/docker-builder", + "deployer": "quay.io/test/deployer", + } + imageProvider := imageprovider.NewFromImages(images) + + // Simulate HCCO setting the Controllers field to disable pull secrets controller + // (e.g., when registry is disabled via managementState: Removed) + existingControllersFromCluster := []string{"*", "-openshift.io/serviceaccount-pull-secrets"} + + config := &openshiftcpv1.OpenShiftControllerManagerConfig{} + config.ServingInfo = &v1.HTTPServingInfo{} + + // Adapt config with ImageRegistry capability enabled (not explicitly disabled) + adaptConfig(config, hcp.Spec.Configuration, imageProvider, &v1.Build{}, nil, []string{}, existingControllersFromCluster) + + // Verify that the Controllers field is preserved + if len(config.Controllers) != 2 { + t.Errorf("expected Controllers to be preserved with 2 entries, got %d: %v", len(config.Controllers), config.Controllers) + } + if config.Controllers[0] != "*" || config.Controllers[1] != "-openshift.io/serviceaccount-pull-secrets" { + t.Errorf("expected Controllers to be preserved as ['*', '-openshift.io/serviceaccount-pull-secrets'], got %v", config.Controllers) + } +} + +func TestAdaptConfig_DisabledImageRegistryCapability(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{}, + }, + Capabilities: &hyperv1.Capabilities{ + Disabled: []hyperv1.OptionalCapability{ + hyperv1.ImageRegistryCapability, + }, + }, + }, + } + images := map[string]string{ + "docker-builder": "quay.io/test/docker-builder", + "deployer": "quay.io/test/deployer", + } + imageProvider := imageprovider.NewFromImages(images) + + config := &openshiftcpv1.OpenShiftControllerManagerConfig{} + config.ServingInfo = &v1.HTTPServingInfo{} + + // Adapt config with ImageRegistry capability explicitly disabled + adaptConfig(config, hcp.Spec.Configuration, imageProvider, &v1.Build{}, hcp.Spec.Capabilities, []string{}, nil) + + // Verify that the Controllers field is set to disable pull secrets controller + if len(config.Controllers) != 2 { + t.Errorf("expected Controllers to be set with 2 entries, got %d: %v", len(config.Controllers), config.Controllers) + } + if config.Controllers[0] != "*" || config.Controllers[1] != "-openshift.io/serviceaccount-pull-secrets" { + t.Errorf("expected Controllers to be set as ['*', '-openshift.io/serviceaccount-pull-secrets'], got %v", config.Controllers) + } +}