From e67f7c6be791217431c2b90bda58e8eb9de5e369 Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Thu, 18 May 2023 10:32:09 +0200 Subject: [PATCH] connectivity: Remove --datapath and add --include-unsafe-tests This commit does the following: * Moves the "--datapath" tests to the common suite, as those tests don't are not specific to the datapath. * Adds "--include-unsafe-tests" to allow users to skip tests which can modify cluster node state. * Protect some from CIDR tests with "--include-unsafe-tests", as those tests install IP routes on nodes w/o Cilium. Signed-off-by: Martynas Pumputis --- .github/workflows/kind.yaml | 5 +- connectivity/check/check.go | 2 +- connectivity/suite.go | 157 ++++++++++++++++--------------- internal/cli/cmd/connectivity.go | 4 +- 4 files changed, 86 insertions(+), 82 deletions(-) diff --git a/.github/workflows/kind.yaml b/.github/workflows/kind.yaml index 6550bdf891..b40cf40e70 100644 --- a/.github/workflows/kind.yaml +++ b/.github/workflows/kind.yaml @@ -100,6 +100,7 @@ jobs: run: | # Run the connectivity test in non-default namespace (i.e. not cilium-test) cilium connectivity test --debug --all-flows --test-namespace test-namespace \ + --include-unsafe-tests \ --collect-sysdump-on-failure --junit-file connectivity-${{ matrix.mode }}.xml - name: Upload junit output @@ -150,6 +151,7 @@ jobs: - name: Connectivity test run: | cilium connectivity test --debug --force-deploy --all-flows --test-namespace test-namespace \ + --include-unsafe-tests \ --collect-sysdump-on-failure --junit-file connectivity-ipsec-${{ matrix.mode }}.xml - name: Upload junit output @@ -300,7 +302,8 @@ jobs: - name: Run the multicluster connectivity tests run: | cilium connectivity test --context $CLUSTER1 --multi-cluster $CLUSTER2 --debug \ - --collect-sysdump-on-failure --junit-file connectivity-clustermesh.xml + --include-unsafe-tests \ + --collect-sysdump-on-failure --junit-file connectivity-clustermesh.xml - name: Upload junit output if: ${{ always() }} diff --git a/connectivity/check/check.go b/connectivity/check/check.go index d391ab16f3..1980f9f1bb 100644 --- a/connectivity/check/check.go +++ b/connectivity/check/check.go @@ -48,7 +48,7 @@ type Parameters struct { JSONMockImage string AgentDaemonSetName string DNSTestServerImage string - Datapath bool + IncludeUnsafeTests bool AgentPodSelector string NodeSelector map[string]string ExternalTarget string diff --git a/connectivity/suite.go b/connectivity/suite.go index 3516c5f8b7..ca80e183cb 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -200,43 +200,6 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { return ct.Run(ctx) } - // Datapath Conformance Tests - if ct.Params().Datapath { - ct.NewTest("north-south-loadbalancing"). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). - WithScenarios( - tests.OutsideToNodePort(), - ) - ct.NewTest("north-south-loadbalancing-with-l7-policy"). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). - WithCiliumVersion(">1.13.2"). - WithCiliumPolicy(echoIngressL7HTTPFromAnywherePolicyYAML). - WithScenarios( - tests.OutsideToNodePort(), - ) - ct.NewTest("pod-to-pod-encryption"). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod)). - WithScenarios( - tests.PodToPodEncryption(), - ) - ct.NewTest("node-to-node-encryption"). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod), - check.RequireFeatureEnabled(check.FeatureEncryptionNode)). - WithScenarios( - tests.NodeToNodeEncryption(), - ) - - ct.NewTest("egress-gateway"). - WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEgressGateway), - check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). - WithScenarios( - tests.EgressGateway(), - ) - - return ct.Run(ctx) - } - // Run all tests without any policies in place. noPoliciesScenarios := []check.Scenario{ tests.PodToPod(), @@ -251,11 +214,12 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { ct.NewTest("no-policies").WithScenarios(noPoliciesScenarios...) - // TODO(brb) --include-unsafe-tests - ct.NewTest("no-policies-from-outside"). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). - WithIPRoutesFromOutsideToPodCIDRs(). - WithScenarios(tests.FromCIDRToPod()) + if ct.Params().IncludeUnsafeTests { + ct.NewTest("no-policies-from-outside"). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). + WithIPRoutesFromOutsideToPodCIDRs(). + WithScenarios(tests.FromCIDRToPod()) + } // Skip the nodeport-related tests in the multicluster scenario if KPR is not // enabled, since global nodeport services are not supported in that case. @@ -328,18 +292,19 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { return check.ResultDrop, check.ResultDefaultDenyIngressDrop }) - // TODO(brb) --include-unsafe-tests - ct.NewTest("all-ingress-deny-from-outside").WithCiliumPolicy(denyAllIngressPolicyYAML). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). - WithIPRoutesFromOutsideToPodCIDRs(). - WithScenarios(tests.FromCIDRToPod()). - WithExpectations(func(a *check.Action) (egress, ingress check.Result) { - if a.Destination().Address(check.GetIPFamily(ct.Params().ExternalOtherIP)) == ct.Params().ExternalOtherIP || - a.Destination().Address(check.GetIPFamily(ct.Params().ExternalIP)) == ct.Params().ExternalIP { - return check.ResultOK, check.ResultNone - } - return check.ResultDrop, check.ResultDefaultDenyIngressDrop - }) + if ct.Params().IncludeUnsafeTests { + ct.NewTest("all-ingress-deny-from-outside").WithCiliumPolicy(denyAllIngressPolicyYAML). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). + WithIPRoutesFromOutsideToPodCIDRs(). + WithScenarios(tests.FromCIDRToPod()). + WithExpectations(func(a *check.Action) (egress, ingress check.Result) { + if a.Destination().Address(check.GetIPFamily(ct.Params().ExternalOtherIP)) == ct.Params().ExternalOtherIP || + a.Destination().Address(check.GetIPFamily(ct.Params().ExternalIP)) == ct.Params().ExternalIP { + return check.ResultOK, check.ResultNone + } + return check.ResultDrop, check.ResultDefaultDenyIngressDrop + }) + } // This policy denies all ingresses by default ct.NewTest("all-ingress-deny-knp").WithK8SPolicy(denyAllIngressPolicyKNPYAML). @@ -433,19 +398,20 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { return check.ResultOK, check.ResultOK }) - // TODO(brb) --include-unsafe-tests - ct.NewTest("echo-ingress-from-outside").WithCiliumPolicy(echoIngressFromOtherClientPolicyYAML). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). - WithIPRoutesFromOutsideToPodCIDRs(). - WithScenarios(tests.FromCIDRToPod()). - WithExpectations(func(a *check.Action) (egress, ingress check.Result) { - if a.Destination().HasLabel("kind", "echo") && !a.Source().HasLabel("other", "client") { - // TCP handshake fails both in egress and ingress when - // L3(/L4) policy drops at either location. - return check.ResultDropCurlTimeout, check.ResultDropCurlTimeout - } - return check.ResultOK, check.ResultOK - }) + if ct.Params().IncludeUnsafeTests { + ct.NewTest("echo-ingress-from-outside").WithCiliumPolicy(echoIngressFromOtherClientPolicyYAML). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). + WithIPRoutesFromOutsideToPodCIDRs(). + WithScenarios(tests.FromCIDRToPod()). + WithExpectations(func(a *check.Action) (egress, ingress check.Result) { + if a.Destination().HasLabel("kind", "echo") && !a.Source().HasLabel("other", "client") { + // TCP handshake fails both in egress and ingress when + // L3(/L4) policy drops at either location. + return check.ResultDropCurlTimeout, check.ResultDropCurlTimeout + } + return check.ResultOK, check.ResultOK + }) + } // This k8s policy allows ingress to echo only from client with a label 'other:client'. ct.NewTest("echo-ingress-knp").WithK8SPolicy(echoIngressFromOtherClientPolicyKNPYAML). @@ -560,17 +526,18 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { return check.ResultOK, check.ResultNone }) - // TODO(brb) --include-unsafe-tests - ct.NewTest("from-cidr-host-netns"). - WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). - WithCiliumPolicy(renderedTemplates["echoIngressFromCIDRYAML"]). - WithIPRoutesFromOutsideToPodCIDRs(). - WithScenarios( - tests.FromCIDRToPod(), - ). - WithExpectations(func(a *check.Action) (egress, ingress check.Result) { - return check.ResultOK, check.ResultNone - }) + if ct.Params().IncludeUnsafeTests { + ct.NewTest("from-cidr-host-netns"). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). + WithCiliumPolicy(renderedTemplates["echoIngressFromCIDRYAML"]). + WithIPRoutesFromOutsideToPodCIDRs(). + WithScenarios( + tests.FromCIDRToPod(), + ). + WithExpectations(func(a *check.Action) (egress, ingress check.Result) { + return check.ResultOK, check.ResultNone + }) + } // Tests with deny policy ct.NewTest("echo-ingress-from-other-client-deny"). @@ -731,8 +698,42 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureHealthChecking)). WithScenarios(tests.CiliumHealth()) + ct.NewTest("north-south-loadbalancing"). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). + WithScenarios( + tests.OutsideToNodePort(), + ) + + ct.NewTest("pod-to-pod-encryption"). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod)). + WithScenarios( + tests.PodToPodEncryption(), + ) + ct.NewTest("node-to-node-encryption"). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod), + check.RequireFeatureEnabled(check.FeatureEncryptionNode)). + WithScenarios( + tests.NodeToNodeEncryption(), + ) + + ct.NewTest("egress-gateway"). + WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEgressGateway), + check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). + WithScenarios( + tests.EgressGateway(), + ) + // The following tests have DNS redirect policies. They should be executed last. + ct.NewTest("north-south-loadbalancing-with-l7-policy"). + WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)). + WithCiliumVersion(">1.13.2"). + WithCiliumPolicy(echoIngressL7HTTPFromAnywherePolicyYAML). + WithScenarios( + tests.OutsideToNodePort(), + ) + // Test L7 HTTP introspection using an ingress policy on echo pods. ct.NewTest("echo-ingress-l7"). WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureL7Proxy)). diff --git a/internal/cli/cmd/connectivity.go b/internal/cli/cmd/connectivity.go index e08646c3e0..c79577738e 100644 --- a/internal/cli/cmd/connectivity.go +++ b/internal/cli/cmd/connectivity.go @@ -136,8 +136,8 @@ func newCmdConnectivityTest() *cobra.Command { cmd.Flags().StringVar(¶ms.JunitFile, "junit-file", "", "Generate junit report and write to file") cmd.Flags().BoolVar(¶ms.SkipIPCacheCheck, "skip-ip-cache-check", true, "Skip IPCache check") cmd.Flags().MarkHidden("skip-ip-cache-check") - cmd.Flags().BoolVar(¶ms.Datapath, "datapath", false, "Run datapath conformance tests") - cmd.Flags().MarkHidden("datapath") + cmd.Flags().BoolVar(¶ms.IncludeUnsafeTests, "include-unsafe-tests", false, "Include tests which can modify cluster nodes state") + cmd.Flags().MarkHidden("include-unsafe-tests") cmd.Flags().StringVar(¶ms.K8sVersion, "k8s-version", "", "Kubernetes server version in case auto-detection fails") cmd.Flags().StringVar(¶ms.HelmChartDirectory, "chart-directory", "", "Helm chart directory")