Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix(ci): empty tag inputs no longer override values.yaml image settings #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
fix(ci): empty tag inputs no longer override values.yaml image settings #76
Changes from all commits
a56f738ddd6a0e36e240020d3c61d316777edd722b6e198876665fb5606c5916244c385d1f00c47b5b9157c6981File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep
theia_cloud_tagas a tag-only override.This block switches the repository from
ghcr.io/eduide/eduide/...toghcr.io/eduide/eduide-cloud/...instead of only changing the tag. Any deployment that passestheia_cloud_tagwill therefore stop matching the images configured incharts/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
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: EduIDE/EduIDE-deployment
Length of output: 2961
🏁 Script executed:
Repository: EduIDE/EduIDE-deployment
Length of output: 805
🏁 Script executed:
Repository: 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/nulland “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
index/hasKeywhen a parent key may not exist (avoids nil-pointer evaluation). [3]defaultonly when you truly want “empty/false/0” to be treated as “unset”; otherwise handle booleans and zeros explicitly. [5]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 anddefaultnot 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 isnil(or a non-string type), template rendering fails (often asinvalid value; expected string). Also,regexMatchcan panic on problems, whilemustRegexMatchreports an error back to the template engine. [1]Safe patterns
1) Default
nilto an empty string (most common){{- $s := default "" .Values.someValue -}} {{- if regexMatch "^foo" $s -}} ... {{- end -}}This ensures
regexMatchalways receives a string.2) Guard the call (short-circuit)
{{- if and .Values.someValue (regexMatch "^foo" .Values.someValue) -}} ... {{- end -}}Only runs
regexMatchwhen 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.ccan error beforedefaultruns ifa/bis 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
hasKeybefore 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
containsandregexMatchfunctions expect string values and will fail with unclear errors ifsidecars[].imageis missing. Userequiredto catch missing images early with a clear failure message.Suggested diff
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
📝 Committable suggestion
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: EduIDE/EduIDE-deployment
Length of output: 748
Avoid PR-scoped image tags in deployment manifests.
Using
:pr-70makes 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
Uh oh!
There was an error while loading. Please reload this page.