Skip to content

Commit 3ab751e

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #618 from Deydra71/appcred-finalizer
Add application credential finalizer management
2 parents 5e81a0e + 703c038 commit 3ab751e

6 files changed

Lines changed: 246 additions & 0 deletions

File tree

‎api/bases/neutron.openstack.org_neutronapis.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,13 @@ spec:
16071607
status:
16081608
description: NeutronAPIStatus defines the observed state of NeutronAPI
16091609
properties:
1610+
applicationCredentialSecret:
1611+
description: |-
1612+
ApplicationCredentialSecret - the AC secret NeutronAPI is currently
1613+
consuming and protecting with the openstack.org/neutronapi-ac-consumer
1614+
finalizer. Tracked so the controller can remove its finalizer from the
1615+
old secret when the openstack-operator rotates the reference.
1616+
type: string
16101617
conditions:
16111618
description: Conditions
16121619
items:

‎api/v1beta1/neutronapi_types.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ type NeutronAPISpecCore struct {
177177
// An empty value "" leaves the notification drivers unconfigured and emitting no notifications at all.
178178
// Avoid colocating it with RabbitMqClusterName used for RPC.
179179
NotificationsBusInstance *string `json:"notificationsBusInstance,omitempty" deprecated:"notificationsBus.cluster"`
180+
180181
}
181182

