fix(tektonresult): default route TLS termination to reencrypt by divyansh42 · Pull Request #4029 · tektoncd/operator

divyansh42 · 2026-09-03T13:50:07Z

Changes

The Results API route on OpenShift was defaulting to edge TLS termination,
which causes TLS handshake failures when clients try to connect. The router
terminates TLS at the edge and forwards unencrypted traffic to the gRPC
backend — but the backend only speaks TLS.

Additionally, when passthrough was used as a workaround, clients had to
pass --insecure-skip-tls-verify because the OpenShift service-serving
certificate was not trusted by external clients.

reencrypt is the correct termination type:

The router presents a trusted certificate to external clients (no --insecure needed)
The router re-encrypts to the backend using the OpenShift service-serving cert
End-to-end encryption is maintained

Changes made

File	Change
`tektonresult_defaults.go`	Default `route_tls_termination` changed from `edge` → `reencrypt`
`route-rbac/rbac.yaml` (kodata)	Route TLS termination updated from `passthrough` → `reencrypt`
`route-rbac/rbac.yaml` (testdata)	RBAC `resourceNames` fixed: `tekton-results-api-service` → `tekton-results-api` (matches actual Route name)
`tektonresult_validation.go`	Added validation: only `edge`, `reencrypt`, `passthrough` accepted
`tektonresult_validation_test.go`	Added tests for valid and invalid TLS termination values
`tektonresult_defaults_test.go`	Updated expected default to `reencrypt`
`extension_test.go`	Added `reencrypt` test case for route injection

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

fix(tektonresult): change default OpenShift Results API route TLS
termination from `edge` to `reencrypt`. This fixes TLS handshake
failures and eliminates the need for `--insecure-skip-tls-verify`
when accessing the Results API via the OpenShift route. Also adds
validation to reject unsupported `route_tls_termination` values.

Made with Cursor

The Results API route was defaulting to `edge` TLS termination which causes TLS handshake failures: the router terminates TLS and forwards unencrypted traffic to the gRPC backend, which only speaks TLS. `reencrypt` is the correct mode: the router presents a trusted cert to external clients while re-encrypting to the backend (which has an OpenShift service-serving cert). This eliminates the need for `--insecure-skip-tls-verify` when using `opc results`. Changes: - Default `route_tls_termination` changed from `edge` to `reencrypt` - Route YAML updated from `passthrough` to `reencrypt` - Fix RBAC `resourceNames` from `tekton-results-api-service` to `tekton-results-api` to match the actual Route resource name - Add validation: only `edge`, `reencrypt`, `passthrough` are accepted Signed-off-by: divyansh42 <diagrawa@redhat.com> Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

codecov · 2026-09-03T14:01:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 26.47%. Comparing base (d1d928c) to head (06d301f).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4029      +/-   ##
==========================================
+ Coverage   26.44%   26.47%   +0.03%     
==========================================
  Files         465      465              
  Lines       24991    25002      +11     
==========================================
+ Hits         6609     6620      +11     
  Misses      17661    17661              
  Partials      721      721

