Add shared cache node helm chart#16
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:
📝 WalkthroughWalkthroughAdds a new cluster-scoped internal TLS Helm chart and workflow step, integrates a shared cache into the main Theia Cloud helm values and deployments, and introduces monitoring dashboards and ServiceMonitors for the shared cache and Reposilite; documentation and value references updated accordingly. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
f3474a5 to
d459ac9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| landing: theia | ||
| instance: instance.theia | ||
|
|
||
| theia-shared-cache: |
There was a problem hiding this comment.
theia-shared-cache values block won’t be passed to the shared cache subchart anymore, because the dependency name was changed to eduide-shared-cache in Chart.yaml. This means the enabled flag here is ignored. Rename this top-level values key to eduide-shared-cache (or set an explicit alias in Chart.yaml and use that key consistently).
| theia-shared-cache: | |
| eduide-shared-cache: |
| enabled: true | ||
| reader: | ||
| username: "reader" | ||
| password: "changeme-reader" | ||
| writer: | ||
| username: "writer" | ||
| password: "changeme-writer" |
There was a problem hiding this comment.
These are hard-coded placeholder passwords committed into an environment values file. If this file is used for real deployments, it creates an easy-to-guess credential risk. Prefer referencing an existing Kubernetes Secret (or disable auth by default in committed env values), and keep credentials out of git.
| enabled: true | |
| reader: | |
| username: "reader" | |
| password: "changeme-reader" | |
| writer: | |
| username: "writer" | |
| password: "changeme-writer" | |
| enabled: false | |
| reader: | |
| username: "" | |
| password: "" | |
| writer: | |
| username: "" | |
| password: "" |
| buildCache: | ||
| enabled: true | ||
| push: true | ||
| url: "https://theia-cloud-combined-cache:8080/cache/" | ||
| secretName: "" | ||
| dependencyCache: | ||
| enabled: false | ||
| url: "http://theia-cloud-combined-reposilite:8080/releases/" |
There was a problem hiding this comment.
The operator caching configuration here (operator.buildCache / operator.dependencyCache) doesn’t match the schema introduced in this PR’s reference/default values (enableBuildCaching, buildCacheUrl, enableBuildCachePush, etc.). As written, these settings are likely ignored by the operator chart. Please align this environment values file with the actual operator values schema used by theia-cloud.
| buildCache: | |
| enabled: true | |
| push: true | |
| url: "https://theia-cloud-combined-cache:8080/cache/" | |
| secretName: "" | |
| dependencyCache: | |
| enabled: false | |
| url: "http://theia-cloud-combined-reposilite:8080/releases/" | |
| enableBuildCaching: true | |
| buildCacheUrl: "https://theia-cloud-combined-cache:8080/cache/" | |
| enableBuildCachePush: true | |
| buildCacheSecretName: "" | |
| enableDependencyCaching: false | |
| dependencyCacheUrl: "http://theia-cloud-combined-reposilite:8080/releases/" |
| apiVersion: trust.cert-manager.io/v1alpha1 | ||
| kind: Bundle | ||
| metadata: | ||
| name: theia-internal-trust |
There was a problem hiding this comment.
This chart introduces a trust.cert-manager.io/v1alpha1 Bundle resource, which requires cert-manager trust-manager (CRDs + controller) to be installed on the cluster. The repository docs/workflow changes here don’t mention installing trust-manager, so helm upgrade --install theia-internal-tls ... will fail on clusters that only have cert-manager.
| target: | ||
| # trust-manager creates a ConfigMap with this name in target namespaces | ||
| configMap: | ||
| key: "trust-bundle.pem" | ||
|
|
||
| # Also generate a JKS truststore (Java KeyStore) | ||
| # This is what Java/Gradle will use directly | ||
| additionalFormats: | ||
| jks: |
There was a problem hiding this comment.
The Bundle spec doesn’t define which namespaces should receive the generated ConfigMap/format outputs (no target.namespaces / target.namespaceSelector), yet README/docs claim the trust bundle is distributed to all namespaces. Please make the target namespace selection explicit (or update the docs if the intent is same-namespace only).
|
|
||
| ### 1.4 Internal TLS | ||
|
|
||
| The internal TLS infrastructure (internal CA + trust bundle) is deployed once per cluster via `charts/theia-internal-tls`. The trust bundle ConfigMap (`theia-internal-trust`) is automatically distributed to all namespaces, so no additional configuration is needed when adding a new environment. |
There was a problem hiding this comment.
This section states the trust bundle ConfigMap (theia-internal-trust) is automatically distributed to all namespaces. The Bundle manifest added in this PR does not currently specify any namespace targeting, so this statement is likely inaccurate unless additional configuration/components exist. Please align the documentation with the actual Bundle behavior (and mention trust-manager if required).
| The internal TLS infrastructure (internal CA + trust bundle) is deployed once per cluster via `charts/theia-internal-tls`. The trust bundle ConfigMap (`theia-internal-trust`) is automatically distributed to all namespaces, so no additional configuration is needed when adding a new environment. | |
| The internal TLS infrastructure (internal CA + trust bundle) is deployed once per cluster via `charts/theia-internal-tls`. Distribution of the trust bundle ConfigMap (`theia-internal-trust`) to application namespaces depends on the `Bundle` target configuration managed by trust-manager; it is not implied automatically for every namespace by this chart alone. When adding a new environment, verify that your namespace is targeted and receives the ConfigMap, or update the trust-manager `Bundle` configuration if required. |
| 4. **Install the internal TLS infrastructure (once per cluster)**: | ||
| ```bash | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls \ | ||
| --namespace cert-manager | ||
| ``` | ||
| This deploys the cluster-scoped internal CA and trust bundle used for TLS | ||
| between internal services (e.g., shared cache and workspaces). The trust | ||
| bundle ConfigMap is automatically distributed to all namespaces. |
There was a problem hiding this comment.
The new install step doesn’t mention prerequisites for the theia-internal-tls chart (at minimum: cert-manager, and trust-manager because the chart creates a trust.cert-manager.io/v1alpha1 Bundle). Without that, readers may follow these steps and hit a hard Helm failure due to missing CRDs/controllers. Please document the required components/versions here.
| issuerRef: | ||
| name: theia-internal-ca-issuer | ||
| kind: ClusterIssuer | ||
| commonName: "theia-shared-cache" |
There was a problem hiding this comment.
commonName is set to theia-shared-cache, but the certificate SANs are for theia-cloud-combined-cache[...]. Since clients validate against SANs, having a different CN is confusing and can cause issues with older tooling. Consider setting commonName to the primary DNS name used in dnsNames (or omit CN entirely and rely on SANs).
| commonName: "theia-shared-cache" | |
| commonName: "theia-cloud-combined-cache" |
|
|
||
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | ||
| # Distributes the trust bundle ConfigMap to all namespaces | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager |
There was a problem hiding this comment.
This workflow step lacks set -euo pipefail (used in other steps) and starts with a blank line, which makes failures easier to miss and inconsistent with the rest of the workflow. Consider adding set -euo pipefail and, if needed, --create-namespace for cert-manager to make the install more robust.
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | |
| # Distributes the trust bundle ConfigMap to all namespaces | |
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager | |
| set -euo pipefail | |
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | |
| # Distributes the trust bundle ConfigMap to all namespaces | |
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager --create-namespace |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
deployments/test2.theia-test.artemis.cit.tum.de/values.yaml:146
- This values file still has unresolved merge markers, which makes the file invalid YAML and will cause the Test2 deployment to fail as soon as Helm tries to read it.
image: ghcr.io/eduide/eduidec-landing-page
sentry:
enable: true
# We can define a default blueprint
appDefinition: "java-17-templates-latest"
deployments/test2.theia-test.artemis.cit.tum.de/values.yaml:160
- There is a second unresolved merge-conflict block here as well, so even if the earlier conflict is fixed this values file still will not parse as valid YAML.
c-latest:
label: C
javascript-latest:
label: Javascript
ocaml-latest:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| image: theiacloud/theia-cloud-operator:1.1.0-next | ||
| sentry: | ||
| enable: true | ||
| eagerStart: false | ||
| replicas: 1 | ||
| sessionsPerUser: 10 | ||
| storageClassName: csi-rbd-sc |
| username: "reader" | ||
| password: "changeme-reader" | ||
| writer: | ||
| username: "writer" | ||
| password: "changeme-writer" |
| run: | | ||
|
|
|
|
||
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | ||
| # Distributes the trust bundle ConfigMap to all namespaces | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager | ||
|
|
| 4. **Install the internal TLS infrastructure (once per cluster)**: | ||
| ```bash | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls \ | ||
| --namespace cert-manager | ||
| ``` | ||
| This deploys the cluster-scoped internal CA and trust bundle used for TLS | ||
| between internal services (e.g., shared cache and workspaces). The trust | ||
| bundle ConfigMap is automatically distributed to all namespaces. |
| "definition": "label_values(gradle_cache_cache_hits_total, namespace)", | ||
| "hide": 0, | ||
| "includeAll": false, | ||
| "label": "Namespace", | ||
| "multi": false, | ||
| "name": "namespace", | ||
| "options": [], | ||
| "query": { | ||
| "qryType": 1, | ||
| "query": "label_values(gradle_cache_cache_hits_total, namespace)", |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
README.md:167
- This local-chart install path still skips
helm dependency update ./charts/theia-cloud-combined, even though this repository already requires that step before installing the combined chart. With the new shared-cache dependency added in this PR, a fresh checkout will fail here because the subchart is not present yet, and a stale lock/build can resolve the old dependency instead.
5. **Install the combined Theia Cloud chart**:
```bash
helm registry login ghcr.io
helm upgrade --install theia-cloud-combined ./charts/theia-cloud-combined \
--namespace your-namespace --create-namespace \
-f deployments/your-environment/values.yaml
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| storageClassName: csi-rbd-sc | ||
| enableBuildCaching: true | ||
| buildCacheUrl: "https://eduide-shared-cache:8080/cache/" | ||
| enableBuildCachePush: true |
| buildCacheUrl: "https://eduide-shared-cache:8080/cache/" | ||
| enableBuildCachePush: true | ||
| enableDependencyCaching: true | ||
| dependencyCacheUrl: "http://eduide-shared-cache-reposilite:8080/releases/" |
| run: | | ||
|
|
|
|
||
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | ||
| # Distributes the trust bundle ConfigMap to all namespaces | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager | ||
|
|
| ```bash | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls \ | ||
| --namespace cert-manager | ||
| ``` | ||
| This deploys the cluster-scoped internal CA and trust bundle used for TLS | ||
| between internal services (e.g., shared cache and workspaces). The trust | ||
| bundle ConfigMap is automatically distributed to all namespaces. |
| - name: theia-shared-cache | ||
| version: "0.3.1" | ||
| - name: eduide-shared-cache | ||
| version: 0.5.0 |
|
|
||
| ### 1.4 Internal TLS | ||
|
|
||
| The internal TLS infrastructure (internal CA + trust bundle) is deployed once per cluster via `charts/theia-internal-tls`. The trust bundle ConfigMap (`theia-internal-trust`) is automatically distributed to all namespaces, so no additional configuration is needed when adding a new environment. |
| dependencies: | ||
| - name: theia-cloud | ||
| version: 1.4.0-next.5 | ||
| version: 1.4.0-next.7.pr-21 | ||
| repository: "oci://ghcr.io/eduide/charts" | ||
|
|
| The internal TLS infrastructure (internal CA + trust bundle) is deployed once per cluster via `charts/theia-internal-tls`. The trust bundle ConfigMap (`theia-internal-trust`) is automatically distributed to all namespaces, so no additional configuration is needed when adding a new environment. | ||
|
|
| gradleUrl: "https://eduide-shared-cache:8080/cache/" | ||
| bazelUrl: "https://eduide-shared-cache:8080/cache/" |
| run: | | ||
|
|
||
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | ||
| # Distributes the trust bundle ConfigMap to all namespaces | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager |
| enableBuildCaching: false | ||
| buildCacheUrl: "" | ||
| enableBuildCachePush: false | ||
| enableDependencyCaching: false | ||
| dependencyCacheUrl: "" |
| # apiVersion: cert-manager.io/v1 | ||
| # kind: Certificate | ||
| # metadata: | ||
| # name: shared-theia-cert | ||
| # namespace: gateway-system | ||
| # spec: | ||
| # secretName: shared-theia-cert | ||
| # issuerRef: | ||
| # kind: ClusterIssuer | ||
| # name: letsencrypt-prod | ||
| # dnsNames: | ||
| # - test1.theia-test.artemis.cit.tum.de | ||
| # - service.test1.theia-test.artemis.cit.tum.de | ||
| # - instance.test1.theia-test.artemis.cit.tum.de | ||
| # - test2.theia-test.artemis.cit.tum.de | ||
| # - service.test2.theia-test.artemis.cit.tum.de | ||
| # - instance.test2.theia-test.artemis.cit.tum.de | ||
| # - cache.test2.theia-test.artemis.cit.tum.de # NEW | ||
| # - repo.test2.theia-test.artemis.cit.tum.de # NEW | ||
| # - test3.theia-test.artemis.cit.tum.de | ||
| # - service.test3.theia-test.artemis.cit.tum.de | ||
| # - instance.test3.theia-test.artemis.cit.tum.de | ||
| # - theia-staging.artemis.cit.tum.de | ||
| # - service.theia-staging.artemis.cit.tum.de | ||
| # - instance.theia-staging.artemis.cit.tum.de No newline at end of file |
| apiVersion: cert-manager.io/v1 | ||
| kind: Certificate | ||
| metadata: | ||
| name: cache-internal-cert | ||
| spec: |
| # - test1.theia-test.artemis.cit.tum.de | ||
| # - service.test1.theia-test.artemis.cit.tum.de | ||
| # - instance.test1.theia-test.artemis.cit.tum.de | ||
| # - test2.theia-test.artemis.cit.tum.de | ||
| # - service.test2.theia-test.artemis.cit.tum.de | ||
| # - instance.test2.theia-test.artemis.cit.tum.de | ||
| # - cache.test2.theia-test.artemis.cit.tum.de # NEW | ||
| # - repo.test2.theia-test.artemis.cit.tum.de # NEW | ||
| # - test3.theia-test.artemis.cit.tum.de | ||
| # - service.test3.theia-test.artemis.cit.tum.de | ||
| # - instance.test3.theia-test.artemis.cit.tum.de | ||
| # - theia-staging.artemis.cit.tum.de | ||
| # - service.theia-staging.artemis.cit.tum.de | ||
| # - instance.theia-staging.artemis.cit.tum.de No newline at end of file |
| enableBuildCaching: false | ||
| buildCacheUrl: "" | ||
| enableBuildCachePush: false | ||
| enableDependencyCaching: false | ||
| dependencyCacheUrl: "" |
| enablePush: true | ||
| dependencyCache: | ||
| enabled: true | ||
| url: "http://repo.test2.theia-test.artemis.cit.tum.de/releases/" |
| key: "ca.crt" | ||
|
|
||
| target: | ||
| # trust-manager creates a ConfigMap with this name in target namespaces |
| 4. **Install the internal TLS infrastructure (once per cluster)**: | ||
| ```bash | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls \ | ||
| --namespace cert-manager | ||
| ``` | ||
| This deploys the cluster-scoped internal CA and trust bundle used for TLS | ||
| between internal services (e.g., shared cache and workspaces). The trust | ||
| bundle ConfigMap is automatically distributed to all namespaces. |
|
|
||
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | ||
| # Distributes the trust bundle ConfigMap to all namespaces | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager |
| - ghcr.io/eduide/eduide/java-17:latest | ||
| - ghcr.io/eduide/eduide/java-17-templates:latest | ||
| - ghcr.io/eduide/eduide/c:latest | ||
| - ghcr.io/eduide/eduide/c-templates:latest | ||
| - ghcr.io/eduide/eduide/javascript:latest | ||
| - ghcr.io/eduide/eduide/ocaml:latest | ||
| - ghcr.io/eduide/eduide/rust:latest |
| - ghcr.io/eduide/eduide/java-17:latest | ||
| - ghcr.io/eduide/eduide/java-17-templates:latest | ||
| - ghcr.io/eduide/eduide/c:latest | ||
| - ghcr.io/eduide/eduide/c-templates:latest |
| - ghcr.io/eduide/eduide/java-17:latest | ||
| - ghcr.io/eduide/eduide/java-17-templates:latest | ||
| - ghcr.io/eduide/eduide/c:latest | ||
| - ghcr.io/eduide/eduide/c-templates:latest |
| - ghcr.io/eduide/eduide/java-17:latest | ||
| - ghcr.io/eduide/eduide/java-17-templates:latest | ||
| - ghcr.io/eduide/eduide/c:latest | ||
| - ghcr.io/eduide/eduide/c-templates:latest |
| dependencies: | ||
| - name: theia-cloud | ||
| version: 1.4.0-next.6 | ||
| version: 1.4.0-next.7.pr-21 | ||
| repository: "oci://ghcr.io/eduide/charts" |
| - name: Install internal TLS infrastructure | ||
| env: | ||
| KUBECONFIG: ${{ github.workspace }}/kubeconfig | ||
| run: | | ||
|
|
||
| # Install cluster-scoped internal CA and trust bundle (once per cluster) | ||
| # Distributes the trust bundle ConfigMap to all namespaces | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls -n cert-manager |
| 4. **Install the internal TLS infrastructure (once per cluster)**: | ||
| ```bash | ||
| helm upgrade --install theia-internal-tls ./charts/theia-internal-tls \ | ||
| --namespace cert-manager | ||
| ``` |
| ### 1.4 Internal TLS | ||
|
|
||
| The internal TLS infrastructure (internal CA + trust bundle) is deployed once per cluster via `charts/theia-internal-tls`. The trust bundle ConfigMap (`theia-internal-trust`) is automatically distributed to all namespaces, so no additional configuration is needed when adding a new environment. |
| enableBuildCaching: false | ||
| buildCacheUrl: "" | ||
| enableBuildCachePush: false | ||
| enableDependencyCaching: false | ||
| dependencyCacheUrl: "" |
| enableBuildCaching: false | ||
| buildCacheUrl: "" | ||
| enableBuildCachePush: false | ||
| enableDependencyCaching: false | ||
| dependencyCacheUrl: "" |
| dependencyCache: | ||
| enabled: true | ||
| url: "http://repo.test2.theia-test.artemis.cit.tum.de/releases/" |
Summary by CodeRabbit
New Features
Documentation