Skip to content

Commit ff70f08

Browse files
lmicciniclaude
andcommitted
Skip NodeSet sync for controlplane-only credential rotation
During RabbitMQ credential rotation, the TransportURL controller uses a two-phase NodeSet hash-sync check to ensure EDPM nodes pick up new credentials before releasing the old user. This is only needed for services that run agents on EDPM nodes (Nova, Neutron). Determine EDPM status from the TransportURL's ownerReference Kind: if the owner is Nova or NeutronAPI, wait for NodeSet deployment; otherwise release the old user immediately. The edpm-service annotation is available as an explicit override for edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8bde9f9 commit ff70f08

4 files changed

Lines changed: 307 additions & 9 deletions

File tree

apis/rabbitmq/v1beta1/conditions.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,29 @@ const (
6161
// The TransportURL controller sets this label after verifying NodeSet deployment
6262
// completion, so the user controller can safely auto-delete the CR on sight.
6363
RabbitMQUserOrphanedLabel = "rabbitmq.openstack.org/orphaned"
64+
65+
// EDPMServiceAnnotation overrides the automatic owner-based EDPM detection
66+
// for a TransportURL. When set to "true", the controller always uses the
67+
// two-phase NodeSet sync check. When "false", it releases the old user
68+
// immediately. When unset, the controller infers EDPM status from the
69+
// ownerReference Kind (Nova and NeutronAPI are EDPM; everything else is not).
70+
EDPMServiceAnnotation = "rabbitmq.openstack.org/edpm-service"
6471
)
6572

73+
// edpmOwnerKinds lists the owner CR Kinds whose TransportURLs serve
74+
// EDPM-deployed agents and therefore require NodeSet hash-sync gating
75+
// during credential rotation.
76+
var edpmOwnerKinds = map[string]bool{
77+
"Nova": true,
78+
"NeutronAPI": true,
79+
}
80+
81+
// IsEDPMOwnerKind reports whether the given owner Kind serves
82+
// EDPM-deployed agents that require NodeSet hash-sync gating.
83+
func IsEDPMOwnerKind(kind string) bool {
84+
return edpmOwnerKinds[kind]
85+
}
86+
6687
// TransportURLFinalizerFor returns the per-consumer finalizer for a TransportURL.
6788
// If the name fits within Kubernetes' 63-char name segment limit, it is used directly
6889
// (preserving human readability and reverse mapping). For longer names, the suffix

internal/controller/rabbitmq/rabbitmquser_controller.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,6 @@ const userFinalizer = "rabbitmquser.openstack.org/finalizer"
5555
// credentialSecretNameField is the field index for the credential secret
5656
const credentialSecretNameField = ".spec.secret"
5757

