Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sentry: support TLS for Redis #21

Merged
merged 2 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/sentry/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ maintainers:
- name: sentry-kubernetes
name: sentry
type: application
version: 19.4.0
version: 19.4.0+1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not totally sure how helm will take to that versioning but we can check

Copy link
Contributor Author

@maciejp-ro maciejp-ro Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helm doesn't complain (docs say it uses semver 2.0, which specifies -prerelease and +metadata suffix). I used metadata to indicate local changes.

Linter complains about it, though, and I checked semver spec - it says that metadata should be ignored when comparing versions (it's for things like build id). Using prerelease suffix (9.4.0-plural1) might calm the linter down. I'm on mobile now but feel free to push an update to check - I allowed maintainer pushes on this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Managed to push the change, linter's still unhappy - and AFAIR Helm doesn't like it when you try to deploy changed chart without a version bump. I'm not sure how to proceed here, feel free to update this branch

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably fine, just wanted to confirm helm won't bork on it

11 changes: 11 additions & 0 deletions charts/sentry/templates/_helper.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,17 @@ Set redis port
{{- end -}}
{{- end -}}

{{/*
Set redis protocol (rediss for TLS, redis for plaintext)
*/}}
{{- define "sentry.redis.protocol" -}}
{{- if .Values.redis.enabled | ternary .Values.redis.tls.enabled .Values.externalRedis.tls -}}
rediss
{{- else -}}
redis
{{- end -}}
{{- end -}}

{{/*
Set redis password
*/}}
Expand Down
5 changes: 3 additions & 2 deletions charts/sentry/templates/configmap-relay.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{{- $redisHost := include "sentry.redis.host" . -}}
{{- $redisPort := include "sentry.redis.port" . -}}
{{- $redisPass := include "sentry.redis.password" . -}}
{{- $redisProtocol := include "sentry.redis.protocol" . -}}
apiVersion: v1
kind: ConfigMap
metadata:
Expand Down Expand Up @@ -33,9 +34,9 @@ data:
value: 50000000 # 50MB or bust
{{- if $redisPass }}
redis: "redis://:{{ $redisPass }}@{{ $redisHost }}:{{ $redisPort }}"
redis: {{ $redisProtocol }}://:{{ $redisPass }}@{{ $redisHost }}:{{ $redisPort }}"
{{- else }}
redis: "redis://{{ $redisHost }}:{{ $redisPort }}"
redis: {{ $redisProtocol }}://{{ $redisHost }}:{{ $redisPort }}"
{{- end }}
topics:
metrics_transactions: ingest-performance-metrics
Expand Down
9 changes: 7 additions & 2 deletions charts/sentry/templates/configmap-sentry.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{{- $redisHost := include "sentry.redis.host" . -}}
{{- $redisPort := include "sentry.redis.port" . -}}
{{- $redisPass := include "sentry.redis.password" . -}}
{{- $redisProtocol := include "sentry.redis.protocol" . -}}
{{- $redisTls := eq "rediss" $redisProtocol -}}
apiVersion: v1
kind: ConfigMap
metadata:
Expand Down Expand Up @@ -81,6 +83,9 @@ data:
{{- if $redisPass }}
password: {{ $redisPass | quote }}
{{- end }}
{{- if $redisTls }}
ssl: true
{{- end }}

################
# File storage #
Expand Down Expand Up @@ -211,9 +216,9 @@ data:
{{- if or (.Values.rabbitmq.enabled) (.Values.rabbitmq.host) }}
BROKER_URL = os.environ.get("BROKER_URL", "amqp://{{ .Values.rabbitmq.auth.username }}:{{ .Values.rabbitmq.auth.password }}@{{ template "sentry.rabbitmq.host" . }}:5672/{{ .Values.rabbitmq.vhost }}")
{{- else if $redisPass }}
BROKER_URL = os.environ.get("BROKER_URL", "redis://:{{ $redisPass }}@{{ $redisHost }}:{{ $redisPort }}/0")
BROKER_URL = os.environ.get("BROKER_URL", "{{ $redisProtocol }}://:{{ $redisPass }}@{{ $redisHost }}:{{ $redisPort }}/0")
{{- else }}
BROKER_URL = os.environ.get("BROKER_URL", "redis://{{ $redisHost }}:{{ $redisPort }}/0")
BROKER_URL = os.environ.get("BROKER_URL", "{{ $redisProtocol }}://{{ $redisHost }}:{{ $redisPort }}/0")
{{- end }}

#########
Expand Down
4 changes: 4 additions & 0 deletions charts/sentry/templates/configmap-snuba.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{{- $redisPass := include "sentry.redis.password" . -}}
{{- $redisTls := eq "rediss" (include "sentry.redis.protocol" .) -}}
apiVersion: v1
kind: ConfigMap
metadata:
Expand Down Expand Up @@ -70,6 +71,9 @@ data:
REDIS_PASSWORD = {{ $redisPass | quote }}
{{- end }}
REDIS_DB = int(env("REDIS_DB", 1))
{{- if $redisTls }}
REDIS_SSL = True
{{- end }}
{{- if .Values.metrics.enabled }}
DOGSTATSD_HOST = "{{ template "sentry.fullname" . }}-metrics"
Expand Down
1 change: 1 addition & 0 deletions charts/sentry/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ externalPostgresql:
username: postgres
externalRedis:
port: 6379
tls: false
filestore:
backend: filesystem
filesystem:
Expand Down
Loading