Allow multiple projects per repo #18774

svenefftinge · 2023-09-22T10:35:20Z

Description

This PR includes implementation changes that are necessary to relax the 1:1 constraint between a repo and a project.

Summary generated by Copilot

`🤖 Generated by Copilot at cc1c7cf`

No summary available (Limit exceeded: required to process 212218 tokens, but only 50000 are allowed per call)

Related Issue(s)

Fixes EXP-640, EXP-649, EXP-648, EXP-653

How to test

Join this org: https://se-multiple-projects.preview.gitpod-dev.com/orgs/join?inviteId=58fb7dc4-2d43-41e0-a7e8-be1a43e7e9ab

It is set up with all SCMs we support

try create multiple projects for the same repo with different prebuild settings.
try start workspaces on them

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

🏷️ Name - se-multiple-projects
🔗 URL - se-multiple-projects.preview.gitpod-dev.com/workspaces.
📚 Documentation - See our internal documentation for information on how to interact with your preview environment.
📦 Version - se-multiple-projects-gha.17481
🗒️ Logs - GCP Logs Explorer

Build Options

Build

/werft with-werft
Run the build with werft instead of GHA
leeway-no-cache
/werft no-test
Run Leeway with --dont-test

Publish

/werft publish-to-npm
/werft publish-to-jb-marketplace

Installer

analytics=segment
with-dedicated-emulation
workspace-feature-flags
Add desired feature flags to the end of the line above, space separated

Preview Environment / Integration Tests

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-gce-vm
If enabled this will create the environment on GCE infra
with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh. If enabled, with-preview and with-large-vm will be enabled.
with-monitoring

/hold

svenefftinge · 2023-09-22T11:11:27Z

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

