Copilot commented on code in PR #67:
URL:
https://github.com/apache/skywalking-banyandb-helm/pull/67#discussion_r3586250801
##########
chart/templates/_helpers.tpl:
##########
@@ -84,6 +84,70 @@ Create the name of the service account to use
{{- end }}
{{- end }}
+{{/*
+Validate a generated Kubernetes name does not exceed a byte limit.
+Usage: include "banyandb.validateNameLength" (dict "name" $name "limit" 63
"description" "..." "release" .Release.Name)
+*/}}
+{{- define "banyandb.validateNameLength" -}}
+{{- $name := .name -}}
+{{- $limit := .limit | default 63 -}}
+{{- $description := .description -}}
+{{- $release := .release -}}
+{{- if gt (len $name) (int $limit) }}
+{{- fail (printf "%s '%s' is %d characters long, which exceeds the %d-byte
Kubernetes limit. Shorten the Helm release name '%s' or set a shorter
fullnameOverride." $description $name (len $name) $limit $release) }}
Review Comment:
The error message mixes units: it reports \"%d characters\" but compares
against a \"%d-byte\" limit. Align terminology to avoid confusion (e.g., use
\"bytes\" consistently or \"characters\" consistently in both the measured
length and the limit).
##########
chart/templates/standalone_statefulset.yaml:
##########
@@ -15,6 +15,7 @@ See the License for the specific language governing
permissions and
limitations under the License.
*/}}
+{{- include "banyandb.validateNames" . }}
Review Comment:
Including `banyandb.validateNames` at the top of many templates causes the
same validation (including iterating over roles) to run repeatedly during a
single `helm install/upgrade`, once per rendered manifest. Consider running
validation exactly once by adding a single dedicated rendered template (e.g.,
`templates/validate-names.yaml`) whose only content is the include, instead of
invoking it from every resource template.
##########
chart/templates/_helpers.tpl:
##########
@@ -84,6 +84,70 @@ Create the name of the service account to use
{{- end }}
{{- end }}
+{{/*
+Validate a generated Kubernetes name does not exceed a byte limit.
+Usage: include "banyandb.validateNameLength" (dict "name" $name "limit" 63
"description" "..." "release" .Release.Name)
+*/}}
+{{- define "banyandb.validateNameLength" -}}
+{{- $name := .name -}}
+{{- $limit := .limit | default 63 -}}
+{{- $description := .description -}}
+{{- $release := .release -}}
+{{- if gt (len $name) (int $limit) }}
+{{- fail (printf "%s '%s' is %d characters long, which exceeds the %d-byte
Kubernetes limit. Shorten the Helm release name '%s' or set a shorter
fullnameOverride." $description $name (len $name) $limit $release) }}
+{{- end }}
+{{- end }}
+
+{{/*
+Validate all generated resource names fit within Kubernetes limits.
+StatefulSet names must leave room for the controller-revision-hash suffix
+(<statefulset-name>-<10-char-hash>) so pod labels stay within 63 bytes.
+*/}}
+{{- define "banyandb.validateNames" -}}
+{{- $fullname := include "banyandb.fullname" . -}}
+{{- $release := .Release.Name -}}
+{{- $ctx := . -}}
Review Comment:
`$ctx` is assigned but not used within `banyandb.validateNames`. Remove it
to reduce clutter, or use it consistently if it was intended to be passed into
nested includes.
--
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]
