From 9b689e72777df8e9825819dfd22800208b913a0a Mon Sep 17 00:00:00 2001 From: Tam Mach Date: Fri, 24 Nov 2023 23:01:54 +1100 Subject: [PATCH] connectivity: Add more tests for Ingress Controller This commit is to cover the cases which the traffic is sent via external node client (i.e. from node without Cilium) to Ingress service. Signed-off-by: Tam Mach --- connectivity/check/context.go | 30 +++++++++------- connectivity/check/deployment.go | 42 ++++++++++++++++++++-- connectivity/suite.go | 60 ++++++++++++++++++++++++++++++++ connectivity/tests/service.go | 54 ++++++++++++++++++++++++++++ 4 files changed, 171 insertions(+), 15 deletions(-) diff --git a/connectivity/check/context.go b/connectivity/check/context.go index b88c0b3904..101bb79963 100644 --- a/connectivity/check/context.go +++ b/connectivity/check/context.go @@ -57,18 +57,19 @@ type ConnectivityTest struct { // Clients for source and destination clusters. clients *deploymentClients - ciliumPods map[string]Pod - echoPods map[string]Pod - echoExternalPods map[string]Pod - clientPods map[string]Pod - clientCPPods map[string]Pod - perfClientPods map[string]Pod - perfServerPod map[string]Pod - PerfResults map[PerfTests]PerfResult - echoServices map[string]Service - ingressService map[string]Service - k8sService Service - externalWorkloads map[string]ExternalWorkload + ciliumPods map[string]Pod + echoPods map[string]Pod + echoExternalPods map[string]Pod + clientPods map[string]Pod + clientCPPods map[string]Pod + clientExternalPods map[string]Pod + perfClientPods map[string]Pod + perfServerPod map[string]Pod + PerfResults map[PerfTests]PerfResult + echoServices map[string]Service + ingressService map[string]Service + k8sService Service + externalWorkloads map[string]ExternalWorkload hostNetNSPodsByNode map[string]Pod secondaryNetworkNodeIPv4 map[string]string // node name => secondary ip @@ -220,6 +221,7 @@ func NewConnectivityTest(client *k8s.Client, p Parameters, version string) (*Con echoExternalPods: make(map[string]Pod), clientPods: make(map[string]Pod), clientCPPods: make(map[string]Pod), + clientExternalPods: make(map[string]Pod), perfClientPods: make(map[string]Pod), perfServerPod: make(map[string]Pod), PerfResults: make(map[PerfTests]PerfResult), @@ -1074,6 +1076,10 @@ func (ct *ConnectivityTest) ControlPlaneClientPods() map[string]Pod { return ct.clientCPPods } +func (ct *ConnectivityTest) ExternalPodClientPods() map[string]Pod { + return ct.clientExternalPods +} + func (ct *ConnectivityTest) HostNetNSPodsByNode() map[string]Pod { return ct.hostNetNSPodsByNode } diff --git a/connectivity/check/deployment.go b/connectivity/check/deployment.go index a83cb70a11..8eed537ecd 100644 --- a/connectivity/check/deployment.go +++ b/connectivity/check/deployment.go @@ -32,9 +32,10 @@ const ( perfHostNetNamingSuffix = "-host-net" - clientDeploymentName = "client" - client2DeploymentName = "client2" - clientCPDeployment = "client-cp" + clientDeploymentName = "client" + client2DeploymentName = "client2" + clientCPDeployment = "client-cp" + clientExternalNodeDeployment = "client-external-node" DNSTestServerContainerName = "dns-test-server" @@ -1055,6 +1056,33 @@ func (ct *ConnectivityTest) deploy(ctx context.Context) error { }, } } + + _, err = ct.clients.src.GetDeployment(ctx, ct.params.TestNamespace, clientExternalNodeDeployment, metav1.GetOptions{}) + if err != nil { + ct.Logf("✨ [%s] Deploying %s deployment...", ct.clients.src.ClusterName(), clientExternalNodeDeployment) + clientDeployment := newDeployment(deploymentParameters{ + Name: clientExternalNodeDeployment, + Kind: kindClientName, + NamedPort: "http-8081", + Port: 8081, + Image: ct.params.CurlImage, + Command: []string{"/bin/ash", "-c", "sleep 10000000"}, + Annotations: ct.params.DeploymentAnnotations.Match(clientExternalNodeDeployment), + NodeSelector: map[string]string{"cilium.io/no-schedule": "true"}, + HostNetwork: true, + Tolerations: []corev1.Toleration{ + {Operator: corev1.TolerationOpExists}, + }, + }) + _, err = ct.clients.src.CreateServiceAccount(ctx, ct.params.TestNamespace, k8s.NewServiceAccount(clientExternalNodeDeployment), metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("unable to create service account %s: %s", clientExternalNodeDeployment, err) + } + _, err = ct.clients.src.CreateDeployment(ctx, ct.params.TestNamespace, clientDeployment, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("unable to create deployment %s: %s", clientExternalNodeDeployment, err) + } + } } return nil } @@ -1192,6 +1220,14 @@ func (ct *ConnectivityTest) validateDeployment(ctx context.Context) error { } for _, pod := range clientPods.Items { + if strings.Contains(pod.Name, clientExternalNodeDeployment) { + ct.clientExternalPods[pod.Name] = Pod{ + K8sClient: ct.client, + Pod: pod.DeepCopy(), + } + continue + } + if err := WaitForCiliumEndpoint(ctx, ct, ct.clients.src, ct.Params().TestNamespace, pod.Name); err != nil { return err } diff --git a/connectivity/suite.go b/connectivity/suite.go index 37bbcacf2f..a2190435f9 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -1039,6 +1039,14 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch tests.PodToIngress(), ) + ct.NewTest("external-node-client-to-ingress-service"). + WithFeatureRequirements( + features.RequireEnabled(features.IngressController), + features.RequireEnabled(features.NodeWithoutCilium)). + WithScenarios( + tests.ExternalNodePodToIngress(), + ) + ct.NewTest("pod-to-ingress-service-deny-all"). WithFeatureRequirements(features.RequireEnabled(features.IngressController)). WithCiliumPolicy(denyAllIngressPolicyYAML). @@ -1049,6 +1057,18 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch return check.ResultDefaultDenyEgressDrop, check.ResultNone }) + ct.NewTest("external-node-client-to-ingress-service-deny-all"). + WithFeatureRequirements( + features.RequireEnabled(features.IngressController), + features.RequireEnabled(features.NodeWithoutCilium)). + WithCiliumPolicy(denyAllIngressPolicyYAML). + WithScenarios( + tests.ExternalNodePodToIngress(), + ). + WithExpectations(func(a *check.Action) (egress check.Result, ingress check.Result) { + return check.ResultDefaultDenyEgressDrop, check.ResultNone + }) + ct.NewTest("pod-to-ingress-service-deny-ingress-identity"). WithFeatureRequirements(features.RequireEnabled(features.IngressController)). WithCiliumPolicy(denyIngressIdentityPolicyYAML). @@ -1059,6 +1079,21 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch return check.ResultDefaultDenyEgressDrop, check.ResultNone }) + ct.NewTest("external-node-client-to-ingress-service-deny-ingress-identity"). + WithFeatureRequirements( + features.RequireEnabled(features.IngressController), + features.RequireEnabled(features.NodeWithoutCilium)). + WithCiliumPolicy(denyIngressIdentityPolicyYAML). + WithScenarios( + tests.ExternalNodePodToIngress(), + ). + WithExpectations(func(a *check.Action) (egress check.Result, ingress check.Result) { + // TODO This should be denied after the below PR is backport + // https://github.com/cilium/cilium/pull/28126 + // return check.ResultDefaultDenyEgressDrop, check.ResultNone + return check.ResultOK, check.ResultOK + }) + ct.NewTest("pod-to-ingress-service-deny-backend-service"). WithFeatureRequirements(features.RequireEnabled(features.IngressController)). WithCiliumPolicy(denyIngressBackendPolicyYAML). @@ -1069,6 +1104,21 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch return check.ResultDefaultDenyEgressDrop, check.ResultNone }) + ct.NewTest("external-node-client-to-ingress-service-deny-backend-service"). + WithFeatureRequirements( + features.RequireEnabled(features.IngressController), + features.RequireEnabled(features.NodeWithoutCilium)). + WithCiliumPolicy(denyIngressBackendPolicyYAML). + WithScenarios( + tests.ExternalNodePodToIngress(), + ). + WithExpectations(func(a *check.Action) (egress check.Result, ingress check.Result) { + // TODO This should be denied after the below PR is backport + // https://github.com/cilium/cilium/pull/28126 + // return check.ResultDefaultDenyEgressDrop, check.ResultNone + return check.ResultOK, check.ResultOK + }) + ct.NewTest("pod-to-ingress-service-allow-ingress-identity"). WithFeatureRequirements(features.RequireEnabled(features.IngressController)). WithCiliumPolicy(denyAllIngressPolicyYAML). @@ -1077,6 +1127,16 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch tests.PodToIngress(), ) + ct.NewTest("external-node-client-to-ingress-service-allow-ingress-identity"). + WithFeatureRequirements( + features.RequireEnabled(features.IngressController), + features.RequireEnabled(features.NodeWithoutCilium)). + WithCiliumPolicy(denyAllIngressPolicyYAML). + WithCiliumPolicy(allowIngressIdentityPolicyYAML). + WithScenarios( + tests.ExternalNodePodToIngress(), + ) + // Only allow UDP:53 to kube-dns, no DNS proxy enabled. ct.NewTest("dns-only").WithCiliumPolicy(clientEgressOnlyDNSPolicyYAML). WithFeatureRequirements(features.RequireEnabled(features.L7Proxy)). diff --git a/connectivity/tests/service.go b/connectivity/tests/service.go index bc83ed46b3..06038479de 100644 --- a/connectivity/tests/service.go +++ b/connectivity/tests/service.go @@ -119,6 +119,60 @@ func (s *podToIngress) Run(ctx context.Context, t *check.Test) { } } +// ExternalNodePodToIngress sends an HTTP request from external node pods +// to all Ingress service in the test context. +func ExternalNodePodToIngress(opts ...Option) check.Scenario { + options := &labelsOption{} + for _, opt := range opts { + opt(options) + } + return &externalNodePodToIngress{ + sourceLabels: options.sourceLabels, + destinationLabels: options.destinationLabels, + } +} + +// externalNodePodToIngress implements a Scenario. +type externalNodePodToIngress struct { + sourceLabels map[string]string + destinationLabels map[string]string +} + +func (s *externalNodePodToIngress) Name() string { + return "pod-to-ingress-service" +} + +func (s *externalNodePodToIngress) Run(ctx context.Context, t *check.Test) { + var i int + ct := t.Context() + + for _, pod := range ct.ExternalPodClientPods() { + pod := pod // copy to avoid memory aliasing when using reference + if !hasAllLabels(pod, s.sourceLabels) { + continue + } + + for _, node := range ct.Nodes() { + for _, svc := range ct.IngressService() { + if !hasAllLabels(svc, s.destinationLabels) { + continue + } + + t.NewAction(s, fmt.Sprintf("curl-%d", i), &pod, svc, features.IPFamilyV4).Run(func(a *check.Action) { + a.ExecInPod(ctx, ct.CurlCommand(svc.ToNodeportService(node), features.IPFamilyV4)) + + a.ValidateFlows(ctx, pod, a.GetEgressRequirements(check.FlowParameters{ + DNSRequired: true, + AltDstPort: svc.Port(), + })) + }) + i++ + } + } + + } +} + // PodToRemoteNodePort sends an HTTP request from all client Pods // to all echo Services' NodePorts, but only to other nodes. func PodToRemoteNodePort() check.Scenario {