search

RedHatQE / newa

New Errata Workflow Automation.
Apache License 2.0
0 stars 3 forks source link

Implement Jira processor configuration logic #8

Closed kkaarreell closed 4 months ago

kkaarreell commented 6 months ago

Jira processor needs to know how to handle an erratum. For that there should be a configuration stored in a single or multiple YAML files.

Below is the proposed format but the actual implementation is up to a discussion, this is just something so we can starting the discussion.

Proposed file format for the Jira issue configuration file

covered use cases

not covered use cases

to improve

errata_base.yaml

issues:
 - summary: "Testing ER#{errata.id} {errata.summary}"
   description: "{errata.url}"
   assignee: '{errata.people_assigned_to}'
   type: epic
   id: errata_epic
   on_respin: keep
-  summary: "Errata respin {errata.respin_count}"
   description: "{errata.srpms}"
   assignee: '{errata.people_assigned_to}'
   type: task
   id: errata_task
   parent: errata_epic
   on_respin: close

errata_checklist.yaml

issues:
 - summary: "Errata filelist check"
   description: "Compare errata filelist with a previously released advisory"
   assignee: '{errata.people_assigned_to}'
   type: subtask
   id: subtask_filelist
   parent: errata_task
   on_respin: close
 - summary: "SPEC file review"
   description: "Review changes made in the SPEC file"
   assignee: '{errata.people_assigned_to}'
   type: subtask
   id: subtask_spec
   parent: errata_task
   on_respin: close
 - summary: "rpminspect review"
   description: "Review rpminspect results in the CI Dashboard for all builds"
   assignee: '{errata.people_assigned_to}'
   type: subtask
   id: subtask_rpminspect
   parent: errata_task
   on_respin: close

opencryptoki_ewa.yaml

project: BASEQESEC

filter:
  build: 'opencryptoki'

include:
 - url: https://path/to/the/errata_base.yaml
 - url: https://path/to/the/errata_checklist.yaml
   filter: !subtask_rpminspect

issues:
 - summary: 'Perform minimal sanity testing'
   description: 'Run minimal test plan to perform basic sanity testing on x86_64'
   assignee: '{errata.people_assigned_to}'
   type: subtask
   parent: errata_task
   on_respin: close
   recipe:
     url: 'https://path/to/recipe-minimal.yaml'
 - summary: 'Perform full regression testing'
   description: 'Run all tests on all supported architectures'
   assignee: '{errata.people_assigned_to}'
   type: subtask
   parent: errata_task
   on_respin: close
   recipe:
     url: 'https://path/to/recipe-full.yaml'
happz commented 6 months ago

There is an example file already merged, it was based on examples shown above, but there are small differences and it's not covering everything shown above. See https://github.com/RedHatQE/newa/blob/main/component-config.yaml.sample

Note the templating looks different, "Testing ER#{{ ERRATUM.event.id }} {{ ERRATUM.summary }}" vs "Testing ER#{errata.id} {errata.summary}" - newa is now using extremely common templating library, Jinja2, just like tmt and TF do, hence {{ ... }} and upper-case for variables, which is common practice in Jinja2 world.

Importing files and filtering has been omitted on purpose, to simplify the introduction of issue rules. We could go with restricted eval for the filtering, we have good experience with it from BaseOS CI and TF.

kkaarreell commented 6 months ago

After a bit of research I have found asteval module based on the ast parser. It seems to support basic operations, maybe we could combine ith with Jinja2 string, i.e. the condition would be written as a Jinja template so that variables would be substituted and later the resulting text would be processed by the parser.

>>> from asteval import Interpreter
>>> aeval = Interpreter()
>>> 
>>> txt = "1 in [1,2,3]"
>>> aeval.eval(txt)
True
>>> txt = "'abc' in 'eeabcee'"
>>> aeval.eval(txt)
True
>>> txt = "'eeabcee'.startswith('ee')"
>>> aeval.eval(txt)
True
>>> txt = "'eeabcee'.startswith('fee')"
>>> aeval.eval(txt)
False

For more info visit https://newville.github.io/asteval/basics.html

happz commented 6 months ago

After a bit of research I have found asteval module based on the ast parser.

Very similar to what we're used to from our gluetool modules. However, I'm afraid the package might not be available in EPEL.

maybe we could combine ith with Jinja2 string, i.e. the condition would be written as a Jinja template so that variables would be substituted and later the resulting text would be processed by the parser.

That's what I was thinking about. Saving the expression alone in the config file would be enough, a simple Jinja2 wrapper would then provide a result. For example:

 - summary: 'Perform full regression testing'
   description: 'Run all tests on all supported architectures'
   ...
   # The condition - just an example, but let's say this issue should exist for errata with even IDs:
   when: "ERRATUM.event.id % 2"
# Inject the `when` key into a template consisting of a single condition.
# If the expression gets evaluated as true, the template would render to a "true" string.
# For non-true expressions, we'd get an empty string
result = render_template(f'{{% if {action["when"]} %}}true{{% endif %}}', ERRATUM=erratum_job)

if result:
    ...
kkaarreell commented 6 months ago

Adding a bit more complicated example with asteval

>>> import jinja2
>>> from asteval import Interpreter
>>> aeval = Interpreter()
>>> data = { 'errata': { 'id': 12345, 'type': 'rpm', 'packages': ['gzip', 'tar'] }}
>>> condition = """ '{{ errata['type'] }}' == 'rpm' and 'tar' in {{ errata['packages'] }} """
>>> template = jinja2.Environment(loader=jinja2.BaseLoader).from_string(condition)
>>> expression = template.render(data)
>>> expression
" 'rpm' == 'rpm' and 'tar' in ['gzip', 'tar'] "
>>> aeval.eval(expression.strip())
True

I am aware that Jinja2 enables using conditionals, loops etc., although I do not have an experience with it. The ease of use should be probably considered. We may need to come up with a list of use stories (conditions a typical user would need) and assess the complexity.

happz commented 6 months ago

Adding a bit more complicated example with asteval

...

The Jinja2 example is more complicated than I'd say it's necessary:

'{{ errata['type'] }}' == 'rpm' and 'tar' in {{ errata['packages'] }}

In what I proposed, the condition could be simpler - just the condition, no Jinja controls:

errata['type'] == 'rpm' and 'tar' in errata['packages']

The condition itself would be "glued into" a dummy "if-else" template, and as such it does not have to contain any Jinja control characters. It's simply an expression one would use after any other if, does not need to be aware it'd be eventually injected into a Jinja template.

So, the complete example with your data and simpler expression:

>>> data = { 'errata': { 'id': 12345, 'type': 'rpm', 'packages': ['gzip', 'tar'] }}
>>> condition = """ errata['type'] == 'rpm' and 'tar' in errata['packages'] """

# There would be a small function defined in newa/__init__.py which takes the condition
# and returns whether it's true or not, by constructing a dummy if-else template, "gluing in"
# the condition:
>>> test_template = f"""{{% if {condition} %}}true{{% endif %}}"""
>>> test_template
"{% if  errata['type'] == 'rpm' and 'tar' in errata['packages']  %}true{% endif %}"

>>> jinja2.Environment(loader=jinja2.BaseLoader).from_string(test_template).render(data)
'true'
kkaarreell commented 6 months ago

That's good. I thought that the condition itself should follow some Jinja syntax and is limited by whatever Jinja supports. Does is mean the the condition can be almost any Python expression (i.e. it would be something like eval)?

kkaarreell commented 6 months ago

In addition to the issue configuration we would like to support (in the future):

project configuration, initially just a Jira project name. Later it could probably define some defaults. import configuration, enabling to (recursively, with some limit) import configuration from a different location, prepending the configuration in the actual config file.

That could look like:

import:
 - url: https://path/to/config/file
   when: """ similar condition to what Milos proposed for individual issues """
   issue_filter: """ some simple filter allowing to include/exclude issues based on their id """

project:
  name: TESTPROJECT

issues:
  ...
happz commented 6 months ago

That's good. I thought that the condition itself should follow some Jinja syntax and is limited by whatever Jinja supports. Does is mean the the condition can be almost any Python expression (i.e. it would be something like eval)?

"Almost any" is a fitting description. It's not a Python expression per se, it's Jinja2 after all, but it's very similar. For example, in Python , isinstance(foo, list) checks whether a variable is a list or not, Jinja writes this as foo is iterable. It's less developer-ish, more friendly to folks who are not used to Python syntax, like web developers. Which I think might be a plus in this case, as the audience of these config files would be generic developers and QEs, not necessarily having an experience with Python, and as such ERRATUM.id is even might seem easier to understand.

Plus, since we would own the environment when evaluating the condition, we would be able to add custom tests to simplify common checks, or to make them more readable. And, of course, we can add attributes to objects we own, so we might really simplify the test even more, e.g. to errata is rpm and errata.has_tar. Users then could focus on expression their needs without being too aware of internals of various objects, leaving us free from refactor them as needed without breaking user config file. errata is rpm or errata is container...

It would be helpful to write down a couple of examples of these conditions you foresee - or already needed - to better see whether the asteval & Python would be more helpful or Jinja

The ease of use should be probably considered. We may need to come up with a list of use stories (conditions a typical user would need) and assess the complexity.

Indeed, we should collect them and check what way would be better. asteval is limited to Python syntax, we should be able to add helper functions into its context, Jinja allows adding custom tests and its syntax is slightly different, closer to natural language.

kkaarreell commented 5 months ago

Dropping few use cases that I plan to utilize myself:

happz commented 5 months ago

Added a trivial bootstrap in https://github.com/RedHatQE/newa/pull/19.

I included a couple of examples based on https://github.com/RedHatQE/newa/issues/8#issuecomment-1993829222 - some of the proposals cannot be implemented yet, but I'm confident we will get them in the future. There is also a rudimentary test we would then convert to a proper unit test. Let me know what you think.