58-
// ConnectionCheckInterval is the polling interval for pending user release checks
59-
// in the TransportURL controller, used as a fallback alongside the NodeSet watch.
60-
const ConnectionCheckInterval = 5 * time.Minute
61-
6258
// generatePassword generates a random password
6359
func generatePassword(length int) (string, error) {
6460
bytes := make([]byte, length)

internal/controller/rabbitmq/transporturl_controller.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ import (
4949
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
5050
)
5151

52+
// PendingReleaseCheckInterval is the requeue interval while waiting for a
53+
// pending user release.
54+
const PendingReleaseCheckInterval = 1 * time.Minute
55+
5256
// GetClient -
5357
func (r *TransportURLReconciler) GetClient() client.Client {
5458
return r.Client
@@ -560,7 +564,7 @@ func (r *TransportURLReconciler) reconcileNormal(ctx context.Context, instance *
560564
if !released {
561565
Log.Info("Pending user release waiting for deployment",
562566
"pendingUser", instance.Status.PreviousRabbitmqUserRef)
563-
return ctrl.Result{RequeueAfter: ConnectionCheckInterval}, nil
567+
return ctrl.Result{RequeueAfter: PendingReleaseCheckInterval}, nil
564568
}
565569
}
566570

@@ -888,6 +892,25 @@ func (r *TransportURLReconciler) tryReleasePendingUser(ctx context.Context, inst
888892
Log := r.GetLogger(ctx)
889893
pendingRef := instance.Status.PreviousRabbitmqUserRef
890894

895+
// Determine whether this TransportURL serves an EDPM-deployed service.
896+
// Explicit annotation wins; otherwise infer from the ownerReference Kind.
897+
edpmService := false
898+
if v, ok := instance.Annotations[rabbitmqv1.EDPMServiceAnnotation]; ok {
899+
edpmService = v == "true"
900+
} else {
901+
for _, ref := range instance.OwnerReferences {
902+
if rabbitmqv1.IsEDPMOwnerKind(ref.Kind) {
903+
edpmService = true
904+
break
905+
}
906+
}
907+
}
908+
if !edpmService {
909+
Log.Info("Non-EDPM service, releasing old user immediately",
910+
"pendingUser", pendingRef)
911+
return r.releasePendingUser(ctx, instance)
912+
}
913+
891914
// No NodeSets with secretHashes -> no computes affected, release immediately
892915
haveNS, err := edpm.HaveNodeSets(ctx, r.Client, instance.Namespace)
893916
if err != nil {

test/functional/transporturl_controller_test.go

Lines changed: 262 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,11 +1208,27 @@ var _ = Describe("TransportURL controller", func() {
12081208
rabbitmq := CreateRabbitMQ(rabbitmqName, spec)
12091209
DeferCleanup(th.DeleteInstance, rabbitmq)
12101210

1211-
tuSpec := map[string]any{
1212-
"rabbitmqClusterName": rabbitmqName.Name,
1213-
"username": "phase-user-a",
1211+
// Create TransportURL with a Nova ownerReference (EDPM service)
1212+
tu := &rabbitmqv1.TransportURL{
1213+
ObjectMeta: metav1.ObjectMeta{
1214+
Name: turlName.Name,
1215+
Namespace: turlName.Namespace,
1216+
OwnerReferences: []metav1.OwnerReference{
1217+
{
1218+
APIVersion: "nova.openstack.org/v1beta1",
1219+
Kind: "Nova",
1220+
Name: "nova",
1221+
UID: "fake-nova-uid",
1222+
},
1223+
},
1224+
},
1225+
Spec: rabbitmqv1.TransportURLSpec{
1226+
RabbitmqClusterName: rabbitmqName.Name,
1227+
Username: "phase-user-a",
1228+
},
12141229
}
1215-
DeferCleanup(th.DeleteInstance, CreateTransportURL(turlName, tuSpec))
1230+
Expect(k8sClient.Create(ctx, tu)).Should(Succeed())
1231+
DeferCleanup(th.DeleteInstance, tu)
12161232

12171233
CreateNodeSet(nodesetName)
12181234
DeferCleanup(DeleteNodeSet, nodesetName)
@@ -1333,6 +1349,248 @@ var _ = Describe("TransportURL controller", func() {
13331349
})
13341350
})
13351351

1352+
When("credential rotation for controlplane-only owner (non-EDPM)", func() {
1353+
var rabbitmqName types.NamespacedName
1354+
var turlName types.NamespacedName
1355+
var nodesetName types.NamespacedName
1356+
1357+
BeforeEach(func() {
1358+
rabbitmqName = types.NamespacedName{
1359+
Name: "rabbitmq-cp-only",
1360+
Namespace: namespace,
1361+
}
1362+
turlName = types.NamespacedName{
1363+
Name: "turl-cp-only",
1364+
Namespace: namespace,
1365+
}
1366+
nodesetName = types.NamespacedName{
1367+
Name: "compute-cp-only",
1368+
Namespace: namespace,
1369+
}
1370+
1371+
SetupMockRabbitMQAPI()
1372+
DeferCleanup(StopMockRabbitMQAPI)
1373+
1374+
CreateRabbitMQCluster(rabbitmqName, GetDefaultRabbitMQClusterSpec(false))
1375+
DeferCleanup(DeleteRabbitMQCluster, rabbitmqName)
1376+
1377+
spec := GetDefaultRabbitMQSpec()
1378+
rabbitmq := CreateRabbitMQ(rabbitmqName, spec)
1379+
DeferCleanup(th.DeleteInstance, rabbitmq)
1380+
1381+
// Create TransportURL with a non-EDPM ownerReference (simulating Cinder)
1382+
tu := &rabbitmqv1.TransportURL{
1383+
ObjectMeta: metav1.ObjectMeta{
1384+
Name: turlName.Name,
1385+
Namespace: turlName.Namespace,
1386+
OwnerReferences: []metav1.OwnerReference{
1387+
{
1388+
APIVersion: "cinder.openstack.org/v1beta1",
1389+
Kind: "Cinder",
1390+
Name: "cinder",
1391+
UID: "fake-cinder-uid",
1392+
},
1393+
},
1394+
},
1395+
Spec: rabbitmqv1.TransportURLSpec{
1396+
RabbitmqClusterName: rabbitmqName.Name,
1397+
Username: "cp-only-user-a",
1398+
},
1399+
}
1400+
Expect(k8sClient.Create(ctx, tu)).Should(Succeed())
1401+
DeferCleanup(th.DeleteInstance, tu)
1402+
1403+
CreateNodeSet(nodesetName)
1404+
DeferCleanup(DeleteNodeSet, nodesetName)
1405+
})
1406+
1407+
It("should release old user immediately without waiting for NodeSet sync", func() {
1408+
SimulateRabbitMQClusterReady(rabbitmqName)
1409+
1410+
userACRName := types.NamespacedName{
1411+
Name: rabbitmqv1.CanonicalUserName(rabbitmqName.Name, "/", "cp-only-user-a"),
1412+
Namespace: namespace,
1413+
}
1414+
Eventually(func(g Gomega) {
1415+
user := &rabbitmqv1.RabbitMQUser{}
1416+
g.Expect(k8sClient.Get(ctx, userACRName, user)).Should(Succeed())
1417+
}, timeout, interval).Should(Succeed())
1418+
1419+
SimulateRabbitMQUserReady(userACRName, "/")
1420+
1421+
transportURLSecretName := types.NamespacedName{
1422+
Name: "rabbitmq-transport-url-" + turlName.Name,
1423+
Namespace: namespace,
1424+
}
1425+
Eventually(func(g Gomega) {
1426+
tr := th.GetTransportURL(turlName)
1427+
g.Expect(tr.Status.RabbitmqUsername).To(Equal("cp-only-user-a"))
1428+
s := &corev1.Secret{}
1429+
g.Expect(k8sClient.Get(ctx, transportURLSecretName, s)).Should(Succeed())
1430+
}, timeout, interval).Should(Succeed())
1431+
1432+
initialHash := GetSecretHash(transportURLSecretName)
1433+
SetNodeSetSecretHashes(nodesetName, map[string]string{
1434+
transportURLSecretName.Name: initialHash,
1435+
})
1436+
1437+
// --- Trigger credential rotation ---
1438+
Eventually(func(g Gomega) {
1439+
tr := th.GetTransportURL(turlName)
1440+
tr.Spec.Username = "cp-only-user-b"
1441+
g.Expect(k8sClient.Update(ctx, tr)).Should(Succeed())
1442+
}, timeout, interval).Should(Succeed())
1443+
1444+
userBCRName := types.NamespacedName{
1445+
Name: rabbitmqv1.CanonicalUserName(rabbitmqName.Name, "/", "cp-only-user-b"),
1446+
Namespace: namespace,
1447+
}
1448+
Eventually(func(g Gomega) {
1449+
user := &rabbitmqv1.RabbitMQUser{}
1450+
g.Expect(k8sClient.Get(ctx, userBCRName, user)).Should(Succeed())
1451+
}, timeout, interval).Should(Succeed())
1452+
1453+
SimulateRabbitMQUserReady(userBCRName, "/")
1454+
1455+
// Old user should be released immediately — owner is Cinder
1456+
// (not in EDPMOwnerKinds), so no NodeSet sync needed
1457+
Eventually(func(g Gomega) {
1458+
user := &rabbitmqv1.RabbitMQUser{}
1459+
err := k8sClient.Get(ctx, userACRName, user)
1460+
g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue(),
1461+
"Old user should be deleted immediately for non-EDPM owner")
1462+
}, timeout, interval).Should(Succeed())
1463+
1464+
// Verify cleanup
1465+
Eventually(func(g Gomega) {
1466+
tr := th.GetTransportURL(turlName)
1467+
g.Expect(tr.Status.PreviousRabbitmqUserRef).To(BeEmpty())
1468+
}, timeout, interval).Should(Succeed())
1469+
})
1470+
})
1471+
1472+
When("annotation overrides owner-based EDPM detection", func() {
1473+
var rabbitmqName types.NamespacedName
1474+
var turlName types.NamespacedName
1475+
var nodesetName types.NamespacedName
1476+
1477+
BeforeEach(func() {
1478+
rabbitmqName = types.NamespacedName{
1479+
Name: "rabbitmq-anno-override",
1480+
Namespace: namespace,
1481+
}
1482+
turlName = types.NamespacedName{
1483+
Name: "turl-anno-override",
1484+
Namespace: namespace,
1485+
}
1486+
nodesetName = types.NamespacedName{
1487+
Name: "compute-anno-override",
1488+
Namespace: namespace,
1489+
}
1490+
1491+
SetupMockRabbitMQAPI()
1492+
DeferCleanup(StopMockRabbitMQAPI)
1493+
1494+
CreateRabbitMQCluster(rabbitmqName, GetDefaultRabbitMQClusterSpec(false))
1495+
DeferCleanup(DeleteRabbitMQCluster, rabbitmqName)
1496+
1497+
spec := GetDefaultRabbitMQSpec()
1498+
rabbitmq := CreateRabbitMQ(rabbitmqName, spec)
1499+
DeferCleanup(th.DeleteInstance, rabbitmq)
1500+
1501+
// Nova-owned TransportURL with edpm-service=false annotation override
1502+
tu := &rabbitmqv1.TransportURL{
1503+
ObjectMeta: metav1.ObjectMeta{
1504+
Name: turlName.Name,
1505+
Namespace: turlName.Namespace,
1506+
Annotations: map[string]string{
1507+
rabbitmqv1.EDPMServiceAnnotation: "false",
1508+
},
1509+
OwnerReferences: []metav1.OwnerReference{
1510+
{
1511+
APIVersion: "nova.openstack.org/v1beta1",
1512+
Kind: "Nova",
1513+
Name: "nova",
1514+
UID: "fake-nova-uid-2",
1515+
},
1516+
},
1517+
},
1518+
Spec: rabbitmqv1.TransportURLSpec{
1519+
RabbitmqClusterName: rabbitmqName.Name,
1520+
Username: "anno-user-a",
1521+
},
1522+
}
1523+
Expect(k8sClient.Create(ctx, tu)).Should(Succeed())
1524+
DeferCleanup(th.DeleteInstance, tu)
1525+
1526+
CreateNodeSet(nodesetName)
1527+
DeferCleanup(DeleteNodeSet, nodesetName)
1528+
})
1529+
1530+
It("should release immediately when annotation says false despite Nova owner", func() {
1531+
SimulateRabbitMQClusterReady(rabbitmqName)
1532+
1533+
userACRName := types.NamespacedName{
1534+
Name: rabbitmqv1.CanonicalUserName(rabbitmqName.Name, "/", "anno-user-a"),
1535+
Namespace: namespace,
1536+
}
1537+
Eventually(func(g Gomega) {
1538+
user := &rabbitmqv1.RabbitMQUser{}
1539+
g.Expect(k8sClient.Get(ctx, userACRName, user)).Should(Succeed())
1540+
}, timeout, interval).Should(Succeed())
1541+
1542+
SimulateRabbitMQUserReady(userACRName, "/")
1543+
1544+
transportURLSecretName := types.NamespacedName{
1545+
Name: "rabbitmq-transport-url-" + turlName.Name,
1546+
Namespace: namespace,
1547+
}
1548+
Eventually(func(g Gomega) {
1549+
tr := th.GetTransportURL(turlName)
1550+
g.Expect(tr.Status.RabbitmqUsername).To(Equal("anno-user-a"))
1551+
s := &corev1.Secret{}
1552+
g.Expect(k8sClient.Get(ctx, transportURLSecretName, s)).Should(Succeed())
1553+
}, timeout, interval).Should(Succeed())
1554+
1555+
initialHash := GetSecretHash(transportURLSecretName)
1556+
SetNodeSetSecretHashes(nodesetName, map[string]string{
1557+
transportURLSecretName.Name: initialHash,
1558+
})
1559+
1560+
// Trigger credential rotation
1561+
Eventually(func(g Gomega) {
1562+
tr := th.GetTransportURL(turlName)
1563+
tr.Spec.Username = "anno-user-b"
1564+
g.Expect(k8sClient.Update(ctx, tr)).Should(Succeed())
1565+
}, timeout, interval).Should(Succeed())
1566+
1567+
userBCRName := types.NamespacedName{
1568+
Name: rabbitmqv1.CanonicalUserName(rabbitmqName.Name, "/", "anno-user-b"),
1569+
Namespace: namespace,
1570+
}
1571+
Eventually(func(g Gomega) {
1572+
user := &rabbitmqv1.RabbitMQUser{}
1573+
g.Expect(k8sClient.Get(ctx, userBCRName, user)).Should(Succeed())
1574+
}, timeout, interval).Should(Succeed())
1575+
1576+
SimulateRabbitMQUserReady(userBCRName, "/")
1577+
1578+
// Annotation override: despite Nova owner, edpm-service=false
1579+
// should cause immediate release
1580+
Eventually(func(g Gomega) {
1581+
user := &rabbitmqv1.RabbitMQUser{}
1582+
err := k8sClient.Get(ctx, userACRName, user)
1583+
g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue(),
1584+
"Annotation override should bypass owner-based EDPM detection")
1585+
}, timeout, interval).Should(Succeed())
1586+
1587+
Eventually(func(g Gomega) {
1588+
tr := th.GetTransportURL(turlName)
1589+
g.Expect(tr.Status.PreviousRabbitmqUserRef).To(BeEmpty())
1590+
}, timeout, interval).Should(Succeed())
1591+
})
1592+
})
1593+
13361594
When("username is changed for standalone TransportURL without owner", func() {
13371595
var rabbitmqName types.NamespacedName
13381596
var transportURLName types.NamespacedName

0 commit comments

Comments
 (0)