From a759480331f29e97533ad9e4864226014c89e386 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Wed, 8 Feb 2023 19:06:52 +0100 Subject: [PATCH] unit tests --- Makefile | 5 +- doc/development.md | 12 + pkg/common/common.go | 13 + pkg/rlptools/limit_index.go | 11 + pkg/rlptools/limit_index_test.go | 522 +++++++++++++++++++++++++++++++ 5 files changed, 562 insertions(+), 1 deletion(-) create mode 100644 pkg/rlptools/limit_index_test.go diff --git a/Makefile b/Makefile index fc4c9b0cb..46830846c 100644 --- a/Makefile +++ b/Makefile @@ -219,8 +219,11 @@ test: test-unit test-integration ## Run all tests test-integration: clean-cov generate fmt vet envtest ## Run Integration tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) $(ARCH_PARAM) use $(ENVTEST_K8S_VERSION) -p path)" USE_EXISTING_CLUSTER=true go test ./... -coverprofile $(PROJECT_PATH)/cover.out -tags integration -ginkgo.v -ginkgo.progress -v -timeout 0 +ifdef TEST_NAME +test-unit: TEST_PATTERN := --run $(TEST_NAME) +endif test-unit: clean-cov generate fmt vet ## Run Unit tests. - go test ./... -coverprofile $(PROJECT_PATH)/cover.out -tags unit -v -timeout 0 + go test ./... -coverprofile $(PROJECT_PATH)/cover.out -tags unit -v -timeout 0 $(TEST_PATTERN) .PHONY: namespace namespace: ## Creates a namespace where to deploy Kuadrant Operator diff --git a/doc/development.md b/doc/development.md index f6ec23e28..234a903f8 100644 --- a/doc/development.md +++ b/doc/development.md @@ -218,6 +218,18 @@ make local-cleanup make test-unit ``` +Optionally, add `TEST_NAME` makefile variable to run specific test + +```sh +make test-unit TEST_NAME=TestLimitIndexEquals +``` + +or even subtest + +```sh +make test-unit TEST_NAME=TestLimitIndexEquals/limit_conditions_order_does_not_matter +``` + ### Integration tests You need an active session open to a kubernetes cluster. diff --git a/pkg/common/common.go b/pkg/common/common.go index 7c28a15e9..eea1076b2 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -105,6 +105,19 @@ func SliceCopy[T any](s1 []T) []T { return s2 } +func ReverseSlice[T any](input []T) []T { + inputLen := len(input) + output := make([]T, inputLen) + + for i, n := range input { + j := inputLen - i - 1 + + output[j] = n + } + + return output +} + // MergeMapStringString Merge desired into existing. // Not Thread-Safe. Does it matter? func MergeMapStringString(existing *map[string]string, desired map[string]string) bool { diff --git a/pkg/rlptools/limit_index.go b/pkg/rlptools/limit_index.go index fb04059e3..c1cb82401 100644 --- a/pkg/rlptools/limit_index.go +++ b/pkg/rlptools/limit_index.go @@ -87,6 +87,17 @@ func SameLimitList(a, b []kuadrantv1beta1.Limit) bool { copy(aCopy, a) copy(bCopy, b) + // two limits with reordered conditions/variables are effectively the same + for idx := range aCopy { + sort.Strings(aCopy[idx].Conditions) + sort.Strings(aCopy[idx].Variables) + } + + for idx := range bCopy { + sort.Strings(bCopy[idx].Conditions) + sort.Strings(bCopy[idx].Variables) + } + sort.Sort(LimitList(aCopy)) sort.Sort(LimitList(bCopy)) diff --git a/pkg/rlptools/limit_index_test.go b/pkg/rlptools/limit_index_test.go new file mode 100644 index 000000000..412433bed --- /dev/null +++ b/pkg/rlptools/limit_index_test.go @@ -0,0 +1,522 @@ +//go:build unit + +package rlptools + +import ( + "os" + "reflect" + "testing" + + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" + "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/log" +) + +func TestLimitIndexEquals(t *testing.T) { + logger := log.NewLogger( + log.WriteTo(os.Stdout), + log.SetLevel(log.DebugLevel), + ) + t.Run("nil indexes are equal", func(subT *testing.T) { + idxA := NewLimitadorIndex(nil, logger) + idxB := NewLimitadorIndex(nil, logger) + + if !idxA.Equals(idxB) { + subT.Fatal("nil indexes are not equal") + } + }) + t.Run("empty indexes are equal", func(subT *testing.T) { + idxA := NewLimitadorIndex(emptyLimitador(), logger) + idxB := NewLimitadorIndex(emptyLimitador(), logger) + + if !idxA.Equals(idxB) { + subT.Fatal("nil indexes are not equal") + } + }) + + // Rate limit order does not matter + // check the order does not matter when limit differ in + // maxValue, seconds, namespace, conditions, variables + testCases := []struct { + name string + limitador *limitadorv1alpha1.Limitador + }{ + { + "rate limit order does not matter: diff maxvalue", + limitadorWithMultipleLimitsMaxValue(), + }, + { + "rate limit order does not matter: diff seconds", + limitadorWithMultipleLimitsSeconds(), + }, + { + "rate limit order does not matter: diff namespace", + limitadorWithMultipleLimitsNamespace(), + }, + { + "rate limit order does not matter: diff conditions", + limitadorWithMultipleLimitsConditions(), + }, + { + "rate limit order does not matter: diff variables", + limitadorWithMultipleLimitsVariables(), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(subT *testing.T) { + limitadorA := tc.limitador + idxA := NewLimitadorIndex(limitadorA, logger) + reversedLimitadorA := *limitadorA + reversedLimitadorA.Spec.Limits = common.ReverseSlice(limitadorA.Spec.Limits) + + idxB := NewLimitadorIndex(&reversedLimitadorA, logger) + + if !idxA.Equals(idxB) { + subT.Fatal("indexes with reversed limits are not equal") + } + }) + } + + t.Run("limit conditions order does not matter", func(subT *testing.T) { + limitadorA := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: []string{"a", "b"}, + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: make([]string, 0), + }, + }, + }, + } + idxA := NewLimitadorIndex(limitadorA, logger) + + limitadorB := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: "nsB"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: []string{"b", "a"}, // reverse order regarding limitadorA + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: make([]string, 0), + }, + }, + }, + } + idxB := NewLimitadorIndex(limitadorB, logger) + + if !idxA.Equals(idxB) { + subT.Fatal("indexes with limits with reversed conditions are not equal") + } + }) + + t.Run("limit variables order does not matter", func(subT *testing.T) { + limitadorA := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: []string{"a", "b"}, + }, + }, + }, + } + idxA := NewLimitadorIndex(limitadorA, logger) + + limitadorB := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "b", Namespace: "nsB"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: []string{"b", "a"}, // reverse order regarding limitadorA + }, + }, + }, + } + idxB := NewLimitadorIndex(limitadorB, logger) + + if !idxA.Equals(idxB) { + subT.Fatal("indexes with limits with reversed variables are not equal") + } + }) +} + +func TestLimitIndexToLimits(t *testing.T) { + logger := log.NewLogger( + log.WriteTo(os.Stdout), + log.SetLevel(log.DebugLevel), + ) + + t.Run("nil index return empty list", func(subT *testing.T) { + idx := NewLimitadorIndex(nil, logger) + + limits := idx.ToLimits() + if limits == nil { + subT.Fatal("returns nil") + } + if len(limits) != 0 { + subT.Fatal("returns not empty") + } + }) + + t.Run("empty index return empty list", func(subT *testing.T) { + idx := NewLimitadorIndex(emptyLimitador(), logger) + + limits := idx.ToLimits() + if limits == nil { + subT.Fatal("returns nil") + } + if len(limits) != 0 { + subT.Fatal("returns not empty") + } + }) + + t.Run("converting one limit index", func(subT *testing.T) { + limitador := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: []string{"c_a", "c_b", "c_c"}, + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: []string{"v_a", "v_b", "v_c"}, + }, + }, + }, + } + idx := NewLimitadorIndex(limitador, logger) + + limits := idx.ToLimits() + if limits == nil { + subT.Fatal("returns nil") + } + if len(limits) != 1 { + subT.Fatal("it should return one limit") + } + + if !reflect.DeepEqual(limits[0], limitador.Spec.Limits[0]) { + subT.Fatal("limit does not match") + } + }) + t.Run("converting limits with nil variables", func(subT *testing.T) { + limitador := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: []string{"c_a", "c_b", "c_c"}, + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: nil, + }, + }, + }, + } + idx := NewLimitadorIndex(limitador, logger) + + limits := idx.ToLimits() + if limits == nil { + subT.Fatal("returns nil") + } + if len(limits) != 1 { + subT.Fatal("it should return one limit") + } + + expectedLimit := limitador.Spec.Limits[0].DeepCopy() + // expected limitador limit should not have nil variables + expectedLimit.Variables = make([]string, 0) + if !reflect.DeepEqual(limits[0], *expectedLimit) { + subT.Fatal("limit does not match") + } + }) + + t.Run("converting limits with nil conditions", func(subT *testing.T) { + limitador := &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: nil, + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: []string{"v_a", "v_b", "v_c"}, + }, + }, + }, + } + idx := NewLimitadorIndex(limitador, logger) + + limits := idx.ToLimits() + if limits == nil { + subT.Fatal("returns nil") + } + if len(limits) != 1 { + subT.Fatal("it should return one limit") + } + + expectedLimit := limitador.Spec.Limits[0].DeepCopy() + // expected limitador limit should not have nil conditions + expectedLimit.Conditions = make([]string, 0) + if !reflect.DeepEqual(limits[0], *expectedLimit) { + subT.Fatal("limit does not match") + } + }) +} + +func TestLimitIndexAddLimit(t *testing.T) { + logger := log.NewLogger( + log.WriteTo(os.Stdout), + log.SetLevel(log.DebugLevel), + ) + + var ( + gwKey = client.ObjectKey{Name: "gwA", Namespace: "nsA"} + domain = "a.com" + maxValue = 2 + seconds = 19 + conditions = []string{"a", "b"} + variables = []string{"c", "d"} + ) + + limit := &kuadrantv1beta1.Limit{ + Conditions: conditions, MaxValue: maxValue, Seconds: seconds, Variables: variables, + } + + idx := NewLimitadorIndex(emptyLimitador(), logger) + idx.AddLimit(gwKey, domain, limit) + limits := idx.ToLimits() + if limits == nil { + t.Fatal("returns nil") + } + if len(limits) != 1 { + t.Fatal("it should return one limit") + } + + expectedLimit := limitadorv1alpha1.RateLimit{ + Conditions: conditions, + MaxValue: maxValue, + Namespace: common.MarshallNamespace(gwKey, domain), + Seconds: seconds, + Variables: variables, + } + if !reflect.DeepEqual(limits[0], expectedLimit) { + t.Fatal("limit does not match") + } +} + +func TestLimitIndexDeleteGateway(t *testing.T) { + // TODO +} + +func limitadorNamespaceA() string { + return common.MarshallNamespace(client.ObjectKey{Name: "gwA", Namespace: "nsA"}, "a.com") +} + +func limitadorNamespaceB() string { + return common.MarshallNamespace(client.ObjectKey{Name: "gwB", Namespace: "nsB"}, "b.com") +} + +func emptyLimitador() *limitadorv1alpha1.Limitador { + return &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: nil, + }, + } +} + +func limitadorWithMultipleLimitsMaxValue() *limitadorv1alpha1.Limitador { + // limits differ in maxValue + return &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: nil, + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: nil, + }, + { + Conditions: nil, + MaxValue: 2, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: nil, + }, + }, + }, + } +} + +func limitadorWithMultipleLimitsSeconds() *limitadorv1alpha1.Limitador { + // limits differ in seconds + return &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: make([]string, 0), + }, + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 2, + Variables: make([]string, 0), + }, + }, + }, + } +} + +func limitadorWithMultipleLimitsNamespace() *limitadorv1alpha1.Limitador { + // limits differ in namespace + return &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: make([]string, 0), + }, + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceB(), + Seconds: 1, + Variables: make([]string, 0), + }, + }, + }, + } +} + +func limitadorWithMultipleLimitsConditions() *limitadorv1alpha1.Limitador { + // limits differ in conditions + return &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: []string{"a"}, + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: make([]string, 0), + }, + { + Conditions: []string{"b"}, + MaxValue: 1, + Namespace: limitadorNamespaceB(), + Seconds: 1, + Variables: make([]string, 0), + }, + }, + }, + } +} + +func limitadorWithMultipleLimitsVariables() *limitadorv1alpha1.Limitador { + // limits differ in variables + return &limitadorv1alpha1.Limitador{ + TypeMeta: metav1.TypeMeta{ + Kind: "Limitador", + APIVersion: "limitador.kuadrant.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "nsA"}, + Spec: limitadorv1alpha1.LimitadorSpec{ + Limits: []limitadorv1alpha1.RateLimit{ + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceA(), + Seconds: 1, + Variables: []string{"a"}, + }, + { + Conditions: make([]string, 0), + MaxValue: 1, + Namespace: limitadorNamespaceB(), + Seconds: 1, + Variables: []string{"b"}, + }, + }, + }, + } +}