PR#518: product_versions in remote rule can now be optional - greenwave

greenwave

#518 product_versions in remote rule can now be optional

Merged 4 years ago by vmaljulin. Opened 4 years ago by vmaljulin.

vmaljulin/greenwave FACTORY-4899 into master

product_versions in remote rule can now be optional

Valerij Maljulin • 4 years ago

0c1f57d

docs/package-specific-policies.rst

file modified

+3 -2

		`@@ -38,9 +38,10 @@`
		`configuration, with the only difference that the "id" key is optional.`

		``product_versions``, ``decision_context`` and ``subject_type`` in the
		`- gating.yaml file should match with the defined values in the global`
		+ :file:`gating.yaml` file should match with the defined values in the global
		policy defined in the Greenwave conf that contains the ``RemoteRule``
		`- that will enable this check.`
		+ that will enable this check. If ``product_versions`` are not specified
		+ in the remote :file:`gating.yaml`, values from the global policy will be used.

		The ``subject_type`` should always be defined and the permitted values are
		``koji_build``, ``redhat-module`` and ``redhat-container-image``.

greenwave/policies.py

file modified

+2 -2

		`@@ -458,7 +458,7 @@`
		`if isinstance(policy, OnDemandPolicy):`
		`return [`
		`sub_policy for sub_policy in policies`
		`- if set(sub_policy.product_versions) == set(policy.product_versions)`
		`+ if any(sub_policy.matches_product_version(pv) for pv in policy.product_versions)`
		`]`

		`return [`
		`@@ -758,7 +758,7 @@`

		`safe_yaml_attributes = {`
		`'id': SafeYAMLString(optional=True),`
		`- 'product_versions': SafeYAMLList(str),`
		`+ 'product_versions': SafeYAMLList(str, default=['*'], optional=True),`
		`'subject_type': SafeYAMLString(optional=True, default='koji_build'),`
		`'decision_context': SafeYAMLString(),`
		`'rules': SafeYAMLList(Rule),`

greenwave/safe_yaml.py

file modified

+5 -2

		`@@ -100,7 +100,10 @@`
		`"""`
		`YAML object attribute represeting a list of values.`
		`"""`
		`- def __init__(self, item_type, **kwargs):`
		`+ def __init__(self, item_type, default=None, **kwargs):`
		`+ if default is None:`
		`+ default = []`
		`+ self.default = default`
		`super().__init__(**kwargs)`
		`self.item_type = item_type`

		`@@ -114,7 +117,7 @@`

		`@property`
		`def default_value(self):`
		`- return []`
		`+ return self.default`

		`def to_json(self, value):`
		`return [self._item_to_json(item) for item in value]`

greenwave/tests/test_policies.py

file modified

+1 -5

		`@@ -493,12 +493,10 @@`
		`policy = policies[0]`

		`results = DummyResultsRetriever()`
		`- expected_details = "Policy 'untitled': Attribute 'product_versions' is required"`
		`decision = policy.check('fedora-26', subject, results)`
		`assert len(decision) == 1`
		`- assert isinstance(decision[0], InvalidGatingYaml)`
		`+ assert isinstance(decision[0], TestResultMissing)`
		`assert decision[0].is_satisfied is False`
		`- assert decision[0].details == expected_details`


		`def test_remote_rule_malformed_yaml(tmpdir):`
		`@@ -997,8 +995,6 @@`
		`remote_fragment = dedent("""`
		`--- !Policy`
		`id: "some-policy-from-a-random-packager"`
		`- product_versions:`
		`- - fedora-26`
		`decision_context: bodhi_update_push_stable_with_remoterule`
		`rules:`
		`- !PassingTestCaseRule {test_case_name: dist.upgradepath}`

vmaljulin commented 4 years ago

This fixes #468
JIRA: FACTORY-4899

Signed-off-by: Valerij Maljulin vmaljuli@redhat.com

lholecek commented on line 7 of greenwave/policies.py 4 years ago

This section should be part of RemotePolicy. Or maybe you can just use:

'product_versions': SafeYAMLList(str, optional=True, default=['*']),

lholecek commented 4 years ago

Can you also change the examples in documentation? (I.e. remove the product_versions there.)

rebased onto ef336214881f276f67c255724413e02bca0a8c78

4 years ago

rebased onto 30012b79f0089111c03b012678489e67c0ab94a9

4 years ago

vmaljulin commented 4 years ago

This section should be part of RemotePolicy. Or maybe you can just use:
'product_versions': SafeYAMLList(str, optional=True, default=['*']),

I've tried to make it multiple ways and it's not working.
So, let's keep it as it is.

vmaljulin commented 4 years ago

Can you also change the examples in documentation? (I.e. remove the product_versions there.)

Add a couple of sentences to docs.

Edited 4 years ago by vmaljulin

lholecek commented 4 years ago

This section should be part of RemotePolicy. Or maybe you can just use:
'product_versions': SafeYAMLList(str, optional=True, default=['*']),

I've tried to make it multiple ways and it's not working.
So, let's keep it as it is.

Oh, it's because SafeYAMLList doesn't support default argument. Can you add it?

Setting default product_versions value really shouldn't be handled outside the RemotePolicy class, it would lead to inconsistencies.

vmaljulin commented 4 years ago

This section should be part of RemotePolicy. Or maybe you can just use:
'product_versions': SafeYAMLList(str, optional=True, default=['*']),

I've tried to make it multiple ways and it's not working.
So, let's keep it as it is.

Oh, it's because SafeYAMLList doesn't support default argument. Can you add it?
Setting default product_versions value really shouldn't be handled outside the RemotePolicy class, it would lead to inconsistencies.

I've tried it, I've even tried to create a subclass just for a product version, but it's not working.

lucarval commented on line 12 of docs/package-specific-policies.rst 4 years ago

"values from the global policy will be used."

But more importantly, if gating.yaml defines product versions, then it will be used for matching.

lholecek commented 4 years ago

I've tried it, I've even tried to create a subclass just for a product version, but it's not working.

You need to add default argument to constructor of SafeYAMLList in "greenwave/safe_yaml.py" and return the value in the default_value() method. Is that what you have tried?

rebased onto 574a393110ade2dcfa56e4c8e4d8af8707c43fcd

4 years ago

2 new commits added