Merge pull request #25 from k11h-de/main

allow skiping ipv6 lookup by adding aaaa-lookups annotation

README.md

@@ -56,6 +56,8 @@ By default, the NetworkPolicy associated with a FQDNNetworkPolicy gets deleted w
 To prevent this behavior, set the `fqdnnetworkpolicies.networking.gke.io/delete-policy` annotation to `abandon` on the
 NetworkPolicy.
 
+You can disable AAAA lookups for an FQDNNetworkPolicy by setting the `fqdnnetworkpolicies.networking.gke.io/aaaa-lookups` annotation to `skip`. The resulting NetworkPolicy will not contain any IPv6 addresses.
+
 ## Limitations
 
 There are a few functional limitations to FQDNNetworkPolicies:

api/v1alpha3/fqdnnetworkpolicy_testing.go

@@ -65,6 +65,10 @@ func (r *FQDNNetworkPolicy) GetValidNonExistentFQDNResource() *FQDNNetworkPolicy
 	return r.LoadResource("./config/samples/networking_v1alpha3_fqdnnetworkpolicy_valid_nonexistentfqdn.yaml")
 }
 
+func (r *FQDNNetworkPolicy) GetValidAaaaLookupsSkippedResource() *FQDNNetworkPolicy {
+	return r.LoadResource("./config/samples/networking_v1alpha3_fqdnnetworkpolicy_valid_aaaalookupsskipped.yaml")
+}
+
 func (r *FQDNNetworkPolicy) GetInvalidResource() *FQDNNetworkPolicy {
 	return r.LoadResource("./config/samples/networking_v1alpha3_fqdnnetworkpolicy_invalid.yaml")
 }

config/samples/networking_v1alpha3_fqdnnetworkpolicy_valid_aaaalookupsskipped.yaml

@@ -0,0 +1,32 @@
+# Copyright 2022 Google LLC.
+#
+# 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.
+
+apiVersion: networking.gke.io/v1alpha3
+kind: FQDNNetworkPolicy
+metadata:
+  name: fqdnnetworkpolicy-valid
+  annotations:
+    fqdnnetworkpolicies.networking.gke.io/aaaa-lookups: "skip"
+spec:
+  podSelector: {}
+  policyTypes:
+  - Egress
+  egress:
+    - to:
+      - fqdns:
+        - github.com
+        - gitlab.com
+      ports:
+      - port: 443
+        protocol: TCP

controllers/fqdnnetworkpolicy_controller.go

@@ -45,6 +45,7 @@ type FQDNNetworkPolicyReconciler struct {
 var (
 	ownerAnnotation        = "fqdnnetworkpolicies.networking.gke.io/owned-by"
 	deletePolicyAnnotation = "fqdnnetworkpolicies.networking.gke.io/delete-policy"
+	aaaaLookupsAnnotation  = "fqdnnetworkpolicies.networking.gke.io/aaaa-lookups"
 	finalizerName          = "finalizer.fqdnnetworkpolicies.networking.gke.io"
 	// TODO make retry configurable
 	retry = time.Second * time.Duration(10)
@@ -465,36 +466,40 @@ func (r *FQDNNetworkPolicyReconciler) getNetworkPolicyEgressRules(ctx context.Co
 						}
 					}
 				}
