Skip to content

Commit cf84927

Browse files
oscarboherzirainarkodg
authored
fix: rateLimitDeployment ignoring pod labels and annotation merge (envoyproxy#4228)
* fix labels and annotation merges for rate limit deployment Signed-off-by: Oscar Boher <[email protected]> * fix tests and label merge Signed-off-by: Oscar Boher <[email protected]> * fix annotation merge if prometheus was disabled and annotations were defined Signed-off-by: Oscar Boher <[email protected]> * renamed labels and annotations to specify they apply to pods only Signed-off-by: Oscar Boher <[email protected]> * linter Signed-off-by: Oscar Boher <[email protected]> * fix resource provider tests to new annotation behavior Signed-off-by: Oscar Boher <[email protected]> * go linter Signed-off-by: Oscar Boher <[email protected]> * fix gen-check Signed-off-by: Oscar Boher <[email protected]> * pod labels selector comment Signed-off-by: Oscar Boher <[email protected]> --------- Signed-off-by: Oscar Boher <[email protected]> Co-authored-by: zirain <[email protected]> Co-authored-by: Arko Dasgupta <[email protected]>
1 parent db1f437 commit cf84927

File tree

11 files changed

+391
-8
lines changed

11 files changed

+391
-8
lines changed

‎internal/infrastructure/kubernetes/ratelimit/resource_provider.go‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
_ "embed"
1010
"strconv"
1111

12+
"golang.org/x/exp/maps"
1213
appsv1 "k8s.io/api/apps/v1"
1314
autoscalingv2 "k8s.io/api/autoscaling/v2"
1415
corev1 "k8s.io/api/core/v1"
@@ -196,19 +197,30 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
196197
}
197198

198199
containers := expectedRateLimitContainers(r.rateLimit, r.rateLimitDeployment, r.Namespace)
199-
labels := rateLimitLabels()
200-
selector := resource.GetSelector(labels)
200+
selector := resource.GetSelector(rateLimitLabels())
201+
202+
podLabels := rateLimitLabels()
203+
if r.rateLimitDeployment.Pod.Labels != nil {
204+
maps.Copy(podLabels, r.rateLimitDeployment.Pod.Labels)
205+
// Copy overwrites values in the dest map if they exist in the src map https://pkg.go.dev/maps#Copy
206+
// It's applied again with the rateLimitLabels that are used as deployment selector to ensure those are not overwritten by user input
207+
maps.Copy(podLabels, rateLimitLabels())
208+
}
201209

