From 64f9680f8ae4b76466ce6cdd9ea29a5dd6775fb6 Mon Sep 17 00:00:00 2001 From: Omer Date: Tue, 20 Oct 2026 16:25:46 +0200 Subject: [PATCH 1/3] Use real per-pool TSIG keys in shared mode Leave the default pool unsigned outside AZ-aware mode and register each non-default pool key with scope POOL and its actual pool UUID. Read and watch the key ID mapping in both modes when generating pools.yaml. Preserve existing key material when repairing pool bindings and retain the legacy shared DB key during migration. Wait for BIND to load the new Secrets before publishing key IDs, and reconcile Secrets while StatefulSets are pending to avoid a bootstrap dependency cycle. Keep nameserver TSIG IDs limited to AZ-aware mode for compatibility with legacy Designate YAML adapters. Refresh completed pool discovery Jobs when required pools are missing, read only the current Job's pod logs, and exclude unregistered dry-run pools without real UUIDs. Make the merged named.conf copy owner-writable so non-root BIND init can append the TSIG include after copying read-only Secret files. Add unit and functional coverage for default pool behavior, per-pool identities, migration readiness, key binding repair, legacy YAML compatibility, discovery refresh and init file permissions. Update multipool KUTTL assertions for per-pool Secret creation and cleanup. Signed-off-by: Omer --- internal/controller/designate_controller.go | 48 +- .../designatebackendbind9_multipool.go | 11 +- .../controller/designatebackendbind9_tsig.go | 413 +++++++----------- ...signatebackendbind9_tsig_migration_test.go | 184 ++++++++ .../designatebackendbind9_tsig_test.go | 221 ++++++++++ .../designate/generate_bind9_pools_yaml.go | 9 +- .../generate_bind9_pools_yaml_test.go | 50 +++ internal/designate/pools.go | 33 +- internal/designate/pools_test.go | 125 ++++++ internal/designate/tsigkeys.go | 55 ++- internal/designate/tsigkeys_test.go | 110 +++++ internal/designatebackendbind9/init_test.go | 93 ++++ templates/designatebackendbind9/bin/init.sh | 2 + test/functional/designate_controller_test.go | 97 ++++ test/functional/designate_multipool_test.go | 4 +- .../tests/designate_multipool/01-assert.yaml | 28 +- .../tests/designate_multipool/04-assert.yaml | 10 + .../tests/designate_multipool/05-assert.yaml | 14 +- .../tests/designate_multipool/06-assert.yaml | 4 + 19 files changed, 1215 insertions(+), 296 deletions(-) create mode 100644 internal/controller/designatebackendbind9_tsig_migration_test.go create mode 100644 internal/controller/designatebackendbind9_tsig_test.go create mode 100644 internal/designate/tsigkeys_test.go create mode 100644 internal/designatebackendbind9/init_test.go diff --git a/internal/controller/designate_controller.go b/internal/controller/designate_controller.go index d3dc4fa5..672e10cf 100644 --- a/internal/controller/designate_controller.go +++ b/internal/controller/designate_controller.go @@ -504,7 +504,7 @@ func (r *DesignateReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man } for _, cr := range designates.Items { - if o.GetName() == cr.Spec.ExternalBindsSecret { + if o.GetName() == cr.Spec.ExternalBindsSecret || o.GetName() == cr.Name+"-backendbind9"+designate.TsigSecretSuffix { name := client.ObjectKey{ Namespace: o.GetNamespace(), Name: cr.Name, @@ -1388,6 +1388,7 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des return ctrl.Result{}, err } + tsigKeysPending := false if len(nsRecords) > 0 && instance.Status.DesignateCentralReadyCount > 0 { Log.Info("NS records data found") poolsYamlConfigMap := &corev1.ConfigMap{ @@ -1399,9 +1400,9 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des Data: make(map[string]string), } - // Read per-pool TSIG key ID mapping if AZ-aware mode is enabled + // Read per-pool TSIG key IDs in both shared and AZ-aware modes. var tsigKeyIDs map[string]string - if instance.Spec.AZAwareMode == designatev1beta1.AZModeEnabled && multipoolConfig != nil { + if multipoolConfig != nil { tsigSecretName := fmt.Sprintf("%s-backendbind9%s", instance.Name, designate.TsigSecretSuffix) tsigSecret := &corev1.Secret{} err := helper.GetClient().Get(ctx, types.NamespacedName{Name: tsigSecretName, Namespace: instance.GetNamespace()}, tsigSecret) @@ -1409,15 +1410,45 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des return ctrl.Result{}, fmt.Errorf("failed to get TSIG secret %s: %w", tsigSecretName, err) } if err == nil { - if data, exists := tsigSecret.Data[designate.TSIGKeyIDsDataKey]; exists { + if tsigSecret.Annotations["tsig-keys-ready"] == "false" { + // Reuse the applied identities until BIND has loaded the keys. + // Continue reconciling service specs and registering new pools: + // either can be needed for the pending rollout to finish. + tsigKeysPending = true + appliedPools := &corev1.ConfigMap{} + if err := helper.GetClient().Get(ctx, types.NamespacedName{Name: designate.PoolsYamlConfigMap, Namespace: instance.Namespace}, appliedPools); err != nil { + if !k8s_errors.IsNotFound(err) { + return ctrl.Result{}, err + } + } else { + var pools []designate.Pool + if err := yaml.Unmarshal([]byte(appliedPools.Data[designate.PoolsYamlContent]), &pools); err != nil { + return ctrl.Result{}, fmt.Errorf("invalid applied pools.yaml: %w", err) + } + tsigKeyIDs = make(map[string]string) + for _, pool := range pools { + for _, target := range pool.Targets { + if target.TSIGKeyID != "" { + tsigKeyIDs[pool.Name] = target.TSIGKeyID + break + } + } + } + } + } else if data, exists := tsigSecret.Data[designate.TSIGKeyIDsDataKey]; exists { if jsonErr := json.Unmarshal(data, &tsigKeyIDs); jsonErr != nil { - Log.Info(fmt.Sprintf("Failed to parse TSIG key ID mapping from secret, pools.yaml will be generated without tsigkey_ids: %v", jsonErr)) + return ctrl.Result{}, fmt.Errorf("invalid TSIG key ID mapping: %w", jsonErr) } } } // If secret doesn't exist yet, tsigKeyIDs stays nil — pools.yaml is generated without tsigkey_ids (first pass) } + if instance.Spec.AZAwareMode != designatev1beta1.AZModeEnabled { + // Also strip a stale default binding during migration from AZ mode. + delete(tsigKeyIDs, designate.DefaultPoolName) + } + poolsYaml, poolsYamlHash, err := designate.GeneratePoolsYamlDataAndHash(updatedBindMap, mdnsConfigMap.Data, nsRecords, multipoolConfig, externalBindData, externalMasterServiceIPs, tsigKeyIDs) if err != nil { return ctrl.Result{}, err @@ -1656,7 +1687,7 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des // We reached the end of the Reconcile, update the Ready condition based on // the sub conditions - if instance.Status.Conditions.AllSubConditionIsTrue() { + if !tsigKeysPending && instance.Status.Conditions.AllSubConditionIsTrue() { instance.Status.Conditions.MarkTrue( condition.ReadyCondition, condition.ReadyMessage) } @@ -1667,7 +1698,7 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des // covers sub-CRs that don't participate in the input-hash correlation // (unbound, backendbind9), so the finalizer is held until the whole // deployment has stabilized on the new credentials. - guardReady := allServicesReady && instance.Status.Conditions.AllSubConditionIsTrue() + guardReady := !tsigKeysPending && allServicesReady && instance.Status.Conditions.AllSubConditionIsTrue() // Finalize transport URL rotation transportSecretName, err := object.FinalizeSecretRotation( @@ -1728,6 +1759,9 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des return ctrl.Result{}, err } + if tsigKeysPending { + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + } Log.Info("Reconciled Service successfully") return ctrl.Result{}, nil } diff --git a/internal/controller/designatebackendbind9_multipool.go b/internal/controller/designatebackendbind9_multipool.go index ac15207f..e088e2fe 100644 --- a/internal/controller/designatebackendbind9_multipool.go +++ b/internal/controller/designatebackendbind9_multipool.go @@ -767,12 +767,13 @@ func (r *DesignateBackendbind9Reconciler) reconcileMultipoolResources( return ctrlResult, err } - ctrlResult, err = r.reconcileMultipoolStatefulSets(ctx, instance, helper, multipoolConfig, inputHash, serviceLabels, serviceAnnotations, topology) - if err != nil || (ctrlResult != ctrl.Result{}) { - return ctrlResult, err + statefulSetResult, err := r.reconcileMultipoolStatefulSets(ctx, instance, helper, multipoolConfig, inputHash, serviceLabels, serviceAnnotations, topology) + if err != nil { + return statefulSetResult, err } - // Create/update TSIG secrets for non-default pools + // Reconcile keys even while StatefulSets are pending: new BIND pods can be + // waiting for these Secrets to exist before their init containers can run. ctrlResult, err = r.reconcileTSIGSecrets(ctx, instance, helper, multipoolConfig) if err != nil || (ctrlResult != ctrl.Result{}) { return ctrlResult, err @@ -780,7 +781,7 @@ func (r *DesignateBackendbind9Reconciler) reconcileMultipoolResources( // Note: Per-pool bind IP ConfigMaps are cleaned up by DesignateReconciler.reconcileBindConfigMaps() - return ctrl.Result{}, nil + return statefulSetResult, nil } // cleanupMultipoolServices deletes pool-specific services during migration to single-pool mode diff --git a/internal/controller/designatebackendbind9_tsig.go b/internal/controller/designatebackendbind9_tsig.go index 12eb57a3..3f7b31fe 100644 --- a/internal/controller/designatebackendbind9_tsig.go +++ b/internal/controller/designatebackendbind9_tsig.go @@ -18,8 +18,8 @@ limitations under the License. // This file owns the controller-level TSIG lifecycle: creating/rotating the // Designate TSIG keys (via internal/designate/tsigkeys.go's Designate API // client) and reconciling the Kubernetes Secrets that carry the resulting -// key material into BIND, in both the shared-key (non-AZ-aware) and -// per-pool (AZ-aware) models. Everything else about multipool orchestration +// key material into BIND using one key per registered pool. +// Everything else about multipool orchestration // (Services, StatefulSets, migration between single-pool and multipool // modes) lives in designatebackendbind9_multipool.go. @@ -38,6 +38,7 @@ import ( "strings" "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -52,30 +53,21 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/util" ) -// reconcileTSIGSecrets manages TSIG secrets for multipool mode. -// When AZAwareMode=Enabled: creates per-pool TSIG keys (one per pool including default). -// Otherwise: uses the shared TSIG key model (one key for all non-default pools). +// reconcileTSIGSecrets manages per-pool TSIG secrets for multipool mode. +// Designate validates each POOL-scoped key against a registered pool and uses +// that pool ID to authorize mDNS transfers, including in non-AZ deployments. func (r *DesignateBackendbind9Reconciler) reconcileTSIGSecrets( ctx context.Context, instance *designatev1beta1.DesignateBackendbind9, helper *helper.Helper, multipoolConfig *designate.MultipoolConfig, ) (ctrl.Result, error) { - // Determine AZ-aware mode from parent Designate CR - designateInstance, err := r.getDesignateCR(ctx, helper, instance) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get parent Designate CR for AZ mode check: %w", err) - } - - if designateInstance.Spec.AZAwareMode == designatev1beta1.AZModeEnabled { - return r.reconcilePerPoolTSIGSecrets(ctx, instance, helper, multipoolConfig) - } - - return r.reconcileSharedTSIGSecret(ctx, instance, helper, multipoolConfig) + return r.reconcilePerPoolTSIGSecrets(ctx, instance, helper, multipoolConfig) } // reconcilePerPoolTSIGSecrets creates per-pool TSIG keys and stores them in a Secret. -// Every pool (including default) gets its own TSIG key with scope=POOL and resource_id=pool-UUID. +// Non-default pools always need a POOL-scoped key with resource_id=pool-UUID. +// The default pool only needs a key in AZ-aware mode. func (r *DesignateBackendbind9Reconciler) reconcilePerPoolTSIGSecrets( ctx context.Context, instance *designatev1beta1.DesignateBackendbind9, @@ -83,7 +75,12 @@ func (r *DesignateBackendbind9Reconciler) reconcilePerPoolTSIGSecrets( multipoolConfig *designate.MultipoolConfig, ) (ctrl.Result, error) { Log := r.GetLogger(ctx) - Log.Info("Reconciling per-pool TSIG secrets (AZ-aware mode)") + Log.Info("Reconciling per-pool TSIG secrets") + designateInstance, err := r.getDesignateCR(ctx, helper, instance) + if err != nil { + return ctrl.Result{}, err + } + azAware := designateInstance.Spec.AZAwareMode == designatev1beta1.AZModeEnabled // The pool0-named secret is canonical: besides its own pool's tsigkeys.conf, it also carries // the full pool-name -> tsigkey-id map that designate_controller.go reads for pools.yaml, so @@ -101,7 +98,7 @@ func (r *DesignateBackendbind9Reconciler) reconcilePerPoolTSIGSecrets( } // Check pool config hash to avoid unnecessary work - poolConfigHash, err := r.getPerPoolConfigHash(multipoolConfig, mdnsIPs) + poolConfigHash, err := r.getPerPoolConfigHash(multipoolConfig, mdnsIPs, azAware) if err != nil { return ctrl.Result{}, err } @@ -109,9 +106,12 @@ func (r *DesignateBackendbind9Reconciler) reconcilePerPoolTSIGSecrets( existingSecret := &corev1.Secret{} err = helper.GetClient().Get(ctx, types.NamespacedName{Name: canonicalSecretName, Namespace: instance.Namespace}, existingSecret) if err == nil { - if existingHash, exists := existingSecret.Annotations["pool-config-hash"]; exists && existingHash == poolConfigHash { - Log.Info("Per-pool TSIG secrets are up to date (pool config unchanged)") - return ctrl.Result{}, nil + current, checkErr := r.perPoolTSIGSecretsCurrent(ctx, helper, instance, multipoolConfig, existingSecret, poolConfigHash, azAware) + if checkErr != nil { + return ctrl.Result{}, checkErr + } + if current { + return r.publishTSIGKeysWhenReady(ctx, helper, instance, multipoolConfig, existingSecret, azAware) } } else if !k8s_errors.IsNotFound(err) { return ctrl.Result{}, err @@ -129,7 +129,7 @@ func (r *DesignateBackendbind9Reconciler) reconcilePerPoolTSIGSecrets( return ctrl.Result{}, err } - if len(tsigKeys) == 0 { + if len(tsigKeys) == 0 && (azAware || len(multipoolConfig.Pools) > 1) { Log.Info("No pools registered in Designate yet, will requeue in 30 seconds") return ctrl.Result{RequeueAfter: time.Second * 30}, nil } @@ -145,23 +145,104 @@ func (r *DesignateBackendbind9Reconciler) reconcilePerPoolTSIGSecrets( // Each pool is a separate BIND process (separate StatefulSet), so it only ever needs its own // key — write one Secret per pool rather than merging every pool's key into one file (BIND's // "server" clause only accepts a single key per remote address). - for poolIdx, pool := range multipoolConfig.Pools { - key, ok := tsigKeys[pool.Name] - if !ok { - // Pool not yet registered in Designate; ensurePerPoolTSIGKeys already logged this. - continue + // Publish the canonical mapping LAST. A failed write to any pool's Secret must + // not leave a matching hash that suppresses reconciliation on the next retry. + for poolIdx := len(multipoolConfig.Pools) - 1; poolIdx >= 0; poolIdx-- { + pool := multipoolConfig.Pools[poolIdx] + key := tsigKeys[pool.Name] + tsigConfigContent := "" + if key != nil { + tsigConfigContent = r.generateTSIGConfig(key, mdnsIPs) } secretName := tsigSecretNameForPool(instance.Name, poolIdx) - tsigConfigContent := r.generateTSIGConfig(key, mdnsIPs) if err := r.createOrUpdatePerPoolTSIGSecret(ctx, helper, instance, secretName, key, tsigConfigContent, poolConfigHash, tsigKeys); err != nil { return ctrl.Result{}, err } } + // Do not delete the legacy shared DB key here: pods from an older operator + // may still use it during rollout. It is never assigned to a pool again. Log.Info(fmt.Sprintf("Per-pool TSIG secrets reconciled (hash: %s, pools: %d)", poolConfigHash, len(tsigKeys))) + // Let the StatefulSet controller roll out the new key material before workers + // start signing SOA checks with the newly published IDs. + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil +} + +// publishTSIGKeysWhenReady prevents workers from using new keys before BIND has +// loaded them. The legacy DB key stays valid while old BIND pods are running. +func (r *DesignateBackendbind9Reconciler) publishTSIGKeysWhenReady( + ctx context.Context, h *helper.Helper, instance *designatev1beta1.DesignateBackendbind9, + config *designate.MultipoolConfig, canonical *corev1.Secret, azAware bool, +) (ctrl.Result, error) { + if canonical.Annotations["tsig-keys-ready"] == "true" { + return ctrl.Result{}, nil + } + for i, pool := range config.Pools { + if pool.Name == designate.DefaultPoolName && !azAware { + continue + } + secret := &corev1.Secret{} + if err := h.GetClient().Get(ctx, types.NamespacedName{Name: tsigSecretNameForPool(instance.Name, i), Namespace: instance.Namespace}, secret); err != nil { + return ctrl.Result{}, err + } + name := strings.TrimSuffix(secret.Name, designate.TsigSecretSuffix) + sts := &appsv1.StatefulSet{} + err := h.GetClient().Get(ctx, types.NamespacedName{Name: name, Namespace: instance.Namespace}, sts) + if k8s_errors.IsNotFound(err) { + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + } + if err != nil { + return ctrl.Result{}, err + } + if sts.Spec.Template.Annotations["tsig-secret-version"] != secret.ResourceVersion || + sts.Status.ObservedGeneration != sts.Generation || sts.Status.ReadyReplicas != pool.BindReplicas || + sts.Status.UpdatedReplicas != pool.BindReplicas || sts.Status.CurrentRevision != sts.Status.UpdateRevision { + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + } + } + base := canonical.DeepCopy() + canonical.Annotations["tsig-keys-ready"] = "true" + if err := h.GetClient().Patch(ctx, canonical, client.MergeFrom(base)); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } +// perPoolTSIGSecretsCurrent checks every Secret before trusting the canonical hash. +// This also recovers from partial writes and deleted per-pool Secrets. +func (r *DesignateBackendbind9Reconciler) perPoolTSIGSecretsCurrent( + ctx context.Context, h *helper.Helper, instance *designatev1beta1.DesignateBackendbind9, + config *designate.MultipoolConfig, canonical *corev1.Secret, hash string, azAware bool, +) (bool, error) { + if canonical.Annotations["pool-config-hash"] != hash || canonical.Annotations["tsig-mode"] != "per-pool" { + return false, nil + } + var ids map[string]string + if err := json.Unmarshal(canonical.Data[designate.TSIGKeyIDsDataKey], &ids); err != nil { + return false, nil + } + if !azAware && (ids[designate.DefaultPoolName] != "" || len(canonical.Data["tsigkeys.conf"]) != 0 || canonical.Annotations["tsigkey-id"] != "") { + return false, nil + } + for i, pool := range config.Pools { + if pool.Name == designate.DefaultPoolName && !azAware { + continue + } + secret := &corev1.Secret{} + err := h.GetClient().Get(ctx, types.NamespacedName{Name: tsigSecretNameForPool(instance.Name, i), Namespace: instance.Namespace}, secret) + if k8s_errors.IsNotFound(err) { + return false, nil + } + if err != nil { + return false, err + } + if ids[pool.Name] == "" || secret.Annotations["tsigkey-id"] != ids[pool.Name] || secret.Annotations["pool-config-hash"] != hash || len(secret.Data["tsigkeys.conf"]) == 0 { + return false, nil + } + } + return true, nil +} + // tsigSecretNameForPool computes the per-pool TSIG Secret name for a given pool index. Every // pool gets its own Secret containing only its own key — pool0 keeps the base name for // backwards compatibility, matching the naming convention used elsewhere for per-pool @@ -214,13 +295,25 @@ func (r *DesignateBackendbind9Reconciler) createOrUpdatePerPoolTSIGSecret( } stringData[designate.TSIGKeyIDsDataKey] = string(tsigKeyIDsJSON) } - tsigSecret.StringData = stringData + // Write Data directly so removed values really disappear, including legacy + // shared key material on the canonical (default pool) Secret. + tsigSecret.Data = make(map[string][]byte, len(stringData)) + for name, value := range stringData { + tsigSecret.Data[name] = []byte(value) + } + tsigSecret.StringData = nil if tsigSecret.Annotations == nil { tsigSecret.Annotations = make(map[string]string) } tsigSecret.Annotations["pool-config-hash"] = poolConfigHash tsigSecret.Annotations["tsig-mode"] = "per-pool" - tsigSecret.Annotations["tsigkey-id"] = key.ID + if secretName == tsigSecretNameForPool(instance.Name, 0) { + tsigSecret.Annotations["tsig-keys-ready"] = "false" + } + delete(tsigSecret.Annotations, "tsigkey-id") + if key != nil { + tsigSecret.Annotations["tsigkey-id"] = key.ID + } return controllerutil.SetControllerReference(instance, tsigSecret, r.Scheme) }) if err != nil { @@ -230,115 +323,6 @@ func (r *DesignateBackendbind9Reconciler) createOrUpdatePerPoolTSIGSecret( return nil } -// reconcileSharedTSIGSecret implements the original shared TSIG key model for non-AZ deployments. -func (r *DesignateBackendbind9Reconciler) reconcileSharedTSIGSecret( - ctx context.Context, - instance *designatev1beta1.DesignateBackendbind9, - helper *helper.Helper, - multipoolConfig *designate.MultipoolConfig, -) (ctrl.Result, error) { - Log := r.GetLogger(ctx) - Log.Info("Reconciling shared TSIG secret for multipool") - - // Early exit: Check if there are any non-default pools that need TSIG - // Pool 0 (default) doesn't need TSIG, so only pools 1+ require it - hasNonDefaultPools := false - for poolIdx := range multipoolConfig.Pools { - if poolIdx > 0 { - hasNonDefaultPools = true - break - } - } - - tsigSecretName := instance.Name + designate.TsigSecretSuffix - - if !hasNonDefaultPools { - // No non-default pools - delete TSIG secret and key from Designate - tsigSecret := &corev1.Secret{} - err := helper.GetClient().Get(ctx, types.NamespacedName{Name: tsigSecretName, Namespace: instance.Namespace}, tsigSecret) - if err == nil { - Log.Info("No non-default pools, deleting TSIG secret and key from Designate") - - // Delete TSIG key from Designate database - osclient, err := designate.GetOpenstackClient(ctx, instance.Namespace, helper) - if err != nil { - Log.Error(err, "Failed to get OpenStack client for TSIG key deletion") - // Continue with secret deletion even if we can't delete from Designate - } else { - err = designate.DeleteTSIGKeyByName(ctx, osclient, designate.SharedTSIGKeyName) - if err != nil { - Log.Error(err, "Failed to delete TSIG key from Designate") - // Continue with secret deletion even if Designate deletion fails - } else { - Log.Info(fmt.Sprintf("Deleted TSIG key %s from Designate", designate.SharedTSIGKeyName)) - } - } - - // Delete Kubernetes secret - err = helper.GetClient().Delete(ctx, tsigSecret) - if err != nil && !k8s_errors.IsNotFound(err) { - return ctrl.Result{}, err - } - } - return ctrl.Result{}, nil - } - - // Check if we need to update the TSIG secret by comparing pool configuration hash - // This avoids expensive operations (fetching pool IDs, querying OpenStack) when nothing has changed - poolConfigHash, err := r.getPoolConfigHash(multipoolConfig) - if err != nil { - return ctrl.Result{}, err - } - - // Check if secret exists and has the same pool config hash - existingSecret := &corev1.Secret{} - err = helper.GetClient().Get(ctx, types.NamespacedName{Name: tsigSecretName, Namespace: instance.Namespace}, existingSecret) - if err == nil { - // Force re-write if per-pool artifacts are present (transition from per-pool to shared) - _, hasPerPoolAnnotation := existingSecret.Annotations["tsig-mode"] - _, hasPerPoolData := existingSecret.Data[designate.TSIGKeyIDsDataKey] - needsCleanup := hasPerPoolAnnotation || hasPerPoolData - - if !needsCleanup { - if existingHash, exists := existingSecret.Annotations["pool-config-hash"]; exists && existingHash == poolConfigHash { - Log.Info("TSIG secret is up to date (pool config unchanged)") - return ctrl.Result{}, nil - } - } else { - Log.Info("Per-pool TSIG artifacts detected in shared mode, forcing secret re-write") - } - } else if !k8s_errors.IsNotFound(err) { - return ctrl.Result{}, err - } - - // Pool config has changed or secret doesn't exist - regenerate TSIG config - Log.Info("Pool config changed or TSIG secret missing, regenerating") - - mdnsIPs, err := r.getMdnsIPsForTSIG(ctx, helper, instance.Namespace) - if err != nil { - // Check if mdns ConfigMap doesn't exist yet (Designate controller hasn't created it) - if k8s_errors.IsNotFound(err) { - Log.Info("mdns-ip-map ConfigMap not found yet, will requeue in 30 seconds") - return ctrl.Result{RequeueAfter: time.Second * 30}, nil - } - return ctrl.Result{}, err - } - - // Get or create shared TSIG key for all non-default pools - tsigKey, err := r.ensureSharedTSIGKey(ctx, helper, instance.Namespace) - if err != nil { - if errors.Is(err, designate.ErrKeystoneNotReady) { - Log.Info("Required services not ready for TSIG key retrieval, will requeue in 30 seconds") - return ctrl.Result{RequeueAfter: time.Second * 30}, nil - } - return ctrl.Result{}, err - } - - tsigConfigContent := r.generateTSIGConfig(tsigKey, mdnsIPs) - - return r.createOrUpdateTSIGSecretWithHash(ctx, helper, instance, tsigConfigContent, poolConfigHash) -} - // getMdnsIPsForTSIG retrieves mdns pod IPs from the mdns-ip-map ConfigMap func (r *DesignateBackendbind9Reconciler) getMdnsIPsForTSIG( ctx context.Context, @@ -368,59 +352,7 @@ func (r *DesignateBackendbind9Reconciler) getMdnsIPsForTSIG( return mdnsIPs, nil } -// ensureSharedTSIGKey retrieves or creates a shared TSIG key for all non-default pools -func (r *DesignateBackendbind9Reconciler) ensureSharedTSIGKey( - ctx context.Context, - helper *helper.Helper, - namespace string, -) (*designate.TSIGKey, error) { - Log := r.GetLogger(ctx) - - osclient, err := designate.GetOpenstackClient(ctx, namespace, helper) - if err != nil { - Log.Error(err, "Failed to get OpenStack client") - return nil, fmt.Errorf("failed to get OpenStack client: %w", err) - } - - // Try to get existing TSIG key - tsigKey, err := designate.GetTSIGKeyByName(ctx, osclient, designate.SharedTSIGKeyName) - if err != nil { - return nil, fmt.Errorf("failed to query TSIG key: %w", err) - } - - if tsigKey != nil { - Log.Info(fmt.Sprintf("Using existing shared TSIG key: %s", tsigKey.Name)) - return tsigKey, nil - } - - // Create key if it doesn't exist - Log.Info("Creating shared TSIG key for multipool") - - // Generate random secret for TSIG key (base64-encoded 32-byte random string) - secret, err := generateTSIGSecret() - if err != nil { - return nil, fmt.Errorf("failed to generate TSIG secret: %w", err) - } - - tsigKey, err = designate.CreateTSIGKey(ctx, osclient, designate.CreateTSIGKeyOpts{ - Name: designate.SharedTSIGKeyName, - Algorithm: "hmac-sha256", - Secret: secret, - Scope: "POOL", - // ResourceID is required by Designate API but only validated for UUID format, - // not checked against actual pools. Using dummy UUID since this key is shared - // across all non-default pools and not tied to any specific pool. - ResourceID: "00000000-0000-0000-0000-000000000000", - }) - if err != nil { - return nil, fmt.Errorf("failed to create TSIG key: %w", err) - } - - Log.Info(fmt.Sprintf("Created shared TSIG key: %s", tsigKey.Name)) - return tsigKey, nil -} - -// ensurePerPoolTSIGKeys creates or retrieves a TSIG key for each pool (including default). +// ensurePerPoolTSIGKeys creates or retrieves keys for pools that need signed transfers. // Returns a map of pool-name -> tsigkey-id (Designate UUID). func (r *DesignateBackendbind9Reconciler) ensurePerPoolTSIGKeys( ctx context.Context, @@ -430,18 +362,26 @@ func (r *DesignateBackendbind9Reconciler) ensurePerPoolTSIGKeys( ) (map[string]*designate.TSIGKey, error) { Log := r.GetLogger(ctx) + designateInstance, err := r.getDesignateCR(ctx, helper, instance) + if err != nil { + return nil, fmt.Errorf("failed to get Designate CR: %w", err) + } + azAware := designateInstance.Spec.AZAwareMode == designatev1beta1.AZModeEnabled + if !azAware && len(multipoolConfig.Pools) == 1 && multipoolConfig.Pools[0].Name == designate.DefaultPoolName { + return map[string]*designate.TSIGKey{}, nil + } osclient, err := designate.GetOpenstackClient(ctx, instance.Namespace, helper) if err != nil { return nil, fmt.Errorf("failed to get OpenStack client: %w", err) } - // Get pool name -> UUID mapping - designateInstance, err := r.getDesignateCR(ctx, helper, instance) - if err != nil { - return nil, fmt.Errorf("failed to get Designate CR: %w", err) + var requiredPools []string + for _, pool := range multipoolConfig.Pools { + if pool.Name != designate.DefaultPoolName || azAware { + requiredPools = append(requiredPools, pool.Name) + } } - - poolNameToID, err := designate.GetPoolNameToIDMap(ctx, helper, instance.Namespace, designateInstance) + poolNameToID, err := designate.GetPoolNameToIDMap(ctx, helper, instance.Namespace, designateInstance, requiredPools...) if err != nil { return nil, fmt.Errorf("failed to get pool name-to-ID mapping: %w", err) } @@ -460,16 +400,23 @@ func (r *DesignateBackendbind9Reconciler) ensurePerPoolTSIGKeys( result := make(map[string]*designate.TSIGKey) for _, pool := range multipoolConfig.Pools { + if pool.Name == designate.DefaultPoolName && !azAware { + continue + } keyName := pool.Name + designate.PerPoolTSIGKeySuffix poolUUID, exists := poolNameToID[pool.Name] - if !exists { - Log.Info(fmt.Sprintf("Pool %s not yet registered in Designate, skipping TSIG key creation", pool.Name)) - continue + if !exists || poolUUID == "" { + return nil, fmt.Errorf("%w: pool %s is not registered yet", designate.ErrNoPoolsFound, pool.Name) } // Check if key already exists if existing, ok := existingKeysByName[keyName]; ok { + // Repair a stale binding without rotating the name, ID, or secret used + // by running BIND pods. + if err := designate.EnsureTSIGKeyPool(ctx, osclient, existing, poolUUID); err != nil { + return nil, err + } Log.Info(fmt.Sprintf("Using existing per-pool TSIG key for pool %s: %s (ID: %s)", pool.Name, keyName, existing.ID)) result[pool.Name] = existing continue @@ -624,71 +571,13 @@ func (r *DesignateBackendbind9Reconciler) computePoolConfigHash(prefix string, p return hash, nil } -// getPoolConfigHash generates a hash of non-default pools (for shared TSIG model). -func (r *DesignateBackendbind9Reconciler) getPoolConfigHash(multipoolConfig *designate.MultipoolConfig) (string, error) { - var poolNames []string - for poolIdx, pool := range multipoolConfig.Pools { - if poolIdx > 0 { // Skip default pool (pool0) - poolNames = append(poolNames, pool.Name) - } - } - return r.computePoolConfigHash("", poolNames, nil) -} - // getPerPoolConfigHash generates a hash including ALL pools and mDNS IPs (for per-pool TSIG model). -func (r *DesignateBackendbind9Reconciler) getPerPoolConfigHash(multipoolConfig *designate.MultipoolConfig, mdnsIPs []string) (string, error) { +func (r *DesignateBackendbind9Reconciler) getPerPoolConfigHash(multipoolConfig *designate.MultipoolConfig, mdnsIPs []string, azAware bool) (string, error) { var poolNames []string for _, pool := range multipoolConfig.Pools { poolNames = append(poolNames, pool.Name) } - return r.computePoolConfigHash("per-pool:", poolNames, mdnsIPs) -} - -// createOrUpdateTSIGSecretWithHash creates or updates the TSIG secret with pool config hash annotation -func (r *DesignateBackendbind9Reconciler) createOrUpdateTSIGSecretWithHash( - ctx context.Context, - helper *helper.Helper, - instance *designatev1beta1.DesignateBackendbind9, - tsigConfigContent string, - poolConfigHash string, -) (ctrl.Result, error) { - Log := r.GetLogger(ctx) - - tsigSecretName := instance.Name + designate.TsigSecretSuffix - - tsigSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: tsigSecretName, - Namespace: instance.Namespace, - Labels: map[string]string{ - "service": "designate-backendbind9", - "component": "designate-backendbind9", - }, - }, - } - - _, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), tsigSecret, func() error { - tsigSecret.Type = corev1.SecretTypeOpaque - // Clear existing Data to remove any leftover keys (e.g. tsigkey-ids.json from per-pool mode) - tsigSecret.Data = nil - tsigSecret.StringData = map[string]string{ - "tsigkeys.conf": tsigConfigContent, - } - if tsigSecret.Annotations == nil { - tsigSecret.Annotations = make(map[string]string) - } - tsigSecret.Annotations["pool-config-hash"] = poolConfigHash - delete(tsigSecret.Annotations, "tsig-mode") - return controllerutil.SetControllerReference(instance, tsigSecret, r.Scheme) - }) - - if err != nil { - Log.Error(err, "Failed to create/update TSIG secret") - return ctrl.Result{}, fmt.Errorf("failed to create/update TSIG secret: %w", err) - } - - Log.Info(fmt.Sprintf("TSIG secret %s reconciled (hash: %s)", tsigSecretName, poolConfigHash)) - return ctrl.Result{}, nil + return r.computePoolConfigHash(fmt.Sprintf("per-pool-v2:az=%t:", azAware), poolNames, mdnsIPs) } // generateTSIGSecret generates a random base64-encoded secret for TSIG keys diff --git a/internal/controller/designatebackendbind9_tsig_migration_test.go b/internal/controller/designatebackendbind9_tsig_migration_test.go new file mode 100644 index 00000000..5f2e8baa --- /dev/null +++ b/internal/controller/designatebackendbind9_tsig_migration_test.go @@ -0,0 +1,184 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/go-logr/logr" + designatev1 "github.com/openstack-k8s-operators/designate-operator/api/v1beta1" + "github.com/openstack-k8s-operators/designate-operator/internal/designate" + keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Exercise the actual reconciler from legacy shared-key state through DB key +// creation and BIND rollout, retaining credentials used by the old pods. +func TestSharedTSIGMigrationReconciliation(t *testing.T) { + ctx := context.Background() + r, _, instance := tsigTestHelper(t, designatev1.AZModeDisabled) + const poolID = "11111111-1111-1111-1111-111111111111" + legacyKey := designate.TSIGKey{ID: "legacy-key", Name: designate.SharedTSIGKeyName, + Algorithm: "hmac-sha256", Secret: "bGVnYWN5", Scope: "POOL", ResourceID: "00000000-0000-0000-0000-000000000000"} // #nosec G101 -- Synthetic TSIG test fixture. + keys := []designate.TSIGKey{legacyKey} + var mu sync.Mutex + var apiURL string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + mu.Lock() + defer mu.Unlock() + w.Header().Set("Content-Type", "application/json") + switch { + case req.Method == http.MethodGet && req.URL.Path == "/": + _, _ = w.Write([]byte(`{"versions":[{"id":"v2","status":"CURRENT"}]}`)) + case req.Method == http.MethodPost && req.URL.Path == "/v3/auth/tokens": + w.Header().Set("X-Subject-Token", "test-token") + w.WriteHeader(http.StatusCreated) + _, _ = fmt.Fprintf(w, `{"token":{"expires_at":"2099-01-01T00:00:00Z","catalog":[ + {"type":"identity","name":"keystone","endpoints":[{"interface":"internal","region":"RegionOne","url":%q}]}, + {"type":"dns","name":"designate","endpoints":[{"interface":"internal","region":"RegionOne","url":%q}]}]}}`, apiURL+"/v3/", apiURL+"/") + case req.Method == http.MethodGet && req.URL.Path == "/v2/tsigkeys": + _ = json.NewEncoder(w).Encode(map[string]interface{}{"tsigkeys": keys}) + case req.Method == http.MethodPost && req.URL.Path == "/v2/tsigkeys": + var key designate.TSIGKey + if err := json.NewDecoder(req.Body).Decode(&key); err != nil { + t.Error(err) + http.Error(w, "invalid key", http.StatusBadRequest) + return + } + if key.Name != "pool1"+designate.PerPoolTSIGKeySuffix || key.Scope != "POOL" || key.ResourceID != poolID { + t.Errorf("key must belong to the registered non-default pool: %+v", key) + } + key.ID = "22222222-2222-2222-2222-222222222222" + keys = append(keys, key) + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(key) // #nosec G117 -- Mock Designate response must include the synthetic TSIG secret for BIND configuration. + case req.Method == http.MethodGet && req.URL.Path == "/api/v1/namespaces/test/pods/discovery/log": + _, _ = fmt.Fprintf(w, "Update Pool: ", poolID) + default: + t.Errorf("unexpected migration request: %s %s", req.Method, req.URL.Path) + http.Error(w, "unexpected request", http.StatusNotFound) + } + })) + defer server.Close() + apiURL = server.URL + keystone := &keystonev1.KeystoneAPI{ObjectMeta: metav1.ObjectMeta{Name: "keystone", Namespace: instance.Namespace}} + keystone.Spec.AdminUser = "admin" + keystone.Spec.AdminProject = "admin" + keystone.Spec.Region = "RegionOne" + keystone.Spec.Secret = "keystone-password" + keystone.Spec.PasswordSelectors.Admin = "AdminPassword" + keystone.Status.APIEndpoints = map[string]string{"internal": apiURL + "/v3/"} + password := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: keystone.Spec.Secret, Namespace: instance.Namespace}, + Data: map[string][]byte{"AdminPassword": []byte("test-password")}} + canonical := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: tsigSecretNameForPool(instance.Name, 0), Namespace: instance.Namespace, + Annotations: map[string]string{"pool-config-hash": "legacy", "tsigkey-id": legacyKey.ID}}, + Data: map[string][]byte{"tsigkeys.conf": []byte(r.generateTSIGConfig(&legacyKey, []string{"192.0.2.1"}))}} + for _, object := range []client.Object{keystone, password, canonical} { + if err := r.Create(ctx, object); err != nil { + t.Fatal(err) + } + } + kube, err := kubernetes.NewForConfig(&rest.Config{Host: apiURL}) + if err != nil { + t.Fatal(err) + } + h, err := helper.NewHelper(instance, r.Client, kube, r.Scheme, logr.Discard()) + if err != nil { + t.Fatal(err) + } + config := &designate.MultipoolConfig{Pools: []designate.PoolConfig{{Name: "default", BindReplicas: 1}, {Name: "pool1", BindReplicas: 1}}} + // Start discovery through reconciliation, then simulate its successful pod. + if result, err := r.reconcileTSIGSecrets(ctx, instance, h, config); err != nil || result.RequeueAfter == 0 { + t.Fatalf("start migration: %+v, %v", result, err) + } + if waiting := getTSIGSecret(t, r, canonical.Name); waiting.Annotations["tsigkey-id"] != legacyKey.ID || string(waiting.Data["tsigkeys.conf"]) != string(canonical.Data["tsigkeys.conf"]) { + t.Fatal("migration changed the legacy Secret before discovering registered pools") + } + discovery := &batchv1.Job{} + if err := r.Get(ctx, types.NamespacedName{Name: "designate-pool-list", Namespace: instance.Namespace}, discovery); err != nil { + t.Fatal(err) + } + discovery.Status.Succeeded = 1 + if err := r.Status().Update(ctx, discovery); err != nil { + t.Fatal(err) + } + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "discovery", Namespace: instance.Namespace, + Labels: map[string]string{"job-name": discovery.Name, batchv1.ControllerUidLabel: string(discovery.UID)}}} + if err := r.Create(ctx, pod); err != nil { + t.Fatal(err) + } + if result, err := r.reconcileTSIGSecrets(ctx, instance, h, config); err != nil || result.RequeueAfter == 0 { + t.Fatalf("migrate shared key: %+v, %v", result, err) + } + canonical = getTSIGSecret(t, r, canonical.Name) + if canonical.Annotations["tsig-keys-ready"] != "false" || canonical.Annotations["tsigkey-id"] != "" || len(canonical.Data["tsigkeys.conf"]) != 0 { + t.Fatal("default pool retained a key or published new identities before BIND rollout") + } + secret := getTSIGSecret(t, r, tsigSecretNameForPool(instance.Name, 1)) + var ids map[string]string + if err := json.Unmarshal(canonical.Data[designate.TSIGKeyIDsDataKey], &ids); err != nil { + t.Fatal(err) + } + if len(ids) != 1 || ids["pool1"] != secret.Annotations["tsigkey-id"] || ids["pool1"] == "" { + t.Fatalf("incorrect pool identity mapping: %v", ids) + } + sts := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: instance.Name + "-pool1", Namespace: instance.Namespace}, + Spec: appsv1.StatefulSetSpec{Template: corev1.PodTemplateSpec{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"tsig-secret-version": secret.ResourceVersion}}}}} + if err := r.Create(ctx, sts); err != nil { + t.Fatal(err) + } + if result, err := r.reconcileTSIGSecrets(ctx, instance, h, config); err != nil || result.RequeueAfter == 0 { + t.Fatalf("must wait for BIND readiness: %+v, %v", result, err) + } + if getTSIGSecret(t, r, canonical.Name).Annotations["tsig-keys-ready"] != "false" { + t.Fatal("migration released key IDs while BIND was still pending") + } + sts.Status = appsv1.StatefulSetStatus{ObservedGeneration: sts.Generation, ReadyReplicas: 1, UpdatedReplicas: 1, CurrentRevision: "new-keys", UpdateRevision: "new-keys"} + if err := r.Status().Update(ctx, sts); err != nil { + t.Fatal(err) + } + if result, err := r.reconcileTSIGSecrets(ctx, instance, h, config); err != nil || result != (ctrl.Result{}) { + t.Fatalf("publish migrated keys: %+v, %v", result, err) + } + if getTSIGSecret(t, r, canonical.Name).Annotations["tsig-keys-ready"] != "true" { + t.Fatal("keys were not released after BIND rollout") + } + mu.Lock() + defer mu.Unlock() + if len(keys) != 2 || keys[0] != legacyKey { + t.Fatal("migration deleted or rotated the legacy key still used by old pods") + } + if !strings.Contains(string(secret.Data["tsigkeys.conf"]), keys[1].Secret) || strings.Contains(string(secret.Data["tsigkeys.conf"]), legacyKey.Name) { + t.Fatal("BIND did not receive its registered per-pool key material") + } +} diff --git a/internal/controller/designatebackendbind9_tsig_test.go b/internal/controller/designatebackendbind9_tsig_test.go new file mode 100644 index 00000000..2f3d4df1 --- /dev/null +++ b/internal/controller/designatebackendbind9_tsig_test.go @@ -0,0 +1,221 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/go-logr/logr" + designatev1 "github.com/openstack-k8s-operators/designate-operator/api/v1beta1" + "github.com/openstack-k8s-operators/designate-operator/internal/designate" + keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func tsigTestHelper(t *testing.T, mode designatev1.DesignateAZMode) (*DesignateBackendbind9Reconciler, *helper.Helper, *designatev1.DesignateBackendbind9) { + t.Helper() + scheme := runtime.NewScheme() + if err := appsv1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := designatev1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := batchv1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := keystonev1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + instance := &designatev1.DesignateBackendbind9{ObjectMeta: metav1.ObjectMeta{ + Name: "designate-backendbind9", Namespace: "test", UID: "backend-uid", + OwnerReferences: []metav1.OwnerReference{{APIVersion: designatev1.GroupVersion.String(), Kind: "Designate", Name: "designate"}}, + }} + parent := &designatev1.Designate{ObjectMeta: metav1.ObjectMeta{Name: "designate", Namespace: instance.Namespace}, Spec: designatev1.DesignateSpec{DesignateSpecBase: designatev1.DesignateSpecBase{AZAwareMode: mode}}} + mdns := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "designate-mdns-ip-map", Namespace: instance.Namespace}, Data: map[string]string{"mdns_address_0": "192.0.2.1"}} + r := &DesignateBackendbind9Reconciler{Scheme: scheme} + r.Client = fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&batchv1.Job{}, &appsv1.StatefulSet{}). + WithObjects(instance, parent, mdns).Build() + h, err := helper.NewHelper(instance, r.Client, nil, scheme, logr.Discard()) + if err != nil { + t.Fatal(err) + } + return r, h, instance +} + +func getTSIGSecret(t *testing.T, r *DesignateBackendbind9Reconciler, name string) *corev1.Secret { + t.Helper() + secret := &corev1.Secret{} + if err := r.Get(context.Background(), types.NamespacedName{Name: name, Namespace: "test"}, secret); err != nil { + t.Fatal(err) + } + return secret +} + +func TestTSIGMigrationAndModeSwitch(t *testing.T) { + for _, mode := range []designatev1.DesignateAZMode{"", designatev1.AZModeDisabled, designatev1.AZModeEnabled} { + t.Run(string(mode), func(t *testing.T) { + ctx := context.Background() + r, h, instance := tsigTestHelper(t, mode) + config := &designate.MultipoolConfig{Pools: []designate.PoolConfig{{Name: "default"}, {Name: "pool1"}, {Name: "pool2"}}} + legacy := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: tsigSecretNameForPool(instance.Name, 0), Namespace: instance.Namespace, Annotations: map[string]string{"pool-config-hash": "legacy", "tsigkey-id": "dummy-key"}}, Data: map[string][]byte{"tsigkeys.conf": []byte("legacy shared key"), designate.TSIGKeyIDsDataKey: []byte(`{"default":"dummy-key"}`)}} + if err := r.Create(ctx, legacy); err != nil { + t.Fatal(err) + } + azAware := mode == designatev1.AZModeEnabled + hash, err := r.getPerPoolConfigHash(config, []string{"192.0.2.1"}, azAware) + if err != nil { + t.Fatal(err) + } + keys := map[string]*designate.TSIGKey{ + "pool1": {ID: "pool1-key-uuid", Name: "pool1-tsig", Algorithm: "hmac-sha256", Secret: "c2VjcmV0", Scope: "POOL", ResourceID: "11111111-1111-1111-1111-111111111111"}, // #nosec G101 -- Synthetic TSIG test fixture, not a real credential. + "pool2": {ID: "pool2-key-uuid", Name: "pool2-tsig", Algorithm: "hmac-sha256", Secret: "c2VjcmV0Mg==", Scope: "POOL", ResourceID: "22222222-2222-2222-2222-222222222222"}, // #nosec G101 -- Synthetic TSIG test fixture, not a real credential. + } + if azAware { + keys["default"] = &designate.TSIGKey{ID: "default-key-uuid", Name: "default-tsig", Algorithm: "hmac-sha256", Secret: "ZGVmYXVsdA=="} // #nosec G101 -- Synthetic TSIG test fixture, not a real credential. + } + // Simulate migration writes with the canonical mapping published last. + for i := len(config.Pools) - 1; i >= 0; i-- { + key := keys[config.Pools[i].Name] + content := "" + if key != nil { + content = r.generateTSIGConfig(key, []string{"192.0.2.1"}) + } + if err := r.createOrUpdatePerPoolTSIGSecret(ctx, h, instance, tsigSecretNameForPool(instance.Name, i), key, content, hash, keys); err != nil { + t.Fatal(err) + } + } + canonical := getTSIGSecret(t, r, legacy.Name) + var ids map[string]string + if err := json.Unmarshal(canonical.Data[designate.TSIGKeyIDsDataKey], &ids); err != nil { + t.Fatal(err) + } + if !azAware && (ids["default"] != "" || len(canonical.Data["tsigkeys.conf"]) != 0 || canonical.Annotations["tsigkey-id"] != "") { + t.Fatal("default pool retained a TSIG binding or legacy key material") + } + for i, pool := range config.Pools[1:] { + secret := getTSIGSecret(t, r, tsigSecretNameForPool(instance.Name, i+1)) + if ids[pool.Name] != keys[pool.Name].ID { + t.Fatalf("missing real key ID for %s", pool.Name) + } + content := string(secret.Data["tsigkeys.conf"]) + if !strings.Contains(content, "keys { "+keys[pool.Name].Name+"; }") || strings.Contains(content, "legacy") { + t.Fatalf("incorrect transfer identity for %s: %s", pool.Name, content) + } + } + canonical.Annotations["tsig-keys-ready"] = "true" + if err := r.Update(ctx, canonical); err != nil { + t.Fatal(err) + } + // Reconciliation must reuse migrated keys without Keystone or pool-list API calls. + if result, err := r.reconcileTSIGSecrets(ctx, instance, h, config); err != nil || result != (ctrl.Result{}) { + t.Fatalf("reconcile migrated secrets: %+v, %v", result, err) + } + missing := getTSIGSecret(t, r, tsigSecretNameForPool(instance.Name, 2)) + if err := r.Delete(ctx, missing); err != nil { + t.Fatal(err) + } + current, err := r.perPoolTSIGSecretsCurrent(ctx, h, instance, config, canonical, hash, azAware) + if err != nil || current { + t.Fatalf("missing pool Secret incorrectly accepted: current=%t err=%v", current, err) + } + }) + } +} + +func TestDefaultOnlyTSIGNeedsNoDesignateAPI(t *testing.T) { + r, h, instance := tsigTestHelper(t, designatev1.AZModeDisabled) + config := &designate.MultipoolConfig{Pools: []designate.PoolConfig{{Name: "default"}}} + if result, err := r.reconcileTSIGSecrets(context.Background(), instance, h, config); err != nil || result.RequeueAfter == 0 { + t.Fatalf("default-only reconciliation: %+v, %v", result, err) + } + canonical := getTSIGSecret(t, r, tsigSecretNameForPool(instance.Name, 0)) + if len(canonical.Data["tsigkeys.conf"]) != 0 || string(canonical.Data[designate.TSIGKeyIDsDataKey]) != "{}" { + t.Fatalf("default-only pool has TSIG material: %v", canonical.Data) + } +} + +func TestTSIGHashChangesWithAZMode(t *testing.T) { + r := &DesignateBackendbind9Reconciler{} + config := &designate.MultipoolConfig{Pools: []designate.PoolConfig{{Name: "default"}, {Name: "pool1"}}} + shared, err := r.getPerPoolConfigHash(config, []string{"192.0.2.1"}, false) + if err != nil { + t.Fatal(err) + } + az, err := r.getPerPoolConfigHash(config, []string{"192.0.2.1"}, true) + if err != nil { + t.Fatal(err) + } + if shared == az { + t.Fatal("AZ mode transition must invalidate the TSIG cache") + } +} + +func TestTSIGMigrationWaitsForBINDRollout(t *testing.T) { + ctx := context.Background() + r, h, instance := tsigTestHelper(t, designatev1.AZModeDisabled) + config := &designate.MultipoolConfig{Pools: []designate.PoolConfig{{Name: "default", BindReplicas: 1}, {Name: "pool1", BindReplicas: 1}}} + key := &designate.TSIGKey{ID: "pool1-key", Name: "pool1-tsig", Algorithm: "hmac-sha256", Secret: "c2VjcmV0"} // #nosec G101 -- Synthetic TSIG test fixture, not a real credential. + keys := map[string]*designate.TSIGKey{"pool1": key} + if err := r.createOrUpdatePerPoolTSIGSecret(ctx, h, instance, tsigSecretNameForPool(instance.Name, 1), key, r.generateTSIGConfig(key, []string{"192.0.2.1"}), "new-hash", keys); err != nil { + t.Fatal(err) + } + if err := r.createOrUpdatePerPoolTSIGSecret(ctx, h, instance, tsigSecretNameForPool(instance.Name, 0), nil, "", "new-hash", keys); err != nil { + t.Fatal(err) + } + canonical := getTSIGSecret(t, r, tsigSecretNameForPool(instance.Name, 0)) + if result, err := r.publishTSIGKeysWhenReady(ctx, h, instance, config, canonical, false); err != nil || result.RequeueAfter == 0 { + t.Fatalf("migration did not wait for missing StatefulSet: %+v %v", result, err) + } + secret := getTSIGSecret(t, r, tsigSecretNameForPool(instance.Name, 1)) + sts := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: instance.Name + "-pool1", Namespace: instance.Namespace, Generation: 1}, + Spec: appsv1.StatefulSetSpec{Template: corev1.PodTemplateSpec{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"tsig-secret-version": "old-secret-version"}}}}, + Status: appsv1.StatefulSetStatus{ObservedGeneration: 1, ReadyReplicas: 1, UpdatedReplicas: 1, CurrentRevision: "ready", UpdateRevision: "ready"}, + } + if err := r.Create(ctx, sts); err != nil { + t.Fatal(err) + } + if result, err := r.publishTSIGKeysWhenReady(ctx, h, instance, config, canonical, false); err != nil || result.RequeueAfter == 0 { + t.Fatalf("migration accepted old BIND key material: %+v %v", result, err) + } + sts.Spec.Template.Annotations["tsig-secret-version"] = secret.ResourceVersion + if err := r.Update(ctx, sts); err != nil { + t.Fatal(err) + } + if result, err := r.publishTSIGKeysWhenReady(ctx, h, instance, config, canonical, false); err != nil || result.RequeueAfter != 0 { + t.Fatalf("migration did not publish after rollout: %+v %v", result, err) + } + canonical = getTSIGSecret(t, r, canonical.Name) + if canonical.Annotations["tsig-keys-ready"] != "true" { + t.Fatal("worker key IDs were not released after BIND rollout") + } +} diff --git a/internal/designate/generate_bind9_pools_yaml.go b/internal/designate/generate_bind9_pools_yaml.go index 9b66a928..63180ebd 100644 --- a/internal/designate/generate_bind9_pools_yaml.go +++ b/internal/designate/generate_bind9_pools_yaml.go @@ -138,6 +138,8 @@ type PoolConfig struct { // MultipoolConfig defines the complete multipool configuration type MultipoolConfig struct { Pools []PoolConfig + // AZAwareMode records the parent CR mode; it is not part of the ConfigMap YAML. + AZAwareMode designatev1.DesignateAZMode `yaml:"-"` } // GetMultipoolConfig reads and parses the multipool ConfigMap @@ -171,7 +173,7 @@ func GetMultipoolConfig(ctx context.Context, k8sClient client.Client, namespace return pools[i].Name < pools[j].Name }) - config := &MultipoolConfig{Pools: pools} + config := &MultipoolConfig{Pools: pools, AZAwareMode: azAwareMode} if err := validateMultipoolConfig(config, azAwareMode); err != nil { return nil, fmt.Errorf("invalid multipool config: %w", err) @@ -443,6 +445,11 @@ func generateMultiplePools(BindMap map[string]string, masterHosts []string, mult globalIndex := bindIndex - int(poolConfig.BindReplicas) + int(i) description := fmt.Sprintf("BIND9 Server for pool %s (%s)", poolConfig.Name, poolBindIPs[i]) targets[i], nameservers[i] = createTargetAndNameserver(poolBindIPs[i], globalIndex, masters, description, poolConfig.View, tsigKeyID) + if multipoolConfig.AZAwareMode != designatev1.AZModeEnabled { + // Legacy Designate accepts TSIG IDs on targets, but not nameservers. + // BIND's per-pool server configuration still signs AXFR requests. + nameservers[i].TSIGKeyID = "" + } } attributes := poolConfig.Attributes diff --git a/internal/designate/generate_bind9_pools_yaml_test.go b/internal/designate/generate_bind9_pools_yaml_test.go index 08f138fb..f67d1470 100644 --- a/internal/designate/generate_bind9_pools_yaml_test.go +++ b/internal/designate/generate_bind9_pools_yaml_test.go @@ -17,6 +17,8 @@ limitations under the License. package designate import ( + "gopkg.in/yaml.v3" + "path/filepath" "strings" "testing" "text/template" @@ -1086,6 +1088,7 @@ func TestGenerateMultiplePoolsWithTSIGKeyIDs(t *testing.T) { masterHosts := []string{"192.168.1.20"} multipoolConfig := &MultipoolConfig{ + AZAwareMode: designatev1.AZModeEnabled, Pools: []PoolConfig{ { Name: "default", @@ -1293,3 +1296,50 @@ func TestTemplateRenderingWithPerPoolTSIGKeyIDs(t *testing.T) { strings.Count(output, "tsigkey_id:"), output) } } + +func TestMultipoolTSIGYamlCompatibility(t *testing.T) { + t.Setenv("OPERATOR_TEMPLATES", filepath.Join("..", "..", "templates")) + for _, mode := range []designatev1.DesignateAZMode{"", designatev1.AZModeDisabled, designatev1.AZModeEnabled} { + t.Run(string(mode), func(t *testing.T) { + config := &MultipoolConfig{AZAwareMode: mode, Pools: []PoolConfig{ + {Name: "default", BindReplicas: 1, NSRecords: []designatev1.DesignateNSRecord{{Hostname: "ns1.example.org.", Priority: 1}}}, + {Name: "pool1", BindReplicas: 1, NSRecords: []designatev1.DesignateNSRecord{{Hostname: "ns2.example.org.", Priority: 1}}}, + }} + ids := map[string]string{"pool1": "11111111-1111-1111-1111-111111111111"} + if mode == designatev1.AZModeEnabled { + ids[DefaultPoolName] = "22222222-2222-2222-2222-222222222222" + } + output, _, err := GeneratePoolsYamlDataAndHash(map[string]string{"bind_address_0": "192.0.2.1", "bind_address_1": "192.0.2.2"}, map[string]string{"mdns_address_0": "192.0.2.3"}, nil, config, nil, nil, ids) + if err != nil { + t.Fatal(err) + } + var pools []Pool + if err := yaml.Unmarshal([]byte(output), &pools); err != nil { + t.Fatal(err) + } + for _, pool := range pools { + for _, target := range pool.Targets { + if target.TSIGKeyID != ids[pool.Name] { + t.Fatalf("pool %s lost its target TSIG identity", pool.Name) + } + } + for _, ns := range pool.Nameservers { + expected := "" + if mode == designatev1.AZModeEnabled { + expected = ids[pool.Name] + } + if ns.TSIGKeyID != expected { + t.Fatalf("pool %s has incompatible nameserver TSIG field: %q", pool.Name, ns.TSIGKeyID) + } + } + } + expectedFields := 1 + if mode == designatev1.AZModeEnabled { + expectedFields = 4 + } + if strings.Count(output, "tsigkey_id:") != expectedFields { + t.Fatalf("unexpected TSIG fields in rendered pools.yaml:\n%s", output) + } + }) + } +} diff --git a/internal/designate/pools.go b/internal/designate/pools.go index 0f401da1..26d17243 100644 --- a/internal/designate/pools.go +++ b/internal/designate/pools.go @@ -24,9 +24,12 @@ import ( "strings" "time" + "github.com/google/uuid" + designatev1beta1 "github.com/openstack-k8s-operators/designate-operator/api/v1beta1" "github.com/openstack-k8s-operators/lib-common/modules/common/helper" "github.com/openstack-k8s-operators/lib-common/modules/common/job" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -69,6 +72,11 @@ func ParsePoolListOutput(output string) ([]PoolInfo, error) { line := scanner.Text() matches := re.FindStringSubmatch(line) if len(matches) == 3 { + // Dry-run output also includes pools that would be created, with + // id:'None'. Only registered pool UUIDs can scope a TSIG key. + if _, err := uuid.Parse(matches[1]); err != nil { + continue + } pools = append(pools, PoolInfo{ ID: matches[1], Name: matches[2], @@ -87,15 +95,18 @@ func ParsePoolListOutput(output string) ([]PoolInfo, error) { return pools, nil } -// GetPoolNameToIDMap retrieves all pools using designate-manage and returns a map of name->ID +// GetPoolNameToIDMap retrieves pools using designate-manage and returns name->ID. +// If required pools are absent, discard the completed discovery Job so the next +// reconciliation observes pools registered after its cached snapshot. func GetPoolNameToIDMap( ctx context.Context, h *helper.Helper, namespace string, instance *designatev1beta1.Designate, + requiredPools ...string, ) (map[string]string, error) { pools, err := ListPoolsFromJob(ctx, h, namespace, instance) - if err != nil { + if err != nil && (len(requiredPools) == 0 || !errors.Is(err, ErrNoPoolsFound)) { return nil, err } @@ -104,6 +115,15 @@ func GetPoolNameToIDMap( poolMap[pool.Name] = pool.ID } + for _, name := range requiredPools { + if poolMap[name] == "" { + jobName := PoolListJob(instance, nil, nil).Name + if err := job.DeleteJob(ctx, h, jobName, namespace); err != nil { + return nil, fmt.Errorf("failed to refresh pool discovery: %w", err) + } + return nil, fmt.Errorf("%w: refreshing discovery for pool %s", ErrPoolListJobNotComplete, name) + } + } return poolMap, nil } @@ -139,11 +159,16 @@ func ListPoolsFromJob( return nil, ErrPoolListJobNotComplete } - // Get the pod created by the job to retrieve logs + // A deleted Job's pods can remain during background garbage collection. + // Read only pods from this Job UID, not an older Job with the same name. + currentJob, err := job.GetJobWithName(ctx, h, jobDef.Name, namespace) + if err != nil { + return nil, fmt.Errorf("failed to get pool discovery job: %w", err) + } podList := &corev1.PodList{} listOpts := []client.ListOption{ client.InNamespace(namespace), - client.MatchingLabels(map[string]string{"job-name": jobDef.Name}), + client.MatchingLabels(map[string]string{"job-name": jobDef.Name, batchv1.ControllerUidLabel: string(currentJob.UID)}), } err = h.GetClient().List(ctx, podList, listOpts...) if err != nil { diff --git a/internal/designate/pools_test.go b/internal/designate/pools_test.go index 6b0392fb..f9c26abe 100644 --- a/internal/designate/pools_test.go +++ b/internal/designate/pools_test.go @@ -17,6 +17,24 @@ limitations under the License. package designate import ( + "context" + "io" + "net/http" + "strings" + + "github.com/go-logr/logr" + designatev1 "github.com/openstack-k8s-operators/designate-operator/api/v1beta1" + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "errors" "testing" ) @@ -50,6 +68,16 @@ func TestParsePoolListOutput(t *testing.T) { }, wantErr: nil, }, + { + name: "unregistered pool in dry run", + output: "Create Pool: ", + wantErr: ErrNoPoolsFound, + }, + { + name: "registered and pending pools in dry run", + output: "Update Pool: \nCreate Pool: ", + want: []PoolInfo{{ID: "794ccc2c-d751-44fe-b57f-8894c9f5c842", Name: "default"}}, + }, { name: "no pools in output", output: "Some other output\nwithout pool information", @@ -177,3 +205,100 @@ func TestGetPoolNameToIDMap(t *testing.T) { }) } } + +// poolLogTransport serves Kubernetes pod logs without starting a network server. +type poolLogTransport struct { + logs map[string]string + paths []string +} + +func (p *poolLogTransport) RoundTrip(req *http.Request) (*http.Response, error) { + p.paths = append(p.paths, req.URL.Path) + body, ok := p.logs[req.URL.Path] + status := http.StatusOK + if !ok { + status = http.StatusNotFound + } + return &http.Response{StatusCode: status, Header: http.Header{"Content-Type": []string{"text/plain"}}, Body: io.NopCloser(strings.NewReader(body)), Request: req}, nil +} + +func TestPoolDiscoveryRefreshesMissingPool(t *testing.T) { + for _, initialOutput := range []string{ + "Update Pool: \nUpdate Pool: ", + "No registered pools yet", + "Create Pool: ", + } { + t.Run(initialOutput, func(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + for _, add := range []func(*runtime.Scheme) error{corev1.AddToScheme, batchv1.AddToScheme, designatev1.AddToScheme} { + if err := add(scheme); err != nil { + t.Fatal(err) + } + } + instance := &designatev1.Designate{ObjectMeta: metav1.ObjectMeta{Name: "designate", Namespace: "test", UID: "designate-uid"}} + c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&batchv1.Job{}).WithObjects(instance).Build() + transport := &poolLogTransport{logs: map[string]string{ + "/api/v1/namespaces/test/pods/a-old-discovery/log": initialOutput, + "/api/v1/namespaces/test/pods/z-new-discovery/log": "Update Pool: \nUpdate Pool: \nUpdate Pool: ", + }} + kube, err := kubernetes.NewForConfig(&rest.Config{Host: "https://kubernetes.test", Transport: transport}) + if err != nil { + t.Fatal(err) + } + h, err := helper.NewHelper(instance, c, kube, scheme, logr.Discard()) + if err != nil { + t.Fatal(err) + } + jobName := types.NamespacedName{Name: PoolListJob(instance, nil, nil).Name, Namespace: instance.Namespace} + complete := func(uid types.UID, podName string) { + t.Helper() + discovery := &batchv1.Job{} + if err := c.Get(ctx, jobName, discovery); err != nil { + t.Fatal(err) + } + discovery.UID = uid + if err := c.Update(ctx, discovery); err != nil { + t.Fatal(err) + } + discovery.Status.Succeeded = 1 + if err := c.Status().Update(ctx, discovery); err != nil { + t.Fatal(err) + } + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: podName, Namespace: instance.Namespace, Labels: map[string]string{"job-name": jobName.Name, batchv1.ControllerUidLabel: string(uid)}}} + if err := c.Create(ctx, pod); err != nil { + t.Fatal(err) + } + } + // The first lookup starts the discovery Job for the original configuration. + if _, err := GetPoolNameToIDMap(ctx, h, instance.Namespace, instance, "pool1"); !errors.Is(err, ErrPoolListJobNotComplete) { + t.Fatalf("expected initial discovery to requeue, got %v", err) + } + complete("old-job-uid", "a-old-discovery") + // After adding pool2, a completed snapshot of the old pools must be discarded. + ids, err := GetPoolNameToIDMap(ctx, h, instance.Namespace, instance, "pool1", "pool2") + if !errors.Is(err, ErrPoolListJobNotComplete) || ids != nil { + t.Fatalf("stale discovery was accepted: %v, %v", ids, err) + } + if err := c.Get(ctx, jobName, &batchv1.Job{}); !k8s_errors.IsNotFound(err) { + t.Fatalf("stale completed Job was not deleted: %v", err) + } + if _, err := GetPoolNameToIDMap(ctx, h, instance.Namespace, instance, "pool1", "pool2"); !errors.Is(err, ErrPoolListJobNotComplete) { + t.Fatalf("expected refreshed discovery to start, got %v", err) + } + complete("new-job-uid", "z-new-discovery") + // Keep the old Pod to model asynchronous garbage collection. Its logs must + // never be confused with the recreated Job's result. + ids, err = GetPoolNameToIDMap(ctx, h, instance.Namespace, instance, "pool1", "pool2") + if err != nil { + t.Fatal(err) + } + if ids["pool1"] != "0502b5a3-11d4-4d4a-9d53-6f576448d3e3" || ids["pool2"] != "22222222-2222-2222-2222-222222222222" { + t.Fatalf("refreshed pool identities missing: %v", ids) + } + if len(transport.paths) != 2 || transport.paths[1] != "/api/v1/namespaces/test/pods/z-new-discovery/log" { + t.Fatalf("incorrect discovery logs read: %v", transport.paths) + } + }) + } +} diff --git a/internal/designate/tsigkeys.go b/internal/designate/tsigkeys.go index ba2f112a..f63b83e8 100644 --- a/internal/designate/tsigkeys.go +++ b/internal/designate/tsigkeys.go @@ -19,6 +19,8 @@ import ( "fmt" "regexp" + "github.com/gophercloud/gophercloud/v2" + "github.com/openstack-k8s-operators/lib-common/modules/openstack" ) @@ -36,10 +38,12 @@ func IsValidTSIGKeyName(name string) bool { // TSIGKey represents a TSIG (Transaction Signature) key for DNS authentication type TSIGKey struct { - ID string `json:"id"` - Name string `json:"name"` - Algorithm string `json:"algorithm"` // e.g., "hmac-sha256" - Secret string `json:"secret"` // Base64-encoded key + ID string `json:"id"` + Scope string `json:"scope"` + ResourceID string `json:"resource_id"` + Name string `json:"name"` + Algorithm string `json:"algorithm"` // e.g., "hmac-sha256" + Secret string `json:"secret"` // Base64-encoded key } // ListAllTSIGKeys retrieves all TSIG keys with full details including ID @@ -143,6 +147,10 @@ func CreateTSIGKey( return nil, fmt.Errorf("failed to get DNS client: %w", err) } + return createTSIGKey(ctx, dnsClient, opts) +} + +func createTSIGKey(ctx context.Context, dnsClient *gophercloud.ServiceClient, opts CreateTSIGKeyOpts) (*TSIGKey, error) { url := dnsClient.ServiceURL("tsigkeys") reqBody := map[string]interface{}{ @@ -159,7 +167,7 @@ func CreateTSIGKey( var result map[string]interface{} - _, err = dnsClient.Post(ctx, url, reqBody, &result, nil) + _, err := dnsClient.Post(ctx, url, reqBody, &result, nil) if err != nil { return nil, fmt.Errorf("failed to create TSIG key: %w", err) } @@ -170,11 +178,40 @@ func CreateTSIGKey( secret, _ := result["secret"].(string) tsigKey := &TSIGKey{ - ID: id, - Name: name, - Algorithm: algorithm, - Secret: secret, + ID: id, + Name: name, + Algorithm: algorithm, + Secret: secret, + Scope: opts.Scope, + ResourceID: opts.ResourceID, } return tsigKey, nil } + +// EnsureTSIGKeyPool repairs a stale pool binding without replacing key material. +// Retaining the key ID, name and secret keeps running BIND pods authenticated. +func EnsureTSIGKeyPool(ctx context.Context, osclient *openstack.OpenStack, key *TSIGKey, poolID string) error { + if key.Scope == "POOL" && key.ResourceID == poolID { + return nil + } + dnsClient, err := GetDNSClient(osclient) + if err != nil { + return err + } + return ensureTSIGKeyPool(ctx, dnsClient, key, poolID) +} + +func ensureTSIGKeyPool(ctx context.Context, dnsClient *gophercloud.ServiceClient, key *TSIGKey, poolID string) error { + if key.Scope == "POOL" && key.ResourceID == poolID { + return nil + } + body := map[string]string{"scope": "POOL", "resource_id": poolID} + _, err := dnsClient.Patch(ctx, dnsClient.ServiceURL("tsigkeys", key.ID), body, nil, &gophercloud.RequestOpts{OkCodes: []int{200}}) + if err != nil { + return fmt.Errorf("failed to bind TSIG key %s to pool %s: %w", key.Name, poolID, err) + } + key.Scope = "POOL" + key.ResourceID = poolID + return nil +} diff --git a/internal/designate/tsigkeys_test.go b/internal/designate/tsigkeys_test.go new file mode 100644 index 00000000..647aff37 --- /dev/null +++ b/internal/designate/tsigkeys_test.go @@ -0,0 +1,110 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package designate + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gophercloud/gophercloud/v2" +) + +func TestTSIGPoolBindingMigration(t *testing.T) { + const poolID = "11111111-1111-1111-1111-111111111111" + for _, fail := range []bool{false, true} { + t.Run(map[bool]string{false: "repair", true: "API failure preserves old binding"}[fail], func(t *testing.T) { + key := &TSIGKey{ID: "existing-key", Name: "pool1-tsig", Secret: "unchanged", Algorithm: "hmac-sha256", Scope: "POOL", ResourceID: "00000000-0000-0000-0000-000000000000"} // #nosec G101 -- Synthetic TSIG test fixture, not a real credential. + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + if r.Method != http.MethodPatch || r.URL.Path != "/v2/tsigkeys/existing-key" { + t.Errorf("unexpected migration request: %s %s", r.Method, r.URL.Path) + } + var body map[string]string + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Error(err) + } + if len(body) != 2 || body["scope"] != "POOL" || body["resource_id"] != poolID { + t.Errorf("migration must only update pool binding: %v", body) + } + w.Header().Set("Content-Type", "application/json") + if fail { + w.WriteHeader(http.StatusBadRequest) + } else { + w.WriteHeader(http.StatusOK) + } + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + client := &gophercloud.ServiceClient{ProviderClient: &gophercloud.ProviderClient{}, Endpoint: server.URL + "/v2/"} + err := ensureTSIGKeyPool(context.Background(), client, key, poolID) + if (err != nil) != fail { + t.Fatalf("unexpected migration result: %v", err) + } + if key.ID != "existing-key" || key.Name != "pool1-tsig" || key.Secret != "unchanged" { + t.Fatal("migration rotated existing key material") + } + if fail { + if key.ResourceID == poolID { + t.Fatal("failed update published an unregistered binding") + } + } else { + if key.Scope != "POOL" || key.ResourceID != poolID { + t.Fatalf("incorrect pool binding: %+v", key) + } + if err := ensureTSIGKeyPool(context.Background(), client, key, poolID); err != nil { + t.Fatal(err) + } + if calls != 1 { + t.Fatalf("valid per-pool key should be reused, got %d writes", calls) + } + } + }) + } +} + +func TestCreateTSIGKeyRegistersRealPool(t *testing.T) { + const poolID = "11111111-1111-1111-1111-111111111111" + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/v2/tsigkeys" { + t.Errorf("unexpected key creation: %s %s", r.Method, r.URL.Path) + } + var body map[string]string + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Error(err) + } + if body["scope"] != "POOL" || body["resource_id"] != poolID { + t.Errorf("key is not registered to a real pool: %v", body) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + body["id"] = "real-key-uuid" + _ = json.NewEncoder(w).Encode(body) + })) + defer server.Close() + client := &gophercloud.ServiceClient{ProviderClient: &gophercloud.ProviderClient{}, Endpoint: server.URL + "/v2/"} + key, err := createTSIGKey(context.Background(), client, CreateTSIGKeyOpts{Name: "pool1-tsig", Algorithm: "hmac-sha256", Secret: "c2VjcmV0", Scope: "POOL", ResourceID: poolID}) // #nosec G101 -- Synthetic TSIG test fixture, not a real credential. + if err != nil { + t.Fatal(err) + } + if key.ID != "real-key-uuid" || key.Scope != "POOL" || key.ResourceID != poolID { + t.Fatalf("key lost its real identity: %+v", key) + } +} diff --git a/internal/designatebackendbind9/init_test.go b/internal/designatebackendbind9/init_test.go new file mode 100644 index 00000000..268e8be7 --- /dev/null +++ b/internal/designatebackendbind9/init_test.go @@ -0,0 +1,93 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package designatebackendbind9 + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// Secret volumes project read-only files. Copying named.conf preserves that +// mode, so the init script must make its own copy writable before appending. +func TestInitScriptWithReadOnlyTSIGConfig(t *testing.T) { + script, err := os.ReadFile(filepath.Join("..", "..", "templates", "designatebackendbind9", "bin", "init.sh")) + if err != nil { + t.Fatal(err) + } + root := t.TempDir() + fixtures := map[string]string{ + "config-data/default/named.conf": "include \"/etc/named/options.conf\";\n", + "config-data/default/named/options.conf": "options { };\n", + "config-data/keys/rndc-key-1": "// synthetic RNDC configuration\n", + "predictableips/rndc_key_0": "rndc-key-1", + "tsig/tsigkeys.conf": "// synthetic TSIG configuration\n", + } + for name, content := range fixtures { + path := filepath.Join(root, name) + if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0440); err != nil { // #nosec G306 -- Match Kubernetes Secret file permissions for this regression test. + t.Fatal(err) + } + } + scriptPath := filepath.Join(root, "init.sh") + if err := os.WriteFile(scriptPath, []byte(strings.ReplaceAll(string(script), "/var/lib", root)), 0600); err != nil { // #nosec G703 -- Path is inside t.TempDir and contains no user input. + t.Fatal(err) + } + // Run twice to exercise recovery from an init container restart as well. + for attempt := 0; attempt < 2; attempt++ { + cmd := exec.Command("bash", scriptPath) // #nosec G204 -- Executes the repository init script against temporary fixtures. + cmd.Env = append(os.Environ(), "POD_NAME=designate-backendbind9-pool1-0", "RNDC_PREFIX=rndc-key") + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("init attempt %d failed: %v\n%s", attempt, err, output) + } + configPath := filepath.Join(root, "config-data/merged/named.conf") + config, err := os.ReadFile(configPath) // #nosec G304 -- Read the generated configuration inside t.TempDir. + if err != nil { + t.Fatal(err) + } + include := "include \"/etc/named/tsigkeys.conf\";" + if strings.Count(string(config), include) != 1 { + t.Fatalf("expected exactly one TSIG include: %s", config) + } + info, err := os.Stat(configPath) + if err != nil { + t.Fatal(err) + } + // Also catch this regression when tests run as root, which bypasses DAC. + if info.Mode().Perm()&0200 == 0 { + t.Fatal("merged named.conf is not writable by the non-root init user") + } + for _, name := range []string{"tsigkeys.conf", "rndc.key"} { + content, err := os.ReadFile(filepath.Join(root, "config-data/merged/named", name)) // #nosec G304 -- Paths are fixed fixture names inside t.TempDir. + if err != nil { + t.Fatal(err) + } + fixture := "tsig/tsigkeys.conf" + if name == "rndc.key" { + fixture = "config-data/keys/rndc-key-1" + } + if string(content) != fixtures[fixture] { + t.Fatalf("incorrect copied %s configuration", name) + } + } + } +} diff --git a/templates/designatebackendbind9/bin/init.sh b/templates/designatebackendbind9/bin/init.sh index 897e1f43..9b75f4d4 100755 --- a/templates/designatebackendbind9/bin/init.sh +++ b/templates/designatebackendbind9/bin/init.sh @@ -25,6 +25,8 @@ cp -f /var/lib/config-data/default/named.conf /var/lib/config-data/merged/named. # Add TSIG configuration if it exists (for multipool non-default pools) if [[ -f "/var/lib/tsig/tsigkeys.conf" ]]; then cp /var/lib/tsig/tsigkeys.conf /var/lib/config-data/merged/named/tsigkeys.conf + # The Secret's read-only mode is preserved by cp; edit only the merged copy. + chmod u+w /var/lib/config-data/merged/named.conf echo 'include "/etc/named/tsigkeys.conf";' >> /var/lib/config-data/merged/named.conf echo "Added TSIG configuration to named.conf" fi diff --git a/test/functional/designate_controller_test.go b/test/functional/designate_controller_test.go index 81af5b08..e3c06e61 100644 --- a/test/functional/designate_controller_test.go +++ b/test/functional/designate_controller_test.go @@ -28,7 +28,9 @@ import ( "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -769,6 +771,101 @@ var _ = Describe("Designate controller", func() { simulateCentralReadyCount(designateName, 1) }) + It("publishes non-default TSIG identities outside AZ mode and watches key mapping updates", func() { + multipool := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: designate.MultipoolConfigMapName, Namespace: namespace}, + Data: map[string]string{"pools": `- name: default + bindReplicas: 1 + nsRecords: + - hostname: ns1.example.org. + priority: 1 +- name: pool1 + bindReplicas: 1 + nsRecords: + - hostname: ns2.example.org. + priority: 1`}, + } + Expect(k8sClient.Create(ctx, multipool)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ctx, multipool) + keyID := "11111111-1111-1111-1111-111111111111" + canonical := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: designateName.Name + "-backendbind9" + designate.TsigSecretSuffix, Namespace: namespace}, + Data: map[string][]byte{designate.TSIGKeyIDsDataKey: []byte(fmt.Sprintf(`{"default":"legacy-default-key","pool1":%q}`, keyID))}, + } + Expect(k8sClient.Create(ctx, canonical)).To(Succeed()) + DeferCleanup(k8sClient.Delete, ctx, canonical) + expectedPools := 2 + pool2KeyID := "" + checkPools := func(g Gomega, expectedKey string) { + cm := th.GetConfigMap(types.NamespacedName{Name: designate.PoolsYamlConfigMap, Namespace: namespace}) + var pools []designate.Pool + g.Expect(yaml.Unmarshal([]byte(cm.Data[designate.PoolsYamlContent]), &pools)).To(Succeed()) + g.Expect(pools).To(HaveLen(expectedPools)) + for _, pool := range pools { + expected := "" + switch pool.Name { + case "pool1": + expected = expectedKey + case "pool2": + expected = pool2KeyID + } + g.Expect(pool.Targets).NotTo(BeEmpty()) + for _, target := range pool.Targets { + g.Expect(target.TSIGKeyID).To(Equal(expected)) + } + for _, ns := range pool.Nameservers { + g.Expect(ns.TSIGKeyID).To(BeEmpty()) + } + } + } + Eventually(func(g Gomega) { checkPools(g, keyID) }, timeout, interval).Should(Succeed()) + // A canonical Secret event must trigger regeneration without a CR spec change. + newID := "22222222-2222-2222-2222-222222222222" + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(canonical), canonical)).To(Succeed()) + canonical.Data[designate.TSIGKeyIDsDataKey] = []byte(fmt.Sprintf(`{"pool1":%q}`, newID)) + canonical.Annotations = map[string]string{"tsig-keys-ready": "false"} + g.Expect(k8sClient.Update(ctx, canonical)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + // A scheduling correction must reach BIND while key publication waits. + // Otherwise a rollout that cannot start would block its own recovery. + nodeSelector := map[string]string{"kubernetes.io/hostname": "recovered-bind-node"} + Eventually(func(g Gomega) { + instance := GetDesignate(designateName) + instance.Spec.DesignateBackendbind9.NodeSelector = &nodeSelector + g.Expect(k8sClient.Update(ctx, instance)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { + backendName := types.NamespacedName{Name: designateName.Name + "-backendbind9", Namespace: namespace} + g.Expect(GetDesignateBackendbind9(backendName).Spec.NodeSelector).To(Equal(&nodeSelector)) + checkPools(g, keyID) + }, timeout, interval).Should(Succeed()) + // A pool added during rollout must still be registered so the backend + // can discover its UUID and create its key. Keep the applied identities. + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(multipool), multipool)).To(Succeed()) + multipool.Data["pools"] += ` +- name: pool2 + bindReplicas: 1 + nsRecords: + - hostname: ns3.example.org. + priority: 1` + g.Expect(k8sClient.Update(ctx, multipool)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + expectedPools = 3 + Eventually(func(g Gomega) { checkPools(g, keyID) }, timeout, interval).Should(Succeed()) + // Keep the last working IDs until BIND acknowledges the new key material. + Consistently(func(g Gomega) { checkPools(g, keyID) }, time.Second, interval).Should(Succeed()) + pool2KeyID = "33333333-3333-3333-3333-333333333333" + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(canonical), canonical)).To(Succeed()) + canonical.Data[designate.TSIGKeyIDsDataKey] = []byte(fmt.Sprintf(`{"pool1":%q,"pool2":%q}`, newID, pool2KeyID)) + canonical.Annotations["tsig-keys-ready"] = "true" + g.Expect(k8sClient.Update(ctx, canonical)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + Eventually(func(g Gomega) { checkPools(g, newID) }, timeout, interval).Should(Succeed()) + }) + It("should have created a valid pools.yaml configmap", func() { nsRecordsConfigMap := th.GetConfigMap(types.NamespacedName{ Name: designate.NsRecordsConfigMap, diff --git a/test/functional/designate_multipool_test.go b/test/functional/designate_multipool_test.go index dea3e924..56fda267 100644 --- a/test/functional/designate_multipool_test.go +++ b/test/functional/designate_multipool_test.go @@ -36,7 +36,7 @@ var _ = Describe("Designate multipool controller", func() { }) When("multipool ConfigMap with two pools is created", func() { - It("should create a shared TSIG secret for non-default pools", func() { + It("should parse non-default pools requiring per-pool TSIG identities", func() { multipoolConfig := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: multipoolConfigMapName.Name, @@ -69,7 +69,7 @@ var _ = Describe("Designate multipool controller", func() { }) When("multipool ConfigMap with only default pool is created", func() { - It("should not create TSIG secret for default-only configuration", func() { + It("should parse a default-only configuration without requiring a TSIG key", func() { multipoolConfig := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: multipoolConfigMapName.Name, diff --git a/test/kuttl/tests/designate_multipool/01-assert.yaml b/test/kuttl/tests/designate_multipool/01-assert.yaml index e6fed21b..8eecce92 100644 --- a/test/kuttl/tests/designate_multipool/01-assert.yaml +++ b/test/kuttl/tests/designate_multipool/01-assert.yaml @@ -71,16 +71,29 @@ metadata: data: rndc_key_0: rndc-key-1 --- -# Verify TSIG secret is created for non-default pools +# The canonical Secret carries the key ID mapping; only non-default pools get keys. apiVersion: v1 kind: Secret metadata: name: designate-backendbind9-tsig + annotations: + tsig-keys-ready: "true" labels: service: designate-backendbind9 component: designate-backendbind9 type: Opaque --- +apiVersion: v1 +kind: Secret +metadata: + name: designate-backendbind9-pool1-tsig + labels: + service: designate-backendbind9 + component: designate-backendbind9 + annotations: + tsig-mode: per-pool +type: Opaque +--- # Verify pools.yaml ConfigMap uses multipool NS records apiVersion: kuttl.dev/v1beta1 kind: TestAssert @@ -98,3 +111,16 @@ commands: echo "$POOLS_YAML" exit 1 fi + - script: | + # Each POOL-scoped key must authorize its own pool's mDNS transfers. + pool0_key=$(oc get secret designate-backendbind9-tsig -n $NAMESPACE -o jsonpath='{.metadata.annotations.tsigkey-id}') + pool1_key=$(oc get secret designate-backendbind9-pool1-tsig -n $NAMESPACE -o jsonpath='{.metadata.annotations.tsigkey-id}') + if [ -n "$pool0_key" ] || [ -z "$pool1_key" ]; then + echo "ERROR: Only non-default pools should have a TSIG key in shared mode" + exit 1 + fi + default_config=$(oc get secret designate-backendbind9-tsig -n "$NAMESPACE" -o jsonpath='{.data.tsigkeys\.conf}') + if [ -n "$default_config" ]; then + echo "ERROR: Default pool Secret still contains TSIG key material" + exit 1 + fi diff --git a/test/kuttl/tests/designate_multipool/04-assert.yaml b/test/kuttl/tests/designate_multipool/04-assert.yaml index fc93ef92..323dac23 100644 --- a/test/kuttl/tests/designate_multipool/04-assert.yaml +++ b/test/kuttl/tests/designate_multipool/04-assert.yaml @@ -33,3 +33,13 @@ metadata: name: designate status: designateBackendbind9ReadyCount: 3 + +--- +# The added pool must receive its own TSIG key and Secret. +apiVersion: v1 +kind: Secret +metadata: + name: designate-backendbind9-pool2-tsig + annotations: + tsig-mode: per-pool +type: Opaque diff --git a/test/kuttl/tests/designate_multipool/05-assert.yaml b/test/kuttl/tests/designate_multipool/05-assert.yaml index 07e8c12d..8f5cfeee 100644 --- a/test/kuttl/tests/designate_multipool/05-assert.yaml +++ b/test/kuttl/tests/designate_multipool/05-assert.yaml @@ -84,12 +84,16 @@ commands: echo "✓ Pool2 ConfigMap deleted" fi - # 4. Verify TSIG secret still exists (shared by pool1, should NOT be deleted) - if ! oc get secret designate-backendbind9-tsig -n $NAMESPACE 2>/dev/null; then - echo "ERROR: TSIG secret was incorrectly deleted (pool1 still needs it)" + # 4. Verify pool2's TSIG secret is deleted and pool1's is retained + if oc get secret designate-backendbind9-pool2-tsig -n $NAMESPACE 2>/dev/null; then + echo "ERROR: Pool2 TSIG secret still exists after pool removal" + ERRORS=$((ERRORS + 1)) + fi + if ! oc get secret designate-backendbind9-pool1-tsig -n $NAMESPACE 2>/dev/null; then + echo "ERROR: Pool1 TSIG secret was incorrectly deleted" ERRORS=$((ERRORS + 1)) else - echo "✓ TSIG secret retained (still needed by pool1)" + echo "✓ Pool1 TSIG secret retained" fi if [ "$ERRORS" -gt 0 ]; then @@ -101,5 +105,5 @@ commands: echo " - StatefulSet: deleted" echo " - Services: deleted" echo " - ConfigMap: deleted" - echo " - TSIG secret: retained (shared resource)" + echo " - TSIG secrets: pool2 deleted, pool1 retained" exit 0 diff --git a/test/kuttl/tests/designate_multipool/06-assert.yaml b/test/kuttl/tests/designate_multipool/06-assert.yaml index d9c04c68..f00ce42d 100644 --- a/test/kuttl/tests/designate_multipool/06-assert.yaml +++ b/test/kuttl/tests/designate_multipool/06-assert.yaml @@ -35,6 +35,10 @@ commands: echo "ERROR: TSIG secret should be deleted in single-pool mode" exit 1 fi + if oc get secret designate-backendbind9-pool1-tsig -n $NAMESPACE 2>/dev/null; then + echo "ERROR: Pool1 TSIG secret should be deleted in single-pool mode" + exit 1 + fi echo "SUCCESS: Migrated to single pool mode with all resources cleaned up" - script: | # Verify pools.yaml uses CR NS records (not multipool ConfigMap NS records) From bca5780bbb156014c9bb5e47b0e426493f16d562 Mon Sep 17 00:00:00 2001 From: Omer Date: Wed, 21 Oct 2026 15:08:11 +0200 Subject: [PATCH 2/3] Wait for producer input Secrets before creating configuration Honor requeue results from required producer Secret lookups so reconciliation stops before creating configuration or deployments. Wait for InputReady=False before asserting that configuration Secrets are absent. Reference the TransportURL Secret in the producer fixture and add deterministic coverage for each missing input Secret. --- .../designateproducer_controller.go | 10 +-- .../designateproducer_controller_test.go | 88 +++++++++++++++++++ test/functional/base_test.go | 1 + .../designateproducer_controller_test.go | 12 +++ 4 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 internal/controller/designateproducer_controller_test.go diff --git a/internal/controller/designateproducer_controller.go b/internal/controller/designateproducer_controller.go index 0473475a..15270fcb 100644 --- a/internal/controller/designateproducer_controller.go +++ b/internal/controller/designateproducer_controller.go @@ -436,7 +436,7 @@ func (r *DesignateProducerReconciler) reconcileNormal(ctx context.Context, insta // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // ctrlResult, err := getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, instance.Spec.Secret, &configMapVars, "secret-") - if err != nil { + if err != nil || (ctrlResult != ctrl.Result{}) { return ctrlResult, err } // run check OpenStack secret - end @@ -445,7 +445,7 @@ func (r *DesignateProducerReconciler) reconcileNormal(ctx context.Context, insta // check for required TransportURL secret holding transport URL string // ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, instance.Spec.TransportURLSecret, &configMapVars, "secret-") - if err != nil { + if err != nil || (ctrlResult != ctrl.Result{}) { return ctrlResult, err } // run check TransportURL secret - end @@ -472,21 +472,21 @@ func (r *DesignateProducerReconciler) reconcileNormal(ctx context.Context, insta // ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, designate.ScriptsVolumeName(designate.GetOwningDesignateName(instance)), &configMapVars, "") - if err != nil { + if err != nil || (ctrlResult != ctrl.Result{}) { return ctrlResult, err } ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, designate.ConfigVolumeName(designate.GetOwningDesignateName(instance)), &configMapVars, "") // note r.getSecret adds Conditions with condition.InputReadyWaitingMessage // when secret is not found - if err != nil { + if err != nil || (ctrlResult != ctrl.Result{}) { return ctrlResult, err } ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, designate.DefaultsVolumeName(designate.GetOwningDesignateName(instance)), &configMapVars, "") // note r.getSecret adds Conditions with condition.InputReadyWaitingMessage // when secret is not found - if err != nil { + if err != nil || (ctrlResult != ctrl.Result{}) { return ctrlResult, err } diff --git a/internal/controller/designateproducer_controller_test.go b/internal/controller/designateproducer_controller_test.go new file mode 100644 index 00000000..12ee4aac --- /dev/null +++ b/internal/controller/designateproducer_controller_test.go @@ -0,0 +1,88 @@ +/* +Copyright 2026 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "path/filepath" + "testing" + + "github.com/go-logr/logr" + designatev1 "github.com/openstack-k8s-operators/designate-operator/api/v1beta1" + "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" +) + +func TestProducerWaitsForRequiredSecrets(t *testing.T) { + t.Setenv("OPERATOR_TEMPLATES", filepath.Join("..", "..", "templates")) + inputs := []string{"passwords", "transport", "designate-scripts", "designate-config-data", "designate-defaults"} + for _, missing := range inputs { + t.Run(missing, func(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := designatev1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + instance := &designatev1.DesignateProducer{ + TypeMeta: metav1.TypeMeta{APIVersion: designatev1.GroupVersion.String(), Kind: "DesignateProducer"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "producer", Namespace: "test", UID: "producer-uid", + OwnerReferences: []metav1.OwnerReference{{APIVersion: designatev1.GroupVersion.String(), Kind: "Designate", Name: "designate"}}, + }, + } + instance.Spec.Secret = "passwords" + instance.Spec.TransportURLSecret = "transport" + objects := []client.Object{instance} + for _, name := range inputs { + if name != missing { + objects = append(objects, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: instance.Namespace}, Data: map[string][]byte{"fixture": []byte("value")}}) + } + } + // Stop at any attempted resource creation. Missing inputs must return + // before configuration rendering or deployment reconciliation starts. + created := false + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).WithInterceptorFuncs(interceptor.Funcs{ + Create: func(context.Context, client.WithWatch, client.Object, ...client.CreateOption) error { + created = true + return k8s_errors.NewBadRequest("unexpected resource creation while inputs are missing") + }, + }).Build() + r := &DesignateProducerReconciler{Client: c, Scheme: scheme} + h, err := helper.NewHelper(instance, c, nil, scheme, logr.Discard()) + if err != nil { + t.Fatal(err) + } + result, err := r.reconcileNormal(ctx, instance, h) + if err != nil || result.RequeueAfter == 0 || created { + t.Fatalf("missing %s must requeue without creating resources: result=%+v err=%v created=%t", missing, result, err, created) + } + if !instance.Status.Conditions.IsFalse(condition.InputReadyCondition) { + t.Fatal("missing input was not reported as InputReady=False") + } + }) + } +} diff --git a/test/functional/base_test.go b/test/functional/base_test.go index 87ab92bb..75d1e763 100644 --- a/test/functional/base_test.go +++ b/test/functional/base_test.go @@ -477,6 +477,7 @@ func GetDefaultDesignateProducerSpec() map[string]any { return map[string]any{ "databaseHostname": "hostname-for-designate-producer", "secret": SecretName, + "transportURLSecret": RabbitmqSecretName, "designateNetworkAttachment": "designate-attachement", "containerImage": "repo/designate-producer-image", "serviceAccount": "designate", diff --git a/test/functional/designateproducer_controller_test.go b/test/functional/designateproducer_controller_test.go index 23f3d860..4022126a 100644 --- a/test/functional/designateproducer_controller_test.go +++ b/test/functional/designateproducer_controller_test.go @@ -85,6 +85,12 @@ var _ = Describe("DesignateProducer controller", func() { }) It("should not create a secret", func() { + th.ExpectCondition( + designateProducerName, + ConditionGetterFunc(DesignateProducerConditionGetter), + condition.InputReadyCondition, + corev1.ConditionFalse, + ) secret := types.NamespacedName{ Namespace: designateProducerName.Namespace, Name: fmt.Sprintf("%s-%s", designateProducerName.Name, "config-data"), @@ -101,6 +107,12 @@ var _ = Describe("DesignateProducer controller", func() { }) It("should not create a secret", func() { + th.ExpectCondition( + designateProducerName, + ConditionGetterFunc(DesignateProducerConditionGetter), + condition.InputReadyCondition, + corev1.ConditionFalse, + ) secret := types.NamespacedName{ Namespace: designateProducerName.Namespace, Name: fmt.Sprintf("%s-%s", designateProducerName.Name, "config-data"), From bfbee9035fd80d16d7aacdfdb6d10b72a19fd1fa Mon Sep 17 00:00:00 2001 From: Omer Date: Thu, 22 Oct 2026 11:24:58 +0200 Subject: [PATCH 3/3] Fix Central reconciliation when required Secrets are missing --- .../controller/designatecentral_controller.go | 10 ++-- .../designatecentral_controller_test.go | 56 ++++++++++++++----- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/internal/controller/designatecentral_controller.go b/internal/controller/designatecentral_controller.go index 8883f63b..baf8c921 100644 --- a/internal/controller/designatecentral_controller.go +++ b/internal/controller/designatecentral_controller.go @@ -416,7 +416,7 @@ func (r *DesignateCentralReconciler) reconcileNormal(ctx context.Context, instan // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // ctrlResult, err := getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, instance.Spec.Secret, &configMapVars, "secret-") - if err != nil { + if err != nil || ctrlResult != (ctrl.Result{}) { return ctrlResult, err } // run check OpenStack secret - end @@ -425,7 +425,7 @@ func (r *DesignateCentralReconciler) reconcileNormal(ctx context.Context, instan // check for required TransportURL secret holding transport URL string // ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, instance.Spec.TransportURLSecret, &configMapVars, "secret-") - if err != nil { + if err != nil || ctrlResult != (ctrl.Result{}) { return ctrlResult, err } // run check TransportURL secret - end @@ -451,21 +451,21 @@ func (r *DesignateCentralReconciler) reconcileNormal(ctx context.Context, instan // check for required Designate config maps that should have been created by parent Designate CR // ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, designate.ScriptsVolumeName(designate.GetOwningDesignateName(instance)), &configMapVars, "") - if err != nil { + if err != nil || ctrlResult != (ctrl.Result{}) { return ctrlResult, err } ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, designate.ConfigVolumeName(designate.GetOwningDesignateName(instance)), &configMapVars, "") // note r.getSecret adds Conditions with condition.InputReadyWaitingMessage // when secret is not found - if err != nil { + if err != nil || ctrlResult != (ctrl.Result{}) { return ctrlResult, err } ctrlResult, err = getSecret(ctx, helper, instance.Namespace, &instance.Status.Conditions, designate.DefaultsVolumeName(designate.GetOwningDesignateName(instance)), &configMapVars, "") // note r.getSecret adds Conditions with condition.InputReadyWaitingMessage // when secret is not found - if err != nil { + if err != nil || ctrlResult != (ctrl.Result{}) { return ctrlResult, err } diff --git a/test/functional/designatecentral_controller_test.go b/test/functional/designatecentral_controller_test.go index fa2b3ef3..de0b79cf 100644 --- a/test/functional/designatecentral_controller_test.go +++ b/test/functional/designatecentral_controller_test.go @@ -18,12 +18,14 @@ package functional_test import ( "fmt" + "time" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports "github.com/openstack-k8s-operators/lib-common/modules/common/condition" corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" // "sigs.k8s.io/controller-runtime/pkg/client" @@ -43,6 +45,7 @@ var _ = Describe("DesignateCentral controller", func() { BeforeEach(func() { name = fmt.Sprintf("designate-central-%s", uuid.New().String()) spec = GetDefaultDesignateCentralSpec() + spec["transportURLSecret"] = RabbitmqSecretName transportURLSecretName = types.NamespacedName{ Namespace: namespace, @@ -93,21 +96,45 @@ var _ = Describe("DesignateCentral controller", func() { }) }) - When("a proper secret is provided and TransportURL is created", func() { - BeforeEach(func() { - DeferCleanup(th.DeleteInstance, CreateDesignateCentral(designateCentralName, spec)) - DeferCleanup(k8sClient.Delete, ctx, CreateDesignateSecret(namespace)) - DeferCleanup(k8sClient.Delete, ctx, CreateTransportURLSecret(transportURLSecretName)) - }) - - It("should not create a secret", func() { - secret := types.NamespacedName{ - Namespace: designateCentralName.Namespace, - Name: fmt.Sprintf("%s-%s", designateCentralName.Name, "config-data"), + DescribeTable("waits for required Secrets before generating configuration", func(missingSecret string) { + DeferCleanup(k8sClient.Delete, ctx, CreateDesignateSecret(namespace)) + DeferCleanup(k8sClient.Delete, ctx, CreateTransportURLSecret(transportURLSecretName)) + createOwnerSecrets(namespace) + + secret := th.GetSecret(types.NamespacedName{Namespace: namespace, Name: missingSecret}) + Expect(k8sClient.Delete(ctx, &secret)).To(Succeed()) + DeferCleanup(th.DeleteInstance, CreateDesignateCentral(designateCentralName, spec)) + + th.ExpectCondition( + designateCentralName, + ConditionGetterFunc(DesignateCentralConditionGetter), + condition.InputReadyCondition, + corev1.ConditionFalse, + ) + Consistently(func(g Gomega) { + for _, suffix := range []string{"config-data", "defaults"} { + err := k8sClient.Get(ctx, types.NamespacedName{ + Namespace: namespace, + Name: fmt.Sprintf("%s-%s", designateCentralName.Name, suffix), + }, &corev1.Secret{}) + g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue()) } - th.AssertSecretDoesNotExist(secret) - }) - }) + }, time.Second, interval).Should(Succeed()) + + th.CreateSecret(types.NamespacedName{Namespace: namespace, Name: missingSecret}, secret.Data) + th.ExpectCondition( + designateCentralName, + ConditionGetterFunc(DesignateCentralConditionGetter), + condition.ServiceConfigReadyCondition, + corev1.ConditionTrue, + ) + }, + Entry("missing service password", SecretName), + Entry("missing transport URL", RabbitmqSecretName), + Entry("missing parent scripts", "designate-scripts"), + Entry("missing parent configuration", "designate-config-data"), + Entry("missing parent defaults", "designate-defaults"), + ) // Notes: DesignateCentral's config file is basically hard coded and merged with the main config file // in the designate controller. @@ -122,6 +149,7 @@ var _ = Describe("DesignateCentral controller", func() { keystonePublicEndpoint = fmt.Sprintf("http://keystone-for-%s-public", designateCentralName.Name) SimulateKeystoneReady(keystoneName, keystonePublicEndpoint, keystoneInternalEndpoint) + createOwnerSecrets(namespace) createAndSimulateRedis(designateRedisName) DeferCleanup(k8sClient.Delete, ctx, CreateDesignateSecret(namespace)) DeferCleanup(k8sClient.Delete, ctx, CreateTransportURLSecret(transportURLSecretName))