fix(chart): make the openshift flavor functional - #3945
Conversation
The openshift RBAC template hardcoded the ServiceAccount name (openshift-pipelines-operator) while the operator Deployment resolves the name via the tekton-operator.serviceAccountName helper. With rbac.create enabled the pod referenced a ServiceAccount the chart never created, so OpenShift users had to set rbac.serviceAccount.name manually. Use the same helper for the ServiceAccount and its ClusterRoleBinding subject so both flavors follow the chart fullname default and the rbac.serviceAccount.name override. Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com> Assisted-by: GLM (via opencode)
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3945 +/- ##
=======================================
Coverage 26.33% 26.33%
=======================================
Files 465 465
Lines 24951 24951
=======================================
Hits 6572 6572
Misses 17661 17661
Partials 718 718
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The operator and webhook Deployments never set the PLATFORM env, so the openshift chart flavor ran the operator in kubernetes mode: IsOpenShiftPlatform() returned false and the openshift-specific reconciler branches, defaults, and TektonConfig platform validation never activated. Set PLATFORM=openshift on the lifecycle and webhook containers when openshift.enabled is true, mirroring the upstream openshift kustomize overlay (the tektoninstallerset cluster-operations container does not use it). Signed-off-by: Abdullah Alaqeel <abdullah.t.aqeel@gmail.com> Assisted-by: GLM (via opencode)
| {{- end }} | ||
| {{- end }} | ||
| {{- if .Values.openshift.enabled }} | ||
| - name: PLATFORM |
There was a problem hiding this comment.
PLATFORM does not switch the compiled operator binary. The OpenShift branch in _helpers.tpl still selects the Kubernetes operator image. The correct OpenShift binary also rejects the default tektondashboard controller. Please make the image and controller defaults flavor-aware, or narrow the PR title, description, and release note.
| fieldRef: | ||
| fieldPath: metadata.namespace | ||
| {{- if .Values.openshift.enabled }} | ||
| - name: PLATFORM |
There was a problem hiding this comment.
The OpenShift branches in _helpers.tpl also select the Kubernetes webhook and proxy images. Please use the OpenShift image repositories, or narrow the PR scope and release note.
Changes
Fixes two bugs in the OpenShift flavor (
openshift.enabled=true) of the Helm chart:ServiceAccount name mismatch.
openshift-rbac.yamlhardcoded the ServiceAccount asopenshift-pipelines-operator, while the Deployments resolve the name via thetekton-operator.serviceAccountNamehelper (chart fullname by default). The operator pods referenced a ServiceAccount the chart never created, so OpenShift users had to manually setrbac.serviceAccount.name: openshift-pipelines-operator. The ServiceAccount and its ClusterRoleBinding subject now use the same helper, so both flavors follow the chart fullname default and therbac.serviceAccount.nameoverride.Missing
PLATFORM=openshiftenv. The Deployments never setPLATFORM, so the operator ran in kubernetes mode even in the OpenShift flavor:IsOpenShiftPlatform()returned false, disabling the openshift-specific reconciler branches, defaults, and TektonConfig platform validation.PLATFORM=openshiftis now set on the lifecycle and webhook containers whenopenshift.enabled=true, mirroring the upstream openshift kustomize overlay (the tektoninstallerset cluster-operations container does not use it upstream).Verified with
helm templatefor both flavors, with and withoutrbac.serviceAccount.nameoverride: SA, binding subject, and DeploymentserviceAccountNameall match, andPLATFORMappears only in the OpenShift flavor. Chart-only change; no Go code touched.Submitter Checklist
make test lintbefore submitting a PRRelease Notes