Skip to content

add c templates appdefinition - #107

Merged
KevinGruber2001 merged 7 commits into
mainfrom
feat/add-c-templates
Jun 5, 2026
Merged

KevinGruber2001 merged 7 commits into
mainfrom
feat/add-c-templates

Conversation

@KevinGruber2001

@KevinGruber2001 KevinGruber2001 commented May 22, 2026 •

Copy link
Copy Markdown
Contributor
  • adds app definition for c templates
  • does not use prewarming (otherwise the templates dont work)

Summary by CodeRabbit

  • New Features

    • Added "C Templates" app to IDE deployments and landing page; supports Bazel and Make and is included in preloaded images.
  • Chores

    • Adjusted deployment and CI workflow handling to account for the new preloaded image and updated image ordering.
    • Rolled several service/operator image tags to latest and updated TLS/gateway configuration.

Copilot AI review requested due to automatic review settings May 22, 2026 19:57
@coderabbitai

coderabbitai Bot commented May 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f8826c8-0269-4ce3-b2dc-f0d44f164330

📥 Commits

Reviewing files that changed from the base of the PR and between caa635d and 2d0b0fc.

📒 Files selected for processing (1)
  • deployments/theia-staging.artemis.cit.tum.de/values.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • deployments/theia-staging.artemis.cit.tum.de/values.yaml

📝 Walkthrough

Walkthrough

Adds the c-templates container to Theia Cloud preloading and landing-page configs, defines a c-templates-latest app, updates staging image tags to latest where applicable, adjusts a shared-gateway TLS line, and updates the deploy workflow's preloading image index overrides.

Changes

C-Templates Image Integration

Layer / File(s) Summary
c-templates AppDefinition
charts/theia-appdefinitions/values.yaml
New c-templates-latest app entry with image ghcr.io/eduide/eduide/c-templates, resource requests/limits, scaling (minInstances 0, maxInstances 1000), and dataBridge options (enabled, port 16281).
Combined chart preloading & landing page
charts/theia-cloud-combined/values.yaml
Adds ghcr.io/eduide/eduide/c-templates:latest to theia-cloud.preloading.images and introduces c-templates-latest in landingPage.additionalApps, setting c-latest.visible: false.
Test environments preload and landing apps
deployments/test1.theia-test.artemis.cit.tum.de/values.yaml, deployments/test2.theia-test.artemis.cit.tum.de/values.yaml, deployments/test3.theia-test.artemis.cit.tum.de/values.yaml
Inserts c-templates:latest into each environment's theia-cloud.app.preloading.images list and adds c-templates-latest landing app entries with bazel and make build systems; updates preload index comments.
Staging values and image-tag updates
deployments/theia-staging.artemis.cit.tum.de/values.yaml
Switches several operator/service/landing preloading image tags from 2026-05-05 to latest, updates preloading lists to include c-templates:latest, sets theia-appdefinitions.defaultImageTag to latest, and adds c-templates-latest to landingPage.additionalApps.
Deployment workflow image index updates
.github/workflows/deploy-theia.yml
When IDE_IMAGES_TAG is set, Helm --set overrides now include c-templates and shift subsequent theia-cloud.preloading.images[] index assignments; inline comment updated to reserve oauth2-proxy at the new index.
Shared gateway TLS configuration
deployments/shared-gateway/values.yaml
Adjusts tlsSecretName placement so it is correctly associated with the staging-webview listener.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

ready to merge

Suggested reviewers

  • lukaskratzel
  • CodeByNikolas

Poem

🐰 A tiny template hops into sight,
Bazel and Make ready to compile right,
Indices shift as lists gently grow,
Workflows and charts now line up in a row,
Hooray — new images land by moonlight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'add c templates appdefinition' directly describes the main change in the PR—adding a C templates application definition across multiple deployment configurations and Helm charts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/add-c-templates

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new C templates app definition and exposes it as a selectable landing-page app, while also adjusting image preloading and shared-gateway deployment assets.

