PR#1365: Move SCM policies to the koji hub - fedora-infra/ansible

fedora-infra / ansible

#1365 Move SCM policies to the koji hub

Merged 10 months ago by kevin. Opened a year ago by tkopecek.

fedora-infra/ tkopecek/ansible issue9728 into main

Move SCM policies to the koji hub

Tomas Kopecek • 10 months ago

8d99c12

roles/koji_builder/templates/kojid.conf

file modified

+3 -47

		`@@ -76,53 +76,9 @@`
		`pkgurl=http://kojipkgs.stg.fedoraproject.org/packages`
		`{% endif %}`

		`- {% if env == 'staging' %}`
		`- ; A whitespace-separated list of hostname:repository pairs that kojid is authorized to checkout from (no quotes)`
		`- allowed_scms=`
		`- !src.stg.fedoraproject.org:/pagure/fork/*`
		`- !src.stg.fedoraproject.org:/pagure/forks/*`
		`- !pkgs.stg.fedoraproject.org:/pagure/fork/*`
		`- !pkgs.stg.fedoraproject.org:/pagure/forks/*`
		`- !src.stg.fedoraproject.org:/fork/*`
		`- !src.stg.fedoraproject.org:/forks/*`
		`- !src.stg.fedoraproject.org:/cgit/*`
		`- src.stg.fedoraproject.org:/container/*:false`
		`- src.stg.fedoraproject.org:/flatpaks/*:false`
		`- src.stg.fedoraproject.org:/git/rpms/*:false:fedpkg,sources`
		`- !src.stg.fedoraproject.org:/git/*`
		`- !pkgs.stg.fedoraproject.org:/fork/*`
		`- !pkgs.stg.fedoraproject.org:/forks/*`
		`- !pkgs.stg.fedoraproject.org:/cgit/*`
		`- !pkgs.stg.fedoraproject.org:/git/*`
		`- pkgs.stg.fedoraproject.org:/container/*:false`
		`- pkgs.stg.fedoraproject.org:/flatpaks/*:false`
		`- pkgs.stg.fedoraproject.org:/rpms/*:false:fedpkg,sources`
		`- pkgs.stg.fedoraproject.org:/*:false:fedpkg,sources`
		`- src.stg.fedoraproject.org:/*:false:fedpkg,sources`
		`- pkgs.fedoraproject.org:/rpms/*:false:fedpkg,sources`
		`- pkgs.fedoraproject.org:/*:false:fedpkg,sources`
		`- pagure.io:/fedora-kickstarts.git:false`
		`- src.fedoraproject.org:/*:false:fedpkg,sources`
		`- {% else %}`
		`- ; A whitespace-separated list of hostname:repository pairs that kojid is authorized to checkout from (no quotes)`
		`- allowed_scms=`
		`- !src.fedoraproject.org:/pagure/fork/*`
		`- !src.fedoraproject.org:/pagure/forks/*`
		`- !pkgs.fedoraproject.org:/pagure/fork/*`
		`- !pkgs.fedoraproject.org:/pagure/forks/*`
		`- !src.fedoraproject.org:/fork/*`
		`- !src.fedoraproject.org:/forks/*`
		`- !src.fedoraproject.org:/cgit/*`
		`- !src.fedoraproject.org:/git/*`
		`- !pkgs.fedoraproject.org:/fork/*`
		`- !pkgs.fedoraproject.org:/forks/*`
		`- !pkgs.fedoraproject.org:/cgit/*`
		`- !pkgs.fedoraproject.org:/git/*`
		`- pkgs.fedoraproject.org:/*:false:fedpkg,sources`
		`- pagure.io:/fedora-kickstarts.git:false`
		`- src.fedoraproject.org:/*:false:fedpkg,sources`
		`- pagure.io:/fork/*/fedora-kickstarts.git:false`
		`- {% endif %}`
		`+ # everything related to allowed scms is now defined at hub`
		`+ allowed_scms_use_config = false`
		`+ allowed_scms_use_policy = true`

		`; allow tasks to continue to completion if a sibling fails`
		`; the parent task will fail but all child tasks will complete`

roles/koji_hub/templates/hub.conf.j2

file modified

