WPB-23896: Handle SonarQube yaml alerts by blackheaven · Pull Request #5160 · wireapp/wire-server

blackheaven · 2026-03-31T06:59:18Z

https://wearezeta.atlassian.net/browse/WPB-23896

Checklist

Add a new entry in an appropriate subdirectory of changelog.d
Read and follow the PR guidelines

battermann · 2026-03-31T07:45:05Z

charts/cassandra-migrations/templates/cassandra-certs.yaml

 data:
-  ca.pem: {{ include "tlsCaBrig" . | b64enc | quote }}
-{{- end}}
+   ca.pem: {{ include "tlsCaBrig" . | b64enc | quote }}


why is indentation increased from 2 to 3 spaces here?
(and also many other places)

My yaml formatter is a bit brittle, thanks, fixed.

battermann · 2026-03-31T07:58:36Z

charts/nginz/templates/configmap.yaml

this file does not render correctly

What do you mean precisely?

I just mean that there was an error during rendering:

Error: parse error at (wire-server/charts/nginz/templates/configmap.yaml:6): undefined variable "$nginx_conf"

It does not there's not $nginx_conf, how do you get this error?

I haven't done it for this branch, but you could helm template the chart with --set-string "foo=bar" for the missing values Helm will scream for.

I cannot find which arguments to give to evaluate this file :/

helm template test charts/nginz \ --set secrets.zAuth.publicKeys="test-key=test-value" \ --set secrets.basicAuth="user:pass"

The values don't really matter. But, the keys need to be defined

Thanks, no more parse error

battermann · 2026-03-31T08:23:16Z

charts/k8ssandra-test-cluster/values.yaml

+    memory: "128Mi"
+    cpu: "1"
+  limits:
+    memory: "512Mi"


are these values actually used?

I don't think so. The resources are defined here:

wire-server/charts/k8ssandra-test-cluster/templates/k8ssandra-cluster.yaml

Line 13 in 1135567

resources:

battermann · 2026-03-31T08:26:59Z

charts/outlook-addin/templates/deployment.yaml

    {{- include "outlook.labels" . | nindent 4 }}
 spec:
  replicas: 3
+  automountServiceAccountToken: false


This is in the deployment spec but should be in the pod spec instead.

battermann · 2026-03-31T08:28:23Z

charts/restund/values.yaml

+resources:
+  requests:
+    memory: "256Mi"
+    cpu: "100m"
+  limits:
+    memory: "512Mi"
+


Are these used?

I'm wondering if we're still using restund (predecessor of coturn) at all 🤔

I'll try to figure that out.

It's unused can will be deleted with https://wearezeta.atlassian.net/browse/WPB-24485 .

IMHO we can ignore it for now (and e.g. add an exclusion to Sonar).

Dropped and ignored

battermann · 2026-03-31T08:32:06Z

changelog.d/5-internal/WPB-23896

+Fixed SonarQube Helm template formatting, variable naming, and resource/RBAC issues in charts/:
+
+## Template Formatting (kubernetes:S6893) - 15 issues
+- charts/wire-server/templates/background-worker/configmap.yaml: Added whitespace after "{{" on lines 107, 110
+- charts/wire-server/templates/gundeck/configmap.yaml: Added whitespace after "{{" on lines 51, 62
+- charts/wire-server/templates/galley/configmap.yaml: Added whitespace before "}}" on line 88
+- charts/wire-server/templates/gundeck/deployment.yaml: Added whitespace before "}}" on line 52
+- charts/wire-server/templates/brig/tests/brig-integration.yaml: Added whitespace before "}}" on lines 51, 56, 122
+- charts/wire-server/templates/gundeck/tests/gundeck-integration.yaml: Added whitespace before "}}" on line 20
+- charts/wire-server/templates/spar/tests/spar-integration.yaml: Added whitespace before "}}" on line 23
+- charts/cassandra-migrations/templates/cassandra-certs.yaml: Added whitespace before "}}" on lines 18, 37, 56, 75
+- charts/elasticsearch-index/templates/elasticsearch-ca-secret.yaml: Added whitespace after "{{" on line 5
+- charts/nginx-ingress-services/templates/issuer.yaml: Added whitespace after "{{" and before "}}" on lines 6, 23, 32
+
+## Variable Naming (kubernetes:S117) - 4 issues
+- charts/nginz/templates/configmap.yaml: Renamed variables to match camelCase convention:
+  - $nginx_conf -> $nginxConf
+  - $external_env_domain -> $externalEnvDomain
+  - $deeplink_json -> $deeplinkJson
+  - $deeplink_html -> $deeplinkHtml
+  Updated all references accordingly
+
+## Resource Limits/Requests - 6 issues
+- charts/outlook-addin/templates/deployment.yaml: Added resources block with memory limit and memory/cpu requests
+- charts/restund/values.yaml: Added resources block with memory/cpu limits and requests
+- charts/k8ssandra-test-cluster/values.yaml: Added resources block with memory/cpu limits and requests
+- charts/backoffice/templates/tests/stern-integration.yaml: Added memory limit to existing resources
+- charts/wire-server/values.yaml: Added ephemeral-storage requests to background-worker and wire-server-enterprise resources
+
+## Service Account RBAC (kubernetes:S6865) - 5 issues
+- charts/wire-server/templates/cargohold/deployment.yaml: Added automountServiceAccountToken: false
+- charts/restund/templates/statefulset.yaml: Added automountServiceAccountToken: false
+- charts/outlook-addin/templates/deployment.yaml: Added automountServiceAccountToken: false
+- charts/k8ssandra-test-cluster/templates/check-cluster-job.yaml: Added automountServiceAccountToken: false
+- charts/backoffice/templates/tests/stern-integration.yaml: Added automountServiceAccountToken: false


