Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces branch-based upstream Helm checkout with an OCI tag-based chart flow in CI, migrates image references from Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant GHCR as GHCR (OCI Registry)
participant Helm as Helm CLI
participant Cluster as Kubernetes Cluster
participant CertMgr as cert-manager
CI->>GHCR: login (ghcr.io) + packages: read
CI->>CI: read `helm_chart_tag` and compute PREVIEW_VERSION
CI->>Helm: pull/install OCI chart@PREVIEW_VERSION (oci://ghcr.io/eduide/charts/...)
Helm->>Cluster: install CRDs & base charts
Helm->>Cluster: upgrade/install theia-cloud (uses eduide images & preloads)
CI->>Cluster: apply shared-gateway manifests (GatewayClass, EnvoyProxy, wildcard Secret)
Cluster->>CertMgr: cert-manager reconciles Certificates (if managedCertificates.enabled)
CertMgr->>Cluster: create/update TLS Secrets
Cluster->>CI: report deployment status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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 |
lukaskratzel
force-pushed
the
fix/org-migration
branch
from
25cb608 to
7cb6ac8
Compare
lukaskratzel
had a problem deploying
to
theia-prod
GitHub Actions
Failure
lukaskratzel
had a problem deploying
to
theia-prod
GitHub Actions
Failure
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy-theia.yml (1)
164-184:⚠️ Potential issue | 🟠 MajorDon't upgrade cluster-wide releases from every environment deploy.
These steps run on every invocation of the reusable workflow, but they mutate shared releases in
default(theia-cloud-base,theia-cloud-crds,theia-monitoring). A test/PR deployment using a customhelm_chart_branchcan therefore change cluster-wide components for all environments or race with another deployment. Please gate this behind a dedicated input/workflow and run it once per cluster.Possible guard
shared_gateway_namespace: description: "Namespace to install shared gateway release" required: false type: string default: "gateway-system" + deploy_cluster_prerequisites: + description: "Whether to install/update cluster-wide base, CRDs, and monitoring" + required: false + type: boolean + default: false ... - - name: Install Theia Cloud base and CRDs + - name: Install Theia Cloud base and CRDs + if: inputs.deploy_cluster_prerequisites ... - - name: Install cluster-wide monitoring + - name: Install cluster-wide monitoring + if: inputs.deploy_cluster_prerequisites🤖 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 164 - 184, The workflow currently upgrades cluster-wide releases (helm releases theia-cloud-base, theia-cloud-crds, theia-monitoring) on every reusable-workflow invocation; gate these steps behind a dedicated boolean input (e.g., input name deploy_cluster_wide or run_cluster_updates) and only run the three steps when that input is true, or move them into a separate workflow that is triggered manually / per-cluster; update the step names (Install Theia Cloud base and CRDs, Install cluster-wide monitoring) to check the input and exit when false so test/PR deploys never mutate shared releases in the default namespace and ensure the guard is documented in the workflow inputs/README.
🤖 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 230-236: The current helm --set calls hard-code indices of
theia-cloud.preloading.images (entries 0..6) which replace the entire array and
drop environment-specific additions; update deploy-theia.yml to stop setting
fixed indices for theia-cloud.preloading.images and instead supply the full
preload list for each environment (or merge from the environment values file) so
you don’t overwrite additional entries like theia-no-ls or langserver-java.
Locate the block that sets theia-cloud.preloading.images (using THEIA_CLOUD_TAG
and IDE_IMAGES_TAG) and change it to generate or load the complete array per
environment (e.g., build the full comma/JSON array from environment-specific
values or merge with values.yaml via yq/helm --values) so helm receives the
entire theia-cloud.preloading.images array rather than fixed index assignments.
---
Outside diff comments:
In @.github/workflows/deploy-theia.yml:
- Around line 164-184: The workflow currently upgrades cluster-wide releases
(helm releases theia-cloud-base, theia-cloud-crds, theia-monitoring) on every
reusable-workflow invocation; gate these steps behind a dedicated boolean input
(e.g., input name deploy_cluster_wide or run_cluster_updates) and only run the
three steps when that input is true, or move them into a separate workflow that
is triggered manually / per-cluster; update the step names (Install Theia Cloud
base and CRDs, Install cluster-wide monitoring) to check the input and exit when
false so test/PR deploys never mutate shared releases in the default namespace
and ensure the guard is documented in the workflow inputs/README.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5925f3c-0d6f-46c5-971c-9d6852fc4190
📒 Files selected for processing (11)
.github/workflows/deploy-theia.ymlcharts/theia-appdefinitions/templates/appdefinition.yamlcharts/theia-appdefinitions/values.yamlcharts/theia-cloud-combined/values.yamldeployments/test1.theia-test.artemis.cit.tum.de/values.yamldeployments/test2.theia-test.artemis.cit.tum.de/values.yamldeployments/test3.theia-test.artemis.cit.tum.de/values.yamldeployments/theia-staging.artemis.cit.tum.de/values.yamldeployments/theia.artemis.cit.tum.de/values.yamlvalue-reference-files/theia-cloud-helm-values.ymlvalue-reference-files/tum-theia-cloud-helm-test-values.yaml
lukaskratzel
temporarily deployed
to
theia-staging
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
charts/theia-shared-gateway/templates/wildcard-secret.yaml (1)
6-10: Avoid letting this Secret overlap with cert-manager-managed Secrets.
charts/theia-shared-gateway/templates/certificates.yaml:1-20can create aCertificatefor anysecretName. If that name matches.Values.wildcardTLSSecret.name, both resources can end up managing the same Secret. Consider making the two modes mutually exclusive, or validate/document that these names must not overlap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/theia-shared-gateway/templates/wildcard-secret.yaml` around lines 6 - 10, The Secret template uses .Values.wildcardTLSSecret.name which can conflict with the Certificate resource in charts/theia-shared-gateway/templates/certificates.yaml that may write to the same secretName; update the chart so the two modes are mutually exclusive — either (A) add a validation hook or template logic to prevent enabling both by checking values (e.g., validate that .Values.wildcardTLSSecret.name is empty when the Certificate is enabled or vice versa), or (B) change naming so the Certificate and the static Secret use distinct names (or document/require users to set different names). Locate and adjust the templates that reference .Values.wildcardTLSSecret.name and the Certificate's secretName to enforce the chosen approach..github/workflows/deploy-theia.yml (1)
163-175: Hardcoded versions may drift from Chart.yaml.The hardcoded
THEIA_CLOUD_BASE_VERSION="1.4.0-next.0"andTHEIA_CLOUD_CRDS_VERSION="1.2.0-next.0"could become inconsistent with thetheia-cloudversion declared incharts/theia-cloud-combined/Chart.yaml(currently1.4.0-next.0). Consider extracting these versions from a single source of truth.Additionally,
normalize_preview_version()is duplicated from the earlier step (lines 130-134). Extract it to a reusable script or define it once in a setup step.♻️ Suggestion to reduce duplication
Create a shared script
scripts/version-utils.sh:#!/bin/bash normalize_preview_version() { local base="$1" local tag="$2" printf '%s.%s' "$base" "$tag" }Then source it in both steps:
+ source ./scripts/version-utils.sh if [ -n "${{ inputs.helm_chart_tag }}" ]; then - normalize_preview_version() { - local base="$1" - local tag="$2" - printf '%s.%s' "$base" "$tag" - } THEIA_CLOUD_BASE_VERSION="$(normalize_preview_version "$THEIA_CLOUD_BASE_VERSION" "${{ inputs.helm_chart_tag }}")"🤖 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 163 - 175, Replace the hardcoded THEIA_CLOUD_BASE_VERSION and THEIA_CLOUD_CRDS_VERSION with values parsed from the single source of truth (charts/theia-cloud-combined/Chart.yaml) at workflow runtime (e.g., using yq or grep+sed to read the chart version and set those env vars), and remove the duplicate normalize_preview_version definition by extracting it into a shared script (e.g., scripts/version-utils.sh) that declares normalize_preview_version() and then source that script in both workflow steps so both can call normalize_preview_version without duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/theia-shared-gateway/templates/wildcard-secret.yaml`:
- Around line 1-10: When .Values.wildcardTLSSecret.create is true the template
should fail render if tls payloads are missing; wrap both
.Values.wildcardTLSSecret.certificate and .Values.wildcardTLSSecret.key with
Helm's required(...) so rendering errors out with a clear message instead of
producing null data—update the tls.crt and tls.key lines in the
wildcard-secret.yaml template to call required with an explicit error string
referencing the missing certificate or key when .Values.wildcardTLSSecret.create
is enabled.
---
Nitpick comments:
In @.github/workflows/deploy-theia.yml:
- Around line 163-175: Replace the hardcoded THEIA_CLOUD_BASE_VERSION and
THEIA_CLOUD_CRDS_VERSION with values parsed from the single source of truth
(charts/theia-cloud-combined/Chart.yaml) at workflow runtime (e.g., using yq or
grep+sed to read the chart version and set those env vars), and remove the
duplicate normalize_preview_version definition by extracting it into a shared
script (e.g., scripts/version-utils.sh) that declares
normalize_preview_version() and then source that script in both workflow steps
so both can call normalize_preview_version without duplication.
In `@charts/theia-shared-gateway/templates/wildcard-secret.yaml`:
- Around line 6-10: The Secret template uses .Values.wildcardTLSSecret.name
which can conflict with the Certificate resource in
charts/theia-shared-gateway/templates/certificates.yaml that may write to the
same secretName; update the chart so the two modes are mutually exclusive —
either (A) add a validation hook or template logic to prevent enabling both by
checking values (e.g., validate that .Values.wildcardTLSSecret.name is empty
when the Certificate is enabled or vice versa), or (B) change naming so the
Certificate and the static Secret use distinct names (or document/require users
to set different names). Locate and adjust the templates that reference
.Values.wildcardTLSSecret.name and the Certificate's secretName to enforce the
chosen approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97884834-ca7c-42bc-a715-40ad5df7f1e6
📒 Files selected for processing (6)
.github/workflows/deploy-pr.yml.github/workflows/deploy-theia.ymlREADME.mdcharts/theia-cloud-combined/Chart.yamlcharts/theia-shared-gateway/templates/wildcard-secret.yamldocs/adding-environments.md
🚧 Files skipped from review as they are similar to previous changes (4)
- README.md
- docs/adding-environments.md
- .github/workflows/deploy-pr.yml
- charts/theia-cloud-combined/Chart.yaml
This reverts commit 21c6de1.
lukaskratzel
temporarily deployed
to
theia-staging
GitHub Actions
Inactive
CodeByNikolas
approved these changes
Mar 10, 2026
Contributor
CodeByNikolas
left a comment
CodeByNikolas
left a comment
There was a problem hiding this comment.
mostly renaming + certs
lukaskratzel
requested a deployment
to
theia-prod
GitHub Actions
Waiting
This was referenced Mar 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by CodeRabbit
New Features
Chores
Documentation