Add denylist for system namespaces in targetNamespace validation by mvanhorn · Pull Request #3507 · tektoncd/operator

mvanhorn · 2026-06-17T11:40:58Z

Changes

CommonSpec.validate() previously accepted any non-empty targetNamespace on Kubernetes, so a component could be told to install into a well-known system namespace such as kube-system, kube-public, kube-node-lease, or default. On OpenShift only openshift-operators was blocked.

This adds a defense-in-depth denylist that rejects those four reserved system namespaces on both Kubernetes and OpenShift, while preserving the existing OpenShift-only openshift-operators rejection. Because validate() is the shared method called by every component validator, the denylist applies across components with no caller changes.

While wiring this up I found that TektonResult bypassed the shared validation entirely: TektonResult.Validate calls TektonResultSpec.validate, which never invoked the embedded CommonSpec.validate. This change also calls the common validation from TektonResultSpec.validate so the denylist (and the pre-existing empty-field check) is enforced consistently for TektonResult too.

Tests cover the happy path, each denied namespace on both platforms, the preserved openshift-operators rejection, the empty-namespace case, and the TektonResult path.

Fixes #3248

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

Reject well-known system namespaces (kube-system, kube-public, kube-node-lease, default) in `targetNamespace` validation across Tekton components.

tekton-robot · 2026-06-17T11:41:03Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

jkhelil · 2026-06-18T07:40:05Z

