cmd/containerboot,kube,ipn/store/kubestore: allow interactive login on kube, check Secret create perms, allow empty state Secret (#11326)

cmd/containerboot,kube,ipn/store/kubestore: allow interactive login and empty state Secrets, check perms

* Allow users to pre-create empty state Secrets

* Add a fake internal kube client, test functionality that has dependencies on kube client operations.

* Fix an issue where interactive login was not allowed in an edge case where state Secret does not exist

* Make the CheckSecretPermissions method report whether we have permissions to create/patch a Secret if it's determined that these operations will be needed

Updates tailscale/tailscale#11170

Signed-off-by: Irbe Krumina <irbe@tailscale.com>

cmd/containerboot/kube.go

@@ -8,6 +8,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -18,20 +19,6 @@ import (
 	"tailscale.com/tailcfg"
 )
 
-// findKeyInKubeSecret inspects the kube secret secretName for a data
-// field called "authkey", and returns its value if present.
-func findKeyInKubeSecret(ctx context.Context, secretName string) (string, error) {
-	s, err := kc.GetSecret(ctx, secretName)
-	if err != nil {
-		return "", err
-	}
-	ak, ok := s.Data["authkey"]
-	if !ok {
-		return "", nil
-	}
-	return string(ak), nil
-}
-
 // storeDeviceInfo writes deviceID into the "device_id" data field of the kube
 // secret secretName.
 func storeDeviceInfo(ctx context.Context, secretName string, deviceID tailcfg.StableNodeID, fqdn string, addresses []netip.Prefix) error {
@@ -88,9 +75,59 @@ func deleteAuthKey(ctx context.Context, secretName string) error {
 	return nil
 }
 
-var kc *kube.Client
+var kc kube.Client
 
-func initKube(root string) {
+// setupKube is responsible for doing any necessary configuration and checks to
+// ensure that tailscale state storage and authentication mechanism will work on
+// Kubernetes.
+func (cfg *settings) setupKube(ctx context.Context) error {
+	if cfg.KubeSecret == "" {
+		return nil
+	}
+	canPatch, canCreate, err := kc.CheckSecretPermissions(ctx, cfg.KubeSecret)
+	if err != nil {
+		return fmt.Errorf("Some Kubernetes permissions are missing, please check your RBAC configuration: %v", err)
+	}
+	cfg.KubernetesCanPatch = canPatch
+
+	s, err := kc.GetSecret(ctx, cfg.KubeSecret)
+	if err != nil && kube.IsNotFoundErr(err) && !canCreate {
+		return fmt.Errorf("Tailscale state Secret %s does not exist and we don't have permissions to create it. "+
+			"If you intend to store tailscale state elsewhere than a Kubernetes Secret, "+
+			"you can explicitly set TS_KUBE_SECRET env var to an empty string. "+
+			"Else ensure that RBAC is set up that allows the service account associated with this installation to create Secrets.", cfg.KubeSecret)
+	} else if err != nil && !kube.IsNotFoundErr(err) {
+		return fmt.Errorf("Getting Tailscale state Secret %s: %v", cfg.KubeSecret, err)
+	}
+
+	if cfg.AuthKey == "" && !isOneStepConfig(cfg) {
+		if s == nil {
+			log.Print("TS_AUTHKEY not provided and kube secret does not exist, login will be interactive if needed.")
+			return nil
+		}
+		keyBytes, _ := s.Data["authkey"]
+		key := string(keyBytes)
+
+		if key != "" {
+			// This behavior of pulling authkeys from kube secrets was added
+			// at the same time as the patch permission, so we can enforce
+			// that we must be able to patch out the authkey after
+			// authenticating if you want to use this feature. This avoids
+			// us having to deal with the case where we might leave behind
+			// an unnecessary reusable authkey in a secret, like a rake in
+			// the grass.
+			if !cfg.KubernetesCanPatch {
+				return errors.New("authkey found in TS_KUBE_SECRET, but the pod doesn't have patch permissions on the secret to manage the authkey.")
+			}
+			cfg.AuthKey = key
+		} else {
+			log.Print("No authkey found in kube secret and TS_AUTHKEY not provided, login will be interactive if needed.")
+		}
+	}
+	return nil
+}
+
+func initKubeClient(root string) {
 	if root != "/" {
 		// If we are running in a test, we need to set the root path to the fake
 		// service account directory.

cmd/containerboot/kube_test.go

@@ -0,0 +1,206 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build linux
+
+package main
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"tailscale.com/kube"
+)
+
+func TestSetupKube(t *testing.T) {
+	tests := []struct {
+		name    string
+		cfg     *settings
+		wantErr bool
+		wantCfg *settings
+		kc      kube.Client
+	}{
+		{
+			name: "TS_AUTHKEY set, state Secret exists",
+			cfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, false, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return nil, nil
+				},
+			},
+			wantCfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+		},
+		{
+			name: "TS_AUTHKEY set, state Secret does not exist, we have permissions to create it",
+			cfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, true, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return nil, &kube.Status{Code: 404}
+				},
+			},
+			wantCfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+		},
+		{
+			name: "TS_AUTHKEY set, state Secret does not exist, we do not have permissions to create it",
+			cfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, false, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return nil, &kube.Status{Code: 404}
+				},
+			},
+			wantCfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			wantErr: true,
+		},
+		{
+			name: "TS_AUTHKEY set, we encounter a non-404 error when trying to retrieve the state Secret",
+			cfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, false, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return nil, &kube.Status{Code: 403}
+				},
+			},
+			wantCfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			wantErr: true,
+		},
+		{
+			name: "TS_AUTHKEY set, we encounter a non-404 error when trying to check Secret permissions",
+			cfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			wantCfg: &settings{
+				AuthKey:    "foo",
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, false, errors.New("broken")
+				},
+			},
+			wantErr: true,
+		},
+		{
+			// Interactive login using URL in Pod logs
+			name: "TS_AUTHKEY not set, state Secret does not exist, we have permissions to create it",
+			cfg: &settings{
+				KubeSecret: "foo",
+			},
+			wantCfg: &settings{
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, true, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return nil, &kube.Status{Code: 404}
+				},
+			},
+		},
+		{
+			// Interactive login using URL in Pod logs
+			name: "TS_AUTHKEY not set, state Secret exists, but does not contain auth key",
+			cfg: &settings{
+				KubeSecret: "foo",
+			},
+			wantCfg: &settings{
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, false, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return &kube.Secret{}, nil
+				},
+			},
+		},
+		{
+			name: "TS_AUTHKEY not set, state Secret contains auth key, we do not have RBAC to patch it",
+			cfg: &settings{
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return false, false, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return &kube.Secret{Data: map[string][]byte{"authkey": []byte("foo")}}, nil
+				},
+			},
+			wantCfg: &settings{
+				KubeSecret: "foo",
+			},
+			wantErr: true,
+		},
+		{
+			name: "TS_AUTHKEY not set, state Secret contains auth key, we have RBAC to patch it",
+			cfg: &settings{
+				KubeSecret: "foo",
+			},
+			kc: &kube.FakeClient{
+				CheckSecretPermissionsImpl: func(context.Context, string) (bool, bool, error) {
+					return true, false, nil
+				},
+				GetSecretImpl: func(context.Context, string) (*kube.Secret, error) {
+					return &kube.Secret{Data: map[string][]byte{"authkey": []byte("foo")}}, nil
+				},
+			},
+			wantCfg: &settings{
+				KubeSecret:         "foo",
+				AuthKey:            "foo",
+				KubernetesCanPatch: true,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		kc = tt.kc
+		t.Run(tt.name, func(t *testing.T) {
+			if err := tt.cfg.setupKube(context.Background()); (err != nil) != tt.wantErr {
+				t.Errorf("settings.setupKube() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if diff := cmp.Diff(*tt.cfg, *tt.wantCfg); diff != "" {
+				t.Errorf("unexpected contents of settings after running settings.setupKube()\n(-got +want):\n%s", diff)
+			}
+		})
+	}
+}

cmd/containerboot/main.go

@@ -171,44 +171,16 @@ func main() {
 		}
 	}
 
