Skip to content

Commit a0ec96e

Browse files
stuggiclaude
stuggi
and
claude
committed
[b/r] Move custom Issuer labeling to ControlPlane controller, fix error handling
Move custom Issuer backup labeling from BackupConfig controller to ca.go where the ControlPlane controller already processes custom issuers. This is more explicit — labels what it knows about from spec.tls.*.ca.customIssuer rather than inferring from ownerRef absence. Remove Issuers from BackupConfig spec, status, conditions, RBAC, watches, and tests. Add error return to GetCertSecretBackupLabels and fix all callers to propagate errors for retry. Rename GetConfig parameter from gvk to crdName. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0598c0a commit a0ec96e

File tree

9 files changed

+18
-328
lines changed

9 files changed

+18
-328
lines changed

‎api/backup/v1beta1/conditions.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ const (
1515
// OpenStackBackupConfigNADsReadyCondition - NetworkAttachmentDefinitions labeling status
1616
OpenStackBackupConfigNADsReadyCondition condition.Type = "NADsReady"
1717

18-
// OpenStackBackupConfigIssuersReadyCondition - cert-manager Issuers labeling status
19-
OpenStackBackupConfigIssuersReadyCondition condition.Type = "IssuersReady"
20-
2118
// OpenStackBackupConfigCRsReadyCondition - CR instances labeling status
2219
OpenStackBackupConfigCRsReadyCondition condition.Type = "CRsReady"
2320
)

‎api/backup/v1beta1/openstackbackupconfig_types.go‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,6 @@ type OpenStackBackupConfigSpec struct {
5555
// +kubebuilder:default={labeling:enabled}
5656
NetworkAttachmentDefinitions ResourceBackupConfig `json:"networkAttachmentDefinitions"`
5757

58-
// Issuers configuration for backup labeling of cert-manager Issuers.
59-
// Only custom (user-provided) Issuers without ownerReferences are labeled.
60-
// Operator-created Issuers (rootca-*, selfsigned-issuer) have ownerRefs
61-
// and are recreated by the operator during reconciliation.
62-
// Custom Issuers default to restore order 20 (after secrets at order 10,
63-
// since Issuers reference CA secrets).
64-
// +kubebuilder:validation:Optional
65-
// +kubebuilder:default={labeling:enabled,restoreOrder:"20"}
66-
Issuers ResourceBackupConfig `json:"issuers"`
6758
}
6859

6960
// ResourceBackupConfig defines backup labeling rules for a resource type
@@ -118,9 +109,6 @@ type ResourceCounts struct {
118109
// +kubebuilder:validation:Optional
119110
NetworkAttachmentDefinitions int `json:"networkAttachmentDefinitions,omitempty"`
120111

121-
// Issuers is the number of cert-manager Issuers labeled for backup
122-
// +kubebuilder:validation:Optional
123-
Issuers int `json:"issuers,omitempty"`
124112
}
125113

