connectivity: add sanity checks for encryption tests

Currently, encryption checks are run only when encryption (either
IPSec or WireGuard) is enabled. While this approach is sensible,
it also led to cases in which the tests were broken in certain
circumstances, and we didn't notice [1,2,3,4].

Let's instead always run these tests, asserting that unencrypted
packets are either seen or not based on the feature set. Hence,
we can immediately notice if there's a regression, and no
unencrypted packets are detected even if encryption is not enabled.

[1]: f512df4 ("connectivity: fix node-to-node encryption tests")
[2]: HEAD~3 ("connectivity: fix pod-to-pod encryption validation")
[3]: HEAD~2 ("connectivity: fix encryption validation when running in ENI mode")
[4]: HEAD~1 ("connectivity: fix encryption validation when host firewall is enabled")

Signed-off-by: Marco Iorio <[email protected]>

connectivity/suite.go

@@ -745,16 +745,18 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch
 			tests.OutsideToNodePort(),
 		)
 
+	// Encryption checks are always executed as a sanity check, asserting whether
+	// unencrypted packets shall, or shall not, be observed based on the feature set.
 	ct.NewTest("pod-to-pod-encryption").
-		WithFeatureRequirements(features.RequireEnabled(features.EncryptionPod)).
 		WithScenarios(
-			tests.PodToPodEncryption(),
+			tests.PodToPodEncryption(features.RequireEnabled(features.EncryptionPod)),
 		)
 	ct.NewTest("node-to-node-encryption").
