From d7164c2a849146a98b715c4dbcff85efdd8a4086 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Fri, 15 Dec 2023 16:54:39 -0700 Subject: [PATCH] Remmove discover of podcidrs: The permissions needed to list nodes and pods in the "kube-system" namespace are too extensive and unneeded by Smee. Also not a good security posture to make these permissions available to Smee. Signed-off-by: Jacob Weinstock --- cmd/smee/backend.go | 84 ------------- cmd/smee/backend_test.go | 251 --------------------------------------- cmd/smee/main.go | 3 - 3 files changed, 338 deletions(-) delete mode 100644 cmd/smee/backend_test.go diff --git a/cmd/smee/backend.go b/cmd/smee/backend.go index 9f674575..cd322112 100644 --- a/cmd/smee/backend.go +++ b/cmd/smee/backend.go @@ -2,14 +2,11 @@ package main import ( "context" - "strings" "github.com/go-logr/logr" "github.com/tinkerbell/dhcp/backend/file" "github.com/tinkerbell/dhcp/backend/kube" "github.com/tinkerbell/dhcp/handler" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -85,84 +82,3 @@ func (s *File) backend(ctx context.Context, logger logr.Logger) (handler.Backend return f, nil } - -// discoverTrustedProxies will use the Kubernetes client to discover the CIDR Ranges for Pods in cluster. -func (k *Kube) discoverTrustedProxies(ctx context.Context, l logr.Logger, trustedProxies []string) []string { - config, err := k.getClient() - if err != nil { - l.Error(err, "failed to get Kubernetes client config") - return nil - } - c, err := corev1client.NewForConfig(config) - if err != nil { - l.Error(err, "failed to create Kubernetes client") - return nil - } - - return combinedCIDRs(ctx, l, c, trustedProxies) -} - -// combinedCIDRs returns the CIDR Ranges for Pods in cluster. Not all Kubernetes distributions provide a way to discover the entire podCIDR. -// Some distributions just provide the podCIDRs assigned to each node. combinedCIDRs tries all known locations where pod CIDRs might exist. -// For example, if a cluster has 3 nodes, each with a /24 podCIDR, and the cluster has a /16 podCIDR, combinedCIDRs will return 4 CIDR ranges. -func combinedCIDRs(ctx context.Context, l logr.Logger, c corev1client.CoreV1Interface, trustedProxies []string) []string { - var tp []string - tp = append(tp, trustedProxies...) - if podCIDRS, err := perNodePodCIDRs(ctx, c); err == nil { - tp = append(tp, podCIDRS...) - } else { - l.V(1).Info("failed to get per node podCIDRs", "err", err) - } - - if clusterCIDR, err := clusterPodCIDR(ctx, c); err == nil { - tp = append(tp, clusterCIDR...) - } else { - l.V(1).Info("failed to get cluster wide podCIDR", "err", err) - } - - return tp -} - -// perNodePodCIDRs returns the CIDR Range for Pods on each node. This is the per node podCIDR as compared to the total podCIDR. -// This will get the podCIDR from each node in the cluster, not the entire cluster podCIDR. If a cluster grows after this is run, -// the new nodes will not be included until this func is run again. -// This should be used in conjunction with ClusterPodCIDR to be as complete and cross distribution compatible as possible. -func perNodePodCIDRs(ctx context.Context, c corev1client.CoreV1Interface) ([]string, error) { - ns, err := c.Nodes().List(ctx, metav1.ListOptions{}) - if err != nil { - return nil, err - } - - var trustedProxies []string - for _, n := range ns.Items { - trustedProxies = append(trustedProxies, n.Spec.PodCIDRs...) - } - - return trustedProxies, nil -} - -// clusterPodCIDR returns the CIDR Range for Pods in cluster. This is the total podCIDR as compared to the per node podCIDR. -// Some Kubernetes distributions do not run a kube-controller-manager pod, so this func should be used in conjunction with PerNodePodCIDRs -// to be as complete and cross distribution compatible as possible. -func clusterPodCIDR(ctx context.Context, c corev1client.CoreV1Interface) ([]string, error) { - // https://kubernetes.io/docs/reference/command-line-tools-reference/kube-controller-manager/ - pods, err := c.Pods("kube-system").List(ctx, metav1.ListOptions{ - LabelSelector: "component=kube-controller-manager", - }) - if err != nil { - return nil, err - } - - var trustedProxies []string - for _, p := range pods.Items { - for _, c := range p.Spec.Containers { - for _, e := range c.Command { - if strings.HasPrefix(e, "--cluster-cidr") { - trustedProxies = append(trustedProxies, strings.Split(e, "=")[1]) - } - } - } - } - - return trustedProxies, nil -} diff --git a/cmd/smee/backend_test.go b/cmd/smee/backend_test.go deleted file mode 100644 index afdc7cac..00000000 --- a/cmd/smee/backend_test.go +++ /dev/null @@ -1,251 +0,0 @@ -package main - -import ( - "context" - "testing" - - "github.com/go-logr/logr" - "github.com/google/go-cmp/cmp" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/kubernetes/fake" -) - -func TestClusterPodCIDR(t *testing.T) { - tests := map[string]struct { - spec []runtime.Object - want []string - }{ - "no podCIDR": {}, - "podCIDR": { - spec: []runtime.Object{ - &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "component": "kube-controller-manager", - }, - Namespace: "kube-system", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Command: []string{ - "kube-controller-manager", - "--allocate-node-cidrs=true", - "--authentication-kubeconfig=/etc/kubernetes/controller-manager.conf", - "--authorization-kubeconfig=/etc/kubernetes/controller-manager.conf", - "--bind-address=127.0.0.1", - "--client-ca-file=/etc/kubernetes/pki/ca.crt", - "--cluster-cidr=10.244.0.0/16", - "--cluster-name=kubernetes", - }, - }, - }, - }, - }, - }, - }, - }, - want: []string{"10.244.0.0/16"}, - }, - } - for name, test := range tests { - t.Run(name, func(t *testing.T) { - c := fake.NewSimpleClientset(test.spec...) - got, err := clusterPodCIDR(context.Background(), c.CoreV1()) - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(got, test.want); diff != "" { - t.Fatalf("unexpected result (+want -got):\n%s", diff) - } - }) - } -} - -func TestPerNodePodCIDRs(t *testing.T) { - tests := map[string]struct { - spec []runtime.Object - want []string - }{ - "no podCIDR": {}, - "podCIDR": { - spec: []runtime.Object{ - &v1.NodeList{ - Items: []v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - }, - Spec: v1.NodeSpec{ - PodCIDRs: []string{"10.42.0.0/24"}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node2", - }, - Spec: v1.NodeSpec{ - PodCIDRs: []string{"10.42.1.0/24"}, - }, - }, - }, - }, - }, - want: []string{"10.42.0.0/24", "10.42.1.0/24"}, - }, - } - for name, test := range tests { - t.Run(name, func(t *testing.T) { - c := fake.NewSimpleClientset(test.spec...) - got, err := perNodePodCIDRs(context.Background(), c.CoreV1()) - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(got, test.want); diff != "" { - t.Fatalf("unexpected result (+want -got):\n%s", diff) - } - }) - } -} - -func TestCombinedCIDRs(t *testing.T) { - tests := map[string]struct { - spec []runtime.Object - want []string - }{ - "no podCIDR": {}, - "podCIDR": { - spec: []runtime.Object{ - &v1.NodeList{ - Items: []v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - }, - Spec: v1.NodeSpec{ - PodCIDRs: []string{"10.42.0.0/24"}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node2", - }, - Spec: v1.NodeSpec{ - PodCIDRs: []string{"10.42.1.0/24"}, - }, - }, - }, - }, - &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "component": "kube-controller-manager", - }, - Namespace: "kube-system", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Command: []string{ - "kube-controller-manager", - "--allocate-node-cidrs=true", - "--authentication-kubeconfig=/etc/kubernetes/controller-manager.conf", - "--authorization-kubeconfig=/etc/kubernetes/controller-manager.conf", - "--bind-address=127.0.0.1", - "--client-ca-file=/etc/kubernetes/pki/ca.crt", - "--cluster-cidr=10.244.0.0/16", - "--cluster-name=kubernetes", - }, - }, - }, - }, - }, - }, - }, - }, - want: []string{"10.42.0.0/24", "10.42.1.0/24", "10.244.0.0/16"}, - }, - } - for name, test := range tests { - t.Run(name, func(t *testing.T) { - c := fake.NewSimpleClientset(test.spec...) - got := combinedCIDRs(context.Background(), logr.Discard(), c.CoreV1(), nil) - if diff := cmp.Diff(got, test.want); diff != "" { - t.Fatalf("unexpected result (+want -got):\n%s", diff) - } - }) - } -} - -func TestDiscoverTrustedProxies(t *testing.T) { - t.Skip("dont think i can mock this") - tests := map[string]struct { - spec []runtime.Object - want []string - }{ - "no podCIDR": {}, - "podCIDR": { - spec: []runtime.Object{ - &v1.NodeList{ - Items: []v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - }, - Spec: v1.NodeSpec{ - PodCIDRs: []string{"10.42.0.0/24"}, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node2", - }, - Spec: v1.NodeSpec{ - PodCIDRs: []string{"10.42.1.0/24"}, - }, - }, - }, - }, - &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "component": "kube-controller-manager", - }, - Namespace: "kube-system", - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Command: []string{ - "kube-controller-manager", - "--allocate-node-cidrs=true", - "--authentication-kubeconfig=/etc/kubernetes/controller-manager.conf", - }, - }, - }, - }, - }, - }, - }, - }, - want: []string{"10.42.1.0/24", "10.42.0.0/24"}, - }, - } - for name, test := range tests { - t.Run(name, func(t *testing.T) { - k := &Kube{} - got := k.discoverTrustedProxies(context.Background(), logr.Discard(), nil) - if diff := cmp.Diff(got, test.want); diff != "" { - t.Fatalf("unexpected result (+want -got):\n%s", diff) - } - }) - } -} diff --git a/cmd/smee/main.go b/cmd/smee/main.go index 571442ae..c54aea3d 100644 --- a/cmd/smee/main.go +++ b/cmd/smee/main.go @@ -209,9 +209,6 @@ func main() { if len(handlers) > 0 { // start the http server for ipxe binaries and scripts tp := parseTrustedProxies(cfg.ipxeHTTPScript.trustedProxies) - if cfg.backends.kubernetes.Enabled { - tp = cfg.backends.kubernetes.discoverTrustedProxies(ctx, log, tp) - } httpServer := &http.Config{ GitRev: GitRev, StartTime: startTime,