Skip to content

WIP: Inject tls policy results #3151

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

Jump to bottom
Open
enarha wants to merge 2 commits into
base: main
Choose a base branch
from
Open

WIP: Inject tls policy results #3151

enarha wants to merge 2 commits into from
+48,799 −5

Conversation

@enarha
Copy link
Contributor

@enarha enarha commented Jan 12, 2026

Changes

The change include two commits. The first one adds generic TLS profiles centrally managed on the cluster level as OpenShift APIServer resources. It allows each Tekton component to use that functionality and set the TLS policy for their services.
The second change enables the centralized TLS profile for Tekton Results.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide for more details.

Release Notes

NONE

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesnt merit a release note. labels Jan 12, 2026
@tekton-robot
Copy link
Contributor

tekton-robot commented Jan 12, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign vdemeester after the PR has been reviewed.
You can assign the PR to them by writing /assign @vdemeester in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 12, 2026
enarha added 2 commits January 14, 2026 16:02
@enarha
Add generic TLS profile infrastructure for OpenShift APIServer
11fdcc4 
Add reusable infrastructure for reading and watching OpenShift's
APIServer TLS security profile configuration:

- tlsprofile.go: Functions to fetch TLS config from APIServer and convert
  OpenSSL cipher names to IANA format. Includes TLS 1.3 cipher support
  copied from library-go (see TODO comments for future cleanup).

- apiserver_watch.go: Generic watch mechanism that triggers component
  reconciliation when cluster TLS policy changes.

- lister_adapters.go: Generic adapter using Go generics to convert typed
  listers to ResourceLister interface for all Tekton component types.

- RBAC: Added permissions to read apiservers.config.openshift.io

Note: TLS cipher conversion functions are copied from library-go to include
TLS 1.3 support without requiring dependency upgrades that conflict with
current k8s versions. These should be replaced with library-go imports once
Tekton components upgrade to k8s 0.34+.
@enarha
Enable TLS profile configuration for TektonResult
a2b6ec3 
Activate the TLS profile infrastructure for the Results component:

- extension.go: Add injectTLSConfig transformer that injects
  TLS_MIN_VERSION and TLS_CIPHER_SUITES env vars into the Results API
  deployment based on cluster APIServer configuration. TLS config is
  fetched once during Transformers() call, not per-resource.

- controller.go: Set up APIServer watch to trigger reconciliation when
  the cluster TLS security profile changes.

The Results API deployment will automatically pick up TLS configuration
from the OpenShift APIServer resource and update when it changes.
@enarha enarha force-pushed the inject-tls-policy-results branch from 81c9fa4 to a2b6ec3 Compare January 14, 2026 14:06
@jkhelil
Copy link
Member

jkhelil commented Jan 15, 2026

@enarha I've reviewed your implementation and wanted to share some feedback and an alternative approach I've been working on.
we will have to use GetTLSEnvVarsFromAPIServer() calls the APIServer directly on every reconcile loop for every component. This creates unnecessary load on the API server and can impact performance at scale.
Why not use the observer pattern as suggested in techincal documentation : This is the standard pattern for OpenShift operators that need to observe cluster configuration:

All use library-go/pkg/operator/configobserver for exactly this purpose.

However i think it is smart to use the env var approch rather than configmaps
I have a proposal here with

  • Use Observer Pattern to watch APIServer TLS profile changes via informer (library-go recommended pattern)
  • Fetch TLS Profile from APIServer at operator startup using library-go/pkg/operator/configobserver/apiserver.ObserveTLSSecurityProfile()
  • Transform Ciphers from OpenSSL names to IANA names, then to Go-compatible tls.Config with proper cipher suite IDs
  • Store in Context as *tls.Config and hash for efficient caching and change detection across all reconcilers
  • Inject via Transformer into all component containers (Pipeline, Triggers, Results, Chains, PAC) as environment variables

#3166

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

Reviewers

@anithapriyanatarajan anithapriyanatarajan Awaiting requested review from anithapriyanatarajan

@pratap0007 pratap0007 Awaiting requested review from pratap0007

Assignees

No one assigned

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesnt merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@enarha @tekton-robot @jkhelil