+
 func (ta *CommonSpec) validate(path string) *apis.FieldError {
 	var errs *apis.FieldError
+	targetNamespace := ta.GetTargetNamespace()


this is cleaner than calling GetTargetNamespace() multile time

jkhelil · 2026-06-18T07:43:53Z

+		},
+		Spec: TektonResultSpec{
+			CommonSpec: CommonSpec{
+				TargetNamespace: "kube-system",


this skips validation kube-public, kube-node-lease, default, and the OpenShift openshift-operators case, without any failing test at the TektonResult level.

jkhelil · 2026-06-18T07:45:28Z

@mvanhorn thank you for you contribution, would you add the validation to pkg/apis/operator/v1alpha1/tektonscheduler_validation.go:27–41
TektonScheduler embeds CommonSpec but its Validate() only calls ts.Spec.MultiClusterConfig.validate() so it never calls CommonSpec.validate().

Copilot

Pull request overview

Adds defense-in-depth validation to prevent installing Tekton components into well-known Kubernetes system namespaces via spec.targetNamespace, and ensures TektonResult applies the shared CommonSpec validation path so the denylist (and existing empty check) are enforced consistently.

Changes:

Add a reserved system namespace denylist (kube-system, kube-public, kube-node-lease, default) to CommonSpec.validate() across platforms, while preserving the OpenShift-only openshift-operators restriction.
Ensure TektonResultSpec.validate() invokes the embedded CommonSpec.validate() so TektonResult no longer bypasses shared validation.
Extend unit tests to cover denylisted namespaces and the TektonResult validation path.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
pkg/apis/operator/v1alpha1/common_validation.go	Adds reserved namespace denylist enforcement to shared `CommonSpec` validation.
pkg/apis/operator/v1alpha1/common_validation_test.go	Updates CommonSpec validation tests for reserved namespace denylist (Kubernetes + one OpenShift case).
pkg/apis/operator/v1alpha1/tektonresult_validation.go	Ensures TektonResultSpec runs shared CommonSpec validation.
pkg/apis/operator/v1alpha1/tektonresult_validation_test.go	Adds a test covering TektonResult’s denylisted targetNamespace validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 		{name: "ns-openshift-operators", targetNamespace: "openshift-operators", err: "", isOpenshift: false},
+		{name: "openshift-ns-default", targetNamespace: "default", err: "invalid value: default: spec.targetNamespace\n'default' is a reserved system namespace and is not allowed", isOpenshift: true},
 		{name: "openshift-ns-openshift-operators", targetNamespace: "openshift-operators", err: "invalid value: openshift-operators: spec.targetNamespace\n'openshift-operators' namespace is not allowed", isOpenshift: true},
 		{name: "openshift-ns-openshift-pipelines", targetNamespace: "openshift-pipelines", err: "", isOpenshift: true},


l-qing · 2026-07-14T02:26:22Z

+func isReservedSystemNamespace(namespace string) bool {
+	for _, reservedNamespace := range reservedSystemNamespaces {


Minor: this loop can be simplified with slices.Contains (Go 1.21+):

return slices.Contains(reservedSystemNamespaces, namespace)

mvanhorn · 2026-07-18T19:17:44Z

Done - TektonScheduler.Validate() now calls ts.Spec.CommonSpec.validate("spec") the same way the other components do, so the common spec checks (targetNamespace presence + reserved-namespace rejection) actually run. Updated the existing scheduler validation fixtures to set a valid targetNamespace and added a test asserting a reserved system namespace is now rejected. go build/vet/test on the package are green.

codecov-commenter · 2026-07-18T19:20:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.51%. Comparing base (0c7ae21) to head (0f7b005).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3507      +/-   ##
==========================================
+ Coverage   25.47%   25.51%   +0.03%     
==========================================
  Files         449      449              
  Lines       23363    23374      +11     
==========================================
+ Hits         5952     5963      +11     
  Misses      16725    16725              
  Partials      686      686

Flag	Coverage Δ
unit-tests	`25.51% <ø> (+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.

CommonSpec.validate() previously accepted any non-empty targetNamespace on Kubernetes, allowing components to be installed into well-known system namespaces such as kube-system, kube-public, kube-node-lease, or default. Add a defense-in-depth denylist that rejects those four reserved namespaces on both Kubernetes and OpenShift, while preserving the existing OpenShift-only openshift-operators rejection. Because validate() is the shared method called by every component validator, the denylist applies to all of them with no caller changes. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

TektonResult.Validate calls TektonResultSpec.validate, which did not call the embedded CommonSpec.validate, so the new system-namespace denylist (and the pre-existing empty-field check) was bypassed for TektonResult. Wire the common validation into TektonResultSpec.validate so the denylist is enforced consistently across every CommonSpec-backed component. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

TektonScheduler embeds CommonSpec, but Validate() only called ts.Spec.MultiClusterConfig.validate(), so the common spec checks (targetNamespace presence and reserved-namespace rejection) never ran. Call ts.Spec.CommonSpec.validate("spec") the way every other component does. Set a valid targetNamespace in the existing scheduler validation fixtures, and add a test asserting a reserved system namespace is now rejected. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

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

tekton-robot requested a review from anithapriyanatarajan June 17, 2026 11:41

tekton-robot requested a review from pramodbindal June 17, 2026 11:41

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 17, 2026

jkhelil reviewed Jun 18, 2026

View reviewed changes

jkhelil requested a review from Copilot June 18, 2026 07:45

Copilot started reviewing on behalf of jkhelil June 18, 2026 07:46 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

l-qing reviewed Jul 14, 2026

View reviewed changes

mvanhorn added 3 commits July 20, 2026 08:08

mvanhorn force-pushed the feat/3248-deny-system-namespaces branch from 68bc9dd to 0f7b005 Compare July 20, 2026 15:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add denylist for system namespaces in targetNamespace validation#3507

Add denylist for system namespaces in targetNamespace validation#3507
mvanhorn wants to merge 3 commits into
tektoncd:mainfrom
mvanhorn:feat/3248-deny-system-namespaces

mvanhorn commented Jun 17, 2026

Uh oh!

tekton-robot commented Jun 17, 2026

Uh oh!

jkhelil Jun 18, 2026

Uh oh!

jkhelil Jun 18, 2026

Uh oh!

jkhelil commented Jun 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

l-qing Jul 14, 2026

Uh oh!

mvanhorn commented Jul 18, 2026

Uh oh!

codecov-commenter commented Jul 18, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		func isReservedSystemNamespace(namespace string) bool {
		for _, reservedNamespace := range reservedSystemNamespaces {

Uh oh!

Conversation

mvanhorn commented Jun 17, 2026

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Jun 17, 2026

Uh oh!

jkhelil Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jkhelil Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jkhelil commented Jun 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

l-qing Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

mvanhorn commented Jul 18, 2026

Uh oh!

codecov-commenter commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Jul 18, 2026 •

edited

Loading