packit / packit-service

Packit provided as a service
https://packit.dev
MIT License
34 stars 46 forks source link

propose-downstream c9s forgets `centpkg new-sources` #2436

Open martinpitt opened 4 weeks ago

martinpitt commented 4 weeks ago

What happened? What is the problem?

We are trying to automate CentOS downstream releases in Cockpit. We started with cockpit-machines a while ago, and in this round we got the first automated packit PR :tada: : https://gitlab.com/redhat/centos-stream/rpms/cockpit-machines/-/merge_requests/62

This already looks quite useful, but unlike the corresponding Fedora MR it forgot to run centpkg new-source apparently -- the new upstream tarball wasn't in the lookaside cache (I did that manually with centpkg new-sources), and the MR diff is missing the sources and .gitignore updates.

Like this, the MR just fails to build.

I can commit these manually, but I'll wait for a few days in case you want to examine this -- that update isn't urgent.

What did you expect to happen?

No response

Example URL(s)

No response

Steps to reproduce

No response

What is the impacted category (job)?

Fedora release automation

Workaround

Participation

lbarcziova commented 4 weeks ago

hi @martinpitt ! Unfortunately, this step is currently being skipped in the service on purpose, see the second warning in the docs. The problem we hit when implementing was accessing the internal lookaside cache. @mfocko can provide more details. For now, we haven't decided what will be the next steps to fix this and were also waiting for the first adopters of the automation to see whether it is worth spending time on it.

martinpitt commented 4 weeks ago

Ah, thanks @lbarcziova ! Feel free to close this then, if that's already tracked elsewhere. To us, this is actually the most annoying step: One needs to install the rhel developer tools, get a kerberos ticket, get the tar from Fedora etc.

mfocko commented 3 weeks ago

Hi, sorry for a bit late response, there are multiple things to unpack here…

Let's start with a cause of this issue. We discussed this feature alongside with downstream Koji builds (for CentOS) somewhere around September year ago with @bookwar, the major concern was downloading sources from who-knows-where[^1] and essentially pushing them to RHEL. As you have said, pushing sources requires Kerberos ticket, thus requires access to internal network (or at least VPN), this again is rather problematic from the security standpoint.

[!NOTE] Fun fact! This concern has been voiced a year ago, even before the xz incident. I have a feeling that pushing for this change would be even more complicated right now.

I think that at this point we should just admit to the fact that the packaging workflow is not suited for external contributions nor automation. You need to be a packager (which is relatively reasonable requirement, not very nice to external contributors, but can prevent spamming[^2]), upload the sources to lookaside cache[^3] and then open the PR. The issue I see here comes from the fact that CI depends on already uploaded sources to the lookaside :/

IMO the best decision here would be to push for “stage” or “temporary” part of the lookaside cache that could be cleaned up regularly of sources that didn't make it or allows easy[^4] deletion in cases of license issues or malicious content.

Other option could be adjusting the downstream CI workflow to allow for running the CI with proposed changes, but uploading the sources after the merge[^5].

Neither one seem like something easy to do :/ OTOH we're collaborating with the CentOS Integration SIG, one of the closest milestones includes building in the CBS Koji, hopefully we can improve more pain points of the current workflow :)

[^1]: I am fully aware that you also maintain upstream, so many of those concerns do not apply to you, but there is no reasonable way for us to differentiate that. [^2]: I guess that's the reason for maintaining git-mail in Linux itself [^3]: Please keep in mind that automated uploading of sources to lookaside cache has been already brought up as a concern even for Fedora, that's how upload_sources came to existence. [^4]: Ticket definitely doesn't count for easy… [^5]: That introduces a different kind of issues though, e.g., outage after merge leaving inconsistent state.

martinpitt commented 3 weeks ago

downloading sources from who-knows-where

Note that I'm not proposing that. Such an automated CentOS MR should only ever work if there also is a corresponding Fedora update. In other words, this should get the sources from fedpkg sources. I agree that a direct download is problematic.

Thanks @mfocko for the updates! If the end result is that CentOS stream updates can't be automated because reasons, that's also a definitive outcome. We could then at least work on trying to build this externally (similar to https://github.com/osbuild/fedora-bot).

lachmanfrantisek commented 3 weeks ago

@martinpitt Regarding doing the upload by another service, @cathay4t has suggested a similar approach already that we can create a separate service that could e.g. react to MR comments and upload this on request.

I think separating these two functionalities (MR submission and archive upload) might be more secure and we can also discuss/design rules when the upload can be done. And as Packit team, we are more than happy to collaborate on this.


A side note and if I understand this correctly, this won't be needed in Konflux if it becomes a reality..;)

lachmanfrantisek commented 3 weeks ago

Btw. I've discussed this with jkaluza and there is no plan to do anything with this as part of *OnGitlab efforts.

M4rtinK commented 3 weeks ago

Well, this is quite a bummer - and puts into questions even spending effort on build automation if the process can't be fully automated and still requires annoying manual steps anyway.

jkonecny12 commented 3 weeks ago

In this case there is a question if CLI is not easier than the service when PR created by the service requires manual steps :thinking:

lachmanfrantisek commented 2 weeks ago

@M4rtinK , @jkonecny12, you are right that this reduces the benefit of automation a lot if we still need to switch to CLI.

But:

jkonecny12 commented 2 weeks ago

This would be great. Thanks for all the work you are doing 👍