202-
var annotations map[string]string
210+
var podAnnotations map[string]string
203211
if enablePrometheus(r.rateLimit) {
204-
annotations = map[string]string{
212+
podAnnotations = map[string]string{
205213
"prometheus.io/path": "/metrics",
206214
"prometheus.io/port": strconv.Itoa(PrometheusPort),
207215
"prometheus.io/scrape": "true",
208216
}
209217
}
210218
if r.rateLimitDeployment.Pod.Annotations != nil {
211-
annotations = r.rateLimitDeployment.Pod.Annotations
219+
if podAnnotations != nil {
220+
maps.Copy(podAnnotations, r.rateLimitDeployment.Pod.Annotations)
221+
} else {
222+
podAnnotations = r.rateLimitDeployment.Pod.Annotations
223+
}
212224
}
213225

214226
deployment := &appsv1.Deployment{
@@ -218,16 +230,16 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) {
218230
},
219231
ObjectMeta: metav1.ObjectMeta{
220232
Namespace: r.Namespace,
221-
Labels: labels,
233+
Labels: rateLimitLabels(),
222234
},
223235
Spec: appsv1.DeploymentSpec{
224236
Replicas: r.rateLimitDeployment.Replicas,
225237
Strategy: *r.rateLimitDeployment.Strategy,
226238
Selector: selector,
227239
Template: corev1.PodTemplateSpec{
228240
ObjectMeta: metav1.ObjectMeta{
229-
Labels: selector.MatchLabels,
230-
Annotations: annotations,
241+
Labels: podLabels,
242+
Annotations: podAnnotations,
231243
},
232244
Spec: corev1.PodSpec{
233245
Containers: containers,

‎internal/infrastructure/kubernetes/ratelimit/resource_provider_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"flag"
1010
"fmt"
1111
"os"
12+
"strconv"
1213
"testing"
1314

1415
"github.com/stretchr/testify/assert"
@@ -678,6 +679,50 @@ func TestDeployment(t *testing.T) {
678679
},
679680
},
680681
},
682+
{
683+
caseName: "merge-labels",
684+
rateLimit: &egv1a1.RateLimit{
685+
Backend: egv1a1.RateLimitDatabaseBackend{
686+
Type: egv1a1.RedisBackendType,
687+
Redis: &egv1a1.RateLimitRedisSettings{
688+
URL: "redis.redis.svc:6379",
689+
},
690+
},
691+
},
692+
deploy: &egv1a1.KubernetesDeploymentSpec{
693+
Pod: &egv1a1.KubernetesPodSpec{
694+
Labels: map[string]string{
695+
"app.kubernetes.io/name": InfraName,
696+
"app.kubernetes.io/component": "ratelimit",
697+
"app.kubernetes.io/managed-by": "envoy-gateway",
698+
"key1": "value1",
699+
"key2": "value2",
700+
},
701+
},
702+
},
703+
},
704+
{
705+
caseName: "merge-annotations",
706+
rateLimit: &egv1a1.RateLimit{
707+
Backend: egv1a1.RateLimitDatabaseBackend{
708+
Type: egv1a1.RedisBackendType,
709+
Redis: &egv1a1.RateLimitRedisSettings{
710+
URL: "redis.redis.svc:6379",
711+
},
712+
},
713+
},
714+
deploy: &egv1a1.KubernetesDeploymentSpec{
715+
Pod: &egv1a1.KubernetesPodSpec{
716+
Annotations: map[string]string{
717+
"prometheus.io/path": "/metrics",
718+
"prometheus.io/port": strconv.Itoa(PrometheusPort),
719+
"prometheus.io/scrape": "true",
720+
"key1": "value1",
721+
"key2": "value2",
722+
},
723+
},
724+
},
725+
},
681726
}
682727
for _, tc := range cases {
683728
t.Run(tc.caseName, func(t *testing.T) {

‎internal/infrastructure/kubernetes/ratelimit/testdata/deployments/custom.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ spec:
2727
template:
2828
metadata:
2929
annotations:
30+
prometheus.io/path: /metrics
31+
prometheus.io/port: "19001"
3032
prometheus.io/scrape: "true"
3133
creationTimestamp: null
3234
labels:

‎internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default-env.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ spec:
2727
template:
2828
metadata:
2929
annotations:
30+
prometheus.io/path: /metrics
31+
prometheus.io/port: "19001"
3032
prometheus.io/scrape: "true"
3133
creationTimestamp: null
3234
labels:

‎internal/infrastructure/kubernetes/ratelimit/testdata/deployments/extension-env.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ spec:
2727
template:
2828
metadata:
2929
annotations:
30+
prometheus.io/path: /metrics
31+
prometheus.io/port: "19001"
3032
prometheus.io/scrape: "true"
3133
creationTimestamp: null
3234
labels:

‎internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-annotations.yaml‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
creationTimestamp: null
5+
labels:
6+
app.kubernetes.io/component: ratelimit
7+
app.kubernetes.io/managed-by: envoy-gateway
8+
app.kubernetes.io/name: envoy-ratelimit
9+
name: envoy-ratelimit
10+
namespace: envoy-gateway-system
11+
ownerReferences:
12+
- apiVersion: apps/v1
13+
kind: Deployment
14+
name: envoy-gateway
15+
uid: test-owner-reference-uid-for-deployment
16+
spec:
17+
progressDeadlineSeconds: 600
18+
revisionHistoryLimit: 10
19+
selector:
20+
matchLabels:
21+
app.kubernetes.io/component: ratelimit
22+
app.kubernetes.io/managed-by: envoy-gateway
23+
app.kubernetes.io/name: envoy-ratelimit
24+
strategy:
25+
type: RollingUpdate
26+
template:
27+
metadata:
28+
annotations:
29+
key1: value1
30+
key2: value2
31+
prometheus.io/path: /metrics
32+
prometheus.io/port: "19001"
33+
prometheus.io/scrape: "true"
34+
creationTimestamp: null
35+
labels:
36+
app.kubernetes.io/component: ratelimit
37+
app.kubernetes.io/managed-by: envoy-gateway
38+
app.kubernetes.io/name: envoy-ratelimit
39+
spec:
40+
automountServiceAccountToken: false
41+
containers:
42+
- command:
43+
- /bin/ratelimit
44+
env:
45+
- name: RUNTIME_ROOT
46+
value: /data
47+
- name: RUNTIME_SUBDIRECTORY
48+
value: ratelimit
49+
- name: RUNTIME_IGNOREDOTFILES
50+
value: "true"
51+
- name: RUNTIME_WATCH_ROOT
52+
value: "false"
53+
- name: LOG_LEVEL
54+
value: info
55+
- name: USE_STATSD
56+
value: "false"
57+
- name: CONFIG_TYPE
58+
value: GRPC_XDS_SOTW
59+
- name: CONFIG_GRPC_XDS_SERVER_URL
60+
value: envoy-gateway:18001
61+
- name: CONFIG_GRPC_XDS_NODE_ID
62+
value: envoy-ratelimit
63+
- name: GRPC_SERVER_USE_TLS
64+
value: "true"
65+
- name: GRPC_SERVER_TLS_CERT
66+
value: /certs/tls.crt
67+
- name: GRPC_SERVER_TLS_KEY
68+
value: /certs/tls.key
69+
- name: GRPC_SERVER_TLS_CA_CERT
70+
value: /certs/ca.crt
71+
- name: CONFIG_GRPC_XDS_SERVER_USE_TLS
72+
value: "true"
73+
- name: CONFIG_GRPC_XDS_CLIENT_TLS_CERT
74+
value: /certs/tls.crt
75+
- name: CONFIG_GRPC_XDS_CLIENT_TLS_KEY
76+
value: /certs/tls.key
77+
- name: CONFIG_GRPC_XDS_SERVER_TLS_CACERT
78+
value: /certs/ca.crt
79+
- name: FORCE_START_WITHOUT_INITIAL_CONFIG
80+
value: "true"
81+
- name: REDIS_SOCKET_TYPE
82+
value: tcp
83+
- name: REDIS_URL
84+
value: redis.redis.svc:6379
85+
- name: USE_PROMETHEUS
86+
value: "true"
87+
- name: PROMETHEUS_ADDR
88+
value: :19001
89+
- name: PROMETHEUS_MAPPER_YAML
90+
value: /etc/statsd-exporter/conf.yaml
91+
image: envoyproxy/ratelimit:master
92+
imagePullPolicy: IfNotPresent
93+
name: envoy-ratelimit
94+
ports:
95+
- containerPort: 8081
96+
name: grpc
97+
protocol: TCP
98+
readinessProbe:
99+
failureThreshold: 1
100+
httpGet:
101+
path: /healthcheck
102+
port: 8080
103+
scheme: HTTP
104+
periodSeconds: 5
105+
successThreshold: 1
106+
timeoutSeconds: 1
107+
resources:
108+
requests:
109+
cpu: 100m
110+
memory: 512Mi
111+
securityContext:
112+
allowPrivilegeEscalation: false
113+
capabilities:
114+
drop:
115+
- ALL
116+
privileged: false
117+
readOnlyRootFilesystem: true
118+
runAsGroup: 65534
119+
runAsNonRoot: true
120+
runAsUser: 65534
121+
seccompProfile:
122+
type: RuntimeDefault
123+
startupProbe:
124+
failureThreshold: 30
125+
httpGet:
126+
path: /healthcheck
127+
port: 8080
128+
scheme: HTTP
129+
periodSeconds: 10
130+
successThreshold: 1
131+
timeoutSeconds: 1
132+
terminationMessagePath: /dev/termination-log
133+
terminationMessagePolicy: File
134+
volumeMounts:
135+
- mountPath: /certs
136+
name: certs
137+
readOnly: true
138+
- mountPath: /etc/statsd-exporter
139+
name: statsd-exporter-config
140+
readOnly: true
141+
dnsPolicy: ClusterFirst
142+
restartPolicy: Always
143+
schedulerName: default-scheduler
144+
serviceAccountName: envoy-ratelimit
145+
terminationGracePeriodSeconds: 300
146+
volumes:
147+
- name: certs
148+
secret:
149+
defaultMode: 420
150+
secretName: envoy-rate-limit
151+
- configMap:
152+
defaultMode: 420
153+
name: statsd-exporter-config
154+
optional: true
155+
name: statsd-exporter-config
156+
status: {}

0 commit comments

Comments
 (0)