fix: rateLimitDeployment ignoring pod labels and annotation merge (#4228

) * fix labels and annotation merges for rate limit deployment Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * fix tests and label merge Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * fix annotation merge if prometheus was disabled and annotations were defined Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * renamed labels and annotations to specify they apply to pods only Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * linter Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * fix resource provider tests to new annotation behavior Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * go linter Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * fix gen-check Signed-off-by: Oscar Boher <oscar.boher@scopely.com> * pod labels selector comment Signed-off-by: Oscar Boher <oscar.boher@scopely.com> --------- Signed-off-by: Oscar Boher <oscar.boher@scopely.com> Co-authored-by: zirain <zirain2009@gmail.com> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
envoyproxy · Oct 1, 2024 · cf84927 · cf84927
1 parent db1f437
commit cf84927
Show file tree

Hide file tree

Showing 11 changed files with 391 additions and 8 deletions.
diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go
@@ -9,6 +9,7 @@ import (
  _ "embed"
  "strconv"
 
+ "golang.org/x/exp/maps"
  appsv1 "k8s.io/api/apps/v1"
  autoscalingv2 "k8s.io/api/autoscaling/v2"
  corev1 "k8s.io/api/core/v1"
@@ -196,19 +197,30 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
  }
 
  containers := expectedRateLimitContainers(r.rateLimit, r.rateLimitDeployment, r.Namespace)
- labels := rateLimitLabels()
- selector := resource.GetSelector(labels)
+ selector := resource.GetSelector(rateLimitLabels())
+
+ podLabels := rateLimitLabels()
+ if r.rateLimitDeployment.Pod.Labels != nil {
+ maps.Copy(podLabels, r.rateLimitDeployment.Pod.Labels)
+ // Copy overwrites values in the dest map if they exist in the src map https://pkg.go.dev/maps#Copy
+ // It's applied again with the rateLimitLabels that are used as deployment selector to ensure those are not overwritten by user input
+ maps.Copy(podLabels, rateLimitLabels())
+ }
 
