Catch conflicts between namespace, pod and service arguments

We catch some common mistakes when querying for namespaces.

* We return an error if the pod and service flags request different
  namespaces, as this will never yield any flows.
* We return an error if the namespace and the pod and/or service flags
  request flow from different namespace, as it is not clear what for
  example `hubble observe -n foo --pod bar/buzz --pod blub` should do.

Signed-off-by: Fabian Fischer <[email protected]>

cmd/observe/flows_filter.go

@@ -95,6 +95,25 @@ func (nm namespaceModifier) applyDest(ff *flowpb.FlowFilter) {
 	ff.DestinationService = addNamespacesToFilter(ff.DestinationService, nm)
 }
 
+func (nm namespaceModifier) conflicts(names []string) error {
+	for _, ns := range nm {
+		for _, name := range names {
+			if nn := namespaceFromName(name); nn != "" && nn != ns {
+				return fmt.Errorf("namespace conflict: %q does not contain %q", ns, name)
+			}
+		}
+	}
+	return nil
+}
+
+func namespaceFromName(name string) string {
+	namespacedName := strings.Split(name, "/")
+	if len(namespacedName) > 1 {
+		return namespacedName[0]
+	}
+	return ""
+}
+
 func addNamespacesToFilter(filter []string, ns []string) []string {
 	if len(ns) == 0 || len(filter) == 0 {
 		return filter
@@ -165,6 +184,10 @@ func newFlowFilter() *flowFilter {
 			{"from-fqdn", "from-ip", "ip", "fqdn", "from-pod", "pod"},
 			{"to-fqdn", "to-ip", "ip", "fqdn", "to-namespace", "namespace"},
 			{"to-fqdn", "to-ip", "ip", "fqdn", "to-pod", "pod"},
+			{"to-pod", "namespace"},
+			{"from-pod", "namespace"},
+			{"to-service", "namespace"},
+			{"from-service", "namespace"},
 			{"label", "from-label"},
 			{"label", "to-label"},
 			{"namespace", "from-service"},
@@ -220,6 +243,41 @@ func (of *flowFilter) checkConflict(t *filterTracker) error {
 	return nil
 }
 
+// checkInconsistentNamespaces checks if the namespaces in pods and services make sense
+// i.e. it checks that we don't request a service in one namespace with pods in another namespace
+func checkInconsistentNamespaces(pods, services []string) error {
+	for _, pod := range pods {
+		podNs := namespaceFromName(pod)
+		if podNs == "" {
+			continue
+		}
+		for _, svc := range services {
+			if ns := namespaceFromName(svc); podNs != ns {
+				return fmt.Errorf("namespace of service %q conflict with pod %q", svc, podNs)
+			}
+		}
+	}
+	return nil
+}
+
+// checkNamespaceConflicts checks for conflicts in namespaces, pods and services
+func (t *filterTracker) checkNamespaceConflicts(ff *flowpb.FlowFilter) error {
+	if ff == nil {
+		return nil
+	}
+	return errors.Join(t.ns.conflicts(ff.SourcePod),
+		t.ns.conflicts(ff.SourceService),
+		t.ns.conflicts(ff.DestinationPod),
+		t.ns.conflicts(ff.DestinationService),
+		t.srcNs.conflicts(ff.SourcePod),
+		t.srcNs.conflicts(ff.SourceService),
+		t.dstNs.conflicts(ff.DestinationPod),
+		t.dstNs.conflicts(ff.DestinationService),
+		checkInconsistentNamespaces(ff.SourcePod, ff.SourceService),
+		checkInconsistentNamespaces(ff.DestinationPod, ff.DestinationService),
+	)
+}
+
 func parseTCPFlags(val string) (*flowpb.TCPFlags, error) {
 	flags := &flowpb.TCPFlags{}
 	s := strings.Split(val, ",")
@@ -627,7 +685,10 @@ func (of *flowFilter) set(f *filterTracker, name, val string, track bool) error
 		}
 	}
 
-	return nil
+	if err := f.checkNamespaceConflicts(f.left); err != nil {
+		return err
+	}
+	return f.checkNamespaceConflicts(f.right)
 }
 
 func (of flowFilter) Type() string {

cmd/observe/flows_filter_test.go

@@ -831,22 +831,41 @@ func TestNamespace(t *testing.T) {
 				{SourcePod: []string{"kube-system/"}, DestinationPod: []string{"cilium/foo-9c76d6c95-tf788"}},
 			},
 		},
+		{
+			name:    "conflicting pod and namespace",
+			flags:   []string{"--to-namespace", "kube-system", "--to-pod", "cilium/foo-9c76d6c95-tf788"},
+			filters: []*flowpb.FlowFilter{},
+			err:     `conflicting namepace: "kube-system" does not contain "cilium/foo-9c76d6c95-tf788"`,
+		},
+		{
+			name:    "conflicting svc and namespace",
+			flags:   []string{"--from-namespace", "kube-system", "--from-service", "cilium/foo"},
+			filters: []*flowpb.FlowFilter{},
+			err:     `conflicting namepace: "kube-system" does not contain "cilium/foo"`,
+		},
+		{
+			name:    "conflicting svc and pod",
+			flags:   []string{"--from-service", "cilium/foo", "--from-pod", "kube-system/hubble"},
+			filters: []*flowpb.FlowFilter{},
+			err:     `conflicting namepace: namespace of service "cilium/foo" conflict with pod "kube-system/hubble"`,
+		},
 	}
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
 			f := newFlowFilter()
 			cmd := newFlowsCmdWithFilter(viper.New(), f)
 			err := cmd.Flags().Parse(tc.flags)
-			diff := cmp.Diff(tc.filters, f.whitelist.flowFilters(), cmpopts.IgnoreUnexported(flowpb.FlowFilter{}))
-			if diff != "" {
-				t.Errorf("mismatch (-want +got):\n%s", diff)
-			}
 			if tc.err != "" {
 				assert.Errorf(t, err, tc.err)
+				return
 			} else {
 				assert.NoError(t, err)
 			}
 			assert.Nil(t, f.blacklist)
+			diff := cmp.Diff(tc.filters, f.whitelist.flowFilters(), cmpopts.IgnoreUnexported(flowpb.FlowFilter{}))
+			if diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
 		})
 	}
 }