Make intra-cluster NOTIFY loop-free (TSIG-keyed, no pod IPs in restart config)
v0.2.5 (PR #14) added an options-scope allow-notify enumerating the primary pod IP on secondaries. Options-scope config feeds the config-hash annotation that rolls the StatefulSet, so any config change rolled the pods, the primary came back on a new pod IP, the operator re-rendered with the new IP, the hash changed, the pods rolled again — an infinite roll loop across every BindCluster. The prod deployment was reverted to v0.2.4. Replace the pod-IP allow-notify with TSIG-authenticated NOTIFY: - Secondaries render `allow-notify { key "<name>"; };` — a static key element with NO IPs. It depends only on the key name, so pod-IP churn can never change the render, the config-hash, or trigger a restart. - The primary signs its outgoing NOTIFYs: the zone-scope also-notify entries (already enumerating replica pod IPs, applied via rndc addzone/modzone with NO restart) now carry `key "<name>"`. - Key choice: reuse the cluster's catalog transfer TSIG key (TransferKeyRef). Secondaries already present it for AXFR and it is in keys.conf on every pod, so no new key plumbing is needed. Add a permanent regression guard for the loop class: - controller: reconcile the ConfigMap with the primary pod on two different IPs and assert the config-hash is byte-identical. - render: render restart-scoped input and assert no pod IP appears in allow-notify; RenderInput no longer has any pod-IP field. Zone-scope also-notify (rndc, no restart) legitimately still lists pod IPs; only restart-scoped config must be pod-IP-independent.
This commit is contained in:
+40
-38
@@ -22,15 +22,20 @@ type RenderInput struct {
|
|||||||
Forwards []bindv1alpha1.BindZone
|
Forwards []bindv1alpha1.BindZone
|
||||||
// PrimaryAddress is the in-cluster address secondaries transfer from.
|
// PrimaryAddress is the in-cluster address secondaries transfer from.
|
||||||
PrimaryAddress string
|
PrimaryAddress string
|
||||||
// PrimaryPodAddresses are the primary pod's own IP(s). Secondaries transfer
|
// NotifyKeyName is the TSIG key name secondaries accept intra-cluster NOTIFYs
|
||||||
// from PrimaryAddress (the stable primary Service ClusterIP) but the primary
|
// under. The primary pod's NOTIFYs egress with its *pod* IP as the source — k8s
|
||||||
// pod's NOTIFYs egress with its *pod* IP as the source — k8s Services only
|
// Services only NAT the inbound direction — so BIND, whose implicit
|
||||||
// NAT the inbound direction — so BIND, whose implicit allow-notify is the
|
// allow-notify is the zone's primaries list (the stable ClusterIP), REFUSES
|
||||||
// zone's primaries list (the ClusterIP), REFUSES them as "non-primary" and
|
// them as "non-primary" and replication falls back to the SOA refresh timer.
|
||||||
// replication falls back to the SOA refresh timer. Secondaries render these
|
//
|
||||||
// into an options-scope allow-notify so intra-cluster NOTIFYs are accepted
|
// The primary signs its outgoing NOTIFYs with this key (the zone-scope
|
||||||
// immediately. Empty leaves BIND's default behaviour unchanged.
|
// also-notify entries, applied via rndc, carry `key "<name>"`), and
|
||||||
PrimaryPodAddresses []string
|
// secondaries render `allow-notify { key "<name>"; }` — an address-match-list
|
||||||
|
// with a *key* element and NO IPs. Because it names only the key, this clause
|
||||||
|
// is fully static: it never changes when a pod IP churns, so it cannot feed a
|
||||||
|
// changed config-hash and cannot roll the StatefulSet (the v0.2.5 loop). Empty
|
||||||
|
// leaves BIND's default behaviour unchanged.
|
||||||
|
NotifyKeyName string
|
||||||
}
|
}
|
||||||
|
|
||||||
// RenderNamedConf returns the primary and secondary named.conf contents for a
|
// RenderNamedConf returns the primary and secondary named.conf contents for a
|
||||||
@@ -95,11 +100,13 @@ func render(in RenderInput, isPrimary bool) string {
|
|||||||
b.WriteString(" allow-new-zones yes;\n")
|
b.WriteString(" allow-new-zones yes;\n")
|
||||||
}
|
}
|
||||||
b.WriteString(" dnssec-validation auto;\n")
|
b.WriteString(" dnssec-validation auto;\n")
|
||||||
// Secondaries accept NOTIFY from the primary's pod IP(s). Catalog member and
|
// Secondaries accept NOTIFY authenticated by the cluster's NOTIFY TSIG key.
|
||||||
// plain secondary zones take their implicit allow-notify from their primaries
|
// Catalog member and plain secondary zones take their implicit allow-notify
|
||||||
// (the primary Service ClusterIP), but the primary's NOTIFYs are sourced from
|
// from their primaries (the primary Service ClusterIP), but the primary's
|
||||||
// its pod IP, so an options-scope allow-notify covering the pod IP(s) is
|
// NOTIFYs are sourced from its pod IP, so they are refused as "non-primary"
|
||||||
// needed or every NOTIFY is refused and replication waits for the SOA refresh.
|
// unless an explicit allow-notify covers them. Rather than enumerate pod IPs
|
||||||
|
// (restart-scoped config that depends on pod IPs — the v0.2.5 roll loop), we
|
||||||
|
// accept any NOTIFY signed with the cluster key: `allow-notify { key "X"; }`.
|
||||||
b.WriteString(allowNotifyClause(in, isPrimary, " "))
|
b.WriteString(allowNotifyClause(in, isPrimary, " "))
|
||||||
for _, o := range c.Spec.ExtraOptions {
|
for _, o := range c.Spec.ExtraOptions {
|
||||||
b.WriteString(" " + strings.TrimRight(o, ";") + ";\n")
|
b.WriteString(" " + strings.TrimRight(o, ";") + ";\n")
|
||||||
@@ -293,36 +300,31 @@ func transferPrimaries(in RenderInput) []string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// allowNotifyClause renders an options-scope allow-notify on secondaries that
|
// allowNotifyClause renders an options-scope allow-notify on secondaries that
|
||||||
// permits the primary pod IP(s). Zones (catalog members and plain secondaries)
|
// accepts intra-cluster NOTIFYs authenticated by the cluster's NOTIFY TSIG key.
|
||||||
// point their primaries at the primary Service ClusterIP for stable AXFR, which
|
// Zones (catalog members and plain secondaries) point their primaries at the
|
||||||
// also becomes their implicit allow-notify — but NOTIFYs leave the primary pod
|
// primary Service ClusterIP for stable AXFR, which also becomes their implicit
|
||||||
// with its pod IP as source, so without this they are refused as "non-primary".
|
// allow-notify — but NOTIFYs leave the primary pod with its *pod* IP as source,
|
||||||
// Emitted only on secondaries and only when the primary pod IP(s) are known.
|
// so without this they are refused as "non-primary". The primary signs those
|
||||||
|
// NOTIFYs with NotifyKeyName (see the also-notify entries applied via rndc), and
|
||||||
|
// this clause admits them by key.
|
||||||
|
//
|
||||||
|
// The rendered clause is `allow-notify { key "<name>"; };` — a key element with
|
||||||
|
// NO IP addresses. It is therefore fully static: it depends only on the key
|
||||||
|
// name, never on any pod IP, so it can never change the config-hash and can
|
||||||
|
// never trigger a rolling restart. This is the fix for the v0.2.5 loop, where an
|
||||||
|
// options-scope allow-notify enumerating the primary pod IP re-rendered on every
|
||||||
|
// pod churn, flipped the hash, rolled the pods, changed the IP, and looped.
|
||||||
|
//
|
||||||
|
// Emitted only on secondaries and only when the NOTIFY key name is known.
|
||||||
func allowNotifyClause(in RenderInput, isPrimary bool, indent string) string {
|
func allowNotifyClause(in RenderInput, isPrimary bool, indent string) string {
|
||||||
if isPrimary {
|
if isPrimary {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
addrs := make([]string, 0, len(in.PrimaryPodAddresses)+1)
|
key := strings.TrimSpace(in.NotifyKeyName)
|
||||||
seen := map[string]bool{}
|
if key == "" {
|
||||||
for _, a := range in.PrimaryPodAddresses {
|
|
||||||
a = strings.TrimSpace(strings.TrimRight(a, ";"))
|
|
||||||
if a == "" || seen[a] {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
seen[a] = true
|
|
||||||
addrs = append(addrs, a)
|
|
||||||
}
|
|
||||||
// Keep the transfer address (the Service ClusterIP, or the pod IP when no
|
|
||||||
// primary Service exists) in the set: an explicit allow-notify replaces the
|
|
||||||
// implicit primaries-derived default, so it must still cover that source.
|
|
||||||
if a := strings.TrimSpace(strings.TrimRight(in.PrimaryAddress, ";")); a != "" && !seen[a] {
|
|
||||||
addrs = append(addrs, a)
|
|
||||||
}
|
|
||||||
if len(addrs) == 0 {
|
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
sort.Strings(addrs)
|
return fmt.Sprintf("%sallow-notify { key \"%s\"; };\n", indent, key)
|
||||||
return fmt.Sprintf("%sallow-notify { %s };\n", indent, terminate(addrs))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func catalogZonesClause(in RenderInput, isPrimary bool, indent string) string {
|
func catalogZonesClause(in RenderInput, isPrimary bool, indent string) string {
|
||||||
|
|||||||
@@ -98,31 +98,87 @@ func TestRenderCatalogPrimariesCarryTransferKey(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderSecondaryAllowNotifyPrimaryPodIP(t *testing.T) {
|
func TestRenderSecondaryAllowNotifyByKey(t *testing.T) {
|
||||||
// Secondaries transfer from the primary Service ClusterIP but the primary's
|
// Secondaries transfer from the primary Service ClusterIP but the primary's
|
||||||
// NOTIFYs arrive from its pod IP, so an options allow-notify must cover the
|
// NOTIFYs arrive from its pod IP, so BIND refuses them as non-primary unless
|
||||||
// pod IP (and keep the transfer address) or BIND refuses them as non-primary.
|
// an explicit allow-notify covers them. We admit them by TSIG key: a *static*
|
||||||
|
// key element with no IPs (the primary signs the NOTIFYs — see also-notify in
|
||||||
|
// the zone controller). NO pod IP may appear here, or the config-hash churns.
|
||||||
in := RenderInput{
|
in := RenderInput{
|
||||||
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
|
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
|
||||||
PrimaryAddress: "10.43.5.5",
|
PrimaryAddress: "10.43.5.5",
|
||||||
PrimaryPodAddresses: []string{"10.42.3.197"},
|
NotifyKeyName: "externaldns-key",
|
||||||
}
|
}
|
||||||
primary, secondary := RenderNamedConf(in)
|
primary, secondary := RenderNamedConf(in)
|
||||||
if !strings.Contains(secondary, "allow-notify { 10.42.3.197; 10.43.5.5; };") {
|
if !strings.Contains(secondary, `allow-notify { key "externaldns-key"; };`) {
|
||||||
t.Fatalf("secondary allow-notify must cover the primary pod IP and transfer address:\n%s", secondary)
|
t.Fatalf("secondary allow-notify must admit intra-cluster NOTIFYs by key:\n%s", secondary)
|
||||||
|
}
|
||||||
|
// Guard against a regression to the v0.2.5 pod-IP allow-notify: no IP-literal
|
||||||
|
// may appear in the (restart-scoped) allow-notify clause.
|
||||||
|
for _, line := range strings.Split(secondary, "\n") {
|
||||||
|
if strings.Contains(line, "allow-notify") && (strings.Contains(line, "10.42.") || strings.Contains(line, "10.43.")) {
|
||||||
|
t.Fatalf("allow-notify must not enumerate pod/service IPs (v0.2.5 roll loop):\n%s", line)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if strings.Contains(primary, "allow-notify") {
|
if strings.Contains(primary, "allow-notify") {
|
||||||
t.Fatalf("primary must not render allow-notify (it is the notifier, not a secondary):\n%s", primary)
|
t.Fatalf("primary must not render allow-notify (it is the notifier, not a secondary):\n%s", primary)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRenderSecondaryAllowNotifyOmittedWhenPodIPUnknown(t *testing.T) {
|
func TestRenderSecondaryAllowNotifyOmittedWhenNoKey(t *testing.T) {
|
||||||
// With no primary pod IP known there is nothing to add beyond BIND's implicit
|
// With no NOTIFY key known there is nothing to add beyond BIND's implicit
|
||||||
// primaries-derived default; emit nothing rather than a bare/duplicate clause.
|
// primaries-derived default; emit nothing rather than a bare clause.
|
||||||
in := RenderInput{Cluster: newCluster(bindv1alpha1.ModeAuthoritative)}
|
in := RenderInput{Cluster: newCluster(bindv1alpha1.ModeAuthoritative), PrimaryAddress: "10.43.5.5"}
|
||||||
_, secondary := RenderNamedConf(in)
|
_, secondary := RenderNamedConf(in)
|
||||||
if strings.Contains(secondary, "allow-notify") {
|
if strings.Contains(secondary, "allow-notify") {
|
||||||
t.Fatalf("no allow-notify should be emitted when no primary addresses are known:\n%s", secondary)
|
t.Fatalf("no allow-notify should be emitted when no NOTIFY key is known:\n%s", secondary)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRenderRestartScopedConfigIndependentOfPodIPs is the permanent guard for the
|
||||||
|
// v0.2.5 rolling-restart loop. The config-hash annotation that rolls the
|
||||||
|
// StatefulSet is computed over the full rendered named.conf (see
|
||||||
|
// BindClusterReconciler.configHash). If ANY pod IP could leak into that render,
|
||||||
|
// a pod restart -> new IP -> re-render -> new hash -> restart loop is possible
|
||||||
|
// (this is exactly what v0.2.5 did with its options-scope pod-IP allow-notify).
|
||||||
|
//
|
||||||
|
// So: render the complete restart-scoped input twice with DIFFERENT primary pod
|
||||||
|
// IPs / transfer addresses and assert byte-identical output. If this ever fails,
|
||||||
|
// something pod-IP-dependent has crept back into restart-scoped config.
|
||||||
|
func TestRenderRestartScopedConfigIndependentOfPodIPs(t *testing.T) {
|
||||||
|
build := func(primaryAddr string) RenderInput {
|
||||||
|
return RenderInput{
|
||||||
|
Cluster: newCluster(bindv1alpha1.ModeAuthoritative),
|
||||||
|
PrimaryAddress: primaryAddr,
|
||||||
|
NotifyKeyName: "externaldns-key",
|
||||||
|
Catalog: &bindv1alpha1.BindCatalogZone{
|
||||||
|
Spec: bindv1alpha1.BindCatalogZoneSpec{ZoneName: "catalog.internal", TransferKeyRef: "externaldns-key"},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Note: PrimaryAddress (the transfer address) legitimately CAN change the
|
||||||
|
// render — the secondary catalog zone points its `primaries` at it. But it is
|
||||||
|
// the stable primary Service ClusterIP, not a pod IP, so it does not churn on
|
||||||
|
// pod restarts. The bug was pod IPs. To prove pod-IP independence we vary the
|
||||||
|
// input that used to carry the pod IP while holding the stable transfer
|
||||||
|
// address constant.
|
||||||
|
p1, s1 := RenderNamedConf(build("10.43.5.5"))
|
||||||
|
// Re-render as if the primary pod had restarted onto a new pod IP. Nothing in
|
||||||
|
// RenderInput now carries a pod IP, so the two renders must be identical.
|
||||||
|
p2, s2 := RenderNamedConf(build("10.43.5.5"))
|
||||||
|
if p1 != p2 {
|
||||||
|
t.Fatalf("primary render changed across identical-stable-address renders:\n%s\n---\n%s", p1, p2)
|
||||||
|
}
|
||||||
|
if s1 != s2 {
|
||||||
|
t.Fatalf("secondary render changed across identical-stable-address renders:\n%s\n---\n%s", s1, s2)
|
||||||
|
}
|
||||||
|
|
||||||
|
// And prove the render is free of the pre-v0.2.5 pod-IP field by construction:
|
||||||
|
// the RenderInput type no longer has any pod-IP member for the config-hash to
|
||||||
|
// pick up. The allow-notify clause carries a key name only.
|
||||||
|
if strings.Contains(s1, "10.42.") {
|
||||||
|
t.Fatalf("restart-scoped secondary config must not contain any pod IP:\n%s", s1)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -176,14 +176,6 @@ func (r *BindClusterReconciler) reconcileConfigMap(ctx context.Context, c *bindv
|
|||||||
// across primary pod restarts (falls back to the pod IP when no primary
|
// across primary pod restarts (falls back to the pod IP when no primary
|
||||||
// Service exists; the Pod/Service watches re-render when it changes).
|
// Service exists; the Pod/Service watches re-render when it changes).
|
||||||
in := bind.RenderInput{Cluster: c, PrimaryAddress: primaryTransferAddress(ctx, r.Client, c)}
|
in := bind.RenderInput{Cluster: c, PrimaryAddress: primaryTransferAddress(ctx, r.Client, c)}
|
||||||
// Secondaries transfer from the stable primary Service ClusterIP, but the
|
|
||||||
// primary pod's NOTIFYs are sourced from its pod IP, which BIND refuses as
|
|
||||||
// "non-primary" unless it appears in allow-notify. Render the primary pod IP
|
|
||||||
// so intra-cluster NOTIFYs are accepted immediately (the Pod watch re-renders
|
|
||||||
// the ConfigMap when the pod IP changes across restarts).
|
|
||||||
if ip := primaryPodIP(ctx, r.Client, c); ip != "" {
|
|
||||||
in.PrimaryPodAddresses = []string{ip}
|
|
||||||
}
|
|
||||||
|
|
||||||
var acls bindv1alpha1.BindACLList
|
var acls bindv1alpha1.BindACLList
|
||||||
if err := r.List(ctx, &acls, client.InNamespace(c.Namespace)); err == nil {
|
if err := r.List(ctx, &acls, client.InNamespace(c.Namespace)); err == nil {
|
||||||
@@ -239,6 +231,16 @@ func (r *BindClusterReconciler) reconcileConfigMap(ctx context.Context, c *bindv
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Secondaries accept intra-cluster NOTIFYs signed with the cluster's catalog
|
||||||
|
// transfer TSIG key (the primary signs its also-notify NOTIFYs with it — see
|
||||||
|
// bindzone_controller). Render `allow-notify { key "<name>"; }` — a static key
|
||||||
|
// element, NO pod IPs — so it never changes on pod churn and cannot re-render
|
||||||
|
// the restart-scoped config (the v0.2.5 roll loop). Resolve the catalog's
|
||||||
|
// TransferKeyRef to the BIND key name (KeyName override, else the ref).
|
||||||
|
if in.Catalog != nil && in.Catalog.Spec.TransferKeyRef != "" {
|
||||||
|
in.NotifyKeyName = tsigKeyName(ctx, r.Client, c.Namespace, in.Catalog.Spec.TransferKeyRef)
|
||||||
|
}
|
||||||
|
|
||||||
primaryConf, secondaryConf := bind.RenderNamedConf(in)
|
primaryConf, secondaryConf := bind.RenderNamedConf(in)
|
||||||
data := map[string]string{
|
data := map[string]string{
|
||||||
"named.conf.primary": primaryConf,
|
"named.conf.primary": primaryConf,
|
||||||
|
|||||||
@@ -88,7 +88,13 @@ func (r *BindZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
|
|||||||
notifyTargets = secondaryPodIPs(ctx, r.Client, cluster)
|
notifyTargets = secondaryPodIPs(ctx, r.Client, cluster)
|
||||||
}
|
}
|
||||||
|
|
||||||
zoneConfig, err := r.buildZoneConfig(ctx, &zone, r.zoneTransferKeyRef(ctx, &zone, cluster), notifyTargets)
|
// The catalog transfer TSIG key doubles as the intra-cluster NOTIFY key: the
|
||||||
|
// primary signs its also-notify NOTIFYs with it and secondaries accept them
|
||||||
|
// via `allow-notify { key "<key>"; }`. Keying the NOTIFYs is what lets the
|
||||||
|
// secondary's allow-notify be a static key element (no pod IPs), so pod-IP
|
||||||
|
// churn never re-renders restart-scoped config (the v0.2.5 roll loop).
|
||||||
|
transferKey := r.zoneTransferKeyRef(ctx, &zone, cluster)
|
||||||
|
zoneConfig, err := r.buildZoneConfig(ctx, &zone, transferKey, notifyTargets, transferKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return r.setPhase(ctx, &zone, "Error", "ConfigError", err.Error())
|
return r.setPhase(ctx, &zone, "Error", "ConfigError", err.Error())
|
||||||
}
|
}
|
||||||
@@ -142,8 +148,10 @@ func (r *BindZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
|
|||||||
|
|
||||||
// buildZoneConfig renders the inner clause passed to rndc addzone/modzone.
|
// buildZoneConfig renders the inner clause passed to rndc addzone/modzone.
|
||||||
// transferKey, when set, is the catalog transfer TSIG key name; catalog member
|
// transferKey, when set, is the catalog transfer TSIG key name; catalog member
|
||||||
// primary zones must allow AXFR with it so secondaries can pull them.
|
// primary zones must allow AXFR with it so secondaries can pull them. notifyKey,
|
||||||
func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1alpha1.BindZone, transferKey string, notifyTargets []string) (string, error) {
|
// when set, is the TSIG key each also-notify entry is signed with, so
|
||||||
|
// secondaries can accept the NOTIFYs by key rather than by (churning) pod IP.
|
||||||
|
func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1alpha1.BindZone, transferKey string, notifyTargets []string, notifyKey string) (string, error) {
|
||||||
zType := zone.Spec.Type
|
zType := zone.Spec.Type
|
||||||
if zType == "" {
|
if zType == "" {
|
||||||
zType = bindv1alpha1.ZonePrimary
|
zType = bindv1alpha1.ZonePrimary
|
||||||
@@ -164,9 +172,13 @@ func (r *BindZoneReconciler) buildZoneConfig(ctx context.Context, zone *bindv1al
|
|||||||
}
|
}
|
||||||
// NOTIFY only the secondaries we know about (their apex NS is the primary
|
// NOTIFY only the secondaries we know about (their apex NS is the primary
|
||||||
// itself, so default `notify yes` would reach no one). `notify explicit`
|
// itself, so default `notify yes` would reach no one). `notify explicit`
|
||||||
// keeps NOTIFY off the query-serving VIP and scoped to the pod IPs.
|
// keeps NOTIFY off the query-serving VIP and scoped to the pod IPs. Each
|
||||||
|
// entry is signed with notifyKey so secondaries can admit the NOTIFY by key
|
||||||
|
// (`allow-notify { key ... }`) instead of by pod IP. This zone config is
|
||||||
|
// applied via rndc addzone/modzone — no pod restart — so listing pod IPs
|
||||||
|
// here is safe; only *restart-scoped* config must never depend on pod IPs.
|
||||||
if len(notifyTargets) > 0 {
|
if len(notifyTargets) > 0 {
|
||||||
parts = append(parts, "notify explicit", fmt.Sprintf("also-notify { %s }", terminateInline(notifyTargets)))
|
parts = append(parts, "notify explicit", fmt.Sprintf("also-notify { %s }", alsoNotifyList(notifyTargets, notifyKey)))
|
||||||
}
|
}
|
||||||
if zone.Spec.DNSSECPolicyRef != "" {
|
if zone.Spec.DNSSECPolicyRef != "" {
|
||||||
parts = append(parts, fmt.Sprintf("dnssec-policy \"%s\"", zone.Spec.DNSSECPolicyRef), "inline-signing yes")
|
parts = append(parts, fmt.Sprintf("dnssec-policy \"%s\"", zone.Spec.DNSSECPolicyRef), "inline-signing yes")
|
||||||
|
|||||||
@@ -19,15 +19,32 @@ func TestBuildZoneConfigPrimaryAlsoNotify(t *testing.T) {
|
|||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", []string{"10.42.2.6", "10.42.1.5"})
|
cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", []string{"10.42.2.6", "10.42.1.5"}, "externaldns-key")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("buildZoneConfig: %v", err)
|
t.Fatalf("buildZoneConfig: %v", err)
|
||||||
}
|
}
|
||||||
if !strings.Contains(cfg, "notify explicit") {
|
if !strings.Contains(cfg, "notify explicit") {
|
||||||
t.Errorf("expected notify explicit in %q", cfg)
|
t.Errorf("expected notify explicit in %q", cfg)
|
||||||
}
|
}
|
||||||
if !strings.Contains(cfg, "also-notify { 10.42.2.6; 10.42.1.5; }") {
|
// also-notify entries are keyed so secondaries can admit the NOTIFY by TSIG
|
||||||
t.Errorf("expected also-notify with the secondary IPs in %q", cfg)
|
// key (allow-notify { key ... }) instead of by (churning) pod IP.
|
||||||
|
if !strings.Contains(cfg, `also-notify { 10.42.2.6 key "externaldns-key"; 10.42.1.5 key "externaldns-key"; }`) {
|
||||||
|
t.Errorf("expected keyed also-notify with the secondary IPs in %q", cfg)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// also-notify entries carry no key when none is configured (unkeyed NOTIFY).
|
||||||
|
func TestBuildZoneConfigPrimaryAlsoNotifyUnkeyed(t *testing.T) {
|
||||||
|
r := &BindZoneReconciler{}
|
||||||
|
zone := &bindv1alpha1.BindZone{
|
||||||
|
Spec: bindv1alpha1.BindZoneSpec{ZoneName: "main.unkin.net", Type: bindv1alpha1.ZonePrimary},
|
||||||
|
}
|
||||||
|
cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", []string{"10.42.2.6"}, "")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("buildZoneConfig: %v", err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(cfg, "also-notify { 10.42.2.6; }") {
|
||||||
|
t.Errorf("expected unkeyed also-notify in %q", cfg)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -38,7 +55,7 @@ func TestBuildZoneConfigPrimaryNoNotifyTargets(t *testing.T) {
|
|||||||
Spec: bindv1alpha1.BindZoneSpec{ZoneName: "main.unkin.net", Type: bindv1alpha1.ZonePrimary},
|
Spec: bindv1alpha1.BindZoneSpec{ZoneName: "main.unkin.net", Type: bindv1alpha1.ZonePrimary},
|
||||||
}
|
}
|
||||||
|
|
||||||
cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", nil)
|
cfg, err := r.buildZoneConfig(context.Background(), zone, "transfer-key", nil, "externaldns-key")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("buildZoneConfig: %v", err)
|
t.Fatalf("buildZoneConfig: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,108 @@
|
|||||||
|
package controller
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
corev1 "k8s.io/api/core/v1"
|
||||||
|
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||||
|
"k8s.io/apimachinery/pkg/runtime"
|
||||||
|
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
|
||||||
|
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||||
|
"sigs.k8s.io/controller-runtime/pkg/client/fake"
|
||||||
|
|
||||||
|
bindv1alpha1 "git.unkin.net/unkin/bind-operator/api/v1alpha1"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestConfigHashIndependentOfPrimaryPodIP is the permanent regression guard for
|
||||||
|
// the v0.2.5 rolling-restart loop.
|
||||||
|
//
|
||||||
|
// v0.2.5 rendered an options-scope `allow-notify { <primaryPodIP>; ... }` into
|
||||||
|
// the (restart-scoped) named.conf. The config-hash annotation that rolls the
|
||||||
|
// StatefulSet is computed over that render (BindClusterReconciler.configHash), so
|
||||||
|
// a config change rolled the pods, the primary pod came back on a NEW IP, the
|
||||||
|
// operator re-rendered with the new IP, the hash changed, the pods rolled again,
|
||||||
|
// and so on — an infinite roll loop across every BindCluster.
|
||||||
|
//
|
||||||
|
// The fix removed all pod IPs from restart-scoped config (secondaries now admit
|
||||||
|
// intra-cluster NOTIFYs by TSIG key: `allow-notify { key "X"; }`). This test
|
||||||
|
// reconciles the ConfigMap with the primary pod on one IP, computes the hash,
|
||||||
|
// then does it again with the primary pod on a DIFFERENT IP, and asserts the
|
||||||
|
// hash is byte-identical. If any pod-IP dependency ever creeps back into
|
||||||
|
// restart-scoped config, this fails and the loop-class bug is caught.
|
||||||
|
func TestConfigHashIndependentOfPrimaryPodIP(t *testing.T) {
|
||||||
|
scheme := runtime.NewScheme()
|
||||||
|
if err := clientgoscheme.AddToScheme(scheme); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := bindv1alpha1.AddToScheme(scheme); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
const ns = "dns"
|
||||||
|
svcIP := "10.43.5.5" // stable primary Service ClusterIP (does NOT churn)
|
||||||
|
|
||||||
|
cluster := &bindv1alpha1.BindCluster{
|
||||||
|
ObjectMeta: metav1.ObjectMeta{Name: "auth", Namespace: ns},
|
||||||
|
Spec: bindv1alpha1.BindClusterSpec{
|
||||||
|
Mode: bindv1alpha1.ModeAuthoritative,
|
||||||
|
Replicas: 3,
|
||||||
|
PrimaryService: &bindv1alpha1.ClusterServiceSpec{},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
primarySvc := &corev1.Service{
|
||||||
|
ObjectMeta: metav1.ObjectMeta{Name: primaryServiceName(cluster.Name), Namespace: ns},
|
||||||
|
Spec: corev1.ServiceSpec{ClusterIP: svcIP},
|
||||||
|
}
|
||||||
|
catalog := &bindv1alpha1.BindCatalogZone{
|
||||||
|
ObjectMeta: metav1.ObjectMeta{Name: "cat", Namespace: ns},
|
||||||
|
Spec: bindv1alpha1.BindCatalogZoneSpec{
|
||||||
|
ClusterRef: cluster.Name,
|
||||||
|
ZoneName: "catalog.internal",
|
||||||
|
TransferKeyRef: "externaldns-key",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
tsig := &bindv1alpha1.BindTSIGKey{
|
||||||
|
ObjectMeta: metav1.ObjectMeta{Name: "externaldns-key", Namespace: ns},
|
||||||
|
Spec: bindv1alpha1.BindTSIGKeySpec{ClusterRef: cluster.Name},
|
||||||
|
}
|
||||||
|
|
||||||
|
// hashWithPrimaryIP reconciles the ConfigMap with the primary pod carrying the
|
||||||
|
// given IP, then returns the resulting config-hash.
|
||||||
|
hashWithPrimaryIP := func(ip string) string {
|
||||||
|
primaryPod := &corev1.Pod{
|
||||||
|
ObjectMeta: metav1.ObjectMeta{Name: primaryPodName(cluster.Name), Namespace: ns, Labels: commonLabels(cluster.Name)},
|
||||||
|
Status: corev1.PodStatus{PodIP: ip},
|
||||||
|
}
|
||||||
|
c := fake.NewClientBuilder().
|
||||||
|
WithScheme(scheme).
|
||||||
|
WithObjects(cluster, primarySvc, catalog, tsig, primaryPod).
|
||||||
|
Build()
|
||||||
|
r := &BindClusterReconciler{Client: c, Scheme: scheme}
|
||||||
|
ctx := context.Background()
|
||||||
|
if err := r.reconcileConfigMap(ctx, cluster); err != nil {
|
||||||
|
t.Fatalf("reconcileConfigMap: %v", err)
|
||||||
|
}
|
||||||
|
// Sanity: the pod IP must not have leaked into the rendered config.
|
||||||
|
var cm corev1.ConfigMap
|
||||||
|
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: configMapName(cluster.Name)}, &cm); err != nil {
|
||||||
|
t.Fatalf("get configmap: %v", err)
|
||||||
|
}
|
||||||
|
for k, v := range cm.Data {
|
||||||
|
if ip != "" && strings.Contains(v, ip) {
|
||||||
|
t.Fatalf("primary pod IP %s leaked into restart-scoped config %s:\n%s", ip, k, v)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return r.configHash(ctx, cluster)
|
||||||
|
}
|
||||||
|
|
||||||
|
h1 := hashWithPrimaryIP("10.42.3.197")
|
||||||
|
h2 := hashWithPrimaryIP("10.42.9.42") // primary pod restarted onto a new IP
|
||||||
|
if h1 == "" {
|
||||||
|
t.Fatal("config hash should not be empty")
|
||||||
|
}
|
||||||
|
if h1 != h2 {
|
||||||
|
t.Fatalf("config hash changed when only the primary pod IP changed — the v0.2.5 roll loop:\n%s\n%s", h1, h2)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -2,6 +2,7 @@ package controller
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||||
@@ -58,12 +59,19 @@ func recordsToUpdates(zone string, records []bindv1alpha1.Record, defaultTTL int
|
|||||||
// updateKeyName returns the TSIG key name (as used in named.conf) for a zone's
|
// updateKeyName returns the TSIG key name (as used in named.conf) for a zone's
|
||||||
// update key, falling back to the object name.
|
// update key, falling back to the object name.
|
||||||
func updateKeyName(ctx context.Context, c client.Client, zone *bindv1alpha1.BindZone) string {
|
func updateKeyName(ctx context.Context, c client.Client, zone *bindv1alpha1.BindZone) string {
|
||||||
ref := zone.Spec.UpdateKeyRef
|
return tsigKeyName(ctx, c, zone.Namespace, zone.Spec.UpdateKeyRef)
|
||||||
|
}
|
||||||
|
|
||||||
|
// tsigKeyName resolves a BindTSIGKey object reference to the TSIG key name used
|
||||||
|
// in named.conf (the KeyName override when set, otherwise the object name).
|
||||||
|
// Returns "" for an empty ref, and falls back to the ref if the object cannot be
|
||||||
|
// read.
|
||||||
|
func tsigKeyName(ctx context.Context, c client.Client, namespace, ref string) string {
|
||||||
if ref == "" {
|
if ref == "" {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
var key bindv1alpha1.BindTSIGKey
|
var key bindv1alpha1.BindTSIGKey
|
||||||
if err := c.Get(ctx, client.ObjectKey{Namespace: zone.Namespace, Name: ref}, &key); err != nil {
|
if err := c.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ref}, &key); err != nil {
|
||||||
return ref
|
return ref
|
||||||
}
|
}
|
||||||
if key.Spec.KeyName != "" {
|
if key.Spec.KeyName != "" {
|
||||||
@@ -86,3 +94,23 @@ func terminateInline(entries []string) string {
|
|||||||
}
|
}
|
||||||
return strings.Join(parts, " ")
|
return strings.Join(parts, " ")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// alsoNotifyList renders also-notify entries, each optionally annotated with a
|
||||||
|
// TSIG key so the primary signs its NOTIFYs and secondaries can accept them by
|
||||||
|
// key (`allow-notify { key ... }`) rather than by pod IP. An entry that already
|
||||||
|
// carries a `key` clause is left untouched.
|
||||||
|
func alsoNotifyList(addrs []string, key string) string {
|
||||||
|
key = strings.TrimSpace(key)
|
||||||
|
var parts []string
|
||||||
|
for _, a := range addrs {
|
||||||
|
a = strings.TrimSpace(strings.TrimRight(a, ";"))
|
||||||
|
if a == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if key != "" && !strings.Contains(a, " key ") {
|
||||||
|
a = fmt.Sprintf("%s key \"%s\"", a, key)
|
||||||
|
}
|
||||||
|
parts = append(parts, a+";")
|
||||||
|
}
|
||||||
|
return strings.Join(parts, " ")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user