Skip to content

Commit c0c83da

Browse files
stuggiclaude
andcommitted
[lb pub endpt] Reconcile when a bring-your-own service cert secret changes
When a user updates the content of a secret referenced by a service template's bring-your-own pod level cert - tls.api.{public,internal} .secretName, or the simple TLS sections of horizon, novncproxy, prometheus and alertmanager - the control plane was not reconciled, so a missing or invalid secret was only noticed through the requeue. Index those secret names of all services so the existing Secret watch enqueues the control plane, like apiOverride.tls.secretName. The tls.api names are collected by walking the spec for tls.APIService, so new services are covered without a list to maintain; the simple TLS sections are read explicitly. Operator owned secrets (ovn, vencrypt, metadata) are not indexed. The indexed spec comes from the informer cache, i.e. the persisted user input, never the operator-issued names written back in memory. Add unit tests for the secret name collection and for findObjectsForSrc. Jira: OSPRH-39364 Jira: OSPRH-39366 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
1 parent a434d27 commit c0c83da

2 files changed

Lines changed: 208 additions & 0 deletions

File tree

‎internal/controller/core/openstackcontrolplane_controller.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"context"
2222
"fmt"
2323
"maps"
24+
"reflect"
2425
"slices"
2526
"strings"
2627
"time"
@@ -41,6 +42,7 @@ import (
4142
"github.com/openstack-k8s-operators/lib-common/modules/common/backup"
4243
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
4344
common_helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
45+
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
4446
"github.com/openstack-k8s-operators/lib-common/modules/common/webhook"
4547
corev1 "k8s.io/api/core/v1"
4648

@@ -804,6 +806,7 @@ const (
804806
tlsPodLevelInternalCACustomIssuer = ".spec.tls.podLevel.internal.ca.customIssuer"
805807
tlsPodLevelOvnCACustomIssuer = ".spec.tls.podLevel.ovn.ca.customIssuer"
806808
apiOverrideTLSSecretField = ".spec.apiOverride.tls.secretName"
809+
tlsServiceSecretField = ".spec.template.tls.serviceSecretName"
807810
)
808811

809812
// fieldIndexers maps each indexed field to the func extracting its value(s)
@@ -847,6 +850,8 @@ var fieldIndexers = map[string]client.IndexerFunc{
847850
},
848851
// the custom public endpoint cert secrets of all services
849852
apiOverrideTLSSecretField: indexAPIOverrideTLSSecret,
853+
// the bring-your-own pod level service cert secrets of all services
854+
tlsServiceSecretField: indexTLSServiceSecret,
850855
}
851856

852857
// SetupWithManager sets up the controller with the Manager.
@@ -966,6 +971,83 @@ func apiOverrideTLSSecretNames(spec *corev1beta1.OpenStackControlPlaneSpec) []st
966971
return names
967972
}
968973

