From 05f7010b408f33db5689020652878eb54d2ba838 Mon Sep 17 00:00:00 2001 From: Richard Su Date: Wed, 18 Feb 2026 14:29:48 -0600 Subject: [PATCH 1/2] AGENT-1443: Add IRI certificate regeneration to MCS cert rotation controller When the MCS CA rotates, the IRI TLS certificate (internal-release-image-tls) was not being regenerated, leaving it signed by the old CA. This adds a reconcileIRICertificate() method that generates a new IRI server cert signed by the current MCS CA and creates/updates the secret. The rotation is triggered on MCS CA rotation via CA bundle ConfigMap events. When the machine-config-server-ca Secret is deleted or expires, library-go's CertRotationController regenerates the CA key pair and updates the machine-config-server-ca ConfigMap (CA bundle). Our addConfigMap and updateConfigMap handlers detect this ConfigMap change and call reconcileIRICertificate() alongside reconcileUserDataSecrets(). Assisted-by: Claude Opus 4.6 --- .../certrotation/certrotation_controller.go | 91 ++++++++++++ .../certrotation_controller_test.go | 140 ++++++++++++++++++ 2 files changed, 231 insertions(+) diff --git a/pkg/controller/certrotation/certrotation_controller.go b/pkg/controller/certrotation/certrotation_controller.go index 823e347eb3..a417d1d162 100644 --- a/pkg/controller/certrotation/certrotation_controller.go +++ b/pkg/controller/certrotation/certrotation_controller.go @@ -10,10 +10,12 @@ import ( "github.com/vincent-petithory/dataurl" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" coreinformersv1 "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" @@ -28,6 +30,7 @@ import ( machineclientset "github.com/openshift/client-go/machine/clientset/versioned" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/crypto" "github.com/openshift/library-go/pkg/operator/certrotation" "github.com/openshift/library-go/pkg/operator/events" @@ -46,6 +49,7 @@ const ( mcsCARefresh = 8 * oneYear mcsTLSKeyExpiry = mcsCAExpiry mcsTLSKeyRefresh = mcsCARefresh + iriTLSKeyExpiry = mcsCAExpiry workQueueKey = "key" ) @@ -336,6 +340,9 @@ func (c *CertRotationController) addConfigMap(obj interface{}) { go func() { c.reconcileUserDataSecrets() }() + go func() { + c.reconcileIRICertificate() + }() } func (c *CertRotationController) updateConfigMap(oldCM, newCM interface{}) { @@ -359,6 +366,9 @@ func (c *CertRotationController) updateConfigMap(oldCM, newCM interface{}) { go func() { c.reconcileUserDataSecrets() }() + go func() { + c.reconcileIRICertificate() + }() } func (c *CertRotationController) addSecret(obj interface{}) { @@ -467,3 +477,84 @@ func (c *CertRotationController) reconcileSecret(secret corev1.Secret) error { klog.Infof("Successfully modified %s secret \n", secret.Name) return nil } + +func (c *CertRotationController) reconcileIRICertificate() { + klog.Infof("Reconciling IRI certificate") + + // Get the MCS CA secret (fresh get, not from lister) + caSecret, err := c.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.MachineConfigServerCAName, metav1.GetOptions{}) + if err != nil { + klog.Errorf("Cannot get MCS CA secret for IRI cert reconciliation: %v", err) + return + } + + caCert := caSecret.Data[corev1.TLSCertKey] + caKey := caSecret.Data[corev1.TLSPrivateKeyKey] + if len(caCert) == 0 || len(caKey) == 0 { + klog.Errorf("MCS CA secret %s is missing cert or key data", ctrlcommon.MachineConfigServerCAName) + return + } + + // Load the CA + ca, err := crypto.GetCAFromBytes(caCert, caKey) + if err != nil { + klog.Errorf("Cannot load MCS CA for IRI cert generation: %v", err) + return + } + + // Get hostnames from the dynamic serving rotation + hostnames := c.hostnamesRotation.GetHostnames() + if len(hostnames) == 0 { + klog.Errorf("No hostnames available for IRI cert generation") + return + } + + // Generate a new IRI TLS certificate signed by the MCS CA + certConfig, err := ca.MakeServerCert(sets.New(hostnames...), iriTLSKeyExpiry) + if err != nil { + klog.Errorf("Cannot generate IRI TLS certificate: %v", err) + return + } + + certPEM, keyPEM, err := certConfig.GetPEMBytes() + if err != nil { + klog.Errorf("Cannot get PEM bytes for IRI TLS certificate: %v", err) + return + } + + // Check if the IRI TLS secret already exists + iriSecret, err := c.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + if errors.IsNotFound(err) { + // Create new secret + newSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.InternalReleaseImageTLSSecretName, + Namespace: ctrlcommon.MCONamespace, + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: certPEM, + corev1.TLSPrivateKeyKey: keyPEM, + }, + } + if _, err := c.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Create(context.TODO(), newSecret, metav1.CreateOptions{}); err != nil { + klog.Errorf("Cannot create IRI TLS secret: %v", err) + return + } + klog.Infof("Successfully created IRI TLS secret %s", ctrlcommon.InternalReleaseImageTLSSecretName) + return + } else if err != nil { + klog.Errorf("Cannot get IRI TLS secret: %v", err) + return + } + + // Update existing secret + updatedSecret := iriSecret.DeepCopy() + updatedSecret.Data[corev1.TLSCertKey] = certPEM + updatedSecret.Data[corev1.TLSPrivateKeyKey] = keyPEM + if _, err := c.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update(context.TODO(), updatedSecret, metav1.UpdateOptions{}); err != nil { + klog.Errorf("Cannot update IRI TLS secret: %v", err) + return + } + klog.Infof("Successfully updated IRI TLS secret %s", ctrlcommon.InternalReleaseImageTLSSecretName) +} diff --git a/pkg/controller/certrotation/certrotation_controller_test.go b/pkg/controller/certrotation/certrotation_controller_test.go index 231a1ea672..3c624dc14f 100644 --- a/pkg/controller/certrotation/certrotation_controller_test.go +++ b/pkg/controller/certrotation/certrotation_controller_test.go @@ -349,6 +349,146 @@ func TestMCSCARotation(t *testing.T) { } } +func TestIRICertificateRotation(t *testing.T) { + tests := []struct { + name string + forceRotation bool + machineObjects []runtime.Object + maoSecrets []runtime.Object + preCreateIRI bool // whether to pre-create the IRI secret before reconciliation + }{ + { + name: "IRI secret is created on initial sync", + maoSecrets: []runtime.Object{getGoodMAOSecret("test-user-data")}, + machineObjects: []runtime.Object{getMachineSet("test-machine")}, + forceRotation: false, + preCreateIRI: false, + }, + { + name: "IRI secret is updated on CA rotation", + maoSecrets: []runtime.Object{getGoodMAOSecret("test-user-data")}, + machineObjects: []runtime.Object{getMachineSet("test-machine")}, + forceRotation: true, + preCreateIRI: false, + }, + { + name: "IRI secret is updated when it already exists", + maoSecrets: []runtime.Object{getGoodMAOSecret("test-user-data")}, + machineObjects: []runtime.Object{getMachineSet("test-machine")}, + forceRotation: false, + preCreateIRI: true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + f := newFixture(t) + f.machineObjects = append(f.machineObjects, test.machineObjects...) + f.objects = append(f.objects, test.maoSecrets...) + for _, obj := range test.maoSecrets { + f.maoSecretLister = append(f.maoSecretLister, obj.(*corev1.Secret)) + } + f.controller = f.newController() + + // Initial sync to create CA and MCS TLS cert + f.runController() + + if test.preCreateIRI { + // Pre-create an IRI secret with dummy data to test the update path + dummySecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.InternalReleaseImageTLSSecretName, + Namespace: ctrlcommon.MCONamespace, + }, + Type: corev1.SecretTypeTLS, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte("dummy-cert"), + corev1.TLSPrivateKeyKey: []byte("dummy-key"), + }, + } + _, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Create(context.TODO(), dummySecret, metav1.CreateOptions{}) + require.NoError(t, err) + } + + if test.forceRotation { + t.Log("Forcing CA rotation") + secret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.MachineConfigServerCAName, metav1.GetOptions{}) + require.NoError(t, err) + newSecret := secret.DeepCopy() + newSecret.Annotations[certrotation.CertificateNotAfterAnnotation] = time.Now().Add(-time.Hour).Format(time.RFC3339) + _, err = f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Update(context.TODO(), newSecret, metav1.UpdateOptions{}) + require.NoError(t, err) + f.syncListers(t) + f.runController() + } + + // Reconcile the IRI certificate + f.controller.reconcileIRICertificate() + + // Verify the IRI TLS secret was created/updated + iriSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + require.NoError(t, err, "IRI TLS secret should exist after reconciliation") + require.Equal(t, corev1.SecretTypeTLS, iriSecret.Type, "IRI secret should be of type TLS") + + // Parse the IRI certificate + iriCertData := iriSecret.Data[corev1.TLSCertKey] + require.NotEmpty(t, iriCertData, "IRI certificate data should not be empty") + + block, _ := pem.Decode(iriCertData) + require.NotNil(t, block, "Should be able to decode IRI PEM certificate") + + iriCert, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err, "Should be able to parse IRI certificate") + + // Get the MCS CA certificate to verify the IRI cert is signed by it + caSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.MachineConfigServerCAName, metav1.GetOptions{}) + require.NoError(t, err) + caCertData := caSecret.Data[corev1.TLSCertKey] + require.NotEmpty(t, caCertData, "CA certificate data should not be empty") + + caBlock, _ := pem.Decode(caCertData) + require.NotNil(t, caBlock, "Should be able to decode CA PEM certificate") + caCert, err := x509.ParseCertificate(caBlock.Bytes) + require.NoError(t, err, "Should be able to parse CA certificate") + + // Verify the IRI cert is signed by the MCS CA + err = iriCert.CheckSignatureFrom(caCert) + require.NoError(t, err, "IRI certificate should be signed by the MCS CA") + + // Verify the IRI cert has the correct SANs (hostnames from hostnamesRotation) + expectedHostnames := f.controller.hostnamesRotation.GetHostnames() + require.NotEmpty(t, expectedHostnames, "Expected hostnames should not be empty") + + for _, hostname := range expectedHostnames { + ip := net.ParseIP(hostname) + if ip != nil { + found := false + for _, certIP := range iriCert.IPAddresses { + if certIP.Equal(ip) { + found = true + break + } + } + require.True(t, found, "IP %s should be present in IRI certificate SAN IP addresses", hostname) + } else { + found := false + for _, dnsName := range iriCert.DNSNames { + if dnsName == hostname { + found = true + break + } + } + require.True(t, found, "Hostname %s should be present in IRI certificate SAN DNS names", hostname) + } + } + + t.Logf("Successfully verified IRI certificate: signed by MCS CA, correct SANs") + }) + } +} + // Update the controller's indexers to capture the new secrets and configmaps func (f *fixture) syncListers(t *testing.T) { signingSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.MachineConfigServerCAName, metav1.GetOptions{}) From 1ecca84cd92ee752ff3fa7f1ddab7782f7e765de Mon Sep 17 00:00:00 2001 From: Richard Su Date: Thu, 26 Feb 2026 16:10:22 -0600 Subject: [PATCH 2/2] AGENT-1443: Add feature gate, idempotency, and localhost SANs to IRI cert rotation Gate IRI certificate reconciliation on the NoRegistryClusterInstall feature flag so it only runs on clusters that use the IRI registry. Add localhost, 127.0.0.1, and ::1 to the IRI certificate SANs to match the installer-generated certificate. Add idempotency check that verifies the existing IRI cert against the current MCS CA before regenerating, preventing unnecessary Secret updates and node rollouts on controller restarts. Assisted-by: Claude Opus 4.6 --- cmd/machine-config-controller/start.go | 1 + .../certrotation/certrotation_controller.go | 84 ++++++- .../certrotation_controller_test.go | 228 ++++++++++-------- 3 files changed, 208 insertions(+), 105 deletions(-) diff --git a/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index 81ec8793f4..b8580e6512 100644 --- a/cmd/machine-config-controller/start.go +++ b/cmd/machine-config-controller/start.go @@ -107,6 +107,7 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx.KubeNamespacedInformerFactory.Core().V1().Secrets(), ctrlctx.KubeNamespacedInformerFactory.Core().V1().ConfigMaps(), ctrlctx.ConfigInformerFactory.Config().V1().Infrastructures(), + ctrlctx.FeatureGatesHandler, ) if err != nil { klog.Fatalf("unable to start cert rotation controller: %v", err) diff --git a/pkg/controller/certrotation/certrotation_controller.go b/pkg/controller/certrotation/certrotation_controller.go index a417d1d162..26659b3cbf 100644 --- a/pkg/controller/certrotation/certrotation_controller.go +++ b/pkg/controller/certrotation/certrotation_controller.go @@ -3,7 +3,9 @@ package certrotationcontroller import ( "bytes" "context" + "crypto/x509" "encoding/json" + "encoding/pem" "fmt" "time" @@ -26,6 +28,7 @@ import ( "k8s.io/utils/clock" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" configclientset "github.com/openshift/client-go/config/clientset/versioned" machineclientset "github.com/openshift/client-go/machine/clientset/versioned" @@ -74,6 +77,8 @@ type CertRotationController struct { recorder events.Recorder cachesToSync []cache.InformerSynced + + featureGatesHandler ctrlcommon.FeatureGatesHandler } // New returns a new cert rotation controller. @@ -86,6 +91,7 @@ func New( mcoSecretInformer coreinformersv1.SecretInformer, mcoConfigMapInfomer coreinformersv1.ConfigMapInformer, infraInformer configinformers.InfrastructureInformer, + featureGatesHandler ctrlcommon.FeatureGatesHandler, ) (*CertRotationController, error) { recorder := events.NewLoggingEventRecorder(componentName, clock.RealClock{}) @@ -110,8 +116,9 @@ func New( hostnamesQueue: workqueue.NewTypedRateLimitingQueueWithConfig( workqueue.DefaultTypedControllerRateLimiter[string](), workqueue.TypedRateLimitingQueueConfig[string]{Name: "Hostnames"}), - infraInformer: infraInformer, - infraLister: infraInformer.Lister(), + infraInformer: infraInformer, + infraLister: infraInformer.Lister(), + featureGatesHandler: featureGatesHandler, } // The cert controller will begin creating "machine-config-server-ca" configmap & secret in the MCO namespace. @@ -335,7 +342,7 @@ func (c *CertRotationController) addConfigMap(obj interface{}) { return } - klog.Infof("configMap %s added, reconciling all user data secrets", configMap.Name) + klog.Infof("configMap %s added, reconciling user data secrets and IRI certificate", configMap.Name) go func() { c.reconcileUserDataSecrets() @@ -360,7 +367,7 @@ func (c *CertRotationController) updateConfigMap(oldCM, newCM interface{}) { return } - klog.Infof("configMap %s updated, reconciling all user data secrets", oldConfigMap.Name) + klog.Infof("configMap %s updated, reconciling user data secrets and IRI certificate", oldConfigMap.Name) // Reconcile all user data secrets go func() { @@ -479,6 +486,10 @@ func (c *CertRotationController) reconcileSecret(secret corev1.Secret) error { } func (c *CertRotationController) reconcileIRICertificate() { + if !c.featureGatesHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) { + klog.V(4).Infof("Skipping IRI certificate reconciliation: %s feature gate is not enabled", features.FeatureGateNoRegistryClusterInstall) + return + } klog.Infof("Reconciling IRI certificate") // Get the MCS CA secret (fresh get, not from lister) @@ -502,12 +513,27 @@ func (c *CertRotationController) reconcileIRICertificate() { return } - // Get hostnames from the dynamic serving rotation + // Check if the existing IRI cert is already valid under the current CA + iriSecret, err := c.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + secretExists := err == nil + if secretExists { + if c.isIRICertValid(iriSecret, ca) { + klog.Infof("IRI TLS certificate is still valid under the current MCS CA, skipping rotation") + return + } + } else if !errors.IsNotFound(err) { + klog.Errorf("Cannot get IRI TLS secret: %v", err) + return + } + + // Get hostnames from the dynamic serving rotation (includes api-int hostname and platform VIPs) hostnames := c.hostnamesRotation.GetHostnames() if len(hostnames) == 0 { klog.Errorf("No hostnames available for IRI cert generation") return } + // IRI registry also serves locally on each master node, matching the installer's SANs + hostnames = append(hostnames, "localhost", "127.0.0.1", "::1") // Generate a new IRI TLS certificate signed by the MCS CA certConfig, err := ca.MakeServerCert(sets.New(hostnames...), iriTLSKeyExpiry) @@ -522,9 +548,7 @@ func (c *CertRotationController) reconcileIRICertificate() { return } - // Check if the IRI TLS secret already exists - iriSecret, err := c.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) - if errors.IsNotFound(err) { + if !secretExists { // Create new secret newSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -543,9 +567,6 @@ func (c *CertRotationController) reconcileIRICertificate() { } klog.Infof("Successfully created IRI TLS secret %s", ctrlcommon.InternalReleaseImageTLSSecretName) return - } else if err != nil { - klog.Errorf("Cannot get IRI TLS secret: %v", err) - return } // Update existing secret @@ -558,3 +579,44 @@ func (c *CertRotationController) reconcileIRICertificate() { } klog.Infof("Successfully updated IRI TLS secret %s", ctrlcommon.InternalReleaseImageTLSSecretName) } + +// isIRICertValid checks whether the existing IRI certificate is signed by the given CA +// and has not expired. +func (c *CertRotationController) isIRICertValid(iriSecret *corev1.Secret, ca *crypto.CA) bool { + certPEM := iriSecret.Data[corev1.TLSCertKey] + if len(certPEM) == 0 { + return false + } + + // Decode the first PEM block (the leaf certificate) + block, _ := pem.Decode(certPEM) + if block == nil { + klog.Warningf("Cannot decode PEM from existing IRI TLS certificate") + return false + } + + iriCert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + klog.Warningf("Cannot parse existing IRI TLS certificate: %v", err) + return false + } + + // Build a cert pool with the current CA to verify against + caPool := x509.NewCertPool() + for _, caCert := range ca.Config.Certs { + caPool.AddCert(caCert) + } + + // Verify the IRI cert is signed by the current CA and is not expired + _, err = iriCert.Verify(x509.VerifyOptions{ + Roots: caPool, + CurrentTime: time.Now(), + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + }) + if err != nil { + klog.Infof("Existing IRI TLS certificate is not valid under current MCS CA: %v", err) + return false + } + + return true +} diff --git a/pkg/controller/certrotation/certrotation_controller_test.go b/pkg/controller/certrotation/certrotation_controller_test.go index 3c624dc14f..1109570f16 100644 --- a/pkg/controller/certrotation/certrotation_controller_test.go +++ b/pkg/controller/certrotation/certrotation_controller_test.go @@ -12,10 +12,12 @@ import ( "github.com/stretchr/testify/require" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" configinformers "github.com/openshift/client-go/config/informers/externalversions" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/certrotation" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -111,7 +113,11 @@ func (f *fixture) newController() *CertRotationController { f.infraLister = append(f.infraLister, infra.(*configv1.Infrastructure)) } - c, err := New(f.kubeClient, f.configClient, f.machineClient, f.aroClient, f.k8sI.Core().V1().Secrets(), f.k8sI.Core().V1().Secrets(), f.k8sI.Core().V1().ConfigMaps(), f.infraInformer.Config().V1().Infrastructures()) + fgHandler := ctrlcommon.NewFeatureGatesHardcodedHandler( + []configv1.FeatureGateName{features.FeatureGateNoRegistryClusterInstall}, + nil, + ) + c, err := New(f.kubeClient, f.configClient, f.machineClient, f.aroClient, f.k8sI.Core().V1().Secrets(), f.k8sI.Core().V1().Secrets(), f.k8sI.Core().V1().ConfigMaps(), f.infraInformer.Config().V1().Infrastructures(), fgHandler) require.NoError(f.t, err) c.StartInformers() @@ -351,32 +357,22 @@ func TestMCSCARotation(t *testing.T) { func TestIRICertificateRotation(t *testing.T) { tests := []struct { - name string - forceRotation bool - machineObjects []runtime.Object - maoSecrets []runtime.Object - preCreateIRI bool // whether to pre-create the IRI secret before reconciliation + name string + iriSecretAlreadyExists bool + forceRotation bool }{ { - name: "IRI secret is created on initial sync", - maoSecrets: []runtime.Object{getGoodMAOSecret("test-user-data")}, - machineObjects: []runtime.Object{getMachineSet("test-machine")}, - forceRotation: false, - preCreateIRI: false, + // Covers the case where the IRI secret has been manually deleted + // (e.g., accidental "oc delete secret internal-release-image-tls") + // and the controller recreates it. + name: "IRI secret is created when it does not already exist", + iriSecretAlreadyExists: false, + forceRotation: false, }, { - name: "IRI secret is updated on CA rotation", - maoSecrets: []runtime.Object{getGoodMAOSecret("test-user-data")}, - machineObjects: []runtime.Object{getMachineSet("test-machine")}, - forceRotation: true, - preCreateIRI: false, - }, - { - name: "IRI secret is updated when it already exists", - maoSecrets: []runtime.Object{getGoodMAOSecret("test-user-data")}, - machineObjects: []runtime.Object{getMachineSet("test-machine")}, - forceRotation: false, - preCreateIRI: true, + name: "IRI secret is updated on CA rotation", + iriSecretAlreadyExists: true, + forceRotation: true, }, } @@ -385,31 +381,25 @@ func TestIRICertificateRotation(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() f := newFixture(t) - f.machineObjects = append(f.machineObjects, test.machineObjects...) - f.objects = append(f.objects, test.maoSecrets...) - for _, obj := range test.maoSecrets { - f.maoSecretLister = append(f.maoSecretLister, obj.(*corev1.Secret)) - } + maoSecret := getGoodMAOSecret("test-user-data") + f.machineObjects = append(f.machineObjects, getMachineSet("test-machine")) + f.objects = append(f.objects, maoSecret) + f.maoSecretLister = append(f.maoSecretLister, maoSecret) f.controller = f.newController() // Initial sync to create CA and MCS TLS cert f.runController() - if test.preCreateIRI { - // Pre-create an IRI secret with dummy data to test the update path - dummySecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: ctrlcommon.InternalReleaseImageTLSSecretName, - Namespace: ctrlcommon.MCONamespace, - }, - Type: corev1.SecretTypeTLS, - Data: map[string][]byte{ - corev1.TLSCertKey: []byte("dummy-cert"), - corev1.TLSPrivateKeyKey: []byte("dummy-key"), - }, - } - _, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Create(context.TODO(), dummySecret, metav1.CreateOptions{}) - require.NoError(t, err) + var originalCertData []byte + if test.iriSecretAlreadyExists { + // Create the IRI cert under the current CA + f.controller.reconcileIRICertificate() + existingSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + require.NoError(t, err, "IRI TLS secret should exist after initial reconciliation") + originalCertData = existingSecret.Data[corev1.TLSCertKey] + } else { + _, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + require.True(t, errors.IsNotFound(err), "IRI TLS secret should not exist before reconciliation") } if test.forceRotation { @@ -432,61 +422,111 @@ func TestIRICertificateRotation(t *testing.T) { require.NoError(t, err, "IRI TLS secret should exist after reconciliation") require.Equal(t, corev1.SecretTypeTLS, iriSecret.Type, "IRI secret should be of type TLS") - // Parse the IRI certificate - iriCertData := iriSecret.Data[corev1.TLSCertKey] - require.NotEmpty(t, iriCertData, "IRI certificate data should not be empty") - - block, _ := pem.Decode(iriCertData) - require.NotNil(t, block, "Should be able to decode IRI PEM certificate") - - iriCert, err := x509.ParseCertificate(block.Bytes) - require.NoError(t, err, "Should be able to parse IRI certificate") - - // Get the MCS CA certificate to verify the IRI cert is signed by it - caSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.MachineConfigServerCAName, metav1.GetOptions{}) - require.NoError(t, err) - caCertData := caSecret.Data[corev1.TLSCertKey] - require.NotEmpty(t, caCertData, "CA certificate data should not be empty") - - caBlock, _ := pem.Decode(caCertData) - require.NotNil(t, caBlock, "Should be able to decode CA PEM certificate") - caCert, err := x509.ParseCertificate(caBlock.Bytes) - require.NoError(t, err, "Should be able to parse CA certificate") - - // Verify the IRI cert is signed by the MCS CA - err = iriCert.CheckSignatureFrom(caCert) - require.NoError(t, err, "IRI certificate should be signed by the MCS CA") - - // Verify the IRI cert has the correct SANs (hostnames from hostnamesRotation) - expectedHostnames := f.controller.hostnamesRotation.GetHostnames() - require.NotEmpty(t, expectedHostnames, "Expected hostnames should not be empty") - - for _, hostname := range expectedHostnames { - ip := net.ParseIP(hostname) - if ip != nil { - found := false - for _, certIP := range iriCert.IPAddresses { - if certIP.Equal(ip) { - found = true - break - } - } - require.True(t, found, "IP %s should be present in IRI certificate SAN IP addresses", hostname) - } else { - found := false - for _, dnsName := range iriCert.DNSNames { - if dnsName == hostname { - found = true - break - } - } - require.True(t, found, "Hostname %s should be present in IRI certificate SAN DNS names", hostname) - } + // If the IRI secret existed before, verify the cert data actually changed + if test.iriSecretAlreadyExists { + require.NotEqual(t, originalCertData, iriSecret.Data[corev1.TLSCertKey], "IRI certificate data should have changed after CA rotation") } - t.Logf("Successfully verified IRI certificate: signed by MCS CA, correct SANs") + f.verifyIRICertificate(t) }) } + + // Verifies idempotency: if the IRI cert is already valid under the + // current CA, reconcileIRICertificate should skip regeneration. + t.Run("IRI secret is not regenerated when already valid", func(t *testing.T) { + t.Parallel() + f := newFixture(t) + maoSecret := getGoodMAOSecret("test-user-data") + f.machineObjects = append(f.machineObjects, getMachineSet("test-machine")) + f.objects = append(f.objects, maoSecret) + f.maoSecretLister = append(f.maoSecretLister, maoSecret) + f.controller = f.newController() + + // Initial sync to create CA and MCS TLS cert + f.runController() + + // First reconciliation creates the IRI cert + f.controller.reconcileIRICertificate() + + // Get the IRI secret after first reconciliation + iriSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + require.NoError(t, err, "IRI TLS secret should exist after first reconciliation") + originalResourceVersion := iriSecret.ResourceVersion + originalCertData := iriSecret.Data[corev1.TLSCertKey] + + // Second reconciliation should skip regeneration + f.controller.reconcileIRICertificate() + + // Verify the secret was not updated + iriSecret, err = f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, originalResourceVersion, iriSecret.ResourceVersion, "IRI secret should not have been updated") + require.Equal(t, originalCertData, iriSecret.Data[corev1.TLSCertKey], "IRI certificate data should not have changed") + + t.Logf("Successfully verified IRI certificate was not regenerated when already valid") + }) +} + +// verifyIRICertificate checks that the IRI TLS certificate is signed by the current +// MCS CA and contains the expected SANs (hostnames from hostnamesRotation + localhost SANs). +func (f *fixture) verifyIRICertificate(t *testing.T) { + t.Helper() + + iriSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.InternalReleaseImageTLSSecretName, metav1.GetOptions{}) + require.NoError(t, err, "IRI TLS secret should exist") + + iriCertData := iriSecret.Data[corev1.TLSCertKey] + require.NotEmpty(t, iriCertData, "IRI certificate data should not be empty") + + block, _ := pem.Decode(iriCertData) + require.NotNil(t, block, "Should be able to decode IRI PEM certificate") + + iriCert, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err, "Should be able to parse IRI certificate") + + // Verify the IRI cert is signed by the MCS CA + caSecret, err := f.kubeClient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), ctrlcommon.MachineConfigServerCAName, metav1.GetOptions{}) + require.NoError(t, err) + caCertData := caSecret.Data[corev1.TLSCertKey] + require.NotEmpty(t, caCertData, "CA certificate data should not be empty") + + caBlock, _ := pem.Decode(caCertData) + require.NotNil(t, caBlock, "Should be able to decode CA PEM certificate") + caCert, err := x509.ParseCertificate(caBlock.Bytes) + require.NoError(t, err, "Should be able to parse CA certificate") + + err = iriCert.CheckSignatureFrom(caCert) + require.NoError(t, err, "IRI certificate should be signed by the MCS CA") + + // Verify the IRI cert has the correct SANs (hostnames from hostnamesRotation + localhost SANs) + expectedHostnames := f.controller.hostnamesRotation.GetHostnames() + require.NotEmpty(t, expectedHostnames, "Expected hostnames should not be empty") + expectedHostnames = append(expectedHostnames, "localhost", "127.0.0.1", "::1") + + for _, hostname := range expectedHostnames { + ip := net.ParseIP(hostname) + if ip != nil { + found := false + for _, certIP := range iriCert.IPAddresses { + if certIP.Equal(ip) { + found = true + break + } + } + require.True(t, found, "IP %s should be present in IRI certificate SAN IP addresses", hostname) + } else { + found := false + for _, dnsName := range iriCert.DNSNames { + if dnsName == hostname { + found = true + break + } + } + require.True(t, found, "Hostname %s should be present in IRI certificate SAN DNS names", hostname) + } + } + + t.Logf("Successfully verified IRI certificate: signed by MCS CA, correct SANs") } // Update the controller's indexers to capture the new secrets and configmaps