-		WithFeatureRequirements(features.RequireEnabled(features.EncryptionPod),
-			features.RequireEnabled(features.EncryptionNode)).
 		WithScenarios(
-			tests.NodeToNodeEncryption(),
+			tests.NodeToNodeEncryption(
+				features.RequireEnabled(features.EncryptionPod),
+				features.RequireEnabled(features.EncryptionNode),
+			),
 		)
 
 	if ct.Params().IncludeUnsafeTests {

connectivity/tests/encryption.go

@@ -107,15 +107,17 @@ func getSourceAddressFilter(ctx context.Context, t *check.Test, client,
 // PodToPodEncryption is a test case which checks the following:
 //   - There is a connectivity between pods on different nodes when any
 //     encryption mode is on (either WireGuard or IPsec).
-//   - No unencrypted packet is leaked.
+//   - No unencrypted packet is leaked. As a sanity check, we additionally
+//     run the same test also when encryption is disabled, asserting that
+//     we effectively observe unencrypted packets.
 //
 // The checks are implemented by curl'ing a server pod from a client pod, and
 // then inspecting tcpdump captures from the client pod's node.
-func PodToPodEncryption() check.Scenario {
-	return &podToPodEncryption{}
+func PodToPodEncryption(reqs ...features.Requirement) check.Scenario {
+	return &podToPodEncryption{reqs}
 }
 
-type podToPodEncryption struct{}
+type podToPodEncryption struct{ reqs []features.Requirement }
 
 func (s *podToPodEncryption) Name() string {
 	return "pod-to-pod-encryption"
@@ -137,14 +139,20 @@ func (s *podToPodEncryption) Run(ctx context.Context, t *check.Test) {
 	// clientHost is a pod running on the same node as the client pod, just in
 	// the host netns.
 	clientHost := ct.HostNetNSPodsByNode()[client.Pod.Spec.NodeName]
+	assertNoLeaks, _ := ct.Features.MatchRequirements(s.reqs...)
+
+	if !assertNoLeaks {
+		t.Debugf("%s test running in sanity mode, expecting unencrypted packets", s.Name())
+	}
 
 	t.ForEachIPFamily(func(ipFam features.IPFamily) {
-		testNoTrafficLeak(ctx, t, s, client, &server, &clientHost, requestHTTP, ipFam)
+		testNoTrafficLeak(ctx, t, s, client, &server, &clientHost, requestHTTP, ipFam, assertNoLeaks)
 	})
 }
 
 func testNoTrafficLeak(ctx context.Context, t *check.Test, s check.Scenario,
 	client, server, clientHost *check.Pod, reqType requestType, ipFam features.IPFamily,
+	assertNoLeaks bool,
 ) {
 	dstAddr := server.Address(ipFam)
 	iface := getInterNodeIface(ctx, t, clientHost, ipFam, client.Address(ipFam), dstAddr)
@@ -233,7 +241,7 @@ func testNoTrafficLeak(ctx context.Context, t *check.Test, s check.Scenario,
 	if err != nil {
 		t.Fatalf("Failed to retrieve tcpdump pkt count: %s", err)
 	}
-	if !strings.HasPrefix(count.String(), "0 packets") {
+	if !strings.HasPrefix(count.String(), "0 packets") && assertNoLeaks {
 		t.Failf("Captured unencrypted pkt (count=%s)", strings.TrimRight(count.String(), "\n\r"))
 
 		// If debug mode is enabled, dump the captured pkts
@@ -246,6 +254,10 @@ func testNoTrafficLeak(ctx context.Context, t *check.Test, s check.Scenario,
 			t.Debugf("Captured pkts:\n%s", out.String())
 		}
 	}
+
+	if strings.HasPrefix(count.String(), "0 packets") && !assertNoLeaks {
+		t.Failf("Expected to see unencrypted packets, but none found. This check might be broken")
+	}
 }
 
 // bytes.Buffer from the stdlib is non-thread safe, thus our custom
@@ -281,11 +293,11 @@ func (b *safeBuffer) ReadString(d byte) (string, error) {
 	return b.b.ReadString(d)
 }
 
-func NodeToNodeEncryption() check.Scenario {
-	return &nodeToNodeEncryption{}
+func NodeToNodeEncryption(reqs ...features.Requirement) check.Scenario {
+	return &nodeToNodeEncryption{reqs}
 }
 
-type nodeToNodeEncryption struct{}
+type nodeToNodeEncryption struct{ reqs []features.Requirement }
 
 func (s *nodeToNodeEncryption) Name() string {
 	return "node-to-node-encryption"
@@ -308,14 +320,19 @@ func (s *nodeToNodeEncryption) Run(ctx context.Context, t *check.Test) {
 	clientHost := t.Context().HostNetNSPodsByNode()[client.Pod.Spec.NodeName]
 	// serverHost is a pod running in a remote node's host netns.
 	serverHost := t.Context().HostNetNSPodsByNode()[server.Pod.Spec.NodeName]
+	assertNoLeaks, _ := t.Context().Features.MatchRequirements(s.reqs...)
+
+	if !assertNoLeaks {
+		t.Debugf("%s test running in sanity mode, expecting unencrypted packets", s.Name())
+	}
 
 	t.ForEachIPFamily(func(ipFam features.IPFamily) {
 		// Test pod-to-remote-host (ICMP Echo instead of HTTP because a remote host
 		// does not have a HTTP server running)
-		testNoTrafficLeak(ctx, t, s, client, &serverHost, &clientHost, requestICMPEcho, ipFam)
+		testNoTrafficLeak(ctx, t, s, client, &serverHost, &clientHost, requestICMPEcho, ipFam, assertNoLeaks)
 		// Test host-to-remote-host
-		testNoTrafficLeak(ctx, t, s, &clientHost, &serverHost, &clientHost, requestICMPEcho, ipFam)
+		testNoTrafficLeak(ctx, t, s, &clientHost, &serverHost, &clientHost, requestICMPEcho, ipFam, assertNoLeaks)
 		// Test host-to-remote-pod
-		testNoTrafficLeak(ctx, t, s, &clientHost, &server, &clientHost, requestHTTP, ipFam)
+		testNoTrafficLeak(ctx, t, s, &clientHost, &server, &clientHost, requestHTTP, ipFam, assertNoLeaks)
 	})
 }