Add application credential finalizer management by Deydra71 · Pull Request #643 · openstack-k8s-operators/heat-operator

Deydra71 · 2026-04-23T14:06:36Z

Application Credential dev-doc: https://github.com/openstack-k8s-operators/dev-docs/blob/main/application_credentials.md

Tracks the active AC secret name in Status.ApplicationCredentialSecret
Add openstack.org/heat-ac-consumer finalizer to the AC secret after service config is rendered
On AC rotation, move the finalizer from the old secret to the new one
On CR deletion, remove the consumer finalizer from the AC secret before cleaning up the CR

This ensures that the keystone-operator cannot revoke a rotated AC secret while Heat is still consuming it.

2026-04-28T11:55:51Z	INFO	Controllers.Heat	Added consumer finalizer	{"controller": "heat", "controllerGroup": "heat.openstack.org", "controllerKind": "Heat", "Heat": {"name":"heat","namespace":"openstack"}, "namespace": "openstack", "name": "heat", "reconcileID": "82672178-a3a3-4bd4-b3d5-87c6d5853d9b", "object": "ac-heat-aa0a4-secret", "finalizer": "openstack.org/heat-ac-consumer"}
2026-04-28T11:55:51Z	INFO	Controllers.Heat	Removed consumer finalizer	{"controller": "heat", "controllerGroup": "heat.openstack.org", "controllerKind": "Heat", "Heat": {"name":"heat","namespace":"openstack"}, "namespace": "openstack", "name": "heat", "reconcileID": "82672178-a3a3-4bd4-b3d5-87c6d5853d9b", "object": "ac-heat-ba179-secret", "finalizer": "openstack.org/heat-ac-consumer"}

Depends-On: openstack-k8s-operators/keystone-operator#685

Assisted-by: Claude Opus 4.6 noreply@anthropic.com

openshift-ci · 2026-04-23T14:06:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Deydra71
Once this PR has been reviewed and has the lgtm label, please assign dprince for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

bshephar

I'm an outsider now, as an outsider it would be nice to have some more context about this change in the commit message. :)

Maybe it's still a WIP, but there's a few important things I've noted in-line.

bshephar · 2026-05-01T22:40:38Z

