[ws-manager] Only extract secrets when FF is set

gitpod-io · Jul 7, 2022 · a396c33 · a396c33
1 parent e334d9e
commit a396c33
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 79 deletions.
diff --git a/components/ws-manager/pkg/manager/create.go b/components/ws-manager/pkg/manager/create.go
@@ -969,30 +969,6 @@ func (m *Manager) newStartWorkspaceContext(ctx context.Context, req *api.StartWo
 		}
 	}
 
-	var (
-		secrets    = make(map[string]string)
-		secretsLen int
-	)
-	for _, env := range req.Spec.Envvars {
-		if env.Secret != nil {
-			continue
-		}
-		if !isProtectedEnvVar(env.Name) {
-			continue
-		}
-
-		name := fmt.Sprintf("%x", sha256.Sum256([]byte(env.Name)))
-		secrets[name] = env.Value
-		secretsLen += len(env.Value)
-	}
-	for k, v := range csapi.ExtractSecretsFromInitializer(req.Spec.Initializer) {
-		secrets[k] = v
-		secretsLen += len(v)
-	}
-	if secretsLen > maxSecretsLength {
-		return nil, xerrors.Errorf("secrets exceed maximum permitted length (%d > %d bytes): please reduce the numer or length of environment variables", secretsLen, maxSecretsLength)
-	}
-
 	return &startWorkspaceContext{
 		Labels:         labels,
 		CLIAPIKey:      cliAPIKey,
@@ -1004,7 +980,6 @@ func (m *Manager) newStartWorkspaceContext(ctx context.Context, req *api.StartWo
 		Headless:       headless,
 		Class:          class,
 		VolumeSnapshot: volumeSnapshot,
-		Secrets:        secrets,
 	}, nil
 }
 

diff --git a/components/ws-manager/pkg/manager/create_test.go b/components/ws-manager/pkg/manager/create_test.go
@@ -17,67 +17,15 @@ import (
 	"sigs.k8s.io/yaml"
 
 	ctesting "github.com/gitpod-io/gitpod/common-go/testing"
-	csapi "github.com/gitpod-io/gitpod/content-service/api"
 	"github.com/gitpod-io/gitpod/ws-manager/api"
 	config "github.com/gitpod-io/gitpod/ws-manager/api/config"
-	"github.com/google/go-cmp/cmp"
 )
 
 var (
 	team    = "awesome"
 	project = "gitpod"
 )
 
