Skip to content

Commit d225332

Browse files
committed
fix(tektonresult): address NetworkPolicy review feedback
Allow port-only DB egress for external Postgres, add Results NetworkPolicy e2e, and document admin DB access. Signed-off-by: adchauha <adchauha@redhat.com>
1 parent 35b09dc commit d225332

5 files changed

Lines changed: 177 additions & 37 deletions

File tree

config/base/generated-crds/operator.tekton.dev_tektonconfigs.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,6 @@ spec:
531531
type: string
532532
type: object
533533
type: array
534-
required:
535-
- options
536534
type: object
537535
multiclusterProxyAAE:
538536
description: MulticlusterProxyAAE holds the customizable options for

docs/NetworkPolicy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ NetworkPolicy is enabled.
188188
**Recommended:** add a dedicated temporary NetworkPolicy that allows a dedicated
189189
admin label. Do **not** reuse `app: tekton-results-api` on a debug pod — that label
190190
is also used by the Results API Service selector and can route API traffic to the
191-
wrong pod.
191+
wrong pod. A dedicated label (below) avoids that.
192192

193193
Apply directly:
194194

pkg/reconciler/kubernetes/tektonresult/networkpolicies.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,13 @@ func resultsDefaultPolicies(params networkpolicy.PlatformParams, props v1alpha1.
4242
postgresPort := intstr.FromInt32(portOrDefault(props.DBPort, defaultDBPort))
4343
tcp := corev1.ProtocolTCP
4444

45-
postgresPeer := networkingv1.NetworkPolicyPeer{
46-
PodSelector: &metav1.LabelSelector{
47-
MatchLabels: map[string]string{
48-
"app.kubernetes.io/name": "tekton-results-postgres",
49-
},
45+
// DB egress is port-only (no pod/CIDR peer). NetworkPolicy cannot match on
46+
// hostname/URL, so restricting To in-cluster postgres would break external DB
47+
// deployments. Any destination on db_port is allowed; for in-cluster postgres,
48+
// results-postgres ingress still limits who may connect.
49+
dbEgress := networkingv1.NetworkPolicyEgressRule{
50+
Ports: []networkingv1.NetworkPolicyPort{
51+
{Protocol: &tcp, Port: &postgresPort},
5052
},
5153
}
5254

@@ -63,7 +65,7 @@ func resultsDefaultPolicies(params networkpolicy.PlatformParams, props v1alpha1.
6365
},
6466
Ingress: []networkingv1.NetworkPolicyIngressRule{
6567
{
66-
// Console Plugin, CLI, watcher, and other internal clients
68+
// Console Plugin, CLI, routes, watcher, and other clients
6769
// live in many namespaces — allow from all namespaces.
6870
From: []networkingv1.NetworkPolicyPeer{
6971
{NamespaceSelector: &metav1.LabelSelector{}},
@@ -76,12 +78,7 @@ func resultsDefaultPolicies(params networkpolicy.PlatformParams, props v1alpha1.
7678
},
7779
Egress: []networkingv1.NetworkPolicyEgressRule{
7880
networkpolicy.DNSEgressRule(params),
79-
{
80-
Ports: []networkingv1.NetworkPolicyPort{
81-
{Protocol: &tcp, Port: &postgresPort},
82-
},
83-
To: []networkingv1.NetworkPolicyPeer{postgresPeer},
84-
},
81+
dbEgress,
8582
// Auth token review / impersonation talks to the API server.
8683
networkpolicy.APIServerEgressRule(),
8784
},
@@ -120,12 +117,7 @@ func resultsDefaultPolicies(params networkpolicy.PlatformParams, props v1alpha1.
120117
},
121118
Egress: []networkingv1.NetworkPolicyEgressRule{
122119
networkpolicy.DNSEgressRule(params),
123-
{
124-
Ports: []networkingv1.NetworkPolicyPort{
125-
{Protocol: &tcp, Port: &postgresPort},
126-
},
127-
To: []networkingv1.NetworkPolicyPeer{postgresPeer},
128-
},
120+
dbEgress,
129121
},
130122
},
131123
},

pkg/reconciler/kubernetes/tektonresult/networkpolicies_test.go

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestResultsDefaultPolicies(t *testing.T) {
4848
api := byName["results-api"]
4949
assertIngressHasPort(t, "results-api", api.Spec.Ingress, 8080)
5050
assertIngressHasPort(t, "results-api", api.Spec.Ingress, 9090)
51-
assertEgressHasPortToPostgres(t, "results-api", api.Spec.Egress, 5432)
51+
assertEgressHasDBPort(t, "results-api", api.Spec.Egress, 5432)
5252

5353
watcher := byName["results-watcher"]
5454
if got := len(watcher.Spec.Egress); got != 2 {
@@ -60,7 +60,7 @@ func TestResultsDefaultPolicies(t *testing.T) {
6060

6161
retention := byName["results-retention-policy-agent"]
6262
assertEgressHasDNS(t, "results-retention-policy-agent", retention.Spec.Egress)
63-
assertEgressHasPortToPostgres(t, "results-retention-policy-agent", retention.Spec.Egress, 5432)
63+
assertEgressHasDBPort(t, "results-retention-policy-agent", retention.Spec.Egress, 5432)
6464

6565
postgres := byName["results-postgres"]
6666
assertIngressHasPort(t, "results-postgres", postgres.Spec.Ingress, 5432)
@@ -82,9 +82,9 @@ func TestResultsDefaultPoliciesUsesSpecPorts(t *testing.T) {
8282

8383
assertIngressHasPort(t, "results-api", byName["results-api"].Spec.Ingress, 18080)
8484
assertIngressHasPort(t, "results-api", byName["results-api"].Spec.Ingress, 19090)
85-
assertEgressHasPortToPostgres(t, "results-api", byName["results-api"].Spec.Egress, 15432)
85+
assertEgressHasDBPort(t, "results-api", byName["results-api"].Spec.Egress, 15432)
8686
assertIngressHasPort(t, "results-watcher", byName["results-watcher"].Spec.Ingress, 19090)
87-
assertEgressHasPortToPostgres(t, "results-retention-policy-agent", byName["results-retention-policy-agent"].Spec.Egress, 15432)
87+
assertEgressHasDBPort(t, "results-retention-policy-agent", byName["results-retention-policy-agent"].Spec.Egress, 15432)
8888
assertIngressHasPort(t, "results-postgres", byName["results-postgres"].Spec.Ingress, 15432)
8989
}
9090

@@ -143,27 +143,22 @@ func assertIngressHasPort(t *testing.T, policy string, rules []networkingv1.Netw
143143
t.Errorf("%s: expected ingress port %d", policy, port)
144144
}
145145

146-
func assertEgressHasPortToPostgres(t *testing.T, policy string, rules []networkingv1.NetworkPolicyEgressRule, port int32) {
146+
// assertEgressHasDBPort checks for a port-only DB egress rule (no To peer), so
147+
// in-cluster and external databases both work without a custom NetworkPolicy.
148+
func assertEgressHasDBPort(t *testing.T, policy string, rules []networkingv1.NetworkPolicyEgressRule, port int32) {
147149
t.Helper()
148150
want := intstr.FromInt32(port)
149151
for _, rule := range rules {
150-
hasPort := false
151-
for _, p := range rule.Ports {
152-
if p.Port != nil && *p.Port == want {
153-
hasPort = true
154-
break
155-
}
156-
}
157-
if !hasPort {
152+
if len(rule.To) != 0 {
158153
continue
159154
}
160-
for _, peer := range rule.To {
161-
if peer.PodSelector != nil && peer.PodSelector.MatchLabels["app.kubernetes.io/name"] == "tekton-results-postgres" {
155+
for _, p := range rule.Ports {
156+
if p.Port != nil && *p.Port == want {
162157
return
163158
}
164159
}
165160
}
166-
t.Errorf("%s: expected egress TCP/%d to tekton-results-postgres", policy, port)
161+
t.Errorf("%s: expected port-only egress TCP/%d (no To peer)", policy, port)
167162
}
168163

169164
func assertEgressHasDNS(t *testing.T, policy string, rules []networkingv1.NetworkPolicyEgressRule) {
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
//go:build e2e
2+
// +build e2e
3+
4+
/*
5+
Copyright 2026 The Tekton Authors
6+
7+
Licensed under the Apache License, Version 2.0 (the "License");
8+
you may not use this file except in compliance with the License.
9+
You may obtain a copy of the License at
10+
11+
http://www.apache.org/licenses/LICENSE-2.0
12+
13+
Unless required by applicable law or agreed to in writing, software
14+
distributed under the License is distributed on an "AS IS" BASIS,
15+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
See the License for the specific language governing permissions and
17+
limitations under the License.
18+
*/
19+
20+
package common
21+
22+
import (
23+
"context"
24+
"fmt"
25+
"testing"
26+
27+
"github.com/tektoncd/operator/test/client"
28+
"github.com/tektoncd/operator/test/resources"
29+
"github.com/tektoncd/operator/test/utils"
30+
pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
31+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32+
)
33+
34+
// TestTektonResultNetworkPolicy verifies NetworkPolicies are created by default
35+
// for Results workloads, that Results stays Ready under those policies (API can
36+
// reach Postgres; watcher can reach the API server), and that toggling
37+
// spec.networkPolicy.disabled correctly adds and removes the policies.
38+
func TestTektonResultNetworkPolicy(t *testing.T) {
39+
40+
crNames := utils.GetResourceNames()
41+
clients := client.Setup(t, crNames.TargetNamespace)
42+
43+
utils.CleanupOnInterrupt(func() { utils.TearDownPipeline(clients, crNames.TektonPipeline) })
44+
utils.CleanupOnInterrupt(func() { utils.TearDownResult(clients, crNames.TektonResult) })
45+
defer utils.TearDownResult(clients, crNames.TektonResult)
46+
defer utils.TearDownPipeline(clients, crNames.TektonPipeline)
47+
48+
resources.EnsureNoTektonConfigInstance(t, clients, crNames)
49+
50+
if _, err := resources.EnsureTektonPipelineExists(clients.TektonPipeline(), crNames); err != nil {
51+
t.Fatalf("TektonPipeline %q failed to create: %v", crNames.TektonPipeline, err)
52+
}
53+
resources.AssertTektonPipelineCRReadyStatus(t, clients, crNames)
54+
55+
// Operator creates default TLS + Postgres secrets when missing.
56+
if _, err := resources.EnsureTektonResultExists(clients.TektonResult(), crNames); err != nil {
57+
t.Fatalf("TektonResult %q failed to create: %v", crNames.TektonResult, err)
58+
}
59+
resources.AssertTektonResultCRReadyStatus(t, clients, crNames)
60+
61+
expectedPolicies := []string{
62+
"results-default-deny",
63+
"results-api",
64+
"results-watcher",
65+
"results-retention-policy-agent",
66+
"results-postgres",
67+
}
68+
69+
t.Run("default-policies-created", func(t *testing.T) {
70+
resources.AssertNetworkPoliciesExist(t, clients, crNames.TargetNamespace, expectedPolicies)
71+
})
72+
73+
// Ready status already proves API↔DB and watcher↔API-server under NP.
74+
// A successful TaskRun additionally exercises watcher API-server egress
75+
t.Run("results-functional-with-networkpolicies", func(t *testing.T) {
76+
resources.AssertTektonResultCRReadyStatus(t, clients, crNames)
77+
78+
taskRun := createResultNPProbeTaskRun(crNames.TargetNamespace)
79+
createdTaskRun, err := clients.TektonClient.TaskRuns(crNames.TargetNamespace).Create(
80+
context.TODO(), taskRun, metav1.CreateOptions{})
81+
if err != nil {
82+
t.Fatalf("failed to create TaskRun: %v", err)
83+
}
84+
85+
if err := resources.WaitForTaskRunHappy(
86+
clients.TektonClient,
87+
crNames.TargetNamespace,
88+
createdTaskRun.Name,
89+
func(tr *pipelinev1.TaskRun) (bool, error) {
90+
if tr.IsDone() {
91+
if tr.IsSuccessful() {
92+
return true, nil
93+
}
94+
return false, fmt.Errorf("TaskRun failed")
95+
}
96+
return false, nil
97+
},
98+
); err != nil {
99+
t.Fatalf("TaskRun did not complete successfully under NetworkPolicy: %v", err)
100+
}
101+
102+
// Results must still be Ready after the TaskRun (watcher/API/DB path).
103+
resources.AssertTektonResultCRReadyStatus(t, clients, crNames)
104+
})
105+
106+
t.Run("disable-removes-policies", func(t *testing.T) {
107+
tr, err := clients.TektonResult().Get(context.TODO(), crNames.TektonResult, metav1.GetOptions{})
108+
if err != nil {
109+
t.Fatalf("failed to get TektonResult: %v", err)
110+
}
111+
tr.Spec.NetworkPolicy.Disabled = true
112+
if _, err := clients.TektonResult().Update(context.TODO(), tr, metav1.UpdateOptions{}); err != nil {
113+
t.Fatalf("failed to disable NetworkPolicy on TektonResult: %v", err)
114+
}
115+
resources.AssertTektonResultCRReadyStatus(t, clients, crNames)
116+
resources.AssertNetworkPoliciesAbsent(t, clients, crNames.TargetNamespace, expectedPolicies)
117+
})
118+
119+
t.Run("reenable-restores-policies", func(t *testing.T) {
120+
tr, err := clients.TektonResult().Get(context.TODO(), crNames.TektonResult, metav1.GetOptions{})
121+
if err != nil {
122+
t.Fatalf("failed to get TektonResult: %v", err)
123+
}
124+
tr.Spec.NetworkPolicy.Disabled = false
125+
if _, err := clients.TektonResult().Update(context.TODO(), tr, metav1.UpdateOptions{}); err != nil {
126+
t.Fatalf("failed to re-enable NetworkPolicy on TektonResult: %v", err)
127+
}
128+
resources.AssertTektonResultCRReadyStatus(t, clients, crNames)
129+
resources.AssertNetworkPoliciesExist(t, clients, crNames.TargetNamespace, expectedPolicies)
130+
})
131+
}
132+
133+
// createResultNPProbeTaskRun creates a minimal TaskRun that completes quickly.
134+
// Completion exercises the Results watcher watch of the API server under NP.
135+
func createResultNPProbeTaskRun(namespace string) *pipelinev1.TaskRun {
136+
return &pipelinev1.TaskRun{
137+
ObjectMeta: metav1.ObjectMeta{
138+
GenerateName: "result-np-probe-taskrun-",
139+
Namespace: namespace,
140+
},
141+
Spec: pipelinev1.TaskRunSpec{
142+
ServiceAccountName: "default",
143+
TaskSpec: &pipelinev1.TaskSpec{
144+
Steps: []pipelinev1.Step{
145+
{
146+
Name: "echo",
147+
Image: "busybox:stable",
148+
Command: []string{"echo"},
149+
Args: []string{"results NetworkPolicy probe"},
150+
},
151+
},
152+
},
153+
},
154+
}
155+
}

0 commit comments

Comments
 (0)