From 62bf770966c247c8c8423104599ab5d5294a5f68 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Fri, 24 Apr 2026 15:14:47 +0800 Subject: [PATCH 1/7] AGENT-1449: Add single-phase IRI registry credential rotation Implement credential rotation that accepts brief registry downtime. When an admin updates iriAuthSecret.Data["password"], the controller: 1. Detects the mismatch between password and htpasswd (via bcrypt compare) 2. Generates a new bcrypt hash and updates iriAuthSecret.Data["htpasswd"] 3. Re-renders the master MachineConfig with the new htpasswd 4. MCD rolls out the updated MC; brief downtime for IRI registry during rollout is accepted Key changes: - Add kubeClient field to IRI controller (needed to update auth secret) - Add reconcileHtpasswd to detect password/htpasswd mismatch and regenerate the bcrypt hash; moved to internalreleaseimage_registry_auth.go alongside the bcrypt helpers (generateHtpasswdEntry, HtpasswdMatchesPassword) - Add NoneStatusAction for /etc/iri-registry/auth/htpasswd in NodeDisruptionPolicy (distribution registry re-reads htpasswd on mtime change, no restart needed) - Add unit tests for reconcileHtpasswd - Add e2e test for the full rotation flow (TestIRIAuth_CredentialRotation); uses ExecCmdOnNode via MCD pod to reach api-int:22625 in CI Assisted-by: Claude Sonnet 4.6 --- pkg/apihelpers/apihelpers.go | 10 + .../internalreleaseimage_bootstrap_test.go | 2 +- .../internalreleaseimage_controller.go | 10 + .../internalreleaseimage_controller_test.go | 101 +++++- .../internalreleaseimage_helpers_test.go | 23 +- .../internalreleaseimage_registry_auth.go | 72 +++++ test/e2e-iri/iri_test.go | 81 +++++ vendor/golang.org/x/crypto/bcrypt/base64.go | 35 ++ vendor/golang.org/x/crypto/bcrypt/bcrypt.go | 304 ++++++++++++++++++ vendor/modules.txt | 1 + 10 files changed, 627 insertions(+), 12 deletions(-) create mode 100644 pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go create mode 100644 vendor/golang.org/x/crypto/bcrypt/base64.go create mode 100644 vendor/golang.org/x/crypto/bcrypt/bcrypt.go diff --git a/pkg/apihelpers/apihelpers.go b/pkg/apihelpers/apihelpers.go index 31809a182f..988e4b5a29 100644 --- a/pkg/apihelpers/apihelpers.go +++ b/pkg/apihelpers/apihelpers.go @@ -33,6 +33,16 @@ var ( }, }, }, + { + // The Distribution registry re-reads htpasswd on mtime change, + // so credential rotation does not require a service restart. + Path: "/etc/iri-registry/auth/htpasswd", + Actions: []opv1.NodeDisruptionPolicyStatusAction{ + { + Type: opv1.NoneStatusAction, + }, + }, + }, { Path: constants.GPGNoRebootPath, Actions: []opv1.NodeDisruptionPolicyStatusAction{ diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go index 5273e23a2e..ee3f896577 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_bootstrap_test.go @@ -7,7 +7,7 @@ import ( ) func TestRunInternalReleaseImageBootstrap(t *testing.T) { - configs, err := RunInternalReleaseImageBootstrap(iriCertSecret().obj, iriRegistryCredentialsSecret().obj, cconfig().withDNS("example.com").obj) + configs, err := RunInternalReleaseImageBootstrap(iriCertSecret().obj, iriAuthSecret().obj, cconfig().withDNS("example.com").obj) assert.NoError(t, err) assert.Len(t, configs, 2) verifyInternalReleaseMasterMachineConfig(t, configs[0]) diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go index 4ca6a1bdb4..9ebeacdf09 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go @@ -53,6 +53,7 @@ var ( // Controller defines the InternalReleaseImage controller. type Controller struct { client mcfgclientset.Interface + kubeClient clientset.Interface eventRecorder record.EventRecorder syncHandler func(mcp string) error @@ -103,6 +104,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](), @@ -546,6 +548,14 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) (syncErr error) { return fmt.Errorf("could not get Secret %s: %w", ctrlcommon.InternalReleaseImageAuthSecretName, err) } + // Ensure the htpasswd field is in sync with the password field. If the + // password was rotated, this generates a new bcrypt hash and updates the + // secret before re-rendering the MachineConfig. + iriRegistryCredentialsSecret, err = reconcileHtpasswd(ctrl.kubeClient, iriRegistryCredentialsSecret) + if err != nil { + return fmt.Errorf("failed to reconcile IRI registry htpasswd: %w", err) + } + for _, role := range SupportedRoles { r := NewRendererByRole(role, iriSecret, iriRegistryCredentialsSecret, cconfig) diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go index cc0c1c2e29..1b08cac2e1 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go @@ -42,7 +42,7 @@ func TestInternalReleaseImageCreate(t *testing.T) { }, { name: "add finalizer if not present", - initialObjects: objs(iri(), clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriRegistryCredentialsSecret(), pullSecret()), + initialObjects: objs(iri(), clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret()), verify: func(t *testing.T, actualIRI *mcfgv1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { assert.Len(t, actualIRI.Finalizers, 1) assert.Contains(t, actualIRI.Finalizers, iriFinalizerName) @@ -52,7 +52,7 @@ func TestInternalReleaseImageCreate(t *testing.T) { name: "update status if not set", initialObjects: objs( iri().finalizer(iriFinalizerName), - clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriRegistryCredentialsSecret(), pullSecret()), + clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret()), verify: func(t *testing.T, actualIRI *mcfgv1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { assert.Len(t, actualIRI.Status.Releases, 1) assert.Equal(t, actualIRI.Status.Releases[0].Name, "ocp-release-bundle-4.21.5-x86_64") @@ -64,7 +64,7 @@ func TestInternalReleaseImageCreate(t *testing.T) { }, { name: "generate iri machine-config if not present", - initialObjects: objs(iri(), clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriRegistryCredentialsSecret(), pullSecret()), + initialObjects: objs(iri(), clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret()), verify: func(t *testing.T, actualIRI *mcfgv1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { verifyInternalReleaseMasterMachineConfig(t, actualMasterMC) verifyInternalReleaseWorkerMachineConfig(t, actualWorkerMC) @@ -74,7 +74,7 @@ func TestInternalReleaseImageCreate(t *testing.T) { name: "avoid machine-config drifting", initialObjects: objs( iri().finalizer(iriFinalizerName), - clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriRegistryCredentialsSecret(), pullSecret(), + clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret(), machineconfigmaster().ignition("some garbage"), machineconfigworker().ignition("other garbage")), verify: func(t *testing.T, actualIRI *mcfgv1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { @@ -86,7 +86,7 @@ func TestInternalReleaseImageCreate(t *testing.T) { name: "refresh machine-config on controllerConfig update", initialObjects: objs( iri().finalizer(iriFinalizerName), - clusterVersion(), cconfig().dockerRegistryImage("a-new-docker-registry-image-pullspec").withDNS("example.com"), iriCertSecret(), iriRegistryCredentialsSecret(), pullSecret(), + clusterVersion(), cconfig().dockerRegistryImage("a-new-docker-registry-image-pullspec").withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret(), machineconfigmaster(), machineconfigworker()), verify: func(t *testing.T, actualIRI *mcfgv1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { verifyInternalReleaseMasterMachineConfig(t, actualMasterMC) @@ -109,7 +109,7 @@ func TestInternalReleaseImageCreate(t *testing.T) { name: "status condition Degraded=False on successful sync", initialObjects: objs( iri().finalizer(iriFinalizerName), - clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriRegistryCredentialsSecret(), pullSecret(), + clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), iriAuthSecret(), pullSecret(), machineconfigmaster(), machineconfigworker()), verify: func(t *testing.T, actualIRI *mcfgv1.InternalReleaseImage, actualMasterMC *mcfgv1.MachineConfig, actualWorkerMC *mcfgv1.MachineConfig) { assert.NotNil(t, actualIRI) @@ -217,6 +217,91 @@ func TestInternalReleaseImageStatusOnError(t *testing.T) { } } +func TestReconcileHtpasswd(t *testing.T) { + cases := []struct { + name string + password string + existingHtpasswd string + expectUpdate bool + }{ + { + name: "htpasswd already matches password, no update", + password: "mypassword", + existingHtpasswd: mustGenerateHtpasswd(t, "mypassword"), + expectUpdate: false, + }, + { + name: "htpasswd missing, generates new", + password: "mypassword", + existingHtpasswd: "", + expectUpdate: true, + }, + { + name: "password changed, regenerates htpasswd", + password: "newpassword", + existingHtpasswd: mustGenerateHtpasswd(t, "oldpassword"), + expectUpdate: true, + }, + } + + t.Run("empty password returns error", func(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.InternalReleaseImageAuthSecretName, + Namespace: ctrlcommon.MCONamespace, + }, + Data: map[string][]byte{ + "password": []byte(""), + }, + } + f := newFixture(t, []runtime.Object{secret}) + _, err := reconcileHtpasswd(f.k8sClient, secret) + assert.Error(t, err) + }) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.InternalReleaseImageAuthSecretName, + Namespace: ctrlcommon.MCONamespace, + }, + Data: map[string][]byte{ + "password": []byte(tc.password), + "htpasswd": []byte(tc.existingHtpasswd), + }, + } + + f := newFixture(t, []runtime.Object{secret}) + result, err := reconcileHtpasswd(f.k8sClient, secret) + assert.NoError(t, err) + + if tc.expectUpdate { + // Verify the returned secret has a valid htpasswd + assert.True(t, HtpasswdMatchesPassword(string(result.Data["htpasswd"]), ctrlcommon.IRIRegistryUsername, tc.password), + "updated htpasswd should match the password") + + // Verify the secret was updated in the API + updated, err := f.k8sClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get( + context.TODO(), ctrlcommon.InternalReleaseImageAuthSecretName, metav1.GetOptions{}) + assert.NoError(t, err) + assert.True(t, HtpasswdMatchesPassword(string(updated.Data["htpasswd"]), ctrlcommon.IRIRegistryUsername, tc.password), + "secret in API should have updated htpasswd") + } else { + // Verify the htpasswd was not changed + assert.Equal(t, tc.existingHtpasswd, string(result.Data["htpasswd"]), + "htpasswd should not change when already matching") + } + }) + } +} + +func mustGenerateHtpasswd(t *testing.T, password string) string { + t.Helper() + entry, err := generateHtpasswdEntry(ctrlcommon.IRIRegistryUsername, password) + assert.NoError(t, err) + return entry +} // The fixture used to setup and run the controller. type fixture struct { t *testing.T @@ -379,7 +464,7 @@ func TestAggregateIRIStatus(t *testing.T) { clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), - iriRegistryCredentialsSecret(), + iriAuthSecret(), pullSecret(), machineconfigmaster(), machineconfigworker(), @@ -414,7 +499,7 @@ func TestAggregateIRIStatus(t *testing.T) { clusterVersion(), cconfig().withDNS("example.com"), iriCertSecret(), - iriRegistryCredentialsSecret(), + iriAuthSecret(), pullSecret(), machineconfigmaster(), machineconfigworker(), diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go index f27e69eb99..f30f6b4cb6 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go @@ -34,7 +34,7 @@ func verifyInternalReleaseMasterMachineConfig(t *testing.T, mc *mcfgv1.MachineCo 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") + verifyIgnitionFileMatches(t, &ignCfg, "/etc/iri-registry/auth/htpasswd", ctrlcommon.IRIRegistryUsername, "testpassword") verifyIgnitionFileContains(t, &ignCfg, "/usr/local/bin/load-registry-image.sh", "docker-registry-image-pullspec") assert.Contains(t, *ignCfg.Systemd.Units[0].Contents, `REGISTRY_STORAGE_MAINTENANCE_READONLY={"enabled":true}`) assert.NotContains(t, *ignCfg.Systemd.Units[0].Contents, "REGISTRY_STORAGE_MAINTENANCE_READONLY_ENABLED") @@ -79,6 +79,16 @@ func verifyIgnitionFileContains(t *testing.T, ignCfg *ign3types.Config, path str assert.Contains(t, string(data), expectedContent, path) } +// verifyIgnitionFileMatches verifies that the ignition file at path contains a +// valid htpasswd entry matching the given username and password. +func verifyIgnitionFileMatches(t *testing.T, ignCfg *ign3types.Config, path, username, password string) { + t.Helper() + data, err := ctrlcommon.GetIgnitionFileDataByPath(ignCfg, path) + assert.NoError(t, err) + assert.True(t, HtpasswdMatchesPassword(string(data), username, password), + "htpasswd at %s should match %s:", path, username) +} + // objs is an helper func to improve the test readability. func objs(builders ...objBuilder) func() []runtime.Object { return func() []runtime.Object { @@ -254,7 +264,14 @@ func pullSecret() *secretBuilder { } } -func iriRegistryCredentialsSecret() *secretBuilder { +// iriAuthSecret returns an auth secret with a testpassword and pre-generated +// bcrypt htpasswd, suitable for both bootstrap and controller tests. +// The controller's reconcileHtpasswd will verify the htpasswd matches and leave it unchanged. +func iriAuthSecret() *secretBuilder { + htpasswd, err := generateHtpasswdEntry(ctrlcommon.IRIRegistryUsername, "testpassword") + if err != nil { + panic(err) + } return &secretBuilder{ obj: &corev1.Secret{ ObjectMeta: v1.ObjectMeta{ @@ -262,8 +279,8 @@ func iriRegistryCredentialsSecret() *secretBuilder { Name: ctrlcommon.InternalReleaseImageAuthSecretName, }, Data: map[string][]byte{ - "htpasswd": []byte("openshift:$2y$05$testhash"), "password": []byte("testpassword"), + "htpasswd": []byte(htpasswd), }, }, } diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go b/pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go new file mode 100644 index 0000000000..09f0683b50 --- /dev/null +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go @@ -0,0 +1,72 @@ +package internalreleaseimage + +import ( + "context" + "fmt" + "strings" + + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "golang.org/x/crypto/bcrypt" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clientset "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" +) + +// generateHtpasswdEntry generates an htpasswd-formatted line for the given username +// and password using bcrypt hashing. +func generateHtpasswdEntry(username, password string) (string, error) { + hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + return "", fmt.Errorf("failed to generate bcrypt hash: %w", err) + } + return fmt.Sprintf("%s:%s", username, string(hash)), nil +} + +// HtpasswdMatchesPassword reports whether the given htpasswd line matches +// the provided username and password. +func HtpasswdMatchesPassword(htpasswd, username, password string) bool { + prefix := username + ":" + if !strings.HasPrefix(htpasswd, prefix) { + return false + } + hash := []byte(strings.TrimPrefix(htpasswd, prefix)) + return bcrypt.CompareHashAndPassword(hash, []byte(password)) == nil +} + +// reconcileHtpasswd ensures the htpasswd field in the IRI auth secret is in +// sync with the password field. If the password has changed (or htpasswd is +// missing), it generates a new bcrypt hash and updates the secret. This is the +// trigger for single-phase credential rotation: the updated htpasswd causes the +// MachineConfig to be re-rendered, which MCDs roll out to nodes. Brief registry +// downtime during the rollout is accepted. +func reconcileHtpasswd(kubeClient clientset.Interface, authSecret *corev1.Secret) (*corev1.Secret, error) { + password := string(authSecret.Data["password"]) + if password == "" { + return nil, fmt.Errorf("IRI auth secret %s/%s missing or empty \"password\" field", authSecret.Namespace, authSecret.Name) + } + htpasswd := string(authSecret.Data["htpasswd"]) + + if HtpasswdMatchesPassword(htpasswd, ctrlcommon.IRIRegistryUsername, password) { + return authSecret, nil + } + + klog.V(4).Infof("IRI auth secret htpasswd is out of sync with password, regenerating") + + newHtpasswd, err := generateHtpasswdEntry(ctrlcommon.IRIRegistryUsername, password) + if err != nil { + return nil, fmt.Errorf("failed to generate htpasswd: %w", err) + } + + updated := authSecret.DeepCopy() + updated.Data["htpasswd"] = []byte(newHtpasswd) + + result, err := kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update( + context.TODO(), updated, metav1.UpdateOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to update IRI auth secret: %w", err) + } + + klog.Infof("Regenerated IRI auth secret htpasswd for credential rotation (secret %s/%s)", authSecret.Namespace, authSecret.Name) + return result, nil +} diff --git a/test/e2e-iri/iri_test.go b/test/e2e-iri/iri_test.go index 88647cafd8..ec7f70f276 100644 --- a/test/e2e-iri/iri_test.go +++ b/test/e2e-iri/iri_test.go @@ -23,6 +23,7 @@ import ( mcfgv1 "github.com/openshift/api/machineconfiguration/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + iripkg "github.com/openshift/machine-config-operator/pkg/controller/internalreleaseimage" "github.com/openshift/machine-config-operator/pkg/daemon/constants" "github.com/openshift/machine-config-operator/test/framework" "github.com/openshift/machine-config-operator/test/helpers" @@ -367,6 +368,86 @@ func TestIRIRegistry_UnauthenticatedReadSucceeds(t *testing.T) { require.Equal(t, "200", statusCode, "unauthenticated read request should succeed") } +func TestIRIAuth_CredentialRotation(t *testing.T) { + cs := framework.NewClientSet("") + ctx := context.Background() + + authSecret, err := cs.Secrets(ctrlcommon.MCONamespace).Get(ctx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if k8serrors.IsNotFound(err) { + t.Skip("IRI auth secret not found, authentication is not enabled") + } + require.NoError(t, err) + + originalPassword := string(authSecret.Data["password"]) + originalHtpasswd := string(authSecret.Data["htpasswd"]) + require.NotEmpty(t, originalPassword) + + baseDomain := getBaseDomain(t, cs) + + // Restore credentials on test completion. + t.Cleanup(func() { + cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) + defer cancel() + + secret, err := cs.Secrets(ctrlcommon.MCONamespace).Get(cleanupCtx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if err != nil { + t.Errorf("cleanup: failed to get auth secret: %v", err) + return + } + secret.Data["password"] = []byte(originalPassword) + secret.Data["htpasswd"] = []byte(originalHtpasswd) + if _, err := cs.Secrets(ctrlcommon.MCONamespace).Update(cleanupCtx, secret, v1.UpdateOptions{}); err != nil { + t.Errorf("cleanup: failed to restore auth secret: %v", err) + return + } + t.Logf("Cleanup: restored auth secret, waiting for MCP rollout...") + if err := helpers.WaitForPoolCompleteAny(t, cs, "master"); err != nil { + t.Errorf("cleanup: MCP rollout did not complete: %v", err) + } + t.Logf("Cleanup: credential restoration complete") + }) + + // Trigger rotation by writing a new password. + newPassword := fmt.Sprintf("rotated-%d", time.Now().UnixNano()) + authSecret.Data["password"] = []byte(newPassword) + delete(authSecret.Data, "htpasswd") // controller will regenerate + _, err = cs.Secrets(ctrlcommon.MCONamespace).Update(ctx, authSecret, v1.UpdateOptions{}) + require.NoError(t, err) + t.Logf("Updated auth secret password to trigger rotation") + + // Wait for the controller to regenerate htpasswd. + t.Logf("Waiting for controller to regenerate htpasswd...") + err = wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { + secret, err := cs.Secrets(ctrlcommon.MCONamespace).Get(ctx, ctrlcommon.InternalReleaseImageAuthSecretName, v1.GetOptions{}) + if err != nil { + return false, err + } + return iripkg.HtpasswdMatchesPassword(string(secret.Data["htpasswd"]), ctrlcommon.IRIRegistryUsername, newPassword), nil + }) + require.NoError(t, err, "timed out waiting for htpasswd regeneration") + t.Logf("Controller regenerated htpasswd") + + // Poll until the new credentials are accepted. The registry only accepts + // them once MCD has written the new htpasswd file to the node, so this + // also serves as the rollout completion check. + node := helpers.GetRandomNode(t, cs, "master") + newAuthHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername+":"+newPassword)) + oldAuthHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername+":"+originalPassword)) + + t.Logf("Waiting for new credentials to be accepted on node...") + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { + return curlIRIRegistry(t, cs, node, baseDomain, "-H", "Authorization: "+newAuthHeader) == "200", nil + }) + require.NoError(t, err, "timed out waiting for new credentials to be accepted after rotation") + t.Logf("New credentials accepted") + + statusCode := curlIRIRegistry(t, cs, node, baseDomain, "-H", "Authorization: "+oldAuthHeader) + require.Equal(t, "401", statusCode, "old credentials should be rejected after rotation") + t.Logf("Old credentials correctly rejected with %s", statusCode) + + t.Logf("Credential rotation completed successfully") +} + func TestIRIController_ShouldPreventDeletionWhenInUse(t *testing.T) { skipIfNoBaremetal(t) diff --git a/vendor/golang.org/x/crypto/bcrypt/base64.go b/vendor/golang.org/x/crypto/bcrypt/base64.go new file mode 100644 index 0000000000..fc31160908 --- /dev/null +++ b/vendor/golang.org/x/crypto/bcrypt/base64.go @@ -0,0 +1,35 @@ +// Copyright 2011 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package bcrypt + +import "encoding/base64" + +const alphabet = "./ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" + +var bcEncoding = base64.NewEncoding(alphabet) + +func base64Encode(src []byte) []byte { + n := bcEncoding.EncodedLen(len(src)) + dst := make([]byte, n) + bcEncoding.Encode(dst, src) + for dst[n-1] == '=' { + n-- + } + return dst[:n] +} + +func base64Decode(src []byte) ([]byte, error) { + numOfEquals := 4 - (len(src) % 4) + for i := 0; i < numOfEquals; i++ { + src = append(src, '=') + } + + dst := make([]byte, bcEncoding.DecodedLen(len(src))) + n, err := bcEncoding.Decode(dst, src) + if err != nil { + return nil, err + } + return dst[:n], nil +} diff --git a/vendor/golang.org/x/crypto/bcrypt/bcrypt.go b/vendor/golang.org/x/crypto/bcrypt/bcrypt.go new file mode 100644 index 0000000000..3e7f8df871 --- /dev/null +++ b/vendor/golang.org/x/crypto/bcrypt/bcrypt.go @@ -0,0 +1,304 @@ +// Copyright 2011 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package bcrypt implements Provos and Mazières's bcrypt adaptive hashing +// algorithm. See http://www.usenix.org/event/usenix99/provos/provos.pdf +package bcrypt + +// The code is a port of Provos and Mazières's C implementation. +import ( + "crypto/rand" + "crypto/subtle" + "errors" + "fmt" + "io" + "strconv" + + "golang.org/x/crypto/blowfish" +) + +const ( + MinCost int = 4 // the minimum allowable cost as passed in to GenerateFromPassword + MaxCost int = 31 // the maximum allowable cost as passed in to GenerateFromPassword + DefaultCost int = 10 // the cost that will actually be set if a cost below MinCost is passed into GenerateFromPassword +) + +// The error returned from CompareHashAndPassword when a password and hash do +// not match. +var ErrMismatchedHashAndPassword = errors.New("crypto/bcrypt: hashedPassword is not the hash of the given password") + +// The error returned from CompareHashAndPassword when a hash is too short to +// be a bcrypt hash. +var ErrHashTooShort = errors.New("crypto/bcrypt: hashedSecret too short to be a bcrypted password") + +// The error returned from CompareHashAndPassword when a hash was created with +// a bcrypt algorithm newer than this implementation. +type HashVersionTooNewError byte + +func (hv HashVersionTooNewError) Error() string { + return fmt.Sprintf("crypto/bcrypt: bcrypt algorithm version '%c' requested is newer than current version '%c'", byte(hv), majorVersion) +} + +// The error returned from CompareHashAndPassword when a hash starts with something other than '$' +type InvalidHashPrefixError byte + +func (ih InvalidHashPrefixError) Error() string { + return fmt.Sprintf("crypto/bcrypt: bcrypt hashes must start with '$', but hashedSecret started with '%c'", byte(ih)) +} + +type InvalidCostError int + +func (ic InvalidCostError) Error() string { + return fmt.Sprintf("crypto/bcrypt: cost %d is outside allowed inclusive range %d..%d", int(ic), MinCost, MaxCost) +} + +const ( + majorVersion = '2' + minorVersion = 'a' + maxSaltSize = 16 + maxCryptedHashSize = 23 + encodedSaltSize = 22 + encodedHashSize = 31 + minHashSize = 59 +) + +// magicCipherData is an IV for the 64 Blowfish encryption calls in +// bcrypt(). It's the string "OrpheanBeholderScryDoubt" in big-endian bytes. +var magicCipherData = []byte{ + 0x4f, 0x72, 0x70, 0x68, + 0x65, 0x61, 0x6e, 0x42, + 0x65, 0x68, 0x6f, 0x6c, + 0x64, 0x65, 0x72, 0x53, + 0x63, 0x72, 0x79, 0x44, + 0x6f, 0x75, 0x62, 0x74, +} + +type hashed struct { + hash []byte + salt []byte + cost int // allowed range is MinCost to MaxCost + major byte + minor byte +} + +// ErrPasswordTooLong is returned when the password passed to +// GenerateFromPassword is too long (i.e. > 72 bytes). +var ErrPasswordTooLong = errors.New("bcrypt: password length exceeds 72 bytes") + +// GenerateFromPassword returns the bcrypt hash of the password at the given +// cost. If the cost given is less than MinCost, the cost will be set to +// DefaultCost, instead. Use CompareHashAndPassword, as defined in this package, +// to compare the returned hashed password with its cleartext version. +// GenerateFromPassword does not accept passwords longer than 72 bytes, which +// is the longest password bcrypt will operate on. +func GenerateFromPassword(password []byte, cost int) ([]byte, error) { + if len(password) > 72 { + return nil, ErrPasswordTooLong + } + p, err := newFromPassword(password, cost) + if err != nil { + return nil, err + } + return p.Hash(), nil +} + +// CompareHashAndPassword compares a bcrypt hashed password with its possible +// plaintext equivalent. Returns nil on success, or an error on failure. +func CompareHashAndPassword(hashedPassword, password []byte) error { + p, err := newFromHash(hashedPassword) + if err != nil { + return err + } + + otherHash, err := bcrypt(password, p.cost, p.salt) + if err != nil { + return err + } + + otherP := &hashed{otherHash, p.salt, p.cost, p.major, p.minor} + if subtle.ConstantTimeCompare(p.Hash(), otherP.Hash()) == 1 { + return nil + } + + return ErrMismatchedHashAndPassword +} + +// Cost returns the hashing cost used to create the given hashed +// password. When, in the future, the hashing cost of a password system needs +// to be increased in order to adjust for greater computational power, this +// function allows one to establish which passwords need to be updated. +func Cost(hashedPassword []byte) (int, error) { + p, err := newFromHash(hashedPassword) + if err != nil { + return 0, err + } + return p.cost, nil +} + +func newFromPassword(password []byte, cost int) (*hashed, error) { + if cost < MinCost { + cost = DefaultCost + } + p := new(hashed) + p.major = majorVersion + p.minor = minorVersion + + err := checkCost(cost) + if err != nil { + return nil, err + } + p.cost = cost + + unencodedSalt := make([]byte, maxSaltSize) + _, err = io.ReadFull(rand.Reader, unencodedSalt) + if err != nil { + return nil, err + } + + p.salt = base64Encode(unencodedSalt) + hash, err := bcrypt(password, p.cost, p.salt) + if err != nil { + return nil, err + } + p.hash = hash + return p, err +} + +func newFromHash(hashedSecret []byte) (*hashed, error) { + if len(hashedSecret) < minHashSize { + return nil, ErrHashTooShort + } + p := new(hashed) + n, err := p.decodeVersion(hashedSecret) + if err != nil { + return nil, err + } + hashedSecret = hashedSecret[n:] + n, err = p.decodeCost(hashedSecret) + if err != nil { + return nil, err + } + hashedSecret = hashedSecret[n:] + + // The "+2" is here because we'll have to append at most 2 '=' to the salt + // when base64 decoding it in expensiveBlowfishSetup(). + p.salt = make([]byte, encodedSaltSize, encodedSaltSize+2) + copy(p.salt, hashedSecret[:encodedSaltSize]) + + hashedSecret = hashedSecret[encodedSaltSize:] + p.hash = make([]byte, len(hashedSecret)) + copy(p.hash, hashedSecret) + + return p, nil +} + +func bcrypt(password []byte, cost int, salt []byte) ([]byte, error) { + cipherData := make([]byte, len(magicCipherData)) + copy(cipherData, magicCipherData) + + c, err := expensiveBlowfishSetup(password, uint32(cost), salt) + if err != nil { + return nil, err + } + + for i := 0; i < 24; i += 8 { + for j := 0; j < 64; j++ { + c.Encrypt(cipherData[i:i+8], cipherData[i:i+8]) + } + } + + // Bug compatibility with C bcrypt implementations. We only encode 23 of + // the 24 bytes encrypted. + hsh := base64Encode(cipherData[:maxCryptedHashSize]) + return hsh, nil +} + +func expensiveBlowfishSetup(key []byte, cost uint32, salt []byte) (*blowfish.Cipher, error) { + csalt, err := base64Decode(salt) + if err != nil { + return nil, err + } + + // Bug compatibility with C bcrypt implementations. They use the trailing + // NULL in the key string during expansion. + // We copy the key to prevent changing the underlying array. + ckey := append(key[:len(key):len(key)], 0) + + c, err := blowfish.NewSaltedCipher(ckey, csalt) + if err != nil { + return nil, err + } + + var i, rounds uint64 + rounds = 1 << cost + for i = 0; i < rounds; i++ { + blowfish.ExpandKey(ckey, c) + blowfish.ExpandKey(csalt, c) + } + + return c, nil +} + +func (p *hashed) Hash() []byte { + arr := make([]byte, 60) + arr[0] = '$' + arr[1] = p.major + n := 2 + if p.minor != 0 { + arr[2] = p.minor + n = 3 + } + arr[n] = '$' + n++ + copy(arr[n:], []byte(fmt.Sprintf("%02d", p.cost))) + n += 2 + arr[n] = '$' + n++ + copy(arr[n:], p.salt) + n += encodedSaltSize + copy(arr[n:], p.hash) + n += encodedHashSize + return arr[:n] +} + +func (p *hashed) decodeVersion(sbytes []byte) (int, error) { + if sbytes[0] != '$' { + return -1, InvalidHashPrefixError(sbytes[0]) + } + if sbytes[1] > majorVersion { + return -1, HashVersionTooNewError(sbytes[1]) + } + p.major = sbytes[1] + n := 3 + if sbytes[2] != '$' { + p.minor = sbytes[2] + n++ + } + return n, nil +} + +// sbytes should begin where decodeVersion left off. +func (p *hashed) decodeCost(sbytes []byte) (int, error) { + cost, err := strconv.Atoi(string(sbytes[0:2])) + if err != nil { + return -1, err + } + err = checkCost(cost) + if err != nil { + return -1, err + } + p.cost = cost + return 3, nil +} + +func (p *hashed) String() string { + return fmt.Sprintf("&{hash: %#v, salt: %#v, cost: %d, major: %c, minor: %c}", string(p.hash), p.salt, p.cost, p.major, p.minor) +} + +func checkCost(cost int) error { + if cost < MinCost || cost > MaxCost { + return InvalidCostError(cost) + } + return nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 50799db436..ac76e30837 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2116,6 +2116,7 @@ go4.org/errorutil # golang.org/x/crypto v0.54.0 ## explicit; go 1.25.0 golang.org/x/crypto/argon2 +golang.org/x/crypto/bcrypt golang.org/x/crypto/blake2b golang.org/x/crypto/blowfish golang.org/x/crypto/cast5 From bdab43272d0e4383d10cabd88ac96c1e49d53de3 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Thu, 30 Apr 2026 09:13:14 +0800 Subject: [PATCH 2/7] AGENT-1449: Move auth token resolution into iriRegistry constructor Move readIRIAuthToken from a standalone function into a method on iriRegistry (readAuthToken), and have newIRIRegistry call it internally rather than requiring the caller to resolve credentials beforehand. newIRIRegistry now returns (*iriRegistry, error) and accepts an optional authTokenOverride used in tests; in production the override is always empty and the token is read from the kubelet auth file at construction time. The manager sync path shrinks from 7 lines to 3. Assisted-by: Claude Sonnet 4.6 --- .../internalreleaseimage_manager.go | 14 +++------- .../internalreleaseimage/iriregistry.go | 28 +++++++++++++------ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/pkg/daemon/internalreleaseimage/internalreleaseimage_manager.go b/pkg/daemon/internalreleaseimage/internalreleaseimage_manager.go index 2bcec6c11a..9083bdd15a 100644 --- a/pkg/daemon/internalreleaseimage/internalreleaseimage_manager.go +++ b/pkg/daemon/internalreleaseimage/internalreleaseimage_manager.go @@ -453,17 +453,11 @@ func (i *Manager) reclaimRegistryStorage() error { // getIRIRegistry creates and returns an IRI registry client. // Returns the registry and an error indicating whether the registry is reachable. func (i *Manager) getIRIRegistry() (*iriRegistry, error) { - authToken := i.authToken - if authToken == "" { - var err error - authToken, err = readIRIAuthToken(net.JoinHostPort(iriRegistryHost, fmt.Sprintf("%d", iriRegistryPort))) - if err != nil { - return nil, fmt.Errorf("could not read IRI auth token: %w", err) - } + iriReg, err := newIRIRegistry(i.nodeName, i.registryClient, i.authToken) + if err != nil { + return nil, fmt.Errorf("could not create IRI registry client: %w", err) } - - iriReg := newIRIRegistry(i.nodeName, i.registryClient, authToken) - err := iriReg.CheckLocalRegistry() + err = iriReg.CheckLocalRegistry() return iriReg, err } diff --git a/pkg/daemon/internalreleaseimage/iriregistry.go b/pkg/daemon/internalreleaseimage/iriregistry.go index 593f92e1a1..c3d1c65c8a 100644 --- a/pkg/daemon/internalreleaseimage/iriregistry.go +++ b/pkg/daemon/internalreleaseimage/iriregistry.go @@ -47,18 +47,30 @@ type registryErrorResponse struct { Errors []registryErrorCode `json:"errors"` } -func newIRIRegistry(nodeName string, client *http.Client, authToken string) *iriRegistry { - return &iriRegistry{ +// newIRIRegistry creates an iriRegistry. If authTokenOverride is non-empty it +// is used directly (for testing); otherwise the auth token is read from the +// kubelet auth file at construction time. +func newIRIRegistry(nodeName string, client *http.Client, authTokenOverride string) (*iriRegistry, error) { + r := &iriRegistry{ nodeName: nodeName, client: client, registryHostPort: net.JoinHostPort(iriRegistryHost, fmt.Sprintf("%d", iriRegistryPort)), - authToken: authToken, } + if authTokenOverride != "" { + r.authToken = authTokenOverride + return r, nil + } + authToken, err := r.readAuthToken() + if err != nil { + return nil, err + } + r.authToken = authToken + return r, nil } -// readIRIAuthToken reads the base64-encoded auth token for the IRI registry -// from the kubelet auth file (/var/lib/kubelet/config.json). -func readIRIAuthToken(registryHostPort string) (string, error) { +// readAuthToken reads the base64-encoded auth token for this registry from the +// kubelet auth file (/var/lib/kubelet/config.json). +func (r *iriRegistry) readAuthToken() (string, error) { data, err := os.ReadFile(constants.KubeletAuthFile) if err != nil { return "", fmt.Errorf("could not read %s for IRI registry auth: %w", constants.KubeletAuthFile, err) @@ -73,10 +85,10 @@ func readIRIAuthToken(registryHostPort string) (string, error) { return "", fmt.Errorf("could not parse %s for IRI registry auth: %w", constants.KubeletAuthFile, err) } - if entry, ok := dockerConfig.Auths[registryHostPort]; ok && entry.Auth != "" { + if entry, ok := dockerConfig.Auths[r.registryHostPort]; ok && entry.Auth != "" { return entry.Auth, nil } - return "", fmt.Errorf("no auth entry found for %s in %s", registryHostPort, constants.KubeletAuthFile) + return "", fmt.Errorf("no auth entry found for %s in %s", r.registryHostPort, constants.KubeletAuthFile) } func (r *iriRegistry) query(endpoint string, headers ...map[string]string) (*http.Response, error) { From ac9703621b1716108ac11d53288d2e1dd52fa4dc Mon Sep 17 00:00:00 2001 From: Richard Su Date: Thu, 30 Apr 2026 09:18:37 +0800 Subject: [PATCH 3/7] AGENT-1449: Filter secret events by namespace and use authSecret.Namespace - addSecret/updateSecret now check namespace (MCONamespace) before name, preventing same-name secrets in other namespaces from triggering noisy IRI requeues - reconcileHtpasswd uses authSecret.Namespace instead of the hardcoded MCONamespace constant when updating the secret Assisted-by: Claude Sonnet 4.6 --- .../internalreleaseimage_controller.go | 10 ++++++---- .../internalreleaseimage_registry_auth.go | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go index 9ebeacdf09..5688b624ec 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go @@ -315,8 +315,9 @@ func (ctrl *Controller) processMachineConfigEvent(obj interface{}, logMsg string func (ctrl *Controller) addSecret(obj interface{}, _ bool) { secret := obj.(*corev1.Secret) - if secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName && - secret.Name != ctrlcommon.InternalReleaseImageAuthSecretName { + if secret.Namespace != ctrlcommon.MCONamespace || + (secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName && + secret.Name != ctrlcommon.InternalReleaseImageAuthSecretName) { return } klog.V(4).Infof("Secret %s added, re-queuing IRI sync", secret.Name) @@ -326,8 +327,9 @@ func (ctrl *Controller) addSecret(obj interface{}, _ bool) { func (ctrl *Controller) updateSecret(_, cur interface{}) { secret := cur.(*corev1.Secret) - if secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName && - secret.Name != ctrlcommon.InternalReleaseImageAuthSecretName { + if secret.Namespace != ctrlcommon.MCONamespace || + (secret.Name != ctrlcommon.InternalReleaseImageTLSSecretName && + secret.Name != ctrlcommon.InternalReleaseImageAuthSecretName) { return } diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go b/pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go index 09f0683b50..b1158f378b 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_registry_auth.go @@ -61,7 +61,7 @@ func reconcileHtpasswd(kubeClient clientset.Interface, authSecret *corev1.Secret updated := authSecret.DeepCopy() updated.Data["htpasswd"] = []byte(newHtpasswd) - result, err := kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update( + result, err := kubeClient.CoreV1().Secrets(authSecret.Namespace).Update( context.TODO(), updated, metav1.UpdateOptions{}) if err != nil { return nil, fmt.Errorf("failed to update IRI auth secret: %w", err) From 071152181648183e5e4f2ed1e9b4d18c90d04f02 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Thu, 30 Apr 2026 09:22:33 +0800 Subject: [PATCH 4/7] AGENT-1449: Use feature gate check instead of nil-informer check in template controller Replace if iriSecretsInformer != nil / if iriInformer != nil guards with fgHandler.Enabled(FeatureGateNoRegistryClusterInstall) checks, making the intent explicit: IRI event handlers and the merger are only wired when the feature gate is on, not as a side-effect of nil informers being passed. The nil-informer approach in start.go is preserved as it correctly prevents the informers from starting on clusters where the CRD is not installed. Assisted-by: Claude Sonnet 4.6 --- pkg/controller/template/template_controller.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/controller/template/template_controller.go b/pkg/controller/template/template_controller.go index 3cdf070f49..02af3e642d 100644 --- a/pkg/controller/template/template_controller.go +++ b/pkg/controller/template/template_controller.go @@ -139,8 +139,6 @@ func New( // Watch the IRI auth secret in the MCO namespace so that when credentials // are rotated the pull secret rendered into 00-master/00-worker is updated. - // Both informers are nil when the NoRegistryClusterInstall feature gate is - // off (the CRD doesn't exist on those clusters). if iriSecretsInformer != nil { iriSecretsInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: ctrl.addSecret, From 565dd42df4e96ebd87c16d5440df6bc57b03198b Mon Sep 17 00:00:00 2001 From: Richard Su Date: Thu, 30 Apr 2026 10:00:06 +0800 Subject: [PATCH 5/7] AGENT-1449: Add pod-based pull verification to rotation e2e test Add verifyCanPullFromIRI helper that creates a pod with imagePullPolicy:Always using the IRI release image (pulled from the local IRI registry, not quay.io) and verifies the kubelet can authenticate and pull it. This exercises the full kubelet credential lookup path (/var/lib/kubelet/config.json) rather than just raw HTTP auth via curl exec. Add getIRIReleasePullSpec helper that queries /v2/openshift/release-images/tags/list on the IRI registry and constructs the local pullspec (api-int.:22625/openshift/release-images:). Add pre-rotation and post-rotation pull checks to TestIRIAuth_CredentialRotation. The existing curlIRIRegistry checks are retained for old-credential rejection verification. Assisted-by: Claude Sonnet 4.6 --- test/e2e-iri/iri_test.go | 103 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/test/e2e-iri/iri_test.go b/test/e2e-iri/iri_test.go index ec7f70f276..1b1d74e0d0 100644 --- a/test/e2e-iri/iri_test.go +++ b/test/e2e-iri/iri_test.go @@ -7,6 +7,8 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/base64" + "encoding/json" "encoding/pem" "fmt" "net/http" @@ -356,6 +358,15 @@ func getBaseDomain(t *testing.T, cs *framework.ClientSet) string { return cconfig.Spec.DNS.Spec.BaseDomain } +func curlIRIRegistry(t *testing.T, cs *framework.ClientSet, node corev1.Node, baseDomain string, extraArgs ...string) string { + t.Helper() + url := fmt.Sprintf("https://api-int.%s:%d/v2/", baseDomain, ctrlcommon.IRIRegistryPort) + args := []string{"curl", "-s", "--cacert", iriRootCAPath, "-o", "/dev/null", "-w", "%{http_code}"} + args = append(args, extraArgs...) + args = append(args, url) + return strings.TrimSpace(helpers.ExecCmdOnNode(t, cs, node, args...)) +} + func TestIRIRegistry_UnauthenticatedReadSucceeds(t *testing.T) { cs := framework.NewClientSet("") @@ -368,6 +379,77 @@ func TestIRIRegistry_UnauthenticatedReadSucceeds(t *testing.T) { require.Equal(t, "200", statusCode, "unauthenticated read request should succeed") } +// getIRIReleasePullSpec queries the IRI registry's release-images tags list and +// returns a pullspec of the form api-int.:/openshift/release-images@sha256:. +func getIRIReleasePullSpec(t *testing.T, cs *framework.ClientSet, node corev1.Node, baseDomain, password string) string { + t.Helper() + const iriRootCAPath = "/rootfs/etc/pki/ca-trust/source/anchors/iri-root-ca.crt" + authHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername+":"+password)) + url := fmt.Sprintf("https://api-int.%s:%d/v2/openshift/release-images/tags/list", baseDomain, ctrlcommon.IRIRegistryPort) + body := strings.TrimSpace(helpers.ExecCmdOnNode(t, cs, node, + "curl", "-s", "--cacert", iriRootCAPath, "-H", "Authorization: "+authHeader, url)) + + var tagsResp struct { + Tags []string `json:"tags"` + } + require.NoError(t, json.Unmarshal([]byte(body), &tagsResp), "failed to parse IRI tags list response: %s", body) + require.NotEmpty(t, tagsResp.Tags, "IRI release-images repository has no tags") + + return fmt.Sprintf("api-int.%s:%d/openshift/release-images:%s", baseDomain, ctrlcommon.IRIRegistryPort, tagsResp.Tags[0]) +} + +// verifyCanPullFromIRI creates a pod with imagePullPolicy:Always using the +// given IRI registry image and verifies the kubelet can authenticate and pull +// it. Returns nil when the image is pulled successfully (pod reaches Running, +// Succeeded, or Failed phase — all indicate auth succeeded). Returns an error +// if the pull fails due to authentication (ImagePullBackOff/ErrImagePull). +func verifyCanPullFromIRI(t *testing.T, cs *framework.ClientSet, ctx context.Context, imageRef, podName string) error { + t.Helper() + pod := &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: podName, + Namespace: ctrlcommon.MCONamespace, + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{ + { + Name: "iri-pull-test", + Image: imageRef, + ImagePullPolicy: corev1.PullAlways, + // The command may fail if the image has no shell; that is + // acceptable — we only care that the image was pulled. + Command: []string{"sh", "-c", "exit 0"}, + }, + }, + }, + } + if _, err := cs.Pods(ctrlcommon.MCONamespace).Create(ctx, pod, v1.CreateOptions{}); err != nil { + return fmt.Errorf("failed to create pull-test pod: %w", err) + } + defer cs.Pods(ctrlcommon.MCONamespace).Delete(context.Background(), podName, v1.DeleteOptions{}) + + return wait.PollUntilContextTimeout(ctx, 5*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { + p, err := cs.Pods(ctrlcommon.MCONamespace).Get(ctx, podName, v1.GetOptions{}) + if err != nil { + return false, err + } + for _, cs := range p.Status.ContainerStatuses { + if cs.State.Waiting != nil { + switch cs.State.Waiting.Reason { + case "ImagePullBackOff", "ErrImagePull": + return false, fmt.Errorf("IRI image pull failed (%s): auth rejected", cs.State.Waiting.Reason) + } + } + } + switch p.Status.Phase { + case corev1.PodRunning, corev1.PodSucceeded, corev1.PodFailed: + return true, nil // image was pulled; command result is irrelevant + } + return false, nil + }) +} + func TestIRIAuth_CredentialRotation(t *testing.T) { cs := framework.NewClientSet("") ctx := context.Background() @@ -384,6 +466,12 @@ func TestIRIAuth_CredentialRotation(t *testing.T) { baseDomain := getBaseDomain(t, cs) + // Get the IRI release image pullspec for pod-based pull verification. + // The pullspec is constructed from the registry's tags list so it references + // the local IRI registry, not the original quay.io source. + node := helpers.GetRandomNode(t, cs, "master") + iriImageRef := getIRIReleasePullSpec(t, cs, node, baseDomain, originalPassword) + // Restore credentials on test completion. t.Cleanup(func() { cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) @@ -407,6 +495,12 @@ func TestIRIAuth_CredentialRotation(t *testing.T) { t.Logf("Cleanup: credential restoration complete") }) + // Verify kubelet can pull from the IRI registry with the current credentials. + t.Logf("Verifying kubelet can pull from IRI registry before rotation...") + require.NoError(t, verifyCanPullFromIRI(t, cs, ctx, iriImageRef, "iri-pull-pre-rotation"), + "kubelet should be able to pull from IRI registry before credential rotation") + t.Logf("Pre-rotation pull verified") + // Trigger rotation by writing a new password. newPassword := fmt.Sprintf("rotated-%d", time.Now().UnixNano()) authSecret.Data["password"] = []byte(newPassword) @@ -430,7 +524,6 @@ func TestIRIAuth_CredentialRotation(t *testing.T) { // Poll until the new credentials are accepted. The registry only accepts // them once MCD has written the new htpasswd file to the node, so this // also serves as the rollout completion check. - node := helpers.GetRandomNode(t, cs, "master") newAuthHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername+":"+newPassword)) oldAuthHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername+":"+originalPassword)) @@ -445,6 +538,14 @@ func TestIRIAuth_CredentialRotation(t *testing.T) { require.Equal(t, "401", statusCode, "old credentials should be rejected after rotation") t.Logf("Old credentials correctly rejected with %s", statusCode) + // Verify kubelet can still pull from the IRI registry with the new credentials. + // This exercises the full kubelet pull path (/var/lib/kubelet/config.json) rather + // than just raw HTTP auth. + t.Logf("Verifying kubelet can pull from IRI registry after rotation...") + require.NoError(t, verifyCanPullFromIRI(t, cs, ctx, iriImageRef, "iri-pull-post-rotation"), + "kubelet should be able to pull from IRI registry after credential rotation") + t.Logf("Post-rotation pull verified") + t.Logf("Credential rotation completed successfully") } From cf50618c808c3fbbe16a461f7698d79b05946a81 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Thu, 30 Apr 2026 14:03:18 +0800 Subject: [PATCH 6/7] AGENT-1449: Wait for full master pool rollout before asserting old credentials rejected The old-credential 401 check after rotation was running immediately after observing a single api-int 200 from curlIRIRegistry. Since api-int is a VIP that load-balances across masters, this only proved one backend had the new htpasswd; the 401 probe could land on an unrotated master and return 200. Wait for WaitForPoolCompleteAny("master") before the old-credential assertion to ensure all masters have applied the new htpasswd before we check rejection. Assisted-by: Claude Sonnet 4.6 --- test/e2e-iri/iri_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e-iri/iri_test.go b/test/e2e-iri/iri_test.go index 1b1d74e0d0..fa15097117 100644 --- a/test/e2e-iri/iri_test.go +++ b/test/e2e-iri/iri_test.go @@ -534,6 +534,14 @@ func TestIRIAuth_CredentialRotation(t *testing.T) { require.NoError(t, err, "timed out waiting for new credentials to be accepted after rotation") t.Logf("New credentials accepted") + // Wait for the full master pool rollout to complete before asserting old + // credentials are rejected. The curlIRIRegistry check above only proves one + // backend accepted the new htpasswd; without waiting for pool completion the + // old-credential probe could land on an unrotated master via the api-int VIP. + t.Logf("Waiting for master pool rollout to complete...") + require.NoError(t, helpers.WaitForPoolCompleteAny(t, cs, "master"), "master pool rollout did not complete after credential rotation") + t.Logf("Master pool rollout complete") + statusCode := curlIRIRegistry(t, cs, node, baseDomain, "-H", "Authorization: "+oldAuthHeader) require.Equal(t, "401", statusCode, "old credentials should be rejected after rotation") t.Logf("Old credentials correctly rejected with %s", statusCode) From 110ca750ff16472bf31b343ad0d2ec1d0600dfad Mon Sep 17 00:00:00 2001 From: Richard Su Date: Thu, 30 Apr 2026 22:18:59 +0800 Subject: [PATCH 7/7] AGENT-1449: Fix post-rotation kubelet pull check in e2e test Three fixes for reliability of the post-rotation verifyCanPullFromIRI check: 1. Retry getIRIReleasePullSpec until tags are available. After credential restores the IRI registry can take a moment to stabilize; querying tags immediately can return an empty list causing a spurious test failure. 2. Wait for /var/lib/kubelet/config.json to contain the new IRI credentials before creating the pull-test pod. Credential rotation triggers two sequential MC rollouts (02-master for htpasswd, 00-master for pull secret); WaitForPoolCompleteAny returns after the first, so without this wait the pod is created before the pull secret is updated. 3. Retry the pull-test pod if it hits ImagePullBackOff. CRI-O can cache authentication failures briefly; deleting and recreating the pod forces a fresh authentication attempt with the updated credentials. Assisted-by: Claude Sonnet 4.6 --- test/e2e-iri/iri_test.go | 137 ++++++++++++++++++++++++++++----------- 1 file changed, 99 insertions(+), 38 deletions(-) diff --git a/test/e2e-iri/iri_test.go b/test/e2e-iri/iri_test.go index fa15097117..a7c5132b30 100644 --- a/test/e2e-iri/iri_test.go +++ b/test/e2e-iri/iri_test.go @@ -10,6 +10,7 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" + "errors" "fmt" "net/http" "reflect" @@ -380,71 +381,109 @@ func TestIRIRegistry_UnauthenticatedReadSucceeds(t *testing.T) { } // getIRIReleasePullSpec queries the IRI registry's release-images tags list and -// returns a pullspec of the form api-int.:/openshift/release-images@sha256:. +// returns a pullspec of the form api-int.:/openshift/release-images:. +// It retries until tags are available to handle brief registry stabilization delays +// after credential restores or pool rollouts. func getIRIReleasePullSpec(t *testing.T, cs *framework.ClientSet, node corev1.Node, baseDomain, password string) string { t.Helper() const iriRootCAPath = "/rootfs/etc/pki/ca-trust/source/anchors/iri-root-ca.crt" authHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername+":"+password)) url := fmt.Sprintf("https://api-int.%s:%d/v2/openshift/release-images/tags/list", baseDomain, ctrlcommon.IRIRegistryPort) - body := strings.TrimSpace(helpers.ExecCmdOnNode(t, cs, node, - "curl", "-s", "--cacert", iriRootCAPath, "-H", "Authorization: "+authHeader, url)) - var tagsResp struct { - Tags []string `json:"tags"` - } - require.NoError(t, json.Unmarshal([]byte(body), &tagsResp), "failed to parse IRI tags list response: %s", body) - require.NotEmpty(t, tagsResp.Tags, "IRI release-images repository has no tags") + var tag string + err := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { + body := strings.TrimSpace(helpers.ExecCmdOnNode(t, cs, node, + "curl", "-s", "--cacert", iriRootCAPath, "-H", "Authorization: "+authHeader, url)) + var tagsResp struct { + Tags []string `json:"tags"` + } + if err := json.Unmarshal([]byte(body), &tagsResp); err != nil || len(tagsResp.Tags) == 0 { + return false, nil + } + tag = tagsResp.Tags[0] + return true, nil + }) + require.NoError(t, err, "timed out waiting for IRI release-images tags to be available") - return fmt.Sprintf("api-int.%s:%d/openshift/release-images:%s", baseDomain, ctrlcommon.IRIRegistryPort, tagsResp.Tags[0]) + return fmt.Sprintf("api-int.%s:%d/openshift/release-images:%s", baseDomain, ctrlcommon.IRIRegistryPort, tag) } // verifyCanPullFromIRI creates a pod with imagePullPolicy:Always using the // given IRI registry image and verifies the kubelet can authenticate and pull // it. Returns nil when the image is pulled successfully (pod reaches Running, -// Succeeded, or Failed phase — all indicate auth succeeded). Returns an error -// if the pull fails due to authentication (ImagePullBackOff/ErrImagePull). +// Succeeded, or Failed phase — all indicate auth succeeded). If the pod hits +// ImagePullBackOff (CRI-O may cache auth failures briefly), it is deleted and +// recreated to force a fresh authentication attempt. func verifyCanPullFromIRI(t *testing.T, cs *framework.ClientSet, ctx context.Context, imageRef, podName string) error { t.Helper() - pod := &corev1.Pod{ - ObjectMeta: v1.ObjectMeta{ - Name: podName, - Namespace: ctrlcommon.MCONamespace, - }, - Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyNever, - Containers: []corev1.Container{ - { - Name: "iri-pull-test", - Image: imageRef, - ImagePullPolicy: corev1.PullAlways, - // The command may fail if the image has no shell; that is - // acceptable — we only care that the image was pulled. - Command: []string{"sh", "-c", "exit 0"}, + newPod := func(name string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + Namespace: ctrlcommon.MCONamespace, + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{ + { + Name: "iri-pull-test", + Image: imageRef, + ImagePullPolicy: corev1.PullAlways, + // The command may fail if the image has no shell; that is + // acceptable — we only care that the image was pulled. + Command: []string{"sh", "-c", "exit 0"}, + }, }, }, - }, - } - if _, err := cs.Pods(ctrlcommon.MCONamespace).Create(ctx, pod, v1.CreateOptions{}); err != nil { - return fmt.Errorf("failed to create pull-test pod: %w", err) + } } - defer cs.Pods(ctrlcommon.MCONamespace).Delete(context.Background(), podName, v1.DeleteOptions{}) + attempt := 0 return wait.PollUntilContextTimeout(ctx, 5*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { + name := fmt.Sprintf("%s-%d", podName, attempt) + attempt++ + if _, err := cs.Pods(ctrlcommon.MCONamespace).Create(ctx, newPod(name), v1.CreateOptions{}); err != nil { + return false, fmt.Errorf("failed to create pull-test pod: %w", err) + } + defer cs.Pods(ctrlcommon.MCONamespace).Delete(context.Background(), name, v1.DeleteOptions{}) + + err := waitForPodImagePull(ctx, cs, name) + if errors.Is(err, errImagePullBackOff) { + // CRI-O cached the auth failure — outer loop retries with a new pod. + return false, nil + } + if err != nil { + return false, err + } + return true, nil + }) +} + +// errImagePullBackOff is returned by waitForPodImagePull when the pod hits +// ImagePullBackOff or ErrImagePull, indicating a cached auth failure. +var errImagePullBackOff = errors.New("ImagePullBackOff") + +// waitForPodImagePull polls the given pod until its image is pulled (pod +// reaches Running/Succeeded/Failed) or returns errImagePullBackOff if CRI-O +// rejects the pull due to a cached auth failure. +func waitForPodImagePull(ctx context.Context, cs *framework.ClientSet, podName string) error { + return wait.PollUntilContextTimeout(ctx, 3*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) { p, err := cs.Pods(ctrlcommon.MCONamespace).Get(ctx, podName, v1.GetOptions{}) if err != nil { return false, err } - for _, cs := range p.Status.ContainerStatuses { - if cs.State.Waiting != nil { - switch cs.State.Waiting.Reason { - case "ImagePullBackOff", "ErrImagePull": - return false, fmt.Errorf("IRI image pull failed (%s): auth rejected", cs.State.Waiting.Reason) - } + for _, s := range p.Status.ContainerStatuses { + if s.State.Waiting == nil { + continue + } + switch s.State.Waiting.Reason { + case "ImagePullBackOff", "ErrImagePull": + return false, errImagePullBackOff } } switch p.Status.Phase { case corev1.PodRunning, corev1.PodSucceeded, corev1.PodFailed: - return true, nil // image was pulled; command result is irrelevant + return true, nil } return false, nil }) @@ -546,6 +585,28 @@ func TestIRIAuth_CredentialRotation(t *testing.T) { require.Equal(t, "401", statusCode, "old credentials should be rejected after rotation") t.Logf("Old credentials correctly rejected with %s", statusCode) + // Wait for the template controller to re-render 00-master with the new pull + // secret credentials and for MCD to apply it. Rotation triggers two sequential + // MC rollouts: first 02-master (htpasswd), then 00-master (pull secret). + // WaitForPoolCompleteAny returns after the first; we need the second to complete + // before the kubelet pull check so /var/lib/kubelet/config.json is updated. + t.Logf("Waiting for pull secret credentials to be updated on node...") + iriHost := fmt.Sprintf("api-int.%s:%d", baseDomain, ctrlcommon.IRIRegistryPort) + newAuthB64 := base64.StdEncoding.EncodeToString([]byte(ctrlcommon.IRIRegistryUsername + ":" + newPassword)) + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 10*time.Minute, true, func(ctx context.Context) (bool, error) { + out := strings.TrimSpace(helpers.ExecCmdOnNode(t, cs, node, + "cat", "/rootfs/var/lib/kubelet/config.json")) + var cfg map[string]interface{} + if jsonErr := json.Unmarshal([]byte(out), &cfg); jsonErr != nil { + return false, nil + } + auths, _ := cfg["auths"].(map[string]interface{}) + entry, _ := auths[iriHost].(map[string]interface{}) + return entry["auth"] == newAuthB64, nil + }) + require.NoError(t, err, "timed out waiting for kubelet config.json to be updated with new IRI credentials") + t.Logf("Kubelet config.json updated with new credentials") + // Verify kubelet can still pull from the IRI registry with the new credentials. // This exercises the full kubelet pull path (/var/lib/kubelet/config.json) rather // than just raw HTTP auth.