Skip to content

Commit c9af0df

Browse files
committed
Add application credential finalizer management
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>
1 parent a25a9ee commit c9af0df

10 files changed

Lines changed: 477 additions & 2 deletions

File tree

api/bases/heat.openstack.org_heats.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,6 +2093,13 @@ spec:
20932093
status:
20942094
description: HeatStatus defines the observed state of Heat
20952095
properties:
2096+
applicationCredentialSecret:
2097+
description: |-
2098+
ApplicationCredentialSecret - the AC secret Heat is currently
2099+
consuming and protecting with the openstack.org/heat-ac-consumer
2100+
finalizer. Tracked so the controller can remove its finalizer from the
2101+
old secret when the openstack-operator rotates the reference.
2102+
type: string
20962103
conditions:
20972104
description: Conditions
20982105
items:

api/v1beta1/heat_types.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ const (
4242
HeatDatabaseMigrationAnnotation = "heat.openstack.org/database-migration"
4343
)
4444

45-
4645
// HeatSpec defines the desired state of Heat
4746
type HeatSpec struct {
4847
HeatSpecBase `json:",inline"`
@@ -177,6 +176,12 @@ type HeatStatus struct {
177176
// ReadyCount of Heat Engine instance
178177
HeatEngineReadyCount int32 `json:"heatEngineReadyCount,omitempty"`
179178

179+
// ApplicationCredentialSecret - the AC secret Heat is currently
180+
// consuming and protecting with the openstack.org/heat-ac-consumer
181+
// finalizer. Tracked so the controller can remove its finalizer from the
182+
// old secret when the openstack-operator rotates the reference.
183+
ApplicationCredentialSecret string `json:"applicationCredentialSecret,omitempty"`
184+
180185
// ObservedGeneration - the most recent generation observed for this
181186
// service. If the observed generation is less than the spec generation,
182187
// then the controller has not processed the latest changes injected by

config/crd/bases/heat.openstack.org_heats.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,6 +2093,13 @@ spec:
20932093
status:
20942094
description: HeatStatus defines the observed state of Heat
20952095
properties:
2096+
applicationCredentialSecret:
2097+
description: |-
2098+
ApplicationCredentialSecret - the AC secret Heat is currently
2099+
consuming and protecting with the openstack.org/heat-ac-consumer
2100+
finalizer. Tracked so the controller can remove its finalizer from the
2101+
old secret when the openstack-operator rotates the reference.
2102+
type: string
20962103
conditions:
20972104
description: Conditions
20982105
items:

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ replace github.com/openstack-k8s-operators/heat-operator/api => ./api
77
require (
88
github.com/go-logr/logr v1.4.3
99
github.com/google/uuid v1.6.0
10+
github.com/gophercloud/gophercloud/v2 v2.8.0
1011
github.com/onsi/ginkgo/v2 v2.28.2
1112
github.com/onsi/gomega v1.41.0
1213
github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240214134649-6643d1b09d49
@@ -52,7 +53,6 @@ require (
5253
github.com/google/go-cmp v0.7.0 // indirect
5354
github.com/google/gofuzz v1.2.0 // indirect
5455
github.com/google/pprof v0.0.0-20260115054156-294ebfa9ad83 // indirect
55-
github.com/gophercloud/gophercloud/v2 v2.8.0 // indirect
5656
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 // indirect
5757
github.com/imdario/mergo v0.3.16 // indirect
5858
github.com/inconshreveable/mousetrap v1.1.0 // indirect

internal/controller/heat_controller.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,19 @@ func (r *HeatReconciler) reconcileDelete(ctx context.Context, instance *heatv1be
466466
}
467467
}
468468

469+
// Remove consumer finalizer from AC secrets Heat was consuming.
470+
// Check both status and spec to handle the edge case where the reconciler
471+
// crashed after adding the finalizer but before updating the status.
472+
for _, secretName := range []string{
473+
instance.Status.ApplicationCredentialSecret,
474+
instance.Spec.Auth.ApplicationCredentialSecret,
475+
} {
476+
if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace,
477+
secretName, heat.ACConsumerFinalizer); err != nil {
478+
return ctrl.Result{}, err
479+
}
480+
}
481+
469482
// Service is deleted so remove the finalizer.
470483
controllerutil.RemoveFinalizer(instance, helper.GetFinalizer())
471484
Log.Info("Reconciled Heat delete successfully")
@@ -813,6 +826,24 @@ func (r *HeatReconciler) reconcileNormal(ctx context.Context, instance *heatv1be
813826

814827
// Create Secrets - end
815828

829+
// Add consumer finalizer to the new AC secret early, before deployment.
830+
// The old secret's finalizer is removed later (after all services deploy)
831+
// so that rapid rotations don't revoke a credential still in use by pods.
832+
if instance.Spec.Auth.ApplicationCredentialSecret != "" {
833+
if err := keystonev1.ManageACSecretFinalizer(ctx, helper, instance.Namespace,
834+
instance.Spec.Auth.ApplicationCredentialSecret,
835+
"",
836+
heat.ACConsumerFinalizer); err != nil {
837+
instance.Status.Conditions.Set(condition.FalseCondition(
838+
condition.ServiceConfigReadyCondition,
839+
condition.ErrorReason,
840+
condition.SeverityWarning,
841+
condition.ServiceConfigReadyErrorMessage,
842+
err.Error()))
843+
return ctrl.Result{}, err
844+
}
845+
}
846+
816847
instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)
817848

818849
//
@@ -1033,6 +1064,25 @@ func (r *HeatReconciler) reconcileNormal(ctx context.Context, instance *heatv1be
10331064
Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
10341065
}
10351066
}
1067+
// Manage the old AC secret's finalizer and status tracking.
1068+
// On rotation (old != new), only remove the old secret's finalizer after
1069+
// all sub-services are ready with the new credentials. This prevents
1070+
// premature revocation during rapid rotations.
1071+
isRotation := instance.Status.ApplicationCredentialSecret != "" && instance.Status.ApplicationCredentialSecret != instance.Spec.Auth.ApplicationCredentialSecret
1072+
1073+
if isRotation {
1074+
allServicesReady := instance.Status.Conditions.AllSubConditionIsTrue()
1075+
if allServicesReady {
1076+
if err := keystonev1.RemoveACSecretConsumerFinalizer(ctx, helper, instance.Namespace,
1077+
instance.Status.ApplicationCredentialSecret, heat.ACConsumerFinalizer); err != nil {
1078+
return ctrl.Result{}, err
1079+
}
1080+
instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret
1081+
}
1082+
} else {
1083+
instance.Status.ApplicationCredentialSecret = instance.Spec.Auth.ApplicationCredentialSecret
1084+
}
1085+
10361086
// We reached the end of the Reconcile, update the Ready condition based on
10371087
// the sub conditions
10381088
if instance.Status.Conditions.AllSubConditionIsTrue() {

internal/heat/const.go

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

9393
// HeatManage is the base command used for DBPurge
9494
HeatManage = "/usr/bin/heat-manage"
95+
96+
// ACConsumerFinalizer is added to AC secrets that heat is actively consuming
97+
ACConsumerFinalizer = "openstack.org/heat-ac-consumer"
9598
)
9699

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

test/functional/api_fixture.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/*
2+
Copyright 2026.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Package functional_test contains functional tests for the heat operator.
18+
package functional_test
19+
20+
import (
21+
"encoding/json"
22+
"fmt"
23+
"io"
24+
"net/http"
25+
26+
"github.com/go-logr/logr"
27+
"github.com/google/uuid"
28+
"github.com/gophercloud/gophercloud/v2/openstack/identity/v3/roles"
29+
keystone_helpers "github.com/openstack-k8s-operators/keystone-operator/api/test/helpers"
30+
api "github.com/openstack-k8s-operators/lib-common/modules/test/apis"
31+
)
32+
33+
func heatKeystoneTokenResponse(keystoneURL string) string {
34+
return fmt.Sprintf(
35+
`{"token":{"catalog":[{"endpoints":[{"interface":"internal","region_id":"RegionOne","url":"%s","region":"RegionOne"},{"interface":"public","region_id":"RegionOne","url":"%s","region":"RegionOne"}],"type":"identity","name":"keystone"}]}}`,
36+
keystoneURL, keystoneURL)
37+
}
38+
39+
func setupHeatKeystoneFixture(logger logr.Logger) *keystone_helpers.KeystoneAPIFixture {
40+
fixture := keystone_helpers.NewKeystoneAPIFixtureWithServer(logger)
41+
roleStore := map[string]roles.Role{
42+
"admin": {
43+
Name: "admin",
44+
ID: uuid.NewString(),
45+
},
46+
}
47+
48+
fixture.Setup(
49+
api.Handler{Pattern: "/", Func: fixture.HandleVersion},
50+
api.Handler{Pattern: "/v3/users", Func: fixture.HandleUsers},
51+
api.Handler{Pattern: "/v3/domains", Func: fixture.HandleDomains},
52+
api.Handler{Pattern: "/v3/roles", Func: func(w http.ResponseWriter, r *http.Request) {
53+
fixture.LogRequest(r)
54+
switch r.Method {
55+
case "GET":
56+
nameFilter := r.URL.Query().Get("name")
57+
var rs []roles.Role
58+
for name, role := range roleStore {
59+
if nameFilter == "" || name == nameFilter {
60+
rs = append(rs, role)
61+
}
62+
}
63+
resp := struct {
64+
Roles []roles.Role `json:"roles"`
65+
}{Roles: rs}
66+
bytes, err := json.Marshal(&resp)
67+
if err != nil {
68+
fixture.InternalError(err, "Error during marshalling response", w, r)
69+
return
70+
}
71+
w.Header().Add("Content-Type", "application/json")
72+
w.WriteHeader(http.StatusOK)
73+
_, _ = fmt.Fprint(w, string(bytes))
74+
case "POST":
75+
bytes, err := io.ReadAll(r.Body)
76+
if err != nil {
77+
fixture.InternalError(err, "Error reading request body", w, r)
78+
return
79+
}
80+
var s struct {
81+
Role roles.Role `json:"role"`
82+
}
83+
if err := json.Unmarshal(bytes, &s); err != nil {
84+
fixture.InternalError(err, "Error during unmarshalling request", w, r)
85+
return
86+
}
87+
if s.Role.ID == "" {
88+
s.Role.ID = uuid.NewString()
89+
}
90+
roleStore[s.Role.Name] = s.Role
91+
respBytes, err := json.Marshal(&s)
92+
if err != nil {
93+
fixture.InternalError(err, "Error during marshalling response", w, r)
94+
return
95+
}
96+
w.Header().Add("Content-Type", "application/json")
97+
w.WriteHeader(http.StatusCreated)
98+
_, _ = fmt.Fprint(w, string(respBytes))
99+
default:
100+
fixture.UnexpectedRequest(w, r)
101+
}
102+
}},
103+
api.Handler{Pattern: "/v3/role_assignments", Func: func(w http.ResponseWriter, r *http.Request) {
104+
fixture.LogRequest(r)
105+
if r.Method != "GET" {
106+
fixture.UnexpectedRequest(w, r)
107+
return
108+
}
109+
w.Header().Add("Content-Type", "application/json")
110+
w.WriteHeader(http.StatusOK)
111+
_, _ = fmt.Fprint(w, `{"role_assignments":[]}`)
112+
}},
113+
api.Handler{Pattern: "/v3/domains/", Func: func(w http.ResponseWriter, r *http.Request) {
114+
fixture.LogRequest(r)
115+
if r.Method == "PUT" {
116+
w.WriteHeader(http.StatusNoContent)
117+
return
118+
}
119+
fixture.UnexpectedRequest(w, r)
120+
}},
121+
api.Handler{Pattern: "/v3/auth/tokens", Func: func(w http.ResponseWriter, r *http.Request) {
122+
fixture.LogRequest(r)
123+
if r.Method != "POST" {
124+
fixture.UnexpectedRequest(w, r)
125+
return
126+
}
127+
w.Header().Add("Content-Type", "application/json")
128+
w.WriteHeader(http.StatusAccepted)
129+
_, _ = fmt.Fprint(w, heatKeystoneTokenResponse(fixture.Endpoint()))
130+
}},
131+
)
132+
return fixture
133+
}

test/functional/base_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@ package functional_test
1919
import (
2020
. "github.com/onsi/gomega" //revive:disable:dot-imports
2121

22+
appsv1 "k8s.io/api/apps/v1"
2223
batchv1 "k8s.io/api/batch/v1"
2324
corev1 "k8s.io/api/core/v1"
2425
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
2526
"k8s.io/apimachinery/pkg/types"
2627
"sigs.k8s.io/controller-runtime/pkg/client"
2728

2829
heatv1 "github.com/openstack-k8s-operators/heat-operator/api/v1beta1"
30+
"github.com/openstack-k8s-operators/heat-operator/internal/heat"
2931
rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1"
3032
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
3133
)
@@ -155,6 +157,81 @@ func GetHeatSpecWithNotificationsBus(notificationsCluster *string, notifications
155157
return spec
156158
}
157159

160+
func simulateHeatSubCRReady(g Gomega, name types.NamespacedName) {
161+
heatAPI := &heatv1.HeatAPI{}
162+
g.Expect(k8sClient.Get(ctx, types.NamespacedName{
163+
Namespace: name.Namespace,
164+
Name: name.Name + "-api",
165+
}, heatAPI)).To(Succeed())
166+
heatAPI.Status.ObservedGeneration = heatAPI.Generation
167+
heatAPI.Status.ReadyCount = 1
168+
heatAPI.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
169+
g.Expect(k8sClient.Status().Update(ctx, heatAPI)).To(Succeed())
170+
171+
heatCfnAPI := &heatv1.HeatCfnAPI{}
172+
g.Expect(k8sClient.Get(ctx, types.NamespacedName{
173+
Namespace: name.Namespace,
174+
Name: name.Name + "-cfnapi",
175+
}, heatCfnAPI)).To(Succeed())
176+
heatCfnAPI.Status.ObservedGeneration = heatCfnAPI.Generation
177+
heatCfnAPI.Status.ReadyCount = 1
178+
heatCfnAPI.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
179+
g.Expect(k8sClient.Status().Update(ctx, heatCfnAPI)).To(Succeed())
180+
181+
heatEngine := &heatv1.HeatEngine{}
182+
g.Expect(k8sClient.Get(ctx, types.NamespacedName{
183+
Namespace: name.Namespace,
184+
Name: name.Name + "-engine",
185+
}, heatEngine)).To(Succeed())
186+
heatEngine.Status.ObservedGeneration = heatEngine.Generation
187+
heatEngine.Status.ReadyCount = 1
188+
heatEngine.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
189+
g.Expect(k8sClient.Status().Update(ctx, heatEngine)).To(Succeed())
190+
}
191+
192+
func simulateHeatSubServicesReady(name types.NamespacedName) {
193+
Eventually(func(g Gomega) {
194+
for _, depName := range []string{"heat-api", "heat-cfnapi", "heat-engine"} {
195+
deployment := &appsv1.Deployment{}
196+
g.Expect(k8sClient.Get(ctx, types.NamespacedName{
197+
Namespace: name.Namespace,
198+
Name: depName,
199+
}, deployment)).To(Succeed())
200+
}
201+
}, timeout, interval).Should(Succeed())
202+
th.SimulateDeploymentReplicaReady(types.NamespacedName{
203+
Namespace: name.Namespace,
204+
Name: "heat-api",
205+
})
206+
th.SimulateDeploymentReplicaReady(types.NamespacedName{
207+
Namespace: name.Namespace,
208+
Name: "heat-cfnapi",
209+
})
210+
th.SimulateDeploymentReplicaReady(types.NamespacedName{
211+
Namespace: name.Namespace,
212+
Name: "heat-engine",
213+
})
214+
Eventually(func(g Gomega) {
215+
simulateHeatSubCRReady(g, name)
216+
}, timeout, interval).Should(Succeed())
217+
keystone.SimulateKeystoneServiceReady(types.NamespacedName{
218+
Namespace: name.Namespace,
219+
Name: heat.ServiceName,
220+
})
221+
keystone.SimulateKeystoneEndpointReady(types.NamespacedName{
222+
Namespace: name.Namespace,
223+
Name: heat.ServiceName,
224+
})
225+
keystone.SimulateKeystoneServiceReady(types.NamespacedName{
226+
Namespace: name.Namespace,
227+
Name: heat.CfnServiceName,
228+
})
229+
keystone.SimulateKeystoneEndpointReady(types.NamespacedName{
230+
Namespace: name.Namespace,
231+
Name: heat.CfnServiceName,
232+
})
233+
}
234+
158235
func GetTransportURL(name types.NamespacedName) *rabbitmqv1.TransportURL {
159236
instance := &rabbitmqv1.TransportURL{}
160237
Eventually(func(g Gomega) {

0 commit comments

Comments
 (0)