From a06468420648653831a276985bc60fa1f8e4a25c Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 7 Jul 2022 02:21:30 +0200 Subject: [PATCH 1/2] fix when listeners not set --- api/v1alpha1/limitador_types.go | 29 ++++++++++++++++-- api/v1alpha1/zz_generated.deepcopy.go | 12 ++++++-- controllers/limitador_controller.go | 6 ++-- controllers/limitador_controller_test.go | 39 ++++++++++++++++++++++-- pkg/helpers/helpers.go | 7 ----- pkg/limitador/k8s_objects.go | 16 +++++----- pkg/limitador/k8s_objects_test.go | 5 ++- 7 files changed, 85 insertions(+), 29 deletions(-) diff --git a/api/v1alpha1/limitador_types.go b/api/v1alpha1/limitador_types.go index bcdded5d..d1dca498 100644 --- a/api/v1alpha1/limitador_types.go +++ b/api/v1alpha1/limitador_types.go @@ -20,6 +20,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + DefaultServiceHTTPPort int32 = 8080 + DefaultServiceGRPCPort int32 = 8081 +) + // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. @@ -61,6 +66,26 @@ type Limitador struct { Status LimitadorStatus `json:"status,omitempty"` } +func (l *Limitador) GRPCPort() int32 { + if l.Spec.Listener == nil || + l.Spec.Listener.GRPC == nil || + l.Spec.Listener.GRPC.Port == nil { + return DefaultServiceGRPCPort + } + + return *l.Spec.Listener.GRPC.Port +} + +func (l *Limitador) HTTPPort() int32 { + if l.Spec.Listener == nil || + l.Spec.Listener.HTTP == nil || + l.Spec.Listener.HTTP.Port == nil { + return DefaultServiceHTTPPort + } + + return *l.Spec.Listener.HTTP.Port +} + //+kubebuilder:object:root=true // LimitadorList contains a list of Limitador @@ -72,9 +97,9 @@ type LimitadorList struct { type Listener struct { // +optional - HTTP TransportProtocol `json:"http,omitempty"` + HTTP *TransportProtocol `json:"http,omitempty"` // +optional - GRPC TransportProtocol `json:"grpc,omitempty"` + GRPC *TransportProtocol `json:"grpc,omitempty"` } type TransportProtocol struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 8c14bea7..e6fee08c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -139,8 +139,16 @@ func (in *LimitadorStatus) DeepCopy() *LimitadorStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Listener) DeepCopyInto(out *Listener) { *out = *in - in.HTTP.DeepCopyInto(&out.HTTP) - in.GRPC.DeepCopyInto(&out.GRPC) + if in.HTTP != nil { + in, out := &in.HTTP, &out.HTTP + *out = new(TransportProtocol) + (*in).DeepCopyInto(*out) + } + if in.GRPC != nil { + in, out := &in.GRPC, &out.GRPC + *out = new(TransportProtocol) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Listener. diff --git a/controllers/limitador_controller.go b/controllers/limitador_controller.go index b81d82cc..c6217114 100644 --- a/controllers/limitador_controller.go +++ b/controllers/limitador_controller.go @@ -19,10 +19,10 @@ package controllers import ( "context" "fmt" - "github.com/kuadrant/limitador-operator/pkg/helpers" - v1 "k8s.io/api/core/v1" "strconv" + v1 "k8s.io/api/core/v1" + "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -126,7 +126,7 @@ func buildServiceUrl(limitadorObj *limitadorv1alpha1.Limitador) string { return "http://" + limitadorObj.Name + "." + limitadorObj.Namespace + ".svc.cluster.local:" + - strconv.Itoa(int(helpers.GetValueOrDefault(*limitadorObj.Spec.Listener.HTTP.Port, limitador.DefaultServiceHTTPPort).(int32))) + strconv.Itoa(int(limitadorObj.HTTPPort())) } func mutateLimitsConfigMap(existingObj, desiredObj client.Object) (bool, error) { diff --git a/controllers/limitador_controller_test.go b/controllers/limitador_controller_test.go index 696ab6b8..701de699 100644 --- a/controllers/limitador_controller_test.go +++ b/controllers/limitador_controller_test.go @@ -2,9 +2,10 @@ package controllers import ( "context" - "github.com/kuadrant/limitador-operator/pkg/limitador" "time" + "github.com/kuadrant/limitador-operator/pkg/limitador" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -36,8 +37,8 @@ var _ = Describe("Limitador controller", func() { replicas := LimitadorReplicas version := LimitadorVersion - httpPort := limitadorv1alpha1.TransportProtocol{Port: &httpPortNumber} - grpcPort := limitadorv1alpha1.TransportProtocol{Port: &grpcPortNumber} + httpPort := &limitadorv1alpha1.TransportProtocol{Port: &httpPortNumber} + grpcPort := &limitadorv1alpha1.TransportProtocol{Port: &grpcPortNumber} limits := []limitadorv1alpha1.RateLimit{ { @@ -83,6 +84,38 @@ var _ = Describe("Limitador controller", func() { deletePropagationPolicy := client.PropagationPolicy(metav1.DeletePropagationForeground) + Context("Creating a new empty Limitador object", func() { + var limitadorObj *limitadorv1alpha1.Limitador + + BeforeEach(func() { + limitadorObj = newLimitador() + limitadorObj.Spec = limitadorv1alpha1.LimitadorSpec{} + err := k8sClient.Delete(context.TODO(), limitadorObj, deletePropagationPolicy) + Expect(err == nil || errors.IsNotFound(err)) + + Expect(k8sClient.Create(context.TODO(), limitadorObj)).Should(Succeed()) + }) + + It("Should create a Limitador service with default ports", func() { + createdLimitadorService := v1.Service{} + Eventually(func() bool { + err := k8sClient.Get( + context.TODO(), + types.NamespacedName{ + Namespace: LimitadorNamespace, + Name: limitadorObj.Name, + }, + &createdLimitadorService) + return err == nil + }, timeout, interval).Should(BeTrue()) + Expect(len(createdLimitadorService.Spec.Ports)).Should(Equal(2)) + Expect(createdLimitadorService.Spec.Ports[0].Name).Should(Equal("http")) + Expect(createdLimitadorService.Spec.Ports[0].Port).Should(Equal(limitadorv1alpha1.DefaultServiceHTTPPort)) + Expect(createdLimitadorService.Spec.Ports[1].Name).Should(Equal("grpc")) + Expect(createdLimitadorService.Spec.Ports[1].Port).Should(Equal(limitadorv1alpha1.DefaultServiceGRPCPort)) + }) + }) + Context("Creating a new Limitador object", func() { var limitadorObj *limitadorv1alpha1.Limitador diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index b8ec09f1..bd2c714e 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -52,10 +52,3 @@ func FetchEnv(key string, def string) string { return val } - -func GetValueOrDefault(expectedValue interface{}, defaultValue interface{}) interface{} { - if expectedValue != nil { - return expectedValue - } - return defaultValue -} diff --git a/pkg/limitador/k8s_objects.go b/pkg/limitador/k8s_objects.go index 4d318e09..1eb7ad10 100644 --- a/pkg/limitador/k8s_objects.go +++ b/pkg/limitador/k8s_objects.go @@ -3,8 +3,8 @@ package limitador import ( "crypto/md5" "fmt" + limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" - "github.com/kuadrant/limitador-operator/pkg/helpers" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -17,8 +17,6 @@ const ( DefaultReplicas = 1 Image = "quay.io/3scale/limitador" StatusEndpoint = "/status" - DefaultServiceHTTPPort = 8080 - DefaultServiceGRPCPort = 8081 LimitadorConfigFileName = "limitador-config.yaml" LimitadorCMHash = "hash" LimitsCMNamePrefix = "limits-config-" @@ -43,13 +41,13 @@ func LimitadorService(limitador *limitadorv1alpha1.Limitador) *v1.Service { { Name: "http", Protocol: v1.ProtocolTCP, - Port: helpers.GetValueOrDefault(*limitador.Spec.Listener.HTTP.Port, DefaultServiceHTTPPort).(int32), + Port: limitador.HTTPPort(), TargetPort: intstr.FromString("http"), }, { Name: "grpc", Protocol: v1.ProtocolTCP, - Port: helpers.GetValueOrDefault(*limitador.Spec.Listener.GRPC.Port, DefaultServiceGRPCPort).(int32), + Port: limitador.GRPCPort(), TargetPort: intstr.FromString("grpc"), }, }, @@ -99,12 +97,12 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym Ports: []v1.ContainerPort{ { Name: "http", - ContainerPort: helpers.GetValueOrDefault(*limitador.Spec.Listener.HTTP.Port, DefaultServiceHTTPPort).(int32), + ContainerPort: limitador.HTTPPort(), Protocol: v1.ProtocolTCP, }, { Name: "grpc", - ContainerPort: helpers.GetValueOrDefault(*limitador.Spec.Listener.GRPC.Port, DefaultServiceGRPCPort).(int32), + ContainerPort: limitador.GRPCPort(), Protocol: v1.ProtocolTCP, }, }, @@ -122,7 +120,7 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym Handler: v1.Handler{ HTTPGet: &v1.HTTPGetAction{ Path: StatusEndpoint, - Port: intstr.FromInt(int(helpers.GetValueOrDefault(*limitador.Spec.Listener.HTTP.Port, DefaultServiceHTTPPort).(int32))), + Port: intstr.FromInt(int(limitador.HTTPPort())), Scheme: v1.URISchemeHTTP, }, }, @@ -136,7 +134,7 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym Handler: v1.Handler{ HTTPGet: &v1.HTTPGetAction{ Path: StatusEndpoint, - Port: intstr.FromInt(int(helpers.GetValueOrDefault(*limitador.Spec.Listener.HTTP.Port, DefaultServiceHTTPPort).(int32))), + Port: intstr.FromInt(int(limitador.HTTPPort())), Scheme: v1.URISchemeHTTP, }, }, diff --git a/pkg/limitador/k8s_objects_test.go b/pkg/limitador/k8s_objects_test.go index 55f82ae6..91c532e3 100644 --- a/pkg/limitador/k8s_objects_test.go +++ b/pkg/limitador/k8s_objects_test.go @@ -1,8 +1,9 @@ package limitador import ( - "gotest.tools/assert" "testing" + + "gotest.tools/assert" ) func TestConstants(t *testing.T) { @@ -10,8 +11,6 @@ func TestConstants(t *testing.T) { assert.Check(t, 1 == DefaultReplicas) assert.Check(t, "quay.io/3scale/limitador" == Image) assert.Check(t, "/status" == StatusEndpoint) - assert.Check(t, 8080 == DefaultServiceHTTPPort) - assert.Check(t, 8081 == DefaultServiceGRPCPort) assert.Check(t, "limitador-config.yaml" == LimitadorConfigFileName) assert.Check(t, "hash" == LimitadorCMHash) assert.Check(t, "limits-config-" == LimitsCMNamePrefix) From f90495a6ca80d45316127bb44fe1167a738cf4d5 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 7 Jul 2022 23:58:48 +0200 Subject: [PATCH 2/2] fix when limits not set --- api/v1alpha1/limitador_types.go | 8 ++++++++ pkg/limitador/k8s_objects.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/limitador_types.go b/api/v1alpha1/limitador_types.go index d1dca498..604fd752 100644 --- a/api/v1alpha1/limitador_types.go +++ b/api/v1alpha1/limitador_types.go @@ -86,6 +86,14 @@ func (l *Limitador) HTTPPort() int32 { return *l.Spec.Listener.HTTP.Port } +func (l *Limitador) Limits() []RateLimit { + if l.Spec.Limits == nil { + return make([]RateLimit, 0) + } + + return l.Spec.Limits +} + //+kubebuilder:object:root=true // LimitadorList contains a list of Limitador diff --git a/pkg/limitador/k8s_objects.go b/pkg/limitador/k8s_objects.go index 1eb7ad10..a3e9e702 100644 --- a/pkg/limitador/k8s_objects.go +++ b/pkg/limitador/k8s_objects.go @@ -172,7 +172,7 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym } func LimitsConfigMap(limitador *limitadorv1alpha1.Limitador) (*v1.ConfigMap, error) { - limitsMarshalled, marshallErr := yaml.Marshal(limitador.Spec.Limits) + limitsMarshalled, marshallErr := yaml.Marshal(limitador.Limits()) if marshallErr != nil { return nil, marshallErr }