TestBuilder shouldRun method replaced with Test.WithConditionFn method.

Signed-off-by: viktor-kurchenko <[email protected]>
cilium · Mar 2, 2024 · 1666230 · 1666230
1 parent fc76422
commit 1666230
Show file tree

Hide file tree

Showing 80 changed files with 44 additions and 481 deletions.
diff --git a/connectivity/builder/all_egress_deny.go b/connectivity/builder/all_egress_deny.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var denyAllEgressPolicyYAML string
 
 type allEgressDeny struct{}
 
-func (t allEgressDeny) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t allEgressDeny) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy denies all egresses by default
 	newTest("all-egress-deny", ct).

diff --git a/connectivity/builder/all_egress_deny_knp.go b/connectivity/builder/all_egress_deny_knp.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var denyAllEgressPolicyKNPYAML string
 
 type allEgressDenyKnp struct{}
 
-func (t allEgressDenyKnp) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t allEgressDenyKnp) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy denies all egresses by default using KNP.
 	newTest("all-egress-deny-knp", ct).

diff --git a/connectivity/builder/all_entities_deny.go b/connectivity/builder/all_entities_deny.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var denyAllEntitiesPolicyYAML string
 
 type allEntitiesDeny struct{}
 
-func (t allEntitiesDeny) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t allEntitiesDeny) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy denies all entities by default
 	newTest("all-entities-deny", ct).

diff --git a/connectivity/builder/all_ingress_deny.go b/connectivity/builder/all_ingress_deny.go
@@ -4,19 +4,13 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 	"github.com/cilium/cilium-cli/utils/features"
 )
 
 type allIngressDeny struct{}
 
-func (t allIngressDeny) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t allIngressDeny) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy denies all ingresses by default.
 	//

diff --git a/connectivity/builder/all_ingress_deny_from_outside.go b/connectivity/builder/all_ingress_deny_from_outside.go
@@ -4,21 +4,16 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 	"github.com/cilium/cilium-cli/utils/features"
 )
 
 type allIngressDenyFromOutside struct{}
 
-func (t allIngressDenyFromOutside) shouldRun(_ semver.Version, params check.Parameters) bool {
-	return params.IncludeUnsafeTests
-}
-
 func (t allIngressDenyFromOutside) build(ct *check.ConnectivityTest, _ map[string]string) {
 	newTest("all-ingress-deny-from-outside", ct).
+		WithConditionFn(func() bool { return ct.Params().IncludeUnsafeTests }).
 		WithCiliumPolicy(denyAllIngressPolicyYAML).
 		WithFeatureRequirements(features.RequireEnabled(features.NodeWithoutCilium)).
 		WithIPRoutesFromOutsideToPodCIDRs().

diff --git a/connectivity/builder/all_ingress_deny_knp.go b/connectivity/builder/all_ingress_deny_knp.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 	"github.com/cilium/cilium-cli/utils/features"
@@ -18,10 +16,6 @@ var denyAllIngressPolicyKNPYAML string
 
 type allIngressDenyKnp struct{}
 
-func (t allIngressDenyKnp) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t allIngressDenyKnp) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy denies all ingresses by default
 	newTest("all-ingress-deny-knp", ct).

diff --git a/connectivity/builder/allow_all_except_world.go b/connectivity/builder/allow_all_except_world.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var allowAllExceptWorldPolicyYAML string
 
 type allowAllExceptWorld struct{}
 
-func (t allowAllExceptWorld) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t allowAllExceptWorld) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// Test with an allow-all-except-world (and unmanaged) policy.
 	newTest("allow-all-except-world", ct).

diff --git a/connectivity/builder/allow_all_with_metrics_check.go b/connectivity/builder/allow_all_with_metrics_check.go
@@ -4,18 +4,12 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
 
 type allowAllWithMetricsCheck struct{}
 
-func (t allowAllWithMetricsCheck) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t allowAllWithMetricsCheck) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy allows traffic pod to pod and checks if the metric cilium_forward_count_total increases on cilium agent.
 	newTest("allow-all-with-metrics-check", ct).

diff --git a/connectivity/builder/builder.go b/connectivity/builder/builder.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/builder/manifests/template"
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/utils/features"
@@ -155,7 +153,6 @@ var (
 )
 
 type testBuilder interface {
-	shouldRun(version semver.Version, params check.Parameters) bool
 	build(ct *check.ConnectivityTest, templates map[string]string)
 }
 
