Skip to content

Commit

Permalink
Prevent Template Units id from being rendered when using Jinja2
Browse files Browse the repository at this point in the history
By default, templates using Jinja2 engine gets their id rendered as soon
as they get accessed, but since they don't have the appropriate
environment, the parameters are replaced with an empty string.

This is why when you run the command `checkbox-cli list template`, you
may see things like this:

    template 'com.canonical.certification::camera/still_'

instead of

    template 'com.canonical.certification::camera/still_{{ name }}'

Note that this does not happen with standard Python formatted strings:

    template 'com.canonical.certification::camera/led_{name}'

In order to make the behavior consistent regardless of the template
engine used, this commit:

- introduces a unit property that always returns "template" (similar to
how the Job unit always returns "job").
- modifies Unit.get_record_value() to prevent rendering using Jinja2
templating if the unit is a Checkbox Template

It also contains unit tests for both the TemplateUnit and its Validator.
  • Loading branch information
pieqq committed Jan 18, 2024
1 parent d8c38cf commit 81c9535
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 0 deletions.
4 changes: 4 additions & 0 deletions checkbox-ng/plainbox/impl/unit/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,13 @@ def __str__(self):
return "{} <~ {}".format(self.id, self.resource_id)

@property
def unit(self):
"""
The value of the unit field (overridden)
The return value is always "template"
"""
return "template"

@property
def resource_partial_id(self):
Expand Down
27 changes: 27 additions & 0 deletions checkbox-ng/plainbox/impl/unit/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@

class TemplateUnitTests(TestCase):

def test_id(self):
template = TemplateUnit({
"template-resource": "resource",
"template-id": "check-devices",
"id": "check-device-{dev_name}",
})
self.assertEqual(template.id, "check-device-{dev_name}")

def test_resource_partial_id__empty(self):
"""
Ensure that ``resource_partial_id`` defaults to None
Expand Down Expand Up @@ -351,6 +359,14 @@ def test_instantiate_all(self):

class TemplateUnitJinja2Tests(TestCase):

def test_id_jinja2(self):
template = TemplateUnit({
'template-resource': 'resource',
'template-engine': 'jinja2',
'id': 'check-device-{{ dev_name }}',
})
self.assertEqual(template.id, "check-device-{{ dev_name }}")

def test_instantiate_one_jinja2(self):
template = TemplateUnit({
'template-resource': 'resource',
Expand All @@ -374,6 +390,17 @@ class TemplateUnitFieldValidationTests(UnitFieldValidationTests):

unit_cls = TemplateUnit

def test_unit__present(self):
"""
TemplateUnit.unit always returns "template", the default error for the
base Unit class should never happen.
"""
issue_list = self.unit_cls({
}, provider=self.provider).check()
message = "field 'unit', unit should explicitly define its type"
self.assertIssueNotFound(issue_list, self.unit_cls.Meta.fields.unit,
Problem.missing, Severity.advice, message)

def test_template_unit__untranslatable(self):
issue_list = self.unit_cls({
# NOTE: the value must be a valid unit!
Expand Down
43 changes: 43 additions & 0 deletions checkbox-ng/plainbox/impl/unit/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,49 @@ def assertIssueFound(self, issue_list, field=None, kind=None,
'\n'.join(" - {!r}".format(issue) for issue in issue_list))
return self.fail(msg)

def assertIssueNotFound(self, issue_list, field=None, kind=None,
severity=None, message=None):
"""
Raise an assertion if no issue matching the provided criteria is found
:param issue_list:
A list of issues to look through
:param field:
(optional) value that must match the same attribute on the Issue
:param kind:
(optional) value that must match the same attribute on the Issue
:param severity:
(optional) value that must match the same attribute on the Issue
:param message:
(optional) value that must match the same attribute on the Issue
:returns:
The issue matching those constraints, if found
"""
for issue in issue_list:
if field is not None and issue.field is not field:
continue
if severity is not None and issue.severity is not severity:
continue
if kind is not None and issue.kind is not kind:
continue
if message is not None and issue.message != message:
continue
# return issue
return self.fail("Issue matching the given criteria found!")
msg = "Issue matching:\n{}\nwas found in:\n{}".format(
'\n'.join(
' * {} is {!r}'.format(issue_attr, value)
for issue_attr, value in
[('field', field),
('severity', severity),
('kind', kind),
('message', message)]
if value is not None),
'\n'.join(" - {!r}".format(issue) for issue in issue_list))
return self.fail(msg)
else:
return issue


class TestUnitDefinition(TestCase):

Expand Down
2 changes: 2 additions & 0 deletions checkbox-ng/plainbox/impl/unit/unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,7 @@ def get_record_value(self, name, default=None):
value is not None
and self.template_engine == "jinja2"
and not self.is_parametric
and not self.unit == "template"
):
tmp_params = {
"__checkbox_env__": self._checkbox_env(),
Expand Down Expand Up @@ -755,6 +756,7 @@ def get_raw_record_value(self, name, default=None):
value is not None
and self.template_engine == "jinja2"
and not self.is_parametric
and not self.unit == "template"
):
tmp_params = {
"__checkbox_env__": self._checkbox_env(),
Expand Down

0 comments on commit 81c9535

Please sign in to comment.