this was helpful for reviewing, but I think it is too much when added to the changelog.
I think it would be better if we put these in the PR description, make the changelog itself much shorter.

Yeah, I'm agreeing with @battermann . Operators probably don't care about renaming variables.

However, the Resource Limits/Requests might be interesting beyond internal. Maybe, add a sentence in another changelog category?

supersven · 2026-03-31T09:03:30Z

charts/wire-server-enterprise/values.yaml

  requests:
    memory: "200Mi"
    cpu: "100m"
+    ephemeral-storage: "1Gi"


wire-server-enterprise doesn't seem to use any storage. 🤔

What's the goal of this change?

supersven · 2026-03-31T09:06:39Z

charts/nginx-ingress-services/templates/issuer.yaml

    # NOTE: this secret doesnt need to be created, it only gets a name with this
    privateKeySecretRef:
-      name: {{ include "nginx-ingress-services.getIssuerName" . -}}-account-key
+      name: {{ include "nginx-ingress-services.getIssuerName" . }}-account-key


What was wrong with the -? I thought, this is killing whitespace to the right? 🤔

charts/nginz/templates/configmap.yaml

supersven · 2026-03-31T09:17:56Z

charts/outlook-addin/templates/deployment.yaml

+          resources:
+            requests:
+              memory: "128Mi"
+              cpu: "100m"
+            limits:
+              memory: "256Mi"


I have to admit that I know almost nothing about the outlook-addin.

I'm wondering where these numbers come from / how they have been choosen?

Me neither, do we have monitoring on that, so I can have real-world values?

supersven · 2026-03-31T09:18:59Z

charts/wire-server/values.yaml

    requests:
      memory: "200Mi"
      cpu: "100m"
+      ephemeral-storage: "1Gi"


Does the background-worker need storage? 🤔

supersven · 2026-04-02T14:16:01Z

charts/cassandra-migrations/templates/cassandra-certs.yaml

    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
 type: Opaque
-data:
+ data:


Is this indentation intended?

Actually, this applies to most indentations in this file.

supersven · 2026-04-02T14:20:57Z

charts/nginz/templates/configmap.yaml

  {{- if and (hasKey .Values.nginx_conf "deeplink") (hasKey .Values.nginx_conf "external_env_domain") }}
-  {{- $backendURL := .Values.nginx_conf.deeplink.endpoints.backendURL }}
-  {{- $deeplink_json := $backendURL | replace "https://" "" | trimSuffix "/" | printf "%s-deeplink.json" }}
-  {{- $deeplink_html := $backendURL | replace "https://" "" | trimSuffix "/" | printf "%s-deeplink.html" }}
-  {{ $deeplink_json }}: |
+  {{- $deeplinkJson := .Values.nginx_conf.deeplink.endpoints.backendURL | replace "https://" "" | trimSuffix "/" | printf "%s-deeplink.json" }}
+  {{- $deeplinkHtml := .Values.nginx_conf.deeplink.endpoints.backendURL | replace "https://" "" | trimSuffix "/" | printf "%s-deeplink.html" }}


Can't we use the alias $nginxConf here?

supersven · 2026-04-02T14:21:39Z

charts/nginz/templates/configmap.yaml

    <html>
-      <head></head>
+      <head>
+        <title>Deeplink for {{ .Values.nginx_conf.deeplink.endpoints.backendURL }}</title>


Same here: There's an alias on .Values.nginx_conf ($nginxConf).

supersven · 2026-04-02T16:01:04Z

charts/cassandra-migrations/templates/cassandra-certs.yaml

+ type: Opaque
+  data:


This is YAML. We unfortunately have to be careful with indentation 😅

These two indents probably slipped through?

WPB-23896: Handle SonarQube yaml alerts

d7b97f0

blackheaven requested review from a team as code owners March 31, 2026 06:59

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Mar 31, 2026

battermann requested changes Mar 31, 2026

View reviewed changes

supersven reviewed Mar 31, 2026

View reviewed changes

charts/nginz/templates/configmap.yaml Show resolved Hide resolved

supersven reviewed Mar 31, 2026

View reviewed changes

supersven requested changes Mar 31, 2026

View reviewed changes

blackheaven added 3 commits March 31, 2026 14:44

fix: try something

1e580ed

fix: feedbacks

f99b72d

feat: another lint

4173142

blackheaven requested review from battermann and supersven April 1, 2026 15:59

Hello CI

28ee720

supersven reviewed Apr 2, 2026

View reviewed changes

fix: indent

d87ae19

blackheaven requested a review from supersven April 2, 2026 15:52

supersven reviewed Apr 2, 2026

View reviewed changes

blackheaven added 2 commits April 2, 2026 18:05

fix: indent

21d63d9

feat: outlook-addin resources/limits

a4be264

Conversation

blackheaven commented Mar 31, 2026

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

supersven Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

supersven Apr 2, 2026 •

edited

Loading