Merge pull request #474 from beagles/osprh-30254-external-master

Support configuring external access to DNS masters

Author: openshift-merge-bot[bot]

internal/controller/designate_controller.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"maps"
 	"slices"
+	"sort"
 	"strings"
 	"time"
 
@@ -111,6 +112,41 @@ func getSecret(
 	return ctrl.Result{}, nil
 }
 
+// getExternalMasterServiceIPs returns the LoadBalancer ingress IPs for external master services
+// matching the given pool name. The second return value is true when at least one matching service
+// has no ingress IP yet, signalling that the caller should requeue and retry later.
+func getExternalMasterServiceIPs(ctx context.Context, helper *helper.Helper, instance *designatev1beta1.Designate, poolName string, Log logr.Logger) ([]string, bool, error) {
+	services := &corev1.ServiceList{}
+	err := helper.GetClient().List(ctx, services, client.InNamespace(instance.Namespace), client.MatchingLabels{
+		designate.ExternalMasterServiceLabel: "true",
+		"service":                            "designate-mdns",
+	})
+	if err != nil {
+		return nil, false, err
+	}
+	ips := make([]string, 0, len(services.Items))
+	pendingIngress := false
+	for _, service := range services.Items {
+		if service.Spec.Type != corev1.ServiceTypeLoadBalancer {
+			continue
+		}
+		// If the service is missing the annotation, apply to all pools by skipping the pool name check.
+		servicePool, ok := service.Annotations[designate.ExternalPoolServiceAnnotation]
+		if ok && servicePool != poolName {
+			continue
+		}
+
+		if len(service.Status.LoadBalancer.Ingress) == 0 {
+			Log.Info(fmt.Sprintf("External master Service %s/%s has no LoadBalancer ingress IP yet, will requeue", service.Namespace, service.Name))
+			pendingIngress = true
+			continue
+		}
+		ips = append(ips, service.Status.LoadBalancer.Ingress[0].IP)
+	}
+	sort.Strings(ips)
+	return ips, pendingIngress, nil
+}
+
 // GetClient -
 func (r *DesignateReconciler) GetClient() client.Client {
 	return r.Client
@@ -299,6 +335,7 @@ func (r *DesignateReconciler) validatePoolRemovals(
 // +kubebuilder:rbac:groups="security.openshift.io",resourceNames=anyuid;privileged,resources=securitycontextconstraints,verbs=use
 // +kubebuilder:rbac:groups="",resources=pods,verbs=create;delete;get;list;patch;update;watch
 // +kubebuilder:rbac:groups="",resources=pods/log,verbs=get
+// +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch
 
 // Reconcile -
 func (r *DesignateReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, _err error) {
@@ -503,6 +540,25 @@ func (r *DesignateReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
 		return result
 	}
 
+	externalMasterServiceFn := func(ctx context.Context, o client.Object) []reconcile.Request {
+		if o.GetLabels()[designate.ExternalMasterServiceLabel] != "true" {
+			return nil
+		}
+
+		designates := &designatev1beta1.DesignateList{}
+		if err := r.List(ctx, designates, client.InNamespace(o.GetNamespace())); err != nil {
+			Log.Error(err, "Unable to retrieve Designate CRs")
+			return nil
+		}
+		var result []reconcile.Request
+		for _, cr := range designates.Items {
+			result = append(result, reconcile.Request{
+				NamespacedName: client.ObjectKey{Namespace: o.GetNamespace(), Name: cr.Name},
+			})
+		}
+		return result
+	}
+
 	// TODO(beagles):
 	// - Watch for changes to the redis PODs and resync the headless hostnames for the PODs if necessary.
 	return ctrl.NewControllerManagedBy(mgr).
@@ -541,6 +597,12 @@ func (r *DesignateReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
 			handler.EnqueueRequestsFromMapFunc(redisWatchFn),
 			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
 		).
+		// Watch for external master Service changes (LoadBalancer IP assignments)
+		Watches(
+			&corev1.Service{},
+			handler.EnqueueRequestsFromMapFunc(externalMasterServiceFn),
+			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}),
+		).
 		Complete(r)
 }
 
@@ -1005,6 +1067,7 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 
 	externalBindData := make(map[string][]designate.ExternalBind)
 	externalRndcSecretData := make(map[string]string)
+	externalMasterServiceIPs := make(map[string][]string)
 	updateRndcSecret := false
 
 	if instance.Spec.ExternalBindsSecret != "" {
@@ -1048,9 +1111,18 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 				externalBindData[poolName][i].RndcFile = rndcFilename
 				externalRndcSecretData[rndcFilename] = rndcBlock
 			}
+			var pendingIngress bool
+			externalMasterServiceIPs[poolName], pendingIngress, err = getExternalMasterServiceIPs(ctx, helper, instance, poolName, Log)
+			if err != nil {
+				return ctrl.Result{}, err
+			}
+			if pendingIngress {
+				return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+			}
 		}
 		updateRndcSecret = hash != instance.Status.Hash[designate.ExternalBindsData]
 		instance.Status.Hash[designate.ExternalBindsData] = hash
