Skip to content

Commit

Permalink
python: Support manifest conditions
Browse files Browse the repository at this point in the history
These pave the way for bundling packages together with the webserver,
and only showing them if the target system actually supports the
corresponding functionality. For example, CoreOS does not have libvirt,
so we can bundle but hide the "Machines" page on such systems.

This also allows us to better handle ostree vs. packagekit -- even if a
user installs cockpit-ostree on a standard RPM system, they should still
see the packagekit version of "Software Updates" [1]. This is a more
flexible approach than the old static `priority` field.

Take inspiration from systemd unit files, which support e.g.
`ConditionPathExists=`. The conditions should be declarative and cheap
to evaluate. Support file exists/not exists tests for now, which should
suffice for most use cases. The format and implementation is easy to
extend in the future.

Comment out the documentation for now. This is still considered
experimental, and it also is not implemented in the C bridge yet. Once
we switch over, we can enable this.

[1] cockpit-project/cockpit-ostree#295
  • Loading branch information
martinpitt authored and allisonkarlitskaya committed Feb 13, 2023
1 parent 173bb62 commit d713f61
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 0 deletions.
32 changes: 32 additions & 0 deletions doc/guide/packages.xml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ $ cockpit-bridge --packages
where there are conflicts. For example given two packages with the same
<code>name</code> a package is chosen based on its priority.</para></listitem>
</varlistentry>
<!-- TODO: enable this once moving to the Python bridge by default; also update example below
<varlistentry>
<term>conditions</term>
<listitem><para>An optional list of <code>{"predicate": "value"}</code> objects. Cockpit will only
consider the package if <emphasis>all</emphasis> conditions are met. Currently supported predicates
are <code>path-exists</code> and <code>path-not-exists</code>. Unknown predicates are ignored.
This is preferable to using <code>priority</code>, but only available since Cockpit FIXME-RELEASE.</para>
</listitem>
</varlistentry>
-->
<varlistentry>
<term>requires</term>
<listitem><para>An optional JSON object that contains a <code>"cockpit"</code>
Expand Down Expand Up @@ -252,6 +262,28 @@ $ cockpit-bridge --packages
}
</programlisting>

<!-- TODO: enable this once moving to the Python bridge by default
<programlisting>
{
"version": 0,
"require": {
"cockpit": "120"
},
"conditions": [
{"path-exists": "/usr/bin/mytool"},
{"path-exists": "/etc/mytool.conf"},
{"path-not-exist": "/etc/incompatible-tool"}
],
"tools": {
"mytool": {
"label": "My Tool",
"path": "tool.html"
}
}
}
</programlisting>
-->

</section>

<section id="package-manifest-override">
Expand Down
24 changes: 24 additions & 0 deletions src/cockpit/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,30 @@ def check(self, at_least_prio):
if self.manifest['priority'] <= at_least_prio:
return False

CONDITIONS = {
'path-exists': os.path.exists,
'path-not-exists': lambda p: not os.path.exists(p),
}

for condition in self.manifest.get('conditions', []):
try:
(predicate, value), = condition.items()
except (AttributeError, ValueError):
# ignore manifests with broken syntax
logger.warning('invalid condition in %s: %s', self.path, condition)
return False

try:
test_fn = CONDITIONS[predicate]
except KeyError:
# do *not* ignore manifests with unknown predicates, for forward compatibility
logger.warning('ignoring unknown predicate in %s: %s', self.path, predicate)
continue

if not test_fn(value):
logger.info('Hiding package %s as its %s condition is not met', self.path, condition)
return False

return True

def get_content_security_policy(self, origin):
Expand Down
50 changes: 50 additions & 0 deletions test/pytest/test_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,53 @@ def test_priority(self):
parsed = json.loads(packages.manifests)
assert parsed['basic'] == {'name': 'basic', 'description': 'VIP', 'priority': 100}
assert parsed['guest'] == {'description': 'Guest'}

def add_conditional_manifest(self, name, conditions):
(self.package_dir / name).mkdir()
(self.package_dir / name / 'manifest.json').write_text(f'{{"conditions": [ {conditions} ] }}')

def test_conditions(self):
self.add_conditional_manifest('empty', '')

# path-exists only
self.add_conditional_manifest('exists-1-yes', '{"path-exists": "/usr"}')
self.add_conditional_manifest('exists-1-no', '{"path-exists": "/nonexisting"}')
self.add_conditional_manifest('exists-2-yes', '{"path-exists": "/usr"}, {"path-exists": "/bin/sh"}')
self.add_conditional_manifest('exists-2-no', '{"path-exists": "/usr"}, {"path-exists": "/nonexisting"}')

# path-not-exists only
self.add_conditional_manifest('notexists-1-yes', '{"path-not-exists": "/nonexisting"}')
self.add_conditional_manifest('notexists-1-no', '{"path-not-exists": "/usr"}')
self.add_conditional_manifest('notexists-2-yes', '{"path-not-exists": "/nonexisting"}, {"path-not-exists": "/obscure"}')
self.add_conditional_manifest('notexists-2-no', '{"path-not-exists": "/nonexisting"}, {"path-not-exists": "/usr"}')

# mixed
self.add_conditional_manifest('mixed-yes', '{"path-exists": "/usr"}, {"path-not-exists": "/nonexisting"}')
self.add_conditional_manifest('mixed-no', '{"path-exists": "/nonexisting"}, {"path-not-exists": "/obscure"}')

packages = Packages()
assert set(packages.packages.keys()) == {
'basic', 'empty', 'exists-1-yes', 'exists-2-yes', 'notexists-1-yes', 'notexists-2-yes', 'mixed-yes'
}

def test_conditions_errors(self):
self.add_conditional_manifest('broken-syntax-1', '1')
self.add_conditional_manifest('broken-syntax-2', '["path-exists"]')
self.add_conditional_manifest('broken-syntax-3', '{"path-exists": "/foo", "path-not-exists": "/bar"}')

self.add_conditional_manifest('unknown-predicate-good', '{"path-exists": "/usr"}, {"frobnicated": true}')
self.add_conditional_manifest('unknown-predicate-bad', '{"path-exists": "/nonexisting"}, {"frobnicated": true}')

packages = Packages()
assert set(packages.packages.keys()) == {'basic', 'unknown-predicate-good'}

def test_condition_hides_priority(self):
(self.package_dir / 'vip').mkdir()
(self.package_dir / 'vip' / 'manifest.json').write_text(
'{"name": "basic", "description": "VIP", "priority": 100, "conditions": [{"path-exists": "/nonexisting"}]}')

packages = Packages()
assert packages.packages['basic'].name == 'basic'
assert packages.packages['basic'].manifest['description'] == 'standard package'
assert packages.packages['basic'].manifest['requires'] == {'cockpit': "42"}
assert packages.packages['basic'].priority == 1

0 comments on commit d713f61

Please sign in to comment.