Skip to content

[server] don't error on project not_found #18400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 1, 2023
Merged

[server] don't error on project not_found #18400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions components/gitpod-protocol/src/messaging/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ export namespace ApplicationError {
export function hasErrorCode(e: any): e is Error & { code: ErrorCode; data?: any } {
return e && e.code !== undefined;
}

export async function notFoundToUndefined<T>(p: Promise<T>): Promise<T | undefined> {
try {
return await p;
} catch (e) {
if (hasErrorCode(e) && e.code === ErrorCodes.NOT_FOUND) {
return undefined;
}
throw e;
}
}
}

export namespace ErrorCode {
Expand Down
6 changes: 5 additions & 1 deletion components/server/src/workspace/env-var-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from "@gitpod/gitpod-protocol";
import { inject, injectable } from "inversify";
import { ProjectsService } from "../projects/projects-service";
import { ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";

export interface ResolvedEnvVars {
// all project env vars, censored included always
Expand Down Expand Up @@ -43,8 +44,11 @@ export class EnvVarService {
};

const projectEnvVars = workspace.projectId
? await this.projectsService.getProjectEnvironmentVariables(workspace.ownerId, workspace.projectId)
? (await ApplicationError.notFoundToUndefined(
this.projectsService.getProjectEnvironmentVariables(workspace.ownerId, workspace.projectId),
)) || []
: [];

if (workspace.type === "prebuild") {
// prebuild does not have access to user env vars and cannot be started via prewfix URL
const withValues = await this.projectDB.getProjectEnvironmentVariableValues(projectEnvVars);
Expand Down
73 changes: 29 additions & 44 deletions components/server/src/workspace/gitpod-server-impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,17 +844,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
await this.userDeletionService.deleteUser(user.id);
}

private async getTeamMembersByProject(projectId: string | undefined): Promise<TeamMemberInfo[]> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was unnecessariuly going through project. We now have an organizationId on workspaces so I replaced these with organizationService.listMembers.

const user = await this.checkUser("getTeamMembersByProject");
if (projectId) {
const project = await this.projectsService.getProject(user.id, projectId);
if (project && project.teamId) {
return await this.organizationService.listMembers(user.id, project.teamId);
}
}
return [];
}

public async getWorkspace(ctx: TraceContext, workspaceId: string): Promise<WorkspaceInfo> {
traceAPIParams(ctx, { workspaceId });
traceWI(ctx, { workspaceId });
Expand All @@ -863,8 +852,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

const workspace = await this.internalGetWorkspace(user, workspaceId, this.workspaceDb.trace(ctx));
const latestInstancePromise = this.workspaceDb.trace(ctx).findCurrentInstance(workspaceId);
const teamMembers = await this.getTeamMembersByProject(workspace.projectId);
await this.guardAccess({ kind: "workspace", subject: workspace, teamMembers }, "get");
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
await this.guardAccess({ kind: "workspace", subject: workspace, teamMembers: teamMembers }, "get");
const latestInstance = await latestInstancePromise;
if (!!latestInstance) {
await this.guardAccess(
Expand Down Expand Up @@ -978,7 +967,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
}
const envVarsPromise = this.envVarService.resolve(workspace);
const projectPromise = workspace.projectId
? this.projectsService.getProject(user.id, workspace.projectId)
? ApplicationError.notFoundToUndefined(this.projectsService.getProject(user.id, workspace.projectId))
: Promise.resolve(undefined);

await mayStartPromise;
Expand Down Expand Up @@ -1008,15 +997,15 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
const workspace = await this.internalGetWorkspace(user, workspaceId, this.workspaceDb.trace(ctx));
if (workspace.type === "prebuild") {
// If this is a team prebuild, any team member can stop it.
const teamMembers = await this.getTeamMembersByProject(workspace.projectId);
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
await this.guardAccess({ kind: "workspace", subject: workspace, teamMembers }, "get");
} else {
// If this is not a prebuild, or it's a personal prebuild, only the workspace owner can stop it.
await this.guardAccess({ kind: "workspace", subject: workspace }, "get");
}

try {
await this.internalStopWorkspace(ctx, workspace, "stopped via API");
await this.internalStopWorkspace(ctx, user.id, workspace, "stopped via API");
} catch (err) {
log.error(logCtx, "stopWorkspace error: ", err);
if (isClusterMaintenanceError(err)) {
Expand All @@ -1031,6 +1020,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

private async internalStopWorkspace(
ctx: TraceContext,
requestorId: string,
workspace: Workspace,
reason: string,
policy?: StopWorkspacePolicy,
Expand All @@ -1049,7 +1039,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
if (!admin) {
if (workspace.type === "prebuild") {
// If this is a team prebuild, any team member can stop it.
const teamMembers = await this.getTeamMembersByProject(workspace.projectId);
const teamMembers = await this.organizationService.listMembers(requestorId, workspace.organizationId);
await this.guardAccess(
{ kind: "workspaceInstance", subject: instance, workspace, teamMembers },
"update",
Expand Down Expand Up @@ -1113,7 +1103,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
await this.guardAccess({ kind: "workspace", subject: ws }, "delete");

// for good measure, try and stop running instances
await this.internalStopWorkspace(ctx, ws, "deleted via API");
await this.internalStopWorkspace(ctx, user.id, ws, "deleted via API");

// actually delete the workspace
await this.workspaceDeletionService.softDeleteWorkspace(ctx, ws, "user");
Expand Down Expand Up @@ -1593,9 +1583,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
const user = await this.checkAndBlockUser("getPrebuildEvents");

const project = await this.projectsService.getProject(user.id, projectId);
if (!project) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed these unnecessary checks, as getProject already throws that error

throw new ApplicationError(ErrorCodes.NOT_FOUND, "Project not found");
}
await this.guardProjectOperation(user, projectId, "get");

const events = await this.projectsService.getPrebuildEvents(user.id, project.cloneUrl);
Expand All @@ -1612,9 +1599,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
const user = await this.checkAndBlockUser("triggerPrebuild");

const project = await this.projectsService.getProject(user.id, projectId);
if (!project) {
throw new ApplicationError(ErrorCodes.NOT_FOUND, "Project not found");
}
await this.guardProjectOperation(user, projectId, "update");

const branchDetails = !!branchName
Expand Down Expand Up @@ -2120,7 +2104,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
return;
}
traceWI(ctx, { instanceId: instance.id });
const teamMembers = await this.getTeamMembersByProject(workspace.projectId);
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
await this.guardAccess({ kind: "workspaceLog", subject: workspace, teamMembers }, "get");

// wait for up to 20s for imageBuildLogInfo to appear due to:
Expand Down Expand Up @@ -2197,7 +2181,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
async getHeadlessLog(ctx: TraceContext, instanceId: string): Promise<HeadlessLogUrls> {
traceAPIParams(ctx, { instanceId });

await this.checkAndBlockUser("getHeadlessLog", { instanceId });
const user = await this.checkAndBlockUser("getHeadlessLog", { instanceId });
const logCtx: LogContext = { instanceId };

const ws = await this.workspaceDb.trace(ctx).findByInstanceId(instanceId);
Expand All @@ -2206,7 +2190,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
}

const wsiPromise = this.workspaceDb.trace(ctx).findInstanceById(instanceId);
const teamMembers = await this.getTeamMembersByProject(ws.projectId);
const teamMembers = await this.organizationService.listMembers(user.id, ws.organizationId);

await this.guardAccess({ kind: "workspaceLog", subject: ws, teamMembers }, "get");

Expand Down Expand Up @@ -2810,9 +2794,6 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

private async guardProjectOperation(user: User, projectId: string, op: ResourceAccessOp): Promise<void> {
const project = await this.projectsService.getProject(user.id, projectId);
if (!project) {
throw new ApplicationError(ErrorCodes.NOT_FOUND, "Project not found");
}
// Anyone who can read a team's information (i.e. any team member) can manage team projects
await this.guardTeamOperation(project.teamId, "get", "not_implemented");
}
Expand Down Expand Up @@ -2870,7 +2851,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

public async getPrebuild(ctx: TraceContext, prebuildId: string): Promise<PrebuildWithStatus | undefined> {
traceAPIParams(ctx, { prebuildId });
await this.checkAndBlockUser("getPrebuild");
const user = await this.checkAndBlockUser("getPrebuild");

const pbws = await this.workspaceDb.trace(ctx).findPrebuiltWorkspaceById(prebuildId);
if (!pbws) {
Expand All @@ -2887,9 +2868,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
return undefined;
}

// TODO(gpl) Ideally, we should not need to query the project-team hierarchy here, but decide on a per-prebuild basis.
// For that we need to fix Prebuild-access semantics, which is out-of-scope for now.
const teamMembers = await this.getTeamMembersByProject(workspace.projectId);
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
await this.guardAccess({ kind: "prebuild", subject: pbws, workspace, teamMembers }, "get");
const result: PrebuildWithStatus = { info, status: pbws.state };
if (pbws.error) {
Expand All @@ -2903,7 +2882,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
workspaceId: string,
): Promise<PrebuiltWorkspace | undefined> {
traceAPIParams(ctx, { workspaceId });
await this.checkAndBlockUser("findPrebuildByWorkspaceID");
const user = await this.checkAndBlockUser("findPrebuildByWorkspaceID");

const [pbws, workspace] = await Promise.all([
this.workspaceDb.trace(ctx).findPrebuildByWorkspaceID(workspaceId),
Expand All @@ -2913,9 +2892,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
return undefined;
}

// TODO(gpl) Ideally, we should not need to query the project-team hierarchy here, but decide on a per-prebuild basis.
// For that we need to fix Prebuild-access semantics, which is out-of-scope for now.
const teamMembers = await this.getTeamMembersByProject(workspace.projectId);
const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
await this.guardAccess({ kind: "prebuild", subject: pbws, workspace, teamMembers }, "get");
return pbws;
}
Expand Down Expand Up @@ -2954,10 +2931,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {

const user = await this.checkAndBlockUser("cancelPrebuild");

const project = await this.projectsService.getProject(user.id, projectId);
if (!project) {
throw new ApplicationError(ErrorCodes.NOT_FOUND, "Project not found");
}
await this.projectsService.getProject(user.id, projectId);
await this.guardProjectOperation(user, projectId, "update");

const prebuild = await this.workspaceDb.trace(ctx).findPrebuildByID(prebuildId);
Expand Down Expand Up @@ -3299,11 +3273,22 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
async adminForceStopWorkspace(ctx: TraceContext, workspaceId: string): Promise<void> {
traceAPIParams(ctx, { workspaceId });

await this.guardAdminAccess("adminForceStopWorkspace", { id: workspaceId }, Permission.ADMIN_WORKSPACES);
const admin = await this.guardAdminAccess(
"adminForceStopWorkspace",
{ id: workspaceId },
Permission.ADMIN_WORKSPACES,
);

const workspace = await this.workspaceDb.trace(ctx).findById(workspaceId);
if (workspace) {
await this.internalStopWorkspace(ctx, workspace, "stopped by admin", StopWorkspacePolicy.IMMEDIATELY, true);
await this.internalStopWorkspace(
ctx,
admin.id,
workspace,
"stopped by admin",
StopWorkspacePolicy.IMMEDIATELY,
true,
);
}
}

Expand Down
5 changes: 4 additions & 1 deletion components/server/src/workspace/headless-log-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { BearerAuth } from "../auth/bearer-authenticator";
import { ProjectsService } from "../projects/projects-service";
import { HostContextProvider } from "../auth/host-context-provider";
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
import { ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";

export const HEADLESS_LOGS_PATH_PREFIX = "/headless-logs";
export const HEADLESS_LOG_DOWNLOAD_PATH_PREFIX = "/headless-log-download";
Expand Down Expand Up @@ -214,7 +215,9 @@ export class HeadlessLogController {

let teamMembers: TeamMemberInfo[] = [];
if (workspace?.projectId) {
const p = await this.projectService.getProject(user.id, workspace.projectId);
const p = await ApplicationError.notFoundToUndefined(
this.projectService.getProject(user.id, workspace.projectId),
);
if (p?.teamId) {
teamMembers = await this.teamDb.findMembersByTeam(p.teamId);
}
Expand Down
5 changes: 1 addition & 4 deletions components/server/src/workspace/workspace-starter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,10 +696,7 @@ export class WorkspaceStarter {
metadata.setMetaId(workspace.id);
if (workspace.projectId) {
metadata.setProject(workspace.projectId);
const project = await this.projectDB.findProjectById(workspace.projectId);
if (project && project.teamId) {
metadata.setTeam(project.teamId);
}
metadata.setTeam(workspace.organizationId);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to fetch the project as we have the org id on the workspace

}

return metadata;
Expand Down