@@ -230,9 +227,7 @@ func newTest(name string, ct *check.ConnectivityTest) *check.Test {
 
 func injectTests(ct *check.ConnectivityTest, templates map[string]string, tests ...testBuilder) {
 	for _, t := range tests {
-		if t.shouldRun(ct.CiliumVersion, ct.Params()) {
-			t.build(ct, templates)
-		}
+		t.build(ct, templates)
 	}
 }
 

diff --git a/connectivity/builder/check_log_errors.go b/connectivity/builder/check_log_errors.go
@@ -4,8 +4,6 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium/pkg/versioncheck"
 
 	"github.com/cilium/cilium-cli/connectivity/check"
@@ -14,12 +12,11 @@ import (
 
 type checkLogErrors struct{}
 
-func (t checkLogErrors) shouldRun(version semver.Version, params check.Parameters) bool {
-	return versioncheck.MustCompile(">=1.14.0")(version) || params.IncludeUnsafeTests
-}
-
 func (t checkLogErrors) build(ct *check.ConnectivityTest, _ map[string]string) {
 	newTest("check-log-errors", ct).
+		WithConditionFn(func() bool {
+			return versioncheck.MustCompile(">=1.14.0")(ct.CiliumVersion) || ct.Params().IncludeUnsafeTests
+		}).
 		WithSysdumpPolicy(check.SysdumpPolicyOnce).
 		WithScenarios(tests.NoErrorsInLogs(ct.CiliumVersion))
 }
diff --git a/connectivity/builder/client_egress.go b/connectivity/builder/client_egress.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var clientEgressToEchoPolicyYAML string
 
 type clientEgress struct{}
 
-func (t clientEgress) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgress) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy allows port 8080 from client to echo, so this should succeed
 	newTest("client-egress", ct).

diff --git a/connectivity/builder/client_egress_expression.go b/connectivity/builder/client_egress_expression.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var clientEgressToEchoExpressionPolicyYAML string
 
 type clientEgressExpression struct{}
 
-func (t clientEgressExpression) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressExpression) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy allows port 8080 from client to echo (using label match expression, so this should succeed
 	newTest("client-egress-expression", ct).

diff --git a/connectivity/builder/client_egress_expression_knp.go b/connectivity/builder/client_egress_expression_knp.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var clientEgressToEchoExpressionPolicyKNPYAML string
 
 type clientEgressExpressionKnp struct{}
 
-func (t clientEgressExpressionKnp) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressExpressionKnp) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy allows port 8080 from client to echo (using label match expression, so this should succeed
 	newTest("client-egress-expression-knp", ct).

diff --git a/connectivity/builder/client_egress_knp.go b/connectivity/builder/client_egress_knp.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 )
@@ -17,10 +15,6 @@ var clientEgressToEchoPolicyKNPYAML string
 
 type clientEgressKnp struct{}
 
-func (t clientEgressKnp) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressKnp) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// This policy allows port 8080 from client to echo, so this should succeed
 	newTest("client-egress-knp", ct).

diff --git a/connectivity/builder/client_egress_l7.go b/connectivity/builder/client_egress_l7.go
@@ -4,19 +4,13 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 	"github.com/cilium/cilium-cli/utils/features"
 )
 
 type clientEgressL7 struct{}
 
-func (t clientEgressL7) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressL7) build(ct *check.ConnectivityTest, templates map[string]string) {
 	// Test L7 HTTP introspection using an egress policy on the clients.
 	newTest("client-egress-l7", ct).

diff --git a/connectivity/builder/client_egress_l7_method.go b/connectivity/builder/client_egress_l7_method.go
@@ -6,8 +6,6 @@ package builder
 import (
 	_ "embed"
 
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 	"github.com/cilium/cilium-cli/utils/features"
@@ -18,10 +16,6 @@ var clientEgressL7HTTPMethodPolicyYAML string
 
 type clientEgressL7Method struct{}
 
-func (t clientEgressL7Method) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressL7Method) build(ct *check.ConnectivityTest, _ map[string]string) {
 	// Test L7 HTTP with different methods introspection using an egress policy on the clients.
 	newTest("client-egress-l7-method", ct).

diff --git a/connectivity/builder/client_egress_l7_named_port.go b/connectivity/builder/client_egress_l7_named_port.go
@@ -4,19 +4,13 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 	"github.com/cilium/cilium-cli/utils/features"
 )
 
 type clientEgressL7NamedPort struct{}
 
-func (t clientEgressL7NamedPort) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressL7NamedPort) build(ct *check.ConnectivityTest, templates map[string]string) {
 	// Test L7 HTTP named port introspection using an egress policy on the clients.
 	newTest("client-egress-l7-named-port", ct).

diff --git a/connectivity/builder/client_egress_l7_set_header.go b/connectivity/builder/client_egress_l7_set_header.go
@@ -4,7 +4,6 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -15,10 +14,6 @@ import (
 
 type clientEgressL7SetHeader struct{}
 
-func (t clientEgressL7SetHeader) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressL7SetHeader) build(ct *check.ConnectivityTest, templates map[string]string) {
 	// Test L7 HTTP with a header replace set in the policy
 	newTest("client-egress-l7-set-header", ct).

diff --git a/connectivity/builder/client_egress_l7_tls_deny_without_headers.go b/connectivity/builder/client_egress_l7_tls_deny_without_headers.go
@@ -4,19 +4,13 @@
 package builder
 
 import (
-	"github.com/blang/semver/v4"
-
 	"github.com/cilium/cilium-cli/connectivity/check"
 	"github.com/cilium/cilium-cli/connectivity/tests"
 	"github.com/cilium/cilium-cli/utils/features"
 )
 
 type clientEgressL7TlsDenyWithoutHeaders struct{}
 
-func (t clientEgressL7TlsDenyWithoutHeaders) shouldRun(_ semver.Version, _ check.Parameters) bool {
-	return true
-}
-
 func (t clientEgressL7TlsDenyWithoutHeaders) build(ct *check.ConnectivityTest, templates map[string]string) {
 	// Test L7 HTTPS interception using an egress policy on the clients.
 	// Fail to load site due to missing headers.