Merge pull request #5429 from arkadeepsen/fix-dnsnameresolver-address-set

Fix dnsnameresolver address set

Author: trozet

.github/workflows/test.yml

@@ -454,14 +454,14 @@ jobs:
           - {"target": "shard-conformance", "ha": "HA", "gateway-mode": "local",  "ipfamily": "dualstack", "disable-snat-multiple-gws": "noSnatGW",   "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default"}
           - {"target": "shard-conformance", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv6",      "disable-snat-multiple-gws": "snatGW",   "second-bridge": "1br", "ic": "ic-single-node-zones"}
           - {"target": "shard-conformance", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4",      "disable-snat-multiple-gws": "snatGW",   "second-bridge": "1br", "ic": "ic-single-node-zones"}
-          - {"target": "control-plane",     "ha": "HA",   "gateway-mode": "shared", "ipfamily": "ipv6",      "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-disabled",          "dns-name-resolver": "enable-dns-name-resolver"}
+          - {"target": "control-plane",     "ha": "HA",   "gateway-mode": "shared", "ipfamily": "ipv6",      "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-disabled"}
           - {"target": "control-plane",     "ha": "HA",   "gateway-mode": "shared", "ipfamily": "ipv4",      "disable-snat-multiple-gws": "snatGW",   "second-bridge": "1br", "ic": "ic-disabled", "traffic-flow-tests": "1,2,3"}
           - {"target": "control-plane-helm","ha": "HA",   "gateway-mode": "shared", "ipfamily": "ipv4",      "disable-snat-multiple-gws": "snatGW",   "second-bridge": "1br", "ic": "ic-disabled", "dns-name-resolver": "enable-dns-name-resolver"}
           - {"target": "control-plane-helm","ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4",      "disable-snat-multiple-gws": "snatGW",   "second-bridge": "1br", "ic": "ic-single-node-zones", "dns-name-resolver": "enable-dns-name-resolver"}
           - {"target": "control-plane",     "ha": "noHA", "gateway-mode": "local",  "ipfamily": "ipv4",      "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "dns-name-resolver": "enable-dns-name-resolver"}
           - {"target": "control-plane",     "ha": "noHA", "gateway-mode": "local",  "ipfamily": "ipv6",      "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones"}
           - {"target": "control-plane",     "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4",      "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "2br", "ic": "ic-single-node-zones"}
-          - {"target": "control-plane",     "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv6",      "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "2br", "ic": "ic-single-node-zones", "dns-name-resolver": "enable-dns-name-resolver"}
+          - {"target": "control-plane",     "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv6",      "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "2br", "ic": "ic-single-node-zones"}
           - {"target": "multi-homing",      "ha": "noHA", "gateway-mode": "local",  "ipfamily": "ipv4",      "disable-snat-multiple-gws": "SnatGW",   "second-bridge": "1br", "ic": "ic-disabled"}
           - {"target": "multi-homing-helm", "ha": "HA",   "gateway-mode": "shared", "ipfamily": "ipv4",      "disable-snat-multiple-gws": "snatGW",   "second-bridge": "1br", "ic": "ic-disabled", "network-segmentation": "enable-network-segmentation"}
           - {"target": "node-ip-mac-migration", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv6",      "disable-snat-multiple-gws": "SnatGW",   "second-bridge": "1br", "ic": "ic-disabled"}
@@ -481,8 +481,8 @@ jobs:
           - {"target": "network-segmentation", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "dualstack", "disable-snat-multiple-gws": "SnatGW", "second-bridge": "1br", "ic": "ic-disabled"}
           - {"target": "network-segmentation", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones"}
           - {"target": "network-segmentation", "ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv6", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones"}
-          - {"target": "bgp", "ha": "noHA", "gateway-mode": "local",  "ipfamily": "dualstack", "disable-snat-multiple-gws": "snatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default", "network-segmentation": "enable-network-segmentation"}
-          - {"target": "bgp", "ha": "noHA", "gateway-mode": "shared",  "ipfamily": "dualstack", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default", "network-segmentation": "enable-network-segmentation"}
+          - {"target": "bgp", "ha": "noHA", "gateway-mode": "local",  "ipfamily": "dualstack", "disable-snat-multiple-gws": "snatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default", "network-segmentation": "enable-network-segmentation", "dns-name-resolver": "enable-dns-name-resolver"}
+          - {"target": "bgp", "ha": "noHA", "gateway-mode": "shared",  "ipfamily": "dualstack", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "routeadvertisements": "advertise-default", "network-segmentation": "enable-network-segmentation", "dns-name-resolver": "enable-dns-name-resolver"}
           - {"target": "traffic-flow-test-only","ha": "noHA", "gateway-mode": "shared", "ipfamily": "ipv4", "disable-snat-multiple-gws": "noSnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "traffic-flow-tests": "1-24", "network-segmentation": "enable-network-segmentation"}
           - {"target": "tools", "ha": "noHA", "gateway-mode": "local", "ipfamily": "dualstack", "disable-snat-multiple-gws": "SnatGW", "second-bridge": "1br", "ic": "ic-single-node-zones", "network-segmentation": "enable-network-segmentation"}
     needs: [ build-pr ]

go-controller/pkg/ovn/dns_name_resolver/external_dns.go

@@ -159,7 +159,7 @@ func (extEgDNS *ExternalEgressDNS) reconcileDNSNameResolver(key string) error {
 			addresses = append(addresses, resolvedAddress.IP)
 		}
 	}
-	err = extEgDNS.dnsTracker.addDNSName(dnsName, addresses)
+	err = extEgDNS.dnsTracker.addOrUpdateDNSName(dnsName, addresses)
 	return err
 }
 

go-controller/pkg/ovn/dns_name_resolver/external_dns_test.go

@@ -52,21 +52,15 @@ func newDNSNameResolverObject(name, namespace, dnsName string, addresses []strin
 }
 
 func expectDNSNameWithAddresses(extEgDNS *ExternalEgressDNS, dnsName string, expectedAddresses []string) {
-	var resolvedName *dnsResolvedName
-	err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 5*time.Minute, true, func(context.Context) (done bool, err error) {
-		var exists bool
-		resolvedName, exists = extEgDNS.getResolvedName(dnsName)
+	gomega.Eventually(func() []string {
+		resolvedName, exists := extEgDNS.getResolvedName(dnsName)
 		if !exists {
-			return false, nil
+			return []string{}
 		}
-
-		return true, nil
-	})
-	gomega.Expect(err).NotTo(gomega.HaveOccurred())
-
-	v4, v6 := resolvedName.dnsAddressSet.GetAddresses()
-	ipStrings := append(v4, v6...)
-	gomega.Expect(ipStrings).To(gomega.ConsistOf(expectedAddresses))
+		v4, v6 := resolvedName.dnsAddressSet.GetAddresses()
+		ipStrings := append(v4, v6...)
+		return ipStrings
+	}).Should(gomega.ConsistOf(expectedAddresses))
 }
 
 var _ = ginkgo.Describe("Egress Firewall External DNS Operations", func() {
@@ -227,6 +221,37 @@ var _ = ginkgo.Describe("Egress Firewall External DNS Operations", func() {
 		})
 	})
 
+	ginkgo.Context("on dns name resolver resource update", func() {
+		ginkgo.It("Should update addresses for a dns name", func() {
+			start()
+
+			config.IPv4Mode = true
+			config.IPv6Mode = true
+
+			addresses := []string{"1.1.1.1", "2.2.2.2", "2001:0db8:85a3:0000:0000:8a2e:0370:7334"}
+			dnsNameResolver := newDNSNameResolverObject("dns-default", config.Kubernetes.OVNConfigNamespace, dnsName, addresses)
+
+			_, err := fakeClient.OCPNetworkClient.NetworkV1alpha1().DNSNameResolvers(dnsNameResolver.Namespace).
+				Create(context.TODO(), dnsNameResolver, metav1.CreateOptions{})
+			gomega.Expect(err).NotTo(gomega.HaveOccurred())
+
+			expectDNSNameWithAddresses(extEgDNS, dnsName, addresses)
+
+			addresses = []string{"2.2.2.2", "3.3.3.3", "2001:0db8:85a3:0000:0000:8a2e:0370:7334", "2001:0db8:85a3:0000:0000:8a2e:0370:7335"}
+			var resolvedAddresses []ocpnetworkapiv1alpha1.DNSNameResolverResolvedAddress
+			for _, address := range addresses {
+				resolvedAddresses = append(resolvedAddresses, ocpnetworkapiv1alpha1.DNSNameResolverResolvedAddress{IP: address})
+			}
+			dnsNameResolver.Status.ResolvedNames[0].ResolvedAddresses = resolvedAddresses
+
+			_, err = fakeClient.OCPNetworkClient.NetworkV1alpha1().DNSNameResolvers(dnsNameResolver.Namespace).
+				Update(context.TODO(), dnsNameResolver, metav1.UpdateOptions{})
+			gomega.Expect(err).NotTo(gomega.HaveOccurred())
+
+			expectDNSNameWithAddresses(extEgDNS, dnsName, addresses)
+		})
+	})
+
 	ginkgo.It("Should not delete added addresses if DNS name is still used in a namespace", func() {
 		start()
 

go-controller/pkg/ovn/dns_name_resolver/external_dns_tracker.go

@@ -59,8 +59,8 @@ func newDNSTracker(addressSetFactory addressset.AddressSetFactory, controllerNam
 	}
 }
 
-// addDNSName is called whenever a DNS name is needed to be added or updated.
-func (dnsTracker *dnsTracker) addDNSName(dnsName string, addresses []string) error {
+// addOrUpdateDNSName is called whenever a DNS name is needed to be added or updated.
+func (dnsTracker *dnsTracker) addOrUpdateDNSName(dnsName string, addresses []string) error {
 	dnsTracker.dnsLock.Lock()
 	defer dnsTracker.dnsLock.Unlock()
 
@@ -92,7 +92,7 @@ func (dnsTracker *dnsTracker) addDNSName(dnsName string, addresses []string) err
 		addresses = filteredIPs
 	}
 
-	if err := resolvedName.dnsAddressSet.AddAddresses(addresses); err != nil {
+	if err := resolvedName.dnsAddressSet.SetAddresses(addresses); err != nil {
 		return fmt.Errorf("cannot add IPs to AddressSet for DNS name %s: %v", dnsName, err)
 	}
 

test/e2e/egress_firewall.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2ekubectl "k8s.io/kubernetes/test/e2e/framework/kubectl"
 	e2enode "k8s.io/kubernetes/test/e2e/framework/node"
+	e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
 	utilnet "k8s.io/utils/net"
 )
 
@@ -664,4 +665,103 @@ spec:
 		table.Entry("", false),
 		table.Entry("with chaos testing using many dnsNames", true),
 	)
+
+	ginkgo.Context("with DNS name resolver", func() {
+		ginkgo.BeforeEach(func() {
+			// DNS resolution for external DNS names does not work on IPv6 clusters. Skip
+			// the test if DNS name resolver is not enabled or IPv4 is not supported.
+			if !isDNSNameResolverEnabled() {
+				e2eskipper.Skipf("DNS name resolver is not enabled")
+				return
+			}
+			if !isIPv4Supported(f.ClientSet) {
+				e2eskipper.Skipf("IPv4 is not supported")
+				return
+			}
+		})
+
+		getMinTTLForDNSName := func(dnsName string, srcPodName string) int {
+			// Get the minimum TTL for the DNS name from the nslookup output.
+			// Ignore the error as it will always return an error because the
+			// cluster local DNS lookup will fail for the following DNS names:
+			// - <dnsName>.<namespace>.svc.<cluster-domain>
+			// - <dnsName>.svc.<cluster-domain>.
+			// - <dnsName>.<cluster-domain>
+			nslookupOutput, _ := e2ekubectl.RunKubectl(f.Namespace.Name, "exec", srcPodName, "--", "nslookup", "-debug", "-timeout=2", dnsName)
+			lines := strings.Split(nslookupOutput, "\n")
+			minTTL := -1
+			for i := 0; i < len(lines); i++ {
+				answerLine := strings.TrimSpace(lines[i])
+				// Skip lines until we find the answer line for the DNS name
+				if !strings.HasPrefix(answerLine, fmt.Sprintf("->  %s", dnsName)) {
+					continue
+				}
+				// Find the TTL line for the DNS name
+				for i++; i < len(lines); i++ {
+					ttlLine := strings.TrimSpace(lines[i])
+					if strings.HasPrefix(ttlLine, "ttl =") {
+						// Extract TTL value
+						ttlParts := strings.Split(ttlLine, "ttl =")
+						if len(ttlParts) == 2 {
+							ttlStr := strings.TrimSpace(ttlParts[1])
+							ttl, err := strconv.Atoi(ttlStr)
+							gomega.Expect(err).NotTo(gomega.HaveOccurred(), "failed to parse TTL value '%s': %v", ttlStr, err)
+
+							// Update minimum TTL
+							if minTTL == -1 || ttl < minTTL {
+								minTTL = ttl
+							}
+						}
+						break
+					}
+				}
+			}
+			return minTTL
+		}
+
+		ginkgo.It("Should validate that egressfirewall policy functionality for allowed DNS name", func() {
+			dnsName := "www.google.com"
+			srcPodName := "e2e-egress-fw-src-pod"
+
+			// egress firewall crd yaml configuration
+			var egressFirewallConfig = fmt.Sprintf(`kind: EgressFirewall
+apiVersion: k8s.ovn.org/v1
+metadata:
+  name: default
+  namespace: %s
+spec:
+  egress:
+  - type: Allow
+    to:
+      dnsName: %s
+  - type: Deny
+    to:
+      cidrSelector: %s
+`, f.Namespace.Name, dnsName, denyAllCIDR)
+			applyEF(egressFirewallConfig, f.Namespace.Name)
+
+			// create the pod that will be used as the source for the connectivity test
+			createSrcPod(srcPodName, serverNodeInfo.name, retryInterval, retryTimeout, f)
+
+			ginkgo.By(fmt.Sprintf("Verifying connectivity to DNS name %s is permitted", dnsName))
+			url := fmt.Sprintf("https://%s", dnsName)
+			_, err := e2ekubectl.RunKubectl(f.Namespace.Name, "exec", srcPodName, "--", "curl", "-g", "--max-time", "5", url)
+			framework.ExpectNoError(err, "failed to curl DNS name %s", dnsName)
+
+			ginkgo.By(fmt.Sprintf("Getting the minimum TTL for DNS name %s", dnsName))
+			minTTL := getMinTTLForDNSName(dnsName, srcPodName)
+			gomega.Expect(minTTL).NotTo(gomega.Equal(-1), "failed to parse nslookup output for DNS name %s", dnsName)
+			framework.Logf("Minimum TTL for DNS name %s is %d", dnsName, minTTL)
+
+			ginkgo.By(fmt.Sprintf("Waiting for the minimum TTL + 5 seconds for IP addresses of DNS name %s to be refreshed", dnsName))
+			time.Sleep(time.Duration(minTTL+5) * time.Second)
+
+			ginkgo.By(fmt.Sprintf("Verifying connectivity to DNS name %s is still permitted", dnsName))
+			_, err = e2ekubectl.RunKubectl(f.Namespace.Name, "exec", srcPodName, "--", "curl", "-g", "--max-time", "5", url)
+			framework.ExpectNoError(err, "failed to curl DNS name %s", dnsName)
+
+			framework.Logf("Deleting EgressFirewall in namespace %s", f.Namespace.Name)
+			e2ekubectl.RunKubectlOrDie(f.Namespace.Name, "delete", "egressfirewall", "default")
+		})
+	})
 })

test/e2e/util.go

@@ -1540,3 +1540,8 @@ func executeFileTemplate(templates *template.Template, directory, name string, d
 	}
 	return nil
 }
+
+func isDNSNameResolverEnabled() bool {
+	val, present := os.LookupEnv("OVN_ENABLE_DNSNAMERESOLVER")
+	return present && val == "true"
+}