Skip to content

Commit 86e89e4

Browse files
akosyakovakosyakov
committed
fix #12208: in workspace project env var management
1 parent 24979dc commit 86e89e4

File tree

11 files changed

+344
-183
lines changed

11 files changed

+344
-183
lines changed

‎components/gitpod-cli/cmd/env.go

+18-10
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ delete environment variables with a repository pattern of */foo, foo/* or */*.
7878

7979
type connectToServerResult struct {
8080
repositoryPattern string
81+
wsInfo *supervisor.WorkspaceInfoResponse
8182
client *serverapi.APIoverJSONRPC
8283
}
8384

@@ -97,17 +98,20 @@ func connectToServer(ctx context.Context) (*connectToServerResult, error) {
9798
return nil, xerrors.New("repository info is missing owner")
9899
}
99100
if wsinfo.Repository.Name == "" {
100-
return nil, xerrors.New("repository info is missing name")
101+
xerrors.New("repository info is missing name")
101102
}
102103
repositoryPattern := wsinfo.Repository.Owner + "/" + wsinfo.Repository.Name
103104
clientToken, err := supervisor.NewTokenServiceClient(supervisorConn).GetToken(ctx, &supervisor.GetTokenRequest{
104105
Host: wsinfo.GitpodApi.Host,
105106
Kind: "gitpod",
106107
Scope: []string{
107-
"function:getEnvVars",
108+
"function:getWorkspaceEnvVars",
109+
"function:getEnvVars", // TODO remove then getWorkspaceEnvVars is deployed
108110
"function:setEnvVar",
109111
"function:deleteEnvVar",
110112
"resource:envVar::" + repositoryPattern + "::create/get/update/delete",
113+
// TODO actually we should have here team resource scope, but we don't have it, only project id,
114+
// shoud we expose it? shoudl we introduce project guard
111115
},
112116
})
113117
if err != nil {
@@ -121,7 +125,7 @@ func connectToServer(ctx context.Context) (*connectToServerResult, error) {
121125
if err != nil {
122126
return nil, xerrors.Errorf("failed connecting to server: %w", err)
123127
}
124-
return &connectToServerResult{repositoryPattern, client}, nil
128+
return &connectToServerResult{repositoryPattern, wsinfo, client}, nil
125129
}
126130

127131
func getEnvs(ctx context.Context) error {
@@ -131,13 +135,17 @@ func getEnvs(ctx context.Context) error {
131135
}
132136
defer result.client.Close()
133137

134-
vars, err := result.client.GetEnvVars(ctx)
138+
vars, err := result.client.GetWorkspaceEnvVars(ctx, result.wsInfo.WorkspaceId)
139+
if err != nil {
140+
// TODO remove then GetWorkspaceEnvVars is deployed
141+
vars, err = result.client.GetEnvVars(ctx)
142+
}
135143
if err != nil {
136144
return xerrors.Errorf("failed to fetch env vars from server: %w", err)
137145
}
138146

139147
for _, v := range vars {
140-
printVar(v, exportEnvs)
148+
printVar(v.Name, v.Value, exportEnvs)
141149
}
142150

143151
return nil
@@ -163,7 +171,7 @@ func setEnvs(ctx context.Context, args []string) error {
163171
if err != nil {
164172
return err
165173
}
166-
printVar(v, exportEnvs)
174+
printVar(v.Name, v.Value, exportEnvs)
167175
return nil
168176
})
169177
}
@@ -189,12 +197,12 @@ func deleteEnvs(ctx context.Context, args []string) error {
189197
return g.Wait()
190198
}
191199

