Skip to content

fix(chart): make the openshift flavor functional and align with kustomize #3945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aqeelat wants to merge 10 commits into
base: main
Choose a base branch
from
+372 −140
Open

fix(chart): make the openshift flavor functional and align with kustomize #3945

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e2adcc0
fix(chart): template openshift operator SA name
aqeelat Aug 17, 2026
6d84cb3
fix(chart): set PLATFORM=openshift in openshift flavor
aqeelat Aug 18, 2026
d1feafa
fix(chart): add missing RBAC roles and align rules
aqeelat Aug 19, 2026
59d667f
fix(chart): use platform images and controllers per flavor
aqeelat Aug 19, 2026
ee0375e
fix(chart): wire leader election and add missing configmaps
aqeelat Aug 19, 2026
c3d009f
fix(chart): wire webhook logLevel to the webhook container
aqeelat Aug 19, 2026
afde9f7
fix(chart): reject custom target namespace on openshift flavor
aqeelat Aug 19, 2026
b7d7c43
chore(chart): add trailing newline to openshift-rbac template
aqeelat Aug 19, 2026
2f345ab
refactor(chart): dedupe shared RBAC into common-rbac
aqeelat Aug 19, 2026
42b8578
fix(chart): address review on image helpers and shared RBAC naming
aqeelat Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions charts/tekton-operator/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,31 @@ tekton-operator
{{- end -}}
{{- end -}}

{{- define "tekton-operator.controllers" -}}
{{- if .Values.openshift.enabled -}}
tektonconfig,tektonpipeline,tektontrigger,tektonchain,tektonaddon,tektonresult,openshiftpipelinesascode,manualapprovalgate,tektonpruner,tektonscheduler,tektonmulticlusterproxyaae,syncerservice
{{- else -}}
tektonconfig,tektonpipeline,tektontrigger,tektonchain,tektonresult,tektondashboard,manualapprovalgate,tektonpruner,tektonscheduler,tektonmulticlusterproxyaae,openshiftpipelinesascode
{{- end -}}
{{- end -}}

{{- define "tekton-operator.validateTargetNamespace" -}}
{{- if and .Values.openshift.enabled .Values.operator.defaultTargetNamespace (ne .Values.operator.defaultTargetNamespace "openshift-pipelines") -}}
{{- fail (printf "operator.defaultTargetNamespace must be \"openshift-pipelines\" when openshift.enabled=true (got %q). The openshift addon sample pipelines hardcode that namespace, so a custom value breaks them; this is also first-install-only - once the TektonConfig CR exists the operator never re-reads it. To change the target namespace, set spec.targetNamespace on the TektonConfig CR (requires deleting and recreating the CR)." .Values.operator.defaultTargetNamespace) -}}
{{- end -}}
{{- end -}}

{{- define "tekton-operator.operator-image" -}}
{{- $tag := default .Chart.AppVersion .Values.operator.image.tag -}}
{{- $image := "" -}}
{{- if .Values.operator.image.repository -}}
{{- $image = .Values.operator.image.repository }}
{{- else -}}
{{- if .Values.openshift.enabled -}}
{{- $image = "ghcr.io/tektoncd/operator/operator-1d69a75f22dd094880847eac907fb2c1" -}}

@jkhelil jkhelil Aug 24, 2026

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.

Add comment for digest pinning
{{/* OpenShift image repositories are digest-pinned to the v0.81.0 .
Update all three slugs (operator/webhook/proxy-webhook) when bumping AppVersion. */}}

@aqeelat aqeelat Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in 42b8578, with one correction to the wording: the repositories are the ko-published binary slugs from the v0.81.0 release assets and the tag follows Chart.AppVersion (no @sha256 pinning), so the comment now says where the slugs come from and that all three must be updated together if a release renames them.

{{- else -}}

@jkhelil jkhelil Aug 24, 2026

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.

ident, remove space for readbility

@aqeelat aqeelat Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Flattened to match the chart style — fixed in 42b8578.

{{- $image = "ghcr.io/tektoncd/operator/operator-303303c315a48490ba6517859ef65b77" -}}
{{- else -}}
{{- $image = "ghcr.io/tektoncd/operator/operator-303303c315a48490ba6517859ef65b77" -}}
{{- end -}}
{{- end -}}

@jkhelil jkhelil Aug 24, 2026

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.

same identation for readbility

@aqeelat aqeelat Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same — flattened in 42b8578.

