PR#1135: Allow hub policy to match build NVRs - koji

koji

#1135 Allow hub policy to match build NVRs

Closed 4 years ago by tkopecek. Opened 5 years ago by mizdebsk.

mizdebsk/koji policy-match-nvr into master

Allow hub policy to match build NVRs

Mikolaj Izdebski • 5 years ago

1d9e990

file modified

		`@@ -174,6 +174,9 @@`
		``package``
		`* Matches its arguments against the package name. Accepts glob patterns.`

		+ ``nvr``
		`+ * Matches its arguments against the build NVR. Accepts glob patterns.`
		`+`
		``tag``
		`* matches its arguments against the tag name. Accepts glob patterns.`
		`* for move operations, the tag name tested is the destination tag (see`

hub/kojihub.py

file modified

+19

		`@@ -8128,6 +8128,16 @@`
		`#else`
		`raise koji.GenericError("policy requires package data")`

		`+ def policy_get_build(data):`
		`+ """Determine build from policy data`
		`+`
		`+ returns dict as get_build`
		`+ """`
		`+ if 'build' in data:`
		`+ return get_build(data['build'], strict=True)`
		`+ #else`
		`+ raise koji.GenericError("policy requires build data")`
		`+`

		`def policy_get_brs(data):`
		`"""Determine content generators from policy data"""`
		`@@ -8192,6 +8202,15 @@`
		`data[self.field] = policy_get_pkg(data)['name']`
		`return super(PackageTest, self).run(data)`

		`+ class NvrTest(koji.policy.MatchTest):`
		`+ """Checks build NVR against glob patterns"""`
		`+ name = 'nvr'`
		`+ field = '_nvr'`
		`+ def run(self, data):`
		`+ #we need to find the build NVR from the base data`
		`+ data[self.field] = policy_get_build(data)['nvr']`
		`+ return super(NvrTest, self).run(data)`
		`+`
		`class VolumeTest(koji.policy.MatchTest):`
		`"""Checks storage volume against glob patterns"""`
		`name = 'volume'`

tests/test_hub/test_policy_tests.py

file modified

		`@@ -24,6 +24,14 @@`
		`policy_get_pkg.return_value = {'name': 'foobar'}`
		`self.assertTrue(obj.run({}))`

		`+ @mock.patch('kojihub.policy_get_build')`
		`+ def test_nvr_test(self, policy_get_build):`
		`+ obj = kojihub.NvrTest('nvr *bar')`
		`+ policy_get_build.return_value = {'nvr': 'mypackage-bar-xyzzy'}`
		`+ self.assertFalse(obj.run({}))`
		`+ policy_get_build.return_value = {'nvr': 'pkg-foo-bar'}`
		`+ self.assertTrue(obj.run({}))`
		`+`
		`@mock.patch('kojihub.policy_get_pkg')`
		`def test_new_package_test(self, policy_get_pkg):`
		`obj = kojihub.NewPackageTest('is_new_package')`

mizdebsk commented 5 years ago

Fixes #1134

ngompa commented 5 years ago

:thumbsup:

tkopecek commented 5 years ago

:thumbsup:
What I was thinking was to split build['nvr'] to nvr dict. But not sure if anybody has a use for such test - glob on whole nvr doesn't need to match properly, if only e.g. release should be tested.

mizdebsk commented 5 years ago

Or we could simply have separate checks for version and release - there is already check for name IIRC.

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

4 years ago

mikem commented 4 years ago

Taking a step back, testing the nvr string with glob patterns is flawed and prone to error. If we add just this, folks will use it to effectively test for versions with patterns like *-1.1-*, but such a pattern could also match a 1.1 in the package name. Note that the issue listed as fixed is asking to match version and release, not nvr.

Edited 4 years ago by mikem

mikem commented 4 years ago

I fiddled around and split it into separate version and release tests

https://github.com/mikem23/koji-playground/commits/pagure/pr/1135

Looking at it, I wonder: Is glob matching sufficient here? Will folks need a version compare (e.g. greater than, less than)?

mikem commented 4 years ago

Is anyone opposed to the separate version and release tests as above? If not, I can merge that. I suppose that an ordered compare could be a future enhancement. The use case that started this doesn't need it.

mikem commented 4 years ago

Since my updated version is fairly different, I've filed it as a separate PR. Please review here:

https://pagure.io/koji/pull-request/1513

tkopecek commented 4 years ago

Dropping this one in favour of merged #1513

Pull-Request has been closed by tkopecek

4 years ago