AGENT-1449: Add single-phase IRI registry credential rotation

pkg/apihelpers/apihelpers.go

@@ -32,6 +32,16 @@ var defaultClusterPolicies = opv1.NodeDisruptionPolicyClusterStatus{
 				},
 			},
 		},
+		{
+			// 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{

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])

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

@@ -51,9 +51,10 @@ var updateBackoff = wait.Backoff{
 // Controller defines the InternalReleaseImage controller.
 type Controller struct {
 	client        mcfgclientset.Interface
+	kubeClient    clientset.Interface
 	eventRecorder record.EventRecorder
 
-	syncHandler func(mcp string) error
+	syncHandler func(ctx context.Context, mcp string) error
 
 	iriLister       mcfglistersv1.InternalReleaseImageLister
 	iriListerSynced cache.InformerSynced
@@ -101,6 +102,7 @@ func New(
 
 	ctrl := &Controller{
 		client:        mcfgClient,
+		kubeClient:    kubeClient,
 		eventRecorder: ctrlcommon.NamespacedEventRecorder(eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "machineconfigcontroller-internalreleaseimagecontroller"})),
 		queue: workqueue.NewTypedRateLimitingQueueWithConfig(
 			workqueue.DefaultTypedControllerRateLimiter[string](),
@@ -188,27 +190,27 @@ func (ctrl *Controller) Run(ctx context.Context, workers int) {
 	defer klog.Info("Shutting down MachineConfigController-InternalReleaseImageController")
 
 	for i := 0; i < workers; i++ {
-		go wait.Until(ctrl.worker, time.Second, ctx.Done())
+		go wait.Until(func() { ctrl.worker(ctx) }, time.Second, ctx.Done())
 	}
 
 	<-ctx.Done()
 }
 
 // worker runs a worker thread that just dequeues items, processes them, and marks them done.
 // It enforces that the syncHandler is never invoked concurrently with the same key.
-func (ctrl *Controller) worker() {
-	for ctrl.processNextWorkItem() {
+func (ctrl *Controller) worker(ctx context.Context) {
+	for ctrl.processNextWorkItem(ctx) {
 	}
 }
 
-func (ctrl *Controller) processNextWorkItem() bool {
+func (ctrl *Controller) processNextWorkItem(ctx context.Context) bool {
 	key, quit := ctrl.queue.Get()
 	if quit {
 		return false
 	}
 	defer ctrl.queue.Done(key)
 
-	err := ctrl.syncHandler(key)
+	err := ctrl.syncHandler(ctx, key)
 	ctrl.handleErr(err, key)
 
 	return true
@@ -311,8 +313,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)
@@ -322,8 +325,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
 	}
 
@@ -472,7 +476,7 @@ func (ctrl *Controller) enqueueInternalReleaseImage() {
 // syncInternalReleaseImage will sync the InternalReleaseImage with the given key.
 // This function is not meant to be invoked concurrently with the same key.
 // nolint: gocyclo
-func (ctrl *Controller) syncInternalReleaseImage(key string) (syncErr error) {
+func (ctrl *Controller) syncInternalReleaseImage(ctx context.Context, key string) (syncErr error) {
 	startTime := time.Now()
 	klog.V(4).Infof("Started syncing InternalReleaseImage %q (%v)", key, startTime)
 	defer func() {
@@ -535,6 +539,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(ctx, 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)
 

pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go

@@ -12,6 +12,7 @@ import (
 	mcfginformers "github.com/openshift/client-go/machineconfiguration/informers/externalversions"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -42,7 +43,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 +53,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 +65,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 +75,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 +87,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 +110,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 +218,92 @@ 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(context.Background(), 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(context.Background(), 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)
+	require.NoError(t, err)
+	return entry
+}
+
 // The fixture used to setup and run the controller.
 type fixture struct {
 	t *testing.T
@@ -358,7 +445,7 @@ func (f *fixture) run(key string) {
 }
 
 func (f *fixture) runController(key string, expectError bool) {
-	err := f.controller.syncHandler(key)
+	err := f.controller.syncHandler(context.Background(), key)
 	if !expectError && err != nil {
 		f.t.Errorf("error syncing internalreleaseimage: %v", err)
 	} else if expectError && err == nil {
@@ -379,7 +466,7 @@ func TestAggregateIRIStatus(t *testing.T) {
 				clusterVersion(),
 				cconfig().withDNS("example.com"),
 				iriCertSecret(),
-				iriRegistryCredentialsSecret(),
+				iriAuthSecret(),
 				pullSecret(),
 				machineconfigmaster(),
 				machineconfigworker(),
@@ -414,7 +501,7 @@ func TestAggregateIRIStatus(t *testing.T) {
 				clusterVersion(),
 				cconfig().withDNS("example.com"),
 				iriCertSecret(),
-				iriRegistryCredentialsSecret(),
+				iriAuthSecret(),
 				pullSecret(),
 				machineconfigmaster(),
 				machineconfigworker(),

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:<password>", path, username)
+}
+
 // objs is an helper func to improve the test readability.
 func objs(builders ...objBuilder) func() []runtime.Object {
 	return func() []runtime.Object {
@@ -254,16 +264,23 @@ 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{
 				Namespace: ctrlcommon.MCONamespace,
 				Name:      ctrlcommon.InternalReleaseImageAuthSecretName,
 			},
 			Data: map[string][]byte{
-				"htpasswd": []byte("openshift:$2y$05$testhash"),
 				"password": []byte("testpassword"),
+				"htpasswd": []byte(htpasswd),
 			},
 		},
 	}