Merge pull request #599 from lmiccini/instanceha-hmac-auto-nullify

Auto-nullify previous HMAC key after EDPM deployment completes

Author: openshift-merge-bot[bot]

apis/bases/instanceha.openstack.org_instancehas.yaml

@@ -233,6 +233,14 @@ spec:
                   HeartbeatHMACSecret is the name of the auto-generated Secret
                   containing the HMAC key for heartbeat authentication
                 type: string
+              hmacKeyRotationSynced:
+                description: |-
+                  HMACKeyRotationSynced tracks EDPM deployment progress during HMAC
+                  key rotation. See HMACRotationSyncState constants.
+                enum:
+                - ""
+                - WaitingForDeployment
+                type: string
               lastAppliedTopology:
                 description: LastAppliedTopology - the last applied Topology
                 properties:

apis/go.mod

@@ -5,7 +5,7 @@ go 1.24.4
 require (
 	github.com/go-logr/logr v1.4.3
 	github.com/onsi/gomega v1.41.0
-	github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db
+	github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e
 	k8s.io/api v0.31.14
 	k8s.io/apiextensions-apiserver v0.33.2
 	k8s.io/apimachinery v0.31.14

apis/go.sum

@@ -83,8 +83,8 @@ github.com/onsi/gomega v1.41.0 h1:OwKp4pXNgVxf6sCplzYo794OFNuoL2q2SBMU5NSWOjA=
 github.com/onsi/gomega v1.41.0/go.mod h1:M/Uqpu/8qTjtzCLUA2zJHX9Iilrau25x1PdoSRbWh5A=
 github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e h1:E1OdwSpqWuDPCedyUt0GEdoAE+r5TXy7YS21yNEo+2U=
 github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db h1:wSTVKqPOgAboAX23GHHW6J1vJxaHCq/5InQDXFbE8EI=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e h1:Tj/jlh16cQ2EPTYbH1qQpRYoQPqpLtWvI6JVQiRA+jA=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

apis/instanceha/v1beta1/instanceha_types.go

@@ -34,6 +34,17 @@ const (
 	OpenStackCloud = "default"
 )
 
+// HMACRotationSyncState tracks EDPM deployment progress during HMAC key rotation.
+// +kubebuilder:validation:Enum="";WaitingForDeployment
+type HMACRotationSyncState string
+
+const (
+	// HMACRotationIdle - no rotation in progress or waiting for EDPM to detect the key change.
+	HMACRotationIdle HMACRotationSyncState = ""
+	// HMACRotationWaitingDeployment - EDPM detected the key change, waiting for deployment to complete.
+	HMACRotationWaitingDeployment HMACRotationSyncState = "WaitingForDeployment"
+)
+
 // AuthSpec defines authentication parameters for InstanceHA.
 // Compatible with the AuthSpec interface used by other openstack-k8s-operators.
 type AuthSpec struct {
@@ -165,6 +176,10 @@ type InstanceHaStatus struct {
 	// HeartbeatHMACSecret is the name of the auto-generated Secret
 	// containing the HMAC key for heartbeat authentication
 	HeartbeatHMACSecret string `json:"heartbeatHMACSecret,omitempty"`
+
+	// HMACKeyRotationSynced tracks EDPM deployment progress during HMAC
+	// key rotation. See HMACRotationSyncState constants.
+	HMACKeyRotationSynced HMACRotationSyncState `json:"hmacKeyRotationSynced,omitempty"`
 }
 
 //+kubebuilder:object:root=true

config/crd/bases/instanceha.openstack.org_instancehas.yaml

@@ -233,6 +233,14 @@ spec:
                   HeartbeatHMACSecret is the name of the auto-generated Secret
                   containing the HMAC key for heartbeat authentication
                 type: string
+              hmacKeyRotationSynced:
+                description: |-
+                  HMACKeyRotationSynced tracks EDPM deployment progress during HMAC
+                  key rotation. See HMACRotationSyncState constants.
+                enum:
+                - ""
+                - WaitingForDeployment
+                type: string
               lastAppliedTopology:
                 description: LastAppliedTopology - the last applied Topology
                 properties:

docs/instanceha_guide.md

@@ -645,13 +645,13 @@ The packet format includes a timestamp for replay protection (packets older than
 kubectl annotate instanceha instanceha instanceha.openstack.org/rotate-hmac-key=true
 ```
 
-This copies the current key to `hmac-key-previous`, generates a new key, and removes the annotation. The receiver accepts both keys during the rotation window. After redeploying compute nodes with the new key, the previous key can be cleared:
+This copies the current key to `hmac-key-previous`, generates a new key, and removes the annotation. The receiver accepts both keys during the rotation window.
 
-```bash
-kubectl patch secret instanceha-heartbeat-hmac -p '{"data":{"hmac-key-previous":""}}'
-```
+After redeploying compute nodes with the new key (via `servicesOverride: [instanceha-monitoring]`), the controller **automatically clears `hmac-key-previous`** once the EDPM deployment completes and the secret hash is back in sync across all NodeSets. This uses a two-phase state machine tracked in `status.hmacKeyRotationSynced`.
+
+If no `OpenStackDataPlaneNodeSets` exist in the namespace, the previous key is cleared immediately.
 
-Replace `instanceha-heartbeat-hmac` with `<instance-name>-heartbeat-hmac` if your CR has a different name.
+> **Prerequisite:** The `instanceha-monitoring` service must be listed in the NodeSet's `spec.services` so its secret hash is tracked. The service's `secretRef.name` must match the actual secret name (`<instance-name>-heartbeat-hmac`). For non-default CR names, create a custom `OpenStackDataPlaneService` with the correct `secretRef`.
 
 The secret name is published in `status.heartbeatHMACSecret` for integration with the EDPM dataplane operator.
 

go.mod

@@ -11,9 +11,9 @@ require (
 	github.com/onsi/gomega v1.41.0
 	github.com/openshift/api v3.9.0+incompatible
 	github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-00010101000000-000000000000
-	github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db
-	github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260526114926-7ebfadd589db
-	github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260526114926-7ebfadd589db
+	github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e
+	github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260608153450-ce00f678385e
+	github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260608153450-ce00f678385e
 	go.uber.org/zap v1.28.0
 	golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67
 	k8s.io/api v0.31.14

go.sum

@@ -118,12 +118,12 @@ github.com/onsi/gomega v1.41.0 h1:OwKp4pXNgVxf6sCplzYo794OFNuoL2q2SBMU5NSWOjA=
 github.com/onsi/gomega v1.41.0/go.mod h1:M/Uqpu/8qTjtzCLUA2zJHX9Iilrau25x1PdoSRbWh5A=
 github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e h1:E1OdwSpqWuDPCedyUt0GEdoAE+r5TXy7YS21yNEo+2U=
 github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db h1:wSTVKqPOgAboAX23GHHW6J1vJxaHCq/5InQDXFbE8EI=
-github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
-github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260526114926-7ebfadd589db h1:5sOkESe/MppdwP0mBbKnzHMnugg3gfGY+pWx0u8npa8=
-github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260526114926-7ebfadd589db/go.mod h1:xsKeDFU3/xEObaVDqd6XEYV3MzvFswbWMlnr2Z3q3ZI=
-github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260526114926-7ebfadd589db h1:tEASfqRI3+4va7+jr/qoUnPn+7R6RCkHitwx6k8960U=
-github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260526114926-7ebfadd589db/go.mod h1:nLS2oK4pBo756JNN1cPgr44S0X9V11QScgVla89Ojok=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e h1:Tj/jlh16cQ2EPTYbH1qQpRYoQPqpLtWvI6JVQiRA+jA=
+github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
+github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260608153450-ce00f678385e h1:jgGl2fxsn2Cl8NQ5+3obEg+H/WOvIQ//q/jJwHmPuRo=
+github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260608153450-ce00f678385e/go.mod h1:xsKeDFU3/xEObaVDqd6XEYV3MzvFswbWMlnr2Z3q3ZI=
+github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260608153450-ce00f678385e h1:k3mCwZ2yO2TK4u/2mdPX2SAXTsMr5m8AXG9gUO1zhzk=
+github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260608153450-ce00f678385e/go.mod h1:nLS2oK4pBo756JNN1cPgr44S0X9V11QScgVla89Ojok=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

internal/controller/instanceha/instanceha_controller.go

@@ -36,6 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -62,6 +63,8 @@ import (
 	commonservice "github.com/openstack-k8s-operators/lib-common/modules/common/service"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 
+	edpm "github.com/openstack-k8s-operators/lib-common/modules/edpm/unstructured"
+
 	networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	instancehav1 "github.com/openstack-k8s-operators/infra-operator/apis/instanceha/v1beta1"
 	topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
@@ -96,6 +99,7 @@ func (r *Reconciler) GetLogger(ctx context.Context) logr.Logger {
 // +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
 // +kubebuilder:rbac:groups=topology.openstack.org,resources=topologies,verbs=get;list;watch;update
+// +kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplanenodesets,verbs=get;list;watch
 
 // Reconcile -
 func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, _err error) {
@@ -564,6 +568,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 		return ctrl.Result{RequeueAfter: time.Second}, nil
 	}
 
+	if ctrlResult, err := r.tryNullifyPreviousKey(ctx, instance, hmacSecret); err != nil || ctrlResult.RequeueAfter > 0 {
+		return ctrlResult, err
+	}
+
 	_, hmacSecretHash, err := secret.GetSecret(ctx, helper, heartbeatHMACSecretName, instance.Namespace)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to hash heartbeat HMAC secret: %w", err)
@@ -848,7 +856,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		return err
 	}
 
-	return ctrl.NewControllerManagedBy(mgr).
+	b := ctrl.NewControllerManagedBy(mgr).
 		For(&instancehav1.InstanceHa{}).
 		Owns(&appsv1.Deployment{}).
 		Owns(&corev1.Service{}).
@@ -869,8 +877,18 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		).
 		Watches(&topologyv1.Topology{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
-			builder.WithPredicates(predicate.GenerationChangedPredicate{})).
-		Complete(r)
+			builder.WithPredicates(predicate.GenerationChangedPredicate{}))
+
+	gk := schema.GroupKind{Group: edpm.NodeSetGVK.Group, Kind: edpm.NodeSetGVK.Kind}
+	if _, err := mgr.GetRESTMapper().RESTMapping(gk, edpm.NodeSetGVK.Version); err == nil {
+		b = b.Watches(
+			edpm.NewNodeSetObject(),
+			handler.EnqueueRequestsFromMapFunc(r.findInstanceHaWithPendingRotation),
+			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
+		)
+	}
+
+	return b.Complete(r)
 }
 
 func (r *Reconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
@@ -921,13 +939,72 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, instance *instancehav1
 		return ctrlResult, err
 	}
 
+	// Clear any in-flight rotation state.
+	instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationIdle
+
 	// Service is deleted so remove the finalizer.
 	controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
 	Log.Info("Reconciled Service delete successfully")
 
 	return ctrl.Result{}, nil
 }
 
+func (r *Reconciler) tryNullifyPreviousKey(ctx context.Context, instance *instancehav1.InstanceHa, hmacSecret *corev1.Secret) (ctrl.Result, error) {
+	Log := r.GetLogger(ctx)
+
+	if len(hmacSecret.Data["hmac-key-previous"]) == 0 {
+		if instance.Status.HMACKeyRotationSynced != instancehav1.HMACRotationIdle {
+			instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationIdle
+		}
+		return ctrl.Result{}, nil
+	}
+
+	secretName := hmacSecret.Name
+	inSync, info, err := edpm.IsSecretHashInSync(ctx, r.Client, instance.Namespace, secretName)
+	if err != nil {
+		Log.Error(err, "Failed to check nodeset sync status for HMAC key rotation, will retry")
+		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
+	}
+
+	if !inSync {
+		if instance.Status.HMACKeyRotationSynced != instancehav1.HMACRotationWaitingDeployment {
+			instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationWaitingDeployment
+			Log.Info("HMAC key change detected by EDPM, waiting for deployment",
+				"secret", secretName, "info", info)
+		} else {
+			Log.Info("HMAC key deployment in progress", "secret", secretName, "info", info)
+		}
+		return ctrl.Result{}, nil
+	}
+
+	hmacSecret.Data["hmac-key-previous"] = []byte{}
+	if err := r.Update(ctx, hmacSecret); err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to nullify previous HMAC key: %w", err)
+	}
+	instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationIdle
+	Log.Info("Nullified previous HMAC key after successful EDPM deployment", "secret", secretName)
+	return ctrl.Result{}, nil
+}
+
+func (r *Reconciler) findInstanceHaWithPendingRotation(ctx context.Context, _ client.Object) []reconcile.Request {
+	crList := &instancehav1.InstanceHaList{}
+	if err := r.List(ctx, crList); err != nil {
+		return nil
+	}
+	var requests []reconcile.Request
+	for _, cr := range crList.Items {
+		if cr.Status.HMACKeyRotationSynced != instancehav1.HMACRotationIdle {
+			requests = append(requests, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      cr.Name,
+					Namespace: cr.Namespace,
+				},
+			})
+		}
+	}
+	return requests
+}
+
 // GetContainerImage returns the container image to use for the instance, either from
 // the provided containerImage parameter or from the infra-instanceha-config ConfigMap
 func (r *Reconciler) GetContainerImage(

test/functional/instanceha_controller_test.go

@@ -511,12 +511,13 @@ var _ = Describe("InstanceHa Controller", func() {
 				g.Expect(k8sClient.Update(ctx, instance)).Should(Succeed())
 			}, timeout, interval).Should(Succeed())
 
-			// Verify the key was rotated: new current key, previous = old current
+			// Verify the key was rotated and previous key auto-cleared
+			// (no NodeSets in envtest, so previous key is nullified immediately)
 			Eventually(func(g Gomega) {
 				hmacSecret := &corev1.Secret{}
 				g.Expect(k8sClient.Get(ctx, hmacSecretName, hmacSecret)).Should(Succeed())
 				g.Expect(hmacSecret.Data["hmac-key"]).ToNot(Equal(originalKey), "current key should have changed")
-				g.Expect(hmacSecret.Data["hmac-key-previous"]).To(Equal(originalKey), "previous key should be the old current key")
+				g.Expect(hmacSecret.Data["hmac-key-previous"]).To(BeEmpty(), "previous key should be auto-cleared when no NodeSets exist")
 			}, timeout, interval).Should(Succeed())
 
 			// Verify the annotation was removed