{{- end -}}
{{- printf "%s:%s" $image $tag -}}
{{- end -}}
Expand All @@ -126,10 +140,10 @@ tekton-operator
{{- $image = .Values.webhook.image.repository }}
{{- else -}}
{{- if .Values.openshift.enabled -}}
{{- $image = "ghcr.io/tektoncd/operator/webhook-340ad78e88ca5477447aa144fedfe1a1" -}}
{{- else -}}
{{- $image = "ghcr.io/tektoncd/operator/webhook-f2bb711aa8f0c0892856a4cbf6d9ddd8" -}}
{{- else -}}
{{- $image = "ghcr.io/tektoncd/operator/webhook-f2bb711aa8f0c0892856a4cbf6d9ddd8" -}}
{{- end -}}
{{- end -}}
{{- end -}}
{{- printf "%s:%s" $image $tag -}}
{{- end -}}
Expand All @@ -149,10 +163,10 @@ tekton-operator
{{- $image = .Values.webhookProxy.image.repository }}
{{- else -}}
{{- if .Values.openshift.enabled -}}
{{- $image = "ghcr.io/tektoncd/operator/proxy-webhook-f8f95c9cea9508fe8915ae3d012d15fb" -}}
{{- else -}}
{{- $image = "ghcr.io/tektoncd/operator/proxy-webhook-f6167da7bc41b96a27c5529f850e63d1" -}}
{{- else -}}
{{- $image = "ghcr.io/tektoncd/operator/proxy-webhook-f6167da7bc41b96a27c5529f850e63d1" -}}
{{- end -}}
{{- end -}}
{{- end -}}
{{- printf "%s:%s" $image $tag -}}
{{- end -}}
156 changes: 156 additions & 0 deletions charts/tekton-operator/templates/common-rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
{{- if .Values.rbac.create -}}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: tekton-operator-info

@jkhelil jkhelil Aug 24, 2026

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.

Orphaned resource risk: kubernetes-rbac.yaml to common-rbac.yaml rename

⚠️ Issue: Before this PR, kubernetes-rbac.yaml shipped:

RoleBinding named {{ include "tekton-operator.fullname" . }}-info
ClusterRole named tekton-config-read-role
ClusterRoleBinding named tekton-config-read-rolebinding
After this PR, the equivalent resources move to common-rbac.yaml with different names (tekton-operator-info for the RoleBinding). Helm upgrade will create the new resources but will not delete the old -info RoleBinding because it no longer appears in any template. Users upgrading will accumulate stale RBAC objects.

@aqeelat aqeelat Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid concern — addressed in two parts: the kubernetes flavor's -info Role/RoleBinding went back to their pre-PR names (so that particular orphan no longer occurs for kubernetes upgrades from released charts), and the PR description now carries an upgrade note with the exact kubectl cleanup commands for the objects that do get orphaned (the literal config-read pair on both flavors, and the literal info pair on openshift). A pre-upgrade hook felt like the wrong trade: it would need its own RBAC to delete RBAC.

@aqeelat aqeelat Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correction to my earlier reply — and to the orphan concern itself: helm upgrade does delete renamed objects. Helm diffs the stored release manifest against the newly rendered one and deletes anything removed (originals.Difference(targets) → delete in helm/pkg/kube/client.go; the helm.sh/resource-policy: keep annotation exists precisely to opt out of this, and same behavior in Helm 4 where server-side apply is now the default). Orphans only occur with helm template | kubectl apply (no prune) or adopted non-helm installs. I've updated the PR description accordingly — the multi-install collision fix from this review stands and was the right call.

labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
rules:
- apiGroups:
- ""
resourceNames:
- tekton-operator-info
resources:
- configmaps
verbs:
- get
- describe
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
labels:
app.kubernetes.io/instance: default
name: tekton-operator-info
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: tekton-operator-info
subjects:
- apiGroup: rbac.authorization.k8s.io
kind: Group
name: system:authenticated
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: tekton-config-read-role
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
rules:
- apiGroups:
- operator.tekton.dev
resources:
- tektonconfigs
verbs:
- get
- watch
- list
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: tekton-config-read-rolebinding

@jkhelil jkhelil Aug 24, 2026

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.

may be not hardcode the name , two simultaneous chart installs in the same cluster will conflict on this ClusterRole and ClusterRoleBinding, use
{{ include "tekton-operator.fullname" . }}