@@ -10,8 +10,7 @@ import { TransactionalDB } from "./typeorm/transactional-db-impl";
 export const ProjectDB = Symbol("ProjectDB");
 export interface ProjectDB extends TransactionalDB<ProjectDB> {
    findProjectById(projectId: string): Promise<Project | undefined>;
-    findProjectByCloneUrl(cloneUrl: string): Promise<Project | undefined>;
-    findProjectsByCloneUrls(cloneUrls: string[]): Promise<(Project & { teamOwners?: string[] })[]>;
+    findProjectsByCloneUrl(cloneUrl: string): Promise<Project[]>;


we still need a method like this because we need to look up the project(s) when a webhook event comes in.

svenefftinge · 2023-09-22T11:12:56Z

components/gitpod-db/src/typeorm/workspace-db-impl.ts

@@ -710,16 +711,19 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl<WorkspaceDB> imp

    // Find the (last triggered) prebuild for a given commit
    public async findPrebuiltWorkspaceByCommit(
-        cloneURL: string,
+        projectId: string,


A cloneURL was used across the board to identify the project. Since we now can have multiple projects per cloneURL this is no longer the case, so we pass on the projected instead. This is the majority of this changeset.

svenefftinge · 2023-09-22T11:13:18Z

components/gitpod-db/src/typeorm/workspace-db-impl.ts

-        if (!commit || !cloneURL) {
-            return undefined;
+        if (!commit || !projectId) {
+            throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "Illegal arguments", { projectId, commit });


we should not silently ignore such a state

Agree, but how comes that this is necessary in the persistence layer given the mandatory arguments. Let's check the call sites, too.

gtsiolis · 2023-09-22T11:17:39Z

svenefftinge · 2023-09-22T11:16:46Z

components/dashboard/src/projects/new-project/NewProjectRepoList.tsx

-                                    )}
+                                    <Button onClick={() => onRepoSelected(r)} loading={isCreating}>
+                                        Select
+                                    </Button>


we no longer forbid creating a project if one for that repo already exists

components/server/src/prebuilds/bitbucket-app.ts

svenefftinge · 2023-09-22T11:28:16Z

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

+                    user.id,
+                    context.repository.cloneUrl,
+                );
+                if (projects.length > 1) {


if the user didn't pass in a projectId but we find more than one candidate we throw and need to tell the user.

gtsiolis

Just stopping by to leave on comment on an issue which could be unrelated to the changes of this PR. Other than that, UX LGTM. 🏁

components/dashboard/src/projects/new-project/NewProjectRepoList.tsx

gtsiolis · 2023-09-22T17:19:36Z

components/server/src/prebuilds/bitbucket-app.ts

-        cloneURL: string,
-        webhookInstaller: User,
-    ): Promise<{ user: User; project?: Project }> {
+    private async findProjectOwner(project: Project, webhookInstaller: User): Promise<User> {


praise: Just wanted to say it feels so good to see this kind of control where 3 repository instances with different prebuilds settings (not enabled, all branches, default branch) work side-by-side like this. Don't know if it's ok, but I only see one webhook in the project, see https://gitlab.com/gitpod-io/rails/-/hooks. Cc @AlexTugarev 🙏

AlexTugarev · 2023-09-25T07:43:36Z

components/server/src/prebuilds/bitbucket-server-app.ts

-                );
-            }
+            const projects = await this.projectService.findProjectsByCloneUrl(user.id, cloneURL);
+            for (const project of projects) {


I think it would be good to guard per project as well, otherwise if anything goes south with the first project at hand, e.g. context parsing, the other projects won't be processed.
(same applies to the other *-app.ts)

AlexTugarev · 2023-09-25T07:49:41Z

components/server/src/prebuilds/github-app.ts

 import { UserService } from "../user/user-service";
+import { ProjectsService } from "../projects/projects-service";
+import { SYSTEM_USER } from "../authorization/authorizer";


AlexTugarev · 2023-09-25T07:51:17Z

components/server/src/prebuilds/github-app.ts

+            const context = (await this.contextParser.handle({ span }, installationOwner, contextURL)) as CommitContext;
+            const projects = await this.projectService.findProjectsByCloneUrl(SYSTEM_USER, context.repository.cloneUrl);
+            for (const project of projects) {
+                const user = await this.findProjectOwner(project, installationOwner);


ah! this is how we handle FGA for prebuild.

AlexTugarev · 2023-09-25T08:07:26Z

A commit resulting in prebuilds being done on two project in separate Orgs 👏🏻

AlexTugarev

LGTM!

svenefftinge · 2023-09-25T08:58:15Z

/unhold

AlexTugarev · 2023-09-29T12:58:46Z

components/gitpod-db/src/typeorm/workspace-db-impl.ts

        const abortedState: PrebuiltWorkspaceState = "aborted";
        const repo = await this.getPrebuiltWorkspaceRepo();

        let query = repo.createQueryBuilder("pws");
-        query = query.where("pws.cloneURL = :cloneURL", { cloneURL });
+        query = query.where("pws.projectId != :projectId", { projectId });


should be a match, correct?

fixing via a840064 in parallel. Shall we create a separate PR?

fine with me to put it into your pr

roboquat added do-not-merge/work-in-progress do-not-merge/hold size/XXL labels Sep 22, 2023

svenefftinge force-pushed the se/multiple-projects branch from cc1c7cf to a19c9a7 Compare September 22, 2023 11:09

svenefftinge marked this pull request as ready for review September 22, 2023 11:10

svenefftinge requested a review from a team as a code owner September 22, 2023 11:10

roboquat removed the do-not-merge/work-in-progress label Sep 22, 2023

github-actions bot added the team: team-experience label Sep 22, 2023

svenefftinge commented Sep 22, 2023

View reviewed changes

svenefftinge force-pushed the se/multiple-projects branch from a19c9a7 to 0dd4dd0 Compare September 22, 2023 11:15

svenefftinge commented Sep 22, 2023

View reviewed changes

svenefftinge force-pushed the se/multiple-projects branch 2 times, most recently from 48a3c89 to faca93c Compare September 22, 2023 13:43

gtsiolis approved these changes Sep 22, 2023

View reviewed changes

AlexTugarev reviewed Sep 25, 2023

View reviewed changes

AlexTugarev approved these changes Sep 25, 2023

View reviewed changes

[projects] lookup projects by id not cloneURL

649cec2

svenefftinge force-pushed the se/multiple-projects branch from faca93c to 649cec2 Compare September 25, 2023 08:58

roboquat removed the do-not-merge/hold label Sep 25, 2023

roboquat merged commit 14a21a7 into main Sep 25, 2023
12 of 13 checks passed

roboquat deleted the se/multiple-projects branch September 25, 2023 09:13

AlexTugarev reviewed Sep 29, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple projects per repo #18774

Allow multiple projects per repo #18774

svenefftinge commented Sep 22, 2023 •

edited

Loading

svenefftinge Sep 22, 2023

svenefftinge Sep 22, 2023

svenefftinge Sep 22, 2023

AlexTugarev Sep 25, 2023

gtsiolis commented Sep 22, 2023

svenefftinge Sep 22, 2023

svenefftinge Sep 22, 2023

gtsiolis left a comment

gtsiolis Sep 22, 2023 •

edited

Loading

AlexTugarev Sep 25, 2023

AlexTugarev Sep 25, 2023

AlexTugarev Sep 25, 2023

AlexTugarev commented Sep 25, 2023

AlexTugarev left a comment

svenefftinge commented Sep 25, 2023

AlexTugarev Sep 29, 2023

svenefftinge Sep 29, 2023

AlexTugarev Sep 29, 2023

svenefftinge Sep 29, 2023

Allow multiple projects per repo #18774

Allow multiple projects per repo #18774

Conversation

svenefftinge commented Sep 22, 2023 • edited Loading

Description

🤖 Generated by Copilot at cc1c7cf

Related Issue(s)

How to test

Documentation

Preview status

Build Options

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtsiolis commented Sep 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtsiolis left a comment

Choose a reason for hiding this comment

gtsiolis Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexTugarev commented Sep 25, 2023

AlexTugarev left a comment

Choose a reason for hiding this comment

svenefftinge commented Sep 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svenefftinge commented Sep 22, 2023 •

edited

Loading

`🤖 Generated by Copilot at cc1c7cf`

gtsiolis Sep 22, 2023 •

edited

Loading