From ccfeae59be318598b25932fd931539aa24e4f6fb Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Tue, 16 May 2023 07:49:58 +0200 Subject: [PATCH] WIP: omg Signed-off-by: Martynas Pumputis --- connectivity/check/check.go | 14 +++- connectivity/check/context.go | 59 ++++++++++------- connectivity/check/test.go | 21 ++++++ .../manifests/echo-ingress-from-cidr.yaml | 4 +- connectivity/suite.go | 64 +++++++++++++------ 5 files changed, 115 insertions(+), 47 deletions(-) diff --git a/connectivity/check/check.go b/connectivity/check/check.go index 2adc07c3e6..003f6eb6e2 100644 --- a/connectivity/check/check.go +++ b/connectivity/check/check.go @@ -55,8 +55,8 @@ type Parameters struct { ExternalCIDR string ExternalIP string ExternalOtherIP string - ExternalFromCIDRs []string - ExternalFromCIDRMasks []int // Derived from ExternalFromCIDRs + PodCIDRs []podCIDRs + NodesWithoutCiliumIPs []nodesWithoutCiliumIP JunitFile string K8sVersion string @@ -221,3 +221,13 @@ func (d *deploymentClients) clients() []*k8s.Client { } return []*k8s.Client{d.src} } + +type podCIDRs struct { + CIDR string + HostIP string +} + +type nodesWithoutCiliumIP struct { + IP string + Mask int +} diff --git a/connectivity/check/context.go b/connectivity/check/context.go index 8588505748..fe8ff18020 100644 --- a/connectivity/check/context.go +++ b/connectivity/check/context.go @@ -293,8 +293,8 @@ func (ct *ConnectivityTest) SetupAndValidate(ctx context.Context) error { } } if ct.features.MatchRequirements(RequireFeatureEnabled(FeatureNodeWithoutCilium)) { - if err := ct.installStaticRoutesForNodesWithoutCilium(ctx); err != nil { - return fmt.Errorf("unable to install static routes: %w", err) + if err := ct.detectPodCIDRsAndNodesWithoutCiliumIPs(ctx); err != nil { + return fmt.Errorf("unable to detect pod CIDRs and nodes w/o Cilium IPs: %w", err) } } return nil @@ -529,7 +529,7 @@ func (ct *ConnectivityTest) enableHubbleClient(ctx context.Context) error { return nil } -func (ct *ConnectivityTest) installStaticRoutesForNodesWithoutCilium(ctx context.Context) error { +func (ct *ConnectivityTest) detectPodCIDRsAndNodesWithoutCiliumIPs(ctx context.Context) error { nodes, err := ct.client.ListNodes(ctx, metav1.ListOptions{}) if err != nil { return fmt.Errorf("failed to list nodes: %w", err) @@ -544,28 +544,39 @@ func (ct *ConnectivityTest) installStaticRoutesForNodesWithoutCilium(ctx context // mismatching the subnet. continue } - for _, withoutCilium := range ct.nodesWithoutCilium { - hostIP := ct.hostNetNSPodsByNode[n.Name].Pod.Status.HostIP - if n.Name == withoutCilium { - addr, err := netip.ParseAddr(hostIP) - if err != nil { - return fmt.Errorf("unable to parse external from cidr %q: %w", hostIP, err) - } - ct.params.ExternalFromCIDRs = append(ct.params.ExternalFromCIDRs, hostIP) - ct.params.ExternalFromCIDRMasks = append(ct.params.ExternalFromCIDRMasks, addr.BitLen()) - } + hostIP := ct.hostNetNSPodsByNode[n.Name].Pod.Status.HostIP + ct.params.PodCIDRs = append(ct.params.PodCIDRs, podCIDRs{cidr, hostIP}) + } + } - pod := ct.hostNetNSPodsByNode[withoutCilium] - _, err := ct.client.ExecInPod(ctx, pod.Pod.Namespace, pod.Pod.Name, hostNetNSDeploymentName, - []string{"ip", "route", "replace", cidr, "via", hostIP}, - ) - ct.Infof("Installing static route on nodes without Cilium (%v): %v", - withoutCilium, - []string{"ip", "route", "replace", cidr, "via", hostIP}, - ) - if err != nil { - return fmt.Errorf("failed to add static route: %w", err) - } + for _, n := range ct.nodesWithoutCilium { + pod := ct.hostNetNSPodsByNode[n] + for _, ip := range pod.Pod.Status.PodIPs { + hostIP, err := netip.ParseAddr(ip.IP) + if err != nil { + return fmt.Errorf("unable to parse nodes without Cilium IP addr %q: %w", ip.IP, err) + } + ct.params.NodesWithoutCiliumIPs = append(ct.params.NodesWithoutCiliumIPs, + nodesWithoutCiliumIP{ip.IP, hostIP.BitLen()}) + } + } + + return nil +} + +func (ct *ConnectivityTest) modifyStaticRoutesForNodesWithoutCilium(ctx context.Context, verb string) error { + for _, e := range ct.params.PodCIDRs { + for _, withoutCilium := range ct.nodesWithoutCilium { + pod := ct.hostNetNSPodsByNode[withoutCilium] + _, err := ct.client.ExecInPod(ctx, pod.Pod.Namespace, pod.Pod.Name, hostNetNSDeploymentName, + []string{"ip", "route", verb, e.CIDR, "via", e.HostIP}, + ) + ct.Infof("Modifying (%s) static route on nodes without Cilium (%v): %v", + verb, withoutCilium, + []string{"ip", "route", verb, e.CIDR, "via", e.HostIP}, + ) + if err != nil { + return fmt.Errorf("failed to %s static route: %w", verb, err) } } } diff --git a/connectivity/check/test.go b/connectivity/check/test.go index 5f1b28dba7..0b5541b451 100644 --- a/connectivity/check/test.go +++ b/connectivity/check/test.go @@ -68,6 +68,10 @@ type Test struct { // for this test to be run requirements []FeatureRequirement + // installIPRoutesFromOutsideToPodCIDRs indicates that the test runner needs + // to install podCIDR => nodeIP routes before running the test + installIPRoutesFromOutsideToPodCIDRs bool + // requiredCiliumVSN is a required Cilium VSN for this test to be run requiredCiliumVSN semver.Range @@ -161,6 +165,14 @@ func (t *Test) setup(ctx context.Context) error { return fmt.Errorf("applying network policies: %w", err) } + if err := t.Context().modifyStaticRoutesForNodesWithoutCilium(ctx, "add"); err != nil { + return fmt.Errorf("installing static routes: %w", err) + } else { + t.finalizers = append(t.finalizers, func() error { + return t.Context().modifyStaticRoutesForNodesWithoutCilium(ctx, "del") + }) + } + return nil } @@ -479,6 +491,15 @@ func (t *Test) WithFeatureRequirements(reqs ...FeatureRequirement) *Test { return t } +// WithIPRoutesFromOutsideToPodCIDRs instructs the test runner that +// podCIDR => nodeIP routes needs to be installed on a node which doesn't run +// Cilium before running the test (and removed after the test completion). +func (t *Test) WithIPRoutesFromOutsideToPodCIDRs() *Test { + t.installIPRoutesFromOutsideToPodCIDRs = true + + return t +} + // WithCiliumVersion adds a requirement for a Cilium vsn in order for the test // to run. func (t *Test) WithCiliumVersion(vsn string) *Test { diff --git a/connectivity/manifests/echo-ingress-from-cidr.yaml b/connectivity/manifests/echo-ingress-from-cidr.yaml index 933c0cec6a..a278b2b3a2 100644 --- a/connectivity/manifests/echo-ingress-from-cidr.yaml +++ b/connectivity/manifests/echo-ingress-from-cidr.yaml @@ -9,6 +9,6 @@ spec: kind: echo ingress: - fromCIDR: -{{ range $i, $cidr := .ExternalFromCIDRs }} - - {{$cidr}}/{{index $.ExternalFromCIDRMasks $i}} +{{ range $i := .NodesWithoutCiliumIPs }} + - {{$i.IP}}/{{$i.Mask}} {{ end }} diff --git a/connectivity/suite.go b/connectivity/suite.go index 0b694feb5f..bf4c7bd2f4 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -211,11 +211,15 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { tests.PodToExternalWorkload(), tests.PodToCIDR(tests.WithRetryAll()), } - if s, ok := ct.Feature(check.FeatureNodeWithoutCilium); ok && s.Enabled { - noPoliciesScenarios = append(noPoliciesScenarios, tests.FromCIDRToPod()) - } + 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()) + // Skip the nodeport-related tests in the multicluster scenario if KPR is not // enabled, since global nodeport services are not supported in that case. var reqs []check.FeatureRequirement @@ -277,9 +281,6 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { // then when replies come back, they are considered as "replies" to the outbound connection. // so they are not subject to ingress policy. allIngressDenyScenarios := []check.Scenario{tests.PodToPod(), tests.PodToCIDR(tests.WithRetryAll())} - if s, ok := ct.Feature(check.FeatureNodeWithoutCilium); ok && s.Enabled { - allIngressDenyScenarios = append(allIngressDenyScenarios, tests.FromCIDRToPod()) - } ct.NewTest("all-ingress-deny").WithCiliumPolicy(denyAllIngressPolicyYAML). WithScenarios(allIngressDenyScenarios...). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { @@ -290,6 +291,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 + }) + // This policy denies all ingresses by default ct.NewTest("all-ingress-deny-knp").WithK8SPolicy(denyAllIngressPolicyKNPYAML). WithScenarios( @@ -371,9 +385,6 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { // This policy allows ingress to echo only from client with a label 'other:client'. echoIngressScenarios := []check.Scenario{tests.PodToPod()} - if s, ok := ct.Feature(check.FeatureNodeWithoutCilium); ok && s.Enabled { - echoIngressScenarios = append(echoIngressScenarios, tests.FromCIDRToPod()) - } ct.NewTest("echo-ingress").WithCiliumPolicy(echoIngressFromOtherClientPolicyYAML). WithScenarios(echoIngressScenarios...). WithExpectations(func(a *check.Action) (egress, ingress check.Result) { @@ -385,6 +396,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 + }) + // This k8s policy allows ingress to echo only from client with a label 'other:client'. ct.NewTest("echo-ingress-knp").WithK8SPolicy(echoIngressFromOtherClientPolicyKNPYAML). WithScenarios( @@ -498,16 +523,17 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error { return check.ResultOK, check.ResultNone }) - if s, ok := ct.Feature(check.FeatureNodeWithoutCilium); ok && s.Enabled { - ct.NewTest("from-cidr-host-netns"). - WithCiliumPolicy(renderedTemplates["echoIngressFromCIDRYAML"]). - WithScenarios( - tests.FromCIDRToPod(), - ). - WithExpectations(func(a *check.Action) (egress, ingress check.Result) { - 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 + }) // Tests with deny policy ct.NewTest("echo-ingress-from-other-client-deny").