fix(tektonresult): default route TLS termination to reencrypt

[divyansh42]

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

Upgrade migration

Changing the default only affects new installs — existing installs persist
the previous edge value in their TektonConfig/TektonResult CR, so they
would stay broken after upgrading. A version-gated pre-upgrade migration
(migrateResultsRouteTLSToReencrypt) updates the Results route TLS termination
from edge → reencrypt on TektonConfig (the source of truth), which then
syncs to the TektonResult CR. It runs on OpenShift only and migrates only
edge; values set manually to reencrypt or passthrough are left untouched.

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	Extracted validateRouteTLSTermination; only edge, reencrypt, passthrough accepted
tektonconfig_validation.go	Reject invalid spec.result.route_tls_termination at the parent CR level too
upgrade.go / pre_upgrade.go	Added version-gated pre-upgrade migration edge → reencrypt
docs/TektonResult.md, docs/TektonConfig.md	Document that only reencrypt is supported on OpenShift; note the upgrade migration
*_test.go	Tests for defaults, TektonResult/TektonConfig validation, route injection, and the migration

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`, and add a pre-upgrade
migration that flips existing installs 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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.71%. Comparing base (7dfc9db) to head (6b2e6f0).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4029      +/-   ##
==========================================
+ Coverage   27.64%   27.71%   +0.06%     
==========================================
  Files         477      477              
  Lines       25458    25485      +27     
==========================================
+ Hits         7039     7062      +23     
- Misses      17696    17699       +3     
- Partials      723      724       +1     

Flag	Coverage Δ
unit-tests	27.71% <ø> (+0.06%)	⬆️

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.

[divyansh42]

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

1. Stale test expectations — Fixed all 6 occurrences of RouteTLSTermination: "edge" in TestResult_SetDefaultsBucketsAutoDefaulting to expect "reencrypt".

2. 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.

3. 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]

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

[jkhelil]

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

[enarha]

/approve

[tekton-robot]

[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]

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

[divyansh42]

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

I have handled the upgrade but still need to test end to end. Will update once done.

[divyansh42]

/hold

[Copilot]

🟡 Changes recommended

The non-OpenShift test is environment-dependent, and the TLS documentation incorrectly associates --insecure with the edge failure.

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

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The documentation inaccurately characterizes external certificate trust and recommends --insecure as mandatory for passthrough.

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

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 2
• Review effort level: Balanced