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

Remove deprecated '#prebuild/' context URL prefix #4353

Closed
3 tasks done
Tracked by #14969 ...
svenefftinge opened this issue May 31, 2021 · 17 comments · Fixed by #15026
Closed
3 tasks done
Tracked by #14969 ...

Remove deprecated '#prebuild/' context URL prefix #4353

svenefftinge opened this issue May 31, 2021 · 17 comments · Fixed by #15026
Assignees
Labels
feature: prebuilds feature: teams and projects [DEPRECATED] Please, use feature: organizations or feature: projects labels instead. ⚙️ Groundwork meta: never-stale This issue can never become stale team: webapp Issue belongs to the WebApp team

Comments

@svenefftinge
Copy link
Member

svenefftinge commented May 31, 2021

When we have projects, we should no longer allow anyone to trigger prebuilds on arbitrary repos. Instead, the context URLs should become an admin-only feature.


Edits by @gitpod-io/engineering-webapp:

  • Prebuilds should only be triggered via the Run Prebuild button in the dashboard or the Gitpod API.
  • If Admins need to trigger prebuilds, we can also add a Run Prebuild button to the Admin dashboard.

However, before the prefix can be definitely removed, we need to ensure that we're not removing functionality or breaking user workflows without adequate replacement.

Depends on

@shaal
Copy link
Contributor

shaal commented Jul 17, 2021