+
 	} else {
 		// No external binds secret, so we need to check if the external rndc secret data is present
 		// and if it is - remove the contents so the workers have up to date.
@@ -1265,7 +1337,7 @@ func (r *DesignateReconciler) reconcileNormal(ctx context.Context, instance *des
 			Data: make(map[string]string),
 		}
 
-		poolsYaml, poolsYamlHash, err := designate.GeneratePoolsYamlDataAndHash(updatedBindMap, mdnsConfigMap.Data, nsRecords, multipoolConfig, externalBindData)
+		poolsYaml, poolsYamlHash, err := designate.GeneratePoolsYamlDataAndHash(updatedBindMap, mdnsConfigMap.Data, nsRecords, multipoolConfig, externalBindData, externalMasterServiceIPs)
 		if err != nil {
 			return ctrl.Result{}, err
 		}

internal/designate/const.go

@@ -114,4 +114,18 @@ const (
 	// ExternalRndcData is the name of the secret for mounting the external binds rndc keys
 	// in the workers.
 	ExternalRndcData = "designate-external-rndc"
+
+	// Fields for Service objects for accessing mDNS/DNS masters from outside the cluster for
+	// external DNS secondaries. Why a label AND an annotation? The label is used to identify the
+	// service as an external master, while the annotation is used to specify the pool name the
+	// external service is intended for. This allows different pools to have different masters
+	// and be configured with different networking as needed.
+
+	// ExternalMasterServiceLabel is the label name for identifying services to be as mDNS
+	// endpoints
+	ExternalMasterServiceLabel = "designate.openstack.org/external_master"
+
+	// ExternalPoolServiceAnnotation is the annotation name for specifying the pool name the external
+	// service is intended for
+	ExternalPoolServiceAnnotation = "designate.openstack.org/external_pool"
 )

internal/designate/generate_bind9_pools_yaml.go

@@ -229,7 +229,7 @@ func validateMultipoolConfig(config *MultipoolConfig, azAwareMode designatev1.De
 }
 
 // GeneratePoolsYamlDataAndHash sorts all pool resources to get the correct hash every time
-func GeneratePoolsYamlDataAndHash(BindMap, MdnsMap map[string]string, nsRecords []designatev1.DesignateNSRecord, multipoolConfig *MultipoolConfig, externalBinds map[string][]ExternalBind) (string, string, error) {
+func GeneratePoolsYamlDataAndHash(BindMap, MdnsMap map[string]string, nsRecords []designatev1.DesignateNSRecord, multipoolConfig *MultipoolConfig, externalBinds map[string][]ExternalBind, externalMasterServiceIPs map[string][]string) (string, string, error) {
 	masterHosts := make([]string, 0, len(MdnsMap))
 	for _, host := range MdnsMap {
 		masterHosts = append(masterHosts, host)
@@ -239,7 +239,7 @@ func GeneratePoolsYamlDataAndHash(BindMap, MdnsMap map[string]string, nsRecords
 	var pools []Pool
 
 	if multipoolConfig == nil {
-		pool, err := generateDefaultPool(BindMap, masterHosts, nsRecords, externalBinds["default"])
+		pool, err := generateDefaultPool(BindMap, masterHosts, nsRecords, externalBinds["default"], externalMasterServiceIPs["default"])
 		if err != nil {
 			return "", "", err
 		}
@@ -348,7 +348,7 @@ func createExternalTargetAndNameserver(bindInfo ExternalBind, masters []Master,
 	return target, nameserver
 }
 
-func generateDefaultPool(BindMap map[string]string, masterHosts []string, nsRecords []designatev1.DesignateNSRecord, externalBinds []ExternalBind) (Pool, error) {
+func generateDefaultPool(BindMap map[string]string, masterHosts []string, nsRecords []designatev1.DesignateNSRecord, externalBinds []ExternalBind, externalMasterServiceIPs []string) (Pool, error) {
 	sortNSRecords(nsRecords)
 
 	bindIPs := make([]string, len(BindMap))
@@ -364,11 +364,14 @@ func generateDefaultPool(BindMap map[string]string, masterHosts []string, nsReco
 		targets[i], nameservers[i] = createTargetAndNameserver(bindIPs[i], i, masters, description, "", "")
 	}
 
-	// NOTE: external BIND9 servers inherit the internal mDNS masters list. These masters may not be
-	// reachable from an external network — the deployer must ensure connectivity (e.g. via NAT, an
-	// exposed mDNS endpoint, or custom master addresses in a future iteration of this feature).
 	externalTargets := make([]Target, len(externalBinds))
 	externalNameservers := make([]Nameserver, len(externalBinds))
+	// Prefer external master service IPs as masters if available; fallback to internal masters if not (may require additional network routing/configuration by the admin).
+	if len(externalMasterServiceIPs) > 0 {
+		masters = createMasters(externalMasterServiceIPs)
+	} else {
+		masters = createMasters(masterHosts)
+	}
 	for i := range externalBinds {
 		description := fmt.Sprintf("External BIND9 Server %d (%s)", i, externalBinds[i].Name)
 		externalTargets[i], externalNameservers[i] = createExternalTargetAndNameserver(externalBinds[i], masters, description)

internal/designate/generate_bind9_pools_yaml_test.go

@@ -283,7 +283,7 @@ func TestGenerateDefaultPool(t *testing.T) {
 		{Hostname: "ns2.example.org.", Priority: 2},
 	}
 
-	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, []ExternalBind{})
+	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, []ExternalBind{}, []string{})
 	if err != nil {
 		t.Fatalf("generateDefaultPool() error = %v", err)
 	}
@@ -457,7 +457,7 @@ func TestSinglePoolModeUsesCRNSRecords(t *testing.T) {
 	}
 
 	// In single-pool mode, generateDefaultPool should use CR NS records
-	pool, err := generateDefaultPool(bindMap, masterHosts, crNSRecords, []ExternalBind{})
+	pool, err := generateDefaultPool(bindMap, masterHosts, crNSRecords, []ExternalBind{}, []string{})
 	if err != nil {
 		t.Fatalf("generateDefaultPool() error = %v", err)
 	}
@@ -602,7 +602,7 @@ func TestGenerateDefaultPoolWithExternalBinds(t *testing.T) {
 		},
 	}
 
-	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, externalBinds)
+	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, externalBinds, []string{})
 	if err != nil {
 		t.Fatalf("generateDefaultPool() error = %v", err)
 	}
@@ -675,7 +675,7 @@ func TestGenerateDefaultPoolWithMultipleExternalBinds(t *testing.T) {
 		},
 	}
 
-	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, externalBinds)
+	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, externalBinds, []string{})
 	if err != nil {
 		t.Fatalf("generateDefaultPool() error = %v", err)
 	}
@@ -733,7 +733,7 @@ func TestGenerateDefaultPoolWithExternalBindsFromYAML(t *testing.T) {
 	}
 	parsedBinds[0].RndcFile = "default-rndc-0"
 
-	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, parsedBinds)
+	pool, err := generateDefaultPool(bindMap, masterHosts, nsRecords, parsedBinds, []string{})
 	if err != nil {
 		t.Fatalf("generateDefaultPool() error = %v", err)
 	}