974+
// indexTLSServiceSecret is the field index func for tlsServiceSecretField
975+
func indexTLSServiceSecret(rawObj client.Object) []string {
976+
return tlsServiceSecretNames(&rawObj.(*corev1beta1.OpenStackControlPlane).Spec)
977+
}
978+
979+
// tlsServiceSecretNames returns the bring-your-own pod level service cert secret
980+
// names, i.e. the user input the operator reads in EnsureEndpointConfig:
981+
// tls.api.{public,internal} of all services (found by walking the spec, so new
982+
// services are covered) and the simple TLS sections of horizon, novncproxy,
983+
// prometheus and alertmanager. Operator owned secrets (ovn, vencrypt, metadata, ...)
984+
// are deliberately not indexed. The indexed spec comes from the informer cache,
985+
// i.e. the persisted user input - never the operator-issued names written back in
986+
// memory.
987+
func tlsServiceSecretNames(spec *corev1beta1.OpenStackControlPlaneSpec) []string {
988+
names := tlsAPISecretNames(reflect.ValueOf(spec))
989+
990+
secrets := []*string{}
991+
if t := spec.Horizon.Template; t != nil {
992+
secrets = append(secrets, t.TLS.SecretName)
993+
}
994+
if t := spec.Nova.Template; t != nil {
995+
for _, cell := range t.CellTemplates {
996+
secrets = append(secrets, cell.NoVNCProxyServiceTemplate.TLS.Service.SecretName)
997+
}
998+
}
999+
if t := spec.Telemetry.Template; t != nil {
1000+
secrets = append(secrets, t.MetricStorage.PrometheusTLS.SecretName, t.MetricStorage.AlertmanagerTLS.SecretName)
1001+
}
1002+
for _, sn := range secrets {
1003+
if sn != nil && *sn != "" {
1004+
names = append(names, *sn)
1005+
}
1006+
}
1007+
return names
1008+
}
1009+
1010+
var tlsAPIServiceType = reflect.TypeFor[tls.APIService]()
1011+
1012+
// tlsAPISecretNames walks v for tls.APIService and returns the secret names of
1013+
// its public and internal endpoint.
1014+
func tlsAPISecretNames(v reflect.Value) []string {
1015+
var names []string
1016+
switch v.Kind() {
1017+
case reflect.Pointer:
1018+
if !v.IsNil() {
1019+
names = tlsAPISecretNames(v.Elem())
1020+
}
1021+
case reflect.Struct:
1022+
if v.Type() == tlsAPIServiceType {
1023+
api := v.Interface().(tls.APIService)
1024+
for _, sn := range []*string{api.Public.SecretName, api.Internal.SecretName} {
1025+
if sn != nil && *sn != "" {
1026+
names = append(names, *sn)
1027+
}
1028+
}
1029+
break
1030+
}
1031+
for i := range v.NumField() {
1032+
if v.Type().Field(i).IsExported() {
1033+
names = append(names, tlsAPISecretNames(v.Field(i))...)
1034+
}
1035+
}
1036+
case reflect.Slice, reflect.Array:
1037+
// skip slices of scalars (e.g. []byte)
1038+
if k := v.Type().Elem().Kind(); k == reflect.Struct || k == reflect.Pointer || k == reflect.Map {
1039+
for i := range v.Len() {
1040+
names = append(names, tlsAPISecretNames(v.Index(i))...)
1041+
}
1042+
}
1043+
case reflect.Map:
1044+
for _, k := range v.MapKeys() {
1045+
names = append(names, tlsAPISecretNames(v.MapIndex(k))...)
1046+
}
1047+
}
1048+
return names
1049+
}
1050+
9691051
func (r *OpenStackControlPlaneReconciler) findObjectsForSrc(ctx context.Context, src client.Object) []reconcile.Request {
9701052
requests := []reconcile.Request{}
9711053

‎internal/controller/core/openstackcontrolplane_controller_test.go‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,16 @@ import (
77
"slices"
88
"testing"
99

10+
horizonv1 "github.com/openstack-k8s-operators/horizon-operator/api/v1beta1"
11+
keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
12+
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
13+
neutronv1 "github.com/openstack-k8s-operators/neutron-operator/api/v1beta1"
14+
novav1 "github.com/openstack-k8s-operators/nova-operator/api/nova/v1beta1"
15+
telemetryv1 "github.com/openstack-k8s-operators/telemetry-operator/api/v1beta1"
1016
corev1 "k8s.io/api/core/v1"
1117
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1218
"k8s.io/apimachinery/pkg/runtime"
19+
"k8s.io/utils/ptr"
1320
"sigs.k8s.io/controller-runtime/pkg/client"
1421
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1522

@@ -94,6 +101,125 @@ func TestAPIOverrideTLSSecretNames(t *testing.T) {
94101
}
95102
}
96103

104+
// setTLSAPISecrets walks v and sets a unique secretName on both endpoints of
105+
// every tls.APIService found, collecting the names.
106+
func setTLSAPISecrets(v reflect.Value, names *[]string) {
107+
switch v.Kind() {
108+
case reflect.Struct:
109+
if v.Type() == tlsAPIServiceType {
110+
api := v.Addr().Interface().(*tls.APIService)
111+
for _, svc := range []*tls.GenericService{&api.Public, &api.Internal} {
112+
name := fmt.Sprintf("tls-secret-%d", len(*names))
113+
svc.SecretName = &name
114+
*names = append(*names, name)
115+
}
116+
return
117+
}
118+
for i := range v.NumField() {
119+
if v.Type().Field(i).IsExported() {
120+
setTLSAPISecrets(v.Field(i), names)
121+
}
122+
}
123+
case reflect.Pointer:
124+
if v.IsNil() && v.Type().Elem().Kind() == reflect.Struct {
125+
v.Set(reflect.New(v.Type().Elem()))
126+
}
127+
if !v.IsNil() {
128+
setTLSAPISecrets(v.Elem(), names)
129+
}
130+
case reflect.Map:
131+
if v.Type().Elem().Kind() == reflect.Struct {
132+
v.Set(reflect.MakeMap(v.Type()))
133+
elem := reflect.New(v.Type().Elem()).Elem()
134+
setTLSAPISecrets(elem, names)
135+
v.SetMapIndex(reflect.ValueOf("key").Convert(v.Type().Key()), elem)
136+
}
137+
}
138+
}
139+
140+
// TestTLSServiceSecretNames verifies the tls.api secrets of every service in the
141+
// spec (incl. map values like glance) are collected.
142+
func TestTLSServiceSecretNames(t *testing.T) {
143+
spec := &corev1beta1.OpenStackControlPlaneSpec{}
144+
if got := tlsServiceSecretNames(spec); len(got) != 0 {
145+
t.Fatalf("expected no secrets for empty spec, got %v", got)
146+
}
147+
148+
var want []string
149+
setTLSAPISecrets(reflect.ValueOf(spec).Elem(), &want)
150+
if len(want) == 0 {
151+
t.Fatal("found no tls.api fields in the spec")
152+
}
153+
got := tlsServiceSecretNames(spec)
154+
slices.Sort(got)
155+
slices.Sort(want)
156+
if !slices.Equal(got, want) {
157+
t.Errorf("tls.api secret names not collected\nwant: %v\ngot: %v", want, got)
158+
}
159+
160+
// empty secretName is ignored
161+
spec = &corev1beta1.OpenStackControlPlaneSpec{}
162+
spec.Keystone.Template = &keystonev1.KeystoneAPISpecCore{}
163+
spec.Keystone.Template.TLS.API.Public.SecretName = ptr.To("")
164+
if got := tlsServiceSecretNames(spec); len(got) != 0 {
165+
t.Errorf("expected empty secretName to be ignored, got %v", got)
166+
}
167+
}
168+
169+
// TestTLSServiceSecretNamesSimpleTLS verifies the simple TLS sections the operator
170+
// reads as bring-your-own are indexed, and the operator owned ones are not.
171+
func TestTLSServiceSecretNamesSimpleTLS(t *testing.T) {
172+
spec := &corev1beta1.OpenStackControlPlaneSpec{}
173+
spec.Horizon.Template = &horizonv1.HorizonSpecCore{}
174+
spec.Horizon.Template.TLS.SecretName = ptr.To("horizon-cert")
175+
spec.Nova.Template = &novav1.NovaSpecCore{CellTemplates: map[string]novav1.NovaCellTemplate{"cell1": {}}}
176+
cell := spec.Nova.Template.CellTemplates["cell1"]
177+
cell.NoVNCProxyServiceTemplate.TLS.Service.SecretName = ptr.To("novnc-cert")
178+
cell.NoVNCProxyServiceTemplate.TLS.Vencrypt.SecretName = ptr.To("vencrypt-operator-owned")
179+
spec.Nova.Template.CellTemplates["cell1"] = cell
180+
spec.Telemetry.Template = &telemetryv1.TelemetrySpecCore{}
181+
spec.Telemetry.Template.MetricStorage.PrometheusTLS.SecretName = ptr.To("prometheus-cert")
182+
spec.Telemetry.Template.MetricStorage.AlertmanagerTLS.SecretName = ptr.To("alertmanager-cert")
183+
spec.Neutron.Template = &neutronv1.NeutronAPISpecCore{}
184+
spec.Neutron.Template.TLS.Ovn.SecretName = ptr.To("ovn-operator-owned")
185+
186+
got := tlsServiceSecretNames(spec)
187+
slices.Sort(got)
188+
want := []string{"alertmanager-cert", "horizon-cert", "novnc-cert", "prometheus-cert"}
189+
if !slices.Equal(got, want) {
190+
t.Errorf("want %v, got %v", want, got)
191+
}
192+
}
193+
194+
// TestFindObjectsForSrcTLSServiceSecret verifies a change of a BYO service cert
195+
// secret enqueues the control plane referencing it.
196+
func TestFindObjectsForSrcTLSServiceSecret(t *testing.T) {
197+
scheme := runtime.NewScheme()
198+
if err := corev1beta1.AddToScheme(scheme); err != nil {
199+
t.Fatal(err)
200+
}
201+
202+
cp := &corev1beta1.OpenStackControlPlane{ObjectMeta: metav1.ObjectMeta{Name: "cp", Namespace: "ns"}}
203+
cp.Spec.Keystone.Template = &keystonev1.KeystoneAPISpecCore{}
204+
cp.Spec.Keystone.Template.TLS.API.Internal.SecretName = ptr.To("byo-cert")
205+
206+
builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cp)
207+
for field, indexer := range fieldIndexers {
208+
builder = builder.WithIndex(&corev1beta1.OpenStackControlPlane{}, field, indexer)
209+
}
210+
r := &OpenStackControlPlaneReconciler{Client: builder.Build()}
211+
212+
secret := func(name string) client.Object {
213+
return &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns"}}
214+
}
215+
if got := r.findObjectsForSrc(context.Background(), secret("byo-cert")); len(got) != 1 || got[0].Name != "cp" {
216+
t.Errorf("expected control plane to be enqueued, got %v", got)
217+
}
218+
if got := r.findObjectsForSrc(context.Background(), secret("other")); len(got) != 0 {
219+
t.Errorf("expected no requests for unrelated secret, got %v", got)
220+
}
221+
}
222+
97223
// TestFindObjectsForSrcAPIOverrideTLSSecret verifies a change of a custom
98224
// route cert secret enqueues the control plane referencing it.
99225
func TestFindObjectsForSrcAPIOverrideTLSSecret(t *testing.T) {

0 commit comments

Comments
 (0)