I have 2 questions regarding this issue:

  1. Would it be possible to add to this issue, that only a person with permission to trigger a prebuild, will be able to see the output of the prebuild? (related to running tmate during dockerfile build https://github.com/shaal/DrupalPod/blob/debug-with-tmate/.gitpod/.gitpod.Dockerfile#L29-L34)
  2. The other URL params (that creates environment variables in the workspace) will remain available for everyone to use, is that correct?

@jankeromnes
Copy link
Contributor

Given that this feature is broken (see #6688 (comment)) I don't think it makes sense to spend a lot of effort to fix it just for admins.

Instead, it's probably better to make it redirect to the most sensible place (e.g. the Prebuilds page of the Project, if you have access to it, so that you can click on the new Run Prebuild button).

@jankeromnes
Copy link
Contributor

Now that #7422 is merged and deployed, I think we no longer need the manual prebuild prefix.

@jldec maybe we can rename this issue to "Deprecate and remove Prebuild Context URLs" and schedule it?

It's a quick fix, and I believe it's both no longer useful and actually broken and harmful from a security standpoint (since anyone can trigger Prebuilds for any context URL).

@jankeromnes
Copy link
Contributor

Side note: the /#incremental-prebuild/ prefix should go away too. It's equally broken (no project association) and was never announced in the first place (it was more of an internal hook for us to test Incremental Prebuilds).

@shaal
Copy link
Contributor

shaal commented Feb 8, 2022

Pleeeeease do not remove manual prebuild prefix.
It is the only way I can prebuild DrupalPod project, which is hosted in a Gitlab self-hosted environment.
https://git.drupalcode.org/project/drupalpod

I still didn't figure out how to add DrupalPod project to gitpod.io/projects, see #7781

@jankeromnes
Copy link
Contributor

As usual, many thanks @shaal for surfacing dependencies we're (or I'm) not aware of! 🙏 🥇

Will edit the issue title + description, and also directly link to any issues that need to be resolved first before the prefix can go.

@jankeromnes jankeromnes changed the title Make Prebuild Context URLs admin only Remove the manual #prebuild/ URL prefix Feb 9, 2022
@jankeromnes jankeromnes changed the title Remove the manual #prebuild/ URL prefix Remove deprecated #prebuild/ context URL prefix Feb 9, 2022
@jankeromnes jankeromnes changed the title Remove deprecated #prebuild/ context URL prefix Remove deprecated '#prebuild/' context URL prefix Feb 9, 2022
@stale
Copy link

stale bot commented May 31, 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 May 31, 2022
@shaal
Copy link
Contributor

shaal commented May 31, 2022

Please add the label meta: never-stale to this issue.
Gitpod UI still doesn't provide a way to trigger manual prebuild whenever I want.
(we also need a way to trigger a manual image-build in the UI)

@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label May 31, 2022
@gtsiolis gtsiolis added the meta: never-stale This issue can never become stale label May 31, 2022
@jankeromnes
Copy link
Contributor

jankeromnes commented Sep 19, 2022

Good news: I believe there is now team-wide alignment on the fact that the "Run Prebuild" button should always trigger a new prebuild (regardless of whether there is already a successful prebuild available for the same commit).

This will allow us to:

  1. Make the Run Prebuild button always trigger a new prebuild consistently
  2. Remove the deprecated and broken #prebuild/ URL prefix

@geropl
Copy link
Member

geropl commented Sep 19, 2022

  1. Make the Run Prebuild button always trigger a new prebuild consistently

I wonder if this is captured by #12010? 🤔

@jankeromnes
Copy link
Contributor

jankeromnes commented Sep 20, 2022

I wonder if this is captured by #12010? 🤔

@geropl Possibly, but that issue mentions several different problems, and is not actionable (i.e. doesn't have indications on what should actually be done).

@jankeromnes
Copy link
Contributor

jankeromnes commented Nov 28, 2022

Hi @shaal! Long time 👋 😁

Just letting you know that we're reviving this topic and we'd like to try again to:

  • Remove the deprecated #prebuild/ URL prefix
  • Drop support for Prebuilds without a Project (i.e. "invisible prebuilds")

Does that look good from your side now? (I believe ddev is now using a Project, correct?)

Also, please let me know if there are any other Prebuilds/Project/Gitpod problems that are bothering you or your community -- I can try to make them actionable & prioritize them.

@shaal
Copy link
Contributor

shaal commented Nov 29, 2022

@jankeromnes 👋 great to hear from you :)

I think the UI works well, where I can trigger prebuilds through it.
I did encounter some prebuilds that got stuck, and then it wouldn't allow trigger a prebuild using UI, so I had to use manual trigger in the URL. But these were rare cases.

Oh, just spoke about it, and here's a live example - https://gitpod.io/t/drupalpod/drupalpod/6e0bc450-94dc-4174-b5ef-d2aa4b575b34
Prebuild ended with non-zero result, and now I cannot re-run DrupalPod prebuild through UI, for the repo hosted in Drupal self-hosted Gitlab, https://git.drupalcode.org/project/drupalpod
(I could use the URL trigger, and it would run a new prebuild.

Not sure if it's related, but I'd love to get auto-prebuilds working also outside of Github, just like the example above of the repo that is in Drupal's self-hosted Gitlab.

@jankeromnes
Copy link
Contributor

Awesome, many thanks for your input @shaal!

Oh, just spoke about it, and here's a live example - https://gitpod.io/t/drupalpod/drupalpod/6e0bc450-94dc-4174-b5ef-d2aa4b575b34
Prebuild ended with non-zero result, and now I cannot re-run DrupalPod prebuild through UI

Ah, I believe this is the same problem as #12010 (comment) i.e. Gitpod considers that your prebuild ran "successfully" (i.e. there was no system error, your prebuild pod didn't crash or so), even though the prebuild task from your .gitpod.yml ended with an unsuccessful status code (i.e. "headless task failed: exit status XXX" -- which, by the way is not a user-friendly error message #9513).

I think the correct way to handle these situations is to make Gitpod always run a new prebuild when you click on the Run Prebuild button (i.e. it would become a "Force Run Prebuild"), regardless of whether there is or isn't a "successful" prebuild for that same commit.

Good reminder, thanks!

Not sure if it's related, but I'd love to get auto-prebuilds working also outside of Github, just like the example above of the repo that is in Drupal's self-hosted Gitlab.

Oh. Are you saying that, whenever you push new commits to Drupal's self-hosted GitLab repository, Gitpod does not automatically run a new prebuild?

I guess this would mean that we still don't fully support the GitLab version that's self-hosted by Drupal. I know that we've previously fixed the "listing of repositories and creation of Project" part (#7781) but maybe the very final step of installing a webhook onto the repository still doesn't work?

This would mean we should open a new issue about fixing webhook installations for Drupal's GitLab version.

@shaal
Copy link
Contributor

shaal commented Nov 29, 2022

@jankeromnes oh, another essential feature, manually triggering image update by #imagebuild/.
It might get deprecated as well, but I wish to have access to that through UI as well.

semi-related issue -
gitpod-io/workspace-images#914

@jankeromnes
Copy link
Contributor

Thanks for the hint @shaal! I'm not aware of any plans to deprecate or remove the #imagebuild/ prefix for now, but I agree that we will want to preserve this functionality (e.g. by providing a suitable UI replacement before removing any other URL prefix).

@jankeromnes jankeromnes moved this from Scheduled to In Progress in 🍎 WebApp Team Nov 30, 2022
Repository owner moved this from In Progress to In Validation in 🍎 WebApp Team Dec 1, 2022
@jankeromnes jankeromnes moved this from In Validation to Done in 🍎 WebApp Team Dec 5, 2022
@jankeromnes
Copy link
Contributor

I think the correct way to handle these situations is to make Gitpod always run a new prebuild when you click on the Run Prebuild button (i.e. it would become a "Force Run Prebuild"), regardless of whether there is or isn't a "successful" prebuild for that same commit.

Now tracked in this issue: #15144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: prebuilds feature: teams and projects [DEPRECATED] Please, use feature: organizations or feature: projects labels instead. ⚙️ Groundwork meta: never-stale This issue can never become stale team: webapp Issue belongs to the WebApp team
Projects
Status: Done
Status: Next iteration 🛹
6 participants