Skip to content

Commit 4bc4e2e

Browse files
lmicciniclaude
andcommitted
Auto-nullify previous HMAC key after EDPM deployment completes
After an HMAC key rotation, the previous key remained valid indefinitely until an operator manually cleared it. This widened the replay/compromise window if the cleanup was forgotten. Add a two-phase state machine (modeled on the RabbitMQ TransportURL credential rotation pattern) that tracks EDPM deployment progress via the new lib-common IsSecretHashInSync helper: Phase 1: detect that the HMAC secret hash has gone out of sync with what NodeSets have deployed (config regenerated with new key). Phase 2: wait for the hash to come back in sync (deployment completed, all compute nodes have the new key). Once both phases complete, hmac-key-previous is automatically cleared. If no NodeSets exist, the previous key is cleared immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e3fdd67 commit 4bc4e2e

10 files changed

Lines changed: 131 additions & 22 deletions

File tree

apis/bases/instanceha.openstack.org_instancehas.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,14 @@ spec:
233233
HeartbeatHMACSecret is the name of the auto-generated Secret
234234
containing the HMAC key for heartbeat authentication
235235
type: string
236+
hmacKeyRotationSynced:
237+
description: |-
238+
HMACKeyRotationSynced tracks EDPM deployment progress during HMAC
239+
key rotation. See HMACRotationSyncState constants.
240+
enum:
241+
- ""
242+
- WaitingForDeployment
243+
type: string
236244
lastAppliedTopology:
237245
description: LastAppliedTopology - the last applied Topology
238246
properties:

