[refactor] Storage options

* No longer passing empty strings * Adding Redis Cached and its options * Removing Infinispan * Behaviour changed, no Storage config means in "memory"
Kuadrant · Sep 21, 2022 · 4d68923 · 4d68923
1 parent 9a55813
commit 4d68923
Show file tree

Hide file tree

Showing 5 changed files with 190 additions and 45 deletions.
diff --git a/api/v1alpha1/limitador_types.go b/api/v1alpha1/limitador_types.go
@@ -17,6 +17,7 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"fmt"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -106,29 +107,91 @@ type LimitadorList struct {
 	Items           []Limitador `json:"items"`
 }
 
+// StorageType defines the valid options for storage
+// +kubebuilder:validation:Enum=memory;redis;redis_cached
+type StorageType string
+
+const (
+	StorageTypeInMemory    StorageType = "memory"
+	StorageTypeRedis                   = "redis"
+	StorageTypeRedisCached             = "redis_cached"
+)
+
 // Storage contains the options for Limitador counters database or in-memory data storage
 type Storage struct {
 	// +optional
-	// +kubebuilder:default="in-memory"
-	Type StorageType `json:"type,omitempty"`
+	Redis *Redis `json:"redis,omitempty"`
+
+	// +optional
+	RedisCached *RedisCached `json:"redis-cached,omitempty"`
+}
+
+func (s *Storage) Validate() bool {
+	return s.Redis != nil && s.Redis.ConfigSecretRef != nil ||
+		s.RedisCached != nil && s.RedisCached.ConfigSecretRef != nil
+}
+
+func (s *Storage) SecretRef() *corev1.ObjectReference {
+	if s.Redis != nil {
+		return s.Redis.ConfigSecretRef
+	}
+	return s.RedisCached.ConfigSecretRef
+}
+
+func (s *Storage) Config(url string) []string {
+	if s.Redis != nil {
+		return []string{
+			StorageTypeRedis,
+			url,
+		}
+	} else if s.RedisCached != nil {
+		return []string{
+			StorageTypeRedisCached,
+			url,
+			fmt.Sprintf("--ttl %d", s.RedisCached.Options.TTL),
+			fmt.Sprintf("--ratio %d", s.RedisCached.Options.Ratio),
+			fmt.Sprintf("--flush-period %d", s.RedisCached.Options.FlushPeriod),
+			fmt.Sprintf("--max-cached %d", s.RedisCached.Options.MaxCached),
+		}
+	}
+	return []string{string(StorageTypeInMemory)}
+}
 
-	// +ConfigSecretRef refers to the secret holding the URL for Redis or Inifinispan.
+type Redis struct {
+	// +ConfigSecretRef refers to the secret holding the URL for Redis.
 	// +optional
 	ConfigSecretRef *corev1.ObjectReference `json:"configSecretRef,omitempty"`
 }
 
