k8s: move filterPodLabels to k8s/utils package for SanitizePodLabels

Currently GetPodMetadata is the only caller of SanitizePodLabels but other callers will be introduced in successive changes. This change ensures the io.cilium.k8s.* labels are filtered for these callers as well. Signed-off-by: Tobias Klauser <tobias@cilium.io>
cilium · Mar 15, 2024 · 2309805 · 2309805
1 parent 9a26446
commit 2309805
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 88 deletions.
diff --git a/pkg/k8s/labels.go b/pkg/k8s/labels.go
@@ -4,11 +4,8 @@
 package k8s
 
 import (
-	"strings"
-
 	"github.com/sirupsen/logrus"
 
-	k8sConst "github.com/cilium/cilium/pkg/k8s/apis/cilium.io"
 	slim_corev1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/api/core/v1"
 	k8sUtils "github.com/cilium/cilium/pkg/k8s/utils"
 	"github.com/cilium/cilium/pkg/logging/logfields"
@@ -32,25 +29,11 @@ func GetPodMetadata(k8sNs *slim_corev1.Namespace, pod *slim_corev1.Pod) (contain
 
 	objMetaCpy := pod.ObjectMeta.DeepCopy()
 	annotations := objMetaCpy.Annotations
-	k8sLabels := filterPodLabels(objMetaCpy.Labels)
+	labels := k8sUtils.SanitizePodLabels(objMetaCpy.Labels, k8sNs, pod.Spec.ServiceAccountName, option.Config.ClusterName)
 
 	for _, containers := range pod.Spec.Containers {
 		containerPorts = append(containerPorts, containers.Ports...)
 	}
 
-	labels := k8sUtils.SanitizePodLabels(k8sLabels, k8sNs, pod.Spec.ServiceAccountName, option.Config.ClusterName)
-
 	return containerPorts, labels, annotations, nil
 }
-
-// filterPodLabels returns a copy of the given labels map, without the labels owned by Cilium.
-func filterPodLabels(labels map[string]string) map[string]string {
-	res := map[string]string{}
-	for k, v := range labels {
-		if strings.HasPrefix(k, k8sConst.LabelPrefix) {
-			continue
-		}
-		res[k] = v
-	}
-	return res
-}
diff --git a/pkg/k8s/labels_test.go b/pkg/k8s/labels_test.go
@@ -4,7 +4,6 @@
 package k8s
 
 import (
-	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -74,67 +73,3 @@ func TestGetPodMetadata(t *testing.T) {
 		})
 	})
 }
-
-func Test_filterPodLabels(t *testing.T) {
-	expectedLabels := map[string]string{
-		"app":                         "test",
-		"io.kubernetes.pod.namespace": "default",
-	}
-	type args struct {
-		labels map[string]string
-	}
-	tests := []struct {
-		name string
-		args args
-		want map[string]string
-	}{
-		{
-			name: "normal scenario",
-			args: args{
-				labels: map[string]string{
-					"app":                         "test",
-					"io.kubernetes.pod.namespace": "default",
-				},
-			},
-			want: expectedLabels,
-		},
-		{
-			name: "having cilium owned namespace labels",
-			args: args{
-				labels: map[string]string{
-					"app":                         "test",
-					"io.kubernetes.pod.namespace": "default",
-					"io.cilium.k8s.namespace.labels.foo.bar/baz":                 "malicious-pod-level-override",
-					"io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name": "kube-system",
-				},
-			},
-			want: expectedLabels,
-		},
-		{
-			name: "having cilium owned policy labels",
-			args: args{
-				labels: map[string]string{
-					"app":                                 "test",
-					"io.kubernetes.pod.namespace":         "default",
-					"io.cilium.k8s.policy.name":           "admin",
-					"io.cilium.k8s.policy.cluster":        "admin-cluster",
-					"io.cilium.k8s.policy.derived-from":   "admin",
-					"io.cilium.k8s.policy.namespace":      "kube-system",
-					"io.cilium.k8s.policy.serviceaccount": "admin-serviceaccount",
-					"io.cilium.k8s.policy.uuid":           "6eadee3e-0121-11ed-b58d-fc3497a92ef6",
-				},
-			},
-			want: map[string]string{
-				"app":                         "test",
-				"io.kubernetes.pod.namespace": "default",
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := filterPodLabels(tt.args.labels); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("filterPodLabels() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
diff --git a/pkg/k8s/utils/utils.go b/pkg/k8s/utils/utils.go
@@ -211,13 +211,23 @@ type nameLabelsGetter interface {
 	GetLabels() map[string]string
 }
 
-// SanitizePodLabels makes sure that no important pod labels were overridden manually
+// filterPodLabels returns a copy of the given labels map, without the labels owned by Cilium.
+func filterPodLabels(labels map[string]string) map[string]string {
+	res := map[string]string{}
+	for k, v := range labels {
+		if strings.HasPrefix(k, k8sconst.LabelPrefix) {
+			continue
+		}
+		res[k] = v
+	}
+	return res
+}
+
+// SanitizePodLabels makes sure that no important pod labels were overridden manually on k8s pod
+// object creation.
 func SanitizePodLabels(podLabels map[string]string, namespace nameLabelsGetter, serviceAccount, clusterName string) map[string]string {
-	sanitizedLabels := make(map[string]string)
+	sanitizedLabels := filterPodLabels(podLabels)
 
-	for k, v := range podLabels {
-		sanitizedLabels[k] = v
-	}
 	// Sanitize namespace labels
 	for k, v := range namespace.GetLabels() {
 		sanitizedLabels[joinPath(k8sconst.PodNamespaceMetaLabels, k)] = v

diff --git a/pkg/k8s/utils/utils_test.go b/pkg/k8s/utils/utils_test.go
@@ -400,3 +400,58 @@ func TestSanitizePodLabels(t *testing.T) {
 		t.Errorf("Expected service account label to be deleted, got %s instead", sa)
 	}
 }
+
+func Test_filterPodLabels(t *testing.T) {
+	expectedLabels := map[string]string{
+		"app":                         "test",
+		"io.kubernetes.pod.namespace": "default",
+	}
+	tests := []struct {
+		name   string
+		labels map[string]string
+		want   map[string]string
+	}{
+		{
+			name: "normal scenario",
+			labels: map[string]string{
+				"app":                         "test",
+				"io.kubernetes.pod.namespace": "default",
+			},
+			want: expectedLabels,
+		},
+		{
+			name: "having cilium owned namespace labels",
+			labels: map[string]string{
+				"app":                         "test",
+				"io.kubernetes.pod.namespace": "default",
+				"io.cilium.k8s.namespace.labels.foo.bar/baz":                 "malicious-pod-level-override",
+				"io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name": "kube-system",
+			},
+			want: expectedLabels,
+		},
+		{
+			name: "having cilium owned policy labels",
+			labels: map[string]string{
+				"app":                                 "test",
+				"io.kubernetes.pod.namespace":         "default",
+				"io.cilium.k8s.policy.name":           "admin",
+				"io.cilium.k8s.policy.cluster":        "admin-cluster",
+				"io.cilium.k8s.policy.derived-from":   "admin",
+				"io.cilium.k8s.policy.namespace":      "kube-system",
+				"io.cilium.k8s.policy.serviceaccount": "admin-serviceaccount",
+				"io.cilium.k8s.policy.uuid":           "6eadee3e-0121-11ed-b58d-fc3497a92ef6",
+			},
+			want: map[string]string{
+				"app":                         "test",
+				"io.kubernetes.pod.namespace": "default",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := filterPodLabels(tt.labels); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("filterPodLabels() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}