Feature/lokal openshift - #559
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are verified functional issues in init-cluster.sh (default bind host mismatch and non-portable loopback alias setup) plus CI inefficiency (Sonar stage re-runs tests) and high-risk RBAC defaults that need clearer safeguards.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the GitOps Playground to better support local OpenShift (CRC) usage, modernizes several dependencies/tooling versions, and improves Kubernetes custom-resource handling by resolving API resources via Kubernetes discovery instead of requiring CRD list permissions.
Changes:
- Bump Vault Helm chart version (and related test/config defaults) and update Vault image mirroring version.
- Update
K8sClientto resolve custom resources through Kubernetes discovery APIs and add tests for the new behavior. - Add local OpenShift deployment assets/docs and extend CI with Sonar scanning.
File summaries
| File | Description |
|---|---|
| src/test/resources/testMainConfig.yaml | Updates test config to use Vault Helm chart version 0.34.1. |
| src/test/groovy/com/cloudogu/gitops/infrastructure/kubernetes/api/K8sClientTest.groovy | Adjusts ConfigMap namespace expectations and adds tests for discovery-based CR resolution. |
| src/test/groovy/com/cloudogu/gitops/cli/GitopsPlaygroundCliTest.groovy | Aligns CLI schema test expectations with new Vault Helm chart version. |
| src/main/groovy/com/cloudogu/gitops/infrastructure/kubernetes/api/K8sClient.groovy | Uses current namespace for ConfigMaps; switches custom resource resolution to discovery; adds dedicated exception type. |
| src/main/groovy/com/cloudogu/gitops/config/Config.groovy | Updates default Vault Helm chart version. |
| scripts/local-openshift/manifest/gop-rbac.yaml | Adds ServiceAccount + cluster-admin ClusterRoleBinding manifest for CRC/local OpenShift runs. |
| scripts/local-openshift/manifest/gop-job.yaml | Adds an OpenShift Job manifest to run the installer from the internal image registry. |
| scripts/local-openshift/helm/gop-values.yaml | Adds Helm values for running GOP on local OpenShift/CRC (image, SA, args, minimal config). |
| scripts/local-openshift/helm/gop-rbac.yaml | Adds RBAC manifest for Helm-based OpenShift install path. |
| scripts/jenkins/plugins/plugins.txt | Updates Jenkins plugin versions list. |
| scripts/init-cluster.sh | Updates k3d/k8s versions, adds loopback binding logic, and improves Docker socket path handling. |
| scripts/dev/mirror_images_to_registry.sh | Updates mirrored Vault image tag. |
| pom.xml | Updates Micronaut parent, adds Sonar Maven plugin, and manages Jackson dependencies for schema generation/tooling. |
| Jenkinsfile | Renames unit-test stage and adds a SonarScanner stage. |
| docs/deploy-local-openshift.md | Adds end-to-end CRC/local OpenShift deployment guide (registry, RBAC/SCC, Helm/Job run paths). |
| docs/Configuration.md | Updates documented default Vault Helm chart version. |
| Dockerfile | Switches runtime base to Alpine and downloads/verifies a pinned Temurin JRE tarball. |
Review details
Suppressed comments (2)
src/main/groovy/com/cloudogu/gitops/infrastructure/kubernetes/api/K8sClient.groovy:1151
- The
log.debug(...)line under thedefault:case is not indented like the surrounding switch branches, which looks unintentional and hurts readability.
default:
log.debug("Searching API resource via discovery for resourceType=${resourceType}, name=${name}, ns=${ns}")
return getCustomResourceClient(resourceType, name, ns)
scripts/init-cluster.sh:269
- The help text and inline comment say the default
--bind-ingress-hostis127.0.0.2, but the actual default is still set to127.0.0.1, so users will get unexpected behavior.
BIND_LOCALHOST=false
# Use 127.0.0.2 to avoid port conflict with CRC/OpenShift which binds *:80/*:443
BIND_INGRESS_HOST="127.0.0.1"
BIND_INGRESS_PORT="80"
- Files reviewed: 17/17 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+81
to
+83
| withSonarQubeEnv('ces-sonar') { | ||
| sh "mvn -B clean verify sonar:sonar -Dsonar.projectKey=gitops-playground -Dsonar.branch.name=${BRANCH_NAME}" | ||
| } |
Comment on lines
+5
to
8
| K3D_VERSION=5.9.0 | ||
| # When updating please also adapt in Dockerfile, vars.tf and Config.groovy | ||
| K8S_VERSION=1.35.3 | ||
| K8S_VERSION=1.36.4 | ||
| K3S_VERSION="rancher/k3s:v${K8S_VERSION}-k3s1" |
Comment on lines
+65
to
+74
| # Ensure loopback alias exists when using a non-default loopback IP (e.g. 127.0.0.2) | ||
| # This avoids port conflicts with CRC/OpenShift which binds *:80/*:443 on the default 127.0.0.1 | ||
| if [[ "${BIND_INGRESS_HOST}" != "127.0.0.1" && "${BIND_INGRESS_HOST}" =~ ^127\. ]]; then | ||
| if ! ifconfig lo0 | grep -q "${BIND_INGRESS_HOST}"; then | ||
| echo "Adding loopback alias ${BIND_INGRESS_HOST} to lo0 (requires sudo)..." | ||
| sudo ifconfig lo0 alias "${BIND_INGRESS_HOST}" | ||
| else | ||
| echo "Loopback alias ${BIND_INGRESS_HOST} already configured." | ||
| fi | ||
| fi |
| name: gop-sa | ||
| namespace: gop | ||
| --- | ||
| apiVersion: rbac.authorization.k8s.io/v1 |
| name: gop-sa | ||
| namespace: gop | ||
| --- | ||
| apiVersion: rbac.authorization.k8s.io/v1 |
Comment on lines
648
to
650
| if (!existingResource) { | ||
| throw new RuntimeException("Resource $resource/$name not found") | ||
| throw new RuntimeException("Resource $resource/$name not found") | ||
| } |
ThomasMichael1811
force-pushed
the
feature/lokal-openshift
branch
from
September 7, 2026 13:55
877effa to
7606a1e
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed functional/script portability issues in init-cluster.sh and several high-risk RBAC/doc clarity gaps (cluster-admin binding) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
scripts/init-cluster.sh:69
- The loopback-alias setup is macOS-specific (lo0/ifconfig) and will fail on Linux when a non-127.0.0.1 loopback address is used (e.g. if the default is changed to 127.0.0.2). This can break cluster creation with
set -e.
# Ensure loopback alias exists when using a non-default loopback IP (e.g. 127.0.0.2)
# This avoids port conflicts with CRC/OpenShift which binds *:80/*:443 on the default 127.0.0.1
if [[ "${BIND_INGRESS_HOST}" != "127.0.0.1" && "${BIND_INGRESS_HOST}" =~ ^127\. ]]; then
if ! ifconfig lo0 | grep -q "${BIND_INGRESS_HOST}"; then
echo "Adding loopback alias ${BIND_INGRESS_HOST} to lo0 (requires sudo)..."
scripts/local-openshift/manifest/gop-rbac.yaml:9
- This manifest grants
cluster-adminto the GOP ServiceAccount via a ClusterRoleBinding, which is highly privileged for a checked-in example. Please add an explicit warning and cleanup guidance (and ideally move toward least-privilege RBAC) so users don’t apply this to non-disposable clusters by accident.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
scripts/local-openshift/helm/gop-rbac.yaml:9
- This Helm-side RBAC file also binds the ServiceAccount to
cluster-admin. To reduce accidental misuse, add the same explicit warning/cleanup guidance here (and consider consolidating RBAC definitions to avoid drift).
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
- Files reviewed: 6/6 changed files
- Comments generated: 5
- Review effort level: Lite
Comment on lines
+267
to
268
| # Use 127.0.0.2 to avoid port conflict with CRC/OpenShift which binds *:80/*:443 | ||
| BIND_INGRESS_HOST="127.0.0.1" |
Comment on lines
+14
to
+16
| ### Recommend | ||
|
|
||
| Start openshift with more cpu and memory! |
Comment on lines
+18
to
+23
| ### CRC should use different default ports, because K3d has to use 80/443. | ||
|
|
||
| ```bash | ||
| crc config set ingress-http-port 8880 | ||
| crc config set ingress-https-port 8843 | ||
| ``` |
Comment on lines
+151
to
+154
| ```bash | ||
| # Apply manifest (as kubeadmin) | ||
| oc apply -f scripts/local-openshift/manifest/gop-rbac.yaml | ||
| ``` |
Comment on lines
+7
to
+8
| # Retain job after completion to preserve logs | ||
| backoffLimit: 0 |
mdroll
self-requested a review
September 7, 2026 14:14
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
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.
There are verified functional issues in
init-cluster.sh(default bind host mismatch and non-portable loopback alias setup) plus CI inefficiency (Sonar stage re-runs tests) and high-risk RBAC defaults that need clearer safeguards.Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the GitOps Playground to better support local OpenShift (CRC) usage, modernizes several dependencies/tooling versions, and improves Kubernetes custom-resource handling by resolving API resources via Kubernetes discovery instead of requiring CRD list permissions.