Copilot commented on code in PR #672: URL: https://github.com/apache/pulsar-helm-chart/pull/672#discussion_r3110314470
########## charts/pulsar/templates/jwt-secret-init.yaml: ########## @@ -0,0 +1,252 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# When JWT authentication is enabled and generateSecrets is true, this pre-install/pre-upgrade +# hook job generates JWT signing keys and per-subject tokens, storing them as Kubernetes secrets. +# The job is skipped entirely (not rendered) if the signing key secret already exists. +# Individual token secrets that already exist are also left untouched. +{{- if and .Values.auth.authentication.enabled .Values.auth.authentication.jwt.enabled .Values.auth.authentication.jwt.generateSecrets.enabled }} +{{- $ns := include "pulsar.namespace" . }} +{{- $keySecret := ternary "symmetric" "asymmetric" .Values.auth.authentication.jwt.usingSecretKey }} +{{- if not (lookup "v1" "Secret" $ns (printf "%s-token-%s-key" .Release.Name $keySecret)) }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +rules: + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "create", "patch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +subjects: + - kind: ServiceAccount + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" +roleRef: + kind: Role + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + apiGroup: rbac.authorization.k8s.io +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "0" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded +spec: + backoffLimit: 1 + template: + metadata: + labels: + {{- include "pulsar.template.labels" . | nindent 8 }} + component: jwt-secret-init + spec: + {{- include "pulsar.imagePullSecrets" . | nindent 6 }} + serviceAccountName: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + restartPolicy: Never + {{- if .Values.auth.authentication.jwt.generateSecrets.nodeSelector }} + nodeSelector: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.nodeSelector | indent 8 }} + {{- end }} + {{- if .Values.auth.authentication.jwt.generateSecrets.tolerations }} + tolerations: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.tolerations | indent 8 }} + {{- end }} + affinity: + {{- if and .Values.affinity.anti_affinity .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity }} + podAntiAffinity: + {{- if eq .Values.auth.authentication.jwt.generateSecrets.affinity.type "requiredDuringSchedulingIgnoredDuringExecution" }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- else }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - weight: 100 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- end }} + {{- end }} + volumes: + - name: jwt-secrets + emptyDir: + sizeLimit: 1Mi + initContainers: + - name: generate-jwt-artifacts + image: "{{ template "pulsar.imageFullName" (dict "image" .Values.images.broker "root" .) }}" + imagePullPolicy: {{ .Values.images.broker.pullPolicy | default .Values.defaultPullPolicy }} + command: + - /bin/bash + - -c + - | + set -e + {{- if .Values.auth.authentication.jwt.usingSecretKey }} + echo "Generating symmetric secret key..." + bin/pulsar tokens create-secret-key --output /jwt-secrets/secret.key + {{- range $role, $subject := .Values.auth.superUsers }} + {{- if $subject }} + echo "Generating token for subject '{{ $subject }}'..." + bin/pulsar tokens create \ + --secret-key /jwt-secrets/secret.key \ + --subject "{{ $subject }}" | tr -d '\n' > "/jwt-secrets/{{ $subject }}.token" + {{- end }} + {{- end }} + {{- else }} + echo "Generating RSA key pair..." + bin/pulsar tokens create-key-pair \ + -a RS256 \ + --output-private-key=/jwt-secrets/private.key \ + --output-public-key=/jwt-secrets/public.key + {{- range $role, $subject := .Values.auth.superUsers }} + {{- if $subject }} + echo "Generating token for subject '{{ $subject }}'..." + bin/pulsar tokens create \ + -a RS256 \ + --private-key=/jwt-secrets/private.key \ + --subject "{{ $subject }}" | tr -d '\n' > "/jwt-secrets/{{ $subject }}.token" + {{- end }} + {{- end }} + {{- end }} + echo "JWT artifact generation complete" + volumeMounts: + - mountPath: /jwt-secrets + name: jwt-secrets + containers: + - name: create-secrets + image: "{{ template "pulsar.imageFullName" (dict "image" .Values.images.kubectl "root" .) }}" + env: + - name: NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + command: + - /bin/sh + - -c + - | + set -e + + create_secret() { + SECRET_NAME="$1" + shift + if output=$(kubectl create secret generic "$SECRET_NAME" -n "$NAMESPACE" "$@" 2>&1); then + echo "Created secret $SECRET_NAME" + return 0 + elif echo "$output" | grep -q "already exists"; then + echo "Secret $SECRET_NAME already exists, skipping" + return 1 Review Comment: The container runs with `set -e`, but `create_secret` returns 1 when the secret "already exists". Since `create_secret` is called unguarded, any pre-existing secret will cause the script (and job) to exit early. Return 0 for the "already exists" case or otherwise prevent `set -e` from treating it as a fatal error. ```suggestion return 0 ``` ########## charts/pulsar/templates/jwt-secret-init.yaml: ########## @@ -0,0 +1,252 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# When JWT authentication is enabled and generateSecrets is true, this pre-install/pre-upgrade +# hook job generates JWT signing keys and per-subject tokens, storing them as Kubernetes secrets. +# The job is skipped entirely (not rendered) if the signing key secret already exists. +# Individual token secrets that already exist are also left untouched. +{{- if and .Values.auth.authentication.enabled .Values.auth.authentication.jwt.enabled .Values.auth.authentication.jwt.generateSecrets.enabled }} +{{- $ns := include "pulsar.namespace" . }} +{{- $keySecret := ternary "symmetric" "asymmetric" .Values.auth.authentication.jwt.usingSecretKey }} +{{- if not (lookup "v1" "Secret" $ns (printf "%s-token-%s-key" .Release.Name $keySecret)) }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +rules: + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "create", "patch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +subjects: + - kind: ServiceAccount + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" +roleRef: + kind: Role + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + apiGroup: rbac.authorization.k8s.io +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "0" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded +spec: + backoffLimit: 1 + template: + metadata: + labels: + {{- include "pulsar.template.labels" . | nindent 8 }} + component: jwt-secret-init + spec: + {{- include "pulsar.imagePullSecrets" . | nindent 6 }} + serviceAccountName: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + restartPolicy: Never + {{- if .Values.auth.authentication.jwt.generateSecrets.nodeSelector }} + nodeSelector: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.nodeSelector | indent 8 }} + {{- end }} + {{- if .Values.auth.authentication.jwt.generateSecrets.tolerations }} + tolerations: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.tolerations | indent 8 }} + {{- end }} + affinity: + {{- if and .Values.affinity.anti_affinity .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity }} + podAntiAffinity: + {{- if eq .Values.auth.authentication.jwt.generateSecrets.affinity.type "requiredDuringSchedulingIgnoredDuringExecution" }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- else }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - weight: 100 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- end }} + {{- end }} + volumes: + - name: jwt-secrets + emptyDir: + sizeLimit: 1Mi + initContainers: + - name: generate-jwt-artifacts + image: "{{ template "pulsar.imageFullName" (dict "image" .Values.images.broker "root" .) }}" + imagePullPolicy: {{ .Values.images.broker.pullPolicy | default .Values.defaultPullPolicy }} Review Comment: The broker initContainer sets `imagePullPolicy` directly from values instead of using the chart’s `pulsar.imagePullPolicy` helper (used widely elsewhere). Using the helper keeps behavior consistent (defaulting/quoting) and ensures future changes to pull-policy logic apply here too. ```suggestion imagePullPolicy: {{ include "pulsar.imagePullPolicy" (dict "image" .Values.images.broker "root" .) }} ``` ########## charts/pulsar/templates/jwt-secret-init.yaml: ########## @@ -0,0 +1,252 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# When JWT authentication is enabled and generateSecrets is true, this pre-install/pre-upgrade +# hook job generates JWT signing keys and per-subject tokens, storing them as Kubernetes secrets. +# The job is skipped entirely (not rendered) if the signing key secret already exists. +# Individual token secrets that already exist are also left untouched. +{{- if and .Values.auth.authentication.enabled .Values.auth.authentication.jwt.enabled .Values.auth.authentication.jwt.generateSecrets.enabled }} +{{- $ns := include "pulsar.namespace" . }} +{{- $keySecret := ternary "symmetric" "asymmetric" .Values.auth.authentication.jwt.usingSecretKey }} +{{- if not (lookup "v1" "Secret" $ns (printf "%s-token-%s-key" .Release.Name $keySecret)) }} Review Comment: The hook job is not rendered if the signing key secret already exists (lookup check). This prevents generating new per-subject token secrets on later upgrades (e.g., adding a new entry in auth.superUsers) and can also leave the release stuck if the first run created the key secret but failed before creating all token secrets. Consider always rendering/running the job and making it idempotent: skip key *creation* if it exists, but still create any missing token secrets (by reading the existing key secret). ```suggestion # The hook must always be rendered so upgrades can create any newly required token secrets # and recover from partial prior runs; idempotency is handled by the job logic itself. {{- if and .Values.auth.authentication.enabled .Values.auth.authentication.jwt.enabled .Values.auth.authentication.jwt.generateSecrets.enabled }} {{- $ns := include "pulsar.namespace" . }} {{- $keySecret := ternary "symmetric" "asymmetric" .Values.auth.authentication.jwt.usingSecretKey }} {{- if true }} ``` ########## charts/pulsar/templates/jwt-secret-init.yaml: ########## @@ -0,0 +1,252 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# When JWT authentication is enabled and generateSecrets is true, this pre-install/pre-upgrade +# hook job generates JWT signing keys and per-subject tokens, storing them as Kubernetes secrets. +# The job is skipped entirely (not rendered) if the signing key secret already exists. +# Individual token secrets that already exist are also left untouched. +{{- if and .Values.auth.authentication.enabled .Values.auth.authentication.jwt.enabled .Values.auth.authentication.jwt.generateSecrets.enabled }} +{{- $ns := include "pulsar.namespace" . }} +{{- $keySecret := ternary "symmetric" "asymmetric" .Values.auth.authentication.jwt.usingSecretKey }} +{{- if not (lookup "v1" "Secret" $ns (printf "%s-token-%s-key" .Release.Name $keySecret)) }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +rules: + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "create", "patch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +subjects: + - kind: ServiceAccount + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" +roleRef: + kind: Role + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + apiGroup: rbac.authorization.k8s.io +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "0" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded +spec: + backoffLimit: 1 + template: + metadata: + labels: + {{- include "pulsar.template.labels" . | nindent 8 }} + component: jwt-secret-init + spec: + {{- include "pulsar.imagePullSecrets" . | nindent 6 }} + serviceAccountName: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + restartPolicy: Never + {{- if .Values.auth.authentication.jwt.generateSecrets.nodeSelector }} + nodeSelector: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.nodeSelector | indent 8 }} + {{- end }} + {{- if .Values.auth.authentication.jwt.generateSecrets.tolerations }} + tolerations: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.tolerations | indent 8 }} + {{- end }} + affinity: + {{- if and .Values.affinity.anti_affinity .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity }} + podAntiAffinity: + {{- if eq .Values.auth.authentication.jwt.generateSecrets.affinity.type "requiredDuringSchedulingIgnoredDuringExecution" }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- else }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - weight: 100 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- end }} + {{- end }} + volumes: + - name: jwt-secrets + emptyDir: + sizeLimit: 1Mi + initContainers: + - name: generate-jwt-artifacts + image: "{{ template "pulsar.imageFullName" (dict "image" .Values.images.broker "root" .) }}" + imagePullPolicy: {{ .Values.images.broker.pullPolicy | default .Values.defaultPullPolicy }} + command: + - /bin/bash + - -c + - | + set -e + {{- if .Values.auth.authentication.jwt.usingSecretKey }} + echo "Generating symmetric secret key..." + bin/pulsar tokens create-secret-key --output /jwt-secrets/secret.key + {{- range $role, $subject := .Values.auth.superUsers }} + {{- if $subject }} + echo "Generating token for subject '{{ $subject }}'..." + bin/pulsar tokens create \ + --secret-key /jwt-secrets/secret.key \ + --subject "{{ $subject }}" | tr -d '\n' > "/jwt-secrets/{{ $subject }}.token" Review Comment: Token generation uses a pipeline (`bin/pulsar ... | tr ... > file`) while the script only has `set -e`. Without `set -o pipefail`, failures in `bin/pulsar` can be masked and still write empty/invalid token files. Add `set -o pipefail` (and ideally `-u`) near the top and apply to both symmetric/asymmetric token-generation blocks. ########## charts/pulsar/values.yaml: ########## @@ -347,6 +349,31 @@ auth: # If the token is generated by a secret key, set the usingSecretKey as true. # If the token is generated by a private key, set the usingSecretKey as false. usingSecretKey: false + # When enabled, a pre-install/pre-upgrade hook job generates JWT signing keys and + # per-subject tokens as Kubernetes secrets. Skipped if the signing key secret already exists. + generateSecrets: + enabled: false + # nodeSelector: + # cloud.google.com/gke-nodepool: default-pool + tolerations: [] + affinity: + anti_affinity: false + anti_affinity_topology_key: kubernetes.io/hostname + # Valid values: + # requiredDuringSchedulingIgnoredDuringExecution - rules must be met for pod to be scheduled (hard) + # preferredDuringSchedulingIgnoredDuringExecution - scheduler will try to enforce but not guarantee + type: preferredDuringSchedulingIgnoredDuringExecution + # Annotations to apply to secrets created by the generateSecrets job + secretAnnotations: + # Annotations added to the signing key secret (asymmetric-key or symmetric-key) + key: {} + # Per-subject annotations for token secrets. Map of subject name to annotation map. + # Example: + # admin: + # reflector.v1.k8s.emberstack.com/reflection-allowed: "true" + # reflector.v1.k8s.emberstack.com/reflection-auto-enabled: "true" + # broker-admin: {} Review Comment: `secretAnnotations.token` is documented as a map keyed by *subject name*, but the init job indexes it by the *role key* from `auth.superUsers` (e.g., broker/proxy/client/manager). Update this comment/example to match the actual key used so users can configure annotations correctly. ```suggestion # Per-role annotations for token secrets. Map of auth.superUsers role key to annotation map # (for example: broker, proxy, client, manager). # Example: # client: # reflector.v1.k8s.emberstack.com/reflection-allowed: "true" # reflector.v1.k8s.emberstack.com/reflection-auto-enabled: "true" # broker: {} ``` ########## charts/pulsar/templates/jwt-secret-init.yaml: ########## @@ -0,0 +1,252 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# When JWT authentication is enabled and generateSecrets is true, this pre-install/pre-upgrade +# hook job generates JWT signing keys and per-subject tokens, storing them as Kubernetes secrets. +# The job is skipped entirely (not rendered) if the signing key secret already exists. +# Individual token secrets that already exist are also left untouched. +{{- if and .Values.auth.authentication.enabled .Values.auth.authentication.jwt.enabled .Values.auth.authentication.jwt.generateSecrets.enabled }} +{{- $ns := include "pulsar.namespace" . }} +{{- $keySecret := ternary "symmetric" "asymmetric" .Values.auth.authentication.jwt.usingSecretKey }} +{{- if not (lookup "v1" "Secret" $ns (printf "%s-token-%s-key" .Release.Name $keySecret)) }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +rules: + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "create", "patch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +subjects: + - kind: ServiceAccount + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" +roleRef: + kind: Role + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + apiGroup: rbac.authorization.k8s.io +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "0" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded +spec: + backoffLimit: 1 + template: + metadata: + labels: + {{- include "pulsar.template.labels" . | nindent 8 }} + component: jwt-secret-init + spec: + {{- include "pulsar.imagePullSecrets" . | nindent 6 }} + serviceAccountName: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + restartPolicy: Never + {{- if .Values.auth.authentication.jwt.generateSecrets.nodeSelector }} + nodeSelector: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.nodeSelector | indent 8 }} + {{- end }} + {{- if .Values.auth.authentication.jwt.generateSecrets.tolerations }} + tolerations: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.tolerations | indent 8 }} + {{- end }} + affinity: + {{- if and .Values.affinity.anti_affinity .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity }} + podAntiAffinity: + {{- if eq .Values.auth.authentication.jwt.generateSecrets.affinity.type "requiredDuringSchedulingIgnoredDuringExecution" }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- else }} + {{ .Values.auth.authentication.jwt.generateSecrets.affinity.type }}: + - weight: 100 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: "app" + operator: In + values: + - "{{ template "pulsar.name" . }}" + - key: "release" + operator: In + values: + - {{ .Release.Name }} + - key: "component" + operator: In + values: + - jwt-secret-init + topologyKey: {{ .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity_topology_key }} + {{- end }} + {{- end }} + volumes: + - name: jwt-secrets + emptyDir: + sizeLimit: 1Mi + initContainers: + - name: generate-jwt-artifacts + image: "{{ template "pulsar.imageFullName" (dict "image" .Values.images.broker "root" .) }}" + imagePullPolicy: {{ .Values.images.broker.pullPolicy | default .Values.defaultPullPolicy }} + command: + - /bin/bash + - -c + - | + set -e + {{- if .Values.auth.authentication.jwt.usingSecretKey }} + echo "Generating symmetric secret key..." + bin/pulsar tokens create-secret-key --output /jwt-secrets/secret.key + {{- range $role, $subject := .Values.auth.superUsers }} + {{- if $subject }} + echo "Generating token for subject '{{ $subject }}'..." + bin/pulsar tokens create \ + --secret-key /jwt-secrets/secret.key \ + --subject "{{ $subject }}" | tr -d '\n' > "/jwt-secrets/{{ $subject }}.token" + {{- end }} + {{- end }} + {{- else }} + echo "Generating RSA key pair..." + bin/pulsar tokens create-key-pair \ + -a RS256 \ + --output-private-key=/jwt-secrets/private.key \ + --output-public-key=/jwt-secrets/public.key + {{- range $role, $subject := .Values.auth.superUsers }} + {{- if $subject }} + echo "Generating token for subject '{{ $subject }}'..." + bin/pulsar tokens create \ + -a RS256 \ + --private-key=/jwt-secrets/private.key \ + --subject "{{ $subject }}" | tr -d '\n' > "/jwt-secrets/{{ $subject }}.token" + {{- end }} + {{- end }} + {{- end }} + echo "JWT artifact generation complete" + volumeMounts: + - mountPath: /jwt-secrets + name: jwt-secrets + containers: + - name: create-secrets + image: "{{ template "pulsar.imageFullName" (dict "image" .Values.images.kubectl "root" .) }}" Review Comment: The kubectl container doesn’t set `imagePullPolicy`, so `images.kubectl.pullPolicy` (added in values.yaml) won’t be honored for this job. Add `imagePullPolicy` using the `pulsar.imagePullPolicy` helper for the kubectl image. ```suggestion image: "{{ template "pulsar.imageFullName" (dict "image" .Values.images.kubectl "root" .) }}" imagePullPolicy: {{ include "pulsar.imagePullPolicy" (dict "image" .Values.images.kubectl "root" .) }} ``` ########## charts/pulsar/templates/jwt-secret-init.yaml: ########## @@ -0,0 +1,252 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# When JWT authentication is enabled and generateSecrets is true, this pre-install/pre-upgrade +# hook job generates JWT signing keys and per-subject tokens, storing them as Kubernetes secrets. +# The job is skipped entirely (not rendered) if the signing key secret already exists. +# Individual token secrets that already exist are also left untouched. +{{- if and .Values.auth.authentication.enabled .Values.auth.authentication.jwt.enabled .Values.auth.authentication.jwt.generateSecrets.enabled }} +{{- $ns := include "pulsar.namespace" . }} +{{- $keySecret := ternary "symmetric" "asymmetric" .Values.auth.authentication.jwt.usingSecretKey }} +{{- if not (lookup "v1" "Secret" $ns (printf "%s-token-%s-key" .Release.Name $keySecret)) }} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +rules: + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "create", "patch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "-10" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed +subjects: + - kind: ServiceAccount + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" +roleRef: + kind: Role + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + apiGroup: rbac.authorization.k8s.io +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + namespace: {{ $ns }} + labels: + {{- include "pulsar.standardLabels" . | nindent 4 }} + component: jwt-secret-init + annotations: + "helm.sh/hook": pre-install,pre-upgrade + "helm.sh/hook-weight": "0" + "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded +spec: + backoffLimit: 1 + template: + metadata: + labels: + {{- include "pulsar.template.labels" . | nindent 8 }} + component: jwt-secret-init + spec: + {{- include "pulsar.imagePullSecrets" . | nindent 6 }} + serviceAccountName: "{{ template "pulsar.fullname" . }}-jwt-secret-init" + restartPolicy: Never + {{- if .Values.auth.authentication.jwt.generateSecrets.nodeSelector }} + nodeSelector: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.nodeSelector | indent 8 }} + {{- end }} + {{- if .Values.auth.authentication.jwt.generateSecrets.tolerations }} + tolerations: +{{ toYaml .Values.auth.authentication.jwt.generateSecrets.tolerations | indent 8 }} + {{- end }} + affinity: + {{- if and .Values.affinity.anti_affinity .Values.auth.authentication.jwt.generateSecrets.affinity.anti_affinity }} + podAntiAffinity: Review Comment: `affinity:` is always emitted, but the only child (`podAntiAffinity:`) is conditionally emitted. With the default job values (generateSecrets.affinity.anti_affinity: false), enabling generateSecrets will render an empty `affinity:` key and produce invalid YAML. Wrap the entire `affinity:` block in the conditional, or emit `affinity: {}` when anti-affinity is disabled. ########## charts/pulsar/templates/jwt-secret-init.yaml: ########## @@ -0,0 +1,252 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# When JWT authentication is enabled and generateSecrets is true, this pre-install/pre-upgrade +# hook job generates JWT signing keys and per-subject tokens, storing them as Kubernetes secrets. +# The job is skipped entirely (not rendered) if the signing key secret already exists. +# Individual token secrets that already exist are also left untouched. Review Comment: The header comment says existing token secrets are left untouched, but the job always runs `kubectl annotate ... --overwrite` even when the secret already exists. Either update the comment to reflect that annotations may be applied/overwritten on existing secrets, or change the logic to only annotate newly created secrets (or make overwriting configurable). ```suggestion # Individual token secrets that already exist are not recreated, but their annotations may be # applied or overwritten by the job. ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
