Skip to content
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.

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

Epic: Deprecate definitely-gp #8473

Open
2 tasks
jldec opened this issue Feb 27, 2022 · 12 comments · Fixed by gitpod-io/definitely-gp#86, #18278 or #18316
Open
2 tasks

Epic: Deprecate definitely-gp #8473

jldec opened this issue Feb 27, 2022 · 12 comments · Fixed by gitpod-io/definitely-gp#86, #18278 or #18316
Labels
meta: never-stale This issue can never become stale team: webapp Issue belongs to the WebApp team type: epic

Comments

@jldec
Copy link
Contributor

jldec commented Feb 27, 2022

Early Gitpod users were given the option of storing their config in https://github.com/gitpod-io/definitely-gp/ instead of in their own repo. This feature is not documented anymore.

Workspaces start by looking for a directory in definitely-gp which matches the repo name, and then use the configuration from that directory. The configuration can include both gitpod.yml and a Dockerfile.

There are several issues with this approach

  • False positive repo name matches result in unexpected errors during workspace initialization e.g. see below.
  • Depending on framework-specific configuration from another Gitpod-owned repo for workspace starts, creates additional maintenance and software supply chain security burdens.
  • References to publicly hosted repos from within Gitpod complicate self-hosted installations on private networks.

See discord discussion

Proposal
Gitpod should deprecate the built-in runtime use of files from definitely-gp for workspace configuration.

A community-driven repo like https://github.com/shaal/awesome-gitpod would be a better way to share best practices of how to configure Gitpod for different frameworks or languages.

If there are significant numbers of repos in use which regulary depend on this capability, we should do this in 2 stages (if the number is small say < 100, we can simply remove the capability immediately):

The length of the deprecation period depends on how heavily used this feature is.

Background

I noticed when opening a repo called kit that the workspace was being initiazlized with pnpm, even though it was configured for npm with a package-lock.json.

  • The repo was not configured for gitpod.
  • No inferred .gitpod.yml was added inside the workspace, making the source of the errors difficult to work out.

Screenshot 2022-02-27 at 22 21 16

Screenshot 2022-02-27 at 22 20 51

A GitHub search revealed that the init and command tasks were probably coming from here: https://github.com/gitpod-io/definitely-gp/blob/master/kit/.gitpod.yml.

From the inference code here and implementation here it appears that opening a workspace on any repo whose name matches a directory in https://github.com/gitpod-io/definitely-gp will attempt to use the configuration from there.

To reproduce

For another example see https://gitpod.io/#https://github.com/gitpod-io/book

@jldec jldec added the team: webapp Issue belongs to the WebApp team label Feb 27, 2022
@jldec jldec added the priority: highest (user impact) Directly user impacting label Feb 27, 2022
@jldec
Copy link
Contributor Author

jldec commented Feb 27, 2022

@jankeromnes, assigned to you, since you helped review #7383 😃

Unless we can do smarter detection, we should probably remove definitely-gp as a source of inferred configurations.
Not copying those configurations into the repo, also makes it really hard for users to understand what's happening.

cc: @svenefftinge

@jldec jldec moved this to Scheduled in 🍎 WebApp Team Feb 28, 2022
@jankeromnes
Copy link
Contributor

@jldec A quick fix for this could be to place the "kit" directory in definitely-gp under a "sveltejs" directory, to make the matching more precise.

From past experience, anything with 4 letters or less is prone to false-positive matches, and should be configured as "org/repo", not just "repo".

@jankeromnes jankeromnes moved this from Scheduled to In Progress in 🍎 WebApp Team Feb 28, 2022
Repository owner moved this from In Progress to Done in 🍎 WebApp Team Feb 28, 2022
@jldec jldec reopened this Feb 28, 2022
Repository owner moved this from Done to In Progress in 🍎 WebApp Team Feb 28, 2022
@jldec
Copy link
Contributor Author

jldec commented Feb 28, 2022