192-
func printVar(v *serverapi.UserEnvVarValue, export bool) {
193-
val := strings.Replace(v.Value, "\"", "\\\"", -1)
200+
func printVar(name string, value string, export bool) {
201+
val := strings.Replace(value, "\"", "\\\"", -1)
194202
if export {
195-
fmt.Printf("export %s=\"%s\"\n", v.Name, val)
203+
fmt.Printf("export %s=\"%s\"\n", name, val)
196204
} else {
197-
fmt.Printf("%s=%s\n", v.Name, val)
205+
fmt.Printf("%s=%s\n", name, val)
198206
}
199207
}
200208

‎components/gitpod-protocol/go/gitpod-service.go

+31-3
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ type APIInterface interface {
6565
ClosePort(ctx context.Context, workspaceID string, port float32) (err error)
6666
GetUserStorageResource(ctx context.Context, options *GetUserStorageResourceOptions) (res string, err error)
6767
UpdateUserStorageResource(ctx context.Context, options *UpdateUserStorageResourceOptions) (err error)
68-
GetEnvVars(ctx context.Context) (res []*UserEnvVarValue, err error)
68+
GetWorkspaceEnvVars(ctx context.Context, workspaceID string) (res []*EnvVar, err error)
69+
GetEnvVars(ctx context.Context) (res []*EnvVar, err error)
6970
SetEnvVar(ctx context.Context, variable *UserEnvVarValue) (err error)
7071
DeleteEnvVar(ctx context.Context, variable *UserEnvVarValue) (err error)
7172
HasSSHPublicKey(ctx context.Context) (res bool, err error)
@@ -1128,15 +1129,35 @@ func (gp *APIoverJSONRPC) UpdateUserStorageResource(ctx context.Context, options
11281129
return
11291130
}
11301131

1132+
// GetWorkspaceEnvVars calls GetWorkspaceEnvVars on the server
1133+
func (gp *APIoverJSONRPC) GetWorkspaceEnvVars(ctx context.Context, workspaceID string) (res []*EnvVar, err error) {
1134+
if gp == nil {
1135+
err = errNotConnected
1136+
return
1137+
}
1138+
var _params []interface{}
1139+
1140+
_params = append(_params, workspaceID)
1141+
1142+
var result []*EnvVar
1143+
err = gp.C.Call(ctx, "GetWorkspaceEnvVars", _params, &result)
1144+
if err != nil {
1145+
return
1146+
}
1147+
res = result
1148+
1149+
return
1150+
}
1151+
11311152
// GetEnvVars calls getEnvVars on the server
1132-
func (gp *APIoverJSONRPC) GetEnvVars(ctx context.Context) (res []*UserEnvVarValue, err error) {
1153+
func (gp *APIoverJSONRPC) GetEnvVars(ctx context.Context) (res []*EnvVar, err error) {
11331154
if gp == nil {
11341155
err = errNotConnected
11351156
return
11361157
}
11371158
var _params []interface{}
11381159

1139-
var result []*UserEnvVarValue
1160+
var result []*EnvVar
11401161
err = gp.C.Call(ctx, "getEnvVars", _params, &result)
11411162
if err != nil {
11421163
return
@@ -1978,6 +1999,13 @@ type WhitelistedRepository struct {
19781999
URL string `json:"url,omitempty"`
19792000
}
19802001

2002+
// EnvVar is the EnvVar message type
2003+
type EnvVar struct {
2004+
ID string `json:"id,omitempty"`
2005+
Name string `json:"name,omitempty"`
2006+
Value string `json:"value,omitempty"`
2007+
}
2008+
19812009
// UserEnvVarValue is the UserEnvVarValue message type
19822010
type UserEnvVarValue struct {
19832011
ID string `json:"id,omitempty"`

‎components/gitpod-protocol/src/gitpod-service.ts

+4
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
UserSSHPublicKeyValue,
2828
SSHPublicKeyValue,
2929
IDESettings,
30+
EnvVarWithValue,
3031
} from "./protocol";
3132
import {
3233
Team,
@@ -152,6 +153,9 @@ export interface GitpodServer extends JsonRpcServer<GitpodClient>, AdminServer,
152153
getUserStorageResource(options: GitpodServer.GetUserStorageResourceOptions): Promise<string>;
153154
updateUserStorageResource(options: GitpodServer.UpdateUserStorageResourceOptions): Promise<void>;
154155

156+
// Workspace env vars
157+
getWorkspaceEnvVars(workspaceId: string): Promise<EnvVarWithValue[]>;
158+
155159
// User env vars
156160
getEnvVars(): Promise<UserEnvVarValue[]>;
157161
getAllEnvVars(): Promise<UserEnvVarValue[]>;

‎components/gitpod-protocol/src/protocol.ts

+2
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,8 @@ export namespace NamedWorkspaceFeatureFlag {
311311
}
312312
}
313313

314+
export type EnvVar = UserEnvVar | ProjectEnvVarWithValue;
315+
314316
export interface EnvVarWithValue {
315317
name: string;
316318
value: string;

‎components/server/src/auth/rate-limiter.ts

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ const defaultFunctions: FunctionsConfig = {
8989
closePort: { group: "default", points: 1 },
9090
getUserStorageResource: { group: "default", points: 1 },
9191
updateUserStorageResource: { group: "default", points: 1 },
92+
getWorkspaceEnvVars: { group: "default", points: 1 },
9293
getEnvVars: { group: "default", points: 1 },
9394
getAllEnvVars: { group: "default", points: 1 },
9495
setEnvVar: { group: "default", points: 1 },

‎components/server/src/workspace/gitpod-server-impl.ts

+93-9
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ import { IDEOptions } from "@gitpod/gitpod-protocol/lib/ide-protocol";
162162
import { PartialProject } from "@gitpod/gitpod-protocol/lib/teams-projects-protocol";
163163
import { ClientMetadata, traceClientMetadata } from "../websocket/websocket-connection-manager";
164164
import { ConfigurationService } from "../config/configuration-service";
165-
import { ProjectEnvVar } from "@gitpod/gitpod-protocol/lib/protocol";
165+
import { EnvVarWithValue, ProjectEnvVar } from "@gitpod/gitpod-protocol/lib/protocol";
166166
import { InstallationAdminSettings, TelemetryData } from "@gitpod/gitpod-protocol";
167167
import { Deferred } from "@gitpod/gitpod-protocol/lib/util/deferred";
168168
import { InstallationAdminTelemetryDataProvider } from "../installation-admin/telemetry-data-provider";
@@ -200,6 +200,7 @@ import {
200200
import { reportCentralizedPermsValidation } from "../prometheus-metrics";
201201
import { RegionService } from "./region-service";
202202
import { isWorkspaceRegion, WorkspaceRegion } from "@gitpod/gitpod-protocol/lib/workspace-cluster";
203+
import { EnvVar } from "@gitpod/gitpod-protocol/src/protocol";
203204

204205
// shortcut
205206
export const traceWI = (ctx: TraceContext, wi: Omit<LogContext, "userId">) => TraceContext.setOWI(ctx, wi); // userId is already taken care of in WebsocketConnectionManager
@@ -741,8 +742,11 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
741742
if (workspace.deleted) {
742743
throw new ResponseError(ErrorCodes.PERMISSION_DENIED, "Cannot (re-)start a deleted workspace.");
743744
}
744-
const userEnvVars = this.userDB.getEnvVars(user.id);
745-
const projectEnvVarsPromise = this.internalGetProjectEnvVars(workspace.projectId);
745+
const envVarsPromise = this.resolveWorkspaceEnvVars(user, {
746+
repository: CommitContext.is(workspace.context) ? workspace.context.repository : undefined,
747+
projectId: workspace.projectId,
748+
includeCensored: false,
749+
});
746750
const projectPromise = workspace.projectId
747751
? this.projectDB.findProjectById(workspace.projectId)
748752
: Promise.resolve(undefined);
@@ -751,20 +755,66 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
751755

752756
options.region = await this.determineWorkspaceRegion(workspace, options.region || "");
753757

758+
const { workspaceEnvVars, projectEnvVars } = await envVarsPromise;
759+
754760
// at this point we're about to actually start a new workspace
755761
const result = await this.workspaceStarter.startWorkspace(
756762
ctx,
757763
workspace,
758764
user,
759765
await projectPromise,
760-
await userEnvVars,
761-
await projectEnvVarsPromise,
766+
workspaceEnvVars,
767+
projectEnvVars,
762768
options,
763769
);
764770
traceWI(ctx, { instanceId: result.instanceID });
765771
return result;
766772
}
767773

774+
private async resolveWorkspaceEnvVars(
775+
user: User,
776+
options: {
777+
repository:
778+
| {
779+
owner: string;
780+
name: string;
781+
}
782+
| undefined;
783+
projectId: string | undefined;
784+
includeCensored: boolean;
785+
},
786+
): Promise<{
787+
workspaceEnvVars: EnvVar[];
788+
projectEnvVars: ProjectEnvVar[];
789+
}> {
790+
let userEnvVars = await this.userDB.getEnvVars(user.id);
791+
if (options.repository) {
792+
// this is a commit context, thus we can filter the env vars
793+
userEnvVars = UserEnvVar.filter(userEnvVars, options.repository.owner, options.repository.name);
794+
}
795+
const workspaceEnvVars: Map<String, EnvVar> = new Map(userEnvVars.map((e) => [e.name, e]));
796+
797+
const projectEnvVars = await this.internalGetProjectEnvVars(options.projectId);
798+
if (projectEnvVars.length > 0) {
799+
// Instead of using an access guard for Project environment variables, we let Project owners decide whether
800+
// a variable should be:
801+
// - exposed in all workspaces (even for non-Project members when the repository is public), or
802+
// - censored from all workspaces (even for Project members)
803+
const availablePrjEnvVars = projectEnvVars.filter(
804+
(variable) => variable.censored === options.includeCensored,
805+
);
806+
const withValues = await this.projectDB.getProjectEnvironmentVariableValues(availablePrjEnvVars);
807+
for (const e of withValues) {
808+
workspaceEnvVars.set(e.name, e);
809+
}
810+
}
811+
812+
return {
813+
workspaceEnvVars: [...workspaceEnvVars.values()],
814+
projectEnvVars,
815+
};
816+
}
817+
768818
public async stopWorkspace(ctx: TraceContext, workspaceId: string): Promise<void> {
769819
traceAPIParams(ctx, { workspaceId });
770820
traceWI(ctx, { workspaceId });
@@ -1087,7 +1137,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
10871137
const user = this.checkAndBlockUser("createWorkspace", { options });
10881138
await this.checkTermsAcceptance();
10891139

1090-
const envVars = this.userDB.getEnvVars(user.id);
10911140
logContext = { userId: user.id };
10921141

10931142
// Credit check runs in parallel with the other operations up until we start consuming resources.
@@ -1225,18 +1274,24 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
12251274
throw err;
12261275
}
12271276

1228-
let projectEnvVarsPromise = this.internalGetProjectEnvVars(workspace.projectId);
1277+
const envVarsPromise = this.resolveWorkspaceEnvVars(user, {
1278+
repository: CommitContext.is(workspace.context) ? workspace.context.repository : undefined,
1279+
projectId: workspace.projectId,
1280+
includeCensored: workspace.type === "prebuild",
1281+
});
12291282
options.region = await this.determineWorkspaceRegion(workspace, options.region || "");
12301283

1284+
const { workspaceEnvVars, projectEnvVars } = await envVarsPromise;
1285+
12311286
logContext.workspaceId = workspace.id;
12321287
traceWI(ctx, { workspaceId: workspace.id });
12331288
const startWorkspaceResult = await this.workspaceStarter.startWorkspace(
12341289
ctx,
12351290
workspace,
12361291
user,
12371292
project,
1238-
await envVars,
1239-
await projectEnvVarsPromise,
1293+
workspaceEnvVars,
1294+
projectEnvVars,
12401295
options,
12411296
);
12421297
ctx.span?.log({ event: "startWorkspaceComplete", ...startWorkspaceResult });
@@ -1837,7 +1892,36 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
18371892
return [];
18381893
}
18391894

1895+
async getWorkspaceEnvVars(ctx: TraceContext, workspaceId: string): Promise<EnvVarWithValue[]> {
1896+
const user = this.checkUser("getWorkspaceEnvVars");
1897+
const workspace = await this.internalGetWorkspace(workspaceId, this.workspaceDb.trace(ctx));
1898+
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
1899+
1900+
if (workspace.projectId) {
1901+
// TODO should we really leak errors to a user? or just return empty and log a warning? block a user?
1902+
await this.guardProjectOperation(user, workspace.projectId, "get");
1903+
}
1904+
1905+
const result: EnvVarWithValue[] = [];
1906+
const { workspaceEnvVars } = await this.resolveWorkspaceEnvVars(user, {
1907+
repository: undefined, // filter below with access guard
1908+
projectId: workspace.projectId,
1909+
includeCensored: false, // for now workspace cannot get hidden, i.e. may change later if someone needs it for prebuild
1910+
});
1911+
for (const value of workspaceEnvVars) {
1912+
if (
1913+
"repositoryPattern" in value &&
1914+
!(await this.resourceAccessGuard.canAccess({ kind: "envVar", subject: value }, "get"))
1915+
) {
1916+
continue;
1917+
}
1918+
result.push(value);
1919+
}
1920+
return result;
1921+
}
1922+
18401923
// Get environment variables (filter by repository pattern precedence)
1924+
// TODO do we really still need it, gitpod cli was the onyl client?
18411925
async getEnvVars(ctx: TraceContext): Promise<UserEnvVarValue[]> {
18421926
const user = this.checkUser("getEnvVars");
18431927
const result = new Map<string, { value: UserEnvVar; score: number }>();

0 commit comments

Comments
 (0)