From a396c339d4f446af512fc34d509f640c6aac5949 Mon Sep 17 00:00:00 2001 From: Christian Weichel Date: Thu, 7 Jul 2022 09:54:21 +0000 Subject: [PATCH] [ws-manager] Only extract secrets when FF is set --- components/ws-manager/pkg/manager/create.go | 25 --------- .../ws-manager/pkg/manager/create_test.go | 52 ------------------- components/ws-manager/pkg/manager/manager.go | 28 +++++++++- .../ws-manager/pkg/manager/manager_test.go | 48 +++++++++++++++++ 4 files changed, 74 insertions(+), 79 deletions(-) diff --git a/components/ws-manager/pkg/manager/create.go b/components/ws-manager/pkg/manager/create.go index 8427d4109ce1d8..b8c362a056e699 100644 --- 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 index 260692b9ec7e4d..93e2db26ea1986 100644 --- a/components/ws-manager/pkg/manager/create_test.go +++ b/components/ws-manager/pkg/manager/create_test.go @@ -17,10 +17,8 @@ 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 ( @@ -28,56 +26,6 @@ var ( 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 index 78a292dfdce004..3b3afb95fda764 100644 --- 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 index 2eed80ebe8b95e..573ae6ce6f2c26 100644 --- 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"`