From 96dfdad398105ba720f4c7c5e1ca7758adc9f771 Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Tue, 7 Jul 2026 16:16:48 -0400 Subject: [PATCH] OCPBUGS-85519: Add Agent IRI credentials to global pull secret The IRI credentials were being added to /var/lib/kubelet/config.json which gave only kubelet the necessary credentials to pull images from the IRI registry. Fix now adds the IRI credentials to the global pull secret so that other components like the MCO could also pull images from this internal registry. --- pkg/controller/common/iri_secret_merger.go | 17 +-- .../internalreleaseimage_controller.go | 2 + .../template/template_controller.go | 29 +++- .../template/template_controller_test.go | 4 +- pkg/operator/operator.go | 1 + pkg/operator/sync.go | 13 ++ pkg/operator/sync_test.go | 133 ++++++++++++++++++ 7 files changed, 184 insertions(+), 15 deletions(-) diff --git a/pkg/controller/common/iri_secret_merger.go b/pkg/controller/common/iri_secret_merger.go index a5188dc5a8..3b590c1652 100644 --- a/pkg/controller/common/iri_secret_merger.go +++ b/pkg/controller/common/iri_secret_merger.go @@ -1,6 +1,7 @@ package common import ( + "crypto/subtle" "encoding/base64" "encoding/json" "errors" @@ -53,7 +54,7 @@ func NewIRISecretMerger( if err != nil { return "", "", fmt.Errorf("could not get ControllerConfig: %w", err) } - return extractIRICredentials(secret, cconfig) + return ExtractIRICredentials(secret, cconfig) }, } } @@ -72,7 +73,7 @@ func NewIRISecretMergerFromObjects( if !iri { return "", "", errIRIDisabled } - return extractIRICredentials(secret, cconfig) + return ExtractIRICredentials(secret, cconfig) }, } } @@ -91,7 +92,7 @@ func (m *IRISecretMerger) Merge(pullSecretRaw []byte) ([]byte, error) { if err != nil { return nil, err } - merged, changed, err := mergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw, password, baseDomain) + merged, changed, err := MergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw, password, baseDomain) if err != nil { return nil, err } @@ -101,9 +102,9 @@ func (m *IRISecretMerger) Merge(pullSecretRaw []byte) ([]byte, error) { return merged, nil } -// extractIRICredentials validates and extracts the password and baseDomain from +// ExtractIRICredentials validates and extracts the password and baseDomain from // the IRI credentials secret and ControllerConfig. -func extractIRICredentials(secret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) (password, baseDomain string, err error) { +func ExtractIRICredentials(secret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) (password, baseDomain string, err error) { if secret == nil { return "", "", fmt.Errorf("IRI registry credentials secret must not be nil") } @@ -124,13 +125,13 @@ func extractIRICredentials(secret *corev1.Secret, cconfig *mcfgv1.ControllerConf return string(pw), bd, nil } -// mergeIRIRegistryCredentialsIntoPullSecret merges IRI registry authentication +// MergeIRIRegistryCredentialsIntoPullSecret merges IRI registry authentication // credentials into a dockerconfigjson pull secret. It adds auth entries for // api-int.: (all nodes) and // localhost: (masters, where the registry runs locally). // Returns the merged bytes, a boolean indicating whether the pull secret was // changed, and any error. -func mergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw []byte, password, baseDomain string) ([]byte, bool, error) { +func MergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw []byte, password, baseDomain string) ([]byte, bool, error) { // The IRI registry is reachable via api-int on all nodes, and also via // localhost on master nodes where it runs locally. registries.conf mirror // rules on masters use localhost:22625, so credentials must be present for @@ -175,5 +176,5 @@ func mergeIRIRegistryCredentialsIntoPullSecret(pullSecretRaw []byte, password, b // matches expected. func pullSecretHasAuth(auths map[string]interface{}, host, expected string) bool { e, ok := auths[host].(map[string]interface{}) - return ok && e["auth"] == expected + return ok && subtle.ConstantTimeCompare([]byte(e["auth"].(string)), []byte(expected)) == 1 } diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go index 44d915a523..f9e763c0b0 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go @@ -51,6 +51,7 @@ var updateBackoff = wait.Backoff{ // Controller defines the InternalReleaseImage controller. type Controller struct { client mcfgclientset.Interface + kubeClient clientset.Interface eventRecorder record.EventRecorder syncHandler func(mcp string) error @@ -101,6 +102,7 @@ func New( ctrl := &Controller{ client: mcfgClient, + kubeClient: kubeClient, eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-internalreleaseimagecontroller"})), queue: workqueue.NewTypedRateLimitingQueueWithConfig( workqueue.DefaultTypedControllerRateLimiter[string](), diff --git a/pkg/controller/template/template_controller.go b/pkg/controller/template/template_controller.go index 5b7588a402..6244decd30 100644 --- a/pkg/controller/template/template_controller.go +++ b/pkg/controller/template/template_controller.go @@ -158,9 +158,28 @@ func New( } func (ctrl *Controller) filterSecret(secret *corev1.Secret) { - if secret.Name == "pull-secret" || secret.Name == ctrlcommon.InternalReleaseImageAuthSecretName { + // Check if this is the IRI auth secret + if secret.Namespace == ctrlcommon.MCONamespace && secret.Name == ctrlcommon.InternalReleaseImageAuthSecretName { ctrl.enqueueController() - klog.Infof("Re-syncing ControllerConfig due to secret %s change", secret.Name) + klog.Infof("Re-syncing ControllerConfig due to secret %s/%s change", secret.Namespace, secret.Name) + return + } + + // Check if this is the configured global pull secret + cfg, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName) + if err != nil { + // If we can't get the ControllerConfig, we can't determine if this secret + // is the pull secret, so skip the check. The controller will eventually + // sync when the ControllerConfig is available. + klog.V(4).Infof("Could not get ControllerConfig to check secret %s/%s: %v", secret.Namespace, secret.Name, err) + return + } + + if cfg.Spec.PullSecret != nil && + secret.Namespace == cfg.Spec.PullSecret.Namespace && + secret.Name == cfg.Spec.PullSecret.Name { + ctrl.enqueueController() + klog.Infof("Re-syncing ControllerConfig due to secret %s/%s change", secret.Namespace, secret.Name) } } @@ -170,7 +189,7 @@ func (ctrl *Controller) addSecret(obj interface{}) { ctrl.deleteSecret(secret) return } - klog.V(4).Infof("Add Secret %v", secret) + klog.V(4).Infof("Add Secret %s/%s", secret.Namespace, secret.Name) ctrl.filterSecret(secret) } @@ -178,7 +197,7 @@ func (ctrl *Controller) updateSecret(old, newObj interface{}) { oldSecret := old.(*corev1.Secret) newSecret := newObj.(*corev1.Secret) - klog.V(4).Infof("Update Secret %v", newSecret) + klog.V(4).Infof("Update Secret %s/%s", newSecret.Namespace, newSecret.Name) // Only trigger resync if the secret data actually changed // This prevents log spam from informer resyncs and watch reconnections @@ -189,7 +208,7 @@ func (ctrl *Controller) updateSecret(old, newObj interface{}) { func (ctrl *Controller) deleteSecret(obj interface{}) { secret, ok := obj.(*corev1.Secret) - klog.V(4).Infof("Delete Secret %v", secret) + klog.V(4).Infof("Delete Secret %s/%s", secret.Namespace, secret.Name) if !ok { tombstone, ok := obj.(cache.DeletedFinalStateUnknown) diff --git a/pkg/controller/template/template_controller_test.go b/pkg/controller/template/template_controller_test.go index d15b9a7501..3ea10d06ee 100644 --- a/pkg/controller/template/template_controller_test.go +++ b/pkg/controller/template/template_controller_test.go @@ -530,8 +530,8 @@ func TestKubeletAutoNodeSizingEnabled(t *testing.T) { } } -// TestMergesIRIRegistryCredentialsIntoPullSecret verifies that the template controller merges -// IRI registry credentials into the pull secret when rendering 00-master, so that +// TestMergesIRIRegistryCredentialsIntoPullSecret verifies that the template +// controller merges IRI registry credentials into the rendered pull secret so // nodes can authenticate to the IRI registry without writing to the user-controlled // global pull secret. func TestMergesIRIRegistryCredentialsIntoPullSecret(t *testing.T) { diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 311c24c643..407b6687d3 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -290,6 +290,7 @@ func New( apiserverInformer.Informer(), moscInformer.Informer(), networkPolicyInformer.Informer(), + iriInformer.Informer(), } for _, i := range informers { i.AddEventHandler(optr.eventHandler()) diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 4d330fa6a7..08dc05d782 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -2324,6 +2324,19 @@ func (optr *Operator) getImageRegistryPullSecrets() ([]byte, error) { return nil, fmt.Errorf("failed to marshal the merged pull secrets: %w", err) } + // Ensure nodes can authenticate to the internal release image (IRI) + // registry when pulling images during OS updates. The credentials + // added here become part of ControllerConfig.Spec.InternalRegistryPullSecret, + // which is the auth source the OS-update path (rpm-ostree/bootc) uses; + // that path is separate from the kubelet's pull secret, so IRI credentials + // must be supplied here in addition to the render-time merge. When IRI is + // not enabled on the cluster this merge makes no changes. + iriMerger := ctrlcommon.NewIRISecretMerger(optr.mcoSecretLister, optr.ccLister, optr.iriLister) + mergedPullSecrets, err = iriMerger.Merge(mergedPullSecrets) + if err != nil { + return nil, fmt.Errorf("failed to merge IRI registry credentials into image registry pull secrets: %w", err) + } + return mergedPullSecrets, nil } diff --git a/pkg/operator/sync_test.go b/pkg/operator/sync_test.go index 7c5ca80260..394e137bcc 100644 --- a/pkg/operator/sync_test.go +++ b/pkg/operator/sync_test.go @@ -2,6 +2,8 @@ package operator import ( "context" + "encoding/base64" + "encoding/json" "testing" configv1 "github.com/openshift/api/config/v1" @@ -14,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" + corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" @@ -308,6 +311,136 @@ func TestMachineOSBuilderSecretReconciliation(t *testing.T) { } } +// newNamespacedIndexer returns an indexer configured with the namespace index, +// as required by the namespaced core listers (Secrets, ServiceAccounts). +func newNamespacedIndexer(objs ...interface{}) cache.Indexer { + idx := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) + for _, o := range objs { + idx.Add(o) + } + return idx +} + +// TestGetImageRegistryPullSecretsIRIMerge verifies that getImageRegistryPullSecrets +// merges InternalReleaseImage (IRI) registry credentials into the assembled +// image-registry pull secret when IRI is in use, and leaves the secret untouched +// when IRI is absent. This blob feeds ControllerConfig.Spec.InternalRegistryPullSecret, +// which the daemon writes to /etc/mco/internal-registry-pull-secret.json for the +// OS-update image-pull path. +func TestGetImageRegistryPullSecretsIRIMerge(t *testing.T) { + const ( + baseDomain = "example.com" + iriPassword = "s3cr3t" + ) + expectedIRIAuth := base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername + ":" + iriPassword)) + iriAPIIntHost := "api-int." + baseDomain + ":22625" + iriLocalHost := "localhost:22625" + + // Common fixtures independent of whether IRI is enabled. + imageRegistryCO := &configv1.ClusterOperator{ObjectMeta: metav1.ObjectMeta{Name: "image-registry"}} + clusterDNS := &configv1.DNS{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.DNSSpec{BaseDomain: baseDomain}, + } + // machine-os-puller SA with no image pull secrets; the cluster pull secret + // alone keeps the assembled "auths" map non-empty so the merge path runs. + machineOSPullerSA := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "machine-os-puller", Namespace: ctrlcommon.MCONamespace}, + } + populatedPullSecretContent := `{"auths":{"registry.example.com":{"auth":"` + + base64.StdEncoding.EncodeToString([]byte("user:pass")) + `"}}}` + + // IRI-specific fixtures. + iriInstance := &mcfgv1.InternalReleaseImage{ + ObjectMeta: metav1.ObjectMeta{Name: ctrlcommon.InternalReleaseImageInstanceName}, + } + iriAuthSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: ctrlcommon.InternalReleaseImageAuthSecretName, Namespace: ctrlcommon.MCONamespace}, + Data: map[string][]byte{"password": []byte(iriPassword)}, + } + controllerConfig := &mcfgv1.ControllerConfig{ + ObjectMeta: metav1.ObjectMeta{Name: ctrlcommon.ControllerConfigName}, + Spec: mcfgv1.ControllerConfigSpec{ + DNS: &configv1.DNS{Spec: configv1.DNSSpec{BaseDomain: baseDomain}}, + }, + } + + cases := []struct { + name string + // pullSecretContent is the raw ".dockerconfigjson" of the cluster pull secret. + pullSecretContent string + iriEnabled bool + // expectNil asserts getImageRegistryPullSecrets returns a nil secret, + // used for the empty-"auths" ("don't roll config") path. + expectNil bool + }{ + {name: "IRI absent - pull secret unchanged", pullSecretContent: populatedPullSecretContent, iriEnabled: false}, + {name: "IRI present - credentials merged", pullSecretContent: populatedPullSecretContent, iriEnabled: true}, + // With no image-pull secrets on the SA and an empty cluster pull secret, + // the assembled "auths" map is empty. Even with IRI enabled the function + // must return nil rather than emitting a secret carrying only IRI creds. + {name: "empty auths with IRI enabled - returns nil", pullSecretContent: "{}", iriEnabled: true, expectNil: true}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + mcoSecretObjs := []interface{}{} + iriObjs := []interface{}{} + if tc.iriEnabled { + mcoSecretObjs = append(mcoSecretObjs, iriAuthSecret) + iriObjs = append(iriObjs, iriInstance) + } + + clusterPullSecret := helpers.NewDockerCfgJSONSecret(ctrlcommon.GlobalPullSecretName, ctrlcommon.OpenshiftConfigNamespace, tc.pullSecretContent) + + optr := &Operator{ + namespace: ctrlcommon.MCONamespace, + clusterOperatorLister: configlistersv1.NewClusterOperatorLister(newNamespacedIndexer(imageRegistryCO)), + dnsLister: configlistersv1.NewDNSLister(newNamespacedIndexer(clusterDNS)), + mcoSALister: corev1listers.NewServiceAccountLister(newNamespacedIndexer(machineOSPullerSA)), + mcoSecretLister: corev1listers.NewSecretLister(newNamespacedIndexer(mcoSecretObjs...)), + ocSecretLister: corev1listers.NewSecretLister(newNamespacedIndexer(clusterPullSecret)), + ccLister: mcplister.NewControllerConfigLister(newNamespacedIndexer(controllerConfig)), + iriLister: mcplister.NewInternalReleaseImageLister(newNamespacedIndexer(iriObjs...)), + } + + raw, err := optr.getImageRegistryPullSecrets() + assert.NoError(t, err) + + if tc.expectNil { + // Empty "auths": nothing to roll, so no secret is emitted (and + // IRI creds are not emitted on their own). + assert.Nil(t, raw) + return + } + assert.NotEmpty(t, raw) + + var parsed struct { + Auths map[string]struct { + Auth string `json:"auth"` + } `json:"auths"` + } + assert.NoError(t, json.Unmarshal(raw, &parsed)) + + // The original registry entry is always present. + assert.Contains(t, parsed.Auths, "registry.example.com") + + if tc.iriEnabled { + assert.Contains(t, parsed.Auths, iriAPIIntHost, "expected api-int IRI auth entry to be merged") + assert.Contains(t, parsed.Auths, iriLocalHost, "expected localhost IRI auth entry to be merged") + assert.Equal(t, expectedIRIAuth, parsed.Auths[iriAPIIntHost].Auth) + assert.Equal(t, expectedIRIAuth, parsed.Auths[iriLocalHost].Auth) + } else { + assert.NotContains(t, parsed.Auths, iriAPIIntHost, "IRI auth entry must not be present when IRI is absent") + assert.NotContains(t, parsed.Auths, iriLocalHost, "IRI auth entry must not be present when IRI is absent") + } + }) + } +} + func TestSyncMachineConfiguration(t *testing.T) { cases := []struct { name string