126114
// +kubebuilder:object:root=true
@@ -129,7 +117,6 @@ type ResourceCounts struct {
129117
// +kubebuilder:printcolumn:name="Secrets",type="integer",JSONPath=".status.labeledResources.secrets",description="Labeled Secrets"
130118
// +kubebuilder:printcolumn:name="ConfigMaps",type="integer",JSONPath=".status.labeledResources.configMaps",description="Labeled ConfigMaps"
131119
// +kubebuilder:printcolumn:name="NADs",type="integer",JSONPath=".status.labeledResources.networkAttachmentDefinitions",description="Labeled NADs"
132-
// +kubebuilder:printcolumn:name="Custom Issuers",type="integer",JSONPath=".status.labeledResources.issuers",description="Labeled custom cert-manager Issuers (without ownerReferences)"
133120
// +kubebuilder:metadata:labels=backup.openstack.org/restore=true
134121
// +kubebuilder:metadata:labels=backup.openstack.org/category=controlplane
135122
// +kubebuilder:metadata:labels=backup.openstack.org/restore-order=20

‎api/backup/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/bases/backup.openstack.org_openstackbackupconfigs.yaml‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ spec:
3333
jsonPath: .status.labeledResources.networkAttachmentDefinitions
3434
name: NADs
3535
type: integer
36-
- description: Labeled custom cert-manager Issuers (without ownerReferences)
37-
jsonPath: .status.labeledResources.issuers
38-
name: Custom Issuers
39-
type: integer
4036
name: v1beta1
4137
schema:
4238
openAPIV3Schema:
@@ -113,52 +109,6 @@ spec:
113109
description: DefaultRestoreOrder is the restore order assigned to
114110
user-provided resources
115111
type: string
116-
issuers:
117-
default:
118-
labeling: enabled
119-
restoreOrder: "20"
120-
description: |-
121-
Issuers configuration for backup labeling of cert-manager Issuers.
122-
Only custom (user-provided) Issuers without ownerReferences are labeled.
123-
Operator-created Issuers (rootca-*, selfsigned-issuer) have ownerRefs
124-
and are recreated by the operator during reconciliation.
125-
Custom Issuers default to restore order 20 (after secrets at order 10,
126-
since Issuers reference CA secrets).
127-
properties:
128-
excludeLabelKeys:
129-
description: |-
130-
ExcludeLabelKeys is a list of label keys - resources with any of these labels are excluded
131-
Example: ["service-cert", "osdp-service"] excludes service-cert and dataplane service secrets
132-
items:
133-
type: string
134-
type: array
135-
excludeNames:
136-
description: |-
137-
ExcludeNames is a list of resource names to exclude from backup labeling
138-
Example: ["kube-root-ca.crt", "openshift-service-ca.crt"] for system ConfigMaps
139-
items:
140-
type: string
141-
type: array
142-
includeLabelSelector:
143-
additionalProperties:
144-
type: string
145-
description: |-
146-
IncludeLabelSelector allows filtering resources by label selector
147-
Only resources matching this selector will be labeled (in addition to ownerRef check)
148-
type: object
149-
labeling:
150-
description: Labeling controls whether to label this resource
151-
type for backup
152-
enum:
153-
- enabled
154-
- disabled
155-
type: string
156-
restoreOrder:
157-
description: |-
158-
RestoreOrder overrides the default restore order for this resource type.
159-
If empty, the global DefaultRestoreOrder is used.
160-
type: string
161-
type: object
162112
networkAttachmentDefinitions:
163113
default:
164114
labeling: enabled
@@ -294,10 +244,6 @@ spec:
294244
description: ConfigMaps is the number of configmaps labeled for
295245
backup
296246
type: integer
297-
issuers:
298-
description: Issuers is the number of cert-manager Issuers labeled
299-
for backup
300-
type: integer
301247
networkAttachmentDefinitions:
302248
description: NetworkAttachmentDefinitions is the number of NADs
303249
labeled for backup

‎config/crd/bases/backup.openstack.org_openstackbackupconfigs.yaml‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ spec:
3333
jsonPath: .status.labeledResources.networkAttachmentDefinitions
3434
name: NADs
3535
type: integer
36-
- description: Labeled custom cert-manager Issuers (without ownerReferences)
37-
jsonPath: .status.labeledResources.issuers
38-
name: Custom Issuers
39-
type: integer
4036
name: v1beta1
4137
schema:
4238
openAPIV3Schema:
@@ -113,52 +109,6 @@ spec:
113109
description: DefaultRestoreOrder is the restore order assigned to
114110
user-provided resources
115111
type: string
116-
issuers:
117-
default:
118-
labeling: enabled
119-
restoreOrder: "20"
120-
description: |-
121-
Issuers configuration for backup labeling of cert-manager Issuers.
122-
Only custom (user-provided) Issuers without ownerReferences are labeled.
123-
Operator-created Issuers (rootca-*, selfsigned-issuer) have ownerRefs
124-
and are recreated by the operator during reconciliation.
125-
Custom Issuers default to restore order 20 (after secrets at order 10,
126-
since Issuers reference CA secrets).
127-
properties:
128-
excludeLabelKeys:
129-
description: |-
130-
ExcludeLabelKeys is a list of label keys - resources with any of these labels are excluded
131-
Example: ["service-cert", "osdp-service"] excludes service-cert and dataplane service secrets
132-
items:
133-
type: string
134-
type: array
135-
excludeNames:
136-
description: |-
137-
ExcludeNames is a list of resource names to exclude from backup labeling
138-
Example: ["kube-root-ca.crt", "openshift-service-ca.crt"] for system ConfigMaps
139-
items:
140-
type: string
141-
type: array
142-
includeLabelSelector:
143-
additionalProperties:
144-
type: string
145-
description: |-
146-
IncludeLabelSelector allows filtering resources by label selector
147-
Only resources matching this selector will be labeled (in addition to ownerRef check)
148-
type: object
149-
labeling:
150-
description: Labeling controls whether to label this resource
151-
type for backup
152-
enum:
153-
- enabled
154-
- disabled
155-
type: string
156-
restoreOrder:
157-
description: |-
158-
RestoreOrder overrides the default restore order for this resource type.
159-
If empty, the global DefaultRestoreOrder is used.
160-
type: string
161-
type: object
162112
networkAttachmentDefinitions:
163113
default:
164114
labeling: enabled
@@ -294,10 +244,6 @@ spec:
294244
description: ConfigMaps is the number of configmaps labeled for
295245
backup
296246
type: integer
297-
issuers:
298-
description: Issuers is the number of cert-manager Issuers labeled
299-
for backup
300-
type: integer
301247
networkAttachmentDefinitions:
302248
description: NetworkAttachmentDefinitions is the number of NADs
303249
labeled for backup

‎internal/controller/backup/openstackbackupconfig_controller.go‎

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ import (
2525
"github.com/go-logr/logr"
2626
backupv1beta1 "github.com/openstack-k8s-operators/openstack-operator/api/backup/v1beta1"
2727

28-
certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
29-
3028
"github.com/openstack-k8s-operators/lib-common/modules/common/backup"
3129
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
3230
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
@@ -229,20 +227,6 @@ func (r *OpenStackBackupConfigReconciler) labelNetworkAttachmentDefinitions(ctx
229227
return r.labelResourceItems(ctx, log, items, instance.Spec.NetworkAttachmentDefinitions, defaultLabels)
230228
}
231229

232-
// labelIssuers labels cert-manager Issuers in the target namespace
233-
func (r *OpenStackBackupConfigReconciler) labelIssuers(ctx context.Context, log logr.Logger, instance *backupv1beta1.OpenStackBackupConfig) (int, error) {
234-
list := &certmgrv1.IssuerList{}
235-
if err := r.List(ctx, list, client.InNamespace(instance.Namespace)); err != nil {
236-
return 0, err
237-
}
238-
items := make([]client.Object, len(list.Items))
239-
for i := range list.Items {
240-
items[i] = &list.Items[i]
241-
}
242-
defaultLabels := backup.GetRestoreLabels(getRestoreOrder(instance.Spec.Issuers, instance.Spec.DefaultRestoreOrder), "")
243-
return r.labelResourceItems(ctx, log, items, instance.Spec.Issuers, defaultLabels)
244-
}
245-
246230
// labelCRInstances labels CR instances based on CRD backup-restore labels
247231
// This labels CRs like OpenStackControlPlane, OpenStackVersion, NetConfig, etc.
248232
// based on their CRD's backup/restore configuration.
@@ -307,8 +291,6 @@ func (r *OpenStackBackupConfigReconciler) labelCRInstances(ctx context.Context,
307291
// +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;update;patch
308292
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;update;patch
309293
// +kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch;update;patch
310-
// +kubebuilder:rbac:groups=cert-manager.io,resources=issuers,verbs=get;list;watch;update;patch
311-
// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch
312294
// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get;list;watch
313295
// RBAC for labeling CR instances across all openstack.org API groups.
314296
// Kubernetes RBAC does not support wildcard group patterns (*.openstack.org),
@@ -376,7 +358,6 @@ func (r *OpenStackBackupConfigReconciler) Reconcile(ctx context.Context, req ctr
376358
condition.UnknownCondition(backupv1beta1.OpenStackBackupConfigSecretsReadyCondition, condition.InitReason, condition.InitReason),
377359
condition.UnknownCondition(backupv1beta1.OpenStackBackupConfigConfigMapsReadyCondition, condition.InitReason, condition.InitReason),
378360
condition.UnknownCondition(backupv1beta1.OpenStackBackupConfigNADsReadyCondition, condition.InitReason, condition.InitReason),
379-
condition.UnknownCondition(backupv1beta1.OpenStackBackupConfigIssuersReadyCondition, condition.InitReason, condition.InitReason),
380361
condition.UnknownCondition(backupv1beta1.OpenStackBackupConfigCRsReadyCondition, condition.InitReason, condition.InitReason),
381362
)
382363
instance.Status.Conditions.Init(&cl)
@@ -454,21 +435,6 @@ func (r *OpenStackBackupConfigReconciler) Reconcile(ctx context.Context, req ctr
454435
"Labeled %d NADs", nadCount))
455436
}
456437

457-
issuerCount, err := r.labelIssuers(ctx, log, instance)
458-
if err != nil {
459-
log.Error(err, "Failed to label issuers")
460-
instance.Status.Conditions.Set(condition.FalseCondition(
461-
backupv1beta1.OpenStackBackupConfigIssuersReadyCondition,
462-
condition.ErrorReason,
463-
condition.SeverityWarning,
464-
"Failed to label issuers: %v", err))
465-
reconcileErrs = append(reconcileErrs, err)
466-
} else {
467-
instance.Status.Conditions.Set(condition.TrueCondition(
468-
backupv1beta1.OpenStackBackupConfigIssuersReadyCondition,
469-
"Labeled %d issuers", issuerCount))
470-
}
471-
472438
// Label CR instances based on CRD backup-restore labels
473439
crCount, err := r.labelCRInstances(ctx, log, instance)
474440
if err != nil {
@@ -489,13 +455,11 @@ func (r *OpenStackBackupConfigReconciler) Reconcile(ctx context.Context, req ctr
489455
instance.Status.LabeledResources.Secrets = secretCount
490456
instance.Status.LabeledResources.ConfigMaps = configMapCount
491457
instance.Status.LabeledResources.NetworkAttachmentDefinitions = nadCount
492-
instance.Status.LabeledResources.Issuers = issuerCount
493-
494458
if len(reconcileErrs) > 0 {
495459
return ctrl.Result{}, stderrors.Join(reconcileErrs...)
496460
}
497461

498-
log.Info("Successfully labeled resources", "secrets", secretCount, "configmaps", configMapCount, "nads", nadCount, "issuers", issuerCount, "crs", crCount)
462+
log.Info("Successfully labeled resources", "secrets", secretCount, "configmaps", configMapCount, "nads", nadCount, "crs", crCount)
499463
return ctrl.Result{}, nil
500464
}
501465

@@ -585,8 +549,7 @@ func (r *OpenStackBackupConfigReconciler) SetupWithManager(mgr ctrl.Manager) err
585549
For(&backupv1beta1.OpenStackBackupConfig{}).
586550
Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(findBackupConfigForSrc), builder.WithPredicates(backupResourcePredicate)).
587551
Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(findBackupConfigForSrc), builder.WithPredicates(backupResourcePredicate)).
588-
Watches(&k8s_networkingv1.NetworkAttachmentDefinition{}, handler.EnqueueRequestsFromMapFunc(findBackupConfigForSrc), builder.WithPredicates(backupResourcePredicate)).
589-
Watches(&certmgrv1.Issuer{}, handler.EnqueueRequestsFromMapFunc(findBackupConfigForSrc), builder.WithPredicates(backupResourcePredicate))
552+
Watches(&k8s_networkingv1.NetworkAttachmentDefinition{}, handler.EnqueueRequestsFromMapFunc(findBackupConfigForSrc), builder.WithPredicates(backupResourcePredicate))
590553