-	if cfg.InKubernetes {
-		initKube(cfg.Root)
-	}
-
 	// Context is used for all setup stuff until we're in steady
 	// state, so that if something is hanging we eventually time out
 	// and crashloop the container.
 	bootCtx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
 	defer cancel()
 
-	if cfg.InKubernetes && cfg.KubeSecret != "" {
-		canPatch, err := kc.CheckSecretPermissions(bootCtx, cfg.KubeSecret)
-		if err != nil {
-			log.Fatalf("Some Kubernetes permissions are missing, please check your RBAC configuration: %v", err)
-		}
-		cfg.KubernetesCanPatch = canPatch
-
-		if cfg.AuthKey == "" && !isOneStepConfig(cfg) {
-			key, err := findKeyInKubeSecret(bootCtx, cfg.KubeSecret)
-			if err != nil {
-				log.Fatalf("Getting authkey from kube secret: %v", err)
-			}
-			if key != "" {
-				// This behavior of pulling authkeys from kube secrets was added
-				// at the same time as the patch permission, so we can enforce
-				// that we must be able to patch out the authkey after
-				// authenticating if you want to use this feature. This avoids
-				// us having to deal with the case where we might leave behind
-				// an unnecessary reusable authkey in a secret, like a rake in
-				// the grass.
-				if !cfg.KubernetesCanPatch {
-					log.Fatalf("authkey found in TS_KUBE_SECRET, but the pod doesn't have patch permissions on the secret to manage the authkey.")
-				}
-				log.Print("Using authkey found in kube secret")
-				cfg.AuthKey = key
-			} else {
-				log.Print("No authkey found in kube secret and TS_AUTHKEY not provided, login will be interactive if needed.")
-			}
+	if cfg.InKubernetes {
+		initKubeClient(cfg.Root)
+		if err := cfg.setupKube(bootCtx); err != nil {
+			log.Fatalf("error setting up for running on Kubernetes: %v", err)
 		}
 	}