From 68fb2dcbcab0c712dc9b8eb55f1f3d633ec01fcb Mon Sep 17 00:00:00 2001 From: Gilberto Bertin Date: Mon, 23 Oct 2023 10:10:01 +0200 Subject: [PATCH 1/5] check: add ExcludedCIDRsKind enum to better document how the ExcludedCIDRs param of CiliumEgressGatewayPolicyParams works Signed-off-by: Gilberto Bertin --- connectivity/check/test.go | 9 +++++++-- .../manifests/egress-gateway-policy-excluded-cidrs.yaml | 3 +-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/connectivity/check/test.go b/connectivity/check/test.go index 78d11955cb..18d09cef4d 100644 --- a/connectivity/check/test.go +++ b/connectivity/check/test.go @@ -460,8 +460,13 @@ func (t *Test) WithK8SPolicy(policy string) *Test { return t } +type ExcludedCIDRsKind int + const ( - NoExcludedCIDRs = iota + // NoExcludedCIDRs does not configure any excluded CIDRs in the policy + NoExcludedCIDRs ExcludedCIDRsKind = iota + + // ExternalNodeExcludedCIDRs adds the IPs of the external nodes (i.e the ones with the "cilium.io/no-schedule" label) to the list of excluded CIDRs ExternalNodeExcludedCIDRs ) @@ -469,7 +474,7 @@ const ( // before being applied. type CiliumEgressGatewayPolicyParams struct { // ExcludedCIDRs controls how the ExcludedCIDRs property should be configured - ExcludedCIDRs int + ExcludedCIDRs ExcludedCIDRsKind } // WithCiliumEgressGatewayPolicy takes a string containing a YAML policy diff --git a/connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml b/connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml index 3bdff0bff0..936a39a8db 100644 --- a/connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml +++ b/connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml @@ -10,8 +10,7 @@ spec: kind: client destinationCIDRs: - 0.0.0.0/0 - excludedCIDRs: - - NODE_WITHOUT_CILIUM_PLACEHOLDER/32 + excludedCIDRs: # set by the check package in WithCiliumEgressGatewayPolicy() egressGateway: nodeSelector: matchLabels: From 47772c46fc5924ca78ae26d2ceee7a31ef6a60cb Mon Sep 17 00:00:00 2001 From: Gilberto Bertin Date: Mon, 23 Oct 2023 10:34:41 +0200 Subject: [PATCH 2/5] check: allow to configure pod selector in WithCiliumEgressPolicy() add a new PodSelectorKind parameter to CiliumEgressGatewayPolicyParams, which allows to select the pod matching the "kind" label Signed-off-by: Gilberto Bertin --- connectivity/check/test.go | 12 +++++++++++ .../egress-gateway-policy-excluded-cidrs.yaml | 17 --------------- .../manifests/egress-gateway-policy.yaml | 21 +++---------------- connectivity/suite.go | 19 +++++++++++------ 4 files changed, 28 insertions(+), 41 deletions(-) delete mode 100644 connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml diff --git a/connectivity/check/test.go b/connectivity/check/test.go index 18d09cef4d..721ad26279 100644 --- a/connectivity/check/test.go +++ b/connectivity/check/test.go @@ -473,6 +473,12 @@ const ( // CiliumEgressGatewayPolicyParams is used to configure how a CiliumEgressGatewayPolicy template should be configured // before being applied. type CiliumEgressGatewayPolicyParams struct { + // Name controls the name of the policy + Name string + + // PodSelectorKind is used to select the client pods. The parameter is used to select pods with a matching "kind" label + PodSelectorKind string + // ExcludedCIDRs controls how the ExcludedCIDRs property should be configured ExcludedCIDRs ExcludedCIDRsKind } @@ -503,6 +509,12 @@ func (t *Test) WithCiliumEgressGatewayPolicy(policy string, params CiliumEgressG } } + // Set the policy name + pl[i].Name = params.Name + + // Set the pod selector + pl[i].Spec.Selectors[0].PodSelector.MatchLabels["kind"] = params.PodSelectorKind + // Set the egress gateway node egressGatewayNode := t.EgressGatewayNode() if egressGatewayNode == "" { diff --git a/connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml b/connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml deleted file mode 100644 index 936a39a8db..0000000000 --- a/connectivity/manifests/egress-gateway-policy-excluded-cidrs.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: cilium.io/v2 -kind: CiliumEgressGatewayPolicy -metadata: - name: cegp-sample-excluded-cidrs -spec: - selectors: - - podSelector: - matchLabels: - io.kubernetes.pod.namespace: cilium-test - kind: client - destinationCIDRs: - - 0.0.0.0/0 - excludedCIDRs: # set by the check package in WithCiliumEgressGatewayPolicy() - egressGateway: - nodeSelector: - matchLabels: - kubernetes.io/hostname: NODE_NAME_PLACEHOLDER diff --git a/connectivity/manifests/egress-gateway-policy.yaml b/connectivity/manifests/egress-gateway-policy.yaml index 6cc6f45a14..19dcd84ae8 100644 --- a/connectivity/manifests/egress-gateway-policy.yaml +++ b/connectivity/manifests/egress-gateway-policy.yaml @@ -1,31 +1,16 @@ apiVersion: cilium.io/v2 kind: CiliumEgressGatewayPolicy metadata: - name: cegp-sample + name: # set by the check package in WithCiliumEgressGatewayPolicy() spec: selectors: - podSelector: matchLabels: io.kubernetes.pod.namespace: cilium-test - kind: client - destinationCIDRs: - - 0.0.0.0/0 - egressGateway: - nodeSelector: - matchLabels: - kubernetes.io/hostname: NODE_NAME_PLACEHOLDER ---- -apiVersion: cilium.io/v2 -kind: CiliumEgressGatewayPolicy -metadata: - name: cegp-sample-echo-service -spec: - selectors: - - podSelector: - matchLabels: - kind: echo + kind: # set by the check package in WithCiliumEgressGatewayPolicy() destinationCIDRs: - 0.0.0.0/0 + excludedCIDRs: # set by the check package in WithCiliumEgressGatewayPolicy() egressGateway: nodeSelector: matchLabels: diff --git a/connectivity/suite.go b/connectivity/suite.go index 7f596f9e66..e671685bfd 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -188,9 +188,6 @@ var ( //go:embed manifests/egress-gateway-policy.yaml egressGatewayPolicyYAML string - - //go:embed manifests/egress-gateway-policy-excluded-cidrs.yaml - egressGatewayPolicyExcludedCIDRsYAML string ) var ( @@ -796,7 +793,14 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch if ct.Params().IncludeUnsafeTests { ct.NewTest("egress-gateway"). - WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML, check.CiliumEgressGatewayPolicyParams{}). + WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML, check.CiliumEgressGatewayPolicyParams{ + Name: "cegp-sample-client", + PodSelectorKind: "client", + }). + WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML, check.CiliumEgressGatewayPolicyParams{ + Name: "cegp-sample-echo", + PodSelectorKind: "echo", + }). WithIPRoutesFromOutsideToPodCIDRs(). WithFeatureRequirements(features.RequireEnabled(features.EgressGateway), features.RequireEnabled(features.NodeWithoutCilium)). @@ -807,8 +811,11 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch if versioncheck.MustCompile(">=1.14.0")(ct.CiliumVersion) { ct.NewTest("egress-gateway-excluded-cidrs"). - WithCiliumEgressGatewayPolicy(egressGatewayPolicyExcludedCIDRsYAML, - check.CiliumEgressGatewayPolicyParams{ExcludedCIDRs: check.ExternalNodeExcludedCIDRs}). + WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML, check.CiliumEgressGatewayPolicyParams{ + Name: "cegp-sample-client", + PodSelectorKind: "client", + ExcludedCIDRs: check.ExternalNodeExcludedCIDRs, + }). WithFeatureRequirements(features.RequireEnabled(features.EgressGateway), features.RequireEnabled(features.NodeWithoutCilium)). WithScenarios( From c58c85f2700ed0f4b50709b5dc0aebf333e9e8fe Mon Sep 17 00:00:00 2001 From: Gilberto Bertin Date: Mon, 23 Oct 2023 14:02:08 +0200 Subject: [PATCH 3/5] check: don't pass policy YAML to WithCiliumEgressPolicy() As CiliumEgressGatewayPolicyParams allows now to use a single policy template across all test scenarios, stop passing the same policy template to all invocations of WithCiliumEgressPolicy() Signed-off-by: Gilberto Bertin --- .../manifests/egress-gateway-policy.yaml | 0 connectivity/check/test.go | 7 +++++-- connectivity/suite.go | 9 +++----- connectivity/tests/egressgateway.go | 21 ++++++++++++------- 4 files changed, 21 insertions(+), 16 deletions(-) rename connectivity/{ => check}/manifests/egress-gateway-policy.yaml (100%) diff --git a/connectivity/manifests/egress-gateway-policy.yaml b/connectivity/check/manifests/egress-gateway-policy.yaml similarity index 100% rename from connectivity/manifests/egress-gateway-policy.yaml rename to connectivity/check/manifests/egress-gateway-policy.yaml diff --git a/connectivity/check/test.go b/connectivity/check/test.go index 721ad26279..955e646016 100644 --- a/connectivity/check/test.go +++ b/connectivity/check/test.go @@ -52,6 +52,9 @@ const ( var ( //go:embed assets/cacert.pem caBundle []byte + + //go:embed manifests/egress-gateway-policy.yaml + egressGatewayPolicyYAML string ) type Test struct { @@ -488,8 +491,8 @@ type CiliumEgressGatewayPolicyParams struct { // Test, to be applied when the test starts running. When calling this method, // note that the egress gateway enabled feature requirement is applied directly // here. -func (t *Test) WithCiliumEgressGatewayPolicy(policy string, params CiliumEgressGatewayPolicyParams) *Test { - pl, err := parseCiliumEgressGatewayPolicyYAML(policy) +func (t *Test) WithCiliumEgressGatewayPolicy(params CiliumEgressGatewayPolicyParams) *Test { + pl, err := parseCiliumEgressGatewayPolicyYAML(egressGatewayPolicyYAML) if err != nil { t.Fatalf("Parsing policy YAML: %s", err) } diff --git a/connectivity/suite.go b/connectivity/suite.go index e671685bfd..936a759cc6 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -185,9 +185,6 @@ var ( //go:embed manifests/echo-ingress-mutual-authentication.yaml echoIngressMutualAuthPolicyYAML string - - //go:embed manifests/egress-gateway-policy.yaml - egressGatewayPolicyYAML string ) var ( @@ -793,11 +790,11 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch if ct.Params().IncludeUnsafeTests { ct.NewTest("egress-gateway"). - WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML, check.CiliumEgressGatewayPolicyParams{ + WithCiliumEgressGatewayPolicy(check.CiliumEgressGatewayPolicyParams{ Name: "cegp-sample-client", PodSelectorKind: "client", }). - WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML, check.CiliumEgressGatewayPolicyParams{ + WithCiliumEgressGatewayPolicy(check.CiliumEgressGatewayPolicyParams{ Name: "cegp-sample-echo", PodSelectorKind: "echo", }). @@ -811,7 +808,7 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch if versioncheck.MustCompile(">=1.14.0")(ct.CiliumVersion) { ct.NewTest("egress-gateway-excluded-cidrs"). - WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML, check.CiliumEgressGatewayPolicyParams{ + WithCiliumEgressGatewayPolicy(check.CiliumEgressGatewayPolicyParams{ Name: "cegp-sample-client", PodSelectorKind: "client", ExcludedCIDRs: check.ExternalNodeExcludedCIDRs, diff --git a/connectivity/tests/egressgateway.go b/connectivity/tests/egressgateway.go index 1078569f9b..db174cf687 100644 --- a/connectivity/tests/egressgateway.go +++ b/connectivity/tests/egressgateway.go @@ -37,10 +37,11 @@ func (e *bpfEgressGatewayPolicyEntry) matches(t bpfEgressGatewayPolicyEntry) boo t.GatewayIP == e.GatewayIP } -// waitForBpfPolicyEntries waits for the egress gateway policy maps on each node to be populated with the entries for -// the cegp-sample CiliumEgressGatewayExcludedCIDRsPolicy +// waitForBpfPolicyEntries waits for the egress gateway policy maps on each node to be populated with the entries +// returned by the targetEntriesCallback func waitForBpfPolicyEntries(ctx context.Context, t *check.Test, - targetEntriesCallback func(ciliumPod check.Pod) []bpfEgressGatewayPolicyEntry) { + targetEntriesCallback func(ciliumPod check.Pod) []bpfEgressGatewayPolicyEntry, +) { ct := t.Context() w := wait.NewObserver(ctx, wait.Parameters{Timeout: 10 * time.Second}) @@ -132,12 +133,17 @@ func extractClientIPFromResponse(res string) net.IP { return net.ParseIP(clientIP.ClientIP).To4() } -// EgressGateway is a test case which, given the cegp-sample CiliumEgressGatewayPolicy targeting: +// EgressGateway is a test case which, given the cegp-sample-client CiliumEgressGatewayPolicy targeting: // - a couple of client pods (kind=client) as source // - the 0.0.0.0/0 destination CIDR // - kind-worker2 as gateway node // -// This suite tests connectivity for: +// and the cegp-sample-echo CiliumEgressGatewayPolicy targeting: +// - the echo service pods (kind=echo) as source +// - the 0.0.0.0/0 destination CIDR +// - kind-worker2 as gateway node +// +// tests connectivity for: // - pod to host traffic // - pod to service traffic // - pod to external IP traffic @@ -290,15 +296,14 @@ func (s *egressGateway) Run(ctx context.Context, t *check.Test) { } } -// EgressGatewayExcludedCIDRs is a test case which, given the cegp-sample-excluded-cidrs CiliumEgressGatewayPolicy +// EgressGatewayExcludedCIDRs is a test case which, given the cegp-sample CiliumEgressGatewayPolicy targeting: // targeting: // - a couple of client pods (kind=client) as source // - the 0.0.0.0/0 destination CIDR // - the IP of the external node as excluded CIDR // - kind-worker2 as gateway node // -// This suite tests tests the excludedCIDRs property and ensure traffic matching -// an excluded CIDR does not get masqueraded with the egress IP. +// This suite tests the excludedCIDRs property and ensure traffic matching an excluded CIDR does not get masqueraded with the egress IP func EgressGatewayExcludedCIDRs() check.Scenario { return &egressGatewayExcludedCIDRs{} } From 162da0a7b698cd7611581ffe02a427291a5ecaab Mon Sep 17 00:00:00 2001 From: Gilberto Bertin Date: Mon, 23 Oct 2023 14:12:27 +0200 Subject: [PATCH 4/5] tests: egressgw: export WatiForEgressGatewayBpfPolicyEntries and make it available to hooks Signed-off-by: Gilberto Bertin --- connectivity/tests/egressgateway.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/connectivity/tests/egressgateway.go b/connectivity/tests/egressgateway.go index db174cf687..eda587a0a9 100644 --- a/connectivity/tests/egressgateway.go +++ b/connectivity/tests/egressgateway.go @@ -37,9 +37,9 @@ func (e *bpfEgressGatewayPolicyEntry) matches(t bpfEgressGatewayPolicyEntry) boo t.GatewayIP == e.GatewayIP } -// waitForBpfPolicyEntries waits for the egress gateway policy maps on each node to be populated with the entries -// returned by the targetEntriesCallback -func waitForBpfPolicyEntries(ctx context.Context, t *check.Test, +// WaitForEgressGatewayBpfPolicyEntries waits for the egress gateway policy maps on each node to WaitForEgressGatewayBpfPolicyEntries +// with the entries returned by the targetEntriesCallback +func WaitForEgressGatewayBpfPolicyEntries(ctx context.Context, t *check.Test, targetEntriesCallback func(ciliumPod check.Pod) []bpfEgressGatewayPolicyEntry, ) { ct := t.Context() @@ -172,7 +172,7 @@ func (s *egressGateway) Run(ctx context.Context, t *check.Test) { t.Fatal("Cannot get egress gateway node internal IP") } - waitForBpfPolicyEntries(ctx, t, func(ciliumPod check.Pod) []bpfEgressGatewayPolicyEntry { + WaitForEgressGatewayBpfPolicyEntries(ctx, t, func(ciliumPod check.Pod) []bpfEgressGatewayPolicyEntry { targetEntries := []bpfEgressGatewayPolicyEntry{} egressIP := "0.0.0.0" @@ -327,7 +327,7 @@ func (s *egressGatewayExcludedCIDRs) Run(ctx context.Context, t *check.Test) { t.Fatal("Cannot get egress gateway node internal IP") } - waitForBpfPolicyEntries(ctx, t, func(ciliumPod check.Pod) []bpfEgressGatewayPolicyEntry { + WaitForEgressGatewayBpfPolicyEntries(ctx, t, func(ciliumPod check.Pod) []bpfEgressGatewayPolicyEntry { targetEntries := []bpfEgressGatewayPolicyEntry{} egressIP := "0.0.0.0" From 5a588ff30c30dfc7cd59f46536f711f4d4f7474e Mon Sep 17 00:00:00 2001 From: Gilberto Bertin Date: Fri, 5 Jan 2024 10:09:05 +0100 Subject: [PATCH 5/5] check: rename ExcludedCIDRs to ExcludedCIDRsConf rename the ExcludedCIDRs CiliumEgressGatewayPolicyParams field to ExcludedCIDRsConf to make it more explicit its use case Signed-off-by: Gilberto Bertin --- connectivity/check/test.go | 6 +++--- connectivity/suite.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/connectivity/check/test.go b/connectivity/check/test.go index 955e646016..957108799c 100644 --- a/connectivity/check/test.go +++ b/connectivity/check/test.go @@ -482,8 +482,8 @@ type CiliumEgressGatewayPolicyParams struct { // PodSelectorKind is used to select the client pods. The parameter is used to select pods with a matching "kind" label PodSelectorKind string - // ExcludedCIDRs controls how the ExcludedCIDRs property should be configured - ExcludedCIDRs ExcludedCIDRsKind + // ExcludedCIDRsConf controls how the ExcludedCIDRsConf property should be configured + ExcludedCIDRsConf ExcludedCIDRsKind } // WithCiliumEgressGatewayPolicy takes a string containing a YAML policy @@ -529,7 +529,7 @@ func (t *Test) WithCiliumEgressGatewayPolicy(params CiliumEgressGatewayPolicyPar // Set the excluded CIDRs pl[i].Spec.ExcludedCIDRs = []v2.IPv4CIDR{} - switch params.ExcludedCIDRs { + switch params.ExcludedCIDRsConf { case ExternalNodeExcludedCIDRs: for _, nodeWithoutCiliumIP := range t.Context().params.NodesWithoutCiliumIPs { if parsedIP := net.ParseIP(nodeWithoutCiliumIP.IP); parsedIP.To4() == nil { diff --git a/connectivity/suite.go b/connectivity/suite.go index 936a759cc6..1ccf0cdfa8 100644 --- a/connectivity/suite.go +++ b/connectivity/suite.go @@ -809,9 +809,9 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch if versioncheck.MustCompile(">=1.14.0")(ct.CiliumVersion) { ct.NewTest("egress-gateway-excluded-cidrs"). WithCiliumEgressGatewayPolicy(check.CiliumEgressGatewayPolicyParams{ - Name: "cegp-sample-client", - PodSelectorKind: "client", - ExcludedCIDRs: check.ExternalNodeExcludedCIDRs, + Name: "cegp-sample-client", + PodSelectorKind: "client", + ExcludedCIDRsConf: check.ExternalNodeExcludedCIDRs, }). WithFeatureRequirements(features.RequireEnabled(features.EgressGateway), features.RequireEnabled(features.NodeWithoutCilium)).