diff --git a/connectivity/check/check.go b/connectivity/check/check.go index 25bdefc66c..a05347bd26 100644 --- a/connectivity/check/check.go +++ b/connectivity/check/check.go @@ -74,8 +74,11 @@ type Parameters struct { ConnDisruptTestRestartsPath string ConnDisruptTestXfrmErrorsPath string ConnDisruptDispatchInterval time.Duration - FlushCT bool - SecondaryNetworkIface string + + ExpectedDropReasons []string + + FlushCT bool + SecondaryNetworkIface string K8sVersion string HelmChartDirectory string diff --git a/connectivity/suite.go b/connectivity/suite.go index 49234627e8..e32d7bfa23 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -261,7 +261,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch } } - ct.NewTest("no-unexpected-packet-drops").WithScenarios(tests.NoUnexpectedPacketDrops()) + ct.NewTest("no-unexpected-packet-drops").WithScenarios(tests.NoUnexpectedPacketDrops(ct.Params().ExpectedDropReasons)) // Run all tests without any policies in place. noPoliciesScenarios := []check.Scenario{ diff --git a/connectivity/tests/errors.go b/connectivity/tests/errors.go index 8d5146e6b0..32d77168bd 100644 --- a/connectivity/tests/errors.go +++ b/connectivity/tests/errors.go @@ -5,6 +5,7 @@ package tests import ( "context" + "fmt" "strings" "time" @@ -76,11 +77,13 @@ func (n *noErrorsInLogs) Run(ctx context.Context, t *check.Test) { // NoUnexpectedPacketDrops checks whether there were no drops due to expected // packet drops. -func NoUnexpectedPacketDrops() check.Scenario { - return &noUnexpectedPacketDrops{} +func NoUnexpectedPacketDrops(expectedDrops []string) check.Scenario { + return &noUnexpectedPacketDrops{expectedDrops} } -type noUnexpectedPacketDrops struct{} +type noUnexpectedPacketDrops struct { + expectedDrops []string +} func (n *noUnexpectedPacketDrops) Name() string { return "no-unexpected-packet-drops" @@ -88,9 +91,11 @@ func (n *noUnexpectedPacketDrops) Name() string { func (n *noUnexpectedPacketDrops) Run(ctx context.Context, t *check.Test) { ct := t.Context() + + filter := computeExpectedDropReasons(defaults.ExpectedDropReasons, n.expectedDrops) cmd := []string{ "/bin/sh", "-c", - "cilium metrics list -o json | jq '.[] | select((.name == \"cilium_drop_count_total\") and (.labels.reason | IN(\"Policy denied\", \"Policy denied by denylist\") | not))'", + fmt.Sprintf("cilium metrics list -o json | jq '.[] | select((.name == \"cilium_drop_count_total\") and (.labels.reason | IN(%s) | not))'", filter), } for _, pod := range ct.CiliumPods() { @@ -104,7 +109,49 @@ func (n *noUnexpectedPacketDrops) Run(ctx context.Context, t *check.Test) { t.Fatalf("Found unexpected packet drops:\n%s", countStr) } } +} +func computeExpectedDropReasons(defaultReasons, inputReasons []string) string { + filter := "" + if len(inputReasons) > 0 { + // Build final list of drop reasons based on default reasons, added + // reasons (+ prefix), and removed reasons (- prefix). + addedAllDefaults := false + dropReasons := map[string]bool{} + for _, reason := range inputReasons { + if reason[0] == '+' || reason[0] == '-' { + if !addedAllDefaults { + for _, r := range defaultReasons { + dropReasons[r] = true + } + addedAllDefaults = true + } + } + switch reason[0] { + case '+': + dropReasons[reason[1:]] = true + case '-': + dropReasons[reason[1:]] = false + default: + dropReasons[reason] = true + } + } + + // Build output string in form of '"reason1", "reason2"'. + i := 0 + for reason, isIn := range dropReasons { + if !isIn { + continue + } + if i == 0 { + filter = fmt.Sprintf("%q", reason) + } else { + filter = fmt.Sprintf("%s, %q", filter, reason) + } + i++ + } + } + return filter } func (n *noErrorsInLogs) checkErrorsInLogs(logs string, t *check.Test) { diff --git a/connectivity/tests/errors_test.go b/connectivity/tests/errors_test.go new file mode 100644 index 0000000000..783b44c7e4 --- /dev/null +++ b/connectivity/tests/errors_test.go @@ -0,0 +1,66 @@ +package tests + +import ( + "fmt" + "strings" + "testing" +) + +func TestComputeExpectedDropReasons(t *testing.T) { + defaultReasons := []string{"reason0", "reason1"} + tests := []struct { + inputReasons []string + expectedReasons []string + }{ + // Empty list of reasons. + { + inputReasons: []string{}, + expectedReasons: []string{}, + }, + // Add a reason to default list. + { + inputReasons: []string{"+reason2"}, + expectedReasons: []string{"reason0", "reason1", "reason2"}, + }, + // Remove a reason from default list. + { + inputReasons: []string{"-reason1"}, + expectedReasons: []string{"reason0"}, + }, + // Add a reason then remove it. + { + inputReasons: []string{"+reason2", "-reason2"}, + expectedReasons: []string{"reason0", "reason1"}, + }, + // Remove a reason then add it back. + { + inputReasons: []string{"-reason1", "+reason1"}, + expectedReasons: []string{"reason0", "reason1"}, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("InputReasons: %v", test.inputReasons), func(t *testing.T) { + result := computeExpectedDropReasons(defaultReasons, test.inputReasons) + + // computeExpectedDropReasons doesn't guarantee the order of the + // emitted string so instead we check the length and that all + // expected elements are listed. + + for _, expReason := range test.expectedReasons { + if !strings.Contains(result, fmt.Sprintf("%q", expReason)) { + t.Errorf("Expected filter to contain %q, but got: %v", expReason, result) + } + } + + // We know the expected length because all our test reasons are the same length. + expectedLength := len(test.expectedReasons) * 9 + if len(test.expectedReasons) > 1 { + expectedLength += (len(test.expectedReasons) - 1) * 2 + } + if len(result) != expectedLength { + t.Errorf("Expected filter of length %d, but got: %v", expectedLength, result) + } + }) + } +} diff --git a/defaults/defaults.go b/defaults/defaults.go index 10cde1bcb6..70626a1d22 100644 --- a/defaults/defaults.go +++ b/defaults/defaults.go @@ -190,4 +190,9 @@ var ( "authentication.mutual.spire.install.agent.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].operator=NotIn", "authentication.mutual.spire.install.agent.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].values[0]=true", } + + ExpectedDropReasons = []string{ + "Policy denied", + "Policy denied by denylist", + } ) diff --git a/internal/cli/cmd/connectivity.go b/internal/cli/cmd/connectivity.go index 0e668cfe4a..e7811b98f2 100644 --- a/internal/cli/cmd/connectivity.go +++ b/internal/cli/cmd/connectivity.go @@ -190,6 +190,10 @@ func newCmdConnectivityTest(hooks Hooks) *cobra.Command { cmd.Flags().StringVar(¶ms.ConnDisruptTestRestartsPath, "conn-disrupt-test-restarts-path", "/tmp/cilium-conn-disrupt-restarts", "Conn disrupt test temporary result file (used internally)") cmd.Flags().StringVar(¶ms.ConnDisruptTestXfrmErrorsPath, "conn-disrupt-test-xfrm-errors-path", "/tmp/cilium-conn-disrupt-xfrm-errors", "Conn disrupt test temporary result file (used internally)") cmd.Flags().DurationVar(¶ms.ConnDisruptDispatchInterval, "conn-disrupt-dispatch-interval", 0, "TCP packet dispatch interval") + + cmd.Flags().StringSliceVar(¶ms.ExpectedDropReasons, "expected-drop-reasons", defaults.ExpectedDropReasons, "List of expected drop reasons") + cmd.Flags().MarkHidden("expected-drop-reasons") + cmd.Flags().BoolVar(¶ms.FlushCT, "flush-ct", false, "Flush conntrack of Cilium on each node") cmd.Flags().MarkHidden("flush-ct") cmd.Flags().StringVar(¶ms.SecondaryNetworkIface, "secondary-network-iface", "", "Secondary network iface name (e.g., to test NodePort BPF on multiple networks)")