From c30480e65c96eb84c069a1436c0f6ad2973e5066 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Fri, 13 Mar 2026 15:52:35 -0500 Subject: [PATCH 1/2] AGENT-1449: Add IRI registry credential rotation support Implement safe credential rotation for the IRI registry using a desired-vs-current pattern with generation-numbered usernames. The auth secret holds the desired password; the pull secret (read from rendered MachineConfig) holds the deployed password. When they differ, a three-phase rotation is performed: 1. Deploy dual htpasswd (old + new credentials with different usernames) 2. Update pull secret after all MCPs finish rolling out 3. Clean up dual htpasswd to single entry after new pull secret is deployed This avoids authentication deadlocks during rolling MachineConfig updates because the pull secret always contains the old credentials, which are present in every version of the htpasswd. Mid-rotation password changes are handled by verifying htpasswd hashes with bcrypt.CompareHashAndPassword and regenerating if they don't match. Key changes: - Add MachineConfigPool lister/informer to IRI controller - Add reconcileAuthCredentials with three-case rotation logic - Add getDeployedIRICredentials (reads from rendered MC, not API) - Add areAllPoolsUpdated (checks all pools including workers) - Add HtpasswdHasValidEntry, GenerateHtpasswdEntry, GenerateDualHtpasswd, NextIRIUsername, ExtractIRICredentialsFromPullSecret helpers - Vendor golang.org/x/crypto/bcrypt for htpasswd hash generation - Add credential rotation design doc Assisted-by: Claude Opus 4.6 --- cmd/machine-config-controller/start.go | 1 + docs/IRIAuthCredentialRotation.md | 218 +++++++++++++ .../internalreleaseimage_controller.go | 279 +++++++++++++++- .../internalreleaseimage_controller_test.go | 282 +++++++++++++++- .../internalreleaseimage_helpers_test.go | 95 ++++++ .../internalreleaseimage/pullsecret.go | 135 +++++++- .../internalreleaseimage/pullsecret_test.go | 161 +++++++++- 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, 1497 insertions(+), 14 deletions(-) create mode 100644 docs/IRIAuthCredentialRotation.md 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/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index a7fc040288..a651614ad8 100644 --- a/cmd/machine-config-controller/start.go +++ b/cmd/machine-config-controller/start.go @@ -143,6 +143,7 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx.InformerFactory.Machineconfiguration().V1alpha1().InternalReleaseImages(), ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(), ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(), + ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(), ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(), ctrlctx.KubeInformerFactory.Core().V1().Secrets(), ctrlctx.ClientBuilder.KubeClientOrDie("internalreleaseimage-controller"), diff --git a/docs/IRIAuthCredentialRotation.md b/docs/IRIAuthCredentialRotation.md new file mode 100644 index 0000000000..93823014f9 --- /dev/null +++ b/docs/IRIAuthCredentialRotation.md @@ -0,0 +1,218 @@ +# IRI Registry Authentication Credential Rotation + +This document describes how the Internal Release Image (IRI) registry +authentication credentials are managed and rotated by the +`InternalReleaseImageController`. + +## Background + +The IRI registry runs on each control plane node behind the `api-int` VIP +(port 22625). Both master and worker nodes pull images from it using +credentials stored in the global pull secret (`openshift-config/pull-secret`). +Because `api-int` load-balances across **all** master nodes, updating +credentials naively causes intermittent authentication failures: a request +may reach a node that still has the old htpasswd while the pull secret already +contains the new password, or vice versa. + +The distribution registry's htpasswd implementation stores entries in a +`map[string][]byte`, so duplicate usernames overwrite each other (last wins). +This means we cannot simply list the same username twice with different +passwords; instead, dual credentials must use **different usernames**. + +## Desired-vs-Current Pattern + +The controller follows the same desired-vs-current reconciliation pattern used +elsewhere in the MCO (e.g., rendered MachineConfig vs node annotations): + +| Secret | Role | +|---|---| +| `openshift-machine-config-operator/internal-release-image-registry-auth` (auth secret) | Holds the **desired** password in `data.password` and the htpasswd file in `data.htpasswd`. | +| `openshift-config/pull-secret` (global pull secret) | Holds the **current** active credentials (as deployed on nodes via rendered MachineConfig). | + +The controller reads the **deployed** state from the rendered MachineConfig's +ignition content (`/var/lib/kubelet/config.json`), not from the pull secret API +object, to accurately determine what is actually on nodes. + +## Generation-Numbered Usernames + +To support dual htpasswd entries during rotation, usernames carry a generation +counter: + +- `openshift` — generation 0 (initial) +- `openshift1` — generation 1 +- `openshift2` — generation 2 +- ... + +Each rotation increments the generation, producing a new username that coexists +with the previous one in the htpasswd file. + +## Rotation Phases + +When the auth secret's desired password differs from the deployed password, the +controller performs a three-phase rotation: + +### Phase 1: Deploy Dual Htpasswd + +The controller generates an htpasswd file containing **two entries**: one for +the currently deployed credentials (old username + old password) and one for +the new credentials (next-generation username + new password). It writes this +dual htpasswd to `data.htpasswd` in the auth secret. + +The renderer reads the htpasswd from the auth secret, so the next MachineConfig +render includes both credentials. As the MachineConfigPool rolls out, each node +receives the dual htpasswd. During this rollout, both old and new passwords are +accepted regardless of which node handles the request. + +### Phase 2: Update Pull Secret + +Once **all** MachineConfigPools report that every machine is updated +(`UpdatedMachineCount == MachineCount == ReadyMachineCount`), the controller +updates the global pull secret with the new username and password. This is safe +because all nodes now accept both passwords. + +The pull secret update triggers the template controller to re-render, creating +a new rendered MachineConfig. Another MCP rollout propagates the updated pull +secret to all nodes. + +### Phase 3: Clean Up to Single Htpasswd + +On subsequent syncs, the controller reads the deployed pull secret from the +rendered MachineConfig. Once the deployed password matches the desired password +(i.e., the Phase 2 rollout is complete), the controller cleans up the dual +htpasswd to a single entry containing only the new credentials. This removes +the old password, eliminating the security risk of leaving stale credentials. + +## State Diagram + +``` + ┌────────────────────────┐ + │ No IRI entry in pull │ + │ secret (first time) │ + └──────────┬─────────────┘ + │ merge directly + v + ┌────────────────────────┐ + │ Deployed == Desired │◄──────────────────┐ + │ (steady state) │ │ + └──────────┬─────────────┘ │ + │ auth secret password changes │ + v │ + ┌────────────────────────┐ │ + │ Phase 1: Write dual │ │ + │ htpasswd to auth secret│ │ + └──────────┬─────────────┘ │ + │ wait for MCP rollout │ + v │ + ┌────────────────────────┐ │ + │ Phase 2: Update pull │ │ + │ secret with new creds │ │ + └──────────┬─────────────┘ │ + │ wait for MCP rollout │ + v │ + ┌────────────────────────┐ │ + │ Phase 3: Clean up dual │ │ + │ htpasswd → single entry│───────────────────┘ + └────────────────────────┘ +``` + +## Triggering a Credential Rotation + +To rotate IRI registry credentials, update the `password` field in the auth +secret: + +```bash +oc -n openshift-machine-config-operator patch secret internal-release-image-registry-auth \ + --type merge -p '{"data":{"password":"'$(echo -n "new-password" | base64)'"}}' +``` + +The controller detects the mismatch between the desired password (auth secret) +and the deployed password (rendered MachineConfig) and automatically performs +the three-phase rotation. No manual intervention is required after updating +the secret. + +**Important:** Only modify the `password` field. The `htpasswd` field is +controller-managed. If the htpasswd is manually edited, the controller will +detect the invalid hash via `bcrypt.CompareHashAndPassword` and regenerate it. + +## User Edits During Rotation + +The `password` and `htpasswd` fields in the auth secret can be changed at any +point during rotation. The controller handles all cases safely: + +- **Password changes** are detected via `HtpasswdHasValidEntry`, which uses + `bcrypt.CompareHashAndPassword` to verify the htpasswd hash matches the + desired password. If it doesn't match (password changed since the dual + htpasswd was written), the dual htpasswd is regenerated. Clients are never + affected because the pull secret always contains the old deployed credentials, + which are preserved as one of the two htpasswd entries in every regeneration. + +- **Htpasswd edits** are detected by the same `HtpasswdHasValidEntry` check. + Invalid or tampered hashes cause regeneration. The controller is the sole + owner of the htpasswd field. + +- **Cross-pool inconsistency** (e.g., password changed after the pull secret + was updated but before all pools re-rendered) is detected by + `getDeployedIRICredentials`, which returns `ErrInconsistentIRICredentials`. + The controller waits for convergence before making any rotation decisions. + +### Per-Phase Analysis + +**Phase 1** (dual htpasswd written, MCP rolling out): Password or htpasswd +changes cause `HtpasswdHasValidEntry` to fail. Dual htpasswd is regenerated +with the latest desired password. New MCP rollout starts. Pull secret is +unchanged. Safe. + +**Phase 2** (all MCPs updated, pull secret about to be updated): Password or +htpasswd changes cause dual htpasswd regeneration **before** the pull secret +update code is reached, so the pull secret is never updated with a stale +password. Goes back to Phase 1. Safe. + +**Phase 2b** (pull secret updated, re-render in progress): Different pools may +have different rendered MCs. `getDeployedIRICredentials` returns +`ErrInconsistentIRICredentials`. Controller takes no action and waits for +convergence. Password or htpasswd edits during this window are ignored until +pools converge. After all pools converge, the next sync picks up the correct +deployed state and reconciles. Safe. + +**Phase 3** (deployed matches desired, cleaning up dual htpasswd): Password +changes cause Case 3 to fire instead of Case 2, skipping cleanup and starting +a new rotation. Htpasswd edits are detected by Case 2's +`HtpasswdHasValidEntry` check and repaired to a single valid entry. Safe. + +## Safety Guarantees + +- **No authentication deadlock**: Dual htpasswd ensures both old and new + passwords are accepted during the entire rollout window. The pull secret + (what clients use) always contains the old credentials, which are always + present in every version of the htpasswd. +- **No stale credentials**: The dual htpasswd is cleaned up to a single entry + once the rotation is fully deployed, removing the old password. +- **Accurate deployed state**: The controller reads credentials from the + rendered MachineConfig ignition content, not the API object, avoiding race + conditions between API updates and node rollouts. +- **Cross-pool consistency**: `getDeployedIRICredentials` checks all pools' + rendered MachineConfigs and returns `ErrInconsistentIRICredentials` if + different pools have different IRI credentials (e.g., one pool's rendered + MC was re-rendered with the updated pull secret while another still has + the old one). When this occurs, the controller treats it as a rollout in + progress and waits for convergence before making any rotation decisions. +- **All pools checked**: Both master and worker MachineConfigPools must + complete rollout before advancing phases, because workers are also IRI + registry clients via `api-int`. +- **Htpasswd self-healing**: If the htpasswd is manually edited, the + controller detects the invalid hash via `bcrypt.CompareHashAndPassword` + and regenerates it to match the expected credentials. +- **Mid-rotation resilience**: Changing the desired password during an active + rotation regenerates the dual htpasswd without causing deadlock. The + `HtpasswdHasValidEntry` check uses bcrypt hash comparison (not string + equality) to detect when the desired password has changed since the dual + htpasswd was written. + +## Related Files + +- `pkg/controller/internalreleaseimage/internalreleaseimage_controller.go` — + `reconcileAuthCredentials`, `getDeployedIRICredentials`, `areAllPoolsUpdated` +- `pkg/controller/internalreleaseimage/pullsecret.go` — + `MergeIRIAuthIntoPullSecret`, `ExtractIRICredentialsFromPullSecret`, + `NextIRIUsername`, `GenerateHtpasswdEntry`, `GenerateDualHtpasswd`, + `HtpasswdHasValidEntry` diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go index 5fc09d5547..9c295869c8 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller.go @@ -4,10 +4,12 @@ import ( "context" "fmt" "reflect" + "strings" "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/labels" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" @@ -40,6 +42,12 @@ const ( maxRetries = 15 ) +// ErrInconsistentIRICredentials is returned by getDeployedIRICredentials when +// different MachineConfigPools have different IRI credentials in their rendered +// MachineConfigs. This indicates a rollout is in progress and the rotation +// state machine should not advance. +var ErrInconsistentIRICredentials = fmt.Errorf("IRI credentials differ across MachineConfigPool rendered configs") + var ( // controllerKind contains the schema.GroupVersionKind for this controller type. controllerKind = mcfgv1alpha1.SchemeGroupVersion.WithKind("InternalReleaseImage") @@ -69,6 +77,9 @@ type Controller struct { mcLister mcfglistersv1.MachineConfigLister mcListerSynced cache.InformerSynced + mcpLister mcfglistersv1.MachineConfigPoolLister + mcpListerSynced cache.InformerSynced + clusterVersionLister configlistersv1.ClusterVersionLister clusterVersionListerSynced cache.InformerSynced @@ -83,6 +94,7 @@ func New( iriInformer mcfginformersv1alpha1.InternalReleaseImageInformer, ccInformer mcfginformersv1.ControllerConfigInformer, mcInformer mcfginformersv1.MachineConfigInformer, + mcpInformer mcfginformersv1.MachineConfigPoolInformer, clusterVersionInformer configinformersv1.ClusterVersionInformer, secretInformer coreinformersv1.SecretInformer, kubeClient clientset.Interface, @@ -132,6 +144,9 @@ func New( ctrl.mcLister = mcInformer.Lister() ctrl.mcListerSynced = mcInformer.Informer().HasSynced + ctrl.mcpLister = mcpInformer.Lister() + ctrl.mcpListerSynced = mcpInformer.Informer().HasSynced + ctrl.clusterVersionLister = clusterVersionInformer.Lister() ctrl.clusterVersionListerSynced = clusterVersionInformer.Informer().HasSynced @@ -146,7 +161,7 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer ctrl.queue.ShutDown() - if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.clusterVersionListerSynced, ctrl.secretListerSynced) { + if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.mcpListerSynced, ctrl.clusterVersionListerSynced, ctrl.secretListerSynced) { return } @@ -347,6 +362,17 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error { // Auth secret may not exist during upgrades from non-auth clusters iriAuthSecret, _ := ctrl.secretLister.Secrets(ctrlcommon.MCONamespace).Get(ctrlcommon.InternalReleaseImageAuthSecretName) + // Handle credential rotation using the desired-vs-current pattern. + // The auth secret holds the desired password; the pull secret holds the current + // active credentials. When they differ, a multi-phase rotation is performed + // to avoid authentication failures during rolling MachineConfig updates. + // See docs/iri-auth-credential-rotation.md for the full design. + if iriAuthSecret != nil { + if err := ctrl.reconcileAuthCredentials(iriAuthSecret, cconfig); err != nil { + return fmt.Errorf("failed to reconcile auth credentials: %w", err) + } + } + for _, role := range SupportedRoles { r := NewRendererByRole(role, iri, iriSecret, iriAuthSecret, cconfig) @@ -375,13 +401,6 @@ 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 @@ -462,6 +481,250 @@ func (ctrl *Controller) addFinalizerToInternalReleaseImage(iri *mcfgv1alpha1.Int return err } +// reconcileAuthCredentials implements the desired-vs-current credential rotation +// pattern. It compares the desired password (from the auth secret) with the +// current password (from the pull secret) and manages a safe multi-phase rotation: +// +// 1. Passwords match: credentials are in sync, ensure htpasswd uses a single entry. +// 2. No IRI entry in pull secret: first-time setup, merge credentials directly. +// 3. Passwords differ: rotation needed — generate dual htpasswd with both old and +// new credentials (using different generation usernames), update the auth secret's +// htpasswd field, and wait for all MachineConfigPools to finish rolling out before +// updating the pull secret with the new credentials. +// +// The "current" state is determined by reading the pull secret from the rendered +// MachineConfig (what's actually deployed on nodes), not from the API object, +// to avoid race conditions during rollout. +// +// See docs/iri-auth-credential-rotation.md for the full design. +func (ctrl *Controller) reconcileAuthCredentials(authSecret *corev1.Secret, cconfig *mcfgv1.ControllerConfig) error { + desiredPassword := string(authSecret.Data["password"]) + if desiredPassword == "" { + return nil + } + + if cconfig.Spec.DNS == nil { + return fmt.Errorf("ControllerConfig DNS is not set") + } + baseDomain := cconfig.Spec.DNS.Spec.BaseDomain + + // Read the deployed pull secret from the rendered MachineConfig to determine + // what credentials are actually on nodes, rather than the API object which + // may not have been rolled out yet. + deployedUsername, deployedPassword, err := ctrl.getDeployedIRICredentials(baseDomain) + if err == ErrInconsistentIRICredentials { + // Different pools have different IRI credentials in their rendered MCs. + // This means a pull secret rollout is in progress. Wait for it to + // complete before making any rotation decisions. + klog.V(4).Infof("IRI credentials inconsistent across pools, waiting for rollout to converge") + return nil + } + if err != nil { + return fmt.Errorf("failed to read deployed IRI credentials: %w", err) + } + + // Case 1: No IRI entry deployed yet — first-time setup + if deployedPassword == "" { + klog.Infof("No IRI credentials deployed, performing first-time merge into pull secret") + return ctrl.mergeIRIAuthIntoPullSecret(cconfig, authSecret) + } + + // Case 2: Deployed password matches desired — credentials are in sync. + if deployedPassword == desiredPassword { + currentHtpasswd := string(authSecret.Data["htpasswd"]) + var needsUpdate bool + + // Clean up dual htpasswd entries left over from a completed rotation. + if strings.Count(currentHtpasswd, "\n") > 1 { + klog.Infof("Cleaning up dual htpasswd after completed rotation (username=%s)", deployedUsername) + needsUpdate = true + } + + // Repair htpasswd if it was manually edited to contain an invalid hash + // (e.g., user directly modified the auth secret's htpasswd field). + if !HtpasswdHasValidEntry(currentHtpasswd, deployedUsername, desiredPassword) { + klog.Infof("Repairing htpasswd: hash does not match desired password (username=%s)", deployedUsername) + needsUpdate = true + } + + if needsUpdate { + singleHtpasswd, err := GenerateHtpasswdEntry(deployedUsername, desiredPassword) + if err != nil { + return fmt.Errorf("failed to generate single htpasswd: %w", err) + } + authSecret.Data["htpasswd"] = []byte(singleHtpasswd) + if _, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update( + context.TODO(), authSecret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to update auth secret htpasswd: %w", err) + } + } + return nil + } + + // Case 3: Passwords differ — rotation needed + klog.Infof("IRI auth credential rotation detected: deployed password differs from desired password") + + newUsername := NextIRIUsername(deployedUsername) + currentHtpasswd := string(authSecret.Data["htpasswd"]) + + // Check whether the htpasswd already has valid dual entries for both the + // deployed credentials and the desired credentials. We use + // bcrypt.CompareHashAndPassword rather than string comparison because bcrypt + // salts differ on every generation. This also handles mid-rotation password + // changes: if the desired password changed since the dual htpasswd was + // written, the new entry's hash won't match and we regenerate. + // + // Safety: the deployed credentials (oldUser, oldPass) are always preserved + // as one of the two htpasswd entries. The pull secret (what clients use to + // authenticate) always contains these old credentials. So even if the desired + // password changes mid-rotation, clients always authenticate successfully + // against every htpasswd version. + hasDualEntries := HtpasswdHasValidEntry(currentHtpasswd, deployedUsername, deployedPassword) && + HtpasswdHasValidEntry(currentHtpasswd, newUsername, desiredPassword) + + if !hasDualEntries { + // Generate dual htpasswd with both deployed and new credentials so that + // both passwords are accepted during the rolling MachineConfig update. + dualHtpasswd, err := GenerateDualHtpasswd(deployedUsername, deployedPassword, newUsername, desiredPassword) + if err != nil { + return fmt.Errorf("failed to generate dual htpasswd for rotation: %w", err) + } + authSecret.Data["htpasswd"] = []byte(dualHtpasswd) + if _, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update( + context.TODO(), authSecret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to update auth secret with dual htpasswd: %w", err) + } + klog.Infof("Updated auth secret htpasswd with dual credentials (%s + %s) for rotation", deployedUsername, newUsername) + // Return so the MC render on the next sync picks up the new htpasswd + return nil + } + + // Dual htpasswd is already set — check if ALL pools have rolled out the + // dual htpasswd MachineConfig. Only then is it safe to update the pull secret. + allUpdated, err := ctrl.areAllPoolsUpdated() + if err != nil { + return fmt.Errorf("failed to check MCP status: %w", err) + } + + if !allUpdated { + klog.V(4).Infof("Waiting for MachineConfigPool rollout to complete before advancing credential rotation") + return nil + } + + // All nodes have the dual htpasswd — safe to update the pull secret. + // After the pull secret API object is updated, the template controller will + // re-render and trigger another rollout. On subsequent syncs, the deployed + // pull secret (from the rendered MC) will eventually match the desired password, + // at which point Case 2 handles cleanup. + klog.Infof("MCP rollout complete, updating pull secret with new credentials (username=%s)", newUsername) + 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) + } + + mergedBytes, err := MergeIRIAuthIntoPullSecretWithUsername( + pullSecret.Data[corev1.DockerConfigJsonKey], newUsername, desiredPassword, baseDomain) + if err != nil { + return fmt.Errorf("failed to merge new credentials into pull secret: %w", err) + } + + pullSecret.Data[corev1.DockerConfigJsonKey] = mergedBytes + if _, err := ctrl.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Update( + context.TODO(), pullSecret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to update pull secret with rotated credentials: %w", err) + } + klog.Infof("Updated pull secret with rotated IRI credentials (username=%s)", newUsername) + + // The pull secret update will trigger the template controller to re-render, + // creating a new rendered MC. On the next sync, getDeployedIRICredentials will + // still return the OLD credentials until that rollout completes. Once the new + // rendered MC (with updated pull secret) is fully deployed, deployedPassword + // will match desiredPassword, and Case 2 will clean up the dual htpasswd to + // a single entry. + return nil +} + +// getDeployedIRICredentials reads the IRI registry credentials from the pull +// secret that is actually deployed on nodes, by parsing the rendered +// MachineConfig for each pool. This is more accurate than reading the +// openshift-config/pull-secret API object, which may not have been rolled out yet. +// +// All pools must have consistent IRI credentials. If different pools have +// different passwords (e.g., one pool's rendered MC was re-rendered with a new +// pull secret while another pool still has the old one), this returns +// ErrInconsistentIRICredentials. The caller should treat this as "rollout in +// progress" and avoid advancing the rotation state machine. +func (ctrl *Controller) getDeployedIRICredentials(baseDomain string) (username, password string, err error) { + pools, err := ctrl.mcpLister.List(labels.Everything()) + if err != nil { + return "", "", fmt.Errorf("failed to list MachineConfigPools: %w", err) + } + + var foundUsername, foundPassword string + + for _, pool := range pools { + renderedMCName := pool.Status.Configuration.Name + if renderedMCName == "" { + continue + } + + renderedMC, err := ctrl.mcLister.Get(renderedMCName) + if err != nil { + return "", "", fmt.Errorf("failed to get rendered MachineConfig %s: %w", renderedMCName, err) + } + + ignConfig, err := ctrlcommon.ParseAndConvertConfig(renderedMC.Spec.Config.Raw) + if err != nil { + return "", "", fmt.Errorf("failed to parse ignition from rendered MachineConfig %s: %w", renderedMCName, err) + } + + pullSecretData, err := ctrlcommon.GetIgnitionFileDataByPath(&ignConfig, "/var/lib/kubelet/config.json") + if err != nil { + return "", "", fmt.Errorf("failed to extract pull secret from rendered MachineConfig %s: %w", renderedMCName, err) + } + if pullSecretData == nil { + continue + } + + u, p := ExtractIRICredentialsFromPullSecret(pullSecretData, baseDomain) + if p == "" { + continue + } + + if foundPassword == "" { + foundUsername = u + foundPassword = p + } else if foundPassword != p || foundUsername != u { + return "", "", ErrInconsistentIRICredentials + } + } + + return foundUsername, foundPassword, nil +} + +// areAllPoolsUpdated checks whether all MachineConfigPools have finished +// rolling out (all machines are at the desired configuration). Both master +// and worker pools must be checked because worker nodes are also clients +// of the IRI registry via api-int. +func (ctrl *Controller) areAllPoolsUpdated() (bool, error) { + pools, err := ctrl.mcpLister.List(labels.Everything()) + if err != nil { + return false, err + } + + for _, pool := range pools { + if pool.Status.MachineCount != pool.Status.UpdatedMachineCount { + return false, nil + } + if pool.Status.MachineCount != pool.Status.ReadyMachineCount { + return false, nil + } + } + + return true, nil +} + func (ctrl *Controller) mergeIRIAuthIntoPullSecret(cconfig *mcfgv1.ControllerConfig, authSecret *corev1.Secret) error { password := string(authSecret.Data["password"]) if password == "" { diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go index bccafba529..7806755e69 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go @@ -3,6 +3,7 @@ package internalreleaseimage import ( "context" "encoding/json" + "strings" "testing" "time" @@ -76,7 +77,7 @@ func TestInternalReleaseImageCreate(t *testing.T) { }, { name: "generate iri machine-config with auth", - initialObjects: objs(iri(), clusterVersion(), cconfig(), iriCertSecret(), iriAuthSecret()), + 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) @@ -204,6 +205,7 @@ type fixture struct { iriLister []*mcfgv1alpha1.InternalReleaseImage ccLister []*mcfgv1.ControllerConfig mcLister []*mcfgv1.MachineConfig + mcpLister []*mcfgv1.MachineConfigPool secretLister []*corev1.Secret clusterVersionLister []*configv1.ClusterVersion @@ -240,6 +242,8 @@ func (f *fixture) setupObjects(objs []runtime.Object) { f.ccLister = append(f.ccLister, o) case *mcfgv1.MachineConfig: f.mcLister = append(f.mcLister, o) + case *mcfgv1.MachineConfigPool: + f.mcpLister = append(f.mcpLister, o) } } } @@ -258,6 +262,7 @@ func (f *fixture) newController() *Controller { i.Machineconfiguration().V1alpha1().InternalReleaseImages(), i.Machineconfiguration().V1().ControllerConfigs(), i.Machineconfiguration().V1().MachineConfigs(), + i.Machineconfiguration().V1().MachineConfigPools(), ci.Config().V1().ClusterVersions(), k.Core().V1().Secrets(), f.k8sClient, @@ -268,6 +273,7 @@ func (f *fixture) newController() *Controller { c.iriListerSynced = alwaysReady c.ccListerSynced = alwaysReady c.mcListerSynced = alwaysReady + c.mcpListerSynced = alwaysReady c.clusterVersionListerSynced = alwaysReady c.secretListerSynced = alwaysReady c.eventRecorder = &record.FakeRecorder{} @@ -291,6 +297,9 @@ func (f *fixture) newController() *Controller { for _, c := range f.mcLister { i.Machineconfiguration().V1().MachineConfigs().Informer().GetIndexer().Add(c) } + for _, c := range f.mcpLister { + i.Machineconfiguration().V1().MachineConfigPools().Informer().GetIndexer().Add(c) + } for _, c := range f.secretLister { k.Core().V1().Secrets().Informer().GetIndexer().Add(c) } @@ -313,3 +322,274 @@ func (f *fixture) runController(key string, expectError bool) { f.t.Error("expected error syncing internalreleaseimage, got nil") } } + +// getAuthSecret fetches the auth secret from the fake k8s client. +func (f *fixture) getAuthSecret() *corev1.Secret { + s, err := f.k8sClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get( + context.TODO(), ctrlcommon.InternalReleaseImageAuthSecretName, metav1.GetOptions{}) + if err != nil { + f.t.Fatalf("failed to get auth secret: %v", err) + } + return s +} + +// getPullSecret fetches the global pull secret from the fake k8s client. +func (f *fixture) getPullSecret() *corev1.Secret { + s, err := f.k8sClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get( + context.TODO(), ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + if err != nil { + f.t.Fatalf("failed to get pull secret: %v", err) + } + return s +} + +// TestCredentialRotation tests the reconcileAuthCredentials state machine. +// Each test case sets up state for a specific phase of the rotation and runs +// a single sync to verify the controller produces the correct output. +// See docs/IRIAuthCredentialRotation.md for the full rotation design. +func TestCredentialRotation(t *testing.T) { + const baseDomain = "example.com" + const oldPassword = "oldpassword" + const newPassword = "newpassword" + const changedPassword = "changedpassword" + + // Generate valid bcrypt htpasswd entries for test fixtures. + oldHtpasswd, err := GenerateHtpasswdEntry(IRIBaseUsername, oldPassword) + assert.NoError(t, err) + + dualHtpasswd, err := GenerateDualHtpasswd(IRIBaseUsername, oldPassword, "openshift1", newPassword) + assert.NoError(t, err) + + cases := []struct { + name string + initialObjects func() []runtime.Object + verify func(t *testing.T, f *fixture) + }{ + // ===== Phase 1: Dual htpasswd written, MCP rolling out ===== + { + name: "phase 1: rotation triggers dual htpasswd when deployed differs from desired", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, oldHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, IRIBaseUsername, oldPassword), + "dual htpasswd should contain old credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "dual htpasswd should contain new credentials") + }, + }, + { + name: "phase 1: mid-rotation password change regenerates dual htpasswd", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(changedPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, IRIBaseUsername, oldPassword), + "dual htpasswd should preserve old credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", changedPassword), + "dual htpasswd should be regenerated with changed password") + assert.False(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "dual htpasswd should not contain stale new password") + }, + }, + { + name: "phase 1: user edits htpasswd directly triggers regeneration", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, "openshift:invalidhash\n"), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, IRIBaseUsername, oldPassword), + "regenerated dual htpasswd should contain old credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "regenerated dual htpasswd should contain new credentials") + }, + }, + // ===== Phase 2: All MCPs updated, pull secret about to be updated ===== + { + name: "phase 2: all pools updated advances to pull secret update", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + ps := f.getPullSecret() + u, p := ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + assert.Equal(t, "openshift1", u, "pull secret should have new username") + assert.Equal(t, newPassword, p, "pull secret should have new password") + }, + }, + { + name: "phase 2: pools not updated blocks pull secret update", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker").notUpdated(), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + ps := f.getPullSecret() + _, p := ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + assert.Empty(t, p, "pull secret should NOT be updated while pools are rolling out") + }, + }, + { + name: "phase 2: password change before pull secret update prevents stale update", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(changedPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + ps := f.getPullSecret() + _, p := ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + assert.Empty(t, p, "pull secret should not be updated with stale password") + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", changedPassword), + "htpasswd should be regenerated with changed password") + }, + }, + // ===== Phase 2b: Pull secret updated, re-render in progress ===== + { + name: "phase 2b: inconsistent credentials across pools causes wait", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, IRIBaseUsername, oldPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + assert.Equal(t, dualHtpasswd, string(authSecret.Data["htpasswd"]), + "htpasswd should not be modified during cross-pool inconsistency") + }, + }, + // ===== Phase 3: Deployed matches desired, cleaning up ===== + { + name: "phase 3: deployed matches desired cleans up dual htpasswd", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, "openshift1", newPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "cleaned up htpasswd should have valid entry for deployed credentials") + lines := strings.Split(strings.TrimSpace(htpasswd), "\n") + assert.Equal(t, 1, len(lines), "cleaned up htpasswd should have single entry") + }, + }, + { + name: "phase 3: user edits htpasswd in steady state triggers repair", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(newPassword, "openshift1:invalidhash\n"), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, "openshift1", newPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "repaired htpasswd should have valid hash") + }, + }, + { + name: "phase 3: password change during cleanup starts new rotation", + initialObjects: objs( + iri().finalizer(masterName(), workerName()), + clusterVersion(), cconfig().withDNS(baseDomain), iriCertSecret(), + iriAuthSecretCustom(changedPassword, dualHtpasswd), + pullSecret(), + renderedMC("rendered-master").withIRICredentials(baseDomain, "openshift1", newPassword), + renderedMC("rendered-worker").withIRICredentials(baseDomain, "openshift1", newPassword), + mcp("master", "rendered-master"), + mcp("worker", "rendered-worker"), + machineconfigmaster(), machineconfigworker(), + ), + verify: func(t *testing.T, f *fixture) { + authSecret := f.getAuthSecret() + htpasswd := string(authSecret.Data["htpasswd"]) + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift1", newPassword), + "new rotation should preserve current deployed credentials") + assert.True(t, HtpasswdHasValidEntry(htpasswd, "openshift2", changedPassword), + "new rotation should use next generation username") + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + objs := tc.initialObjects() + f := newFixture(t, objs) + f.run(ctrlcommon.InternalReleaseImageInstanceName) + tc.verify(t, f) + }) + } +} diff --git a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go index 36e8ef100b..254c3d6674 100644 --- a/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go +++ b/pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.go @@ -3,6 +3,7 @@ package internalreleaseimage // Test builders and helper methods. import ( + "encoding/json" "fmt" "testing" @@ -307,3 +308,97 @@ func clusterVersion() *clusterVersionBuilder { func (cvb *clusterVersionBuilder) build() runtime.Object { return cvb.obj } + +// mcpBuilder simplifies the creation of a MachineConfigPool resource in the test. +type mcpBuilder struct { + obj *mcfgv1.MachineConfigPool +} + +func mcp(name string, renderedMCName string) *mcpBuilder { + return &mcpBuilder{ + obj: &mcfgv1.MachineConfigPool{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Status: mcfgv1.MachineConfigPoolStatus{ + Configuration: mcfgv1.MachineConfigPoolStatusConfiguration{ + ObjectReference: corev1.ObjectReference{ + Name: renderedMCName, + }, + }, + MachineCount: 3, + UpdatedMachineCount: 3, + ReadyMachineCount: 3, + }, + }, + } +} + +func (mb *mcpBuilder) notUpdated() *mcpBuilder { + mb.obj.Status.UpdatedMachineCount = 1 + mb.obj.Status.ReadyMachineCount = 1 + return mb +} + +func (mb *mcpBuilder) build() runtime.Object { + return mb.obj +} + +// renderedMCBuilder creates a rendered MachineConfig with a pull secret embedded +// in ignition at /var/lib/kubelet/config.json. +type renderedMCBuilder struct { + obj *mcfgv1.MachineConfig + pullSecret string +} + +func renderedMC(name string) *renderedMCBuilder { + return &renderedMCBuilder{ + obj: &mcfgv1.MachineConfig{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + }, + Spec: mcfgv1.MachineConfigSpec{}, + }, + } +} + +func (rb *renderedMCBuilder) withIRICredentials(baseDomain string, username string, password string) *renderedMCBuilder { + rb.pullSecret = pullSecretWithIRIAuthAndUsername(baseDomain, username, password) + return rb +} + +func (rb *renderedMCBuilder) build() runtime.Object { + if rb.pullSecret != "" { + ignCfg := ctrlcommon.NewIgnConfig() + ignCfg.Storage.Files = append(ignCfg.Storage.Files, + ctrlcommon.NewIgnFile("/var/lib/kubelet/config.json", rb.pullSecret)) + raw, _ := json.Marshal(ignCfg) + rb.obj.Spec.Config.Raw = raw + } + return rb.obj +} + +// iriAuthSecretBuilder simplifies the creation of an IRI auth secret with +// customizable password and htpasswd fields. +type iriAuthSecretBuilder struct { + obj *corev1.Secret +} + +func iriAuthSecretCustom(password string, htpasswd string) *iriAuthSecretBuilder { + return &iriAuthSecretBuilder{ + obj: &corev1.Secret{ + ObjectMeta: v1.ObjectMeta{ + Namespace: ctrlcommon.MCONamespace, + Name: ctrlcommon.InternalReleaseImageAuthSecretName, + }, + Data: map[string][]byte{ + "htpasswd": []byte(htpasswd), + "password": []byte(password), + }, + }, + } +} + +func (ab *iriAuthSecretBuilder) build() runtime.Object { + return ab.obj +} diff --git a/pkg/controller/internalreleaseimage/pullsecret.go b/pkg/controller/internalreleaseimage/pullsecret.go index 194a525a95..0f02a6687d 100644 --- a/pkg/controller/internalreleaseimage/pullsecret.go +++ b/pkg/controller/internalreleaseimage/pullsecret.go @@ -4,16 +4,35 @@ import ( "encoding/base64" "encoding/json" "fmt" + "strconv" + "strings" + + "golang.org/x/crypto/bcrypt" ) +// IRIBaseUsername is the base username for IRI registry authentication (generation 0). +// During credential rotation, a generation counter is appended to form successive +// usernames: "openshift" (generation 0), "openshift1" (generation 1), +// "openshift2" (generation 2), etc. +const IRIBaseUsername = "openshift" + // 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. +// into a dockerconfigjson pull secret using the default username (generation 0). +// 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) { + return MergeIRIAuthIntoPullSecretWithUsername(pullSecretRaw, IRIBaseUsername, password, baseDomain) +} + +// MergeIRIAuthIntoPullSecretWithUsername merges IRI registry authentication credentials +// into a dockerconfigjson pull secret using the specified username. This is used during +// credential rotation to update the pull secret with the new generation username +// (e.g., "openshift1") after all nodes have been updated with the new htpasswd. +func MergeIRIAuthIntoPullSecretWithUsername(pullSecretRaw []byte, username string, password string, baseDomain string) ([]byte, error) { if password == "" { return pullSecretRaw, nil } @@ -30,7 +49,7 @@ func MergeIRIAuthIntoPullSecret(pullSecretRaw []byte, password string, baseDomai return nil, fmt.Errorf("pull secret missing 'auths' field") } - authValue := base64.StdEncoding.EncodeToString([]byte("openshift:" + password)) + authValue := base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) // Check if IRI entry already exists and is current if existing, ok := auths[iriRegistryHost].(map[string]interface{}); ok { @@ -50,3 +69,113 @@ func MergeIRIAuthIntoPullSecret(pullSecretRaw []byte, password string, baseDomai return mergedBytes, nil } + +// ExtractIRICredentialsFromPullSecret extracts the current IRI registry username +// and password from a dockerconfigjson pull secret. It looks for the auth entry +// keyed by api-int.:22625 and decodes the credentials from the +// base64 "username:password" auth value. +// Returns empty strings if the entry is not found or cannot be parsed. +func ExtractIRICredentialsFromPullSecret(pullSecretRaw []byte, baseDomain string) (username, password string) { + iriRegistryHost := fmt.Sprintf("api-int.%s:22625", baseDomain) + + var dockerConfig map[string]interface{} + if err := json.Unmarshal(pullSecretRaw, &dockerConfig); err != nil { + return "", "" + } + + auths, ok := dockerConfig["auths"].(map[string]interface{}) + if !ok { + return "", "" + } + + entry, ok := auths[iriRegistryHost].(map[string]interface{}) + if !ok { + return "", "" + } + + authValue, ok := entry["auth"].(string) + if !ok { + return "", "" + } + + decoded, err := base64.StdEncoding.DecodeString(authValue) + if err != nil { + return "", "" + } + + // Auth format is "username:password" + parts := strings.SplitN(string(decoded), ":", 2) + if len(parts) != 2 { + return "", "" + } + + return parts[0], parts[1] +} + +// NextIRIUsername computes the next generation username for credential rotation. +// "openshift" (generation 0) becomes "openshift1" (generation 1), +// "openshift1" becomes "openshift2", and so on. +func NextIRIUsername(currentUsername string) string { + if currentUsername == IRIBaseUsername { + return IRIBaseUsername + "1" + } + + suffix := strings.TrimPrefix(currentUsername, IRIBaseUsername) + gen, err := strconv.Atoi(suffix) + if err != nil { + // Unrecognized format, start at generation 1 + return IRIBaseUsername + "1" + } + + return fmt.Sprintf("%s%d", IRIBaseUsername, gen+1) +} + +// HtpasswdHasValidEntry checks whether the htpasswd content contains an entry +// for the given username whose bcrypt hash matches the given password. +func HtpasswdHasValidEntry(htpasswd string, username string, password string) bool { + for _, line := range strings.Split(strings.TrimSpace(htpasswd), "\n") { + parts := strings.SplitN(line, ":", 2) + if len(parts) != 2 { + continue + } + if parts[0] == username { + if bcrypt.CompareHashAndPassword([]byte(parts[1]), []byte(password)) == nil { + return true + } + } + } + return false +} + +// GenerateHtpasswdEntry generates a single htpasswd entry in the format +// "username:bcrypt-hash\n". +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\n", username, string(hash)), nil +} + +// GenerateDualHtpasswd generates an htpasswd file with two entries: one for the +// current credentials (from the pull secret) and one for the new credentials +// (from the auth secret). This is used during credential rotation to ensure +// that both old and new passwords are accepted during the MachineConfig rollout. +// +// The distribution registry's htpasswd implementation uses a map keyed by +// username, so the two entries MUST have different usernames. The rotation +// scheme uses generation-numbered usernames: "openshift" (generation 0), +// "openshift1" (generation 1), "openshift2" (generation 2), etc. +func GenerateDualHtpasswd(currentUsername, currentPassword, newUsername, newPassword string) (string, error) { + currentEntry, err := GenerateHtpasswdEntry(currentUsername, currentPassword) + if err != nil { + return "", fmt.Errorf("failed to generate current htpasswd entry: %w", err) + } + + newEntry, err := GenerateHtpasswdEntry(newUsername, newPassword) + if err != nil { + return "", fmt.Errorf("failed to generate new htpasswd entry: %w", err) + } + + return currentEntry + newEntry, nil +} diff --git a/pkg/controller/internalreleaseimage/pullsecret_test.go b/pkg/controller/internalreleaseimage/pullsecret_test.go index 33cc694a72..5e52b01108 100644 --- a/pkg/controller/internalreleaseimage/pullsecret_test.go +++ b/pkg/controller/internalreleaseimage/pullsecret_test.go @@ -3,6 +3,7 @@ package internalreleaseimage import ( "encoding/base64" "encoding/json" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -100,9 +101,165 @@ func TestMergeIRIAuthIntoPullSecret(t *testing.T) { } } -// pullSecretWithIRIAuth creates a pull secret JSON that already contains an IRI auth entry. +func TestExtractIRICredentialsFromPullSecret(t *testing.T) { + tests := []struct { + name string + pullSecret string + baseDomain string + expectedUsername string + expectedPassword string + }{ + { + name: "extracts credentials from valid entry", + pullSecret: pullSecretWithIRIAuth("example.com", "mypassword"), + baseDomain: "example.com", + expectedUsername: "openshift", + expectedPassword: "mypassword", + }, + { + name: "extracts credentials with generation username", + pullSecret: pullSecretWithIRIAuthAndUsername("example.com", "openshift2", "mypassword"), + baseDomain: "example.com", + expectedUsername: "openshift2", + expectedPassword: "mypassword", + }, + { + name: "returns empty for missing IRI entry", + pullSecret: `{"auths":{"quay.io":{"auth":"dGVzdDp0ZXN0"}}}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for wrong domain", + pullSecret: pullSecretWithIRIAuth("other.com", "mypassword"), + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for invalid JSON", + pullSecret: "not-json", + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for missing auths", + pullSecret: `{"registry":"quay.io"}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for invalid base64", + pullSecret: `{"auths":{"api-int.example.com:22625":{"auth":"!!!invalid"}}}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "returns empty for auth without colon", + pullSecret: `{"auths":{"api-int.example.com:22625":{"auth":"bm9jb2xvbg=="}}}`, + baseDomain: "example.com", + expectedUsername: "", + expectedPassword: "", + }, + { + name: "handles password containing colon", + pullSecret: pullSecretWithIRIAuth("example.com", "pass:word"), + baseDomain: "example.com", + expectedUsername: "openshift", + expectedPassword: "pass:word", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + username, password := ExtractIRICredentialsFromPullSecret([]byte(tt.pullSecret), tt.baseDomain) + assert.Equal(t, tt.expectedUsername, username) + assert.Equal(t, tt.expectedPassword, password) + }) + } +} + +func TestNextIRIUsername(t *testing.T) { + tests := []struct { + current string + expected string + }{ + {"openshift", "openshift1"}, + {"openshift1", "openshift2"}, + {"openshift2", "openshift3"}, + {"openshift99", "openshift100"}, + {"unknown", "openshift1"}, + } + + for _, tt := range tests { + t.Run(tt.current+"->"+tt.expected, func(t *testing.T) { + assert.Equal(t, tt.expected, NextIRIUsername(tt.current)) + }) + } +} + +func TestHtpasswdHasValidEntry(t *testing.T) { + entry, err := GenerateHtpasswdEntry("openshift", "mypassword") + assert.NoError(t, err) + + dual, err := GenerateDualHtpasswd("openshift", "oldpass", "openshift1", "newpass") + assert.NoError(t, err) + + tests := []struct { + name string + htpasswd string + username string + password string + expected bool + }{ + {"valid single entry", entry, "openshift", "mypassword", true}, + {"wrong password", entry, "openshift", "wrongpassword", false}, + {"wrong username", entry, "other", "mypassword", false}, + {"dual entry matches first", dual, "openshift", "oldpass", true}, + {"dual entry matches second", dual, "openshift1", "newpass", true}, + {"dual entry wrong password", dual, "openshift1", "oldpass", false}, + {"empty htpasswd", "", "openshift", "mypassword", false}, + {"malformed line", "nocolon", "openshift", "mypassword", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, HtpasswdHasValidEntry(tt.htpasswd, tt.username, tt.password)) + }) + } +} + +func TestGenerateHtpasswdEntry(t *testing.T) { + entry, err := GenerateHtpasswdEntry("openshift", "testpassword") + assert.NoError(t, err) + assert.True(t, strings.HasPrefix(entry, "openshift:$2"), "should start with username and bcrypt prefix") + assert.True(t, strings.HasSuffix(entry, "\n"), "should end with newline") +} + +func TestGenerateDualHtpasswd(t *testing.T) { + dual, err := GenerateDualHtpasswd("openshift", "oldpass", "openshift1", "newpass") + assert.NoError(t, err) + + lines := strings.Split(strings.TrimSpace(dual), "\n") + assert.Equal(t, 2, len(lines), "should have two htpasswd entries") + assert.True(t, strings.HasPrefix(lines[0], "openshift:$2"), "first entry should be current username") + assert.True(t, strings.HasPrefix(lines[1], "openshift1:$2"), "second entry should be new username") +} + +// pullSecretWithIRIAuth creates a pull secret JSON with an IRI auth entry using +// the default "openshift" username (generation 0). func pullSecretWithIRIAuth(baseDomain string, password string) string { - authValue := base64.StdEncoding.EncodeToString([]byte("openshift:" + password)) + return pullSecretWithIRIAuthAndUsername(baseDomain, "openshift", password) +} + +// pullSecretWithIRIAuthAndUsername creates a pull secret JSON with an IRI auth +// entry using the specified username. +func pullSecretWithIRIAuthAndUsername(baseDomain string, username string, password string) string { + authValue := base64.StdEncoding.EncodeToString([]byte(username + ":" + password)) host := "api-int." + baseDomain + ":22625" dockerConfig := map[string]interface{}{ "auths": map[string]interface{}{ 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 08ef873959..bad62046c1 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2002,6 +2002,7 @@ go4.org/errorutil # golang.org/x/crypto v0.43.0 ## explicit; go 1.24.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 9c8ca8ef4a28877f65c36ae85f4922e38c1bb53b Mon Sep 17 00:00:00 2001 From: Richard Su Date: Fri, 13 Mar 2026 16:32:32 -0500 Subject: [PATCH 2/2] AGENT-1449: Add e2e tests for IRI registry authentication and credential rotation Add three new e2e tests: - TestIRIAuth_UnauthenticatedRequestReturns401: verifies registry rejects unauthenticated requests with 401 when auth is enabled - TestIRIAuth_AuthenticatedRequestSucceeds: verifies registry accepts requests with valid Basic Auth credentials - TestIRIAuth_CredentialRotation: end-to-end test of the three-phase credential rotation (dual htpasswd, pull secret update, cleanup) Assisted-by: Claude Opus 4.6 --- test/e2e-iri/iri_test.go | 226 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 226 insertions(+) diff --git a/test/e2e-iri/iri_test.go b/test/e2e-iri/iri_test.go index b9d85570bd..7f3af9a49a 100644 --- a/test/e2e-iri/iri_test.go +++ b/test/e2e-iri/iri_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/base64" "fmt" "net/http" "os" @@ -20,7 +21,9 @@ import ( configv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + iri "github.com/openshift/machine-config-operator/pkg/controller/internalreleaseimage" "github.com/openshift/machine-config-operator/test/framework" + "github.com/openshift/machine-config-operator/test/helpers" ) func TestIRIResource_Available(t *testing.T) { @@ -179,6 +182,229 @@ func TestIRIController_ShouldPreventDeletionWhenInUse(t *testing.T) { require.NotNil(t, iri, "IRI should not be nil") } +// getBaseDomain retrieves the cluster's base domain from the ControllerConfig. +func getBaseDomain(t *testing.T, cs *framework.ClientSet) string { + ctx := context.Background() + cconfig, err := cs.ControllerConfigs().Get(ctx, ctrlcommon.ControllerConfigName, v1.GetOptions{}) + require.NoError(t, err) + require.NotNil(t, cconfig.Spec.DNS) + require.NotEmpty(t, cconfig.Spec.DNS.Spec.BaseDomain) + return cconfig.Spec.DNS.Spec.BaseDomain +} + +// getIRIRegistryClient returns an HTTP client configured with the cluster's +// root CA for making requests to the IRI registry. +func getIRIRegistryClient(t *testing.T, cs *framework.ClientSet) *http.Client { + ctx := context.Background() + cm, err := cs.ConfigMaps("openshift-machine-config-operator").Get(ctx, "machine-config-server-ca", v1.GetOptions{}) + require.NoError(t, err) + rootCA := []byte(cm.Data["ca-bundle.crt"]) + roots := x509.NewCertPool() + require.True(t, roots.AppendCertsFromPEM(rootCA)) + + return &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: roots, + }, + }, + Timeout: 30 * time.Second, + } +} + +// getAPIIntIP returns the api-int IP address from the infrastructure resource. +func getAPIIntIP(t *testing.T, cs *framework.ClientSet) string { + ctx := context.Background() + infra, err := cs.Infrastructures().Get(ctx, "cluster", v1.GetOptions{}) + require.NoError(t, err) + require.NotEmpty(t, infra.Status.PlatformStatus.BareMetal.APIServerInternalIPs) + return infra.Status.PlatformStatus.BareMetal.APIServerInternalIPs[0] +} + +func TestIRIAuth_UnauthenticatedRequestReturns401(t *testing.T) { + skipIfOpenShiftCI(t) + skipIfNoBaremetal(t) + + cs := framework.NewClientSet("") + + // Verify the auth secret exists (auth is enabled). + ctx := context.Background() + _, 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) + + client := getIRIRegistryClient(t, cs) + apiIntIP := getAPIIntIP(t, cs) + url := fmt.Sprintf("https://%s:%d/v2/", apiIntIP, 22625) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + require.NoError(t, err) + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusUnauthorized, resp.StatusCode, + "unauthenticated request should return 401 when auth is enabled") +} + +func TestIRIAuth_AuthenticatedRequestSucceeds(t *testing.T) { + skipIfOpenShiftCI(t) + skipIfNoBaremetal(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + + // Get the auth secret to read credentials. + 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) + + password := string(authSecret.Data["password"]) + require.NotEmpty(t, password) + + // Extract the current username from the pull secret. + baseDomain := getBaseDomain(t, cs) + pullSecret, err := cs.Secrets(ctrlcommon.OpenshiftConfigNamespace).Get(ctx, ctrlcommon.GlobalPullSecretName, v1.GetOptions{}) + require.NoError(t, err) + username, _ := iri.ExtractIRICredentialsFromPullSecret(pullSecret.Data[corev1.DockerConfigJsonKey], baseDomain) + if username == "" { + username = iri.IRIBaseUsername + } + + client := getIRIRegistryClient(t, cs) + apiIntIP := getAPIIntIP(t, cs) + url := fmt.Sprintf("https://%s:%d/v2/", apiIntIP, 22625) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(username+":"+password))) + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode, + "authenticated request should succeed") + require.Equal(t, "registry/2.0", resp.Header.Get("Docker-Distribution-Api-Version")) +} + +func TestIRIAuth_CredentialRotation(t *testing.T) { + skipIfOpenShiftCI(t) + skipIfNoBaremetal(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + + // Get the auth secret. + 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"]) + require.NotEmpty(t, originalPassword) + + baseDomain := getBaseDomain(t, cs) + + // Record the original pull secret credentials. + pullSecret, err := cs.Secrets(ctrlcommon.OpenshiftConfigNamespace).Get(ctx, ctrlcommon.GlobalPullSecretName, v1.GetOptions{}) + require.NoError(t, err) + originalUsername, originalPullSecretPassword := iri.ExtractIRICredentialsFromPullSecret(pullSecret.Data[corev1.DockerConfigJsonKey], baseDomain) + require.NotEmpty(t, originalPullSecretPassword, "pull secret should have IRI credentials before rotation") + + // Record the current rendered MC names for master and worker pools so we + // can detect when the MCP rollout completes. + masterMCP, err := cs.MachineConfigPools().Get(ctx, "master", v1.GetOptions{}) + require.NoError(t, err) + workerMCP, err := cs.MachineConfigPools().Get(ctx, "worker", v1.GetOptions{}) + require.NoError(t, err) + + t.Logf("Before rotation: username=%s, master rendered=%s, worker rendered=%s", + originalUsername, masterMCP.Status.Configuration.Name, workerMCP.Status.Configuration.Name) + + // Trigger rotation by updating the password in the auth secret. + newPassword := fmt.Sprintf("rotated-%d", time.Now().UnixNano()) + authSecret.Data["password"] = []byte(newPassword) + _, err = cs.Secrets(ctrlcommon.MCONamespace).Update(ctx, authSecret, v1.UpdateOptions{}) + require.NoError(t, err) + t.Logf("Updated auth secret password to trigger rotation") + + // Phase 1: Wait for dual htpasswd to be written to the auth secret. + t.Logf("Waiting for phase 1: dual htpasswd deployment...") + 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 + } + htpasswd := string(secret.Data["htpasswd"]) + // Dual htpasswd has the new password under a new username. + newUsername := iri.NextIRIUsername(originalUsername) + return iri.HtpasswdHasValidEntry(htpasswd, newUsername, newPassword), nil + }) + require.NoError(t, err, "timed out waiting for dual htpasswd (phase 1)") + t.Logf("Phase 1 complete: dual htpasswd written") + + // Phase 2: Wait for MCP rollout to complete and pull secret to be updated. + // The controller waits for all MCPs to be fully updated before writing the + // pull secret, so we poll the pull secret for the new credentials. + t.Logf("Waiting for phase 2: pull secret update...") + expectedUsername := iri.NextIRIUsername(originalUsername) + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 30*time.Minute, true, func(ctx context.Context) (bool, error) { + ps, err := cs.Secrets(ctrlcommon.OpenshiftConfigNamespace).Get(ctx, ctrlcommon.GlobalPullSecretName, v1.GetOptions{}) + if err != nil { + return false, err + } + u, p := iri.ExtractIRICredentialsFromPullSecret(ps.Data[corev1.DockerConfigJsonKey], baseDomain) + return u == expectedUsername && p == newPassword, nil + }) + require.NoError(t, err, "timed out waiting for pull secret update (phase 2)") + t.Logf("Phase 2 complete: pull secret updated with username=%s", expectedUsername) + + // Wait for both pools to finish rolling out the updated pull secret. + t.Logf("Waiting for MCP rollout after pull secret update...") + helpers.WaitForPoolCompleteAny(t, cs, "master") + helpers.WaitForPoolCompleteAny(t, cs, "worker") + + // Phase 3: Wait for dual htpasswd to be cleaned up to a single entry. + t.Logf("Waiting for phase 3: htpasswd cleanup...") + err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 30*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 + } + htpasswd := string(secret.Data["htpasswd"]) + // After cleanup, the htpasswd should only have the new entry. + hasNew := iri.HtpasswdHasValidEntry(htpasswd, expectedUsername, newPassword) + hasOld := iri.HtpasswdHasValidEntry(htpasswd, originalUsername, originalPullSecretPassword) + return hasNew && !hasOld, nil + }) + require.NoError(t, err, "timed out waiting for htpasswd cleanup (phase 3)") + t.Logf("Phase 3 complete: htpasswd cleaned up to single entry") + + // Verify the registry accepts the new credentials. + client := getIRIRegistryClient(t, cs) + apiIntIP := getAPIIntIP(t, cs) + url := fmt.Sprintf("https://%s:%d/v2/", apiIntIP, 22625) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(expectedUsername+":"+newPassword))) + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode, + "registry should accept the new rotated credentials") + t.Logf("Credential rotation completed successfully") +} + func TestIRIController_ShouldRestoreMachineConfigsWhenModified(t *testing.T) { cases := []struct { name string