diff --git a/bindata/rbac/rbac.yaml b/bindata/rbac/rbac.yaml index 7aed4290f..320db60b4 100644 --- a/bindata/rbac/rbac.yaml +++ b/bindata/rbac/rbac.yaml @@ -234,6 +234,7 @@ rules: - config.openshift.io resources: - imagedigestmirrorsets + - images - networks verbs: - get diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 574879be7..21d03fa6a 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -185,6 +185,7 @@ rules: - config.openshift.io resources: - imagedigestmirrorsets + - images - networks verbs: - get diff --git a/internal/controller/dataplane/openstackdataplanenodeset_controller.go b/internal/controller/dataplane/openstackdataplanenodeset_controller.go index ea529dc05..b0f77ebac 100644 --- a/internal/controller/dataplane/openstackdataplanenodeset_controller.go +++ b/internal/controller/dataplane/openstackdataplanenodeset_controller.go @@ -134,6 +134,7 @@ func (r *OpenStackDataPlaneNodeSetReconciler) GetLogger(ctx context.Context) log // RBAC for ImageContentSourcePolicy and MachineConfig // +kubebuilder:rbac:groups="operator.openshift.io",resources=imagecontentsourcepolicies,verbs=get;list;watch // +kubebuilder:rbac:groups="config.openshift.io",resources=imagedigestmirrorsets,verbs=get;list;watch +// +kubebuilder:rbac:groups="config.openshift.io",resources=images,verbs=get;list;watch // +kubebuilder:rbac:groups="machineconfiguration.openshift.io",resources=machineconfigs,verbs=get;list;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to diff --git a/internal/dataplane/inventory.go b/internal/dataplane/inventory.go index ade650c22..6d78a18de 100644 --- a/internal/dataplane/inventory.go +++ b/internal/dataplane/inventory.go @@ -136,12 +136,12 @@ func GenerateNodeSetInventory(ctx context.Context, helper *helper.Helper, // add the NodeSet name variable nodeSetGroup.Vars["edpm_nodeset_name"] = instance.Name - isDisconnected, err := util.IsDisconnectedOCP(ctx, helper) + hasMirrorRegistries, err := util.HasMirrorRegistries(ctx, helper) if err != nil { return "", err } - if isDisconnected { + if hasMirrorRegistries { registryConfig, err := util.GetMCRegistryConf(ctx, helper) if err != nil { // CRD not installed (non-OpenShift or no MCO) - log warning and continue. @@ -160,11 +160,11 @@ func GenerateNodeSetInventory(ctx context.Context, helper *helper.Helper, return "", fmt.Errorf("failed to get MachineConfig registry configuration: %w", err) } } else { - helper.GetLogger().Info("disconnected registry was identified via the ImageContentSourcePolicy. Using OCP registry.") + helper.GetLogger().Info("Mirror registries detected via IDMS/ICSP. Using OCP registry configuration.") - // Use OCP registries.conf for disconnected deployments + // Use OCP registries.conf for mirror registry deployments nodeSetGroup.Vars["edpm_podman_registries_conf"] = registryConfig - nodeSetGroup.Vars["edpm_podman_disconnected_ocp"] = isDisconnected + nodeSetGroup.Vars["edpm_podman_disconnected_ocp"] = hasMirrorRegistries } } diff --git a/internal/dataplane/util/image_registry.go b/internal/dataplane/util/image_registry.go index 066d9ca13..b1f24a109 100644 --- a/internal/dataplane/util/image_registry.go +++ b/internal/dataplane/util/image_registry.go @@ -7,12 +7,12 @@ import ( "fmt" "strings" - ocpidms "github.com/openshift/api/config/v1" + ocpconfigv1 "github.com/openshift/api/config/v1" mc "github.com/openshift/api/machineconfiguration/v1" ocpicsp "github.com/openshift/api/operator/v1alpha1" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" - "sigs.k8s.io/controller-runtime/pkg/client" - + corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -36,43 +36,29 @@ type machineConfigIgnition struct { } `json:"storage"` } -// IsDisconnectedOCP - Will retrieve a CR's related to disconnected OCP deployments. If the list is not -// empty, we can infer that the OCP cluster is a disconnected deployment. +// HasMirrorRegistries checks if OCP has IDMS/ICSP mirror registries configured. +// Note: The presence of IDMS/ICSP doesn't necessarily mean the cluster is disconnected. +// Mirror registries may be configured for other reasons (performance, policy, caching, etc.). // Returns false without error if the CRDs don't exist (non-OpenShift cluster). -func IsDisconnectedOCP(ctx context.Context, helper *helper.Helper) (bool, error) { - icspList := ocpicsp.ImageContentSourcePolicyList{} - idmsList := ocpidms.ImageDigestMirrorSetList{} - - listOpts := []client.ListOption{} - - var icspCount, idmsCount int - - err := helper.GetClient().List(ctx, &icspList, listOpts...) - if err != nil { - // If the CRD doesn't exist, this is not an OpenShift cluster or ICSP is not available - // This is not an error condition - just means we're not in a disconnected environment - if IsNoMatchError(err) { - helper.GetLogger().Info("ImageContentSourcePolicy CRD not available, assuming not a disconnected environment") - } else { +func HasMirrorRegistries(ctx context.Context, helper *helper.Helper) (bool, error) { + // Check IDMS first (current API), then fall back to ICSP (deprecated) + idmsList := &ocpconfigv1.ImageDigestMirrorSetList{} + if err := helper.GetClient().List(ctx, idmsList); err != nil { + if !IsNoMatchError(err) { return false, err } - } else { - icspCount = len(icspList.Items) + // CRD doesn't exist, continue to check ICSP + } else if len(idmsList.Items) > 0 { + return true, nil } - err = helper.GetClient().List(ctx, &idmsList, listOpts...) - if err != nil { - // If the CRD doesn't exist, this is not an OpenShift cluster or IDMS is not available - if IsNoMatchError(err) { - helper.GetLogger().Info("ImageDigestMirrorSet CRD not available, assuming not a disconnected environment") - } else { + icspList := &ocpicsp.ImageContentSourcePolicyList{} + if err := helper.GetClient().List(ctx, icspList); err != nil { + if !IsNoMatchError(err) { return false, err } - } else { - idmsCount = len(idmsList.Items) - } - - if icspCount != 0 || idmsCount != 0 { + // CRD doesn't exist, fall through to return false + } else if len(icspList.Items) > 0 { return true, nil } @@ -90,6 +76,40 @@ func IsNoMatchError(err error) bool { strings.Contains(errStr, "no kind is registered") } +// GetMirrorRegistryCACerts retrieves CA certificates from image.config.openshift.io/cluster. +// Returns nil without error if: +// - not on OpenShift (Image CRD doesn't exist) +// - no additional CA is configured +// - the referenced ConfigMap doesn't exist +func GetMirrorRegistryCACerts(ctx context.Context, helper *helper.Helper) (map[string]string, error) { + imageConfig := &ocpconfigv1.Image{} + if err := helper.GetClient().Get(ctx, types.NamespacedName{Name: "cluster"}, imageConfig); err != nil { + if IsNoMatchError(err) { + return nil, nil + } + return nil, fmt.Errorf("failed to get image.config.openshift.io/cluster: %w", err) + } + + if imageConfig.Spec.AdditionalTrustedCA.Name == "" { + return nil, nil + } + + caConfigMap := &corev1.ConfigMap{} + if err := helper.GetClient().Get(ctx, types.NamespacedName{ + Name: imageConfig.Spec.AdditionalTrustedCA.Name, + Namespace: "openshift-config", + }, caConfigMap); err != nil { + if k8s_errors.IsNotFound(err) { + // ConfigMap referenced but doesn't exist - log warning but don't fail + return nil, nil + } + return nil, fmt.Errorf("failed to get ConfigMap %s in openshift-config: %w", + imageConfig.Spec.AdditionalTrustedCA.Name, err) + } + + return caConfigMap.Data, nil +} + // GetMCRegistryConf - will unmarshal the MachineConfig ignition file the machineConfigIgnition object. // This is then parsed and the base64 decoded string is returned. func GetMCRegistryConf(ctx context.Context, helper *helper.Helper) (string, error) { diff --git a/internal/dataplane/util/image_registry_test.go b/internal/dataplane/util/image_registry_test.go index 8755011f0..62481bcc9 100644 --- a/internal/dataplane/util/image_registry_test.go +++ b/internal/dataplane/util/image_registry_test.go @@ -135,8 +135,8 @@ func TestIsNoMatchError(t *testing.T) { } } -// Test IsDisconnectedOCP scenarios -func TestIsDisconnectedOCP_WithICSP(t *testing.T) { +// Test HasMirrorRegistries scenarios +func TestHasMirrorRegistries_WithICSP(t *testing.T) { g := NewWithT(t) ctx := context.Background() @@ -157,12 +157,12 @@ func TestIsDisconnectedOCP_WithICSP(t *testing.T) { h := setupTestHelper(true, icsp) - disconnected, err := IsDisconnectedOCP(ctx, h) + hasMirrors, err := HasMirrorRegistries(ctx, h) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(disconnected).To(BeTrue(), "Should detect disconnected environment when ICSP exists") + g.Expect(hasMirrors).To(BeTrue(), "Should detect mirror registries when ICSP exists") } -func TestIsDisconnectedOCP_WithIDMS(t *testing.T) { +func TestHasMirrorRegistries_WithIDMS(t *testing.T) { g := NewWithT(t) ctx := context.Background() @@ -185,12 +185,12 @@ func TestIsDisconnectedOCP_WithIDMS(t *testing.T) { h := setupTestHelper(true, idms) - disconnected, err := IsDisconnectedOCP(ctx, h) + hasMirrors, err := HasMirrorRegistries(ctx, h) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(disconnected).To(BeTrue(), "Should detect disconnected environment when IDMS exists") + g.Expect(hasMirrors).To(BeTrue(), "Should detect mirror registries when IDMS exists") } -func TestIsDisconnectedOCP_WithBothICSPAndIDMS(t *testing.T) { +func TestHasMirrorRegistries_WithBothICSPAndIDMS(t *testing.T) { g := NewWithT(t) ctx := context.Background() @@ -208,33 +208,33 @@ func TestIsDisconnectedOCP_WithBothICSPAndIDMS(t *testing.T) { h := setupTestHelper(true, icsp, idms) - disconnected, err := IsDisconnectedOCP(ctx, h) + hasMirrors, err := HasMirrorRegistries(ctx, h) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(disconnected).To(BeTrue(), "Should detect disconnected environment when both ICSP and IDMS exist") + g.Expect(hasMirrors).To(BeTrue(), "Should detect mirror registries when both ICSP and IDMS exist") } -func TestIsDisconnectedOCP_EmptyLists(t *testing.T) { +func TestHasMirrorRegistries_EmptyLists(t *testing.T) { g := NewWithT(t) ctx := context.Background() // No ICSP or IDMS resources, but CRDs are registered h := setupTestHelper(true) - disconnected, err := IsDisconnectedOCP(ctx, h) + hasMirrors, err := HasMirrorRegistries(ctx, h) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(disconnected).To(BeFalse(), "Should not detect disconnected environment when lists are empty") + g.Expect(hasMirrors).To(BeFalse(), "Should not detect mirror registries when lists are empty") } -func TestIsDisconnectedOCP_CRDsNotInstalled(t *testing.T) { +func TestHasMirrorRegistries_CRDsNotInstalled(t *testing.T) { g := NewWithT(t) ctx := context.Background() // Don't register OpenShift CRDs - simulates non-OpenShift cluster h := setupTestHelper(false) - disconnected, err := IsDisconnectedOCP(ctx, h) + hasMirrors, err := HasMirrorRegistries(ctx, h) g.Expect(err).ToNot(HaveOccurred(), "Should not return error when CRDs don't exist") - g.Expect(disconnected).To(BeFalse(), "Should return false when CRDs don't exist (graceful degradation)") + g.Expect(hasMirrors).To(BeFalse(), "Should return false when CRDs don't exist (graceful degradation)") } // Test GetMCRegistryConf scenarios @@ -290,7 +290,7 @@ func TestGetMCRegistryConf_MachineConfigNotFound(t *testing.T) { // MachineConfig CRD is registered but no resource exists // This simulates the case where MCO is installed but the specific // registry MachineConfig doesn't exist - this should be treated as an error, - // not a warning, because if MCO is present and disconnected env is detected, + // not a warning, because if MCO is present and mirror registries are detected, // the registry config should exist. h := setupTestHelper(true) @@ -412,11 +412,11 @@ func TestGetMCRegistryConf_InvalidBase64Content(t *testing.T) { } // Test real-world scenarios -func TestDisconnectedEnvironment_FullScenario(t *testing.T) { +func TestMirrorRegistryEnvironment_FullScenario(t *testing.T) { g := NewWithT(t) ctx := context.Background() - // Simulate a full disconnected environment with IDMS and MachineConfig + // Simulate a mirror registry environment with IDMS and MachineConfig expectedConfig := `[[registry]] prefix = "" location = "registry.redhat.io/rhosp-dev-preview" @@ -467,10 +467,10 @@ func TestDisconnectedEnvironment_FullScenario(t *testing.T) { h := setupTestHelper(true, idms, machineConfig) - // Check for disconnected environment - disconnected, err := IsDisconnectedOCP(ctx, h) + // Check for mirror registries + hasMirrors, err := HasMirrorRegistries(ctx, h) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(disconnected).To(BeTrue()) + g.Expect(hasMirrors).To(BeTrue()) // Get the registry configuration config, err := GetMCRegistryConf(ctx, h) @@ -485,10 +485,10 @@ func TestNonOpenShiftCluster_GracefulDegradation(t *testing.T) { // Simulate a non-OpenShift Kubernetes cluster (no OpenShift CRDs registered) h := setupTestHelper(false) - // IsDisconnectedOCP should return false without error - disconnected, err := IsDisconnectedOCP(ctx, h) + // HasMirrorRegistries should return false without error + hasMirrors, err := HasMirrorRegistries(ctx, h) g.Expect(err).ToNot(HaveOccurred(), "Should gracefully handle missing CRDs") - g.Expect(disconnected).To(BeFalse()) + g.Expect(hasMirrors).To(BeFalse()) // GetMCRegistryConf should return an error that can be identified as "CRD not found" config, err := GetMCRegistryConf(ctx, h) @@ -496,3 +496,56 @@ func TestNonOpenShiftCluster_GracefulDegradation(t *testing.T) { g.Expect(IsNoMatchError(err)).To(BeTrue(), "Error should indicate CRD is not installed") g.Expect(config).To(BeEmpty()) } + +func TestGetMirrorRegistryCACerts(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + // Test successful retrieval of CA certificates + imageConfig := &ocpidms.Image{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: ocpidms.ImageSpec{AdditionalTrustedCA: ocpidms.ConfigMapNameReference{Name: "registry-cas"}}, + } + caConfigMap := &k8s_corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "registry-cas", Namespace: "openshift-config"}, + Data: map[string]string{"mirror.example.com": "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----"}, + } + h := setupTestHelper(true, imageConfig, caConfigMap) + caCerts, err := GetMirrorRegistryCACerts(ctx, h) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(caCerts).To(HaveLen(1)) + g.Expect(caCerts).To(HaveKey("mirror.example.com")) + + // Test with no additionalTrustedCA configured - should return nil + imageConfigNoCA := &ocpidms.Image{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: ocpidms.ImageSpec{}, + } + h = setupTestHelper(true, imageConfigNoCA) + caCerts, err = GetMirrorRegistryCACerts(ctx, h) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(caCerts).To(BeNil()) + + // Test non-OpenShift cluster (CRDs not registered) - should return nil without error + h = setupTestHelper(false) + caCerts, err = GetMirrorRegistryCACerts(ctx, h) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(caCerts).To(BeNil()) +} + +func TestGetMirrorRegistryCACerts_ConfigMapNotFound(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + + // Test when additionalTrustedCA references a ConfigMap that doesn't exist + // This should return nil, nil (graceful handling) instead of an error + imageConfig := &ocpidms.Image{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: ocpidms.ImageSpec{AdditionalTrustedCA: ocpidms.ConfigMapNameReference{Name: "non-existent-ca"}}, + } + h := setupTestHelper(true, imageConfig) + + caCerts, err := GetMirrorRegistryCACerts(ctx, h) + g.Expect(err).ToNot(HaveOccurred(), "Should not return error when ConfigMap is not found") + g.Expect(caCerts).To(BeNil(), "Should return nil when ConfigMap is not found") +} diff --git a/internal/openstack/ca.go b/internal/openstack/ca.go index e7e469431..ad56260eb 100644 --- a/internal/openstack/ca.go +++ b/internal/openstack/ca.go @@ -10,6 +10,7 @@ import ( "math" "os" "slices" + "sort" "strings" "time" @@ -21,6 +22,7 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" "github.com/openstack-k8s-operators/lib-common/modules/common/util" + dputil "github.com/openstack-k8s-operators/openstack-operator/internal/dataplane/util" k8s_errors "k8s.io/apimachinery/pkg/api/errors" corev1 "github.com/openstack-k8s-operators/openstack-operator/api/core/v1beta1" @@ -445,6 +447,31 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h } } + // Add mirror registry CA certificates from image.config.openshift.io/cluster if configured. + // This is needed for dataplane nodes to trust mirror registries using private/self-signed CAs. + mirrorRegistryCACerts, err := dputil.GetMirrorRegistryCACerts(ctx, helper) + if err != nil { + // Log but don't fail - mirror registry CAs are optional + Log.Info("Could not retrieve mirror registry CA certificates", "error", err.Error()) + } else if len(mirrorRegistryCACerts) > 0 { + Log.Info("Adding mirror registry CA certificates to bundle", "count", len(mirrorRegistryCACerts)) + // Sort registry hosts for deterministic ordering to avoid unnecessary secret updates + registryHosts := make([]string, 0, len(mirrorRegistryCACerts)) + for host := range mirrorRegistryCACerts { + registryHosts = append(registryHosts, host) + } + sort.Strings(registryHosts) + + for _, registryHost := range registryHosts { + caCert := mirrorRegistryCACerts[registryHost] + if parseErr := bundle.getCertsFromPEM([]byte(caCert)); parseErr != nil { + Log.Info("Failed to parse mirror registry CA certificate", "registry", registryHost, "error", parseErr.Error()) + // Continue with other certs even if one fails + continue + } + } + } + instance.Status.Conditions.MarkTrue(corev1.OpenStackControlPlaneCAReadyCondition, corev1.OpenStackControlPlaneCAReadyMessage) // get CA bundle from operator image. Downstream and upstream build use a different