Changes:

  • Add a new c-templates-latest AppDefinition (image ghcr.io/eduide/eduide/c-templates).
  • Expose c-templates-latest as an additional app (with Bazel/Make build systems) in the combined chart and the test2 environment.
  • Remove deployments/shared-gateway/values.yaml.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
deployments/test2.theia-test.artemis.cit.tum.de/values.yaml Switch the test2 landing page to offer c-templates-latest (Bazel/Make) instead of the previous C entry.
deployments/shared-gateway/values.yaml Removed the shared-gateway values file for non-prod clusters.
charts/theia-cloud-combined/values.yaml Add c-templates preload image and add/hide C apps in landing page configuration (c-latest hidden, c-templates-latest added).
charts/theia-appdefinitions/values.yaml Add the c-templates-latest AppDefinition pointing at the new C templates image.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread charts/theia-cloud-combined/values.yaml
Comment thread charts/theia-cloud-combined/values.yaml
Comment thread charts/theia-cloud-combined/values.yaml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comment thread deployments/test2.theia-test.artemis.cit.tum.de/values.yaml
Comment thread charts/theia-cloud-combined/values.yaml
Comment thread .github/workflows/deploy-theia.yml
Comment thread deployments/shared-gateway/values.yaml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@charts/theia-cloud-combined/values.yaml`:
- Line 86: The comment documenting the preloading index contract is now
incorrect after inserting "c-templates"
(ghcr.io/eduide/eduide/c-templates:latest); oauth2-proxy has moved from index 12
to index 13, so update the nearby comment that explains
theia-cloud.preloading.images[...] indexing to reflect the new indexes
(including that oauth2-proxy is index 13) and ensure any example or enumeration
in that comment is renumbered consistently after the insertion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0c842659-9fd4-44f1-aaaa-18233a33dba1

📥 Commits

Reviewing files that changed from the base of the PR and between 8f34014 and 4396513.

📒 Files selected for processing (7)
  • .github/workflows/deploy-theia.yml
  • charts/theia-appdefinitions/values.yaml
  • charts/theia-cloud-combined/values.yaml
  • deployments/shared-gateway/values.yaml
  • deployments/test1.theia-test.artemis.cit.tum.de/values.yaml
  • deployments/test2.theia-test.artemis.cit.tum.de/values.yaml
  • deployments/test3.theia-test.artemis.cit.tum.de/values.yaml

Comment thread charts/theia-cloud-combined/values.yaml

@Fangoling Fangoling left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works on the test server. I have some minor comments regarding the 'documentation' comments.

Do we need to add the new template images to the staging and prod envs as well?

Comment thread deployments/test1.theia-test.artemis.cit.tum.de/values.yaml Outdated
Comment thread deployments/test2.theia-test.artemis.cit.tum.de/values.yaml Outdated
Comment thread deployments/test3.theia-test.artemis.cit.tum.de/values.yaml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Comment thread deployments/theia-staging.artemis.cit.tum.de/values.yaml
Comment thread charts/theia-cloud-combined/values.yaml
Comment thread deployments/theia-staging.artemis.cit.tum.de/values.yaml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
deployments/theia-staging.artemis.cit.tum.de/values.yaml (1)

76-77: ⚠️ Potential issue | 🔴 Critical

Preload index comment is incorrect for staging.

The comment states "Index 13 is oauth2-proxy", but oauth2-proxy is currently at index 12 in staging because c-templates is missing from the preloading list. Once c-templates:latest is added (as it is in test1), oauth2-proxy will shift to index 13 and the comment will become correct.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deployments/theia-staging.artemis.cit.tum.de/values.yaml` around lines 76 -
77, The staging values.yaml comment about preload indices is wrong because
oauth2-proxy is currently at index 12 (not 13) due to the missing c-templates
image; either add "c-templates:latest" to the preloading list so oauth2-proxy
moves to index 13 (matching the comment) or update the comment to state that
oauth2-proxy is at index 12; locate the preload list and the entries for
"c-templates" and "oauth2-proxy" and make the change so indices and comment
remain consistent.
🧹 Nitpick comments (1)
deployments/theia-staging.artemis.cit.tum.de/values.yaml (1)