test/functional/base_test.go

@@ -50,7 +50,9 @@ const (
 	ExternalBindAddress         = "192.168.100.50"
 	ExternalBindName            = "bind9-external"
 	// Fake RNDC secret for functional tests only; not a real credential.
-	ExternalRndcSecretValue = "c2VjcmV0MTIz" // #nosec G101
+	ExternalRndcSecretValue    = "c2VjcmV0MTIz" // #nosec G101
+	ExternalMasterServiceIP    = "10.0.0.100"
+	ExternalMasterServiceAltIP = "10.0.0.101"
 
 	PublicCertSecretName   = "public-tls-certs"   // #nosec G101
 	InternalCertSecretName = "internal-tls-certs" // #nosec G101
@@ -638,6 +640,44 @@ func createAndSimulateExternalBinds(secretName types.NamespacedName) {
 	DeferCleanup(k8sClient.Delete, ctx, secret)
 }
 
+func createExternalMasterService(ns, name, poolAnnotation, ingressIP string) *corev1.Service {
+	labels := map[string]string{
+		designate.ExternalMasterServiceLabel: "true",
+		"service":                            "designate-mdns",
+	}
+	annotations := map[string]string{}
+	if poolAnnotation != "" {
+		annotations[designate.ExternalPoolServiceAnnotation] = poolAnnotation
+	}
+	svc := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        name,
+			Namespace:   ns,
+			Labels:      labels,
+			Annotations: annotations,
+		},
+		Spec: corev1.ServiceSpec{
+			Type: corev1.ServiceTypeLoadBalancer,
+			Ports: []corev1.ServicePort{
+				{Name: "mdns", Port: 5354},
+			},
+		},
+	}
+	Expect(k8sClient.Create(ctx, svc)).Should(Succeed())
+
+	if ingressIP != "" {
+		Eventually(func(g Gomega) {
+			current := &corev1.Service{}
+			g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(svc), current)).Should(Succeed())
+			current.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{{IP: ingressIP}}
+			g.Expect(k8sClient.Status().Update(ctx, current)).Should(Succeed())
+		}, timeout, interval).Should(Succeed())
+	}
+
+	DeferCleanup(k8sClient.Delete, ctx, svc)
+	return svc
+}
+
 func CreateDesignateNSRecordsConfigMap(name types.NamespacedName) client.Object {
 	return &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{