-func TestNewStartWorkspaceContext(t *testing.T) {
-	type Expectation struct {
-		Context *startWorkspaceContext
-		Error   string
-	}
-	tests := []struct {
-		Name        string
-		Req         *api.StartWorkspaceRequest
-		Expectation Expectation
-	}{
-		{
-			Name: "oversized secrets",
-			Req: &api.StartWorkspaceRequest{
-				Metadata: &api.WorkspaceMetadata{Owner: "foo"},
-				Spec: &api.StartWorkspaceSpec{
-					Initializer: &csapi.WorkspaceInitializer{
-						Spec: &csapi.WorkspaceInitializer_Empty{},
-					},
-					Envvars: []*api.EnvironmentVariable{
-						{Name: "too_large", Value: string(func() []byte { return make([]byte, 2*maxSecretsLength) }())},
-					},
-				},
-			},
-			Expectation: Expectation{
-				Error: "secrets exceed maximum permitted length (1610612736 > 805306368 bytes): please reduce the numer or length of environment variables",
-			},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.Name, func(t *testing.T) {
-			mgmtCfg := forTestingOnlyManagerConfig()
-			manager := &Manager{Config: mgmtCfg}
-
-			sctx, err := manager.newStartWorkspaceContext(context.Background(), test.Req)
-
-			act := Expectation{
-				Context: sctx,
-			}
-			if err != nil {
-				act.Error = err.Error()
-			}
-
-			if diff := cmp.Diff(test.Expectation, act); diff != "" {
-				t.Errorf("unexpected newStartWorkspaceContext (-want +got):\n%s", diff)
-			}
-		})
-	}
-}
-
 func TestCreateDefiniteWorkspacePod(t *testing.T) {
 	type WorkspaceClass struct {
 		DefaultTemplate    *corev1.Pod                   `json:"defaultTemplate,omitempty"`

diff --git a/components/ws-manager/pkg/manager/manager.go b/components/ws-manager/pkg/manager/manager.go
@@ -6,6 +6,7 @@ package manager
 
 import (
 	"context"
+	"crypto/sha256"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -39,6 +40,7 @@ import (
 	wsk8s "github.com/gitpod-io/gitpod/common-go/kubernetes"
 	"github.com/gitpod-io/gitpod/common-go/log"
 	"github.com/gitpod-io/gitpod/common-go/tracing"
+	csapi "github.com/gitpod-io/gitpod/content-service/api"
 	"github.com/gitpod-io/gitpod/content-service/pkg/layer"
 	regapi "github.com/gitpod-io/gitpod/registry-facade/api"
 	wsdaemon "github.com/gitpod-io/gitpod/ws-daemon/api"
@@ -82,7 +84,6 @@ type startWorkspaceContext struct {
 	Headless       bool                           `json:"headless"`
 	Class          *config.WorkspaceClass         `json:"class"`
 	VolumeSnapshot *workspaceVolumeSnapshotStatus `json:"volumeSnapshot"`
-	Secrets        map[string]string              `json:"secrets"`
 }
 
 func (swctx *startWorkspaceContext) ContainerConfiguration() config.ContainerConfiguration {
@@ -267,13 +268,36 @@ func (m *Manager) StartWorkspace(ctx context.Context, req *api.StartWorkspaceReq
 		}
 	}
 	if createSecret {
+		var (
+			secrets    = make(map[string]string)
+			secretsLen int
+		)
+		for _, env := range req.Spec.Envvars {
+			if env.Secret != nil {
+				continue
+			}
+			if !isProtectedEnvVar(env.Name) {
+				continue
+			}
+
+			name := fmt.Sprintf("%x", sha256.Sum256([]byte(env.Name)))
+			secrets[name] = env.Value
+			secretsLen += len(env.Value)
+		}
+		for k, v := range csapi.ExtractSecretsFromInitializer(req.Spec.Initializer) {
+			secrets[k] = v
+			secretsLen += len(v)
+		}
+		if secretsLen > maxSecretsLength {
+			return nil, xerrors.Errorf("secrets exceed maximum permitted length (%d > %d bytes): please reduce the numer or length of environment variables", secretsLen, maxSecretsLength)
+		}
 		secret := &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      podName(startContext.Request),
 				Namespace: m.Config.Namespace,
 				Labels:    startContext.Labels,
 			},
-			StringData: startContext.Secrets,
+			StringData: secrets,
 		}
 		err = m.Clientset.Create(ctx, secret)
 		if err != nil && !k8serr.IsAlreadyExists(err) {

diff --git a/components/ws-manager/pkg/manager/manager_test.go b/components/ws-manager/pkg/manager/manager_test.go
@@ -11,8 +11,10 @@ import (
 	"time"
 
 	ctesting "github.com/gitpod-io/gitpod/common-go/testing"
+	csapi "github.com/gitpod-io/gitpod/content-service/api"
 	"github.com/gitpod-io/gitpod/ws-manager/api"
 	"github.com/gitpod-io/gitpod/ws-manager/pkg/manager/internal/grpcpool"
+	"github.com/google/go-cmp/cmp"
 	"google.golang.org/grpc"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -21,6 +23,52 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+func TestStartWorkspace(t *testing.T) {
+	type Expectation struct {
+		Response *api.StartWorkspaceResponse
+		Error    string
+	}
+	tests := []struct {
+		Name        string
+		Req         *api.StartWorkspaceRequest
+		Expectation Expectation
+	}{
+		{
+			Name: "oversized secrets",
+			Req: &api.StartWorkspaceRequest{
+				Metadata: &api.WorkspaceMetadata{Owner: "foo"},
+				Spec: &api.StartWorkspaceSpec{
+					Initializer: &csapi.WorkspaceInitializer{
+						Spec: &csapi.WorkspaceInitializer_Empty{},
+					},
+					Envvars: []*api.EnvironmentVariable{
+						{Name: "too_large", Value: string(func() []byte { return make([]byte, 2*maxSecretsLength) }())},
+					},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			mgmtCfg := forTestingOnlyManagerConfig()
+			manager := &Manager{Config: mgmtCfg}
+
+			resp, err := manager.StartWorkspace(context.Background(), test.Req)
+
+			act := Expectation{
+				Response: resp,
+			}
+			if err != nil {
+				act.Error = err.Error()
+			}
+
+			if diff := cmp.Diff(test.Expectation, act); diff != "" {
+				t.Errorf("unexpected StartWorkspace (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestValidateStartWorkspaceRequest(t *testing.T) {
 	type fixture struct {
 		Req *api.StartWorkspaceRequest `json:"request"`