fix(ci): empty tag inputs no longer override values.yaml image settings#76
fix(ci): empty tag inputs no longer override values.yaml image settings#76CodeByNikolas wants to merge 13 commits intomainfrom
Conversation
Aligns with EduIDE-Helm PR #11 which adds v1beta11 support. With helm_chart_tag=pr-11, CI will now resolve theia-cloud-crds:1.2.0-next.1.pr-11 from GHCR.
Points theia-cloud-crds at the eduide-fork conversion-webhook image (conversion-webhook:pr-70) which includes the v1beta11 AppDefinition mapper required for the sidecar redesign.
theia-no-ls:pr-46 and langserver-java:pr-46 no longer exist in the registry. The no-ls AppDefinitions reference images from ls1intum/theia directly, so these preloading entries served no purpose.
- preserve explicit mountWorkspace=false in Helm template (avoid default overriding false) - support sidecar imageTag/defaultImageTag fallback when sidecar image has no explicit tag - set landing default app to a defined sidecar app and remove stale additionalApps entries - remove hardcoded latest tags in sidecar appdefs to honor defaultImageTag overrides - preload no-ls IDE + sidecar langserver images used by test3
- replace ls1intum/theia image references with ghcr.io/eduide/eduide for no-ls IDE and sidecar images - align test3 appdefinitions and preloading with PR-tagged image pipeline
- add explicit workflow overrides for preloading image indices 7..10 - ensure java-17-no-ls/rust-no-ls and langserver-java/langserver-rust track IDE tag input
- Change theia_cloud_tag and ide_images_tag defaults from 'latest' to '' - Update input descriptions to clarify empty means no override - deploy-pr.yml: forward empty tags as-is (remove || 'latest' fallback) - deploy-theia.yml: apply --set image overrides only when tag is non-empty, otherwise leave values.yaml settings untouched; add set -euo pipefail
📝 WalkthroughWalkthroughThe PR modifies GitHub Actions workflows to allow optional image tag inputs (empty defaults instead of 'latest'), implements conditional Helm command building based on provided tags, bumps CRDs version from 1.2.0-next.0 to 1.2.0-next.1, updates AppDefinition API from v1beta10 to v1beta11 with sidecars support, and configures deployment environments with conversion webhook images and updated test app definitions using sidecar-based language servers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR primarily adjusts the GitHub Actions deployment workflows so that empty tag inputs no longer force Helm --set image overrides, allowing the image settings in each environment’s values.yaml to remain authoritative unless an explicit tag is provided.
Changes:
- Update
deploy-pr.ymlanddeploy-theia.ymlinputs/defaults so empty tags don’t fall back tolatest, and only apply Helm image overrides when tag inputs are non-empty. - Refactor the Helm install step in
deploy-theia.ymlto build a basehelm upgrade --installcommand and conditionally append overrides. - Include additional chart + environment configuration changes (AppDefinition apiVersion/sidecars rendering, CRD conversion webhook image, test3 values updates, garbage-collector image repository change).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/deploy-theia.yml |
Makes image overrides conditional on non-empty inputs; refactors Helm invocation; bumps CRDs chart version. |
.github/workflows/deploy-pr.yml |
Removes ` |
charts/theia-appdefinitions/templates/appdefinition.yaml |
Switches AppDefinition apiVersion to v1beta11 and adds spec.sidecars rendering. |
charts/theia-cloud-combined/values.yaml |
Changes garbage-collector image repository value. |
deployments/theia.artemis.cit.tum.de/theia-crds-helm-values.yml |
Pins CRD conversion webhook image to :pr-70. |
deployments/theia-staging.artemis.cit.tum.de/theia-crds-helm-values.yml |
Pins CRD conversion webhook image to :pr-70. |
deployments/test1.theia-test.artemis.cit.tum.de/theia-crds-helm-values.yml |
Pins CRD conversion webhook image to :pr-70. |
deployments/test2.theia-test.artemis.cit.tum.de/theia-crds-helm-values.yml |
Pins CRD conversion webhook image to :pr-70. |
deployments/test3.theia-test.artemis.cit.tum.de/theia-crds-helm-values.yml |
Pins CRD conversion webhook image to :pr-70 (also removes the header comment). |
deployments/test3.theia-test.artemis.cit.tum.de/values.yaml |
Updates test3 environment to sidecar-based appdefinitions + related image/app changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --set "theia-cloud.preloading.images[7]=ghcr.io/eduide/eduide/java-17-no-ls:${IDE_IMAGES_TAG}" | ||
| --set "theia-cloud.preloading.images[8]=ghcr.io/eduide/eduide/rust-no-ls:${IDE_IMAGES_TAG}" | ||
| --set "theia-cloud.preloading.images[9]=ghcr.io/eduide/eduide/langserver-java:${IDE_IMAGES_TAG}" | ||
| --set "theia-cloud.preloading.images[10]=ghcr.io/eduide/eduide/langserver-rust:${IDE_IMAGES_TAG}" |
| THEIA_CLOUD_BASE_VERSION="1.4.0-next.0" | ||
| THEIA_CLOUD_CRDS_VERSION="1.2.0-next.0" | ||
| THEIA_CLOUD_CRDS_VERSION="1.2.0-next.1" | ||
|
|
| clusterIssuer: theia-cloud-selfsigned-issuer | ||
|
|
||
| conversion: | ||
| image: ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70 |
| clusterIssuer: theia-cloud-selfsigned-issuer | ||
|
|
||
| conversion: | ||
| image: ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70 |
| theia-appdefinitions: | ||
| apps: | ||
| - name: java-ls-test | ||
| image: ghcr.io/eduide/eduide/theia-no-ls | ||
| imageTag: pr-46 | ||
| options: | ||
| langserver-image: ghcr.io/eduide/eduide/langserver-java:pr-46 | ||
| - name: java-17-no-ls | ||
| image: ghcr.io/eduide/eduide/java-17-no-ls | ||
| minInstances: 1 | ||
| sidecars: | ||
| - name: langserver | ||
| image: ghcr.io/eduide/eduide/langserver-java | ||
| port: 5000 | ||
| languages: [java] |
| replicaCount: 1 | ||
| image: | ||
| repository: ghcr.io/eduide/eduide/garbage-collector | ||
| repository: ghcr.io/eduide/garbage-collector | ||
| tag: latest |
| @@ -14,7 +14,7 @@ | |||
| {{- $maxInstances = get $existingSpec "maxInstances" }} | |||
| {{- end }} | |||
| --- | |||
| apiVersion: theia.cloud/v1beta10 | |||
| apiVersion: theia.cloud/v1beta11 | |||
| kind: AppDefinition | |||
| clusterIssuer: theia-cloud-selfsigned-issuer | ||
|
|
||
| conversion: | ||
| image: ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70 |
| clusterIssuer: theia-cloud-selfsigned-issuer | ||
|
|
||
| conversion: | ||
| image: ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70 |
| clusterIssuer: theia-cloud-selfsigned-issuer | ||
|
|
||
| conversion: | ||
| image: ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.github/workflows/deploy-pr.yml (1)
50-51: Normalizetheia_cloud_tagandide_images_tagto matchhelm_chart_tagpattern.The workflow runs on both
pull_requestandworkflow_dispatchevents. Forpull_requestruns,inputs.*are unset and evaluate to null. This is already handled forhelm_chart_tagusing|| ''but missing fortheia_cloud_tagandide_images_tag. Add the same normalization for consistency.Suggested diff
- theia_cloud_tag: ${{ inputs.theia_cloud_tag }} - ide_images_tag: ${{ inputs.ide_images_tag }} + theia_cloud_tag: ${{ inputs.theia_cloud_tag || '' }} + ide_images_tag: ${{ inputs.ide_images_tag || '' }}Also applies to: 68-69, 86-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-pr.yml around lines 50 - 51, Normalize theia_cloud_tag and ide_images_tag wherever they are set to mirror the existing helm_chart_tag pattern: replace occurrences that assign theia_cloud_tag: ${{ inputs.theia_cloud_tag }} and ide_images_tag: ${{ inputs.ide_images_tag }} with the normalized form using a fallback empty string (e.g., theia_cloud_tag: ${{ inputs.theia_cloud_tag || '' }} and ide_images_tag: ${{ inputs.ide_images_tag || '' }}), and apply the same change for the other two places mentioned (the second and third blocks where these variables appear) so all three tags (helm_chart_tag, theia_cloud_tag, ide_images_tag) consistently use the || '' normalization..github/workflows/deploy-theia.yml (1)
255-268: Consider preserving each environment’s IDE preload list whenide_images_tagis set.This block still rewrites
theia-cloud.preloading.images[1..10]to a shared fixed set. That means tagged deployments stop respecting environment-specific image catalogs; for example,deployments/test3.theia-test.artemis.cit.tum.de/values.yamlnow exposes only the sidecar-based apps, but a tagged deploy would still force the legacy preload entries back in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-theia.yml around lines 255 - 268, The current workflow branch unconditionally overwrites theia-cloud.preloading.images[1..10] when IDE_IMAGES_TAG is set, which discards environment-specific preload lists; instead remove the hardcoded --set "theia-cloud.preloading.images[...]" entries from the HELM_CMD block and only set theia-appdefinitions.defaultImageTag="${IDE_IMAGES_TAG}" (i.e., keep the IDE_IMAGES_TAG handling but do not touch theia-cloud.preloading.images in the IDE_IMAGES_TAG branch), so that existing environment values files continue to provide their own preloading lists while tagged deploys only inject the shared tag via theia-appdefinitions.defaultImageTag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deploy-theia.yml:
- Around line 245-250: The current helm overrides replace the full image
repository/name instead of just the tag; update the HELM_CMD entries that set
theia-cloud.landingPage.image, theia-cloud.operator.image,
theia-cloud.service.image and theia-cloud.preloading.images[0] so they only
override the image tag (e.g. use
theia-cloud.landingPage.image.tag="${THEIA_CLOUD_TAG}",
theia-cloud.operator.image.tag="${THEIA_CLOUD_TAG}",
theia-cloud.service.image.tag="${THEIA_CLOUD_TAG}" and
theia-cloud.preloading.images[0].tag="${THEIA_CLOUD_TAG}" or the equivalent
tag-only keys used by the chart) to preserve the repository/name from
charts/theia-cloud-combined/values.yaml while changing only the tag.
In `@charts/theia-appdefinitions/templates/appdefinition.yaml`:
- Around line 61-66: The template should explicitly validate that each sidecar
image string is present before calling contains/regexMatch: change the
assignment of $sidecarImage to use the required function (e.g., set
$sidecarImage := required "missing sidecar image for sidecar <name or index>"
.image) so a clear failure occurs if .image is absent; keep the existing image
tag logic that checks contains "@sha256:" and regexMatch and still appends
.imageTag | default $.Values.defaultImageTag | default "latest" when
appropriate. Ensure the required message is descriptive (mention sidecar name or
index) and reference $sidecarImage, .image, and .imageTag in the fix.
In `@deployments/test3.theia-test.artemis.cit.tum.de/values.yaml`:
- Around line 102-103: Update the landing-page copy by removing the internal PR
reference from the infoText value: edit the infoText string that currently
contains "PR `#70`" so it reads without the PR mention (e.g., "Test environment
for Theia with external language server architecture. The IDE runs separately
from the language server for improved performance and modularity."), leaving
infoTitle unchanged.
In `@deployments/theia.artemis.cit.tum.de/theia-crds-helm-values.yml`:
- Around line 4-5: The manifest uses a PR-scoped image tag in the
conversion.image field (ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70);
replace this ephemeral tag with an immutable release tag or digest (e.g., a
semantically versioned tag or sha256 digest) so deployments are reproducible and
stable across environments, update any other occurrences of :pr-* in your helm
values to the chosen release tag/digest, and ensure the CI/CD pipeline and
imagePullSecrets (if any) are updated accordingly.
---
Nitpick comments:
In @.github/workflows/deploy-pr.yml:
- Around line 50-51: Normalize theia_cloud_tag and ide_images_tag wherever they
are set to mirror the existing helm_chart_tag pattern: replace occurrences that
assign theia_cloud_tag: ${{ inputs.theia_cloud_tag }} and ide_images_tag: ${{
inputs.ide_images_tag }} with the normalized form using a fallback empty string
(e.g., theia_cloud_tag: ${{ inputs.theia_cloud_tag || '' }} and ide_images_tag:
${{ inputs.ide_images_tag || '' }}), and apply the same change for the other two
places mentioned (the second and third blocks where these variables appear) so
all three tags (helm_chart_tag, theia_cloud_tag, ide_images_tag) consistently
use the || '' normalization.
In @.github/workflows/deploy-theia.yml:
- Around line 255-268: The current workflow branch unconditionally overwrites
theia-cloud.preloading.images[1..10] when IDE_IMAGES_TAG is set, which discards
environment-specific preload lists; instead remove the hardcoded --set
"theia-cloud.preloading.images[...]" entries from the HELM_CMD block and only
set theia-appdefinitions.defaultImageTag="${IDE_IMAGES_TAG}" (i.e., keep the
IDE_IMAGES_TAG handling but do not touch theia-cloud.preloading.images in the
IDE_IMAGES_TAG branch), so that existing environment values files continue to
provide their own preloading lists while tagged deploys only inject the shared
tag via theia-appdefinitions.defaultImageTag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8718c52d-0a30-4b00-b864-8882ed879787
📒 Files selected for processing (10)
.github/workflows/deploy-pr.yml.github/workflows/deploy-theia.ymlcharts/theia-appdefinitions/templates/appdefinition.yamlcharts/theia-cloud-combined/values.yamldeployments/test1.theia-test.artemis.cit.tum.de/theia-crds-helm-values.ymldeployments/test2.theia-test.artemis.cit.tum.de/theia-crds-helm-values.ymldeployments/test3.theia-test.artemis.cit.tum.de/theia-crds-helm-values.ymldeployments/test3.theia-test.artemis.cit.tum.de/values.yamldeployments/theia-staging.artemis.cit.tum.de/theia-crds-helm-values.ymldeployments/theia.artemis.cit.tum.de/theia-crds-helm-values.yml
| if [ -n "${THEIA_CLOUD_TAG}" ]; then | ||
| HELM_CMD+=( | ||
| --set theia-cloud.landingPage.image="ghcr.io/eduide/eduide-cloud/landing-page:${THEIA_CLOUD_TAG}" | ||
| --set theia-cloud.operator.image="ghcr.io/eduide/eduide-cloud/operator:${THEIA_CLOUD_TAG}" | ||
| --set theia-cloud.service.image="ghcr.io/eduide/eduide-cloud/service:${THEIA_CLOUD_TAG}" | ||
| --set "theia-cloud.preloading.images[0]=ghcr.io/eduide/eduide-cloud/landing-page:${THEIA_CLOUD_TAG}" |
There was a problem hiding this comment.
Keep theia_cloud_tag as a tag-only override.
This block switches the repository from ghcr.io/eduide/eduide/... to ghcr.io/eduide/eduide-cloud/... instead of only changing the tag. Any deployment that passes theia_cloud_tag will therefore stop matching the images configured in charts/theia-cloud-combined/values.yaml.
Proposed fix
if [ -n "${THEIA_CLOUD_TAG}" ]; then
HELM_CMD+=(
- --set theia-cloud.landingPage.image="ghcr.io/eduide/eduide-cloud/landing-page:${THEIA_CLOUD_TAG}"
- --set theia-cloud.operator.image="ghcr.io/eduide/eduide-cloud/operator:${THEIA_CLOUD_TAG}"
- --set theia-cloud.service.image="ghcr.io/eduide/eduide-cloud/service:${THEIA_CLOUD_TAG}"
- --set "theia-cloud.preloading.images[0]=ghcr.io/eduide/eduide-cloud/landing-page:${THEIA_CLOUD_TAG}"
+ --set theia-cloud.landingPage.image="ghcr.io/eduide/eduide/landing-page:${THEIA_CLOUD_TAG}"
+ --set theia-cloud.operator.image="ghcr.io/eduide/eduide/operator:${THEIA_CLOUD_TAG}"
+ --set theia-cloud.service.image="ghcr.io/eduide/eduide/service:${THEIA_CLOUD_TAG}"
+ --set "theia-cloud.preloading.images[0]=ghcr.io/eduide/eduide/landing-page:${THEIA_CLOUD_TAG}"
)
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -n "${THEIA_CLOUD_TAG}" ]; then | |
| HELM_CMD+=( | |
| --set theia-cloud.landingPage.image="ghcr.io/eduide/eduide-cloud/landing-page:${THEIA_CLOUD_TAG}" | |
| --set theia-cloud.operator.image="ghcr.io/eduide/eduide-cloud/operator:${THEIA_CLOUD_TAG}" | |
| --set theia-cloud.service.image="ghcr.io/eduide/eduide-cloud/service:${THEIA_CLOUD_TAG}" | |
| --set "theia-cloud.preloading.images[0]=ghcr.io/eduide/eduide-cloud/landing-page:${THEIA_CLOUD_TAG}" | |
| if [ -n "${THEIA_CLOUD_TAG}" ]; then | |
| HELM_CMD+=( | |
| --set theia-cloud.landingPage.image="ghcr.io/eduide/eduide/landing-page:${THEIA_CLOUD_TAG}" | |
| --set theia-cloud.operator.image="ghcr.io/eduide/eduide/operator:${THEIA_CLOUD_TAG}" | |
| --set theia-cloud.service.image="ghcr.io/eduide/eduide/service:${THEIA_CLOUD_TAG}" | |
| --set "theia-cloud.preloading.images[0]=ghcr.io/eduide/eduide/landing-page:${THEIA_CLOUD_TAG}" | |
| ) | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-theia.yml around lines 245 - 250, The current helm
overrides replace the full image repository/name instead of just the tag; update
the HELM_CMD entries that set theia-cloud.landingPage.image,
theia-cloud.operator.image, theia-cloud.service.image and
theia-cloud.preloading.images[0] so they only override the image tag (e.g. use
theia-cloud.landingPage.image.tag="${THEIA_CLOUD_TAG}",
theia-cloud.operator.image.tag="${THEIA_CLOUD_TAG}",
theia-cloud.service.image.tag="${THEIA_CLOUD_TAG}" and
theia-cloud.preloading.images[0].tag="${THEIA_CLOUD_TAG}" or the equivalent
tag-only keys used by the chart) to preserve the repository/name from
charts/theia-cloud-combined/values.yaml while changing only the tag.
| {{- $sidecarImage := .image }} | ||
| {{- if or (contains "@sha256:" $sidecarImage) (regexMatch ".*:[^/]+$" $sidecarImage) }} | ||
| image: {{ $sidecarImage }} | ||
| {{- else }} | ||
| image: {{ $sidecarImage }}:{{ .imageTag | default $.Values.defaultImageTag | default "latest" }} | ||
| {{- end }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check current values for sidecar blocks that might miss "image".
# Expected: no sidecar entries without image.
rg -n -A8 -B2 'sidecars:' charts deploymentsRepository: EduIDE/EduIDE-deployment
Length of output: 2961
🏁 Script executed:
#!/bin/bash
# Find all sidecar entries across all deployment values files
# Check if any sidecar is missing the image field
find deployments -name "values.yaml" -o -name "*.yaml" | xargs rg -A10 "^\s+- name:" | rg -B10 "sidecars:" | grep -E "(sidecars:|name:|image:)" | head -100Repository: EduIDE/EduIDE-deployment
Length of output: 805
🏁 Script executed:
#!/bin/bash
# More direct: find all sidecar entries and check for missing image field
rg -n -A15 "sidecars:" deployments --type yaml | grep -E "(sidecars:|name:|image:|port:|languages:)" | head -150Repository: EduIDE/EduIDE-deployment
Length of output: 1198
🌐 Web query:
Helm template contains function nil value behavior
💡 Result:
In Helm templates (Go text/template + Sprig), nil/null and “missing values” mainly matter in two places: field access and “emptiness” checks.
-
Accessing nested fields on a missing map/object can hard-fail with errors like
nil pointer evaluating interface {}(e.g.,.Values.foo.barwhenfooisn’t set). This happens before functions likedefaultcan run if the evaluation of.Values.foo.baritself panics. A common workaround is to avoid direct dot-chains and use safer access patterns (index,hasKey, etc.). [1], [2], [3] -
defaultuses “empty” semantics, not “is nil” semantics. In Helm,default X YreturnsXwhenYis considered “empty”. “Empty” includesnil,"",0,false, and empty lists/maps—sodefault true .Values.flagwill override an explicitly-setfalse. [4], [5] -
requiredfails on empty values using the same emptiness rules (so it fails fornil,"", etc.). [4]
Practical patterns
- Prefer
index/hasKeywhen a parent key may not exist (avoids nil-pointer evaluation). [3] - Use
defaultonly when you truly want “empty/false/0” to be treated as “unset”; otherwise handle booleans and zeros explicitly. [5] - If you expected
defaultto protect you from missing intermediate objects, it won’t in cases where the intermediate lookup panics (this is a known/recurring issue pattern). [2]
Sources: Helm function docs and guides on default/empty/required, plus community reports on nil-pointer evaluation and default not preventing it in missing-parent scenarios. [1], [2], [4], [5]
[1] https://helm.sh/docs/v3/chart_template_guide/functions_and_pipelines/
[2] helm/helm#8026
[3] https://stackoverflow.com/questions/59571842/helm-charts-how-do-i-use-default-on-undefined-object-property-values
[4] https://blog.helm.sh/docs/chart_template_guide/function_list/
[5] https://stackoverflow.com/questions/74257084/how-to-pass-false-value-to-helm-default
🌐 Web query:
Helm regexMatch function nil value error handling
💡 Result:
In Helm, regexMatch (Sprig) expects its second argument to be a string; if the value is nil (or a non-string type), template rendering fails (often as invalid value; expected string). Also, regexMatch can panic on problems, while mustRegexMatch reports an error back to the template engine. [1]
Safe patterns
1) Default nil to an empty string (most common)
{{- $s := default "" .Values.someValue -}}
{{- if regexMatch "^foo" $s -}}
...
{{- end -}}This ensures regexMatch always receives a string.
2) Guard the call (short-circuit)
{{- if and .Values.someValue (regexMatch "^foo" .Values.someValue) -}}
...
{{- end -}}Only runs regexMatch when the value is non-empty/non-nil.
3) If the value is nested and parent maps may be missing, don’t dot-walk directly
Dot-walking like .Values.a.b.c can error before default runs if a/b is missing; instead, default intermediate maps first: [2]
{{- $a := default dict .Values.a -}}
{{- $b := default dict $a.b -}}
{{- $s := default "" $b.c -}}
{{- if regexMatch "^foo" $s -}} ... {{- end -}}4) Use hasKey before accessing missing parents (alternative)
{{- if hasKey .Values "a" -}}
{{- /* then safely read .Values.a ... */ -}}
{{- end -}}This avoids nil-pointer-style failures when parent keys don’t exist. [3]
Sources: Sprig string/regex function docs [1]; Helm templating nil/default handling examples on missing nested keys [2], [3].
Add explicit validation for required sidecar image.
The contains and regexMatch functions expect string values and will fail with unclear errors if sidecars[].image is missing. Use required to catch missing images early with a clear failure message.
Suggested diff
- {{- $sidecarImage := .image }}
+ {{- $sidecarImage := required "sidecars[].image is required" .image }}
{{- if or (contains "@sha256:" $sidecarImage) (regexMatch ".*:[^/]+$" $sidecarImage) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/theia-appdefinitions/templates/appdefinition.yaml` around lines 61 -
66, The template should explicitly validate that each sidecar image string is
present before calling contains/regexMatch: change the assignment of
$sidecarImage to use the required function (e.g., set $sidecarImage := required
"missing sidecar image for sidecar <name or index>" .image) so a clear failure
occurs if .image is absent; keep the existing image tag logic that checks
contains "@sha256:" and regexMatch and still appends .imageTag | default
$.Values.defaultImageTag | default "latest" when appropriate. Ensure the
required message is descriptive (mention sidecar name or index) and reference
$sidecarImage, .image, and .imageTag in the fix.
| infoTitle: "🚀 Welcome to Test3 IDE Environment" | ||
| infoText: "This is a testing environment for Theia Cloud. Select your programming language and start coding in seconds!" | ||
| infoText: "Test environment for Theia with external language server architecture (PR #70). The IDE runs separately from the language server for improved performance and modularity." |
There was a problem hiding this comment.
Drop the internal PR reference from the landing-page copy.
PR #70`` is user-visible here, but it is internal context and will go stale quickly.
Suggested wording
- infoText: "Test environment for Theia with external language server architecture (PR `#70`). The IDE runs separately from the language server for improved performance and modularity."
+ infoText: "Test environment for Theia with external language server architecture. The IDE runs separately from the language server for improved performance and modularity."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| infoTitle: "🚀 Welcome to Test3 IDE Environment" | |
| infoText: "This is a testing environment for Theia Cloud. Select your programming language and start coding in seconds!" | |
| infoText: "Test environment for Theia with external language server architecture (PR #70). The IDE runs separately from the language server for improved performance and modularity." | |
| infoTitle: "🚀 Welcome to Test3 IDE Environment" | |
| infoText: "Test environment for Theia with external language server architecture. The IDE runs separately from the language server for improved performance and modularity." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deployments/test3.theia-test.artemis.cit.tum.de/values.yaml` around lines 102
- 103, Update the landing-page copy by removing the internal PR reference from
the infoText value: edit the infoText string that currently contains "PR `#70`" so
it reads without the PR mention (e.g., "Test environment for Theia with external
language server architecture. The IDE runs separately from the language server
for improved performance and modularity."), leaving infoTitle unchanged.
| conversion: | ||
| image: ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where PR-scoped conversion webhook tags are committed.
# Expected for long-lived envs: no pr-* tags.
rg -nP 'conversion-webhook:pr-\d+' deploymentsRepository: EduIDE/EduIDE-deployment
Length of output: 748
Avoid PR-scoped image tags in deployment manifests.
Using :pr-70 makes deployments dependent on a potentially ephemeral CI artifact. This tag pattern appears across multiple environments (production, staging, test). Replace with an immutable release tag or digest for reproducibility and stability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deployments/theia.artemis.cit.tum.de/theia-crds-helm-values.yml` around lines
4 - 5, The manifest uses a PR-scoped image tag in the conversion.image field
(ghcr.io/eduide/eduide-cloud/conversion-webhook:pr-70); replace this ephemeral
tag with an immutable release tag or digest (e.g., a semantically versioned tag
or sha256 digest) so deployments are reproducible and stable across
environments, update any other occurrences of :pr-* in your helm values to the
chosen release tag/digest, and ensure the CI/CD pipeline and imagePullSecrets
(if any) are updated accordingly.
Summary
values.yamlContext
This fix was validated and merged into the feature branch via PR #75. This PR proposes the same change to
mainfor human approval.Commit
57c6981fix(ci): empty tag inputs no longer override values.yaml image settingsSummary by CodeRabbit
Release Notes
New Features
Chores