fix(ci): empty tag inputs no longer override values.yaml image settings#77
Conversation
- 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
📝 WalkthroughWalkthroughUpdated GitHub Actions deployment workflows to allow empty image-tag inputs to defer to Helm values.yaml (removed 'latest' defaults), removed per-environment gating in PR-triggered deploy jobs, and added stricter shell error handling plus conditional Helm override logic that only applies when tags are provided. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHubActions as Actions
participant Runner
participant DeployScript as deploy-theia.yml
participant Helm
participant K8s as Kubernetes
User->>Actions: open PR / workflow_dispatch
Actions->>Runner: start jobs (deploy-test*)
Runner->>DeployScript: execute deploy steps (with inputs)
DeployScript->>DeployScript: build HELM_CMD (conditionally append tag overrides if non-empty)
DeployScript->>Helm: helm upgrade --install "${HELM_CMD[@]}"
Helm->>K8s: apply chart / image tags
K8s-->>Helm: status
Helm-->>Runner: exit status
Runner-->>Actions: job result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 fixes CI workflows so that empty tag inputs no longer override image settings defined in values.yaml. Previously, the defaults were 'latest', which would always override values.yaml settings even when no tag was intended. Now, empty strings are the default and image overrides are applied conditionally.
Changes:
- Default values for
theia_cloud_tagandide_images_tagchanged from'latest'to''in bothdeploy-pr.ymlanddeploy-theia.yml, with updated descriptions. deploy-pr.ymlno longer applies|| 'latest'fallbacks when forwarding tag inputs.deploy-theia.ymlrestructured to build the Helm command as a bash array, conditionally appending image overrides only when tags are non-empty, and addsset -euo pipefailfor safer script execution.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/deploy-pr.yml |
Change tag defaults to empty, remove 'latest' fallbacks, update descriptions |
.github/workflows/deploy-theia.yml |
Change tag defaults to empty, conditionally apply image overrides via bash array, add set -euo pipefail |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When dispatching manually, selecting 'all' (the new default) triggers all three deploy-test* jobs in a single run. Each job is still gated by its individual environment approval, so operators can approve only the environments they want without having to re-trigger the workflow for each one.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 255-268: Add fallback definitions for the four preload images that
are only conditionally set by IDE_IMAGES_TAG so deployments still preload them
when IDE_IMAGES_TAG is empty: add default entries for theia-cloud.preloading
images corresponding to java-17-no-ls, rust-no-ls, langserver-java and
langserver-rust (the same images referenced by the HELM_CMD --set lines
theia-cloud.preloading.images[7] through images[10]) into the base or
environment-specific values files (or set equivalent
theia-appdefinitions.defaultImageTag fallbacks) so they exist even when
IDE_IMAGES_TAG is not provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 977e28b3-c0ed-4508-bcb2-bbd985a32b03
📒 Files selected for processing (2)
.github/workflows/deploy-pr.yml.github/workflows/deploy-theia.yml
There was a problem hiding this comment.
Pull request overview
This PR fixes CI deployment workflows so that empty image tag inputs no longer override the values.yaml image settings with an empty string. Previously, the theia_cloud_tag and ide_images_tag inputs defaulted to 'latest' and were always passed as --set overrides, meaning values.yaml settings could never take effect. Now, the defaults are empty strings and --set image overrides are conditionally applied only when a tag is explicitly provided.
Changes:
- Change input defaults from
'latest'to''in bothdeploy-theia.ymlanddeploy-pr.yml, and remove the|| 'latest'fallback expressions when forwarding tags. - Make Helm
--setimage overrides conditional on non-empty tag values indeploy-theia.yml, using a bash array to build the command incrementally. Also addsset -euo pipefailfor robustness. - Remove the
environmentchoice input fromdeploy-pr.ymlso manual dispatch always triggers all three environment jobs (gated by environment approvals).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/deploy-theia.yml |
Defaults tags to empty, conditionally applies --set image overrides, adds set -euo pipefail |
.github/workflows/deploy-pr.yml |
Defaults tags to empty, removes ` |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/deploy-pr.yml (2)
42-54: Add apermissionsblock to follow the principle of least privilege.CodeQL flags that this workflow does not limit the permissions of the
GITHUB_TOKEN. Since this workflow calls a reusable workflow (deploy-theia.yml) that likely needs specific permissions, you should explicitly declare only what's required at the workflow or job level. This prevents over-privileged tokens if the workflow or called workflows are compromised.🔒 Proposed fix: Add minimal permissions block
Add a top-level
permissionsblock after line 13 (aftername:), or within each job. Example at workflow level:name: Deploy PR to Environment permissions: contents: read # Add other permissions as needed by deploy-theia.yml (e.g., id-token: write for OIDC)If the reusable workflow requires additional permissions (like
id-token: writefor cloud authentication), add those explicitly.🤖 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 42 - 54, Add an explicit permissions block to limit GITHUB_TOKEN scope for this workflow (or at least the deploy-test1 job) instead of using default full permissions; update the workflow (near the top-level name or inside the deploy-test1 job) to specify only the minimum permissions required by the reusable workflow deploy-theia.yml (for example contents: read and id-token: write if OIDC is needed) so the deploy-test1 job and inputs like theia_cloud_tag/ide_images_tag only run with least privilege.
43-43: Redundant conditions can be simplified.The condition
github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch'is always true since the workflow only triggers on these two events (defined in theon:block). The jobs will run regardless.That said, this is a minor nit—keeping explicit conditions can serve as defensive documentation if additional triggers are added later.
♻️ Optional: Remove redundant conditions
If you prefer brevity, you can remove the
if:clauses entirely since theon:block already constrains execution:deploy-test1: - if: github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' name: Deploy to Test1Apply similarly to
deploy-test2anddeploy-test3.Also applies to: 61-61, 79-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-pr.yml at line 43, The conditional "if: github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch'" in the deploy jobs is redundant because the workflow's on: block already restricts triggers; remove that if: clause from the deploy job(s) (the lines matching "if: github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch'") — apply the same removal for the deploy-test, deploy-test2, and deploy-test3 job definitions so the jobs rely on the on: triggers, or alternatively keep a single concise comment above the job explaining the defensive intent if you want to preserve explicitness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/deploy-pr.yml:
- Around line 42-54: Add an explicit permissions block to limit GITHUB_TOKEN
scope for this workflow (or at least the deploy-test1 job) instead of using
default full permissions; update the workflow (near the top-level name or inside
the deploy-test1 job) to specify only the minimum permissions required by the
reusable workflow deploy-theia.yml (for example contents: read and id-token:
write if OIDC is needed) so the deploy-test1 job and inputs like
theia_cloud_tag/ide_images_tag only run with least privilege.
- Line 43: The conditional "if: github.event_name == 'pull_request' ||
github.event_name == 'workflow_dispatch'" in the deploy jobs is redundant
because the workflow's on: block already restricts triggers; remove that if:
clause from the deploy job(s) (the lines matching "if: github.event_name ==
'pull_request' || github.event_name == 'workflow_dispatch'") — apply the same
removal for the deploy-test, deploy-test2, and deploy-test3 job definitions so
the jobs rely on the on: triggers, or alternatively keep a single concise
comment above the job explaining the defensive intent if you want to preserve
explicitness.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3614d3cb-9454-4d06-a7b0-6fa810e79357
📒 Files selected for processing (1)
.github/workflows/deploy-pr.yml
Summary
Cherry-pick of commit 57c6981 onto a clean base from
main.This is a targeted single-commit PR containing only the CI empty-tag fix, with no unrelated commits from feature branches.
Changes
theia_cloud_tagandide_images_tagdefaults from'latest'to''deploy-pr.yml: forward empty tags as-is (remove|| 'latest'fallback)deploy-theia.yml: apply--set imageoverrides only when tag is non-empty, otherwise leavevalues.yamlsettings untouched; addset -euo pipefailAdditional fix (latest commit)
deploy-pr.yml: removed theenvironmentselector fromworkflow_dispatchinputs entirely. Manual dispatch now always triggers all three environment jobs (test1, test2, test3) unconditionally. Per-environment approvals configured in GitHub Environment settings remain the gate before each deployment runs.Replaces
PR #76 (
fix/empty-tag-no-override → main) which was closed because it incorrectly included 13 unrelated commits from its branch ancestry.Summary by CodeRabbit