+	// crashed after adding the finalizer but before updating the status.
+	for _, secretName := range []string{
+		instance.Status.ApplicationCredentialSecret,
+		instance.Spec.Auth.ApplicationCredentialSecret,


Where is Auth coming from? The HeatSpec doesn't have a Auth defined. Should this change also be adding Auth?

Hi @bshephar! Thanks for reviewing.

Auth comes from HeatTemplate (embedded inline in HeatSpecBase -> HeatSpec), defined in common_types.go https://github.com/openstack-k8s-operators/heat-operator/blob/main/api/v1beta1/common_types.go#L63 . So, it already comes from earlier change.

I think my fork was out of sync. Yeah, I see it there, my bad.

bshephar · 2026-05-01T22:45:43Z

+	if instance.Spec.Auth.ApplicationCredentialSecret != "" || instance.Status.ApplicationCredentialSecret != "" {
+		if err := keystonev1.ManageACSecretFinalizer(ctx, helper, instance.Namespace,
+			instance.Spec.Auth.ApplicationCredentialSecret,
+			instance.Status.ApplicationCredentialSecret,


Just a note that Status.ApplicationCredentialSecret is set to omitempty, so if this is undefined for any reason, you will be passing nil here which could end up with nil pointer deref issues. I would either check that instance.Status.ApplicationCredentialSecret != nil before doing this, or if you think it will always be set, remove the omitempty from the Status field?

ApplicationCredentialSecret is a string type, so its zero value is "", not nil, as far as I see there's no nil pointer dereference risk here. And the != "" check on the line above already guards against passing an empty string to the finalizer function

openshift-ci · 2026-05-01T22:49:57Z

@bshephar: changing LGTM is restricted to collaborators

Details

In response to this:

I'm an outsider now, as an outsider it would be nice to have some more context about this change in the commit message. :)

Maybe it's still a WIP, but there's a few important things I've noted in-line.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

centosinfra-prod-github-app · 2026-05-04T08:31:11Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/rdo/buildset/849a9504196145fea3f5e5dbc21e1456

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 56m 15s
❌ heat-operator-tempest-multinode FAILURE in 1h 36m 21s

centosinfra-prod-github-app · 2026-05-20T11:16:10Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/rdoproject.org/buildset/595b169ceb084e46ae2d6f70db2a7b00

❌ openstack-k8s-operators-content-provider FAILURE in 4m 09s
⚠️ heat-operator-tempest-multinode SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

Deydra71 · 2026-05-21T15:21:23Z

/test heat-operator-build-deploy-kuttl

Deydra71 · 2026-05-21T15:21:28Z

recheck

Heat now tracks which AC secret it is consuming via Status.ApplicationCredentialSecret and manages the openstack.org/heat-ac-consumer finalizer on that secret. This ensures keystone-operator does not prematurely revoke the application credential while Heat is still using it. On rotation (when the spec reference changes), the finalizer is moved from the old secret to the new one. On Heat CR deletion, the finalizer is cleaned up from all referenced secrets. Signed-off-by: Veronika Fisarova <vfisarov@redhat.com>

Deydra71 · 2026-05-25T11:40:03Z

Following the discussion in watcher-operator the AC finalizer management is now split into two phases:

Early phase: adds consumer finalizer to the new AC secret immediately (protects it from premature revocation)
Late phase: removes consumer finalizer from the old AC secret only after AllSubConditionIsTrue() (all sub-services deployed with new credentials)

This prevents a race condition where rapid AC rotations could revoke credentials still in use by running pods.

The new file api_fixture.go: heat's functional tests need a fake Keystone HTTP server to satisfy Keystone calls. SImilar pattern is used e.g. in octavia-operator - https://github.com/openstack-k8s-operators/octavia-operator/blob/main/test/functional/api_fixture.go

Deydra71 added the do-not-merge/hold label Apr 23, 2026

openshift-ci Bot requested review from abays and dprince April 23, 2026 14:06

Deydra71 changed the title ~~Add AC finalizer management~~ Add application credential finalizer management Apr 23, 2026

Deydra71 force-pushed the appcred-finalizer branch from d14e484 to ec6c565 Compare April 24, 2026 10:49

openshift-ci Bot added the needs-rebase label Apr 26, 2026

Deydra71 force-pushed the appcred-finalizer branch from ec6c565 to 60522f3 Compare April 27, 2026 08:20

openshift-ci Bot removed the needs-rebase label Apr 27, 2026

bshephar suggested changes May 1, 2026

View reviewed changes

Deydra71 force-pushed the appcred-finalizer branch from 60522f3 to 4e9ab56 Compare May 4, 2026 06:33

openshift-ci Bot added the needs-rebase label May 19, 2026

Deydra71 force-pushed the appcred-finalizer branch from 4e9ab56 to 32c942f Compare May 20, 2026 11:04

openshift-ci Bot removed the needs-rebase label May 20, 2026

Deydra71 removed the do-not-merge/hold label May 20, 2026

Deydra71 force-pushed the appcred-finalizer branch from 32c942f to 422f1b9 Compare May 25, 2026 11:38

Deydra71 requested a review from rabi May 28, 2026 13:28

Conversation

Deydra71 commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci Bot commented Apr 23, 2026

Uh oh!

bshephar left a comment

Choose a reason for hiding this comment

Uh oh!

bshephar May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Deydra71 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

bshephar May 26, 2026

Choose a reason for hiding this comment

Uh oh!

bshephar May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Deydra71 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented May 1, 2026

Uh oh!

centosinfra-prod-github-app Bot commented May 4, 2026

Uh oh!

centosinfra-prod-github-app Bot commented May 20, 2026

Uh oh!

Deydra71 commented May 21, 2026

Uh oh!

Deydra71 commented May 21, 2026

Uh oh!

Deydra71 commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Deydra71 commented Apr 23, 2026 •

edited

Loading