Skip to content

Add application credential finalizer management #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Jun 2, 2026
Merged

Add application credential finalizer management #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions api/bases/barbican.openstack.org_barbicans.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions api/v1beta1/barbican_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions config/crd/bases/barbican.openstack.org_barbicans.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
2 changes: 2 additions & 0 deletions internal/barbican/const.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
51 changes: 51 additions & 0 deletions internal/controller/barbican_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
173 changes: 173 additions & 0 deletions test/functional/barbican_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})

@mauricioharley mauricioharley Apr 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice set of tests here - covers the add, the status tracking, and the rotation move. One thing I'm missing though: what happens when the Barbican CR itself gets deleted? The reconcileDelete logic removes the consumer finalizer from the AC secret, but there's no test exercising that path.

Something along the lines of:

It("should remove the consumer finalizer from AC secret on Barbican CR deletion", func() {
    // Wait for the consumer finalizer to appear on the AC secret
    // ...
    // Delete the Barbican CR
    // ...
    // Verify the AC secret no longer carries the barbican-ac-consumer finalizer
    // ...
})

Would be good to have that covered so regressions don't sneak in.


// 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.
Expand Down
Loading