Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bindata/rbac/rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ rules:
- config.openshift.io
resources:
- imagedigestmirrorsets
- images
- networks
verbs:
- get
Expand Down
1 change: 1 addition & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ rules:
- config.openshift.io
resources:
- imagedigestmirrorsets
- images
- networks
verbs:
- get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions internal/dataplane/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
}

Expand Down
86 changes: 53 additions & 33 deletions internal/dataplane/util/image_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
}

Expand All @@ -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) {
Expand Down
103 changes: 78 additions & 25 deletions internal/dataplane/util/image_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()

Expand All @@ -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()

Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -485,14 +485,67 @@ 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)
g.Expect(err).To(HaveOccurred())
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")
}
Loading