182183
type NeutronApiTLS struct {
@@ -237,6 +238,12 @@ type NeutronAPIStatus struct {
237238
// NetworkAttachments status of the deployment pods
238239
NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`
239240

241+
// ApplicationCredentialSecret - the AC secret NeutronAPI is currently
242+
// consuming and protecting with the openstack.org/neutronapi-ac-consumer
243+
// finalizer. Tracked so the controller can remove its finalizer from the
244+
// old secret when the openstack-operator rotates the reference.
245+
ApplicationCredentialSecret string `json:"applicationCredentialSecret,omitempty"`
246+
240247
// ObservedGeneration - the most recent generation observed for this
241248
// service. If the observed generation is less than the spec generation,
242249
// then the controller has not processed the latest changes injected by

‎config/crd/bases/neutron.openstack.org_neutronapis.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,6 +1607,13 @@ spec:
16071607
status:
16081608
description: NeutronAPIStatus defines the observed state of NeutronAPI
16091609
properties:
1610+
applicationCredentialSecret:
1611+
description: |-
1612+
ApplicationCredentialSecret - the AC secret NeutronAPI is currently
1613+
consuming and protecting with the openstack.org/neutronapi-ac-consumer
1614+
finalizer. Tracked so the controller can remove its finalizer from the
1615+
old secret when the openstack-operator rotates the reference.
1616+
type: string
16101617
conditions:
16111618
description: Conditions
16121619
items:

‎internal/controller/neutronapi_controller.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,19 @@ func (r *NeutronAPIReconciler) reconcileDelete(ctx context.Context, instance *ne
492492
); err != nil {
493493
return ctrlResult, err
494494
}
495+
// Remove consumer finalizer from AC secrets NeutronAPI was consuming.
496+
// Check both status and spec to handle the edge case where the reconciler
497+
// crashed after adding the finalizer but before updating the status.
498+
for _, secretName := range []string{
499+
instance.Status.ApplicationCredentialSecret,
500+
instance.Spec.Auth.ApplicationCredentialSecret,
501+
} {
502+
if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace,
503+
secretName, neutronapi.ACConsumerFinalizer); err != nil {
504+
return ctrl.Result{}, err
505+
}
506+
}
507+
495508
// Service is deleted so remove the finalizer.
496509
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
497510
Log.Info("Reconciled Service delete successfully")
@@ -648,6 +661,24 @@ func (r *NeutronAPIReconciler) reconcileInit(
648661

649662
// Create Secrets - end
650663

664+
// Add consumer finalizer to the new AC secret early, before deployment.
665+
// The old secret's finalizer is removed later (after all services deploy)
666+
// so that rapid rotations don't revoke a credential still in use by pods.
667+
if instance.Spec.Auth.ApplicationCredentialSecret != "" {
668+
if err := keystonev1.ManageACSecretFinalizer(ctx, helper, instance.Namespace,
669+
instance.Spec.Auth.ApplicationCredentialSecret,
670+
"",
671+
neutronapi.ACConsumerFinalizer); err != nil {
672+
instance.Status.Conditions.Set(condition.FalseCondition(
673+
condition.ServiceConfigReadyCondition,
674+
condition.ErrorReason,
675+
condition.SeverityWarning,
676+
condition.ServiceConfigReadyErrorMessage,
677+
err.Error()))
678+
return ctrl.Result{}, err
679+
}
680+
}
681+
651682
instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)
652683

653684
//
@@ -1376,6 +1407,25 @@ func (r *NeutronAPIReconciler) reconcileNormal(ctx context.Context, instance *ne
13761407
return ctrl.Result{}, err
13771408
}
13781409
}
1410+
// Manage the old AC secret's finalizer and status tracking.
1411+
// On rotation (old != new), only remove the old secret's finalizer after
1412+
// all sub-services are ready with the new credentials. This prevents
1413+
// premature revocation during rapid rotations.
1414+
isRotation := instance.Status.ApplicationCredentialSecret != "" && instance.Status.ApplicationCredentialSecret != instance.Spec.Auth.ApplicationCredentialSecret
1415+
1416+
if isRotation {
1417+
allServicesReady := instance.Status.Conditions.AllSubConditionIsTrue()
1418+
if allServicesReady {
1419+
if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace,
1420+
instance.Status.ApplicationCredentialSecret, neutronapi.ACConsumerFinalizer); err != nil {
1421+
return ctrl.Result{}, err
1422+
}
1423+
instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret
1424+
}
1425+
} else {
1426+
instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret
1427+
}
1428+
13791429
// We reached the end of the Reconcile, update the Ready condition based on
13801430
// the sub conditions
13811431
if instance.Status.Conditions.AllSubConditionIsTrue() {

‎internal/neutronapi/const.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ const (
5050

5151
// NeutronDhcpAgentSecretKey is the key in external Secret for Neutron DHCP Agent with agent config
5252
NeutronDhcpAgentSecretKey = "10-neutron-dhcp.conf"
53+
54+
// ACConsumerFinalizer is added to AC secrets that neutron is actively consuming
55+
ACConsumerFinalizer = "openstack.org/neutronapi-ac-consumer"
5356
)
5457

5558
// DbsyncPropagation keeps track of the DBSync Service Propagation Type

‎test/functional/neutronapi_controller_test.go‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2202,6 +2202,178 @@ func getNeutronAPIControllerSuite(ml2MechanismDrivers []string) func() {
22022202
}, timeout, interval).Should(Succeed())
22032203
})
22042204
})
2205+
2206+
When("ApplicationCredential consumer finalizer is managed", func() {
2207+
var acSecretName string
2208+
2209+
BeforeEach(func() {
2210+
acSecretName = "ac-neutron-a1b2c-secret" //nolint:gosec // G101
2211+
secret := &corev1.Secret{
2212+
ObjectMeta: metav1.ObjectMeta{
2213+
Namespace: namespace,
2214+
Name: acSecretName,
2215+
},
2216+
Data: map[string][]byte{
2217+
keystonev1.ACIDSecretKey: []byte("a1b2ctest-ac-id"),
2218+
keystonev1.ACSecretSecretKey: []byte("test-ac-secret"),
2219+
},
2220+
}
2221+
DeferCleanup(k8sClient.Delete, ctx, secret)
2222+
Expect(k8sClient.Create(ctx, secret)).To(Succeed())
2223+
2224+
spec["auth"] = map[string]any{
2225+
"applicationCredentialSecret": acSecretName,
2226+
}
2227+
2228+
DeferCleanup(th.DeleteInstance, CreateNeutronAPI(neutronAPIName.Namespace, neutronAPIName.Name, spec))
2229+
DeferCleanup(k8sClient.Delete, ctx, CreateNeutronAPISecret(namespace, SecretName))
2230+
DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, "memcached", memcachedSpec))
2231+
infra.SimulateMemcachedReady(memcachedName)
2232+
DeferCleanup(
2233+
mariadb.DeleteDBService,
2234+
mariadb.CreateDBService(
2235+
namespace,
2236+
GetNeutronAPI(neutronAPIName).Spec.DatabaseInstance,
2237+
corev1.ServiceSpec{
2238+
Ports: []corev1.ServicePort{{Port: 3306}},
2239+
},
2240+
),
2241+
)
2242+
SimulateTransportURLReady(apiTransportURLName)
2243+
mariadb.SimulateMariaDBAccountCompleted(types.NamespacedName{Namespace: namespace, Name: GetNeutronAPI(neutronAPIName).Spec.DatabaseAccount})
2244+
mariadb.SimulateMariaDBDatabaseCompleted(types.NamespacedName{Namespace: namespace, Name: neutronapi.DatabaseCRName})
2245+
2246+
if isOVNEnabled {
2247+
DeferCleanup(DeleteOVNDBClusters, CreateOVNDBClusters(namespace))
2248+
}
2249+
DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(namespace))
2250+
})
2251+
2252+
It("should add the consumer finalizer to the AC secret", func() {
2253+
Eventually(func(g Gomega) {
2254+
secret := th.GetSecret(types.NamespacedName{
2255+
Namespace: namespace,
2256+
Name: acSecretName,
2257+
})
2258+
g.Expect(secret.Finalizers).To(
2259+
ContainElement(neutronapi.ACConsumerFinalizer))
2260+
}, timeout, interval).Should(Succeed())
2261+
})
2262+
2263+
It("should track the consumed AC secret in status", func() {
2264+
th.SimulateJobSuccess(neutronDBSyncJobName)
2265+
keystone.SimulateKeystoneServiceReady(types.NamespacedName{
2266+
Namespace: namespace,
2267+
Name: "neutron",
2268+
})
2269+
keystone.SimulateKeystoneEndpointReady(types.NamespacedName{
2270+
Namespace: namespace,
2271+
Name: "neutron",
2272+
})
2273+
Eventually(func(g Gomega) {
2274+
n := GetNeutronAPI(neutronAPIName)
2275+
g.Expect(n.Status.ApplicationCredentialSecret).To(Equal(acSecretName))
2276+
}, timeout, interval).Should(Succeed())
2277+
})
2278+
2279+
It("should move the finalizer from the old to the new secret on rotation", func() {
2280+
Eventually(func(g Gomega) {
2281+
secret := th.GetSecret(types.NamespacedName{
2282+
Namespace: namespace,
2283+
Name: acSecretName,
2284+
})
2285+
g.Expect(secret.Finalizers).To(
2286+
ContainElement(neutronapi.ACConsumerFinalizer))
2287+
}, timeout, interval).Should(Succeed())
2288+
2289+
th.SimulateJobSuccess(neutronDBSyncJobName)
2290+
th.SimulateDeploymentReplicaReady(types.NamespacedName{
2291+
Namespace: namespace,
2292+
Name: "neutron",
2293+
})
2294+
keystone.SimulateKeystoneServiceReady(types.NamespacedName{
2295+
Namespace: namespace,
2296+
Name: "neutron",
2297+
})
2298+
keystone.SimulateKeystoneEndpointReady(types.NamespacedName{
2299+
Namespace: namespace,
2300+
Name: "neutron",
2301+
})
2302+
Eventually(func(g Gomega) {
2303+
n := GetNeutronAPI(neutronAPIName)
2304+
g.Expect(n.Status.Conditions.IsTrue(condition.ReadyCondition)).To(BeTrue())
2305+
}, timeout, interval).Should(Succeed())
2306+
2307+
newACSecretName := "ac-neutron-x9y8z-secret" //nolint:gosec // G101
2308+
newSecret := &corev1.Secret{
2309+
ObjectMeta: metav1.ObjectMeta{
2310+
Namespace: namespace,
2311+
Name: newACSecretName,
2312+
},
2313+
Data: map[string][]byte{
2314+
keystonev1.ACIDSecretKey: []byte("x9y8zrotated-ac-id"),
2315+
keystonev1.ACSecretSecretKey: []byte("rotated-ac-secret"),
2316+
},
2317+
}
2318+
DeferCleanup(k8sClient.Delete, ctx, newSecret)
2319+
Expect(k8sClient.Create(ctx, newSecret)).To(Succeed())
2320+
2321+
Eventually(func(g Gomega) {
2322+
n := GetNeutronAPI(neutronAPIName)
2323+
n.Spec.Auth.ApplicationCredentialSecret = newACSecretName
2324+
g.Expect(k8sClient.Update(ctx, n)).Should(Succeed())
2325+
}, timeout, interval).Should(Succeed())
2326+
2327+
Eventually(func(g Gomega) {
2328+
secret := th.GetSecret(types.NamespacedName{
2329+
Namespace: namespace,
2330+
Name: newACSecretName,
2331+
})
2332+
g.Expect(secret.Finalizers).To(
2333+
ContainElement(neutronapi.ACConsumerFinalizer))
2334+
}, timeout, interval).Should(Succeed())
2335+
2336+
Eventually(func(g Gomega) {
2337+
th.SimulateDeploymentReplicaReady(types.NamespacedName{
2338+
Namespace: namespace,
2339+
Name: "neutron",
2340+
})
2341+
secret := th.GetSecret(types.NamespacedName{
2342+
Namespace: namespace,
2343+
Name: acSecretName,
2344+
})
2345+
g.Expect(secret.Finalizers).NotTo(
2346+
ContainElement(neutronapi.ACConsumerFinalizer))
2347+
}, timeout, interval).Should(Succeed())
2348+
2349+
Eventually(func(g Gomega) {
2350+
n := GetNeutronAPI(neutronAPIName)
2351+
g.Expect(n.Status.ApplicationCredentialSecret).To(Equal(newACSecretName))
2352+
}, timeout, interval).Should(Succeed())
2353+
})
2354+
2355+
It("should remove the consumer finalizer from AC secret on CR deletion", func() {
2356+
Eventually(func(g Gomega) {
2357+
secret := th.GetSecret(types.NamespacedName{
2358+
Namespace: namespace,
2359+
Name: acSecretName,
2360+
})
2361+
g.Expect(secret.Finalizers).To(
2362+
ContainElement(neutronapi.ACConsumerFinalizer))
2363+
}, timeout, interval).Should(Succeed())
2364+
2365+
th.DeleteInstance(GetNeutronAPI(neutronAPIName))
2366+
2367+
Eventually(func(g Gomega) {
2368+
secret := th.GetSecret(types.NamespacedName{
2369+
Namespace: namespace,
2370+
Name: acSecretName,
2371+
})
2372+
g.Expect(secret.Finalizers).NotTo(
2373+
ContainElement(neutronapi.ACConsumerFinalizer))
2374+
}, timeout, interval).Should(Succeed())
2375+
})
2376+
})
22052377
}
22062378
}
22072379

0 commit comments

Comments
 (0)