60-60: Consider the implications of using :latest tags in staging.

Switching from pinned tags (2026-05-05) to :latest means staging will automatically pull the most recent images. While this enables continuous testing of new builds, it can introduce:

  • Non-reproducible deployments if images change between rollouts
  • Unexpected breaking changes from upstream image updates
  • Difficulty troubleshooting issues tied to specific image versions

For a staging environment, this trade-off may be acceptable if the goal is to detect integration issues early. If stability is needed for regression testing or demos, consider using short-lived pinned tags (e.g., weekly releases) instead.

Also applies to: 69-69, 80-91, 103-103, 171-171

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deployments/theia-staging.artemis.cit.tum.de/values.yaml` at line 60, The
deployment is using an unpinned image tag ("image:
ghcr.io/eduide/eduide-cloud/operator:latest") which causes non-reproducible and
potentially unstable staging rollouts; change the operator image references to a
pinned tag or configurable value (e.g., set a specific release tag like
2026-05-05 or wire the tag to a chart value such as image.tag) for all
occurrences (the "image: ghcr.io/eduide/eduide-cloud/operator:latest" lines and
the other referenced image entries) so staging pulls a deterministic image, or
implement a short-lived automated tag strategy (weekly release tag) if you want
frequent updates while retaining reproducibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deployments/theia-staging.artemis.cit.tum.de/values.yaml`:
- Around line 78-93: The staging values.yaml preloading list is missing the
c-templates image causing app launch failures and index mismatches; add the
ghcr.io/eduide/eduide/c-templates:latest entry into the preloading.images array
(near the other eduide language/template images in the preloading block) so the
c-templates image is pre-pulled and the image index ordering used by the landing
page apps and workflow --set overrides remains correct.

---

Duplicate comments:
In `@deployments/theia-staging.artemis.cit.tum.de/values.yaml`:
- Around line 76-77: The staging values.yaml comment about preload indices is
wrong because oauth2-proxy is currently at index 12 (not 13) due to the missing
c-templates image; either add "c-templates:latest" to the preloading list so
oauth2-proxy moves to index 13 (matching the comment) or update the comment to
state that oauth2-proxy is at index 12; locate the preload list and the entries
for "c-templates" and "oauth2-proxy" and make the change so indices and comment
remain consistent.

---

Nitpick comments:
In `@deployments/theia-staging.artemis.cit.tum.de/values.yaml`:
- Line 60: The deployment is using an unpinned image tag ("image:
ghcr.io/eduide/eduide-cloud/operator:latest") which causes non-reproducible and
potentially unstable staging rollouts; change the operator image references to a
pinned tag or configurable value (e.g., set a specific release tag like
2026-05-05 or wire the tag to a chart value such as image.tag) for all
occurrences (the "image: ghcr.io/eduide/eduide-cloud/operator:latest" lines and
the other referenced image entries) so staging pulls a deterministic image, or
implement a short-lived automated tag strategy (weekly release tag) if you want
frequent updates while retaining reproducibility.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3ecf51a7-a5ce-4765-b9ca-c3360650301e

📥 Commits

Reviewing files that changed from the base of the PR and between 4396513 and caa635d.

📒 Files selected for processing (4)
  • deployments/test1.theia-test.artemis.cit.tum.de/values.yaml
  • deployments/test2.theia-test.artemis.cit.tum.de/values.yaml
  • deployments/test3.theia-test.artemis.cit.tum.de/values.yaml
  • deployments/theia-staging.artemis.cit.tum.de/values.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • deployments/test3.theia-test.artemis.cit.tum.de/values.yaml
  • deployments/test2.theia-test.artemis.cit.tum.de/values.yaml

Comment thread deployments/theia-staging.artemis.cit.tum.de/values.yaml

@Fangoling Fangoling left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the new change to staging to use latest tags make sense.

@KevinGruber2001
KevinGruber2001 merged commit a4a005a into main Jun 5, 2026
3 of 6 checks passed

This branch was previously deployed

3 inactive deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants