#2757 RFE: scratch build specific allowed_scms?
Closed: Fixed 2 years ago by tkopecek. Opened 3 years ago by kevin.

It might be nice to be able to specify an additional or seperate set of allowed scms for scratch builds.

For example, in the fedora case, it would be nice to allow scratch builds of forks with PR changes on them, but disallow forks for official builds.

See https://pagure.io/fedora-infrastructure/issue/9728 for CI folks asking for this (they want to scratch build a PR to test the rpms).


Metadata Update from @tkopecek:
- Custom field Size adjusted to None
- Issue set to the milestone: 1.26

3 years ago

In theory this should be quite easy to implement.

  • Add new argument to koji.daemon.SCM.assert_allowed()scratch=False
  • Add new option allowed_scms_scratch_extra which would be list of SCMs that would be allowed for scratch builds (they will implicitly inherit allowed_scms)
  • Pass scratch parameter from the build tasks where SCM.assert_allowed() is called

To keep compatibility with older hubs, wrap in extra try / except calls to assert_allowed()

@tkopecek does this make sense? if so, I can work on a PR for this.

Yep, I think it is ok - @mikem ?

or, one or more new flag(s) in allowed_scms entry? like *:scratch_only:* or *:regular:scratch:*

@julian I'd like to avoid complicating parsing of those values even more… which is already over-complicated. Plus, it would be even more confusing to work with it.

or, one or more new flag(s) in allowed_scms entry? like :scratch_only: or :regular:scratch:

Agree with Igor here. This is making an already tedious configuration even worse. It might be time to provide a better way to configure the allowed scms.

I am ok with the overall intent here. However, once we start down the road of different scm rules for different scenarios, I wonder how complex we might need to get. Do we need to consider making this a policy?

Do we need to consider making this a policy?

I'm curious if allowed_scms is used per build on some environment. IMO, a policy section supporting channel test is sufficient.

Maybe even simple policy could work here similar to e.g. build_from_scm

build = 
    bool scratch :: allow
    source scratch_only_scm :: deny
    all :: allow

A candidate PR: #2951

This almost does the same as assert_allowed, maybe we could split the setting of use_common and source_cmd out.

Metadata Update from @jcupova:
- Issue tagged with: testing-ready

2 years ago

Metadata Update from @jcupova:
- Issue tagged with: testing-done

2 years ago

Thanks you for implementing this! :wink:

Login to comment on this ticket.

Metadata
Related Pull Requests
  • #2951 Merged 2 years ago