[ws-manager-mk2] Protect tokens (#16806)

* [wsman-mk2] Create token secret

* [installer] Fix casing for namespace type metadata

* [installer] Create secrets namespace

* [installer] Configure roles and bindings

* [installer] Move namespace constant to common pkg

* [installer] Create permissions for ws-daemon

* [wsman-mk2] Watch on multiple ns

* [ws-daemon] Use token secret

* [wsman-mk2] Remove secret from initializer

* [wsman-mk2] Test token secret

* [werft] Fix document index

* [installer] Update render tests

* [wsman-mk2] Fix imports

* [wsman-mk2] Ensure maintenance controller required permissions

* [wsman-mk2] Retry deleting secret

* [installer] Ensure objects in secrets namespace are only created with mk2 option

Author: Furisto

.werft/jobs/build/installer/post-process.sh

@@ -52,7 +52,7 @@ MATCHES="$(grep -c -- --- k8s.yaml)"
 # get the read number of K8s manifest docs
 # K8s object names and kinds are duplicated in a config map to faciliate deletion
 # subtract one (the config map) and then divide by 2 to get the actual # of docs we'll loop through
-DOCS="$((((MATCHES - 1) / 2) + 1))"
+DOCS="$(((MATCHES - 1) / 2))"
 documentIndex=0
 
 while [ "$documentIndex" -le "$DOCS" ]; do

components/ws-daemon/pkg/controller/workspace_controller.go

@@ -22,7 +22,9 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 
 	"google.golang.org/protobuf/proto"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -55,9 +57,10 @@ type WorkspaceController struct {
 	maxConcurrentReconciles int
 	operations              *WorkspaceOperations
 	metrics                 *workspaceMetrics
+	secretNamespace         string
 }
 
-func NewWorkspaceController(c client.Client, nodeName string, maxConcurrentReconciles int, ops *WorkspaceOperations, reg prometheus.Registerer) (*WorkspaceController, error) {
+func NewWorkspaceController(c client.Client, nodeName, secretNamespace string, maxConcurrentReconciles int, ops *WorkspaceOperations, reg prometheus.Registerer) (*WorkspaceController, error) {
 	metrics := newWorkspaceMetrics()
 	reg.Register(metrics)
 
@@ -67,6 +70,7 @@ func NewWorkspaceController(c client.Client, nodeName string, maxConcurrentRecon
 		maxConcurrentReconciles: maxConcurrentReconciles,
 		operations:              ops,
 		metrics:                 metrics,
+		secretNamespace:         secretNamespace,
 	}, nil
 }
 
@@ -139,10 +143,8 @@ func (wsc *WorkspaceController) handleWorkspaceInit(ctx context.Context, ws *wor
 	defer tracing.FinishSpan(span, &err)
 
 	if c := wsk8s.GetCondition(ws.Status.Conditions, string(workspacev1.WorkspaceConditionContentReady)); c == nil {
-		var init csapi.WorkspaceInitializer
-		err = proto.Unmarshal(ws.Spec.Initializer, &init)
+		init, err := wsc.prepareInitializer(ctx, ws)
 		if err != nil {
-			err = fmt.Errorf("cannot unmarshal initializer config: %w", err)
 			return ctrl.Result{}, err
 		}
 
@@ -153,7 +155,7 @@ func (wsc *WorkspaceController) handleWorkspaceInit(ctx context.Context, ws *wor
 				WorkspaceId: ws.Spec.Ownership.WorkspaceID,
 				InstanceId:  ws.Name,
 			},
-			Initializer: &init,
+			Initializer: init,
 			Headless:    ws.IsHeadless(),
 		})
 
@@ -300,6 +302,27 @@ func (wsc *WorkspaceController) handleWorkspaceStop(ctx context.Context, ws *wor
 	return ctrl.Result{}, err
 }
 
+func (wsc *WorkspaceController) prepareInitializer(ctx context.Context, ws *workspacev1.Workspace) (*csapi.WorkspaceInitializer, error) {
+	var init csapi.WorkspaceInitializer
+	err := proto.Unmarshal(ws.Spec.Initializer, &init)
+	if err != nil {
+		err = fmt.Errorf("cannot unmarshal initializer config: %w", err)
+		return nil, err
+	}
+
+	var tokenSecret corev1.Secret
+	err = wsc.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s-tokens", ws.Name), Namespace: wsc.secretNamespace}, &tokenSecret)
+	if err != nil {
+		return nil, fmt.Errorf("could not get token secret for workspace: %w", err)
+	}
+
+	if err = csapi.InjectSecretsToInitializer(&init, tokenSecret.Data); err != nil {
+		return nil, fmt.Errorf("failed to inject secrets into initializer: %w", err)
+	}
+
+	return &init, nil
+}
+
 func toWorkspaceGitStatus(status *csapi.GitStatus) *workspacev1.GitStatus {
 	if status == nil {
 		return nil

components/ws-daemon/pkg/daemon/config.go

@@ -42,6 +42,7 @@ type RuntimeConfig struct {
 	Container           *container.Config `json:"containerRuntime"`
 	Kubeconfig          string            `json:"kubeconfig"`
 	KubernetesNamespace string            `json:"namespace"`
+	SecretsNamespace    string            `json:"secretsNamespace"`
 }
 
 type IOLimitConfig struct {

components/ws-daemon/pkg/daemon/daemon.go

@@ -22,6 +22,7 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 
@@ -175,6 +176,7 @@ func NewDaemon(config Config) (*Daemon, error) {
 			Namespace:              config.Runtime.KubernetesNamespace,
 			HealthProbeBindAddress: "0",
 			MetricsBindAddress:     "0", // Metrics are exposed through baseserver.
+			NewCache:               cache.MultiNamespacedCacheBuilder([]string{config.Runtime.KubernetesNamespace, config.Runtime.SecretsNamespace}),
 		})
 		if err != nil {
 			return nil, err
@@ -207,7 +209,8 @@ func NewDaemon(config Config) (*Daemon, error) {
 			return nil, err
 		}
 
-		wsctrl, err := controller.NewWorkspaceController(mgr.GetClient(), nodename, config.WorkspaceController.MaxConcurrentReconciles, workspaceOps, wrappedReg)
+		wsctrl, err := controller.NewWorkspaceController(
+			mgr.GetClient(), nodename, config.Runtime.SecretsNamespace, config.WorkspaceController.MaxConcurrentReconciles, workspaceOps, wrappedReg)
 		if err != nil {
 			return nil, err
 		}

components/ws-manager-api/go/config/config.go

@@ -77,6 +77,8 @@ type ServiceConfiguration struct {
 type Configuration struct {
 	// Namespace is the kubernetes namespace the workspace manager operates in
 	Namespace string `json:"namespace"`
+	// SecretsNamespace is the kubernetes namespace which contains workspace secrets
+	SecretsNamespace string `json:"secretsNamespace"`
 	// SchedulerName is the name of the workspace scheduler all pods are created with
 	SchedulerName string `json:"schedulerName"`
 	// SeccompProfile names the seccomp profile workspaces will use

components/ws-manager-mk2/controllers/maintenance_controller.go

@@ -50,7 +50,6 @@ func (r *MaintenanceReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	log := log.FromContext(ctx).WithValues("configMap", req.NamespacedName)
 
 	if req.Name != configMapName {
-		log.Info("ignoring unexpected ConfigMap")
 		return ctrl.Result{}, nil
 	}
 

components/ws-manager-mk2/controllers/suite_test.go

@@ -24,16 +24,19 @@ import (
 	"github.com/gitpod-io/gitpod/ws-manager-mk2/pkg/activity"
 	"github.com/gitpod-io/gitpod/ws-manager/api/config"
 	workspacev1 "github.com/gitpod-io/gitpod/ws-manager/api/crd/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	//+kubebuilder:scaffold:imports
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
 const (
-	timeout  = time.Second * 20
-	duration = time.Second * 2
-	interval = time.Millisecond * 250
+	timeout          = time.Second * 20
+	duration         = time.Second * 2
+	interval         = time.Millisecond * 250
+	secretsNamespace = "workspace-secrets"
 )
 
 // var cfg *rest.Config
@@ -113,6 +116,7 @@ var _ = BeforeSuite(func() {
 	Expect(timeoutReconciler.SetupWithManager(k8sManager)).To(Succeed())
 
 	ctx, cancel = context.WithCancel(context.Background())
+	_ = createNamespace(secretsNamespace)
 
 	go func() {
 		defer GinkgoRecover()
@@ -127,6 +131,7 @@ func newTestConfig() config.Configuration {
 		GitpodHostURL:     "gitpod.io",
 		HeartbeatInterval: util.Duration(30 * time.Second),
 		Namespace:         "default",
+		SecretsNamespace:  secretsNamespace,
 		SeccompProfile:    "default.json",
 		Timeouts: config.WorkspaceTimeoutConfiguration{
 			AfterClose:          util.Duration(1 * time.Minute),
@@ -156,6 +161,19 @@ func (f *fakeMaintenance) IsEnabled() bool {
 	return f.enabled
 }
 
+func createNamespace(name string) *corev1.Namespace {
+	GinkgoHelper()
+
+	namespace := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+	}
+
+	Expect(k8sClient.Create(ctx, namespace)).To(Succeed())
+	return namespace
+}
+
 var _ = AfterSuite(func() {
 	cancel()
 	By("tearing down the test environment")

components/ws-manager-mk2/controllers/workspace_controller.go

@@ -7,13 +7,15 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/wait"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -195,7 +197,9 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 				}
 			}
 
-			r.deleteWorkspaceSecrets(ctx, workspace)
+			if err := r.deleteWorkspaceSecrets(ctx, workspace); err != nil {
+				return ctrl.Result{RequeueAfter: 10 * time.Second}, err
+			}
 
 			// Workspace might have already been in a deleting state,
 			// but not guaranteed, so try deleting anyway.
@@ -257,7 +261,10 @@ func (r *WorkspaceReconciler) actOnStatus(ctx context.Context, workspace *worksp
 		}
 
 	case workspace.Status.Phase == workspacev1.WorkspacePhaseRunning:
-		r.deleteWorkspaceSecrets(ctx, workspace)
+		err := r.deleteWorkspaceSecrets(ctx, workspace)
+		if err != nil {
+			log.Error(err, "could not delete workspace secrets")
+		}
 
 	// we've disposed already - try to remove the finalizer and call it a day
 	case workspace.Status.Phase == workspacev1.WorkspacePhaseStopped:
@@ -349,37 +356,64 @@ func (r *WorkspaceReconciler) deleteWorkspacePod(ctx context.Context, pod *corev
 	return ctrl.Result{}, nil
 }
 
-func (r *WorkspaceReconciler) deleteWorkspaceSecrets(ctx context.Context, ws *workspacev1.Workspace) {
+func (r *WorkspaceReconciler) deleteWorkspaceSecrets(ctx context.Context, ws *workspacev1.Workspace) error {
 	log := log.FromContext(ctx)
 
 	// if a secret cannot be deleted we do not return early because we want to attempt
 	// the deletion of the remaining secrets
+	var errs []string
 	err := r.deleteSecret(ctx, fmt.Sprintf("%s-%s", ws.Name, "env"), r.Config.Namespace)
 	if err != nil {
+		errs = append(errs, err.Error())
 		log.Error(err, "could not delete environment secret", "workspace", ws.Name)
 	}
-}
-
-func (r *WorkspaceReconciler) deleteSecret(ctx context.Context, name, namespace string) error {
-	var secret corev1.Secret
-	err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, &secret)
-	if errors.IsNotFound(err) {
-		// nothing to delete
-		return nil
-	}
 
+	err = r.deleteSecret(ctx, fmt.Sprintf("%s-%s", ws.Name, "tokens"), r.Config.SecretsNamespace)
 	if err != nil {
-		return fmt.Errorf("could not retrieve secret %s: %w", name, err)
+		errs = append(errs, err.Error())
+		log.Error(err, "could not delete token secret", "workspace", ws.Name)
 	}
 
-	err = r.Client.Delete(ctx, &secret)
-	if err != nil && !errors.IsNotFound(err) {
-		return fmt.Errorf("could not delete secret %s: %w", name, err)
+	if len(errs) != 0 {
+		return fmt.Errorf(strings.Join(errs, ":"))
 	}
 
 	return nil
 }
 
+func (r *WorkspaceReconciler) deleteSecret(ctx context.Context, name, namespace string) error {
+	log := log.FromContext(ctx)
+
+	err := wait.ExponentialBackoffWithContext(ctx, wait.Backoff{
+		Duration: 100 * time.Millisecond,
+		Factor:   1.5,
+		Jitter:   0.2,
+		Steps:    3,
+	}, func() (bool, error) {
+		var secret corev1.Secret
+		err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, &secret)
+		if errors.IsNotFound(err) {
+			// nothing to delete
+			return true, nil
+		}
+
+		if err != nil {
+			log.Error(err, "cannot retrieve secret scheduled for deletion", "secret", name)
+			return false, nil
+		}
+
+		err = r.Client.Delete(ctx, &secret)
+		if err != nil && !errors.IsNotFound(err) {
+			log.Error(err, "cannot delete secret", "secret", name)
+			return false, nil
+		}
+
+		return true, nil
+	})
+
+	return err
+}
+
 var (
 	wsOwnerKey = ".metadata.controller"
 	apiGVStr   = workspacev1.GroupVersion.String()

components/ws-manager-mk2/controllers/workspace_controller_test.go

@@ -33,8 +33,11 @@ var _ = Describe("WorkspaceController", func() {
 	Context("with regular workspaces", func() {
 		It("should handle successful workspace creation and stop request", func() {
 			name := uuid.NewString()
+
+			envSecret := createSecret(fmt.Sprintf("%s-env", name), "default")
+			tokenSecret := createSecret(fmt.Sprintf("%s-tokens", name), secretsNamespace)
+
 			ws := newWorkspace(name, "default")
-			secret := createSecret(fmt.Sprintf("%s-env", name), "default")
 			m := collectMetricCounts(wsMetrics, ws)
 			pod := createWorkspaceExpectPod(ws)
 
@@ -73,7 +76,8 @@ var _ = Describe("WorkspaceController", func() {
 			})
 
 			expectPhaseEventually(ws, workspacev1.WorkspacePhaseRunning)
-			expectSecretCleanup(secret)
+			expectSecretCleanup(envSecret)
+			expectSecretCleanup(tokenSecret)
 
 			markContentReady(ws)
 
@@ -255,7 +259,10 @@ var _ = Describe("WorkspaceController", func() {
 		It("deleting workspace resource should gracefully clean up", func() {
 			name := uuid.NewString()
 			ws := newWorkspace(name, "default")
-			secret := createSecret(fmt.Sprintf("%s-env", name), "default")
+
+			envSecret := createSecret(fmt.Sprintf("%s-env", name), "default")
+			tokenSecret := createSecret(fmt.Sprintf("%s-tokens", name), secretsNamespace)
+
 			m := collectMetricCounts(wsMetrics, ws)
 			pod := createWorkspaceExpectPod(ws)
 
@@ -269,7 +276,8 @@ var _ = Describe("WorkspaceController", func() {
 
 			expectWorkspaceCleanup(ws, pod)
 
-			expectSecretCleanup(secret)
+			expectSecretCleanup(envSecret)
+			expectSecretCleanup(tokenSecret)
 
 			expectMetricsDelta(m, collectMetricCounts(wsMetrics, ws), metricCounts{
 				restores: 1,

components/ws-manager-mk2/main.go

@@ -18,12 +18,9 @@ import (
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/credentials/insecure"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
-	"k8s.io/client-go/rest"
 
 	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
 	"github.com/prometheus/client_golang/prometheus"
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -109,16 +106,7 @@ func main() {
 		HealthProbeBindAddress: cfg.Health.Addr,
 		LeaderElection:         enableLeaderElection,
 		LeaderElectionID:       "ws-manager-mk2-leader.gitpod.io",
-		Namespace:              cfg.Manager.Namespace,
-		NewCache: func(conf *rest.Config, opts cache.Options) (cache.Cache, error) {
-			// Only watch the maintenance mode ConfigMap.
-			opts.SelectorsByObject = cache.SelectorsByObject{
-				&corev1.ConfigMap{}: cache.ObjectSelector{
-					Label: labels.SelectorFromSet(labels.Set{controllers.LabelMaintenance: "true"}),
-				},
-			}
-			return cache.New(conf, opts)
-		},
+		NewCache:               cache.MultiNamespacedCacheBuilder([]string{cfg.Manager.Namespace, cfg.Manager.SecretsNamespace}),
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")