chore: Update secret values for Redis

[alexandreferris]

---

PR Submission Checklist for internal contributors

• The PR Title

  • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
  • contains a reference JIRA issue number like SQPIT-764
  • answers the question: If merged, this PR will: ... ³

• The PR Description

  • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

---

What's new in this PR?

Solutions

• Remove direct values for Redis
• Add values based on secret

[claude]

PR Review: chore: Update secret values for Redis

Thanks for moving the Redis connection details from plain env values to secretKeyRefs — sourcing host/port from a secret is the right direction. A few issues to address before merge:

🔴 Blocking: redis values block is missing from values.yaml

The template now references .Values.redis.secretName, .Values.redis.hostKey, and .Values.redis.portKey:

- name: GHAPP_REDIS_HOST
  valueFrom:
    secretKeyRef:
      name: {{ .Values.redis.secretName }}
      key: {{ .Values.redis.hostKey }}

but there is no redis: section anywhere in helm/githubapp/values.yaml (only the two old GHAPP_REDIS_* plain values were removed). With redis undefined, helm template will either fail with a nil pointer evaluating interface {}.secretName error, or render an empty name:/key: — and an empty secretKeyRef.name/key is rejected by the API server. As written, the chart no longer installs.

Please add a matching block to values.yaml, e.g.:

redis:
  secretName: "githubapp-redis-secrets"
  hostKey: "GHAPP_REDIS_HOST"
  portKey: "GHAPP_REDIS_PORT"

🟠 Redis env is gated on the wrong condition

The new env vars were added inside the existing {{- if .Values.secrets.secretName }} block. This couples Redis configuration to the unrelated secrets.secretName (the Wire SDK secret): if secrets.secretName is empty the Redis vars silently disappear; if it is set but redis is not configured you hit the nil/empty problem above. Consider gating Redis on its own guard so the two concerns are independent:

{{- if .Values.redis }}
- name: GHAPP_REDIS_HOST
  valueFrom:
    secretKeyRef:
      name: {{ .Values.redis.secretName }}
      key: {{ .Values.redis.hostKey }}
- name: GHAPP_REDIS_PORT
  valueFrom:
    secretKeyRef:
      name: {{ .Values.redis.secretName }}
      key: {{ .Values.redis.portKey }}
{{- end }}

GHAPP_REDIS_HOST/GHAPP_REDIS_PORT are required by the app (Modules.kt -> RedisClient.create("$ENV_VAR_REDIS_HOST:$ENV_VAR_REDIS_PORT")), so they should not be optional in practice — an explicit guard avoids a confusing partial deployment.

🟡 Minor

• Consistency: the other secret keys in this template (WIRE_SDK_API_TOKEN, WIRE_SDK_CRYPTOGRAPHY_STORAGE_PASSWORD) use hard-coded literal key names rather than configurable *Key values. Either approach is fine, but mixing the two styles in one file is a little inconsistent — worth a deliberate choice.
• Docs: if there is a values reference in README.md or chart docs, add the new redis block there so operators know which secret keys to provision.

Security

Good improvement — Redis host/port no longer sit as plaintext chart values. (They are not especially sensitive on their own, but consistency with secret-based sourcing is reasonable.)

Test coverage

There are no automated chart tests in the repo, so nothing is verifiable in CI. Please at least run helm template helm/githubapp (or helm lint) locally with a populated redis block to confirm the rendered output before merging — the current diff would not render successfully on its own.

---

Summary: the template change is sound, but it is missing the matching redis values definition, which makes the chart un-renderable as-is. Adding that block (and ideally a dedicated guard) resolves the blocker.