@aqeelat aqeelat Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in 42b8578: the info Role/RoleBinding and config-read ClusterRole/ClusterRoleBinding are now fullname-prefixed, so two installs no longer collide on cluster-scoped names. One deliberate exception: the Role's resourceNames entry stays the literal tekton-operator-info, because it refers to the ConfigMap that tkn version reads — that object keeps a fixed name.

roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: tekton-config-read-role
subjects:
- apiGroup: rbac.authorization.k8s.io
kind: Group
name: system:authenticated
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "tekton-operator.fullname" . }}-scheduler-role
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
rules:
- apiGroups:
- kueue.x-k8s.io
resources:
- resourceflavors
- workloads
- workloads/finalizers
- workloads/status
- workloadpriorityclasses
- multikueueclusters
verbs:
- get
- list
- create
- update
- delete
- patch
- watch
- apiGroups:
- scheduling.k8s.io
resources:
- priorityclasses
verbs:
- get
- list
- watch
- apiGroups:
- cert-manager.io
resources:
- certificates
- issuers
verbs:
- get
- create
- update
- list
- nonResourceURLs:
- /metrics
verbs:
- get
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ include "tekton-operator.fullname" . }}-scheduler-rolebinding
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: {{ include "tekton-operator.fullname" . }}-scheduler-role
subjects:
- kind: ServiceAccount
name: {{ include "tekton-operator.serviceAccountName" . }}
namespace: {{ .Release.Namespace }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "tekton-operator.fullname" . }}-result-read-role
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
rules:
- apiGroups:
- operator.tekton.dev
resources:
- tektonresults
verbs:
- get
- watch
- list
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ include "tekton-operator.fullname" . }}-result-read-rolebinding
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: {{ include "tekton-operator.fullname" . }}-result-read-role
subjects:
- apiGroup: rbac.authorization.k8s.io
kind: Group
name: system:authenticated
{{- end -}}
79 changes: 79 additions & 0 deletions charts/tekton-operator/templates/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,85 @@ data:
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "tekton-operator.fullname" . }}-controller-config-leader-election
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
data:
_example: |
################################
# #
# EXAMPLE CONFIGURATION #
# #
################################
# This block is not actually functional configuration,
# but serves to illustrate the available configuration
# options and document them in a way that is accessible
# to users that `kubectl edit` this config map.
#
# lease-duration is how long non-leaders will wait to try to acquire the
# lock; 15 seconds is the value used by core kubernetes controllers.
lease-duration: "60s"
# renew-deadline is how long a leader will try to renew the lease before
# giving up; 10 seconds is the value used by core kubernetes controllers.
renew-deadline: "40s"
# retry-period is how long the leader election client waits between tries of
# actions; 2 seconds is the value used by core kubernetes controllers.
retry-period: "10s"
# buckets is the number of buckets used to partition key space of each
# Reconciler. If this number is M and the replica number of the controller
# is N, the N replicas will compete for the M buckets. The owner of a
# bucket will take care of the reconciling for the keys partitioned into
# that bucket.
buckets: "1"
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "tekton-operator.fullname" . }}-webhook-config-leader-election
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
data:
_example: |
################################
# #
# EXAMPLE CONFIGURATION #
# #
################################
# This block is not actually functional configuration,
# but serves to illustrate the available configuration
# options and document them in a way that is accessible
# to users that `kubectl edit` this config map.
#
# lease-duration is how long non-leaders will wait to try to acquire the
# lock; 15 seconds is the value used by core kubernetes controllers.
lease-duration: "60s"
# renew-deadline is how long a leader will try to renew the lease before
# giving up; 10 seconds is the value used by core kubernetes controllers.
renew-deadline: "40s"
# retry-period is how long the leader election client waits between tries of
# actions; 2 seconds is the value used by core kubernetes controllers.
retry-period: "10s"
# buckets is the number of buckets used to partition key space of each
# Reconciler. If this number is M and the replica number of the controller
# is N, the N replicas will compete for the M buckets. The owner of a
# bucket will take care of the reconciling for the keys partitioned into
# that bucket.
buckets: "1"
---
apiVersion: v1
kind: ConfigMap
metadata:
name: tekton-operator-info
labels:
{{- include "tekton-operator.labels" . | nindent 4 }}
data:
# Contains operator version which can be queried by external
# tools such as CLI.
version: {{ .Chart.AppVersion | quote }}
---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "tekton-operator.fullname" . }}-observability
labels:
Expand Down
Loading
Loading