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)
}
}

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

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

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 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 from 2de7f83 to b744aba Compare February 25, 2026 15:20

enarha added 2 commits February 26, 2026 17:25

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

enarha Feb 24, 2026 •

edited

Loading