Skip to content

Commit 580e7d8

Browse files
geroplgeropl
committed
[db, server] Fix DB queries, mapping to image-build args and fixed tests
1 parent 0e70168 commit 580e7d8

File tree

10 files changed

+81
-135
lines changed

10 files changed

+81
-135
lines changed

‎components/dashboard/src/teams/TeamSettings.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { PlainMessage } from "@bufbuild/protobuf";
3434
import { useToast } from "../components/toasts/Toasts";
3535
import { NamedOrganizationEnvvarItem } from "./variables/NamedOrganizationEnvvarItem";
3636
import { useListOrganizationEnvironmentVariables } from "../data/organizations/org-envvar-queries";
37-
import { UserEnvVar } from "@gitpod/gitpod-protocol";
37+
import { EnvVar } from "@gitpod/gitpod-protocol";
3838

3939
export default function TeamSettingsPage() {
4040
useDocumentTitle("Organization Settings - General");
@@ -50,7 +50,7 @@ export default function TeamSettingsPage() {
5050
const [updated, setUpdated] = useState(false);
5151

5252
const orgEnvVars = useListOrganizationEnvironmentVariables(org?.id || "");
53-
const gitpodImageAuthEnvVar = orgEnvVars.data?.find((v) => v.name === UserEnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME);
53+
const gitpodImageAuthEnvVar = orgEnvVars.data?.find((v) => v.name === EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME);
5454

5555
const updateOrg = useUpdateOrgMutation();
5656

@@ -227,9 +227,9 @@ export default function TeamSettingsPage() {
227227
<Subheading>Configure Docker registry permissions for the whole organization.</Subheading>
228228

229229
<NamedOrganizationEnvvarItem
230-
key={`envvar-named-${UserEnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME}`}
230+
key={`envvar-named-${EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME}`}
231231
disabled={!isOwner}
232-
name={UserEnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME}
232+
name={EnvVar.GITPOD_IMAGE_AUTH_ENV_VAR_NAME}
233233
organizationId={org.id}
234234
variable={gitpodImageAuthEnvVar}
235235
/>

‎components/gitpod-db/src/project-db.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@ export interface ProjectDB extends TransactionalDB<ProjectDB> {
1616
storeProject(project: Project): Promise<Project>;
1717
updateProject(partialProject: PartialProject): Promise<Project>;
1818
markDeleted(projectId: string): Promise<void>;
19-
findProjectEnvironmentVariable(
20-
projectId: string,
21-
envVar: ProjectEnvVarWithValue,
22-
): Promise<ProjectEnvVar | undefined>;
19+
findProjectEnvironmentVariableByName(projectId: string, name: string): Promise<ProjectEnvVar | undefined>;
2320
addProjectEnvironmentVariable(projectId: string, envVar: ProjectEnvVarWithValue): Promise<ProjectEnvVar>;
2421
updateProjectEnvironmentVariable(
2522
projectId: string,
@@ -28,7 +25,9 @@ export interface ProjectDB extends TransactionalDB<ProjectDB> {
2825
getProjectEnvironmentVariables(projectId: string): Promise<ProjectEnvVar[]>;
2926
getProjectEnvironmentVariableById(variableId: string): Promise<ProjectEnvVar | undefined>;
3027
deleteProjectEnvironmentVariable(variableId: string): Promise<void>;
31-
getProjectEnvironmentVariableValues(envVars: ProjectEnvVar[]): Promise<ProjectEnvVarWithValue[]>;
28+
getProjectEnvironmentVariableValues(
29+
envVars: Pick<ProjectEnvVar, "id" | "projectId">[],
30+
): Promise<ProjectEnvVarWithValue[]>;
3231
findCachedProjectOverview(projectId: string): Promise<Project.Overview | undefined>;
3332
storeCachedProjectOverview(projectId: string, overview: Project.Overview): Promise<void>;
3433
getProjectUsage(projectId: string): Promise<ProjectUsage | undefined>;

‎components/gitpod-db/src/team-db.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,6 @@ export interface TeamDB extends TransactionalDB<TeamDB> {
5151
getOrgEnvironmentVariableById(id: string): Promise<OrgEnvVar | undefined>;
5252
findOrgEnvironmentVariableByName(orgId: string, name: string): Promise<OrgEnvVar | undefined>;
5353
getOrgEnvironmentVariables(orgId: string): Promise<OrgEnvVar[]>;
54-
getOrgEnvironmentVariableValues(envVars: OrgEnvVar[]): Promise<OrgEnvVarWithValue[]>;
54+
getOrgEnvironmentVariableValues(envVars: Pick<OrgEnvVar, "id" | "orgId">[]): Promise<OrgEnvVarWithValue[]>;
5555
deleteOrgEnvironmentVariable(id: string): Promise<void>;
5656
}

‎components/gitpod-db/src/typeorm/project-db-impl.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,12 +166,12 @@ export class ProjectDBImpl extends TransactionalDBImpl<ProjectDB> implements Pro
166166
await projectUsageRepo.delete({ projectId });
167167
}
168168

169-
public async findProjectEnvironmentVariable(
169+
public async findProjectEnvironmentVariableByName(
170170
projectId: string,
171-
envVar: ProjectEnvVarWithValue,
171+
name: string,
172172
): Promise<ProjectEnvVar | undefined> {
173173
const envVarRepo = await this.getProjectEnvVarRepo();
174-
return envVarRepo.findOne({ projectId, name: envVar.name, deleted: false });
174+
return envVarRepo.findOne({ projectId, name, deleted: false });
175175
}
176176

177177
public async addProjectEnvironmentVariable(
@@ -237,9 +237,13 @@ export class ProjectDBImpl extends TransactionalDBImpl<ProjectDB> implements Pro
237237
await envVarRepo.delete({ id: variableId });
238238
}
239239

240-
public async getProjectEnvironmentVariableValues(envVars: ProjectEnvVar[]): Promise<ProjectEnvVarWithValue[]> {
240+
public async getProjectEnvironmentVariableValues(
241+
envVars: Pick<ProjectEnvVar, "id" | "projectId">[],
242+
): Promise<ProjectEnvVarWithValue[]> {
241243
const envVarRepo = await this.getProjectEnvVarRepo();
242-
const envVarsWithValues = await envVarRepo.findByIds(envVars);
244+
const envVarsWithValues = await envVarRepo.findByIds(
245+
envVars.map((v) => ({ id: v.id, projectId: v.projectId })),
246+
);
243247
return envVarsWithValues;
244248
}
245249

‎components/gitpod-db/src/typeorm/team-db-impl.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,9 +480,11 @@ export class TeamDBImpl extends TransactionalDBImpl<TeamDB> implements TeamDB {
480480
return envVars;
481481
}
482482

483-
public async getOrgEnvironmentVariableValues(envVars: OrgEnvVar[]): Promise<OrgEnvVarWithValue[]> {
483+
public async getOrgEnvironmentVariableValues(
484+
envVars: Pick<OrgEnvVar, "id" | "orgId">[],
485+
): Promise<OrgEnvVarWithValue[]> {
484486
const envVarRepo = await this.getOrgEnvVarRepo();
485-
const envVarsWithValues = await envVarRepo.findByIds(envVars);
487+
const envVarsWithValues = await envVarRepo.findByIds(envVars.map((v) => ({ id: v.id, orgId: v.orgId })));
486488
return envVarsWithValues;
487489
}
488490

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -270,32 +270,47 @@ export interface UserEnvVar extends UserEnvVarValue {
270270
deleted?: boolean;
271271
}
272272

273+
export namespace EnvVar {
274+
export const GITPOD_IMAGE_AUTH_ENV_VAR_NAME = "GITPOD_IMAGE_AUTH";
275+
/**
276+
* - GITPOD_IMAGE_AUTH is documented https://www.gitpod.io/docs/configure/workspaces/workspace-image#use-a-private-docker-image
277+
*/
278+
export const WhiteListFromReserved = [GITPOD_IMAGE_AUTH_ENV_VAR_NAME];
279+
280+
export function is(data: any): data is EnvVar {
281+
return data.hasOwnProperty("name") && data.hasOwnProperty("value");
282+
}
283+
}
284+
273285
export namespace UserEnvVar {
274286
export const DELIMITER = "/";
275287
export const WILDCARD_ASTERISK = "*";
276288
// This wildcard is only allowed on the last segment, and matches arbitrary segments to the right
277289
export const WILDCARD_DOUBLE_ASTERISK = "**";
278290
const WILDCARD_SHARP = "#"; // TODO(gpl) Where does this come from? Bc we have/had patterns as part of URLs somewhere, maybe...?
279291
const MIN_PATTERN_SEGMENTS = 2;
280-
export const GITPOD_IMAGE_AUTH_ENV_VAR_NAME = "GITPOD_IMAGE_AUTH";
281-
282-
/**
283-
* - GITPOD_IMAGE_AUTH is documented https://www.gitpod.io/docs/configure/workspaces/workspace-image#use-a-private-docker-image
284-
*/
285-
export const WhiteListFromReserved = [GITPOD_IMAGE_AUTH_ENV_VAR_NAME];
286292

287293
function isWildcard(c: string): boolean {
288294
return c === WILDCARD_ASTERISK || c === WILDCARD_SHARP;
289295
}
290296

297+
export function is(data: any): data is UserEnvVar {
298+
return (
299+
EnvVar.is(data) &&
300+
data.hasOwnProperty("id") &&
301+
data.hasOwnProperty("userId") &&
302+
data.hasOwnProperty("repositoryPattern")
303+
);
304+
}
305+
291306
/**
292307
* @param variable
293308
* @returns Either a string containing an error message or undefined.
294309
*/
295310
export function validate(variable: UserEnvVarValue): string | undefined {
296311
const name = variable.name;
297312
const pattern = variable.repositoryPattern;
298-
if (!WhiteListFromReserved.includes(name) && name.startsWith("GITPOD_")) {
313+
if (!EnvVar.WhiteListFromReserved.includes(name) && name.startsWith("GITPOD_")) {
299314
return "Name with prefix 'GITPOD_' is reserved.";
300315
}
301316
if (name.trim() === "") {

‎components/server/src/user/env-var-service.spec.db.ts‎

Lines changed: 27 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -359,13 +359,7 @@ describe("EnvVarService", async () => {
359359
await es.addProjectEnvVar(owner.id, project.id, bazProjectEnvVar);
360360

361361
const envVars = await es.resolveEnvVariables(member.id, project.teamId, undefined, "regular", commitContext);
362-
envVars.workspace.forEach((e) => {
363-
delete (e as any).id;
364-
delete (e as any).userId;
365-
delete (e as any).deleted;
366-
});
367-
expect(envVars.project.length).to.be.equal(0);
368-
expect(envVars.workspace).to.have.deep.members([fooAnyUserEnvVar, barUserCommitEnvVar]);
362+
expectEnvVars(envVars, [fooAnyUserEnvVar, barUserCommitEnvVar]);
369363
});
370364

371365
it("should resolve env variables prebuild", async () => {
@@ -377,10 +371,7 @@ describe("EnvVarService", async () => {
377371
await es.addProjectEnvVar(owner.id, project.id, bazProjectEnvVar);
378372

379373
const envVars = await es.resolveEnvVariables(member.id, project.teamId, undefined, "prebuild", commitContext);
380-
expect(envVars).to.deep.equal({
381-
project: [],
382-
workspace: [],
383-
});
374+
expectEnvVars(envVars, []);
384375
});
385376

386377
it("should resolve env variables regular project", async () => {
@@ -406,26 +397,8 @@ describe("EnvVarService", async () => {
406397
commitContext,
407398
workspaceConfig,
408399
);
409-
envVars.project.forEach((e) => {
410-
delete (e as any).id;
411-
delete (e as any).projectId;
412-
delete (e as any).creationTime;
413-
delete (e as any).deleted;
414-
});
415-
envVars.workspace.forEach((e) => {
416-
delete (e as any).id;
417-
delete (e as any).projectId;
418-
delete (e as any).userId;
419-
delete (e as any).creationTime;
420-
delete (e as any).deleted;
421-
});
422-
expect(envVars.project).to.have.deep.members(
423-
[barProjectCensoredEnvVar, bazProjectEnvVar].map((e) => ({
424-
name: e.name,
425-
censored: e.censored,
426-
})),
427-
);
428-
expect(envVars.workspace).to.have.deep.members([
400+
401+
expectEnvVars(envVars, [
429402
fooAnyUserEnvVar,
430403
barUserCommitEnvVar,
431404
bazProjectEnvVar,
@@ -459,15 +432,7 @@ describe("EnvVarService", async () => {
459432
workspaceConfig,
460433
);
461434

462-
envVars.project = cleanEnvVarShapes(envVars.project);
463-
envVars.workspace = cleanEnvVarShapes(envVars.workspace);
464-
expect(envVars.project).to.have.deep.members(
465-
[barProjectCensoredEnvVar, bazProjectEnvVar].map((e) => ({
466-
name: e.name,
467-
censored: e.censored,
468-
})),
469-
);
470-
expect(envVars.workspace).to.have.deep.members([
435+
expectEnvVars(envVars, [
471436
gitpodImageAuthOrgEnvVar,
472437
fooAnyUserEnvVar,
473438
barUserCommitEnvVar,
@@ -477,17 +442,6 @@ describe("EnvVarService", async () => {
477442
});
478443

479444
it("user should have precedence over org, project over user", async () => {
480-
function expectEnvVars(resolved: ResolvedEnvVars, expected: EnvVarWithValue[]) {
481-
resolved.project = cleanEnvVarShapes(resolved.project, true);
482-
resolved.workspace = cleanEnvVarShapes(resolved.workspace, true);
483-
expect(resolved.workspace).to.have.deep.members(
484-
expected.map((e) => ({
485-
name: e.name,
486-
value: e.value,
487-
})),
488-
);
489-
}
490-
491445
await es.addOrgEnvVar(owner.id, org.id, gitpodImageAuthOrgEnvVar);
492446
let envVars = await es.resolveEnvVariables(member.id, project.teamId, project.id, "regular", commitContext);
493447
expectEnvVars(envVars, [gitpodImageAuthOrgEnvVar]);
@@ -528,15 +482,7 @@ describe("EnvVarService", async () => {
528482
await es.addProjectEnvVar(owner.id, project.id, bazProjectEnvVar);
529483

530484
const envVars = await es.resolveEnvVariables(member.id, project.teamId, project.id, "prebuild", commitContext);
531-
envVars.project = cleanEnvVarShapes(envVars.project);
532-
envVars.workspace = cleanEnvVarShapes(envVars.workspace);
533-
expect(envVars.project).to.have.deep.members(
534-
[barProjectCensoredEnvVar, bazProjectEnvVar].map((e) => ({
535-
name: e.name,
536-
censored: e.censored,
537-
})),
538-
);
539-
expect(envVars.workspace).to.have.deep.members([barProjectCensoredEnvVar, bazProjectEnvVar]);
485+
expectEnvVars(envVars, [barProjectCensoredEnvVar, bazProjectEnvVar]);
540486
});
541487

542488
it("should not match single segment ", async () => {
@@ -551,10 +497,7 @@ describe("EnvVarService", async () => {
551497
await es.addUserEnvVar(member.id, member.id, userEnvVars[0]);
552498

553499
const envVars = await es.resolveEnvVariables(member.id, project.teamId, project.id, "prebuild", commitContext);
554-
expect(envVars).to.deep.equal({
555-
project: [],
556-
workspace: [],
557-
});
500+
expectEnvVars(envVars, []);
558501
});
559502

560503
it("should resolve env variables from context ", async () => {
@@ -569,9 +512,8 @@ describe("EnvVarService", async () => {
569512
...commitContext,
570513
...contextEnvVars,
571514
});
572-
envVars.workspace = cleanEnvVarShapes(envVars.workspace);
573-
expect(envVars.project.length).to.be.equal(0);
574-
expect(envVars.workspace).to.have.deep.members([fooAnyUserEnvVar, barContextEnvVar]);
515+
516+
expectEnvVars(envVars, [fooAnyUserEnvVar, barContextEnvVar]);
575517
});
576518

577519
it("should resolve env variables from context with project ", async () => {
@@ -586,15 +528,8 @@ describe("EnvVarService", async () => {
586528
...commitContext,
587529
...contextEnvVars,
588530
});
589-
envVars.project = cleanEnvVarShapes(envVars.project);
590-
envVars.workspace = cleanEnvVarShapes(envVars.workspace);
591-
expect(envVars.project).to.have.deep.members(
592-
[barProjectCensoredEnvVar, bazProjectEnvVar].map((e) => ({
593-
name: e.name,
594-
censored: e.censored,
595-
})),
596-
);
597-
expect(envVars.workspace).to.have.deep.members([fooAnyUserEnvVar, barContextEnvVar, bazProjectEnvVar]);
531+
532+
expectEnvVars(envVars, [fooAnyUserEnvVar, barContextEnvVar, bazProjectEnvVar]);
598533
});
599534

600535
it("should resolve env variables with precedence", async () => {
@@ -634,11 +569,8 @@ describe("EnvVarService", async () => {
634569
const envVars = await es.resolveEnvVariables(member.id, project.teamId, project.id, "regular", {
635570
...commitContext,
636571
});
637-
envVars.workspace = cleanEnvVarShapes(envVars.workspace);
638-
expect(envVars, `test case: ${i}`).to.deep.equal({
639-
project: [],
640-
workspace: expectedVars,
641-
});
572+
573+
expectEnvVars(envVars, expectedVars, `test case: ${i}`);
642574

643575
for (let j = 0; j < inputVars.length; j++) {
644576
await es.deleteUserEnvVar(member.id, member.id, inputVars[j]);
@@ -701,24 +633,22 @@ describe("EnvVarService", async () => {
701633
const envVars = await es.resolveEnvVariables(member.id, project.teamId, project.id, "regular", {
702634
...gitlabSubgroupCommitContext,
703635
});
704-
envVars.workspace = cleanEnvVarShapes(envVars.workspace);
705-
expect(envVars.project.length).to.be.equal(0);
706-
expect(envVars.workspace).to.have.deep.members(userEnvVars.filter((ev) => ev.value === "true"));
636+
expectEnvVars(
637+
envVars,
638+
userEnvVars.filter((ev) => ev.value === "true"),
639+
);
707640
});
708641
});
709642

710-
function cleanEnvVarShapes<T extends object>(envVars: T[], dropAllProperties: boolean = false): T[] {
711-
return envVars.map((ev) => {
712-
delete (ev as any).id;
713-
delete (ev as any).orgId;
714-
delete (ev as any).projectId;
715-
delete (ev as any).userId;
716-
delete (ev as any).creationTime;
717-
delete (ev as any).deleted;
718-
if (dropAllProperties) {
719-
delete (ev as any).repositoryPattern;
720-
delete (ev as any).censored;
721-
}
722-
return ev;
643+
function envVars(evs: EnvVarWithValue[]): Pick<EnvVarWithValue, "name" | "value">[] {
644+
return evs.map((ev) => {
645+
return {
646+
name: ev.name,
647+
value: ev.value,
648+
};
723649
});
724650
}
651+
652+
function expectEnvVars(resolved: ResolvedEnvVars, expected: EnvVarWithValue[], message?: string) {
653+
expect(envVars(resolved.workspace), message).to.have.deep.members(envVars(expected));
654+
}

0 commit comments

Comments
 (0)