Check of allowed_pr_authors for dist-git PRs with multiple commits is not correct

[lbarcziova]

For pushes to dist-git, when downstream Koji builds are configured, we check in case the commit is coming from a PR whether the PR author matches the configuration of allowed_pr_authors.

The problem is in getting the PR here in case we don't react to the head commit (there are multiple commits in the PR and the head commit doesn't have specfile change):

packit-service/packit_service/worker/mixin.py

Lines 259 to 272 in d222ac1

	def pull_request(self):
	if not self._pull_request and self.data.event_dict["committer"] == "pagure":
	logger.debug(
	f"Getting pull request with head commit {self.data.commit_sha}"
	f"for repo {self.project.namespace}/{self.project.repo}"
	)
	prs = [
	pr
	for pr in self.project.get_pr_list(status=PRStatus.all)
	if pr.head_commit == self.data.commit_sha
	]
	if prs:
	self._pull_request = prs[0]
	return self._pull_request

Example:

• PR: https://src.fedoraproject.org/rpms/python-ibm-cloud-sdk-core/pull-request/6
• we reacted to this commit as it has a specfile change: https://apps.fedoraproject.org/datagrepper/v2/id?id=63799bf2-5bcf-417a-ab42-e2eb3f0039c2&is_raw=true&size=extra-large which was not the PR head commit and therefore we were not able to get the PR (author) . Therefore the pre-check failed on this which resulted in Koji build not being triggered.

Logs:

2023-11-16T07:03:31.506+00:00	Getting pull request with head commit 43b577b8642d26db0509c342d3c2dfcf674320a9for repo rpms/python-ibm-cloud-sdk-core
2023-11-16T07:03:32.091+00:00	PR author: None
2023-11-16T07:03:32.092+00:00	Push event rawhide with corresponding PR created by None that is not allowed in project configuration: ['packit'].

[lbarcziova]

I opened draft PRs for the proposed solution: packit/ogr#826, #2275 and packit/packit-service-fedmsg#99 - in fedmsg, filter the pushes (with the check for specfile changes) only if the commit doesn't come from a PR. If the commit comes for a PR, we should run the Koji build for the head commit of the PR which doesn't necessarily have to have the specfile change, so we will now check whether the whole PR has a specfile change. WDYT about this solution?

[nforro]

I think it makes sense, but it doesn't solve the case when someone pushes multiple commits directly (with the HEAD commit not containing a spec file change), right? That could theoretically happen if someone follows this workflow: https://packit.dev/docs/fedora-releases-guide#keeping-dist-git-branches-non-divergent

[lbarcziova]

good point, but I have to correct myself, after looking into the code of running the build, we just checkout the corresponding dist-git branch and do not checkout the specific commit, so that should be ok.

But this proabbly still seems to me like one of the easier solutions, as I haven't found a way (API) to get a list of all commits of a Pagure PR => a way of getting the PR from a commit that is not the first/last commit of a PR (that way, only the method for getting the corresponding PR would need to be adjusted).