Skip to content

Commit 0c3f6b7

Browse files
committed
Add mirror registry CA certificate support for dataplane nodes
Mirror registries configured via IDMS/ICSP require TLS verification by default. For registries using private or self-signed CA certificates, dataplane nodes need access to these CA certificates to verify TLS connections when pulling container images. Note: The presence of IDMS/ICSP doesn't necessarily mean the cluster is disconnected. Mirror registries may be configured for other reasons (performance, policy, etc.). This change retrieves CA certificates from the ConfigMap referenced by image.config.openshift.io/cluster's additionalTrustedCA field (located in openshift-config namespace) and adds them to the combined-ca-bundle secret. The existing bootstrap service copies this bundle to EDPM nodes and updates the system trust store, so no edpm-ansible changes are required. Assisted-By: Claude Signed-off-by: rabi <ramishra@redhat.com>
1 parent cf82a75 commit 0c3f6b7

7 files changed

Lines changed: 167 additions & 67 deletions

File tree

bindata/rbac/rbac.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ rules:
234234
- config.openshift.io
235235
resources:
236236
- imagedigestmirrorsets
237+
- images
237238
- networks
238239
verbs:
239240
- get

config/rbac/role.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ rules:
185185
- config.openshift.io
186186
resources:
187187
- imagedigestmirrorsets
188+
- images
188189
- networks
189190
verbs:
190191
- get

internal/controller/dataplane/openstackdataplanenodeset_controller.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ func (r *OpenStackDataPlaneNodeSetReconciler) GetLogger(ctx context.Context) log
134134
// RBAC for ImageContentSourcePolicy and MachineConfig
135135
// +kubebuilder:rbac:groups="operator.openshift.io",resources=imagecontentsourcepolicies,verbs=get;list;watch
136136
// +kubebuilder:rbac:groups="config.openshift.io",resources=imagedigestmirrorsets,verbs=get;list;watch
137+
// +kubebuilder:rbac:groups="config.openshift.io",resources=images,verbs=get;list;watch
137138
// +kubebuilder:rbac:groups="machineconfiguration.openshift.io",resources=machineconfigs,verbs=get;list;watch
138139

139140
// Reconcile is part of the main kubernetes reconciliation loop which aims to

