diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go index c6c8334b5e44..39a1a25aba29 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go @@ -47,6 +47,7 @@ import ( hyperapi "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/azureutil" "github.com/openshift/hypershift/support/capabilities" + "github.com/openshift/hypershift/support/certs" "github.com/openshift/hypershift/support/config" "github.com/openshift/hypershift/support/globalconfig" "github.com/openshift/hypershift/support/releaseinfo" @@ -1071,6 +1072,13 @@ func (r *reconciler) reconcileConfig(ctx context.Context, hcp *hyperv1.HostedCon proxy := globalconfig.ProxyConfig() if _, err := r.CreateOrUpdate(ctx, r.client, proxy, func() error { globalconfig.ReconcileInClusterProxyConfig(proxy, hcp.Spec.Configuration) + // When no proxy.trustedCA is set but additionalTrustBundle is, point the + // Proxy object at openshift-config/user-ca-bundle (already written by + // reconcileUserCertCABundle) so CNO merges it into + // openshift-config-managed/trusted-ca-bundle. + if proxy.Spec.TrustedCA.Name == "" && hcp.Spec.AdditionalTrustBundle != nil { + proxy.Spec.TrustedCA.Name = manifests.UserCABundle().Name + } return nil }); err != nil { errs = append(errs, fmt.Errorf("failed to reconcile proxy config: %w", err)) @@ -1131,19 +1139,23 @@ func (r *reconciler) reconcileProxyTrustedCAConfigMap(ctx context.Context, hcp * currentConfigMapRef := proxy.Spec.TrustedCA.Name if currentConfigMapRef != "" && currentConfigMapRef != configMapRef { - // cleanup old configMaps - cm := &corev1.ConfigMap{} - cm.Name = currentConfigMapRef - - // log and ignore deletion errors, should not disrupt normal workflow - cm.Namespace = hcp.Namespace - if err := r.cpClient.Delete(ctx, cm); err != nil { - log.Error(err, "failed to delete configmap", "name", cm.Name, "namespace", cm.Namespace) - } + // Do not clean up user-ca-bundle when it is being managed via additionalTrustBundle; + // reconcileConfig sets proxy.Spec.TrustedCA.Name to user-ca-bundle in that case. + if !(currentConfigMapRef == manifests.UserCABundle().Name && hcp.Spec.AdditionalTrustBundle != nil) { + // cleanup old configMaps + cm := &corev1.ConfigMap{} + cm.Name = currentConfigMapRef + + // log and ignore deletion errors, should not disrupt normal workflow + cm.Namespace = hcp.Namespace + if err := r.cpClient.Delete(ctx, cm); err != nil { + log.Error(err, "failed to delete configmap", "name", cm.Name, "namespace", cm.Namespace) + } - cm.Namespace = manifests.ProxyTrustedCAConfigMap("").Namespace - if err := r.client.Delete(ctx, cm); err != nil { - log.Error(err, "failed to delete configmap in hosted cluster", "name", cm.Name, "namespace", cm.Namespace) + cm.Namespace = manifests.ProxyTrustedCAConfigMap("").Namespace + if err := r.client.Delete(ctx, cm); err != nil { + log.Error(err, "failed to delete configmap in hosted cluster", "name", cm.Name, "namespace", cm.Namespace) + } } } @@ -1158,7 +1170,26 @@ func (r *reconciler) reconcileProxyTrustedCAConfigMap(ctx context.Context, hcp * destCM := manifests.ProxyTrustedCAConfigMap(sourceCM.Name) if _, err := r.CreateOrUpdate(ctx, r.client, destCM, func() error { - destCM.Data = sourceCM.Data + destCM.Data = make(map[string]string) + for k, v := range sourceCM.Data { + destCM.Data[k] = v + } + // Also merge additionalTrustBundle so that CNO writes both CAs into + // openshift-config-managed/trusted-ca-bundle, benefiting all consumers + // (including CVO's update service TLS verification). + if hcp.Spec.AdditionalTrustBundle != nil { + atbCM := &corev1.ConfigMap{} + if err := r.cpClient.Get(ctx, client.ObjectKey{Namespace: hcp.Namespace, Name: hcp.Spec.AdditionalTrustBundle.Name}, atbCM); err != nil { + return fmt.Errorf("failed to get additionalTrustBundle configmap %s/%s: %w", hcp.Namespace, hcp.Spec.AdditionalTrustBundle.Name, err) + } + if v := atbCM.Data[certs.UserCABundleMapKey]; v != "" { + existing := destCM.Data[certs.UserCABundleMapKey] + if existing != "" && !strings.HasSuffix(existing, "\n") { + existing += "\n" + } + destCM.Data[certs.UserCABundleMapKey] = existing + v + } + } return nil }); err != nil { return fmt.Errorf("failed to reconcile referenced TrustedCA config map %s/%s: %w", destCM.Namespace, destCM.Name, err) diff --git a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go index 67225afb258f..ff1d12f0adc3 100644 --- a/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go +++ b/control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources_test.go @@ -637,6 +637,238 @@ func TestReconcileUserCertCABundle(t *testing.T) { } } +func TestReconcileProxyTrustedCAConfigMap(t *testing.T) { + const ( + testNamespace = "master-cluster1" + testHCPName = "cluster1" + proxyCAName = "my-proxy-ca" + ) + + proxyCAData := "-----BEGIN CERTIFICATE-----\nMIIBfakeProxyCACert==\n-----END CERTIFICATE-----\n" + atbCAData := "-----BEGIN CERTIFICATE-----\nMIIBfakeAdditionalTrustBundleCert==\n-----END CERTIFICATE-----\n" + + proxyCAConfigMap := func() *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: proxyCAName, Namespace: testNamespace}, + Data: map[string]string{"ca-bundle.crt": proxyCAData}, + } + } + additionalTrustBundleCM := func() *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "user-ca-bundle", Namespace: testNamespace}, + Data: map[string]string{"ca-bundle.crt": atbCAData}, + } + } + guestProxyObj := func() *configv1.Proxy { + return globalconfig.ProxyConfig() + } + + tests := map[string]struct { + inputHCP *hyperv1.HostedControlPlane + mgmtObjects []client.Object + guestObjects []client.Object + wantErr bool + wantDestCMExists bool + wantDestCMData string + }{ + "When neither proxy.trustedCA nor additionalTrustBundle is set, it should not create a dest ConfigMap": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + }, + mgmtObjects: []client.Object{}, + guestObjects: []client.Object{guestProxyObj()}, + wantErr: false, + wantDestCMExists: false, + }, + "When only proxy.trustedCA is set, it should create dest ConfigMap with proxy CA data only": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Spec: hyperv1.HostedControlPlaneSpec{ + Configuration: &hyperv1.ClusterConfiguration{ + Proxy: &configv1.ProxySpec{TrustedCA: configv1.ConfigMapNameReference{Name: proxyCAName}}, + }, + }, + }, + mgmtObjects: []client.Object{proxyCAConfigMap()}, + guestObjects: []client.Object{guestProxyObj()}, + wantErr: false, + wantDestCMExists: true, + wantDestCMData: proxyCAData, + }, + "When both proxy.trustedCA and additionalTrustBundle are set, it should merge both CA bundles": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Spec: hyperv1.HostedControlPlaneSpec{ + AdditionalTrustBundle: &corev1.LocalObjectReference{Name: "user-ca-bundle"}, + Configuration: &hyperv1.ClusterConfiguration{ + Proxy: &configv1.ProxySpec{TrustedCA: configv1.ConfigMapNameReference{Name: proxyCAName}}, + }, + }, + }, + mgmtObjects: []client.Object{proxyCAConfigMap(), additionalTrustBundleCM()}, + guestObjects: []client.Object{guestProxyObj()}, + wantErr: false, + wantDestCMExists: true, + wantDestCMData: proxyCAData + atbCAData, + }, + "When only additionalTrustBundle is set, it should return early without creating dest ConfigMap": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Spec: hyperv1.HostedControlPlaneSpec{ + AdditionalTrustBundle: &corev1.LocalObjectReference{Name: "user-ca-bundle"}, + }, + }, + mgmtObjects: []client.Object{additionalTrustBundleCM()}, + guestObjects: []client.Object{guestProxyObj()}, + wantErr: false, + wantDestCMExists: false, + }, + "When additionalTrustBundle ConfigMap does not exist on management cluster, it should return error": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Spec: hyperv1.HostedControlPlaneSpec{ + AdditionalTrustBundle: &corev1.LocalObjectReference{Name: "user-ca-bundle"}, + Configuration: &hyperv1.ClusterConfiguration{ + Proxy: &configv1.ProxySpec{TrustedCA: configv1.ConfigMapNameReference{Name: proxyCAName}}, + }, + }, + }, + mgmtObjects: []client.Object{proxyCAConfigMap()}, + guestObjects: []client.Object{guestProxyObj()}, + wantErr: true, + wantDestCMExists: false, + }, + "When proxy data has no trailing newline and ATB is set, it should insert newline separator": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Spec: hyperv1.HostedControlPlaneSpec{ + AdditionalTrustBundle: &corev1.LocalObjectReference{Name: "user-ca-bundle"}, + Configuration: &hyperv1.ClusterConfiguration{ + Proxy: &configv1.ProxySpec{TrustedCA: configv1.ConfigMapNameReference{Name: proxyCAName}}, + }, + }, + }, + mgmtObjects: []client.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: proxyCAName, Namespace: testNamespace}, + Data: map[string]string{"ca-bundle.crt": "-----BEGIN CERTIFICATE-----\nnotrailingnewline\n-----END CERTIFICATE-----"}, + }, + additionalTrustBundleCM(), + }, + guestObjects: []client.Object{guestProxyObj()}, + wantErr: false, + wantDestCMExists: true, + wantDestCMData: "-----BEGIN CERTIFICATE-----\nnotrailingnewline\n-----END CERTIFICATE-----\n" + atbCAData, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + r := &reconciler{ + client: fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(tc.guestObjects...).Build(), + CreateOrUpdateProvider: &simpleCreateOrUpdater{}, + cpClient: fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(tc.mgmtObjects...).Build(), + hcpName: testHCPName, + hcpNamespace: testNamespace, + } + err := r.reconcileProxyTrustedCAConfigMap(t.Context(), tc.inputHCP) + if tc.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).ToNot(HaveOccurred()) + } + + if tc.wantDestCMExists { + destCM := manifests.ProxyTrustedCAConfigMap(proxyCAName) + g.Expect(r.client.Get(t.Context(), client.ObjectKeyFromObject(destCM), destCM)).To(Succeed()) + g.Expect(destCM.Data["ca-bundle.crt"]).To(Equal(tc.wantDestCMData)) + } else { + destCM := manifests.ProxyTrustedCAConfigMap(proxyCAName) + getErr := r.client.Get(t.Context(), client.ObjectKeyFromObject(destCM), destCM) + g.Expect(apierrors.IsNotFound(getErr)).To(BeTrue()) + } + }) + } +} + +func TestReconcileConfigProxyTrustedCA(t *testing.T) { + const ( + testNamespace = "master-cluster1" + testHCPName = "cluster1" + proxyCAName = "my-proxy-ca" + ) + + tests := map[string]struct { + inputHCP *hyperv1.HostedControlPlane + guestObjects []client.Object + wantTrustedCAName string + }{ + "When ATB is set and no proxy config, it should set trustedCA.name to user-ca-bundle": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Spec: hyperv1.HostedControlPlaneSpec{ + AdditionalTrustBundle: &corev1.LocalObjectReference{Name: "user-ca-bundle"}, + }, + Status: hyperv1.HostedControlPlaneStatus{ + ControlPlaneEndpoint: hyperv1.APIEndpoint{Host: "api.example.com"}, + }, + }, + guestObjects: []client.Object{globalconfig.ProxyConfig()}, + wantTrustedCAName: manifests.UserCABundle().Name, + }, + "When both ATB and proxy trustedCA are set, it should preserve the proxy trustedCA name": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Spec: hyperv1.HostedControlPlaneSpec{ + AdditionalTrustBundle: &corev1.LocalObjectReference{Name: "user-ca-bundle"}, + Configuration: &hyperv1.ClusterConfiguration{ + Proxy: &configv1.ProxySpec{TrustedCA: configv1.ConfigMapNameReference{Name: proxyCAName}}, + }, + }, + Status: hyperv1.HostedControlPlaneStatus{ + ControlPlaneEndpoint: hyperv1.APIEndpoint{Host: "api.example.com"}, + }, + }, + guestObjects: []client.Object{globalconfig.ProxyConfig()}, + wantTrustedCAName: proxyCAName, + }, + "When neither ATB nor proxy trustedCA is set, it should leave trustedCA.name empty": { + inputHCP: &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: testHCPName, Namespace: testNamespace}, + Status: hyperv1.HostedControlPlaneStatus{ + ControlPlaneEndpoint: hyperv1.APIEndpoint{Host: "api.example.com"}, + }, + }, + guestObjects: []client.Object{globalconfig.ProxyConfig()}, + wantTrustedCAName: "", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + r := &reconciler{ + client: fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(tc.guestObjects...).Build(), + CreateOrUpdateProvider: &simpleCreateOrUpdater{}, + cpClient: fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(tc.inputHCP).Build(), + hcpName: testHCPName, + hcpNamespace: testNamespace, + } + // reconcileConfig will produce errors for other resources (infra, dns, etc.) + // that aren't seeded, but we only care about the Proxy mutation. + _ = r.reconcileConfig(t.Context(), tc.inputHCP) + + proxy := globalconfig.ProxyConfig() + err := r.client.Get(t.Context(), client.ObjectKeyFromObject(proxy), proxy) + g.Expect(err).To(BeNil()) + g.Expect(proxy.Spec.TrustedCA.Name).To(Equal(tc.wantTrustedCAName)) + }) + } +} + var _ manifestReconciler = manifestAndReconcile[*rbacv1.ClusterRole]{} func TestDestroyCloudResources(t *testing.T) {