@jankeromnes

  • this does not address other commonly used repo names like 'book' or 'vscode' which will still match
  • users are not in control of how this matching works - we simply take their repo name and look for a directory match. I think this is still likely to produce a lot of bad matches unless we make all the directory names really unlikely.
  • Even if we rely on users to know how to name their directories in order to get a match, or we document the directory names, I'm not sure this additional magic is worth the security risk of auto-injecting dependency config into workspaces from a repo which is not as tightly controlled as gitpod-io/gitpod.

I would prefer to stop auto-matching on directories in definitely-gp and just document it's existence in our docs so that users can copy and maintain themselves.

@jldec jldec changed the title Bad auto-inferred gipod.yaml when repo-name matches directory in definitely-gp Epic: Deprecate definitely-gp Mar 1, 2022
@jldec jldec removed the priority: highest (user impact) Directly user impacting label Mar 1, 2022
@jldec jldec moved this from In Progress to Scheduled in 🍎 WebApp Team Mar 1, 2022
@jeanp413
Copy link
Member

jeanp413 commented Mar 1, 2022

💯 to deprecate definetely-gp, I created an issue a while ago to at least make it configurable to opt-out #7336 as it was causing me trouble when opening upstream vscode repo
@jldec feel free to close #7336 as a duplicate of this issue

@loujaybee
Copy link
Member

Related: #3746

@andreafalzetti
Copy link
Contributor

+1 on deprecating the current behavior, too magical and causes undesired side-effects.

However, also in light of #3746, it would be interesting to discuss how to keep this feature reimplementing it slightly differently.

If I could have a user setting in which I can maintain my own list of repos that are checked fist, I could decide to opt-in for using the community-driven repo or even have my own private repo in which I configure Gitpod for private repos to which I cannot commit the .gitpod.yml.

@axonasif
Copy link
Member

Maybe ask the user whether to load a gitpod provided config or not during workspace start?

@Pothulapati
Copy link
Contributor

Hey 👋🏼 ,

With https://github.com/gitpod-io/gitpod/pull/9094/files, We are setting the definitelyGpDisabled option to be true, and thus disabling it by default. Should we update the SAAS and preview environments deployment configs to enable it (i.e setting it to false) as a part of that PR? 🤔

@laushinka
Copy link
Contributor

Should we update the SAAS and preview environments deployment configs to enable it (i.e setting it to false) as a part of that PR? 🤔

@Pothulapati Isn't the SAAS deployment set to false by default already?

@Pothulapati
Copy link
Contributor

Should we update the SAAS and preview environments deployment configs to enable it (i.e setting it to false) as a part of that PR? 🤔

@Pothulapati Isn't the SAAS deployment set to false by default already?

Hmm, Where is it being set? I couldn't find the configuration anywhere and hence the question! If it is already being set explicitly , then the change would go smooth! If not then it will be set to true after it's merged as the installer's default is being changed. 🤔

@geropl geropl moved this from Scheduled to Epic in Progress in 🍎 WebApp Team Apr 7, 2022
@stale
Copy link

stale bot commented Jul 10, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jul 10, 2022
@jeanp413 jeanp413 added meta: never-stale This issue can never become stale and removed meta: stale This issue/PR is stale and will be closed soon labels Jul 10, 2022
@jldec jldec removed their assignment Feb 7, 2023
@github-project-automation github-project-automation bot moved this to In Validation in 🍎 WebApp Team Jul 13, 2023
@AlexTugarev
Copy link
Member

Reverted in #18306. It should be possible to re-apply the PR with some changes.

@AlexTugarev AlexTugarev reopened this Jul 19, 2023
@github-project-automation github-project-automation bot moved this from In Validation to Scheduled in 🍎 WebApp Team Jul 19, 2023
@github-project-automation github-project-automation bot moved this from Scheduled to In Validation in 🍎 WebApp Team Jul 20, 2023
@AlexTugarev AlexTugarev reopened this Jul 25, 2023
@github-project-automation github-project-automation bot moved this from In Validation to Scheduled in 🍎 WebApp Team Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: never-stale This issue can never become stale team: webapp Issue belongs to the WebApp team type: epic
Projects
Status: Scheduled
9 participants