From 7d91c6bd31d98521cd68bf06d2fbd75d2d3c4abe Mon Sep 17 00:00:00 2001 From: Richard Su Date: Mon, 9 Mar 2026 22:24:58 -0500 Subject: [PATCH 1/2] AGENT-1449: Add IRI registry authentication support to MCO Add htpasswd-based authentication to the IRI registry. The installer generates credentials and provides them via a bootstrap secret. The MCO mounts the htpasswd file into the registry container and configures registry auth environment variables. The registry password is merged into the node pull secret so kubelet can authenticate when pulling the release image. Co-Authored-By: Claude Opus 4.6 --- pkg/controller/bootstrap/bootstrap.go | 6 +- pkg/controller/common/constants.go | 3 + .../internalreleaseimage_bootstrap.go | 4 +- .../internalreleaseimage_bootstrap_test.go | 10 ++- .../internalreleaseimage_controller.go | 79 ++++++++++++++++++- .../internalreleaseimage_controller_test.go | 39 ++++++++- .../internalreleaseimage_helpers_test.go | 62 ++++++++++++++- .../internalreleaseimage_renderer.go | 29 ++++--- .../files/iri-registry-auth-htpasswd.yaml | 5 ++ .../usr-local-bin-load-registry-image-sh.yaml | 2 +- .../master/units/iri-registry.service.yaml | 8 +- 11 files changed, 226 insertions(+), 21 deletions(-) create mode 100644 pkg/controller/internalreleaseimage/templates/master/files/iri-registry-auth-htpasswd.yaml diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 504ce97bb0..c35ed349ef 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -115,6 +115,7 @@ func (b *Bootstrap) Run(destDir string) error { imageStream *imagev1.ImageStream iri *mcfgv1alpha1.InternalReleaseImage iriTLSCert *corev1.Secret + iriAuthSecret *corev1.Secret ) for _, info := range infos { if info.IsDir() { @@ -199,6 +200,9 @@ func (b *Bootstrap) Run(destDir string) error { if obj.GetName() == ctrlcommon.InternalReleaseImageTLSSecretName { iriTLSCert = obj } + if obj.GetName() == ctrlcommon.InternalReleaseImageAuthSecretName { + iriAuthSecret = obj + } default: klog.Infof("skipping %q [%d] manifest because of unhandled %T", file.Name(), idx+1, obji) } @@ -305,7 +309,7 @@ func (b *Bootstrap) Run(destDir string) error { if fgHandler != nil && fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) { if iri != nil { - iriConfigs, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, iriTLSCert, cconfig) + iriConfigs, err := internalreleaseimage.RunInternalReleaseImageBootstrap(iri, iriTLSCert, iriAuthSecret, cconfig) if err != nil { return err } diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index 31ba8b1040..cf27b799be 100644 --- a/pkg/controller/common/constants.go +++ b/pkg/controller/common/constants.go @@ -72,6 +72,9 @@ const ( // InternalReleaseImageTLSSecretName is the name of the secret manifest containing the InternalReleaseImage TLS certificate. InternalReleaseImageTLSSecretName = "internal-release-image-tls" + // InternalReleaseImageAuthSecretName is the name of the secret containing IRI registry htpasswd auth credentials. + InternalReleaseImageAuthSecretName = "internal-release-image-registry-auth" + // APIServerInstanceName is a singleton name for APIServer configuration APIServerBootstrapFileLocation = "/etc/mcs/bootstrap/api-server/api-server.yaml" diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go index 6d2a243eab..64b029f6db 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap.go @@ -8,11 +8,11 @@ import ( ) // RunInternalReleaseImageBootstrap generates the MachineConfig objects for InternalReleaseImage that would have been generated by syncInternalReleaseImage. -func RunInternalReleaseImageBootstrap(iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) ([]*mcfgv1.MachineConfig, error) { +func RunInternalReleaseImageBootstrap(iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, iriAuthSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) ([]*mcfgv1.MachineConfig, error) { configs := []*mcfgv1.MachineConfig{} for _, role := range SupportedRoles { - r := NewRendererByRole(role, iri, iriSecret, cconfig) + r := NewRendererByRole(role, iri, iriSecret, iriAuthSecret, cconfig) mc, err := r.CreateEmptyMachineConfig() if err != nil { return nil, err diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go index be62296f9f..6ced16f731 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go @@ -8,7 +8,15 @@ import ( ) func TestRunInternalReleaseImageBootstrap(t *testing.T) { - configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, cconfig().obj) + configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, nil, cconfig().obj) assert.NoError(t, err) verifyAllInternalReleaseImageMachineConfigs(t, configs) } + +func TestRunInternalReleaseImageBootstrapWithAuth(t *testing.T) { + configs, err := RunInternalReleaseImageBootstrap(&mcfgv1alpha1.InternalReleaseImage{}, iriCertSecret().obj, iriAuthSecret().obj, cconfig().obj) + assert.NoError(t, err) + assert.Len(t, configs, 2) + verifyInternalReleaseMasterMachineConfigWithAuth(t, configs[0]) + verifyInternalReleaseWorkerMachineConfig(t, configs[1]) +} diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go index 23a53d671c..40ca08836e 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go @@ -2,6 +2,8 @@ package internalreleaseimage import ( "context" + "encoding/base64" + "encoding/json" "fmt" "reflect" "time" @@ -54,6 +56,7 @@ var ( // Controller defines the InternalReleaseImage controller. type Controller struct { client mcfgclientset.Interface + kubeClient clientset.Interface eventRecorder record.EventRecorder syncHandler func(mcp string) error @@ -93,6 +96,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](), @@ -278,7 +282,8 @@ func (ctrl *Controller) updateSecret(obj, _ interface{}) { secret := obj.(*corev1.Secret) // Skip any event not related to the InternalReleaseImage secrets - if secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName { + if secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName && + secret.Name != ctrlcommon.InternalReleaseImageAuthSecretName { return } @@ -341,8 +346,11 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error { return fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageTLSSecretName, err) } + // Auth secret may not exist during upgrades from non-auth clusters + iriAuthSecret, _ := ctrl.secretLister.Secrets(ctrlcommon.MCONamespace).Get(ctrlcommon.InternalReleaseImageAuthSecretName) + for _, role := range SupportedRoles { - r := NewRendererByRole(role, iri, iriSecret, cconfig) + r := NewRendererByRole(role, iri, iriSecret, iriAuthSecret, cconfig) mc, err := ctrl.mcLister.Get(r.GetMachineConfigName()) isNotFound := errors.IsNotFound(err) @@ -369,6 +377,13 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error { } } + // Merge IRI auth credentials into the global pull secret + if iriAuthSecret != nil { + if err := ctrl.mergeIRIAuthIntoPullSecret(cconfig, iriAuthSecret); err != nil { + klog.Warningf("Failed to merge IRI auth into pull secret: %v", err) + } + } + // Initialize status if empty if err := ctrl.initializeInternalReleaseImageStatus(iri); err != nil { return err @@ -449,6 +464,66 @@ func (ctrl *Controller) addFinalizerToInternalReleaseImage(iri *mcfgv1alpha1.Int return err } +func (ctrl *Controller) mergeIRIAuthIntoPullSecret(cconfig *mcfgv1.ControllerConfig, authSecret *corev1.Secret) error { + password := string(authSecret.Data["password"]) + if password == "" { + return nil + } + + // Get cluster domain from ControllerConfig DNS + if cconfig.Spec.DNS == nil { + return fmt.Errorf("ControllerConfig DNS is not set") + } + baseDomain := cconfig.Spec.DNS.Spec.BaseDomain + iriRegistryHost := fmt.Sprintf("api-int.%s:22625", baseDomain) + + // Fetch current pull secret from openshift-config + pullSecret, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( + context.TODO(), ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("could not get pull-secret: %w", err) + } + + // Parse existing dockerconfigjson + raw := pullSecret.Data[corev1.DockerConfigJsonKey] + var dockerConfig map[string]interface{} + if err := json.Unmarshal(raw, &dockerConfig); err != nil { + return fmt.Errorf("could not parse pull secret: %w", err) + } + + auths, ok := dockerConfig["auths"].(map[string]interface{}) + if !ok { + return fmt.Errorf("pull secret missing 'auths' field") + } + + // Check if IRI entry already exists and is current + authValue := base64.StdEncoding.EncodeToString([]byte("openshift:" + password)) + if existing, ok := auths[iriRegistryHost].(map[string]interface{}); ok { + if existing["auth"] == authValue { + return nil // Already up to date + } + } + + // Merge IRI auth entry + auths[iriRegistryHost] = map[string]interface{}{ + "auth": authValue, + } + + // Marshal and update + mergedBytes, err := json.Marshal(dockerConfig) + if err != nil { + return fmt.Errorf("could not marshal merged pull secret: %w", err) + } + + pullSecret.Data[corev1.DockerConfigJsonKey] = mergedBytes + _, err = ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Update( + context.TODO(), pullSecret, metav1.UpdateOptions{}) + if err == nil { + klog.Infof("Updated pull secret with IRI registry auth credentials from secret %s/%s (uid=%s, resourceVersion=%s)", authSecret.Namespace, authSecret.Name, authSecret.UID, authSecret.ResourceVersion) + } + return err +} + func (ctrl *Controller) cascadeDelete(iri *mcfgv1alpha1.InternalReleaseImage) error { mcName := iri.GetFinalizers()[0] err := ctrl.client.MachineconfigurationV1().MachineConfigs().Delete(context.TODO(), mcName, metav1.DeleteOptions{}) diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go index 2743142c8a..bccafba529 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go @@ -2,6 +2,7 @@ package internalreleaseimage import ( "context" + "encoding/json" "testing" "time" @@ -28,9 +29,10 @@ import ( func TestInternalReleaseImageCreate(t *testing.T) { cases := []struct { - name string - initialObjects func() []runtime.Object - verify func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) + name string + initialObjects func() []runtime.Object + verify func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) + verifyPullSecret func(t *testing.T, f *fixture) }{ { name: "feature inactive", @@ -72,6 +74,34 @@ func TestInternalReleaseImageCreate(t *testing.T) { verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC) }, }, + { + name: "generate iri machine-config with auth", + initialObjects: objs(iri(), clusterVersion(), cconfig(), iriCertSecret(), iriAuthSecret()), + verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { + verifyInternalReleaseMasterMachineConfigWithAuth(t, actualMasterMC) + verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC) + }, + }, + { + name: "merge iri auth into pull secret", + initialObjects: objs(iri(), clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret()), + verify: func(t *testing.T, actualIRI *mcfgv1alpha1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { + verifyInternalReleaseMasterMachineConfigWithAuth(t, actualMasterMC) + verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC) + }, + verifyPullSecret: func(t *testing.T, f *fixture) { + ps, err := f.k8sClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( + context.TODO(), ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + assert.NoError(t, err) + var dockerConfig map[string]interface{} + err = json.Unmarshal(ps.Data[corev1.DockerConfigJsonKey], &dockerConfig) + assert.NoError(t, err) + auths := dockerConfig["auths"].(map[string]interface{}) + iriEntry, ok := auths["api-int.example.com:22625"] + assert.True(t, ok, "IRI auth entry should be present in pull secret") + assert.NotNil(t, iriEntry) + }, + }, { name: "avoid machine-config drifting", initialObjects: objs( @@ -155,6 +185,9 @@ func TestInternalReleaseImageCreate(t *testing.T) { } tc.verify(t, actualIRI, actualMasterMC, actualWorkerMC) } + if tc.verifyPullSecret != nil { + tc.verifyPullSecret(t, f) + } }) } diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go index b0cfc7a1e1..36e8ef100b 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go @@ -35,10 +35,32 @@ func verifyInternalReleaseMasterMachineConfig(t *testing.T, mc *mcfgv1.MachineCo assert.Len(t, ignCfg.Systemd.Units, 1) assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "docker-registry-image-pullspec") - assert.Len(t, ignCfg.Storage.Files, 4, "Found an unexpected file") + assert.Len(t, ignCfg.Storage.Files, 5, "Found an unexpected file") verifyIgnitionFile(t, &ignCfg, "/etc/pki/ca-trust/source/anchors/iri-root-ca.crt", "iri-root-ca-data") verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.key", "iri-tls-key") verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.crt", "iri-tls-crt") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/auth/htpasswd", "") + verifyIgnitionFileContains(t, &ignCfg, "/usr/local/bin/load-registry-image.sh", "docker-registry-image-pullspec") +} + +func verifyInternalReleaseMasterMachineConfigWithAuth(t *testing.T, mc *mcfgv1.MachineConfig) { + assert.Equal(t, masterName(), mc.Name) + assert.Equal(t, ctrlcommon.MachineConfigPoolMaster, mc.Labels[mcfgv1.MachineConfigRoleLabelKey]) + assert.Equal(t, controllerKind.Kind, mc.OwnerReferences[0].Kind) + + ignCfg, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw) + assert.NoError(t, err, mc.Name) + + assert.Len(t, ignCfg.Systemd.Units, 1) + assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "docker-registry-image-pullspec") + assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_AUTH_HTPASSWD_REALM") + assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_AUTH_HTPASSWD_PATH") + + assert.Len(t, ignCfg.Storage.Files, 5, "Found an unexpected file") + verifyIgnitionFile(t, &ignCfg, "/etc/pki/ca-trust/source/anchors/iri-root-ca.crt", "iri-root-ca-data") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.key", "iri-tls-key") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/certs/tls.crt", "iri-tls-crt") + verifyIgnitionFile(t, &ignCfg, "/etc/iri-registry/auth/htpasswd", "openshift:$2y$05$testhash") verifyIgnitionFileContains(t, &ignCfg, "/usr/local/bin/load-registry-image.sh", "docker-registry-image-pullspec") } @@ -140,6 +162,15 @@ func cconfig() *controllerConfigBuilder { } } +func (ccb *controllerConfigBuilder) withDNS(baseDomain string) *controllerConfigBuilder { + ccb.obj.Spec.DNS = &configv1.DNS{ + Spec: configv1.DNSSpec{ + BaseDomain: baseDomain, + }, + } + return ccb +} + func (ccb *controllerConfigBuilder) dockerRegistryImage(image string) *controllerConfigBuilder { ccb.obj.Spec.Images[templatectrl.DockerRegistryKey] = image return ccb @@ -224,6 +255,35 @@ func (sb *secretBuilder) build() runtime.Object { return sb.obj } +func pullSecret() *secretBuilder { + return &secretBuilder{ + obj: &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: ctrlcommon.OpenshiftConfigNamespace, + Name: ctrlcommon.GlobalPullSecretName, + }, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{"quay.io":{"auth":"dGVzdDp0ZXN0"}}}`), + }, + }, + } +} + +func iriAuthSecret() *secretBuilder { + return &secretBuilder{ + obj: &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: ctrlcommon.MCONamespace, + Name: ctrlcommon.InternalReleaseImageAuthSecretName, + }, + Data: map[string][]byte{ + "htpasswd": []byte("openshift:$2y$05$testhash"), + "password": []byte("testpassword"), + }, + }, + } +} + // clusterVersionBuilder simplifies the creation of a Secret resource in the test. type clusterVersionBuilder struct { obj *configv1.ClusterVersion diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go b/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go index 36806638fc..2f11d19bd0 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_renderer.go @@ -37,20 +37,22 @@ var ( // the InternalReleaseImage machine config resources. It can also create // a MachineConfig instance when required. type Renderer struct { - role string - iri *mcfgv1alpha1.InternalReleaseImage - iriSecret *corev1.Secret - cconfig *mcfgv1.ControllerConfig + role string + iri *mcfgv1alpha1.InternalReleaseImage + iriSecret *corev1.Secret + iriAuthSecret *corev1.Secret // may be nil + cconfig *mcfgv1.ControllerConfig } // NewRendererByRole creates a new Renderer instance for generating // the machine config for the given role. -func NewRendererByRole(role string, iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) *Renderer { +func NewRendererByRole(role string, iri *mcfgv1alpha1.InternalReleaseImage, iriSecret *corev1.Secret, iriAuthSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) *Renderer { return &Renderer{ - role: role, - iri: iri, - iriSecret: iriSecret, - cconfig: cconfig, + role: role, + iri: iri, + iriSecret: iriSecret, + iriAuthSecret: iriAuthSecret, + cconfig: cconfig, } } @@ -103,6 +105,7 @@ type renderContext struct { IriTLSKey string IriTLSCert string RootCA string + IriHtpasswd string } // newRenderContext creates a new renderContext instance. @@ -115,11 +118,19 @@ func (r *Renderer) newRenderContext() (*renderContext, error) { if err != nil { return nil, err } + iriHtpasswd := "" + if r.iriAuthSecret != nil { + if raw, found := r.iriAuthSecret.Data["htpasswd"]; found { + iriHtpasswd = string(raw) + } + } + return &renderContext{ DockerRegistryImage: r.cconfig.Spec.Images[templatectrl.DockerRegistryKey], IriTLSKey: iriTLSKey, IriTLSCert: iriTLSCert, RootCA: string(r.cconfig.Spec.RootCAData), + IriHtpasswd: iriHtpasswd, }, nil } diff --git a/pkg/controller/internalreleaseimage/templates/master/files/iri-registry-auth-htpasswd.yaml b/pkg/controller/internalreleaseimage/templates/master/files/iri-registry-auth-htpasswd.yaml new file mode 100644 index 0000000000..37d62b1b71 --- /dev/null +++ b/pkg/controller/internalreleaseimage/templates/master/files/iri-registry-auth-htpasswd.yaml @@ -0,0 +1,5 @@ +mode: 0600 +path: "/etc/iri-registry/auth/htpasswd" +contents: + inline: |- +{{indent 4 .IriHtpasswd}} diff --git a/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml b/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml index 2d93e79a1f..49f3e07bcc 100644 --- a/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml +++ b/pkg/controller/internalreleaseimage/templates/master/files/usr-local-bin-load-registry-image-sh.yaml @@ -37,7 +37,7 @@ contents: # As a fallback, let's try to fetch the registry image remotely echo "Trying to pull ${registryImage} from remote registry..." - if podman pull '${registryImage}'; then + if podman pull --authfile /var/lib/kubelet/config.json "${registryImage}"; then echo "Successfully pulled ${registryImage} from remote registry" exit 0 fi diff --git a/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml b/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml index e3635e2f85..f7b86dcc3c 100644 --- a/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml +++ b/pkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml @@ -11,7 +11,13 @@ contents: | Environment=PODMAN_SYSTEMD_UNIT=%n ExecStartPre=/usr/local/bin/load-registry-image.sh ExecStartPre=/bin/rm -f %t/%n.ctr-id - ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --log-driver=journald --replace --name=iri-registry -v ${REGISTRY_DIR}:/var/lib/registry:ro,Z -v /etc/iri-registry/certs:/certs:ro -e REGISTRY_HTTP_ADDR=0.0.0.0:22625 -e REGISTRY_HTTP_TLS_CERTIFICATE=/certs/tls.crt -e REGISTRY_HTTP_TLS_KEY=/certs/tls.key -u 0 --entrypoint=/usr/bin/distribution {{ .DockerRegistryImage }} serve /etc/registry/config.yaml + ExecStart=podman run --net host --cidfile=%t/%n.ctr-id --log-driver=journald --replace --name=iri-registry \ + -v ${REGISTRY_DIR}:/var/lib/registry:ro,Z -v /etc/iri-registry/certs:/certs:ro \ + {{ if .IriHtpasswd }}-v /etc/iri-registry/auth:/etc/registry/auth:ro,Z \ + {{ end }}-e REGISTRY_HTTP_ADDR=0.0.0.0:22625 \ + -e REGISTRY_HTTP_TLS_CERTIFICATE=/certs/tls.crt -e REGISTRY_HTTP_TLS_KEY=/certs/tls.key \ + {{ if .IriHtpasswd }}-e REGISTRY_AUTH_HTPASSWD_REALM=openshift-registry -e REGISTRY_AUTH_HTPASSWD_PATH=/etc/registry/auth/htpasswd \ + {{ end }}-u 0 --entrypoint=/usr/bin/distribution {{ .DockerRegistryImage }} serve /etc/registry/config.yaml ExecStop=/usr/bin/podman stop --ignore --cidfile=%t/%n.ctr-id ExecStopPost=/usr/bin/podman rm -f --ignore --cidfile=%t/%n.ctr-id From a5a65dc15d369ae2d2425322d2bec23ffecea982 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Wed, 11 Mar 2026 15:35:12 -0500 Subject: [PATCH 2/2] AGENT-1449: Fix bootstrap/in-cluster rendered MC hash mismatch The IRI controller merges registry auth credentials into the global pull secret after bootstrap. This triggers the template controller to re-render template MCs (00-master, etc.) with the updated pull secret, producing a different rendered MC hash than what bootstrap created. The mismatch causes the MCD DaemonSet pod to fail during bootstrap: it reads the bootstrap-rendered MC name from the node annotation, but that MC no longer exists in-cluster (replaced by the re-rendered one). The MCD falls back to reading /etc/machine-config-daemon/currentconfig, which was never written because the firstboot MCD detected "no changes" and skipped it. Both master nodes go Degraded and never recover. Fix by merging IRI auth into the pull secret during bootstrap before template MC rendering, so both bootstrap and in-cluster produce identical rendered MC hashes. Extract the pull secret merge logic into a shared MergeIRIAuthIntoPullSecret function used by both the bootstrap path and the in-cluster IRI controller. Assisted-by: Claude Opus 4.6 --- pkg/controller/bootstrap/bootstrap.go | 18 +++ .../internalreleaseimage_controller.go | 37 +----- .../internalreleaseimage/pullsecret.go | 52 ++++++++ .../internalreleaseimage/pullsecret_test.go | 119 ++++++++++++++++++ 4 files changed, 195 insertions(+), 31 deletions(-) create mode 100644 pkg/controller/internalreleaseimage/pullsecret.go create mode 100644 pkg/controller/internalreleaseimage/pullsecret_test.go diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index c35ed349ef..c2b3b716bf 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -247,6 +247,24 @@ func (b *Bootstrap) Run(destDir string) error { } pullSecretBytes := pullSecret.Data[corev1.DockerConfigJsonKey] + + // If IRI auth is enabled, merge the IRI registry credentials into the pull + // secret before rendering template MCs. This ensures the bootstrap-rendered + // MCs use the same pull secret content as the in-cluster IRI controller + // will produce, avoiding a rendered MachineConfig hash mismatch. + if fgHandler != nil && fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) { + if iriAuthSecret != nil && cconfig.Spec.DNS != nil { + password := string(iriAuthSecret.Data["password"]) + merged, mergeErr := internalreleaseimage.MergeIRIAuthIntoPullSecret(pullSecretBytes, password, cconfig.Spec.DNS.Spec.BaseDomain) + if mergeErr != nil { + klog.Warningf("Failed to merge IRI auth into pull secret during bootstrap: %v", mergeErr) + } else { + pullSecretBytes = merged + klog.Infof("Merged IRI registry auth into pull secret for bootstrap rendering") + } + } + } + iconfigs, err := template.RunBootstrap(b.templatesDir, cconfig, pullSecretBytes, apiServer) if err != nil { return err diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go index 40ca08836e..5fc09d5547 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go @@ -2,8 +2,6 @@ package internalreleaseimage import ( "context" - "encoding/base64" - "encoding/json" "fmt" "reflect" "time" @@ -470,12 +468,10 @@ func (ctrl *Controller) mergeIRIAuthIntoPullSecret(cconfig *mcfgv1.ControllerCon return nil } - // Get cluster domain from ControllerConfig DNS if cconfig.Spec.DNS == nil { return fmt.Errorf("ControllerConfig DNS is not set") } baseDomain := cconfig.Spec.DNS.Spec.BaseDomain - iriRegistryHost := fmt.Sprintf("api-int.%s:22625", baseDomain) // Fetch current pull secret from openshift-config pullSecret, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( @@ -484,35 +480,14 @@ func (ctrl *Controller) mergeIRIAuthIntoPullSecret(cconfig *mcfgv1.ControllerCon return fmt.Errorf("could not get pull-secret: %w", err) } - // Parse existing dockerconfigjson - raw := pullSecret.Data[corev1.DockerConfigJsonKey] - var dockerConfig map[string]interface{} - if err := json.Unmarshal(raw, &dockerConfig); err != nil { - return fmt.Errorf("could not parse pull secret: %w", err) - } - - auths, ok := dockerConfig["auths"].(map[string]interface{}) - if !ok { - return fmt.Errorf("pull secret missing 'auths' field") - } - - // Check if IRI entry already exists and is current - authValue := base64.StdEncoding.EncodeToString([]byte("openshift:" + password)) - if existing, ok := auths[iriRegistryHost].(map[string]interface{}); ok { - if existing["auth"] == authValue { - return nil // Already up to date - } - } - - // Merge IRI auth entry - auths[iriRegistryHost] = map[string]interface{}{ - "auth": authValue, + mergedBytes, err := MergeIRIAuthIntoPullSecret(pullSecret.Data[corev1.DockerConfigJsonKey], password, baseDomain) + if err != nil { + return err } - // Marshal and update - mergedBytes, err := json.Marshal(dockerConfig) - if err != nil { - return fmt.Errorf("could not marshal merged pull secret: %w", err) + // No change needed + if string(mergedBytes) == string(pullSecret.Data[corev1.DockerConfigJsonKey]) { + return nil } pullSecret.Data[corev1.DockerConfigJsonKey] = mergedBytes diff --git a/pkg/controller/internalreleaseimage/pullsecret.go b/pkg/controller/internalreleaseimage/pullsecret.go new file mode 100644 index 0000000000..194a525a95 --- /dev/null +++ b/pkg/controller/internalreleaseimage/pullsecret.go @@ -0,0 +1,52 @@ +package internalreleaseimage + +import ( + "encoding/base64" + "encoding/json" + "fmt" +) + +// MergeIRIAuthIntoPullSecret merges IRI registry authentication credentials +// into a dockerconfigjson pull secret. It adds an auth entry for the IRI +// registry host (api-int.:22625) so that kubelet can pull from it. +// +// This must be called during both bootstrap and in-cluster rendering to ensure +// the pull secret content is consistent, avoiding a rendered MachineConfig +// hash mismatch between bootstrap and in-cluster. +func MergeIRIAuthIntoPullSecret(pullSecretRaw []byte, password string, baseDomain string) ([]byte, error) { + if password == "" { + return pullSecretRaw, nil + } + + iriRegistryHost := fmt.Sprintf("api-int.%s:22625", baseDomain) + + var dockerConfig map[string]interface{} + if err := json.Unmarshal(pullSecretRaw, &dockerConfig); err != nil { + return nil, fmt.Errorf("could not parse pull secret: %w", err) + } + + auths, ok := dockerConfig["auths"].(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("pull secret missing 'auths' field") + } + + authValue := base64.StdEncoding.EncodeToString([]byte("openshift:" + password)) + + // Check if IRI entry already exists and is current + if existing, ok := auths[iriRegistryHost].(map[string]interface{}); ok { + if existing["auth"] == authValue { + return pullSecretRaw, nil + } + } + + auths[iriRegistryHost] = map[string]interface{}{ + "auth": authValue, + } + + mergedBytes, err := json.Marshal(dockerConfig) + if err != nil { + return nil, fmt.Errorf("could not marshal merged pull secret: %w", err) + } + + return mergedBytes, nil +} diff --git a/pkg/controller/internalreleaseimage/pullsecret_test.go b/pkg/controller/internalreleaseimage/pullsecret_test.go new file mode 100644 index 0000000000..33cc694a72 --- /dev/null +++ b/pkg/controller/internalreleaseimage/pullsecret_test.go @@ -0,0 +1,119 @@ +package internalreleaseimage + +import ( + "encoding/base64" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeIRIAuthIntoPullSecret(t *testing.T) { + basePullSecret := `{"auths":{"quay.io":{"auth":"dGVzdDp0ZXN0"}}}` + + tests := []struct { + name string + pullSecret string + password string + baseDomain string + expectChanged bool + expectError bool + verifyAuthHost string + }{ + { + name: "adds IRI auth entry", + pullSecret: basePullSecret, + password: "testpassword", + baseDomain: "example.com", + expectChanged: true, + verifyAuthHost: "api-int.example.com:22625", + }, + { + name: "empty password returns unchanged", + pullSecret: basePullSecret, + password: "", + baseDomain: "example.com", + expectChanged: false, + }, + { + name: "already up-to-date returns unchanged", + pullSecret: pullSecretWithIRIAuth("example.com", "testpassword"), + password: "testpassword", + baseDomain: "example.com", + expectChanged: false, + }, + { + name: "updates stale entry", + pullSecret: pullSecretWithIRIAuth("example.com", "oldpassword"), + password: "newpassword", + baseDomain: "example.com", + expectChanged: true, + verifyAuthHost: "api-int.example.com:22625", + }, + { + name: "invalid JSON returns error", + pullSecret: "not-json", + password: "testpassword", + baseDomain: "example.com", + expectError: true, + }, + { + name: "missing auths field returns error", + pullSecret: `{"registry":"quay.io"}`, + password: "testpassword", + baseDomain: "example.com", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := MergeIRIAuthIntoPullSecret([]byte(tt.pullSecret), tt.password, tt.baseDomain) + + if tt.expectError { + assert.Error(t, err) + return + } + assert.NoError(t, err) + + if !tt.expectChanged { + assert.Equal(t, tt.pullSecret, string(result), "pull secret should not change") + return + } + + // Verify the IRI auth entry was added + var dockerConfig map[string]interface{} + err = json.Unmarshal(result, &dockerConfig) + assert.NoError(t, err) + + auths := dockerConfig["auths"].(map[string]interface{}) + iriEntry, ok := auths[tt.verifyAuthHost].(map[string]interface{}) + assert.True(t, ok, "IRI auth entry should be present") + + expectedAuth := base64.StdEncoding.EncodeToString([]byte("openshift:" + tt.password)) + assert.Equal(t, expectedAuth, iriEntry["auth"]) + + // Verify original entries are preserved + _, hasQuay := auths["quay.io"] + assert.True(t, hasQuay, "original quay.io entry should be preserved") + }) + } +} + +// pullSecretWithIRIAuth creates a pull secret JSON that already contains an IRI auth entry. +func pullSecretWithIRIAuth(baseDomain string, password string) string { + authValue := base64.StdEncoding.EncodeToString([]byte("openshift:" + password)) + host := "api-int." + baseDomain + ":22625" + dockerConfig := map[string]interface{}{ + "auths": map[string]interface{}{ + "quay.io": map[string]interface{}{ + "auth": "dGVzdDp0ZXN0", + }, + host: map[string]interface{}{ + "auth": authValue, + }, + }, + } + b, _ := json.Marshal(dockerConfig) + return string(b) +}