Flag	Coverage Δ
unit-tests	`26.47% <ø> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

🟡 Changes recommended

Existing installations are not migrated, TektonConfig bypasses the new validation, and stale test expectations will fail.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates OpenShift Results routes to use re-encrypt TLS and validates supported termination modes.

Changes:

Defaults route TLS termination to reencrypt.
Updates route manifests and RBAC resource names.
Adds validation and tests for TLS termination values.

File summaries

File	Description
`pkg/reconciler/openshift/tektonresult/testdata/static/tekton-results/route-rbac/rbac.yaml`	Updates test route and RBAC.
`pkg/reconciler/openshift/tektonresult/extension_test.go`	Tests re-encrypt route injection.
`pkg/apis/operator/v1alpha1/tektonresult_validation.go`	Validates termination modes.
`pkg/apis/operator/v1alpha1/tektonresult_validation_test.go`	Tests termination validation.
`pkg/apis/operator/v1alpha1/tektonresult_defaults.go`	Changes the default to re-encrypt.
`pkg/apis/operator/v1alpha1/tektonresult_defaults_test.go`	Updates the primary default assertion.
`cmd/openshift/operator/kodata/static/tekton-results/route-rbac/rbac.yaml`	Updates deployed route and RBAC.

Review details

Files reviewed: 7/7 changed files
Comments generated: 3
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

 		ResultsAPIProperties: ResultsAPIProperties{
 			RouteEnabled:        ptr.Bool(true),
-			RouteTLSTermination: "edge",
+			RouteTLSTermination: "reencrypt",


 	if c.RouteTLSTermination == "" {
-		c.RouteTLSTermination = "edge"
+		c.RouteTLSTermination = "reencrypt"


+	if trs.RouteTLSTermination != "" {
+		switch trs.RouteTLSTermination {
+		case "edge", "reencrypt", "passthrough":


- Fix 6 stale RouteTLSTermination: "edge" assertions in TestResult_SetDefaultsBucketsAutoDefaulting; all now expect "reencrypt" to match the updated SetDefaults output. - Extract route TLS termination validation into a dedicated method ResultsAPIProperties.validateRouteTLSTermination so it can be called from both TektonResult.Validate and TektonConfig.Validate. - Add the validation call to TektonConfig.Validate so an invalid spec.result.route_tls_termination is rejected at the parent CR level before the child TektonResult reconcile ever fires. - Add Test_ValidateTektonConfig_InvalidResultRouteTLSTermination to cover the new TektonConfig validation path. Signed-off-by: divyansh42 <diagrawa@redhat.com> Assisted-by: Claude Sonnet 4.6 (via Cursor) Co-authored-by: Cursor <cursoragent@cursor.com>

divyansh42 · 2026-09-03T14:49:56Z

Addressed the Copilot review comments in the follow-up commit (06d301f):

Stale test expectations — Fixed all 6 occurrences of RouteTLSTermination: "edge" in TestResult_SetDefaultsBucketsAutoDefaulting to expect "reencrypt".
TektonConfig bypasses validation — Extracted the TLS termination check into ResultsAPIProperties.validateRouteTLSTermination() and wired it into TektonConfig.Validate (at spec.result.route_tls_termination), with a corresponding test Test_ValidateTektonConfig_InvalidResultRouteTLSTermination.
Existing installations not migrated — The edge termination never worked correctly (it causes TLS handshake failures), so any existing cluster with edge is already broken. Rather than adding a forced migration (which risks overwriting deliberate user choices for users who set edge knowingly), we are treating this as a known upgrade action: users on existing installations who have edge persisted should manually patch their TektonResult or TektonConfig CR to reencrypt.

Copilot

🔵 Needs a closer look

The release note overstates external certificate trust, which still depends on the cluster ingress certificate configuration.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

cmd/openshift/operator/kodata/static/tekton-results/route-rbac/rbac.yaml:61

The PR states that this eliminates --insecure-skip-tls-verify, but reencrypt only changes router-to-backend encryption; this Route still presents the cluster ingress certificate. A default OpenShift ingress certificate is internally signed and may remain untrusted by external clients. Please qualify the description/release note so this benefit is conditional on clients trusting the ingress CA (or on a trusted custom ingress certificate being configured).

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Balanced

pratap0007 · 2026-09-04T03:42:05Z

Don't we also need to handle the upgrade scenario (updating TLS termination edge to reencrypt)?

jkhelil · 2026-09-04T09:08:10Z

@divyansh42 can you adress @pratap0007 concerns? what happens on upgrade, please post a proof of work

enarha

/approve

tekton-robot · 2026-09-06T13:02:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enarha

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [enarha]

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

jkhelil · 2026-09-08T05:58:12Z

@divyansh42 can we deprecate passthroug ?
and eventually remove edge from valide values ?

tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 3, 2026

tekton-robot requested review from pramodbindal and pratap0007 September 3, 2026 13:50

tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 3, 2026

divyansh42 force-pushed the fix/results-route-reencrypt-tls branch from a1fc393 to 9146282 Compare September 3, 2026 13:55

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 3, 2026

divyansh42 requested a balanced review from Copilot September 3, 2026 13:57

Copilot started reviewing on behalf of divyansh42 September 3, 2026 13:58 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 3, 2026

divyansh42 requested a balanced review from Copilot September 3, 2026 14:53

Copilot started reviewing on behalf of divyansh42 September 3, 2026 14:56 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

enarha approved these changes Sep 6, 2026

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 6, 2026

Navigation Menu

Uh oh!

fix(tektonresult): default route TLS termination to reencrypt - #4029

fix(tektonresult): default route TLS termination to reencrypt#4029
divyansh42 wants to merge 2 commits into
tektoncd:mainfrom
divyansh42:fix/results-route-reencrypt-tls

divyansh42 commented Sep 3, 2026

Uh oh!

codecov Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

divyansh42 commented Sep 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

pratap0007 commented Sep 4, 2026

Uh oh!

jkhelil commented Sep 4, 2026

Uh oh!

enarha left a comment

Uh oh!

tekton-robot commented Sep 6, 2026

Uh oh!

jkhelil commented Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

divyansh42 commented Sep 3, 2026

Changes

Changes made

Submitter Checklist

Release Notes

Uh oh!

codecov Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

divyansh42 commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (1)

Uh oh!

pratap0007 commented Sep 4, 2026

Uh oh!

jkhelil commented Sep 4, 2026

Uh oh!

enarha left a comment

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Sep 6, 2026

Uh oh!

jkhelil commented Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov Bot commented Sep 3, 2026 •

edited

Loading

divyansh42 commented Sep 3, 2026 •

edited

Loading