Conversation
* Add installation of Tracebloc client Helm chart - Updated install-k8s.ps1 to include a new function for installing the Tracebloc client Helm chart as step 6. - Created install-client-helm.sh script to handle user input for configuration values and manage Helm chart installation. - Modified cluster.sh to ensure necessary directories for Tracebloc are created before cluster setup. - Updated install.sh and install-k8s.sh to source the new installation script and call the installation function during the setup process. * Update install-client-helm.sh to set Helm repository name and chart name - Set TRACEBLOC_HELM_REPO_NAME to "tracebloc" for clarity. - Updated TRACEBLOC_CHART_NAME to "client" for consistency with naming conventions. * Update values schema and test cases for docker registry secret creation - Modified the values schema to allow the dockerRegistry type to be either an object or null, clarifying its usage for public images. - Updated test case descriptions to accurately reflect the behavior of docker registry secret creation when the 'create' field is omitted. * Add client ID and password prompts in installation scripts - Enhanced install-k8s.ps1 and install-client-helm.sh to include prompts for client ID and password during installation. - Provided a link for users to create a client, emphasizing that the setup is free.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| local gpu_val | ||
| if [[ "${GPU_VENDOR:-}" == "nvidia" ]]; then | ||
| gpu_val="nvidia.com/gpu=1" | ||
| info "NVIDIA GPU detected — setting GPU_LIMITS and GPU_REQUESTS to nvidia.com/gpu=1" |
There was a problem hiding this comment.
Bash GPU check missing NVIDIA driver verification
Medium Severity
The bash script checks only GPU_VENDOR == "nvidia" when deciding to set GPU resource requests/limits, but the equivalent PowerShell code in install-k8s.ps1 correctly checks both $GPU_VENDOR -eq "nvidia" -and $NVIDIA_DRIVER_OK. Per detect-gpu.sh, GPU_VENDOR is set to "nvidia" even when detected only via lspci without working drivers (NVIDIA_DRIVER_OK=false). This causes the bash script to write GPU_LIMITS and GPU_REQUESTS of "nvidia.com/gpu=1" into the values file even when the driver isn't installed, potentially preventing pods from scheduling.
- Changed the color of the client creation link in install-k8s.ps1 to white for better visibility. - Modified permissions in cluster.sh and install-client-helm.sh to restrict access to specific subdirectories instead of the entire HOST_DATA_DIR. - Added a command to set the permissions of the values.yaml file to 600 for enhanced security.
- Updated _extract_yaml_value function to handle single-quoted YAML values correctly by unescaping '' back to ' for proper credential management. - Refactored directory creation and permission setting in install_client_helm function to use _ensure_tracebloc_dirs for improved clarity and maintainability.
- Improved handling of quoted YAML scalars by stripping surrounding quotes and unescaping single-quoted values for accurate retrieval. - Removed deprecated hostPath configuration entries for clarity and to streamline the script.
| clusterScope: true | ||
|
|
||
| clientId: "$TB_CLIENT_ID" | ||
| clientPassword: '$TB_CLIENT_PASSWORD_ESCAPED' |
There was a problem hiding this comment.
clientId value not YAML-escaped unlike clientPassword
Low Severity
clientPassword is carefully single-quote-escaped for YAML (replacing ' with ''), but clientId is placed in YAML double quotes with no escaping at all. If a clientId contains ", \, or other YAML double-quote special characters, the generated values.yaml will be syntactically invalid or misinterpreted by the YAML parser, causing helm upgrade --install to fail. The same inconsistency exists in both the bash and PowerShell scripts.
Additional Locations (1)
- Added "required" fields for server, username, password, and email in the values schema to ensure necessary user information is provided during configuration.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
- Updated the values schema to enforce that server, username, password, and email are required only when the 'create' property is set to true, enhancing validation logic for user credential input.
1 participant
Note
Medium Risk
Modifies installer flow and Helm chart templating (StorageClass naming and imagePullSecrets/registry secret gating), which can change cluster resources created on upgrade and affect image pull behavior. Scope is contained to deployment/installation paths but could impact existing releases if assumptions about StorageClass or registry secrets differ.
Overview
Installer now deploys the Tracebloc client Helm chart as part of cluster setup. Both
install-k8s.shandinstall-k8s.ps1add a new final step that prompts for namespace + client credentials, generates avalues.yamlinHOST_DATA_DIR, and runshelm upgrade --installafter adding/updating thetraceblocHelm repo.Helm chart behavior changes around storage and private registries. StorageClass names are now release-unique when
storageClass.create: true(and templates/PVCs referencetracebloc.storageClassName), and Docker registry secrets +imagePullSecretsare only rendered whendockerRegistryis set anddockerRegistry.create: true(schema/tests updated; public images supported viadockerRegistry: null).Chart version is bumped to
1.0.2.Written by Cursor Bugbot for commit 9838758. This will update automatically on new commits. Configure here.