WIP: Centralized TLS config by enarha · Pull Request #3225 · tektoncd/operator

enarha · 2026-02-17T14:44:12Z

Changes

Submitter Checklist

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

Run make test lint before submitting a PR
Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Release Notes

NONE

tekton-robot · 2026-02-17T14:44:22Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign piyush-garg after the PR has been reviewed.
You can assign the PR to them by writing /assign @piyush-garg 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:

OWNERS

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

enarha · 2026-02-17T14:46:06Z

This is still missing documentation, maybe add few more tests, but in general it's tested working and ready for review.

jkhelil · 2026-02-18T05:50:01Z

@enarha Please add PR deescrtipion with changes, architecture diagram,

theakshaypant · 2026-02-18T07:49:56Z

pkg/reconciler/openshift/common/tlsprofile.go

@@ -0,0 +1,229 @@
+/*
+Copyright 2024 The Tekton Authors


Question: Should it not be 2026?

theakshaypant · 2026-02-18T07:53:19Z

pkg/reconciler/openshift/common/tlsprofile_test.go

@@ -0,0 +1,167 @@
+/*
+Copyright 2024 The Tekton Authors


jkhelil · 2026-02-18T06:02:05Z

pkg/reconciler/openshift/common/tlsprofile.go

@@ -0,0 +1,229 @@
+/*
+Copyright 2024 The Tekton Authors


s/2024/2026

jkhelil · 2026-02-18T06:02:24Z

pkg/reconciler/openshift/common/tlsprofile_test.go

@@ -0,0 +1,167 @@
+/*
+Copyright 2024 The Tekton Authors


s/2024/2026

jkhelil · 2026-02-18T06:25:46Z

pkg/reconciler/openshift/common/tlsprofile.go

+// GetTLSEnvVarsFromAPIServer fetches the TLS security profile from the OpenShift APIServer
+// resource and converts it to environment variable values that can be used by Tekton components.
+// Returns nil if the APIServer has no TLS profile configured or if the shared lister is not initialized.
+func GetTLSEnvVarsFromAPIServer(ctx context.Context, _ *rest.Config) (*TLSEnvVars, error) {


Can we return tls.config here from crypto/tls
We Could need other configration in the future
We could use then func TLSEnvVarsFromConfig(cfg *tls.Config) *TLSEnvVars to turn it to envvars

We can. But we'll need to do the conversation twice APIServer string -> tls.Config -> env var strings instead of APIServer string -> env var strings. If you think we can immediately use the tls.Config with a specific consumer in mind (webhooks, nginx or whatever), then I'll follow your suggestion. If we do not have specific case immediately waiting for that, then I'll suggest we add this when we need it.
Please let me know what do you think.

jkhelil · 2026-02-18T06:29:18Z

pkg/reconciler/openshift/common/tlsprofile.go

+	recorder := events.NewLoggingEventRecorder("tekton-operator")
+	observedConfig, errs := apiserver.ObserveTLSSecurityProfile(listers, recorder, existingConfig)
+	if len(errs) > 0 {
+		logger.Warnf("Errors observing TLS security profile: %v", errs)


should we return error here

Good question. It is a matter of policy - what do we want to happen in the case of a failure. I encountered two types of errors. In the first case we are able to read the APIServer, but failed to populate the map existingConfig with values. It was a bug in the way library-go handles this case in the older version we use, which I fixed, thus we have that two level check. The second case is we fail to read the APIServer resource. If we return an error and bubble it up, we'll break the reconciliation of the operands (e.g. TektonResult). The current behavior is to log a warning, degrade the TLS funcitonality (centralized TLS configuration does not have an effect), but operands continue working. The question is if we prefer that harder or softer form of failure. I believe with the hard failure, the Tekton administrator can still disable the centralized TLS configuration by disabling it through the TektonConfig and if that's the case I'm inclined to agree with a hard failure. WDYT?

jkhelil · 2026-02-18T06:29:44Z

pkg/reconciler/openshift/common/tlsprofile.go

+
+	// If no TLS configuration is present, return nil
+	if minVersion == "" && len(cipherSuites) == 0 {
+		return nil, nil


should we log and return error ?

jkhelil · 2026-02-18T07:27:07Z

pkg/reconciler/openshift/common/tlsprofile.go

+	return &TLSEnvVars{
+		MinVersion:       convertTLSVersionToEnvFormat(minVersion),
+		CipherSuites:     strings.Join(cipherSuites, ","),
+		CurvePreferences: "", // Will be populated once openshift/api#2583 is merged


can we mention this in PR description openshift/api#2583
because empty curve preferences means go defaults

jkhelil · 2026-02-18T07:36:55Z

pkg/reconciler/openshift/common/tlsprofile.go

+
+// convertTLSVersionToEnvFormat converts library-go TLS version format (VersionTLSxx) to
+// the format expected by Go's crypto/tls (1.x)
+func convertTLSVersionToEnvFormat(version string) string {


We should consider parse TLSVversion to go format, webhook for example are expecting this in go native constant , I suppose the same thing for go apis using tls
https://github.com/knative/pkg/blob/main/webhook/webhook.go#L53

A conversion in this format feels more suitable to error prone constant
func parseTLSVersion(version string) (uint16, error) {
switch version {
case "VersionTLS10", "TLSv1.0":
return tls.VersionTLS10, nil
case "VersionTLS11", "TLSv1.1":
return tls.VersionTLS11, nil
case "VersionTLS12", "TLSv1.2":
return tls.VersionTLS12, nil
case "VersionTLS13", "TLSv1.3":
return tls.VersionTLS13, nil
default:
return 0, fmt.Errorf("unknown TLS version: %s", version)
}
}

Switched to function which returns an error if unknown tls version is set through the apiserver record. Also split the retrieval of the tls configuration from the transformation. If we need to add more transformations in the future (e.g. to tsl.Config) we can do that without affecting existing code.

jkhelil · 2026-02-18T07:54:58Z

pkg/reconciler/openshift/tektonresult/extension.go

+	if oe.resolvedTLSConfig == nil {
+		return ""
+	}
+	return fmt.Sprintf("%s:%s:%s", oe.resolvedTLSConfig.MinVersion, oe.resolvedTLSConfig.CipherSuites, oe.resolvedTLSConfig.CurvePreferences)


can we use a real hash function for the fingerprint like sha256, the ":" thingy cna be present in ciphers ro curves

Ok first look, thaught that this function is calculating a hash, but hte hash is calculted in isntallerset code
mm I think the name is misleading here, could we called GetPlatformData rather than GetHash Data

jkhelil · 2026-02-18T07:59:18Z

pkg/reconciler/openshift/tektonresult/extension.go

 	}
 }
+
+// injectTLSConfig injects the TLS configuration as environment variables into the Results API deployment


this should be common i think

jkhelil · 2026-02-18T08:02:51Z

pkg/reconciler/openshift/tektonconfig/controller.go

+	if err := setupAPIServerTLSWatch(ctx, ctrl); err != nil {
+		// On OpenShift clusters the APIServer resource should always exist.
+		// This env var is an escape hatch for edge cases and must be explicitly enabled.
+		if os.Getenv("SKIP_APISERVER_TLS_WATCH") == "true" {


may be move this to a constant

jkhelil · 2026-02-19T06:29:58Z

pkg/reconciler/kubernetes/tektonresult/installerset.go

+	// and check with annotation, if they are same then we skip updating the object
 	// otherwise we update the manifest
-	specHash, err := hash.Compute(tr.Spec)
+	hashInput := struct {


This seems redudant and duplciate for all component
Can we have a common function that computes Hash give spec and common extension

Would make thing clear when redaing

I kept it per component for now. Each component has specific deployments to update. I considered a separate function which takes a resource type (e.g. deployment/statefulset) and resource object (e.g. tekton-results-api), but it requires any type params and extra import of the common package, so I think for now the advantages are not much bigger than the disadvantages.

jkhelil · 2026-02-19T06:42:41Z

pkg/reconciler/openshift/tektonresult/extension.go

+	if oe.resolvedTLSConfig == nil {
+		return ""
+	}
+	return fmt.Sprintf("%s:%s:%s", oe.resolvedTLSConfig.MinVersion, oe.resolvedTLSConfig.CipherSuites, oe.resolvedTLSConfig.CurvePreferences)


Ok first look, thaught that this function is calculating a hash, but hte hash is calculted in isntallerset code
mm I think the name is misleading here, could we called GetPlatformData rather than GetHash Data

jkhelil · 2026-03-02T07:34:50Z

@enarha can we add in this PR transformation for other components
we will need to inject tls env vars to pipeline,, pac, triggers,

jkhelil · 2026-03-02T06:07:20Z

docs/OpenShiftCentralizedTLSManagement.md

+
+### 3. Extension Interface Enhancement
+
+- Added `GetHashData() string` method to the Extension interface


can you use the new function name GetPlatformData

jkhelil · 2026-03-02T06:14:07Z

pkg/reconciler/openshift/tektonconfig/controller.go

+
+	// Wait for caches to sync
+	if !cache.WaitForCacheSync(ctx.Done(), apiServerInformer.Informer().HasSynced) {
+		logger.Warn("Failed to sync APIServer informer cache")


What happens if the cache is not synced (because of network issue or ai server not reachable), is the code safe on this ? We should probably retrigger a reconcile if cache is not synced, but that will be more complex as for now, let just verify code is safe nothing will panic just after, may be we need to add unit test

From what I see, nothing will panic if the cluster apiserver resource is not found in the cache. The reconciliation of the operand (e.g. tektonresult) will fail repeatedly though. There will be some very rapid reconcile retries with an exponential backoff. The same goes for the cache sync. The admin can still disable the central TLS configuration. If they opt-in into that feature, then I think louder failure without a panic makes sense. If you prefer we do not affect the operand reconciliation, then we can ignore that error and threat it like "no TLS config available" in the APIServer resource and just log some warning.

jkhelil · 2026-03-02T06:22:22Z

pkg/reconciler/openshift/common/tlsprofile.go

+func ResolveCentralTLSToEnvVars(ctx context.Context, lister TektonConfigLister) (*TLSEnvVars, error) {
+	tc, err := lister.Get(v1alpha1.ConfigResourceName)
+	if err != nil {
+		return nil, nil


Here we should propagate the error: failed to get tektonconfig

jkhelil · 2026-03-02T08:06:51Z

pkg/reconciler/openshift/common/tlsprofile.go

+	}
+
+	// TLS 1.3 cipher suite names (IANA names)
+	tls13Ciphers := map[string]bool{


should we add this
As per this https://go.dev/blog/tls-cipher-suites#:~:text=Go%20does%20allow%20configuring%20cipher,order%20by%20default%2C%20unless%20Config.

ciphers are not configrable. with tls 1.3, because they are considered as safe
Now the issue if we add them here, that may procude issues, right ?

The reason this was included in the first place is that the newer version of library-go does the same. They are commented out in the 2023 version, but in the newer ones they are not.
They are not configurable indeed and not required for golang based operands. But we have some non golang based dependencies like PostgreSQL and Nginx maybe more. I'm not sure how will these handle the 1.3 ciphers if we do not explicitly specify them. So that's one reason to include them. For Tekton Results we already have the code which handles the ciphers list and there the 1.3 ciphers are simply ignored. To me it makes more sense to keep these for now. WDYT?

jkhelil · 2026-03-02T08:19:05Z

@pratap0007 @anithapriyanatarajan @mbpavan Can you review please ?

Add support for centralized TLS configuration derived from the OpenShift APIServer TLS security profile. This enables Tekton components to inherit cluster-wide TLS settings (minimum version, cipher suites) via the TektonConfig CR. Key changes: - Watch the APIServer resource for TLS profile changes and enqueue TektonConfig for reconciliation - Add GetTLSProfileFromAPIServer and TLSEnvVarsFromProfile to extract and convert TLS profiles to environment variables - Add ResolveCentralTLSToEnvVars and InjectTLSEnvVars as reusable helpers for component extensions - Add EnableCentralTLSConfig flag in TektonConfig OpenShift platform spec (opt-in, default false) - Add GetPlatformData to the Extension interface for platform-specific hash data (e.g., TLS config fingerprint) - Add RBAC permissions for reading the APIServer resource Note: curve preferences (CurvePreferences) are currently omitted and default to Go's standard library values until openshift/api#2583 is merged. Assisted-by: Cursor

Activate the centralized TLS configuration infrastructure for the TektonResult component: - Resolve TLS config from APIServer via ResolveCentralTLSToEnvVars in PreReconcile - Inject TLS env vars into the results-api deployment using the generic InjectTLSEnvVars transformer - Include TLS config fingerprint in GetPlatformData for installer set hash computation, triggering updates on TLS profile changes - Log injected TLS config at Info level for observability Assisted-by: Cursor

tekton-robot · 2026-03-04T10:24:43Z

@enarha: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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 Feb 17, 2026

tekton-robot requested review from PuneetPunamiya and jkhelil February 17, 2026 14:44

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 17, 2026

enarha mentioned this pull request Feb 17, 2026

WIP: Inject tls policy results #3151

Closed

4 tasks

enarha force-pushed the central-tls-config branch 2 times, most recently from c641544 to bded46d Compare February 17, 2026 16:45

theakshaypant reviewed Feb 18, 2026

View reviewed changes

jkhelil reviewed Feb 18, 2026

View reviewed changes

enarha force-pushed the central-tls-config branch from bded46d to 2de7f83 Compare February 18, 2026 12:07

jkhelil reviewed Feb 19, 2026

View reviewed changes

enarha force-pushed the central-tls-config branch 2 times, most recently from b744aba to 63a684f Compare February 26, 2026 15:59

jkhelil reviewed Mar 2, 2026

View reviewed changes

enarha added 2 commits March 2, 2026 20:12

enarha force-pushed the central-tls-config branch from 63a684f to 6137592 Compare March 2, 2026 18:58

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2026


		### 3. Extension Interface Enhancement

		- Added `GetHashData() string` method to the Extension interface

Conversation

enarha commented Feb 17, 2026

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Feb 17, 2026

Uh oh!

enarha commented Feb 17, 2026

Uh oh!

jkhelil commented Feb 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enarha Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkhelil commented Mar 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkhelil commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tekton-robot commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

enarha Feb 24, 2026 •

edited

Loading

jkhelil commented Mar 2, 2026 •

edited

Loading