591554
// Build CRD label cache and add watches for CRD instance types.
592555
// Uses the API reader since the manager's cache is not started yet.

‎internal/openstack/backup.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,6 @@ func ReconcileBackupConfig(ctx context.Context, instance *corev1beta1.OpenStackC
8080
if backupConfig.Spec.NetworkAttachmentDefinitions.Labeling == nil {
8181
backupConfig.Spec.NetworkAttachmentDefinitions.Labeling = &defaultLabeling
8282
}
83-
if backupConfig.Spec.Issuers.Labeling == nil {
84-
backupConfig.Spec.Issuers.Labeling = &defaultLabeling
85-
}
86-
if backupConfig.Spec.Issuers.RestoreOrder == "" {
87-
backupConfig.Spec.Issuers.RestoreOrder = "20"
88-
}
89-
9083
return nil
9184
})
9285

‎internal/openstack/ca.go‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,10 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
134134
} else {
135135
customIssuer := *instance.Spec.TLS.Ingress.Ca.CustomIssuer
136136

137-
// add CA labelselector to issuer
138-
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace, issuerLabels, issuerAnnotations)
137+
// add CA labelselector and backup/restore labels to custom issuer
138+
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace,
139+
util.MergeMaps(issuerLabels, backup.GetRestoreLabels(backup.RestoreOrder20, backup.CategoryControlPlane)),
140+
issuerAnnotations)
139141
if err != nil {
140142
instance.Status.Conditions.Set(condition.FalseCondition(
141143
corev1.OpenStackControlPlaneCAReadyCondition,
@@ -211,8 +213,10 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
211213
}
212214
} else {
213215
customIssuer := *instance.Spec.TLS.PodLevel.Internal.Ca.CustomIssuer
214-
// add CA labelselector to issuer
215-
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace, issuerLabels, issuerAnnotations)
216+
// add CA labelselector and backup/restore labels to custom issuer
217+
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace,
218+
util.MergeMaps(issuerLabels, backup.GetRestoreLabels(backup.RestoreOrder20, backup.CategoryControlPlane)),
219+
issuerAnnotations)
216220
if err != nil {
217221
instance.Status.Conditions.Set(condition.FalseCondition(
218222
corev1.OpenStackControlPlaneCAReadyCondition,
@@ -289,8 +293,10 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
289293
}
290294
} else {
291295
customIssuer := *instance.Spec.TLS.PodLevel.Libvirt.Ca.CustomIssuer
292-
// add CA labelselector to issuer
293-
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace, issuerLabels, issuerAnnotations)
296+
// add CA labelselector and backup/restore labels to custom issuer
297+
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace,
298+
util.MergeMaps(issuerLabels, backup.GetRestoreLabels(backup.RestoreOrder20, backup.CategoryControlPlane)),
299+
issuerAnnotations)
294300
if err != nil {
295301
instance.Status.Conditions.Set(condition.FalseCondition(
296302
corev1.OpenStackControlPlaneCAReadyCondition,
@@ -366,8 +372,10 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h
366372
}
367373
} else {
368374
customIssuer := *instance.Spec.TLS.PodLevel.Ovn.Ca.CustomIssuer
369-
// add CA labelselector to issuer
370-
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace, issuerLabels, issuerAnnotations)
375+
// add CA labelselector and backup/restore labels to custom issuer
376+
caCertSecretName, err := addIssuerLabelAnnotation(ctx, helper, customIssuer, instance.Namespace,
377+
util.MergeMaps(issuerLabels, backup.GetRestoreLabels(backup.RestoreOrder20, backup.CategoryControlPlane)),
378+
issuerAnnotations)
371379
if err != nil {
372380
instance.Status.Conditions.Set(condition.FalseCondition(
373381
corev1.OpenStackControlPlaneCAReadyCondition,

0 commit comments

Comments
 (0)