diff --git a/api/bases/barbican.openstack.org_barbicans.yaml b/api/bases/barbican.openstack.org_barbicans.yaml index 98382ef2..4bf79dce 100644 --- a/api/bases/barbican.openstack.org_barbicans.yaml +++ b/api/bases/barbican.openstack.org_barbicans.yaml @@ -1962,6 +1962,13 @@ spec: status: description: BarbicanStatus defines the observed state of Barbican properties: + applicationCredentialSecret: + description: |- + ApplicationCredentialSecret - the AC secret barbican is currently + consuming and protecting with the openstack.org/barbican-ac-consumer + finalizer. Tracked so the controller can remove its finalizer from the + old secret when the openstack-operator rotates the reference. + type: string barbicanAPIReadyCount: description: ReadyCount of Barbican API instances format: int32 diff --git a/api/v1beta1/barbican_types.go b/api/v1beta1/barbican_types.go index 09e35600..9553e9f5 100644 --- a/api/v1beta1/barbican_types.go +++ b/api/v1beta1/barbican_types.go @@ -153,6 +153,12 @@ type BarbicanStatus struct { // Barbican Database Hostname DatabaseHostname string `json:"databaseHostname,omitempty"` + // ApplicationCredentialSecret - the AC secret barbican is currently + // consuming and protecting with the openstack.org/barbican-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/barbican.openstack.org_barbicans.yaml b/config/crd/bases/barbican.openstack.org_barbicans.yaml index 98382ef2..4bf79dce 100644 --- a/config/crd/bases/barbican.openstack.org_barbicans.yaml +++ b/config/crd/bases/barbican.openstack.org_barbicans.yaml @@ -1962,6 +1962,13 @@ spec: status: description: BarbicanStatus defines the observed state of Barbican properties: + applicationCredentialSecret: + description: |- + ApplicationCredentialSecret - the AC secret barbican is currently + consuming and protecting with the openstack.org/barbican-ac-consumer + finalizer. Tracked so the controller can remove its finalizer from the + old secret when the openstack-operator rotates the reference. + type: string barbicanAPIReadyCount: description: ReadyCount of Barbican API instances format: int32 diff --git a/go.mod b/go.mod index d4bbf36b..9b302bb8 100644 --- a/go.mod +++ b/go.mod @@ -10,14 +10,14 @@ require ( github.com/onsi/gomega v1.41.0 github.com/openstack-k8s-operators/barbican-operator/api v0.0.0-00010101000000-000000000000 github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260513130700-78e1a15a8289 - github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260513130126-175a0958de92 + 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.20260515134210-2e2a0d06648c - github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260515134210-2e2a0d06648c - github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260513143847-4b70b899997a + 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 go.uber.org/zap v1.28.0 golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67 - gopkg.in/ini.v1 v1.67.2 + gopkg.in/ini.v1 v1.67.1 k8s.io/api v0.31.14 k8s.io/apimachinery v0.31.14 k8s.io/client-go v0.31.14 diff --git a/go.sum b/go.sum index d2e24620..43ec42ab 100644 --- a/go.sum +++ b/go.sum @@ -120,18 +120,18 @@ 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.20260513130700-78e1a15a8289 h1:c7jcPJt1jINUqWcYhmpeUDzeesOd0SfrFOpxEHHoJ7c= github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20260513130700-78e1a15a8289/go.mod h1:1FRevwpKwNgNjDcd8Rz+mxlRleFsi7gNS0BC09x+oYE= -github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260513130126-175a0958de92 h1:9brYhCAbx+Cwfq8cEtn/g4xF0w6FlyM7CSuDqertxNQ= -github.com/openstack-k8s-operators/keystone-operator/api v0.6.1-0.20260513130126-175a0958de92/go.mod h1:/nxao9LzRStCZPOpyY61Sbfzcogy5BrEP5nExUwFYZ0= +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.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.20260515134210-2e2a0d06648c h1:yEjjh0YObWmY0lKqrvBOZpStS832nUrOHvRfDc+t3d0= github.com/openstack-k8s-operators/lib-common/modules/storage v0.6.1-0.20260515134210-2e2a0d06648c/go.mod h1:tft3oDiN+v6wX3ILPXGUM/gCLJz6QtrPN63hxpJ3E24= -github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260515134210-2e2a0d06648c h1:R68ZhKmcepRdiIC7m7pHXSD3Ycayt6RdF2dP6cg3ga4= -github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20260515134210-2e2a0d06648c/go.mod h1:ZYG9CQe7cOePOKQbenEZFA28kPdkUOe9QKbDRwGhEV0= -github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260513143847-4b70b899997a h1:5P4XUoB5s2NcPZiYWV2s7xBDLNJg4kMzJNxRreR65Aw= -github.com/openstack-k8s-operators/mariadb-operator/api v0.6.1-0.20260513143847-4b70b899997a/go.mod h1:WHqmWfERjBKzbWUN9FU6n8p4FXcF9BKSFNklApqYgHw= +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/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= @@ -270,8 +270,8 @@ gopkg.in/evanphx/json-patch.v4 v4.12.0 h1:n6jtcsulIzXPJaxegRbvFNNrZDjbij7ny3gmSP gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= -gopkg.in/ini.v1 v1.67.2 h1:JtOSMb9OuaCZKr7h5D/h6iii14sK0hLbplTc6frx4Ss= -gopkg.in/ini.v1 v1.67.2/go.mod h1:x/cyOwCgZqOkJoDIJ3c1KNHMo10+nLGAhh+kn3Zizss= +gopkg.in/ini.v1 v1.67.1 h1:tVBILHy0R6e4wkYOn3XmiITt/hEVH4TFMYvAX2Ytz6k= +gopkg.in/ini.v1 v1.67.1/go.mod h1:x/cyOwCgZqOkJoDIJ3c1KNHMo10+nLGAhh+kn3Zizss= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= diff --git a/internal/barbican/const.go b/internal/barbican/const.go index 333a2e9d..94d72202 100644 --- a/internal/barbican/const.go +++ b/internal/barbican/const.go @@ -73,4 +73,6 @@ const ( BarbicanUID int64 = 42403 // BarbicanGID - based on https://github.com/openstack-k8s-operators/tcib/blob/main/container-images/kolla/base/uid_gid_manage.sh BarbicanGID int64 = 42403 + // ACConsumerFinalizer is added to AC secrets that barbican is actively consuming + ACConsumerFinalizer = "openstack.org/barbican-ac-consumer" ) diff --git a/internal/controller/barbican_controller.go b/internal/controller/barbican_controller.go index f5bcc2dd..662f0674 100644 --- a/internal/controller/barbican_controller.go +++ b/internal/controller/barbican_controller.go @@ -389,6 +389,23 @@ func (r *BarbicanReconciler) reconcileNormal(ctx context.Context, instance *barb return ctrl.Result{}, err } + // 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, + "", + barbican.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) // networks to attach to @@ -517,6 +534,27 @@ func (r *BarbicanReconciler) reconcileNormal(ctx context.Context, instance *barb // TODO(dmendiza): Understand what Glance is doing with the API conditions and maybe do it here too + // 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.IsTrue(barbicanv1beta1.BarbicanAPIReadyCondition) && + instance.Status.Conditions.IsTrue(barbicanv1beta1.BarbicanWorkerReadyCondition) && + instance.Status.Conditions.IsTrue(barbicanv1beta1.BarbicanKeystoneListenerReadyCondition) + if allServicesReady { + if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace, + instance.Status.ApplicationCredentialSecret, barbican.ACConsumerFinalizer); err != nil { + return ctrl.Result{}, err + } + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + } else { + instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret + } + // Update the lastObserved generation before evaluating conditions instance.Status.ObservedGeneration = instance.Generation // We reached the end of the Reconcile, update the Ready condition based on @@ -611,6 +649,19 @@ func (r *BarbicanReconciler) reconcileDelete(ctx context.Context, instance *barb } } + // Remove consumer finalizer from AC secrets barbican 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, barbican.ACConsumerFinalizer); err != nil { + return ctrl.Result{}, err + } + } + // Service is deleted so remove the finalizer. controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) Log.Info(fmt.Sprintf("Reconciled Service '%s' delete successfully", instance.Name)) diff --git a/test/functional/barbican_controller_test.go b/test/functional/barbican_controller_test.go index 3b72e40e..53a50160 100644 --- a/test/functional/barbican_controller_test.go +++ b/test/functional/barbican_controller_test.go @@ -1974,6 +1974,179 @@ var _ = Describe("Barbican controller", func() { }) }) + When("ApplicationCredential consumer finalizer is managed", func() { + var ( + acSecretName string + servicePasswordSecret string + ) + + BeforeEach(func() { + servicePasswordSecret = "ac-test-osp-secret" //nolint:gosec // G101 + + DeferCleanup(k8sClient.Delete, ctx, + CreateBarbicanMessageBusSecret( + barbicanTest.Instance.Namespace, + barbicanTest.RabbitmqSecretName, + ), + ) + DeferCleanup(k8sClient.Delete, ctx, + CreateBarbicanSecret( + barbicanTest.Instance.Namespace, servicePasswordSecret)) + + acSecretName = "ac-barbican-a1b2c-secret" //nolint:gosec // G101 + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: barbicanTest.Instance.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 := GetDefaultBarbicanSpec() + spec["secret"] = servicePasswordSecret + spec["simpleCryptoBackendSecret"] = servicePasswordSecret + spec["auth"] = map[string]any{ + "applicationCredentialSecret": acSecretName, + } + DeferCleanup(th.DeleteInstance, + CreateBarbican(barbicanTest.Instance, spec)) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + barbicanTest.Instance.Namespace, + GetBarbican(barbicanTest.Instance).Spec.DatabaseInstance, + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}})) + + DeferCleanup(keystone.DeleteKeystoneAPI, + keystone.CreateKeystoneAPI(barbicanTest.Instance.Namespace)) + + infra.SimulateTransportURLReady(barbicanTest.BarbicanTransportURL) + mariadb.SimulateMariaDBAccountCompleted(barbicanTest.BarbicanDatabaseAccount) + mariadb.SimulateMariaDBDatabaseCompleted(barbicanTest.BarbicanDatabaseName) + th.SimulateJobSuccess(barbicanTest.BarbicanDBSync) + keystone.SimulateKeystoneEndpointReady(barbicanTest.BarbicanKeystoneEndpoint) + }) + + It("should add the consumer finalizer to the AC secret", func() { + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: barbicanTest.Instance.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(barbican.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + }) + + It("should track the consumed AC secret in status", func() { + Eventually(func(g Gomega) { + b := GetBarbican(barbicanTest.Instance) + g.Expect(b.Status.ApplicationCredentialSecret).To(Equal(acSecretName)) + }, timeout, interval).Should(Succeed()) + }) + + It("should move the finalizer from the old to the new secret on rotation", func() { + // Wait for the initial finalizer to appear + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: barbicanTest.Instance.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(barbican.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + // Simulate all sub-services becoming Ready so that the split + // finalizer pattern allows removing the old secret's finalizer + // after rotation. + th.SimulateDeploymentReplicaReady(barbicanTest.BarbicanAPI) + th.SimulateDeploymentReplicaReady(barbicanTest.BarbicanKeystoneListener) + th.SimulateDeploymentReplicaReady(barbicanTest.BarbicanWorker) + Eventually(func(g Gomega) { + b := GetBarbican(barbicanTest.Instance) + g.Expect(b.Status.Conditions.IsTrue(barbicanv1beta1.BarbicanAPIReadyCondition)).To(BeTrue()) + g.Expect(b.Status.Conditions.IsTrue(barbicanv1beta1.BarbicanWorkerReadyCondition)).To(BeTrue()) + g.Expect(b.Status.Conditions.IsTrue(barbicanv1beta1.BarbicanKeystoneListenerReadyCondition)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) + + // Create a new AC secret + newACSecretName := "ac-barbican-x9y8z-secret" //nolint:gosec // G101 + newSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: barbicanTest.Instance.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()) + + // Update the Barbican CR to reference the new AC secret + Eventually(func(g Gomega) { + b := GetBarbican(barbicanTest.Instance) + b.Spec.Auth.ApplicationCredentialSecret = newACSecretName + g.Expect(k8sClient.Update(ctx, b)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + // New secret should gain the consumer finalizer + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: barbicanTest.Instance.Namespace, + Name: newACSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(barbican.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + // Old secret should lose the consumer finalizer + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: barbicanTest.Instance.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).NotTo( + ContainElement(barbican.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + // Status should reflect the new secret + Eventually(func(g Gomega) { + b := GetBarbican(barbicanTest.Instance) + g.Expect(b.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: barbicanTest.Instance.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).To( + ContainElement(barbican.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + + th.DeleteInstance(GetBarbican(barbicanTest.Instance)) + + Eventually(func(g Gomega) { + secret := th.GetSecret(types.NamespacedName{ + Namespace: barbicanTest.Instance.Namespace, + Name: acSecretName, + }) + g.Expect(secret.Finalizers).NotTo( + ContainElement(barbican.ACConsumerFinalizer)) + }, timeout, interval).Should(Succeed()) + }) + }) + // Run MariaDBAccount suite tests. these are pre-packaged ginkgo tests // that exercise standard account create / update patterns that should be // common to all controllers that ensure MariaDBAccount CRs.