-// StorageType defines the valid options for storage
-// +kubebuilder:validation:Enum=in-memory;redis;infinispan
-type StorageType string
+type RedisCachedOptions struct {
+	// +optional
+	// +kubebuilder:default=5000
+	// TTL for cached counters in milliseconds [default: 5000]
+	TTL *int `json:"ttl,omitempty"`
 
-const (
-	StorageTypeInMemory   StorageType = "in-memory"
-	StorageTypeRedis                  = "redis"
-	StorageTypeInfinispan             = "infinispan"
-)
+	// +optional
+	// +kubebuilder:default=10
+	// Ratio to apply to the TTL from Redis on cached counters [default: 10]
+	Ratio *int `json:"ratio,omitempty"`
 
-func (s *Storage) Validate() bool {
-	return s.Type == StorageTypeInMemory || s.ConfigSecretRef != nil
+	// +optional
+	// +kubebuilder:default=1000
+	// FlushPeriod for counters in milliseconds [default: 1000]
+	FlushPeriod *int `json:"flush-period,omitempty"`
+
+	// +optional
+	// +kubebuilder:default=10000
+	// MaxCached refers to the maximum amount of counters cached [default: 10000]
+	MaxCached *int `json:"max-cached,omitempty"`
+}
+
+type RedisCached struct {
+	// +ConfigSecretRef refers to the secret holding the URL for Redis.
+	// +optional
+	ConfigSecretRef *corev1.ObjectReference `json:"configSecretRef,omitempty"`
+
+	// +optional
+	Options *RedisCachedOptions `json:"options,omitempty"`
 }
 
 type Listener struct {

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go
@@ -81,13 +81,14 @@ func (r *LimitadorReconciler) Reconcile(eventCtx context.Context, req ctrl.Reque
 
 	limitadorStorage := limitadorObj.Spec.Storage
 	var storageConfigSecret *v1.Secret
-	if limitadorStorage != nil && limitadorStorage.Type != limitadorv1alpha1.StorageTypeInMemory {
-		if !limitadorStorage.Validate() {
+	if limitadorStorage != nil {
+		if limitadorStorage.Validate() {
+			if storageConfigSecret, err = getStorageConfigSecret(ctx, r.Client(), limitadorObj.Namespace, limitadorStorage.SecretRef()); err != nil {
+				return ctrl.Result{}, err
+			}
+		} else {
 			return ctrl.Result{}, fmt.Errorf("there's no ConfigSecretRef set")
 		}
-		if storageConfigSecret, err = getStorageConfigSecret(ctx, r.Client(), limitadorStorage.ConfigSecretRef); err != nil {
-			return ctrl.Result{}, err
-		}
 	}
 
 	deployment := limitador.LimitadorDeployment(limitadorObj, storageConfigSecret)
@@ -291,13 +292,18 @@ func mutateLimitadorDeployment(existingObj, desiredObj client.Object) (bool, err
 	return updated, nil
 }
 
-func getStorageConfigSecret(ctx context.Context, client client.Client, secretRef *v1.ObjectReference) (*v1.Secret, error) {
+func getStorageConfigSecret(ctx context.Context, client client.Client, limitadorNamespace string, secretRef *v1.ObjectReference) (*v1.Secret, error) {
 	storageConfigSecret := &v1.Secret{}
 	if err := client.Get(
 		ctx,
 		types.NamespacedName{
-			Name:      secretRef.Name,
-			Namespace: secretRef.Namespace,
+			Name: secretRef.Name,
+			Namespace: func() string {
+				if secretRef.Namespace != "" {
+					return secretRef.Namespace
+				}
+				return limitadorNamespace
+			}(),
 		},
 		storageConfigSecret,
 	); err != nil {
@@ -307,5 +313,5 @@ func getStorageConfigSecret(ctx context.Context, client client.Client, secretRef
 	if len(storageConfigSecret.Data) > 0 && storageConfigSecret.Data["URL"] != nil {
 		return storageConfigSecret, nil
 	}
-	return nil, fmt.Errorf("the storage config Secret doesn't have the `URL` field")
+	return nil, errors.NewBadRequest("the storage config Secret doesn't have the `URL` field")
 }
diff --git a/controllers/limitador_controller_test.go b/controllers/limitador_controller_test.go
@@ -78,10 +78,6 @@ var _ = Describe("Limitador controller", func() {
 					GRPC: grpcPort,
 				},
 				Limits: limits,
-				Storage: &limitadorv1alpha1.Storage{
-					Type:            limitadorv1alpha1.StorageTypeInMemory,
-					ConfigSecretRef: nil,
-				},
 			},
 		}
 	}
@@ -167,8 +163,7 @@ var _ = Describe("Limitador controller", func() {
 					[]string{
 						"limitador-server",
 						"/home/limitador/etc/limitador-config.yaml",
-						"in-memory",
-						"",
+						"memory",
 					},
 				),
 			)

diff --git a/pkg/limitador/k8s_objects.go b/pkg/limitador/k8s_objects.go
@@ -89,14 +89,9 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador, storageConfigSe
 				Spec: v1.PodSpec{
 					Containers: []v1.Container{
 						{
-							Name:  "limitador",
-							Image: image,
-							Command: []string{
-								"limitador-server",
-								fmt.Sprintf("%s%s", LimitadorCMMountPath, LimitadorConfigFileName),
-								storageType(limitador.Spec.Storage),
-								storageConfig(limitador.Spec.Storage, storageConfigSecret),
-							},
+							Name:    "limitador",
+							Image:   image,
+							Command: deploymentContainerCommand(limitador.Spec.Storage, storageConfigSecret),
 							Ports: []v1.ContainerPort{
 								{
 									Name:          "http",
@@ -199,16 +194,17 @@ func ownerRefToLimitador(limitador *limitadorv1alpha1.Limitador) metav1.OwnerRef
 	}
 }
 
-func storageType(storage *limitadorv1alpha1.Storage) string {
-	if storage != nil {
-		return string(storage.Type)
+func deploymentContainerCommand(storage *limitadorv1alpha1.Storage, storageConfigSecret *v1.Secret) []string {
+	command := []string{
+		"limitador-server",
+		fmt.Sprintf("%s%s", LimitadorCMMountPath, LimitadorConfigFileName),
 	}
-	return string(limitadorv1alpha1.StorageTypeInMemory)
+	return append(command, storageConfig(storage, storageConfigSecret)...)
 }
 
-func storageConfig(storage *limitadorv1alpha1.Storage, storageConfigSecret *v1.Secret) string {
-	if storage != nil && storage.Type != limitadorv1alpha1.StorageTypeInMemory {
-		return string(storageConfigSecret.Data["URL"])
+func storageConfig(storage *limitadorv1alpha1.Storage, storageConfigSecret *v1.Secret) []string {
+	if storage == nil {
+		return []string{string(limitadorv1alpha1.StorageTypeInMemory)}
 	}
-	return ""
+	return storage.Config(string(storageConfigSecret.Data["URL"]))
 }