apis/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ go 1.24.4
55
require (
66
github.com/go-logr/logr v1.4.3
77
github.com/onsi/gomega v1.41.0
8-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db
8+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e
99
k8s.io/api v0.31.14
1010
k8s.io/apiextensions-apiserver v0.33.2
1111
k8s.io/apimachinery v0.31.14

apis/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ github.com/onsi/gomega v1.41.0 h1:OwKp4pXNgVxf6sCplzYo794OFNuoL2q2SBMU5NSWOjA=
8383
github.com/onsi/gomega v1.41.0/go.mod h1:M/Uqpu/8qTjtzCLUA2zJHX9Iilrau25x1PdoSRbWh5A=
8484
github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e h1:E1OdwSpqWuDPCedyUt0GEdoAE+r5TXy7YS21yNEo+2U=
8585
github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
86-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db h1:wSTVKqPOgAboAX23GHHW6J1vJxaHCq/5InQDXFbE8EI=
87-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
86+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e h1:Tj/jlh16cQ2EPTYbH1qQpRYoQPqpLtWvI6JVQiRA+jA=
87+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
8888
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
8989
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
9090
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

apis/instanceha/v1beta1/instanceha_types.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ const (
3434
OpenStackCloud = "default"
3535
)
3636

37+
// HMACRotationSyncState tracks EDPM deployment progress during HMAC key rotation.
38+
// +kubebuilder:validation:Enum="";WaitingForDeployment
39+
type HMACRotationSyncState string
40+
41+
const (
42+
// HMACRotationIdle - no rotation in progress or waiting for EDPM to detect the key change.
43+
HMACRotationIdle HMACRotationSyncState = ""
44+
// HMACRotationWaitingDeployment - EDPM detected the key change, waiting for deployment to complete.
45+
HMACRotationWaitingDeployment HMACRotationSyncState = "WaitingForDeployment"
46+
)
47+
3748
// AuthSpec defines authentication parameters for InstanceHA.
3849
// Compatible with the AuthSpec interface used by other openstack-k8s-operators.
3950
type AuthSpec struct {
@@ -165,6 +176,10 @@ type InstanceHaStatus struct {
165176
// HeartbeatHMACSecret is the name of the auto-generated Secret
166177
// containing the HMAC key for heartbeat authentication
167178
HeartbeatHMACSecret string `json:"heartbeatHMACSecret,omitempty"`
179+
180+
// HMACKeyRotationSynced tracks EDPM deployment progress during HMAC
181+
// key rotation. See HMACRotationSyncState constants.
182+
HMACKeyRotationSynced HMACRotationSyncState `json:"hmacKeyRotationSynced,omitempty"`
168183
}
169184

170185
//+kubebuilder:object:root=true

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,14 @@ spec:
233233
HeartbeatHMACSecret is the name of the auto-generated Secret
234234
containing the HMAC key for heartbeat authentication
235235
type: string
236+
hmacKeyRotationSynced:
237+
description: |-
238+
HMACKeyRotationSynced tracks EDPM deployment progress during HMAC
239+
key rotation. See HMACRotationSyncState constants.
240+
enum:
241+
- ""
242+
- WaitingForDeployment
243+
type: string
236244
lastAppliedTopology:
237245
description: LastAppliedTopology - the last applied Topology
238246
properties:

docs/instanceha_guide.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -645,13 +645,13 @@ The packet format includes a timestamp for replay protection (packets older than
645645
kubectl annotate instanceha instanceha instanceha.openstack.org/rotate-hmac-key=true
646646
```
647647

648-
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:
648+
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.
649649

650-
```bash
651-
kubectl patch secret instanceha-heartbeat-hmac -p '{"data":{"hmac-key-previous":""}}'
652-
```
650+
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`.
651+
652+
If no `OpenStackDataPlaneNodeSets` exist in the namespace, the previous key is cleared immediately.
653653

654-
Replace `instanceha-heartbeat-hmac` with `<instance-name>-heartbeat-hmac` if your CR has a different name.
654+
> **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`.
655655

656656
The secret name is published in `status.heartbeatHMACSecret` for integration with the EDPM dataplane operator.
657657

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ require (
1111
github.com/onsi/gomega v1.41.0
1212
github.com/openshift/api v3.9.0+incompatible
1313
github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-00010101000000-000000000000
14-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db
15-
github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260526114926-7ebfadd589db
16-
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260526114926-7ebfadd589db
14+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e
15+
github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260608153450-ce00f678385e
16+
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260608153450-ce00f678385e
1717
go.uber.org/zap v1.28.0
1818
golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67
1919
k8s.io/api v0.31.14

go.sum

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,12 @@ github.com/onsi/gomega v1.41.0 h1:OwKp4pXNgVxf6sCplzYo794OFNuoL2q2SBMU5NSWOjA=
118118
github.com/onsi/gomega v1.41.0/go.mod h1:M/Uqpu/8qTjtzCLUA2zJHX9Iilrau25x1PdoSRbWh5A=
119119
github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e h1:E1OdwSpqWuDPCedyUt0GEdoAE+r5TXy7YS21yNEo+2U=
120120
github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
121-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db h1:wSTVKqPOgAboAX23GHHW6J1vJxaHCq/5InQDXFbE8EI=
122-
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260526114926-7ebfadd589db/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
123-
github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260526114926-7ebfadd589db h1:5sOkESe/MppdwP0mBbKnzHMnugg3gfGY+pWx0u8npa8=
124-
github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260526114926-7ebfadd589db/go.mod h1:xsKeDFU3/xEObaVDqd6XEYV3MzvFswbWMlnr2Z3q3ZI=
125-
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260526114926-7ebfadd589db h1:tEASfqRI3+4va7+jr/qoUnPn+7R6RCkHitwx6k8960U=
126-
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260526114926-7ebfadd589db/go.mod h1:nLS2oK4pBo756JNN1cPgr44S0X9V11QScgVla89Ojok=
121+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e h1:Tj/jlh16cQ2EPTYbH1qQpRYoQPqpLtWvI6JVQiRA+jA=
122+
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260608153450-ce00f678385e/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM=
123+
github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260608153450-ce00f678385e h1:jgGl2fxsn2Cl8NQ5+3obEg+H/WOvIQ//q/jJwHmPuRo=
124+
github.com/openstack-k8s-operators/lib-common/modules/edpm v0.0.0-20260608153450-ce00f678385e/go.mod h1:xsKeDFU3/xEObaVDqd6XEYV3MzvFswbWMlnr2Z3q3ZI=
125+
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260608153450-ce00f678385e h1:k3mCwZ2yO2TK4u/2mdPX2SAXTsMr5m8AXG9gUO1zhzk=
126+
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260608153450-ce00f678385e/go.mod h1:nLS2oK4pBo756JNN1cPgr44S0X9V11QScgVla89Ojok=
127127
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
128128
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
129129
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

internal/controller/instanceha/instanceha_controller.go

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"k8s.io/apimachinery/pkg/types"
3737

3838
"k8s.io/apimachinery/pkg/runtime"
39+
"k8s.io/apimachinery/pkg/runtime/schema"
3940
"k8s.io/client-go/kubernetes"
4041
ctrl "sigs.k8s.io/controller-runtime"
4142
"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -62,6 +63,8 @@ import (
6263
commonservice "github.com/openstack-k8s-operators/lib-common/modules/common/service"
6364
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
6465

66+
edpm "github.com/openstack-k8s-operators/lib-common/modules/edpm/unstructured"
67+
6568
networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
6669
instancehav1 "github.com/openstack-k8s-operators/infra-operator/apis/instanceha/v1beta1"
6770
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
@@ -96,6 +99,7 @@ func (r *Reconciler) GetLogger(ctx context.Context) logr.Logger {
9699
// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch
97100
// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
98101
// +kubebuilder:rbac:groups=topology.openstack.org,resources=topologies,verbs=get;list;watch;update
102+
// +kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplanenodesets,verbs=get;list;watch
99103

100104
// Reconcile -
101105
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
564568
return ctrl.Result{RequeueAfter: time.Second}, nil
565569
}
566570

571+
if ctrlResult, err := r.tryNullifyPreviousKey(ctx, instance, hmacSecret); err != nil || ctrlResult.RequeueAfter > 0 {
572+
return ctrlResult, err
573+
}
574+
567575
_, hmacSecretHash, err := secret.GetSecret(ctx, helper, heartbeatHMACSecretName, instance.Namespace)
568576
if err != nil {
569577
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 {
848856
return err
849857
}
850858

851-
return ctrl.NewControllerManagedBy(mgr).
859+
b := ctrl.NewControllerManagedBy(mgr).
852860
For(&instancehav1.InstanceHa{}).
853861
Owns(&appsv1.Deployment{}).
854862
Owns(&corev1.Service{}).
@@ -869,8 +877,18 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
869877
).
870878
Watches(&topologyv1.Topology{},
871879
handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
872-
builder.WithPredicates(predicate.GenerationChangedPredicate{})).
873-
Complete(r)
880+
builder.WithPredicates(predicate.GenerationChangedPredicate{}))
881+
882+
gk := schema.GroupKind{Group: edpm.NodeSetGVK.Group, Kind: edpm.NodeSetGVK.Kind}
883+
if _, err := mgr.GetRESTMapper().RESTMapping(gk, edpm.NodeSetGVK.Version); err == nil {
884+
b = b.Watches(
885+
edpm.NewNodeSetObject(),
886+
handler.EnqueueRequestsFromMapFunc(r.findInstanceHaWithPendingRotation),
887+
builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
888+
)
889+
}
890+
891+
return b.Complete(r)
874892
}
875893

876894
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
921939
return ctrlResult, err
922940
}
923941

942+
// Clear any in-flight rotation state.
943+
instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationIdle
944+
924945
// Service is deleted so remove the finalizer.
925946
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
926947
Log.Info("Reconciled Service delete successfully")
927948

928949
return ctrl.Result{}, nil
929950
}
930951

952+
func (r *Reconciler) tryNullifyPreviousKey(ctx context.Context, instance *instancehav1.InstanceHa, hmacSecret *corev1.Secret) (ctrl.Result, error) {
953+
Log := r.GetLogger(ctx)
954+
955+
if len(hmacSecret.Data["hmac-key-previous"]) == 0 {
956+
if instance.Status.HMACKeyRotationSynced != instancehav1.HMACRotationIdle {
957+
instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationIdle
958+
}
959+
return ctrl.Result{}, nil
960+
}
961+
962+
secretName := hmacSecret.Name
963+
inSync, info, err := edpm.IsSecretHashInSync(ctx, r.Client, instance.Namespace, secretName)
964+
if err != nil {
965+
Log.Error(err, "Failed to check nodeset sync status for HMAC key rotation, will retry")
966+
return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
967+
}
968+
969+
if !inSync {
970+
if instance.Status.HMACKeyRotationSynced != instancehav1.HMACRotationWaitingDeployment {
971+
instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationWaitingDeployment
972+
Log.Info("HMAC key change detected by EDPM, waiting for deployment",
973+
"secret", secretName, "info", info)
974+
} else {
975+
Log.Info("HMAC key deployment in progress", "secret", secretName, "info", info)
976+
}
977+
return ctrl.Result{}, nil
978+
}
979+
980+
hmacSecret.Data["hmac-key-previous"] = []byte{}
981+
if err := r.Update(ctx, hmacSecret); err != nil {
982+
return ctrl.Result{}, fmt.Errorf("failed to nullify previous HMAC key: %w", err)
983+
}
984+
instance.Status.HMACKeyRotationSynced = instancehav1.HMACRotationIdle
985+
Log.Info("Nullified previous HMAC key after successful EDPM deployment", "secret", secretName)
986+
return ctrl.Result{}, nil
987+
}
988+
989+
func (r *Reconciler) findInstanceHaWithPendingRotation(ctx context.Context, _ client.Object) []reconcile.Request {
990+
crList := &instancehav1.InstanceHaList{}
991+
if err := r.List(ctx, crList); err != nil {
992+
return nil
993+
}
994+
var requests []reconcile.Request
995+
for _, cr := range crList.Items {
996+
if cr.Status.HMACKeyRotationSynced != instancehav1.HMACRotationIdle {
997+
requests = append(requests, reconcile.Request{
998+
NamespacedName: types.NamespacedName{
999+
Name: cr.Name,
1000+
Namespace: cr.Namespace,
1001+
},
1002+
})
1003+
}
1004+
}
1005+
return requests
1006+
}
1007+
9311008
// GetContainerImage returns the container image to use for the instance, either from
9321009
// the provided containerImage parameter or from the infra-instanceha-config ConfigMap
9331010
func (r *Reconciler) GetContainerImage(

test/functional/instanceha_controller_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,12 +511,13 @@ var _ = Describe("InstanceHa Controller", func() {
511511
g.Expect(k8sClient.Update(ctx, instance)).Should(Succeed())
512512
}, timeout, interval).Should(Succeed())
513513

514-
// Verify the key was rotated: new current key, previous = old current
514+
// Verify the key was rotated and previous key auto-cleared
515+
// (no NodeSets in envtest, so previous key is nullified immediately)
515516
Eventually(func(g Gomega) {
516517
hmacSecret := &corev1.Secret{}
517518
g.Expect(k8sClient.Get(ctx, hmacSecretName, hmacSecret)).Should(Succeed())
518519
g.Expect(hmacSecret.Data["hmac-key"]).ToNot(Equal(originalKey), "current key should have changed")
519-
g.Expect(hmacSecret.Data["hmac-key-previous"]).To(Equal(originalKey), "previous key should be the old current key")
520+
g.Expect(hmacSecret.Data["hmac-key-previous"]).To(BeEmpty(), "previous key should be auto-cleared when no NodeSets exist")
520521
}, timeout, interval).Should(Succeed())
521522

522523
// Verify the annotation was removed

0 commit comments

Comments
 (0)