- var annotations map[string]string
+ var podAnnotations map[string]string
  if enablePrometheus(r.rateLimit) {
- annotations = map[string]string{
+ podAnnotations = map[string]string{
  "prometheus.io/path": "/metrics",
  "prometheus.io/port": strconv.Itoa(PrometheusPort),
  "prometheus.io/scrape": "true",
  }
  }
  if r.rateLimitDeployment.Pod.Annotations != nil {
- annotations = r.rateLimitDeployment.Pod.Annotations
+ if podAnnotations != nil {
+ maps.Copy(podAnnotations, r.rateLimitDeployment.Pod.Annotations)
+ } else {
+ podAnnotations = r.rateLimitDeployment.Pod.Annotations
+ }
  }
 
  deployment := &appsv1.Deployment{
@@ -218,16 +230,16 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
  },
  ObjectMeta: metav1.ObjectMeta{
  Namespace: r.Namespace,
- Labels: labels,
+ Labels: rateLimitLabels(),
  },
  Spec: appsv1.DeploymentSpec{
  Replicas: r.rateLimitDeployment.Replicas,
  Strategy: *r.rateLimitDeployment.Strategy,
  Selector: selector,
  Template: corev1.PodTemplateSpec{
  ObjectMeta: metav1.ObjectMeta{
- Labels: selector.MatchLabels,
- Annotations: annotations,
+ Labels: podLabels,
+ Annotations: podAnnotations,
  },
  Spec: corev1.PodSpec{
  Containers: containers,

diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go
@@ -9,6 +9,7 @@ import (
  "flag"
  "fmt"
  "os"
+ "strconv"
  "testing"
 
  "github.com/stretchr/testify/assert"
@@ -678,6 +679,50 @@ func TestDeployment(t *testing.T) {
  },
  },
  },
+ {
+ caseName: "merge-labels",
+ rateLimit: &egv1a1.RateLimit{
+ Backend: egv1a1.RateLimitDatabaseBackend{
+ Type: egv1a1.RedisBackendType,
+ Redis: &egv1a1.RateLimitRedisSettings{
+ URL: "redis.redis.svc:6379",
+ },
+ },
+ },
+ deploy: &egv1a1.KubernetesDeploymentSpec{
+ Pod: &egv1a1.KubernetesPodSpec{
+ Labels: map[string]string{
+ "app.kubernetes.io/name": InfraName,
+ "app.kubernetes.io/component": "ratelimit",
+ "app.kubernetes.io/managed-by": "envoy-gateway",
+ "key1": "value1",
+ "key2": "value2",
+ },
+ },
+ },
+ },
+ {
+ caseName: "merge-annotations",
+ rateLimit: &egv1a1.RateLimit{
+ Backend: egv1a1.RateLimitDatabaseBackend{
+ Type: egv1a1.RedisBackendType,
+ Redis: &egv1a1.RateLimitRedisSettings{
+ URL: "redis.redis.svc:6379",
+ },
+ },
+ },
+ deploy: &egv1a1.KubernetesDeploymentSpec{
+ Pod: &egv1a1.KubernetesPodSpec{
+ Annotations: map[string]string{
+ "prometheus.io/path": "/metrics",
+ "prometheus.io/port": strconv.Itoa(PrometheusPort),
+ "prometheus.io/scrape": "true",
+ "key1": "value1",
+ "key2": "value2",
+ },
+ },
+ },
+ },
  }
  for _, tc := range cases {
  t.Run(tc.caseName, func(t *testing.T) {

diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/custom.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/custom.yaml
@@ -27,6 +27,8 @@ spec:
  template:
  metadata:
  annotations:
+ prometheus.io/path: /metrics
+ prometheus.io/port: "19001"
  prometheus.io/scrape: "true"
  creationTimestamp: null
  labels:

diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default-env.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default-env.yaml
@@ -27,6 +27,8 @@ spec:
  template:
  metadata:
  annotations:
+ prometheus.io/path: /metrics
+ prometheus.io/port: "19001"
  prometheus.io/scrape: "true"
  creationTimestamp: null
  labels:

diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/extension-env.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/extension-env.yaml
@@ -27,6 +27,8 @@ spec:
  template:
  metadata:
  annotations:
+ prometheus.io/path: /metrics
+ prometheus.io/port: "19001"
  prometheus.io/scrape: "true"
  creationTimestamp: null
  labels:

diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-annotations.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-annotations.yaml
@@ -0,0 +1,156 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ creationTimestamp: null
+ labels:
+ app.kubernetes.io/component: ratelimit
+ app.kubernetes.io/managed-by: envoy-gateway
+ app.kubernetes.io/name: envoy-ratelimit
+ name: envoy-ratelimit
+ namespace: envoy-gateway-system
+ ownerReferences:
+ - apiVersion: apps/v1
+ kind: Deployment
+ name: envoy-gateway
+ uid: test-owner-reference-uid-for-deployment
+spec:
+ progressDeadlineSeconds: 600
+ revisionHistoryLimit: 10
+ selector:
+ matchLabels:
+ app.kubernetes.io/component: ratelimit
+ app.kubernetes.io/managed-by: envoy-gateway
+ app.kubernetes.io/name: envoy-ratelimit
+ strategy:
+ type: RollingUpdate
+ template:
+ metadata:
+ annotations:
+ key1: value1
+ key2: value2
+ prometheus.io/path: /metrics
+ prometheus.io/port: "19001"
+ prometheus.io/scrape: "true"
+ creationTimestamp: null
+ labels:
+ app.kubernetes.io/component: ratelimit
+ app.kubernetes.io/managed-by: envoy-gateway
+ app.kubernetes.io/name: envoy-ratelimit
+ spec:
+ automountServiceAccountToken: false
+ containers:
+ - command:
+ - /bin/ratelimit
+ env:
+ - name: RUNTIME_ROOT
+ value: /data
+ - name: RUNTIME_SUBDIRECTORY
+ value: ratelimit
+ - name: RUNTIME_IGNOREDOTFILES
+ value: "true"
+ - name: RUNTIME_WATCH_ROOT
+ value: "false"
+ - name: LOG_LEVEL
+ value: info
+ - name: USE_STATSD
+ value: "false"
+ - name: CONFIG_TYPE
+ value: GRPC_XDS_SOTW
+ - name: CONFIG_GRPC_XDS_SERVER_URL
+ value: envoy-gateway:18001
+ - name: CONFIG_GRPC_XDS_NODE_ID
+ value: envoy-ratelimit
+ - name: GRPC_SERVER_USE_TLS
+ value: "true"
+ - name: GRPC_SERVER_TLS_CERT
+ value: /certs/tls.crt
+ - name: GRPC_SERVER_TLS_KEY
+ value: /certs/tls.key
+ - name: GRPC_SERVER_TLS_CA_CERT
+ value: /certs/ca.crt
+ - name: CONFIG_GRPC_XDS_SERVER_USE_TLS
+ value: "true"
+ - name: CONFIG_GRPC_XDS_CLIENT_TLS_CERT
+ value: /certs/tls.crt
+ - name: CONFIG_GRPC_XDS_CLIENT_TLS_KEY
+ value: /certs/tls.key
+ - name: CONFIG_GRPC_XDS_SERVER_TLS_CACERT
+ value: /certs/ca.crt
+ - name: FORCE_START_WITHOUT_INITIAL_CONFIG
+ value: "true"
+ - name: REDIS_SOCKET_TYPE
+ value: tcp
+ - name: REDIS_URL
+ value: redis.redis.svc:6379
+ - name: USE_PROMETHEUS
+ value: "true"
+ - name: PROMETHEUS_ADDR
+ value: :19001
+ - name: PROMETHEUS_MAPPER_YAML
+ value: /etc/statsd-exporter/conf.yaml
+ image: envoyproxy/ratelimit:master
+ imagePullPolicy: IfNotPresent
+ name: envoy-ratelimit
+ ports:
+ - containerPort: 8081
+ name: grpc
+ protocol: TCP
+ readinessProbe:
+ failureThreshold: 1
+ httpGet:
+ path: /healthcheck
+ port: 8080
+ scheme: HTTP
+ periodSeconds: 5
+ successThreshold: 1
+ timeoutSeconds: 1
+ resources:
+ requests:
+ cpu: 100m
+ memory: 512Mi
+ securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop:
+ - ALL
+ privileged: false
+ readOnlyRootFilesystem: true
+ runAsGroup: 65534
+ runAsNonRoot: true
+ runAsUser: 65534
+ seccompProfile:
+ type: RuntimeDefault
+ startupProbe:
+ failureThreshold: 30
+ httpGet:
+ path: /healthcheck
+ port: 8080
+ scheme: HTTP
+ periodSeconds: 10
+ successThreshold: 1
+ timeoutSeconds: 1
+ terminationMessagePath: /dev/termination-log
+ terminationMessagePolicy: File
+ volumeMounts:
+ - mountPath: /certs
+ name: certs
+ readOnly: true
+ - mountPath: /etc/statsd-exporter
+ name: statsd-exporter-config
+ readOnly: true
+ dnsPolicy: ClusterFirst
+ restartPolicy: Always
+ schedulerName: default-scheduler
+ serviceAccountName: envoy-ratelimit
+ terminationGracePeriodSeconds: 300
+ volumes:
+ - name: certs
+ secret:
+ defaultMode: 420
+ secretName: envoy-rate-limit
+ - configMap:
+ defaultMode: 420
+ name: statsd-exporter-config
+ optional: true
+ name: statsd-exporter-config
+status: {}