+41 -12

		`@@ -203,16 +203,45 @@`
		`all :: deny`

		`{% if env == "staging" %}`
		`- # Policy for building scratch builds`
		`build_from_scm =`
		`- # allow scratch build for anything from anywhere`
		`- bool scratch :: allow`
		`- # allow to build from forks`
		`- match scm_type GIT GIT+SSH && match scm_host src.fedoraproject.org/forks/* :: allow`
		`- {% endif %}`
		`-`
		`- scm =`
		`- # allow scratch builds from any commits`
		`- bool scratch :: allow`
		`- match_all branches * !! deny Commit must be present on some branch`
		`- all :: allow`
		`+ match scm_host src.stg.fedoraproject.org :: {`
		`+ bool scratch :: fedpkg sources`
		`+ match scm_repository /rpms/* :: fedpkg sources`
		`+ match scm_repository /modules/* :: fedpkg sources`
		`+ match scm_repository /containers/* :: fedpkg sources`
		`+ match scm_repository /flatpaks/* :: fedpkg sources`
		`+ }`
		`+ match scm_host pkgs.stg.fedoraproject.org :: {`
		`+ bool scratch :: fedpkg sources`
		`+ match scm_repository /rpms/* :: fedpkg sources`
		`+ match scm_repository /modules/* :: fedpkg sources`
		`+ match scm_repository /containers/* :: fedpkg sources`
		`+ match scm_repository /flatpaks/* :: fedpkg sources`
		`+ }`
		`+ match scm_host pkgs.fedoraproject.org && match scm_repository /* :: allow fedpkg sources`
		`+ match scm_host pagure.io && match scm_repository /fedora-kickstarts.git :: allow`
		`+ match scm_host src.fedoraproject.org :: allow fedpkg sources`
		`+ all :: deny`
		`+ {% else %}`
		`+ build_from_scm =`
		`+ match scm_host src.fedoraproject.org :: {`
		`+ bool scratch :: fedpkg sources`
		`+ match scm_repository /rpms/* :: fedpkg sources`
		`+ match scm_repository /modules/* :: fedpkg sources`
		`+ match scm_repository /containers/* :: fedpkg sources`
		`+ match scm_repository /flatpaks/* :: fedpkg sources`
		`+ }`
		`+ match scm_host pkgs.fedoraproject.org :: {`
		`+ bool scratch :: fedpkg sources`
		`+ match scm_repository /rpms/* :: fedpkg sources`
		`+ match scm_repository /modules/* :: fedpkg sources`
		`+ match scm_repository /containers/* :: fedpkg sources`
		`+ match scm_repository /flatpaks/* :: fedpkg sources`
		`+ }`
		`+ match scm_host pagure.io :: {`
		`+ bool scratch :: allow`
		`+ match scm_repository /fedora-kickstarts.git :: allow`
		`+ match scm_repository /fork/*/fedora-kickstarts.git :: allow`
		`+ }`
		`+ all :: deny`
		`+ {%endif}`

tkopecek commented a year ago

Moving all SCM policies previously defined in each builder to
centralized hub configuration. From now on, any SCM policy change just
needs updating the hub config and reloading it. Builders need nor change
nor reload.

Signed-off-by: Tomas Kopecek tkopecek@redhat.com

Metadata Update from @zlopez:
- Pull-request tagged with: post-freeze

a year ago

zuul commented a year ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/ce428b4bc28f4634ab60dedab6303993

fi-ansible-lint-diff : SUCCESS in 5m 26s
fi-yamllint-diff : SUCCESS in 3m 03s

zlopez commented on line 42 of roles/koji_hub/templates/hub.conf.j2 a year ago

Is this enough to allow scratch builds from forks?

tkopecek commented a year ago

Is this enough to allow scratch builds from forks?

Yes, and rule on line 46 will forbid non-scratch builds from the same server forks. Maybe you would also like to allow /pagure/forks/*?

zlopez commented a year ago

I'm not sure if we want to allow builds from pagure as well. @kevin What do you think?

kevin commented a year ago

Official builds need to pull from pagure.io for things like comps and kickstarts (for image builds) and some ostree config (for ostree composes). We don't want to allow forks there either, just those specific repos. I guess comps and such are not directly koji, but are pungi. so I guess thats right.

It's also worth noting that I have seen a build from a commit thats only in a fork... it was of the form:
https://src.fedoraproject.org/rpms/package/c/<sha1sum>

but I don't understand how koji found it. It should have only been in the fork. ;(

zlopez commented a year ago

@kevin Do we want to merge and deploy this now, when the freeze is over?

kevin commented a year ago

We are waiting for a few more koji fixes... lets hold this and push it at the same time we do that?

mikem commented a year ago

I don't think that /fork/ repos should ever be allowed. This seems to be coming from confusion about they way that PRs work in pagure. The main pagure repo includes non-branch refs for all PRs with names like "refs/pull/NNN/head" (see git ls-remote for a pagure checkout). These match the source of the PR, which is typically a fork, but could also be an external git repo, however the build should be pulling from the main pagure repo.

mikem commented a year ago

I don't believe you can use the repository field to check for whether the build is from a PR. That requires deeper analysis of the ref.

mikem commented a year ago

There are in fact zero builds in koji.fp.o whose source points at a fork url

koji=> select id, source from build where source like '%/fork/%';
 id | source 
----+--------
(0 rows)

Edited a year ago by mikem

mikem commented a year ago

Here are where non-scratch builds are currently coming from (over the last 4 years).

koji=> select count(id), regexp_match(source, '([^:]*://[^/]*/[^/]*)/.*|[^/]*(.src.rpm$)') as srv from build where source is not null and source != '' and state=1 and completion_time > now() - '4 year'::interval group by srv;
 count  |                       srv                        
--------+--------------------------------------------------
 619135 | {git+https://src.fedoraproject.org/rpms,NULL}
    222 | {https://src.fedoraproject.org/container,NULL}
   6461 | {NULL,.src.rpm}
    973 | {git+https://src.fedoraproject.org/modules,NULL}
   3869 | {https://src.fedoraproject.org/flatpaks,NULL}
   5028 | {https://src.fedoraproject.org/modules,NULL}
(6 rows)

all from src.fp.o or from srpm (the srpms are predominantly mbs)
all src.fp.o paths are fall under rpms, container, flatpaks, or modules
no fork scm urls

kevin commented 11 months ago

ok then, shall we drop forks (and cgit, since we don't even use cgit anymore)... and can you rebase? :)

1 new commit added

limit builds to rpms/container/flatpaks/modules

11 months ago

tkopecek commented 11 months ago

So, are we going opposite way to allow just these four paths?

zuul commented 11 months ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging pagure.io/fedora-infra/ansible for 1365,70baf199770666300199b0a6d18266d1a5901ea4

kevin commented 11 months ago

So, what we want is this: