diff --git a/internal/controller/pipeline_controller.go b/internal/controller/pipeline_controller.go index ebdf5a5..ebdecbf 100644 --- a/internal/controller/pipeline_controller.go +++ b/internal/controller/pipeline_controller.go @@ -193,6 +193,10 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if pipelineCR.GetNamespace() != "" { for _, vector := range vectorAggregators { + // VectorPipeline should only be validated against VectorAggregator in the same namespace + if vector.Namespace != pipelineCR.GetNamespace() { + continue + } var selectorLabels map[string]string if vector.Spec.Selector != nil { selectorLabels = vector.Spec.Selector.MatchLabels diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index ffece64..38c3f6e 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -783,6 +783,17 @@ func (f *Framework) DeleteClusterResource(kind, name string) { } } +// DeleteResourceInNamespace deletes a Kubernetes resource in a specific namespace +func (f *Framework) DeleteResourceInNamespace(kind, name, namespace string) { + By(fmt.Sprintf("deleting %s %s in namespace %s", kind, name, namespace)) + client := kubectl.NewClient(namespace) + err := client.Delete(kind, name) + if err != nil { + // Log warning but don't fail - resource might already be deleted + GinkgoWriter.Printf("Warning: failed to delete %s %s in namespace %s: %v\n", kind, name, namespace, err) + } +} + // WaitForPodReadyInNamespace waits for a pod to become ready in a specific namespace func (f *Framework) WaitForPodReadyInNamespace(podName, namespace string) { By(fmt.Sprintf("waiting for pod %s to be ready in namespace %s", podName, namespace)) diff --git a/test/e2e/namespace_validation_e2e_test.go b/test/e2e/namespace_validation_e2e_test.go new file mode 100644 index 0000000..b894f23 --- /dev/null +++ b/test/e2e/namespace_validation_e2e_test.go @@ -0,0 +1,124 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/kaasops/vector-operator/test/e2e/framework" + "github.com/kaasops/vector-operator/test/e2e/framework/config" +) + +// Resource names used in namespace validation tests +const ( + nsValidationSecondNamespace = "test-ns-validation-other" + nsValidationAggregatorWithSecret = "aggregator-with-secret" + nsValidationAggregatorNoSecret = "aggregator-without-secret" + nsValidationPipeline = "test-pipeline" + nsValidationSecret = "test-credentials" +) + +// Namespace Validation tests verify that VectorPipeline is validated only against +// VectorAggregator instances in the SAME namespace, not all aggregators with matching selectors. +// +// This is a regression test for the issue where VectorPipeline validation checked against +// ALL VectorAggregators with matching label selectors, regardless of namespace. +// This caused validation failures when aggregators in other namespaces were missing +// required secrets/resources that only existed in the pipeline's namespace. +// +// Related to issue #201 - the fix for ClusterVectorPipeline selector matching did not +// address the namespace isolation for namespaced VectorPipeline resources. +var _ = Describe("Namespace Validation Isolation", Label(config.LabelRegression), Ordered, func() { + f := framework.NewUniqueFramework("test-ns-validation") + + BeforeAll(func() { + f.Setup() + + By("creating second namespace for isolation test") + f.ApplyTestDataWithoutNamespaceReplacement("namespace-validation/second-namespace.yaml") + + // Give the namespace time to be created + time.Sleep(2 * time.Second) + }) + + AfterAll(func() { + By("cleaning up VectorPipeline") + f.DeleteResource("vectorpipeline", nsValidationPipeline) + + By("cleaning up VectorAggregators") + f.DeleteResource("vectoraggregator", nsValidationAggregatorWithSecret) + f.DeleteResourceInNamespace("vectoraggregator", nsValidationAggregatorNoSecret, nsValidationSecondNamespace) + + By("cleaning up secret") + f.DeleteResource("secret", nsValidationSecret) + + By("cleaning up second namespace") + f.DeleteClusterResource("namespace", nsValidationSecondNamespace) + + f.Teardown() + f.PrintMetrics() + }) + + Context("VectorPipeline with VectorAggregators in different namespaces", func() { + It("should validate VectorPipeline only against VectorAggregator in the same namespace", func() { + By("creating secret in main namespace") + f.ApplyTestData("namespace-validation/secret.yaml") + + By("deploying VectorAggregator WITH secret in main namespace") + f.ApplyTestData("namespace-validation/aggregator-ns1.yaml") + + By("deploying VectorAggregator WITHOUT secret in second namespace") + // This aggregator references the same secret that does NOT exist in the second namespace + // If the bug exists, the pipeline will be validated against this aggregator and fail + f.ApplyTestDataWithoutNamespaceReplacement("namespace-validation/aggregator-ns2.yaml") + + By("waiting for aggregator in main namespace to be ready") + f.WaitForDeploymentReady(nsValidationAggregatorWithSecret + "-aggregator") + + // Note: The aggregator in the second namespace may not become ready + // because the secret doesn't exist there, but that's expected and irrelevant + // for this test. What matters is that the pipeline in namespace 1 + // should NOT be validated against it. + + By("creating VectorPipeline with matching labels in main namespace") + f.ApplyTestData("namespace-validation/pipeline.yaml") + + By("waiting for VectorPipeline to become valid") + // If the bug exists: + // - Pipeline is validated against BOTH aggregators (both have matching selector) + // - Validation against aggregator in second namespace FAILS (missing secret) + // - Pipeline status becomes invalid + // + // If the bug is fixed: + // - Pipeline is validated ONLY against aggregator in the same namespace + // - Secret exists in main namespace + // - Pipeline status becomes valid + f.WaitForPipelineValid(nsValidationPipeline) + + By("verifying VectorPipeline has aggregator role") + role := f.GetPipelineStatus(nsValidationPipeline, "role") + Expect(role).To(Equal("aggregator"), "Pipeline with vector source should have aggregator role") + + // Note: We don't verify that the aggregator config contains the pipeline here + // because that's tested in other tests. The key assertion is that validation + // passes quickly (not timing out waiting for the aggregator in the other namespace). + }) + }) +}) diff --git a/test/e2e/testdata/namespace-validation/aggregator-ns1.yaml b/test/e2e/testdata/namespace-validation/aggregator-ns1.yaml new file mode 100644 index 0000000..244170a --- /dev/null +++ b/test/e2e/testdata/namespace-validation/aggregator-ns1.yaml @@ -0,0 +1,18 @@ +# VectorAggregator in the MAIN namespace with the required secret +# This aggregator has an env var from a secret that EXISTS in this namespace +apiVersion: observability.kaasops.io/v1alpha1 +kind: VectorAggregator +metadata: + name: aggregator-with-secret +spec: + image: timberio/vector:0.40.0-alpine + replicas: 1 + selector: + matchLabels: + app: ns-validation-test + env: + - name: API_KEY + valueFrom: + secretKeyRef: + name: test-credentials + key: api-key diff --git a/test/e2e/testdata/namespace-validation/aggregator-ns2.yaml b/test/e2e/testdata/namespace-validation/aggregator-ns2.yaml new file mode 100644 index 0000000..ed29b0d --- /dev/null +++ b/test/e2e/testdata/namespace-validation/aggregator-ns2.yaml @@ -0,0 +1,21 @@ +# VectorAggregator in the SECOND namespace WITHOUT the required secret +# This aggregator references the same secret that does NOT exist in this namespace +# If the bug exists, VectorPipeline from namespace 1 will be validated against this +# aggregator, and validation will fail because the secret doesn't exist here +apiVersion: observability.kaasops.io/v1alpha1 +kind: VectorAggregator +metadata: + name: aggregator-without-secret + namespace: test-ns-validation-other +spec: + image: timberio/vector:0.40.0-alpine + replicas: 1 + selector: + matchLabels: + app: ns-validation-test + env: + - name: API_KEY + valueFrom: + secretKeyRef: + name: test-credentials + key: api-key diff --git a/test/e2e/testdata/namespace-validation/pipeline.yaml b/test/e2e/testdata/namespace-validation/pipeline.yaml new file mode 100644 index 0000000..b9c6465 --- /dev/null +++ b/test/e2e/testdata/namespace-validation/pipeline.yaml @@ -0,0 +1,28 @@ +# VectorPipeline with labels matching the aggregators' selector +# This pipeline should ONLY be validated against the aggregator in the SAME namespace +# Bug: It gets validated against ALL aggregators with matching selector (regardless of namespace) +apiVersion: observability.kaasops.io/v1alpha1 +kind: VectorPipeline +metadata: + name: test-pipeline + labels: + app: ns-validation-test +spec: + sources: + vector_source: + type: vector + address: "0.0.0.0:9000" + transforms: + process: + type: remap + inputs: + - vector_source + source: | + .namespace_validation_test = true + sinks: + console: + type: console + inputs: + - process + encoding: + codec: json diff --git a/test/e2e/testdata/namespace-validation/second-namespace.yaml b/test/e2e/testdata/namespace-validation/second-namespace.yaml new file mode 100644 index 0000000..96f60d0 --- /dev/null +++ b/test/e2e/testdata/namespace-validation/second-namespace.yaml @@ -0,0 +1,6 @@ +# Second namespace for testing namespace isolation in VectorPipeline validation +# This namespace will have a VectorAggregator WITHOUT the required secret +apiVersion: v1 +kind: Namespace +metadata: + name: test-ns-validation-other diff --git a/test/e2e/testdata/namespace-validation/secret.yaml b/test/e2e/testdata/namespace-validation/secret.yaml new file mode 100644 index 0000000..c2be536 --- /dev/null +++ b/test/e2e/testdata/namespace-validation/secret.yaml @@ -0,0 +1,10 @@ +# Secret that will be used by VectorAggregator in the main namespace +# This secret will NOT exist in the second namespace, causing validation failure +# if the bug exists (pipeline validated against aggregator in wrong namespace) +apiVersion: v1 +kind: Secret +metadata: + name: test-credentials +type: Opaque +stringData: + api-key: "test-api-key-value"