internal/dataplane/inventory.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,12 @@ func GenerateNodeSetInventory(ctx context.Context, helper *helper.Helper,
136136
// add the NodeSet name variable
137137
nodeSetGroup.Vars["edpm_nodeset_name"] = instance.Name
138138

139-
isDisconnected, err := util.IsDisconnectedOCP(ctx, helper)
139+
hasMirrorRegistries, err := util.HasMirrorRegistries(ctx, helper)
140140
if err != nil {
141141
return "", err
142142
}
143143

144-
if isDisconnected {
144+
if hasMirrorRegistries {
145145
registryConfig, err := util.GetMCRegistryConf(ctx, helper)
146146
if err != nil {
147147
// CRD not installed (non-OpenShift or no MCO) - log warning and continue.
@@ -160,11 +160,11 @@ func GenerateNodeSetInventory(ctx context.Context, helper *helper.Helper,
160160
return "", fmt.Errorf("failed to get MachineConfig registry configuration: %w", err)
161161
}
162162
} else {
163-
helper.GetLogger().Info("disconnected registry was identified via the ImageContentSourcePolicy. Using OCP registry.")
163+
helper.GetLogger().Info("Mirror registries detected via IDMS/ICSP. Using OCP registry configuration.")
164164

165-
// Use OCP registries.conf for disconnected deployments
165+
// Use OCP registries.conf for mirror registry deployments
166166
nodeSetGroup.Vars["edpm_podman_registries_conf"] = registryConfig
167-
nodeSetGroup.Vars["edpm_podman_disconnected_ocp"] = isDisconnected
167+
nodeSetGroup.Vars["edpm_podman_disconnected_ocp"] = hasMirrorRegistries
168168
}
169169
}
170170

internal/dataplane/util/image_registry.go

Lines changed: 52 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import (
77
"fmt"
88
"strings"
99

10-
ocpidms "github.com/openshift/api/config/v1"
10+
ocpconfigv1 "github.com/openshift/api/config/v1"
1111
mc "github.com/openshift/api/machineconfiguration/v1"
1212
ocpicsp "github.com/openshift/api/operator/v1alpha1"
1313
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
14-
"sigs.k8s.io/controller-runtime/pkg/client"
15-
14+
corev1 "k8s.io/api/core/v1"
15+
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
1616
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1717
"k8s.io/apimachinery/pkg/types"
1818
)
@@ -36,43 +36,29 @@ type machineConfigIgnition struct {
3636
} `json:"storage"`
3737
}
3838

39-
// IsDisconnectedOCP - Will retrieve a CR's related to disconnected OCP deployments. If the list is not
40-
// empty, we can infer that the OCP cluster is a disconnected deployment.
39+
// HasMirrorRegistries checks if OCP has IDMS/ICSP mirror registries configured.
40+
// Note: The presence of IDMS/ICSP doesn't necessarily mean the cluster is disconnected.
41+
// Mirror registries may be configured for other reasons (performance, policy, caching, etc.).
4142
// Returns false without error if the CRDs don't exist (non-OpenShift cluster).
42-
func IsDisconnectedOCP(ctx context.Context, helper *helper.Helper) (bool, error) {
43-
icspList := ocpicsp.ImageContentSourcePolicyList{}
44-
idmsList := ocpidms.ImageDigestMirrorSetList{}
45-
46-
listOpts := []client.ListOption{}
47-
48-
var icspCount, idmsCount int
49-
50-
err := helper.GetClient().List(ctx, &icspList, listOpts...)
51-
if err != nil {
52-
// If the CRD doesn't exist, this is not an OpenShift cluster or ICSP is not available
53-
// This is not an error condition - just means we're not in a disconnected environment
54-
if IsNoMatchError(err) {
55-
helper.GetLogger().Info("ImageContentSourcePolicy CRD not available, assuming not a disconnected environment")
56-
} else {
43+
func HasMirrorRegistries(ctx context.Context, helper *helper.Helper) (bool, error) {
44+
// Check IDMS first (current API), then fall back to ICSP (deprecated)
45+
idmsList := &ocpconfigv1.ImageDigestMirrorSetList{}
46+
if err := helper.GetClient().List(ctx, idmsList); err != nil {
47+
if !IsNoMatchError(err) {
5748
return false, err
5849
}
59-
} else {
60-
icspCount = len(icspList.Items)
50+
// CRD doesn't exist, continue to check ICSP
51+
} else if len(idmsList.Items) > 0 {
52+
return true, nil
6153
}
6254

63-
err = helper.GetClient().List(ctx, &idmsList, listOpts...)
64-
if err != nil {
65-
// If the CRD doesn't exist, this is not an OpenShift cluster or IDMS is not available
66-
if IsNoMatchError(err) {
67-
helper.GetLogger().Info("ImageDigestMirrorSet CRD not available, assuming not a disconnected environment")
68-
} else {
55+
icspList := &ocpicsp.ImageContentSourcePolicyList{}
56+
if err := helper.GetClient().List(ctx, icspList); err != nil {
57+
if !IsNoMatchError(err) {
6958
return false, err
7059
}
71-
} else {
72-
idmsCount = len(idmsList.Items)
73-
}
74-
75-
if icspCount != 0 || idmsCount != 0 {
60+
// CRD doesn't exist, fall through to return false
61+
} else if len(icspList.Items) > 0 {
7662
return true, nil
7763
}
7864

@@ -84,8 +70,6 @@ func IsNoMatchError(err error) bool {
8470
errStr := err.Error()
8571
// Check for "no matches for kind" type errors which indicate the CRD doesn't exist.
8672
// Also check for "no kind is registered" which occurs when the type isn't in the scheme.
87-
// This is specifically needed for functional tests where the fake client returns a different
88-
// error type than a real Kubernetes API server when CRDs are not installed.
8973
return strings.Contains(errStr, "no matches for kind") ||
9074
strings.Contains(errStr, "no kind is registered")
9175
}
@@ -166,3 +150,36 @@ func getMachineConfig(ctx context.Context, helper *helper.Helper) (mc.MachineCon
166150

167151
return masterMachineConfig, nil
168152
}
153+
154+
// GetMirrorRegistryCACerts retrieves CA certificates from image.config.openshift.io/cluster.
155+
// Returns nil without error if:
156+
// - not on OpenShift (Image CRD doesn't exist)
157+
// - no additional CA is configured
158+
// - the referenced ConfigMap doesn't exist
159+
func GetMirrorRegistryCACerts(ctx context.Context, helper *helper.Helper) (map[string]string, error) {
160+
imageConfig := &ocpconfigv1.Image{}
161+
if err := helper.GetClient().Get(ctx, types.NamespacedName{Name: "cluster"}, imageConfig); err != nil {
162+
if IsNoMatchError(err) {
163+
return nil, nil
164+
}
165+
return nil, fmt.Errorf("failed to get image.config.openshift.io/cluster: %w", err)
166+
}
167+
168+
if imageConfig.Spec.AdditionalTrustedCA.Name == "" {
169+
return nil, nil
170+
}
171+
172+
caConfigMap := &corev1.ConfigMap{}
173+
if err := helper.GetClient().Get(ctx, types.NamespacedName{
174+
Name: imageConfig.Spec.AdditionalTrustedCA.Name,
175+
Namespace: "openshift-config",
176+
}, caConfigMap); err != nil {
177+
if k8s_errors.IsNotFound(err) {
178+
return nil, nil
179+
}
180+
return nil, fmt.Errorf("failed to get ConfigMap %s in openshift-config: %w",
181+
imageConfig.Spec.AdditionalTrustedCA.Name, err)
182+
}
183+
184+
return caConfigMap.Data, nil
185+
}

internal/dataplane/util/image_registry_test.go

Lines changed: 73 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ func TestIsNoMatchError(t *testing.T) {
135135
}
136136
}
137137

138-
// Test IsDisconnectedOCP scenarios
139-
func TestIsDisconnectedOCP_WithICSP(t *testing.T) {
138+
// Test HasMirrorRegistries scenarios
139+
func TestHasMirrorRegistries_WithICSP(t *testing.T) {
140140
g := NewWithT(t)
141141
ctx := context.Background()
142142

@@ -157,12 +157,12 @@ func TestIsDisconnectedOCP_WithICSP(t *testing.T) {
157157

158158
h := setupTestHelper(true, icsp)
159159

160-
disconnected, err := IsDisconnectedOCP(ctx, h)
160+
hasMirrors, err := HasMirrorRegistries(ctx, h)
161161
g.Expect(err).ToNot(HaveOccurred())
162-
g.Expect(disconnected).To(BeTrue(), "Should detect disconnected environment when ICSP exists")
162+
g.Expect(hasMirrors).To(BeTrue(), "Should detect mirror registries when ICSP exists")
163163
}
164164

165-
func TestIsDisconnectedOCP_WithIDMS(t *testing.T) {
165+
func TestHasMirrorRegistries_WithIDMS(t *testing.T) {
166166
g := NewWithT(t)
167167
ctx := context.Background()
168168

@@ -185,12 +185,12 @@ func TestIsDisconnectedOCP_WithIDMS(t *testing.T) {
185185

186186
h := setupTestHelper(true, idms)
187187

188-
disconnected, err := IsDisconnectedOCP(ctx, h)
188+
hasMirrors, err := HasMirrorRegistries(ctx, h)
189189
g.Expect(err).ToNot(HaveOccurred())
190-
g.Expect(disconnected).To(BeTrue(), "Should detect disconnected environment when IDMS exists")
190+
g.Expect(hasMirrors).To(BeTrue(), "Should detect mirror registries when IDMS exists")
191191
}
192192

193-
func TestIsDisconnectedOCP_WithBothICSPAndIDMS(t *testing.T) {
193+
func TestHasMirrorRegistries_WithBothICSPAndIDMS(t *testing.T) {
194194
g := NewWithT(t)
195195
ctx := context.Background()
196196

@@ -208,33 +208,33 @@ func TestIsDisconnectedOCP_WithBothICSPAndIDMS(t *testing.T) {
208208

209209
h := setupTestHelper(true, icsp, idms)
210210

211-
disconnected, err := IsDisconnectedOCP(ctx, h)
211+
hasMirrors, err := HasMirrorRegistries(ctx, h)
212212
g.Expect(err).ToNot(HaveOccurred())
213-
g.Expect(disconnected).To(BeTrue(), "Should detect disconnected environment when both ICSP and IDMS exist")
213+
g.Expect(hasMirrors).To(BeTrue(), "Should detect mirror registries when both ICSP and IDMS exist")
214214
}
215215

216-
func TestIsDisconnectedOCP_EmptyLists(t *testing.T) {
216+
func TestHasMirrorRegistries_EmptyLists(t *testing.T) {
217217
g := NewWithT(t)
218218
ctx := context.Background()
219219

220220
// No ICSP or IDMS resources, but CRDs are registered
221221
h := setupTestHelper(true)
222222

223-
disconnected, err := IsDisconnectedOCP(ctx, h)
223+
hasMirrors, err := HasMirrorRegistries(ctx, h)
224224
g.Expect(err).ToNot(HaveOccurred())
225-
g.Expect(disconnected).To(BeFalse(), "Should not detect disconnected environment when lists are empty")
225+
g.Expect(hasMirrors).To(BeFalse(), "Should not detect mirror registries when lists are empty")
226226
}
227227

228-
func TestIsDisconnectedOCP_CRDsNotInstalled(t *testing.T) {
228+
func TestHasMirrorRegistries_CRDsNotInstalled(t *testing.T) {
229229
g := NewWithT(t)
230230
ctx := context.Background()
231231

232232
// Don't register OpenShift CRDs - simulates non-OpenShift cluster
233233
h := setupTestHelper(false)
234234

235-
disconnected, err := IsDisconnectedOCP(ctx, h)
235+
hasMirrors, err := HasMirrorRegistries(ctx, h)
236236
g.Expect(err).ToNot(HaveOccurred(), "Should not return error when CRDs don't exist")
237-
g.Expect(disconnected).To(BeFalse(), "Should return false when CRDs don't exist (graceful degradation)")
237+
g.Expect(hasMirrors).To(BeFalse(), "Should return false when CRDs don't exist (graceful degradation)")
238238
}
239239

240240
// Test GetMCRegistryConf scenarios
@@ -290,7 +290,7 @@ func TestGetMCRegistryConf_MachineConfigNotFound(t *testing.T) {
290290
// MachineConfig CRD is registered but no resource exists
291291
// This simulates the case where MCO is installed but the specific
292292
// registry MachineConfig doesn't exist - this should be treated as an error,
293-
// not a warning, because if MCO is present and disconnected env is detected,
293+
// not a warning, because if MCO is present and mirror registries are detected,
294294
// the registry config should exist.
295295
h := setupTestHelper(true)
296296

@@ -412,11 +412,11 @@ func TestGetMCRegistryConf_InvalidBase64Content(t *testing.T) {
412412
}
413413

414414
// Test real-world scenarios
415-
func TestDisconnectedEnvironment_FullScenario(t *testing.T) {
415+
func TestMirrorRegistryEnvironment_FullScenario(t *testing.T) {
416416
g := NewWithT(t)
417417
ctx := context.Background()
418418

419-
// Simulate a full disconnected environment with IDMS and MachineConfig
419+
// Simulate a mirror registry environment with IDMS and MachineConfig
420420
expectedConfig := `[[registry]]
421421
prefix = ""
422422
location = "registry.redhat.io/rhosp-dev-preview"
@@ -467,10 +467,10 @@ func TestDisconnectedEnvironment_FullScenario(t *testing.T) {
467467

468468
h := setupTestHelper(true, idms, machineConfig)
469469

470-
// Check for disconnected environment
471-
disconnected, err := IsDisconnectedOCP(ctx, h)
470+
// Check for mirror registries
471+
hasMirrors, err := HasMirrorRegistries(ctx, h)
472472
g.Expect(err).ToNot(HaveOccurred())
473-
g.Expect(disconnected).To(BeTrue())
473+
g.Expect(hasMirrors).To(BeTrue())
474474

475475
// Get the registry configuration
476476
config, err := GetMCRegistryConf(ctx, h)
@@ -485,14 +485,62 @@ func TestNonOpenShiftCluster_GracefulDegradation(t *testing.T) {
485485
// Simulate a non-OpenShift Kubernetes cluster (no OpenShift CRDs registered)
486486
h := setupTestHelper(false)
487487

488-
// IsDisconnectedOCP should return false without error
489-
disconnected, err := IsDisconnectedOCP(ctx, h)
488+
// HasMirrorRegistries should return false without error
489+
hasMirrors, err := HasMirrorRegistries(ctx, h)
490490
g.Expect(err).ToNot(HaveOccurred(), "Should gracefully handle missing CRDs")
491-
g.Expect(disconnected).To(BeFalse())
491+
g.Expect(hasMirrors).To(BeFalse())
492492

493493
// GetMCRegistryConf should return an error that can be identified as "CRD not found"
494494
config, err := GetMCRegistryConf(ctx, h)
495495
g.Expect(err).To(HaveOccurred())
496496
g.Expect(IsNoMatchError(err)).To(BeTrue(), "Error should indicate CRD is not installed")
497497
g.Expect(config).To(BeEmpty())
498498
}
499+
500+
func TestGetMirrorRegistryCACerts(t *testing.T) {
501+
g := NewWithT(t)
502+
ctx := context.Background()
503+
504+
imageConfig := &ocpidms.Image{
505+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
506+
Spec: ocpidms.ImageSpec{AdditionalTrustedCA: ocpidms.ConfigMapNameReference{Name: "registry-cas"}},
507+
}
508+
caConfigMap := &k8s_corev1.ConfigMap{
509+
ObjectMeta: metav1.ObjectMeta{Name: "registry-cas", Namespace: "openshift-config"},
510+
Data: map[string]string{"mirror.example.com": "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----"},
511+
}
512+
h := setupTestHelper(true, imageConfig, caConfigMap)
513+
caCerts, err := GetMirrorRegistryCACerts(ctx, h)
514+
g.Expect(err).ToNot(HaveOccurred())
515+
g.Expect(caCerts).To(HaveLen(1))
516+
g.Expect(caCerts).To(HaveKey("mirror.example.com"))
517+
518+
imageConfigNoCA := &ocpidms.Image{
519+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
520+
Spec: ocpidms.ImageSpec{},
521+
}
522+
h = setupTestHelper(true, imageConfigNoCA)
523+
caCerts, err = GetMirrorRegistryCACerts(ctx, h)
524+
g.Expect(err).ToNot(HaveOccurred())
525+
g.Expect(caCerts).To(BeNil())
526+
527+
h = setupTestHelper(false)
528+
caCerts, err = GetMirrorRegistryCACerts(ctx, h)
529+
g.Expect(err).ToNot(HaveOccurred())
530+
g.Expect(caCerts).To(BeNil())
531+
}
532+
533+
func TestGetMirrorRegistryCACerts_ConfigMapNotFound(t *testing.T) {
534+
g := NewWithT(t)
535+
ctx := context.Background()
536+
537+
imageConfig := &ocpidms.Image{
538+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
539+
Spec: ocpidms.ImageSpec{AdditionalTrustedCA: ocpidms.ConfigMapNameReference{Name: "non-existent-ca"}},
540+
}
541+
h := setupTestHelper(true, imageConfig)
542+
543+
caCerts, err := GetMirrorRegistryCACerts(ctx, h)
544+
g.Expect(err).ToNot(HaveOccurred())
545+
g.Expect(caCerts).To(BeNil())
546+
}

0 commit comments

Comments
 (0)