Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Jira processor configuration logic #8

Closed
kkaarreell opened this issue Feb 27, 2024 · 10 comments
Closed

Implement Jira processor configuration logic #8

kkaarreell opened this issue Feb 27, 2024 · 10 comments
Assignees

Comments

@kkaarreell
Copy link
Collaborator

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

  • parametrization
  • split configuration into multiple files
  • issue hierarchy
  • errata respin processing
  • connection with YAML recipe for automation
  • filtering wanted/unwanted issues using issue ID

not covered use cases

  • multiple releases and builds

to improve

  • poor errata-recipe matching, based on build (could be multiple builds, etc.)
  • issue filtering is only ID based

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
Copy link
Collaborator

happz commented Feb 27, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

happz commented Feb 28, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

happz commented Feb 28, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator

happz commented Feb 29, 2024

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
Copy link
Collaborator Author

Dropping few use cases that I plan to utilize myself:

  • Checking if event type equals to "errata", "compose", ...
  • Checking if errata artifact type equals to "rpm", "container" etc.
  • Checking if errata build list contains a specific package
  • Checking if errata number starts with (or contains or matches regexp) string "RHSA"
  • Checking if errata release starts with (or contains or matches regexp) string "rhel-x.y"
  • Checking if errata batch starts with (or contains or matches regexp) string "RHEL"
  • Negations of all checks above

@happz
Copy link
Collaborator

happz commented Mar 25, 2024

Added a trivial bootstrap in #19.

I included a couple of examples based on #8 (comment) - 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.

happz added a commit that referenced this issue Apr 3, 2024
happz added a commit that referenced this issue Apr 3, 2024
The-Mule pushed a commit to The-Mule/newa that referenced this issue Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants