[b/r] Label cert secrets at creation time, simplify backup controller

Move cert-manager secret labeling from the backup controller to the
controllers that create the certificates:

- CA cert secrets: labeled restore=true with order 10 via SecretTemplate
  in ca.go (controlplane controller)
- Leaf cert secrets: labeled restore=false in common.go (controlplane
  controller) — cert-manager regenerates these from restored CA
- EDPM node cert secrets: labeled restore=false in cert.go (dataplane
  controller)

This removes cert-manager introspection from the backup controller
(isCertManagerOperatorSecret, labelResourceRestoreFalse). The controller
now skips secrets that already have a restore label and only manages
user-provided resources without ownerRefs.

Also removes the excludeLabelKeys default for secrets (no longer needed)
and fixes operator-lint by removing omitempty from fields with defaults.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: stuggi

api/backup/v1beta1/openstackbackupconfig_types.go

@@ -29,24 +29,23 @@ type OpenStackBackupConfigSpec struct {
 	// DefaultRestoreOrder is the restore order assigned to user-provided resources
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default="10"
-	DefaultRestoreOrder string `json:"defaultRestoreOrder,omitempty"`
+	DefaultRestoreOrder string `json:"defaultRestoreOrder"`
 
 	// Secrets configuration for backup labeling
-	// Defaults: Excludes service-cert and osdp-service labeled secrets
 	// +kubebuilder:validation:Optional
-	// +kubebuilder:default={enabled:true,excludeLabelKeys:{"service-cert","osdp-service"}}
-	Secrets ResourceBackupConfig `json:"secrets,omitempty"`
+	// +kubebuilder:default={enabled:true}
+	Secrets ResourceBackupConfig `json:"secrets"`
 
 	// ConfigMaps configuration for backup labeling
 	// Defaults: Excludes kube-root-ca.crt and openshift-service-ca.crt
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default={enabled:true,excludeNames:{"kube-root-ca.crt","openshift-service-ca.crt"}}
-	ConfigMaps ResourceBackupConfig `json:"configMaps,omitempty"`
+	ConfigMaps ResourceBackupConfig `json:"configMaps"`
 
 	// NetworkAttachmentDefinitions configuration for backup labeling
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default={enabled:true}
-	NetworkAttachmentDefinitions ResourceBackupConfig `json:"networkAttachmentDefinitions,omitempty"`
+	NetworkAttachmentDefinitions ResourceBackupConfig `json:"networkAttachmentDefinitions"`
 
 	// Issuers configuration for backup labeling of cert-manager Issuers.
 	// Only custom (user-provided) Issuers without ownerReferences are labeled.
@@ -56,15 +55,15 @@ type OpenStackBackupConfigSpec struct {
 	// since Issuers reference CA secrets).
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default={enabled:true,restoreOrder:"20"}
-	Issuers ResourceBackupConfig `json:"issuers,omitempty"`
+	Issuers ResourceBackupConfig `json:"issuers"`
 }
 
 // ResourceBackupConfig defines backup labeling rules for a resource type
 type ResourceBackupConfig struct {
 	// Enabled controls whether to label this resource type for backup
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=true
-	Enabled bool `json:"enabled,omitempty"`
+	Enabled bool `json:"enabled"`
 
 	// RestoreOrder overrides the default restore order for this resource type.
 	// If empty, the global DefaultRestoreOrder is used.

api/bases/backup.openstack.org_openstackbackupconfigs.yaml

@@ -196,12 +196,7 @@ spec:
               secrets:
                 default:
                   enabled: true
-                  excludeLabelKeys:
-                  - service-cert
-                  - osdp-service
-                description: |-
-                  Secrets configuration for backup labeling
-                  Defaults: Excludes service-cert and osdp-service labeled secrets
+                description: Secrets configuration for backup labeling
                 properties:
                   enabled:
                     default: true

bindata/crds/crds.yaml

@@ -234,11 +234,6 @@ spec:
                       If empty, the global DefaultRestoreOrder is used.
                     type: string
                 type: object
-              targetNamespace:
-                default: openstack
-                description: TargetNamespace is the namespace to watch for resources
-                  to label
-                type: string
             type: object
           status:
             description: OpenStackBackupConfigStatus defines the observed state of

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

@@ -196,12 +196,7 @@ spec:
               secrets:
                 default:
                   enabled: true
-                  excludeLabelKeys:
-                  - service-cert
-                  - osdp-service
-                description: |-
-                  Secrets configuration for backup labeling
-                  Defaults: Excludes service-cert and osdp-service labeled secrets
+                description: Secrets configuration for backup labeling
                 properties:
                   enabled:
                     default: true

internal/controller/backup/openstackbackupconfig_controller.go

@@ -237,80 +237,10 @@ func (r *OpenStackBackupConfigReconciler) labelResource(ctx context.Context, log
 	return r.Update(ctx, obj)
 }
 
-// labelResourceRestoreFalse explicitly sets restore: "false" on a resource to
-// indicate it should not be restored. This makes the exclusion visible.
-// Also removes restore-order since it's meaningless when restore is disabled.
-func (r *OpenStackBackupConfigReconciler) labelResourceRestoreFalse(ctx context.Context, log logr.Logger, obj client.Object) error {
-	labels := obj.GetLabels()
-	if labels == nil {
-		labels = make(map[string]string)
-	}
-
-	// Check if already set correctly
-	_, hasOrder := labels[backup.BackupRestoreOrderLabel]
-	if labels[backup.BackupRestoreLabel] == "false" && !hasOrder {
-		return nil
-	}
-
-	labels[backup.BackupRestoreLabel] = "false"
-	delete(labels, backup.BackupRestoreOrderLabel)
-	obj.SetLabels(labels)
-
-	log.Info("Labeled resource restore=false", "name", obj.GetName())
-	return r.Update(ctx, obj)
-}
-
-// isCertManagerOperatorSecret checks if a secret is managed by cert-manager for an
-// operator-created (non-CA) Certificate. Such secrets do not need the restore label
-// because cert-manager will regenerate them from the restored CA Issuer and Certificate CRs.
-// The secrets are still included in the namespace-wide backup (no label needed for that).
-//
-// Returns true (skip restore label) for:
-//   - Operator-created non-CA cert secrets (Certificate CR has ownerRef, isCA != true)
-//
-// Returns false (set restore label) for:
-//   - Non-cert-manager secrets
-//   - CA certificate secrets (spec.isCA: true) — must be restored to preserve CA identity
-//   - Secrets from user-created Certificates (no ownerRef on Certificate CR)
-func (r *OpenStackBackupConfigReconciler) isCertManagerOperatorSecret(ctx context.Context, log logr.Logger, secret *corev1.Secret) bool {
-	certName, hasCertAnnotation := secret.Annotations["cert-manager.io/certificate-name"]
-	if !hasCertAnnotation {
-		return false
-	}
-
-	// Look up the Certificate CR
-	cert := &certmgrv1.Certificate{}
-	if err := r.Get(ctx, types.NamespacedName{
-		Name:      certName,
-		Namespace: secret.Namespace,
-	}, cert); err != nil {
-		// Certificate not found — could be deleted, treat secret as user-provided
-		log.V(1).Info("Certificate CR not found for secret, treating as user-provided",
-			"secret", secret.Name, "certificate", certName)
-		return false
-	}
-
-	// CA certificates must be restored to preserve the CA identity
-	if cert.Spec.IsCA {
-		log.V(1).Info("Secret is for CA certificate, will be backed up",
-			"secret", secret.Name, "certificate", certName)
-		return false
-	}
-
-	// If the Certificate CR has no ownerRef, it's user-created — restore the secret
-	if len(cert.GetOwnerReferences()) == 0 {
-		return false
-	}
-
-	// Operator-created, non-CA certificate — cert-manager will regenerate
-	log.V(1).Info("Skipping operator-managed cert secret (will be regenerated)",
-		"secret", secret.Name, "certificate", certName)
-	return true
-}
-
 // labelSecrets labels secrets in the target namespace.
-// Secrets managed by cert-manager for operator-created non-CA Certificates get
-// restore: "false" — cert-manager will regenerate them from the restored CA Issuer.
+// Secrets that already have a restore label (set by the controlplane controller
+// for cert-manager secrets) are skipped — the operator knows best whether a cert
+// secret should be restored or regenerated.
 // User annotation overrides take precedence over all default behavior.
 func (r *OpenStackBackupConfigReconciler) labelSecrets(ctx context.Context, log logr.Logger, instance *backupv1beta1.OpenStackBackupConfig) (int, error) {
 	secretList := &corev1.SecretList{}
@@ -335,16 +265,13 @@ func (r *OpenStackBackupConfigReconciler) labelSecrets(ctx context.Context, log
 			continue
 		}
 
-		if !shouldLabelResource(secret, instance.Spec.Secrets) {
+		// Skip secrets that already have a restore label (set by the operator
+		// at creation time, e.g. cert-manager CA/leaf cert secrets)
+		if _, hasRestoreLabel := secret.Labels[backup.BackupRestoreLabel]; hasRestoreLabel {
 			continue
 		}
 
-		// Operator-managed non-CA cert secrets: explicitly set restore=false
-		if r.isCertManagerOperatorSecret(ctx, log, secret) {
-			if err := r.labelResourceRestoreFalse(ctx, log, secret); err != nil {
-				log.Error(err, "Failed to label cert secret restore=false", "name", secret.Name)
-				errs = append(errs, fmt.Errorf("secret %s: %w", secret.Name, err))
-			}
+		if !shouldLabelResource(secret, instance.Spec.Secrets) {
 			continue
 		}
 

internal/dataplane/cert.go

@@ -37,6 +37,7 @@ import (
 	certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
 	"github.com/openstack-k8s-operators/lib-common/modules/certmanager"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/backup"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/secret"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
@@ -114,10 +115,11 @@ func EnsureTLSCerts(ctx context.Context, helper *helper.Helper,
 		// For now we just add the hostname so we can select all the certs on one node
 		hostName := node.HostName
 		labels := map[string]string{
-			HostnameLabel:   hostName,
-			ServiceLabel:    service.Name,
-			ServiceKeyLabel: certKey,
-			NodeSetLabel:    instance.Name,
+			HostnameLabel:             hostName,
+			ServiceLabel:              service.Name,
+			ServiceKeyLabel:           certKey,
+			NodeSetLabel:              instance.Name,
+			backup.BackupRestoreLabel: "false",
 		}
 		certName = service.Name + "-" + certKey + "-" + hostName
 

internal/openstack/ca.go

@@ -17,6 +17,7 @@ import (
 	certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	certmgrmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	"github.com/openstack-k8s-operators/lib-common/modules/certmanager"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/backup"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/secret"
@@ -654,9 +655,11 @@ func createRootCACertAndIssuer(
 			Duration:    caCfg.Duration,
 			RenewBefore: caCfg.RenewBefore,
 			SecretTemplate: &certmgrv1.CertificateSecretTemplate{
-				Labels: map[string]string{
-					caCertSelector: "",
-				},
+				Labels: util.MergeMaps(
+					map[string]string{caCertSelector: ""},
+					backup.GetBackupLabels(backup.CategoryControlPlane),
+					backup.GetRestoreLabels(backup.RestoreOrder10, backup.CategoryControlPlane),
+				),
 			},
 		})
 	cert := certmanager.NewCertificate(caCertReq, 5)

internal/openstack/common.go

@@ -21,6 +21,7 @@ import (
 	ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
 	keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/lib-common/modules/certmanager"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/backup"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/clusterdns"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
@@ -331,7 +332,7 @@ func EnsureEndpointConfig(
 						},
 						Ips:         nil,
 						Annotations: ed.Annotations,
-						Labels:      util.MergeMaps(ed.Labels, map[string]string{serviceCertSelector: ""}),
+						Labels:      util.MergeMaps(ed.Labels, map[string]string{serviceCertSelector: "", backup.BackupRestoreLabel: "false"}),
 						Usages:      nil,
 					}
 
@@ -381,7 +382,7 @@ func EnsureEndpointConfig(
 					},
 					Ips:         nil,
 					Annotations: ed.Annotations,
-					Labels:      util.MergeMaps(ed.Labels, map[string]string{serviceCertSelector: ""}),
+					Labels:      util.MergeMaps(ed.Labels, map[string]string{serviceCertSelector: "", backup.BackupRestoreLabel: "false"}),
 					Usages:      nil,
 				}
 
@@ -638,7 +639,7 @@ func (ed *EndpointDetail) CreateRoute(
 				Hostnames:   []string{*ed.Hostname},
 				Ips:         nil,
 				Annotations: ed.Annotations,
-				Labels:      util.MergeMaps(ed.Labels, map[string]string{serviceCertSelector: ""}),
+				Labels:      util.MergeMaps(ed.Labels, map[string]string{serviceCertSelector: "", backup.BackupRestoreLabel: "false"}),
 				Usages:      nil,
 			}
 			if instance.Spec.TLS.Ingress.Cert.Duration != nil {