Don't decline SRs from non-devel project for scmsync packages #2977
Conversation
dcermak
force-pushed
the
source-review-scmsync
branch
from
d2dea82
to
cda05b1
Compare
|
This needs some explanation (commit message, code) IMO. I assume I know the answers to this, but not everyone might and my interpretation might even be wrong.
|
| not scm_sync or | ||
| ( | ||
| scm_sync and | ||
| not scm_sync.text.startswith(f"https://src.opensuse.org/pool/{source_package}") |
There was a problem hiding this comment.
Using startswith here will return false positives for URLs like https://src.opensuse.org/pool/{source_package}-subpkg or https://src.opensuse.org/pool/{source_package}/../../hacker/pwned.git
There was a problem hiding this comment.
if the devel project has a hacker/pwned reference, we have larger issues
There was a problem hiding this comment.
Right, I see this only checks the scmsync tag of the devel pkg, not the checked request. That should probably be done as well somewhere?
There was a problem hiding this comment.
The SR will be generated by a bot, thus I don't really see the need for that. It will not really make anything more secure, only hinder us in changing things later on.
Because the devel project also provides the functionality to build the same package for older distributions via additional repositories. that functionality (that many users care about as there are > 50% leap users) would otherwise get lost.
its the location of where the sources of a package are authoritatively living once git workflow has been activated. |
That's orthogonal, I'm just referring to the
|
How do you know that its from git? "somewhere" that information needs to come from, until we have switched over completely. plus the information of devel projects is still useful for package maintainership rights handling. Eventually we can switch over user access control to git but its still a long way to get there. it would be good to be able to start somewhere. do you have a better suggestion? |
At some point the scmsync property is set on those packages in oS:F and the devel prop removed. Until then the submissions will be from the devel prj I imagine?
|
maybe, that will be a longer road though, because it means we need to reimplement staging outside buildservice. currently staging and all the bots operate on submitrequest, and submitrequests do not work with scmsyncs. so we can't set this right now. Setting it in the devel project instead still keeps the functionality of devel projects alive plus makes it obvious that this package is no longer maintained in the devel project but elsewhere (and srs to devel project will fail, which is a feature in that regard). |
dcermak
force-pushed
the
source-review-scmsync
branch
from
cda05b1
to
f3595b6
Compare
|
I've changed the bot to now check the name of the source project and grab the allowed sources from OBS as well instead of hardcoding them. |
dcermak
force-pushed
the
source-review-scmsync
branch
from
944fa3f
to
8ad40eb
Compare
dcermak
force-pushed
the
source-review-scmsync
branch
from
8ad40eb
to
7170e57
Compare
…e bot
We want to start transitioning to a git based development workflow. For the
first iteration, we would allow maintainers to opt-in by changing their package
in the devel project to use scmsync from src.opensuse.org/pool/${pkg_name} and
submit changes via pull requests on gitea. These pull requests will get
submitted directly by the scm-staging-bot to Factory from its home project as
submit requests.
Currently, such a submission would get auto-declined from the factory-auto
bot. With this commit, factory-auto will no longer decline such a submission
provided that the above conditions have been met.
Co-authored-by: Dirk Mueller <[email protected]>
dcermak
force-pushed
the
source-review-scmsync
branch
from
7170e57
to
5470fa4
Compare
|
Max Lin ***@***.***> writes:
@nilxam commented on this pull request.
> @@ -62,6 +62,8 @@ def target_project_config(self, project):
self.allow_valid_source_origin = str2bool(config.get('check-source-allow-valid-source-origin', 'False'))
self.valid_source_origins = set(config.get('check-source-valid-source-origins', '').split(' '))
self.add_devel_project_review = str2bool(config.get('check-source-add-devel-project-review', 'False'))
+ self.allowed_scm_submission_sources = set(
+ config.get('allowed-scm-submission-sources', 'devel:Factory:git-workflow:staging').split(' '))
Yes, the default value should be empty.
Done
|
| ( | ||
| scm_sync and | ||
| not scm_sync.text.startswith(f"https://src.opensuse.org/pool/{source_package}") | ||
| and all(not source_project.startswith(allowed_src) for allowed_src in self.allowed_scm_submission_sources) |
There was a problem hiding this comment.
isn't that supposed to be any( rather than all( ?
There was a problem hiding this comment.
No, the branch should only be taken if the source project name begins with something that is none of the allowed sources.
No description provided.