From 7b63bb81dca84f4da4cc74cd1828ce34097e2cce Mon Sep 17 00:00:00 2001 From: Veronika Fisarova Date: Wed, 25 Feb 2026 11:57:13 +0100 Subject: [PATCH] Add AC finalizer management Signed-off-by: Veronika Fisarova --- .../neutron.openstack.org_neutronapis.yaml | 7 + api/v1beta1/neutronapi_types.go | 7 + .../neutron.openstack.org_neutronapis.yaml | 7 + go.mod | 10 +- go.sum | 20 +- internal/controller/neutronapi_controller.go | 50 +++++ internal/neutronapi/const.go | 3 + test/functional/neutronapi_controller_test.go | 172 ++++++++++++++++++ 8 files changed, 261 insertions(+), 15 deletions(-) diff --git a/api/bases/neutron.openstack.org_neutronapis.yaml b/api/bases/neutron.openstack.org_neutronapis.yaml index 7a7f64a44..f2912df8b 100644 --- a/api/bases/neutron.openstack.org_neutronapis.yaml +++ b/api/bases/neutron.openstack.org_neutronapis.yaml @@ -1607,6 +1607,13 @@ spec: status: description: NeutronAPIStatus defines the observed state of NeutronAPI properties: + applicationCredentialSecret: + description: |- + ApplicationCredentialSecret - the AC secret NeutronAPI is currently + consuming and protecting with the openstack.org/neutronapi-ac-consumer + finalizer. Tracked so the controller can remove its finalizer from the + old secret when the openstack-operator rotates the reference. + type: string conditions: description: Conditions items: diff --git a/api/v1beta1/neutronapi_types.go b/api/v1beta1/neutronapi_types.go index eebb64933..03d632b51 100644 --- a/api/v1beta1/neutronapi_types.go +++ b/api/v1beta1/neutronapi_types.go @@ -177,6 +177,7 @@ type NeutronAPISpecCore struct { // An empty value "" leaves the notification drivers unconfigured and emitting no notifications at all. // Avoid colocating it with RabbitMqClusterName used for RPC. NotificationsBusInstance *string `json:"notificationsBusInstance,omitempty" deprecated:"notificationsBus.cluster"` + } type NeutronApiTLS struct { @@ -237,6 +238,12 @@ type NeutronAPIStatus struct { // NetworkAttachments status of the deployment pods NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"` + // ApplicationCredentialSecret - the AC secret NeutronAPI is currently + // consuming and protecting with the openstack.org/neutronapi-ac-consumer + // finalizer. Tracked so the controller can remove its finalizer from the + // old secret when the openstack-operator rotates the reference. + ApplicationCredentialSecret string `json:"applicationCredentialSecret,omitempty"` + // ObservedGeneration - the most recent generation observed for this // service. If the observed generation is less than the spec generation, // then the controller has not processed the latest changes injected by diff --git a/config/crd/bases/neutron.openstack.org_neutronapis.yaml b/config/crd/bases/neutron.openstack.org_neutronapis.yaml index 7a7f64a44..f2912df8b 100644 --- a/config/crd/bases/neutron.openstack.org_neutronapis.yaml +++ b/config/crd/bases/neutron.openstack.org_neutronapis.yaml @@ -1607,6 +1607,13 @@ spec: status: description: NeutronAPIStatus defines the observed state of NeutronAPI properties: + applicationCredentialSecret: + description: |- + ApplicationCredentialSecret - the AC secret NeutronAPI is currently + consuming and protecting with the openstack.org/neutronapi-ac-consumer + finalizer. Tracked so the controller can remove its finalizer from the + old secret when the openstack-operator rotates the reference. + type: string conditions: description: Conditions items: diff --git a/go.mod b/go.mod index 0b06a4e24..6d74e0ccb 100644 --- a/go.mod +++ b/go.mod @@ -9,13 +9,13 @@ require ( github.com/onsi/ginkgo/v2 v2.28.2 github.com/onsi/gomega v1.41.0 github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260508091801-73f228e6af31 - github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260507114237-f0b612d6c21f + github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260520090027-4d7b7a01c0bf github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260518125357-72bdd580c587 github.com/openstack-k8s-operators/lib-common/modules/storage v0.6.1-0.20260506154724-30a976ba8ef0 - github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260506154724-30a976ba8ef0 - github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260503164939-40728ae44d65 + github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260518125357-72bdd580c587 + github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260421135251-4fb605db7d18 github.com/openstack-k8s-operators/neutron-operator/api v0.0.0-00010101000000-000000000000 - github.com/openstack-k8s-operators/ovn-operator/api v0.6.1-0.20260428063332-55b3554934b5 + github.com/openstack-k8s-operators/ovn-operator/api v0.6.1-0.20260422180129-f058fa795f8e go.uber.org/zap v1.28.0 gopkg.in/ini.v1 v1.67.1 k8s.io/api v0.31.14 @@ -64,7 +64,7 @@ require ( github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/openshift/api v3.9.0+incompatible // indirect - github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20260430090237-a4265c18a162 // indirect + github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20260506154724-30a976ba8ef0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/prometheus/client_golang v1.22.0 // indirect github.com/prometheus/client_model v0.6.2 // indirect diff --git a/go.sum b/go.sum index 2be8af748..fa3ae9369 100644 --- a/go.sum +++ b/go.sum @@ -120,20 +120,20 @@ github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e h1:E1OdwSpqWuDPCedyU github.com/openshift/api v0.0.0-20250711200046-c86d80652a9e/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo= github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260508091801-73f228e6af31 h1:FWa0vNs175LpV1eSZ60YOGFdbJ3LqxQ1fxfprBRg7T4= github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260508091801-73f228e6af31/go.mod h1:/S2AN21zV70V1XuL0Of2dCjYWNkKwQSyNI8l/iQVrMs= -github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260507114237-f0b612d6c21f h1:28WYAUIef3uion0Pps6doCSSbgZtIcodGzwG6BHhCOw= -github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260507114237-f0b612d6c21f/go.mod h1:4ryvbSYuoN522BIPijnm0wMemPgJVKf7jCv8BNDq46I= +github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260520090027-4d7b7a01c0bf h1:FoKK0zNo48i4ZMFxScupCK/YAmy6Ps4IILz3CK4BCTI= +github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260520090027-4d7b7a01c0bf/go.mod h1:VNX1Mda2u5+yGxycIyVrgABucitMDR9ct3Lj6ROS92I= github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260518125357-72bdd580c587 h1:p03uEXoSreyu7LpFmb9YyYM8tEx2D2+7qqhLXNWHTq0= github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20260518125357-72bdd580c587/go.mod h1:JC04T5G4E/he5ukonV1oCqa0QzFkLv761VbLruVghJM= -github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20260430090237-a4265c18a162 h1:kUfZlcl+EbUBEWe6EGLXjzlUeYj7xZ21QsPA5jMJlwE= -github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20260430090237-a4265c18a162/go.mod h1:7yqbVpg0k0vW+kZks+TMU/cd1ovoejyHfVPWcyGYLHI= +github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20260506154724-30a976ba8ef0 h1:kMie+G0aHlGwDHjimjj8AUxTl2R7LGfai/8pev2T+TY= +github.com/openstack-k8s-operators/lib-common/modules/openstack v0.6.1-0.20260506154724-30a976ba8ef0/go.mod h1:7yqbVpg0k0vW+kZks+TMU/cd1ovoejyHfVPWcyGYLHI= github.com/openstack-k8s-operators/lib-common/modules/storage v0.6.1-0.20260506154724-30a976ba8ef0 h1:wuzSibIT9F/5RbMmxvBVFj6fy2vtKo58nibzmk5L4PM= github.com/openstack-k8s-operators/lib-common/modules/storage v0.6.1-0.20260506154724-30a976ba8ef0/go.mod h1:tft3oDiN+v6wX3ILPXGUM/gCLJz6QtrPN63hxpJ3E24= -github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260506154724-30a976ba8ef0 h1:mG3QhS/QWv9Y/AkZZ5OzO6hu6+l5oDXnI/Q5ZUbj6Zs= -github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260506154724-30a976ba8ef0/go.mod h1:ZYG9CQe7cOePOKQbenEZFA28kPdkUOe9QKbDRwGhEV0= -github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260503164939-40728ae44d65 h1:MNjmU326MKwYU6xT6AL2aKbr/0ids87wP5B+s5CL2O0= -github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260503164939-40728ae44d65/go.mod h1:I2LO+wGIzbirVczQ9qo/49y2F5Zo/MEg/pabkXOrY2M= -github.com/openstack-k8s-operators/ovn-operator/api v0.6.1-0.20260428063332-55b3554934b5 h1:E8KZkBUK2QzIEksFVxMUg0ETzvAX7uNuEg+2UqmX45I= -github.com/openstack-k8s-operators/ovn-operator/api v0.6.1-0.20260428063332-55b3554934b5/go.mod h1:ODYNTFMUlzvjlqXAh9AGXrzpBNQBAOkWiNQ6UldsqFw= +github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260518125357-72bdd580c587 h1:jpouKcgs2Kc5z2JHIpvsXMxEonfXLgzX3KswuBoeKQ0= +github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260518125357-72bdd580c587/go.mod h1:nLS2oK4pBo756JNN1cPgr44S0X9V11QScgVla89Ojok= +github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260421135251-4fb605db7d18 h1:fMMY6pp7+PEkcsdDoodTfJ6ct2RyGI8VB+toVJxxB5w= +github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260421135251-4fb605db7d18/go.mod h1:g/xgMnzNHxdTkqnEgAKwVOv75uPN4nuApbkGqSvASvs= +github.com/openstack-k8s-operators/ovn-operator/api v0.6.1-0.20260422180129-f058fa795f8e h1:Aks3Y8z8ZcvYim+qpYRr0OxtPCg9QqlHuloVowWrqZs= +github.com/openstack-k8s-operators/ovn-operator/api v0.6.1-0.20260422180129-f058fa795f8e/go.mod h1:ODYNTFMUlzvjlqXAh9AGXrzpBNQBAOkWiNQ6UldsqFw= 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= diff --git a/internal/controller/neutronapi_controller.go b/internal/controller/neutronapi_controller.go index ca0f01a22..6f0379b69 100644 --- a/internal/controller/neutronapi_controller.go +++ b/internal/controller/neutronapi_controller.go @@ -492,6 +492,19 @@ func (r *NeutronAPIReconciler) reconcileDelete(ctx context.Context, instance *ne ); err != nil { return ctrlResult, err } + // Remove consumer finalizer from AC secrets NeutronAPI was consuming. + // Check both status and spec to handle the edge case where the reconciler + // crashed after adding the finalizer but before updating the status. + for _, secretName := range []string{ + instance.Status.ApplicationCredentialSecret, + instance.Spec.Auth.ApplicationCredentialSecret, + } { + if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace, + secretName, neutronapi.ACConsumerFinalizer); err != nil { + return ctrl.Result{}, err + } + } + // Service is deleted so remove the finalizer. controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) Log.Info("Reconciled Service delete successfully") @@ -648,6 +661,24 @@ func (r *NeutronAPIReconciler) reconcileInit( // Create Secrets - end + // Add consumer finalizer to the new AC secret early, before deployment. + // The old secret's finalizer is removed later (after all services deploy) + // so that rapid rotations don't revoke a credential still in use by pods. + if instance.Spec.Auth.ApplicationCredentialSecret != "" { + if err := keystonev1.ManageACSecretFinalizer(ctx, helper, instance.Namespace, + instance.Spec.Auth.ApplicationCredentialSecret, + "", + neutronapi.ACConsumerFinalizer); err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.ServiceConfigReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.ServiceConfigReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err + } + } + instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) // @@ -1376,6 +1407,25 @@ func (r *NeutronAPIReconciler) reconcileNormal(ctx context.Context, instance *ne return ctrl.Result{}, err } } + // Manage the old AC secret's finalizer and status tracking. + // On rotation (old != new), only remove the old secret's finalizer after + // all sub-services are ready with the new credentials. This prevents + // premature revocation during rapid rotations. + isRotation := instance.Status.ApplicationCredentialSecret != "" && instance.Status.ApplicationCredentialSecret != instance.Spec.Auth.ApplicationCredentialSecret + + if isRotation { + allServicesReady := instance.Status.Conditions.AllSubConditionIsTrue() + if allServicesReady { + if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace, + instance.Status.ApplicationCredentialSecret, neutronapi.ACConsumerFinalizer); err != nil { + return ctrl.Result{}, err + } + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + } else { + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + // We reached the end of the Reconcile, update the Ready condition based on // the sub conditions if instance.Status.Conditions.AllSubConditionIsTrue() { diff --git a/internal/neutronapi/const.go b/internal/neutronapi/const.go index 23f313233..8b0c570cf 100644 --- a/internal/neutronapi/const.go +++ b/internal/neutronapi/const.go @@ -50,6 +50,9 @@ const ( // NeutronDhcpAgentSecretKey is the key in external Secret for Neutron DHCP Agent with agent config NeutronDhcpAgentSecretKey = "10-neutron-dhcp.conf" + + // ACConsumerFinalizer is added to AC secrets that neutron is actively consuming + ACConsumerFinalizer = "openstack.org/neutronapi-ac-consumer" ) // DbsyncPropagation keeps track of the DBSync Service Propagation Type diff --git a/test/functional/neutronapi_controller_test.go b/test/functional/neutronapi_controller_test.go index 7dea19eda..9e17796ae 100644 --- a/test/functional/neutronapi_controller_test.go +++ b/test/functional/neutronapi_controller_test.go @@ -2202,6 +2202,178 @@ func getNeutronAPIControllerSuite(ml2MechanismDrivers []string) func() { }, timeout, interval).Should(Succeed()) }) }) + + When("ApplicationCredential consumer finalizer is managed", func() { + var acSecretName string + + BeforeEach(func() { + acSecretName = "ac-neutron-a1b2c-secret" //nolint:gosec // G101 + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: acSecretName, + }, + Data: map[string][]byte{ + keystonev1.ACIDSecretKey: []byte("a1b2ctest-ac-id"), + keystonev1.ACSecretSecretKey: []byte("test-ac-secret"), + }, + } + DeferCleanup(k8sClient.Delete, ctx, secret) + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + + spec["auth"] = map[string]any{ + "applicationCredentialSecret": acSecretName, + } + + DeferCleanup(th.DeleteInstance, CreateNeutronAPI(neutronAPIName.Namespace, neutronAPIName.Name, spec)) + DeferCleanup(k8sClient.Delete, ctx, CreateNeutronAPISecret(namespace, SecretName)) + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, "memcached", memcachedSpec)) + infra.SimulateMemcachedReady(memcachedName) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + namespace, + GetNeutronAPI(neutronAPIName).Spec.DatabaseInstance, + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + SimulateTransportURLReady(apiTransportURLName) + mariadb.SimulateMariaDBAccountCompleted(types.NamespacedName{Namespace: namespace, Name: GetNeutronAPI(neutronAPIName).Spec.DatabaseAccount}) + mariadb.SimulateMariaDBDatabaseCompleted(types.NamespacedName{Namespace: namespace, Name: neutronapi.DatabaseCRName}) + + if isOVNEnabled { + DeferCleanup(DeleteOVNDBClusters, CreateOVNDBClusters(namespace)) + } + DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(namespace)) + }) + + It("should add the consumer finalizer to the AC secret", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(neutronapi.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + }) + + It("should track the consumed AC secret in status", func() { + th.SimulateJobSuccess(neutronDBSyncJobName) + keystone.SimulateKeystoneServiceReady(types.NamespacedName{ + Namespace: namespace, + Name: "neutron", + }) + keystone.SimulateKeystoneEndpointReady(types.NamespacedName{ + Namespace: namespace, + Name: "neutron", + }) + Eventually(func(g Gomega) { + n := GetNeutronAPI(neutronAPIName) + g.Expect(n.Status.ApplicationCredentialSecret).To(Equal(acSecretName)) + }, timeout, interval).Should(Succeed()) + }) + + It("should move the finalizer from the old to the new secret on rotation", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(neutronapi.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + th.SimulateJobSuccess(neutronDBSyncJobName) + th.SimulateDeploymentReplicaReady(types.NamespacedName{ + Namespace: namespace, + Name: "neutron", + }) + keystone.SimulateKeystoneServiceReady(types.NamespacedName{ + Namespace: namespace, + Name: "neutron", + }) + keystone.SimulateKeystoneEndpointReady(types.NamespacedName{ + Namespace: namespace, + Name: "neutron", + }) + Eventually(func(g Gomega) { + n := GetNeutronAPI(neutronAPIName) + g.Expect(n.Status.Conditions.IsTrue(condition.ReadyCondition)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) + + newACSecretName := "ac-neutron-x9y8z-secret" //nolint:gosec // G101 + newSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: newACSecretName, + }, + Data: map[string][]byte{ + keystonev1.ACIDSecretKey: []byte("x9y8zrotated-ac-id"), + keystonev1.ACSecretSecretKey: []byte("rotated-ac-secret"), + }, + } + DeferCleanup(k8sClient.Delete, ctx, newSecret) + Expect(k8sClient.Create(ctx, newSecret)).To(Succeed()) + + Eventually(func(g Gomega) { + n := GetNeutronAPI(neutronAPIName) + n.Spec.Auth.ApplicationCredentialSecret = newACSecretName + g.Expect(k8sClient.Update(ctx, n)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: namespace, + Name: newACSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(neutronapi.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateDeploymentReplicaReady(types.NamespacedName{ + Namespace: namespace, + Name: "neutron", + }) + secret := th.GetSecret(types.NamespacedName{ + Namespace: namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).NotTo( + ContainElement(neutronapi.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + n := GetNeutronAPI(neutronAPIName) + g.Expect(n.Status.ApplicationCredentialSecret).To(Equal(newACSecretName)) + }, timeout, interval).Should(Succeed()) + }) + + It("should remove the consumer finalizer from AC secret on CR deletion", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(neutronapi.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + th.DeleteInstance(GetNeutronAPI(neutronAPIName)) + + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).NotTo( + ContainElement(neutronapi.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + }) + }) } }