-
-				// AAAA records
-				m6 := new(dns.Msg)
-				m6.SetQuestion(f, dns.TypeAAAA)
-
-				// TODO: We're always using the first nameserver. Should we do
-				// something different? Note from Jens:
-				// by default only if options rotate is set in resolv.conf
-				// they are rotated. Otherwise the first is used, after a (5s)
-				// timeout the next etc. So this is not too bad for now.
-				r6, _, err := c.Exchange(m6, "["+ns[0]+"]:53")
-				if err != nil {
-					log.Error(err, "unable to resolve "+f)
-					continue
-				}
-				if len(r6.Answer) == 0 {
-					log.V(1).Info("could not find AAAA record for " + f)
-				}
-				for _, ans := range r6.Answer {
-					if t, ok := ans.(*dns.AAAA); ok {
-						// Adding a peer per answer
-						peers = append(peers, networking.NetworkPolicyPeer{
-							IPBlock: &networking.IPBlock{CIDR: t.AAAA.String() + "/128"}})
-						// We want the next sync for the FQDNNetworkPolicy to happen
-						// just after the TTL of the DNS record has expired.
-						// Because a single FQDNNetworkPolicy may have different DNS
-						// records with different TTLs, we pick the lowest one
-						// and resynchronise after that.
-						if ans.Header().Ttl < nextSync {
-							nextSync = ans.Header().Ttl
+				// check for AAAA lookups skip annotation
+				if fqdnNetworkPolicy.Annotations[aaaaLookupsAnnotation] == "skip" {
+					log.Info("FQDNNetworkPolicy has AAAA lookups policy set to skip, not resolving AAAA records")
+				} else {
+					// AAAA records
+					m6 := new(dns.Msg)
+					m6.SetQuestion(f, dns.TypeAAAA)
+
+					// TODO: We're always using the first nameserver. Should we do
+					// something different? Note from Jens:
+					// by default only if options rotate is set in resolv.conf
+					// they are rotated. Otherwise the first is used, after a (5s)
+					// timeout the next etc. So this is not too bad for now.
+					r6, _, err := c.Exchange(m6, "["+ns[0]+"]:53")
+					if err != nil {
+						log.Error(err, "unable to resolve "+f)
+						continue
+					}
+					if len(r6.Answer) == 0 {
+						log.V(1).Info("could not find AAAA record for " + f)
+					}
+					for _, ans := range r6.Answer {
+						if t, ok := ans.(*dns.AAAA); ok {
+							// Adding a peer per answer
+							peers = append(peers, networking.NetworkPolicyPeer{
+								IPBlock: &networking.IPBlock{CIDR: t.AAAA.String() + "/128"}})
+							// We want the next sync for the FQDNNetworkPolicy to happen
+							// just after the TTL of the DNS record has expired.
+							// Because a single FQDNNetworkPolicy may have different DNS
+							// records with different TTLs, we pick the lowest one
+							// and resynchronise after that.
+							if ans.Header().Ttl < nextSync {
+								nextSync = ans.Header().Ttl
+							}
 						}
 					}
 				}

controllers/fqdnnetworkpolicy_controller_test.go

@@ -350,6 +350,84 @@ var _ = Describe("FQDNNetworkPolicy controller", func() {
 				Expect(k8sClient.Delete(ctx, &networkPolicy)).Should(Succeed())
 			})
 		})
+		Context("when the NetworkPolicy has the aaaa-lookups annotation set to skip", func() {
+			ctx := context.Background()
+			fqdnNetworkPolicy := getFQDNNetworkPolicy("context5", "default")
+			fqdnNetworkPolicy.GetValidAaaaLookupsSkippedResource()
+
+			nn := types.NamespacedName{
+				Namespace: fqdnNetworkPolicy.Namespace,
+				Name:      fqdnNetworkPolicy.Name,
+			}
+			It("Shouldn't lookup AAAA records", func() {
+				Expect(k8sClient.Create(ctx, &fqdnNetworkPolicy)).Should(Succeed())
+				Eventually(func() error {
+					networkPolicy := networking.NetworkPolicy{}
+					return k8sClient.Get(ctx, nn, &networkPolicy)
+				}).Should(Succeed())
+
+				// check only ipv4 adresses are present
+				Eventually(func() error {
+					r := net.Resolver{}
+					// computing the expected IPs in the NetworkPolicy
+					// from the FQDNs in the FQDNNetworkPolicy
+					// We use a different lib for resolving than the one in the main code
+					expectedIPs := []string{}
+					for _, fer := range fqdnNetworkPolicy.Spec.Egress {
+						for _, to := range fer.To {
+							for _, fqdn := range to.FQDNs {
+								ip4s, err := r.LookupIP(ctx, "ip4", fqdn)
+								if err != nil {
+									continuing := false
+									derr, ok := err.(*net.DNSError)
+									if ok && derr.IsNotFound {
+										continuing = true
+									}
+									aerr, ok := err.(*net.AddrError)
+									if ok && aerr.Err == "no suitable address found" {
+										continuing = true
+									}
+									if !continuing {
+										return err
+									}
+								}
+								for _, ip := range ip4s {
+									expectedIPs = append(expectedIPs, ip.String()+"/32")
+								}
+							}
+						}
+					}
+					// Getting the NetworkPolicy
+					networkPolicy := networking.NetworkPolicy{}
+					err := k8sClient.Get(ctx, nn, &networkPolicy)
+					if err != nil {
+						return err
+					}
+					if len(networkPolicy.Spec.PolicyTypes) != 1 ||
+						networkPolicy.Spec.PolicyTypes[0] != networking.PolicyTypeEgress {
+						return errors.New("Unexpected PolicyType: " + fmt.Sprintf("%v", networkPolicy.Spec.PolicyTypes) +
+							". Expected PolicyType: [Egress]")
+					}
+					total := 0
+					for _, egressRule := range networkPolicy.Spec.Egress {
+						// checking that every CIDR in the NetworkPolicy
+						// is in the expect list of IPs
+						total += len(egressRule.To)
+						for _, to := range egressRule.To {
+							// removing the /32 at the end of the CIDR
+							if !containsString(expectedIPs, string(to.IPBlock.CIDR)) {
+								return errors.New("Unexpected IP in NetworkPolicy: " + string(to.IPBlock.CIDR) +
+									". Expected IPs: " + fmt.Sprint(expectedIPs))
+							}
+						}
+					}
+					if total != len(expectedIPs) {
+						return errors.New("Some expected IPs are not present in the NetworkPolicy")
+					}
+					return nil
+				}).Should(Succeed())
+			})
+		})
 	})
 })