Skip to content

Commit 3a8738c

Browse files
authored
Fix/vectorpipeline namespace validation (#216)
* test(e2e): add regression test for VectorPipeline namespace validation * fix: validate VectorPipeline only against VectorAggregator in same namespace
1 parent 0cab6ac commit 3a8738c

8 files changed

Lines changed: 222 additions & 0 deletions

File tree

internal/controller/pipeline_controller.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,10 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
193193

194194
if pipelineCR.GetNamespace() != "" {
195195
for _, vector := range vectorAggregators {
196+
// VectorPipeline should only be validated against VectorAggregator in the same namespace
197+
if vector.Namespace != pipelineCR.GetNamespace() {
198+
continue
199+
}
196200
var selectorLabels map[string]string
197201
if vector.Spec.Selector != nil {
198202
selectorLabels = vector.Spec.Selector.MatchLabels

test/e2e/framework/framework.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,17 @@ func (f *Framework) DeleteClusterResource(kind, name string) {
783783
}
784784
}
785785

786+
// DeleteResourceInNamespace deletes a Kubernetes resource in a specific namespace
787+
func (f *Framework) DeleteResourceInNamespace(kind, name, namespace string) {
788+
By(fmt.Sprintf("deleting %s %s in namespace %s", kind, name, namespace))
789+
client := kubectl.NewClient(namespace)
790+
err := client.Delete(kind, name)
791+
if err != nil {
792+
// Log warning but don't fail - resource might already be deleted
793+
GinkgoWriter.Printf("Warning: failed to delete %s %s in namespace %s: %v\n", kind, name, namespace, err)
794+
}
795+
}
796+
786797
// WaitForPodReadyInNamespace waits for a pod to become ready in a specific namespace
787798
func (f *Framework) WaitForPodReadyInNamespace(podName, namespace string) {
788799
By(fmt.Sprintf("waiting for pod %s to be ready in namespace %s", podName, namespace))
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
Copyright 2024.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package e2e
18+
19+
import (
20+
"time"
21+
22+
. "github.com/onsi/ginkgo/v2"
23+
. "github.com/onsi/gomega"
24+
25+
"github.com/kaasops/vector-operator/test/e2e/framework"
26+
"github.com/kaasops/vector-operator/test/e2e/framework/config"
27+
)
28+
29+
// Resource names used in namespace validation tests
30+
const (
31+
nsValidationSecondNamespace = "test-ns-validation-other"
32+
nsValidationAggregatorWithSecret = "aggregator-with-secret"
33+
nsValidationAggregatorNoSecret = "aggregator-without-secret"
34+
nsValidationPipeline = "test-pipeline"
35+
nsValidationSecret = "test-credentials"
36+
)
37+
38+
// Namespace Validation tests verify that VectorPipeline is validated only against
39+
// VectorAggregator instances in the SAME namespace, not all aggregators with matching selectors.
40+
//
41+
// This is a regression test for the issue where VectorPipeline validation checked against
42+
// ALL VectorAggregators with matching label selectors, regardless of namespace.
43+
// This caused validation failures when aggregators in other namespaces were missing
44+
// required secrets/resources that only existed in the pipeline's namespace.
45+
//
46+
// Related to issue #201 - the fix for ClusterVectorPipeline selector matching did not
47+
// address the namespace isolation for namespaced VectorPipeline resources.
48+
var _ = Describe("Namespace Validation Isolation", Label(config.LabelRegression), Ordered, func() {
49+
f := framework.NewUniqueFramework("test-ns-validation")
50+
51+
BeforeAll(func() {
52+
f.Setup()
53+
54+
By("creating second namespace for isolation test")
55+
f.ApplyTestDataWithoutNamespaceReplacement("namespace-validation/second-namespace.yaml")
56+
57+
// Give the namespace time to be created
58+
time.Sleep(2 * time.Second)
59+
})
60+
61+
AfterAll(func() {
62+
By("cleaning up VectorPipeline")
63+
f.DeleteResource("vectorpipeline", nsValidationPipeline)
64+
65+
By("cleaning up VectorAggregators")
66+
f.DeleteResource("vectoraggregator", nsValidationAggregatorWithSecret)
67+
f.DeleteResourceInNamespace("vectoraggregator", nsValidationAggregatorNoSecret, nsValidationSecondNamespace)
68+
69+
By("cleaning up secret")
70+
f.DeleteResource("secret", nsValidationSecret)
71+
72+
By("cleaning up second namespace")
73+
f.DeleteClusterResource("namespace", nsValidationSecondNamespace)
74+
75+
f.Teardown()
76+
f.PrintMetrics()
77+
})
78+
79+
Context("VectorPipeline with VectorAggregators in different namespaces", func() {
80+
It("should validate VectorPipeline only against VectorAggregator in the same namespace", func() {
81+
By("creating secret in main namespace")
82+
f.ApplyTestData("namespace-validation/secret.yaml")
83+
84+
By("deploying VectorAggregator WITH secret in main namespace")
85+
f.ApplyTestData("namespace-validation/aggregator-ns1.yaml")
86+
87+
By("deploying VectorAggregator WITHOUT secret in second namespace")
88+
// This aggregator references the same secret that does NOT exist in the second namespace
89+
// If the bug exists, the pipeline will be validated against this aggregator and fail
90+
f.ApplyTestDataWithoutNamespaceReplacement("namespace-validation/aggregator-ns2.yaml")
91+
92+
By("waiting for aggregator in main namespace to be ready")
93+
f.WaitForDeploymentReady(nsValidationAggregatorWithSecret + "-aggregator")
94+
95+
// Note: The aggregator in the second namespace may not become ready
96+
// because the secret doesn't exist there, but that's expected and irrelevant
97+
// for this test. What matters is that the pipeline in namespace 1
98+
// should NOT be validated against it.
99+
100+
By("creating VectorPipeline with matching labels in main namespace")
101+
f.ApplyTestData("namespace-validation/pipeline.yaml")
102+
103+
By("waiting for VectorPipeline to become valid")
104+
// If the bug exists:
105+
// - Pipeline is validated against BOTH aggregators (both have matching selector)
106+
// - Validation against aggregator in second namespace FAILS (missing secret)
107+
// - Pipeline status becomes invalid
108+
//
109+
// If the bug is fixed:
110+
// - Pipeline is validated ONLY against aggregator in the same namespace
111+
// - Secret exists in main namespace
112+
// - Pipeline status becomes valid
113+
f.WaitForPipelineValid(nsValidationPipeline)
114+
115+
By("verifying VectorPipeline has aggregator role")
116+
role := f.GetPipelineStatus(nsValidationPipeline, "role")
117+
Expect(role).To(Equal("aggregator"), "Pipeline with vector source should have aggregator role")
118+
119+
// Note: We don't verify that the aggregator config contains the pipeline here
120+
// because that's tested in other tests. The key assertion is that validation
121+
// passes quickly (not timing out waiting for the aggregator in the other namespace).
122+
})
123+
})
124+
})
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# VectorAggregator in the MAIN namespace with the required secret
2+
# This aggregator has an env var from a secret that EXISTS in this namespace
3+
apiVersion: observability.kaasops.io/v1alpha1
4+
kind: VectorAggregator
5+
metadata:
6+
name: aggregator-with-secret
7+
spec:
8+
image: timberio/vector:0.40.0-alpine
9+
replicas: 1
10+
selector:
11+
matchLabels:
12+
app: ns-validation-test
13+
env:
14+
- name: API_KEY
15+
valueFrom:
16+
secretKeyRef:
17+
name: test-credentials
18+
key: api-key
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# VectorAggregator in the SECOND namespace WITHOUT the required secret
2+
# This aggregator references the same secret that does NOT exist in this namespace
3+
# If the bug exists, VectorPipeline from namespace 1 will be validated against this
4+
# aggregator, and validation will fail because the secret doesn't exist here
5+
apiVersion: observability.kaasops.io/v1alpha1
6+
kind: VectorAggregator
7+
metadata:
8+
name: aggregator-without-secret
9+
namespace: test-ns-validation-other
10+
spec:
11+
image: timberio/vector:0.40.0-alpine
12+
replicas: 1
13+
selector:
14+
matchLabels:
15+
app: ns-validation-test
16+
env:
17+
- name: API_KEY
18+
valueFrom:
19+
secretKeyRef:
20+
name: test-credentials
21+
key: api-key
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# VectorPipeline with labels matching the aggregators' selector
2+
# This pipeline should ONLY be validated against the aggregator in the SAME namespace
3+
# Bug: It gets validated against ALL aggregators with matching selector (regardless of namespace)
4+
apiVersion: observability.kaasops.io/v1alpha1
5+
kind: VectorPipeline
6+
metadata:
7+
name: test-pipeline
8+
labels:
9+
app: ns-validation-test
10+
spec:
11+
sources:
12+
vector_source:
13+
type: vector
14+
address: "0.0.0.0:9000"
15+
transforms:
16+
process:
17+
type: remap
18+
inputs:
19+
- vector_source
20+
source: |
21+
.namespace_validation_test = true
22+
sinks:
23+
console:
24+
type: console
25+
inputs:
26+
- process
27+
encoding:
28+
codec: json
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Second namespace for testing namespace isolation in VectorPipeline validation
2+
# This namespace will have a VectorAggregator WITHOUT the required secret
3+
apiVersion: v1
4+
kind: Namespace
5+
metadata:
6+
name: test-ns-validation-other
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Secret that will be used by VectorAggregator in the main namespace
2+
# This secret will NOT exist in the second namespace, causing validation failure
3+
# if the bug exists (pipeline validated against aggregator in wrong namespace)
4+
apiVersion: v1
5+
kind: Secret
6+
metadata:
7+
name: test-credentials
8+
type: Opaque
9+
stringData:
10+
api-key: "test-api-key-value"

0 commit comments

Comments
 (0)