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

pybridge: Implement cockpit-bridge --version; some small cleanups #19234

Merged
merged 4 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/man/cockpit-bridge.xml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@
available to the user running this command.</para>
</listitem>
</varlistentry>
<varlistentry>
<term><option>--version</option></term>
<listitem>
<para>Show Cockpit version information.</para>
</listitem>
</varlistentry>
</variablelist>
</refsect1>

Expand Down
16 changes: 10 additions & 6 deletions src/cockpit/bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@
import shlex
import socket
import subprocess
from typing import Iterable, List, Optional, Tuple, Type
from typing import Iterable, List, Optional, Sequence, Tuple, Type

from cockpit._vendor.ferny import interaction_client
from cockpit._vendor.systemd_ctypes import bus, run_async

from . import polyfills
from ._version import __version__
from .channel import ChannelRoutingRule
from .channels import CHANNEL_TYPES
from .config import Config, Environment
Expand Down Expand Up @@ -62,12 +63,12 @@ def export(self, path: str, obj: bus.BaseObject) -> None:
class Bridge(Router, PackagesListener):
internal_bus: InternalBus
packages: Optional[Packages]
bridge_rules: List[JsonObject]
martinpitt marked this conversation as resolved.
Show resolved Hide resolved
bridge_configs: Sequence[BridgeConfig]
args: argparse.Namespace

def __init__(self, args: argparse.Namespace):
self.internal_bus = InternalBus(EXPORTS)
self.bridge_rules = []
self.bridge_configs = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess () would be nicer here, but no biggie.

self.args = args

self.superuser_rule = SuperuserRoutingRule(self, privileged=args.privileged)
Expand Down Expand Up @@ -146,9 +147,10 @@ def do_send_init(self) -> None:
self.write_control(command='init', version=1, **init_args)

# PackagesListener interface
def packages_loaded(self):
def packages_loaded(self) -> None:
assert self.packages
bridge_configs = self.packages.get_bridge_configs()
if self.bridge_rules != bridge_configs:
if self.bridge_configs != bridge_configs:
self.superuser_rule.set_configs(bridge_configs)
self.peers_rule.set_configs(bridge_configs)
self.bridge_configs = bridge_configs
Expand Down Expand Up @@ -253,7 +255,6 @@ def main(*, beipack: bool = False) -> None:
parser.add_argument('--privileged', action='store_true', help='Privileged copy of the bridge')
parser.add_argument('--packages', action='store_true', help='Show Cockpit package information')
parser.add_argument('--bridges', action='store_true', help='Show Cockpit bridges information')
parser.add_argument('--rules', action='store_true', help='Show Cockpit bridge rules')
parser.add_argument('--debug', action='store_true', help='Enable debug output (very verbose)')
parser.add_argument('--version', action='store_true', help='Show Cockpit version information')
args = parser.parse_args()
Expand All @@ -275,6 +276,9 @@ def main(*, beipack: bool = False) -> None:
if args.packages:
Packages().show()
return
elif args.version:
print(f'Version: {__version__}\nProtocol: 1')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C bridge looks like this:

Version: 287
Protocol: 1
Payloads: dbus-json3 http-stream1 http-stream2 stream packet fsread1 
    fsreplace1 fswatch1 fslist1 null echo websocket-stream1 
Authorization: crypt1

The "Authorization: crypt1" is hardcoded in the C bridge, and thus not really useful. We could implement "Payloads:" if we care, but I doubt it's actually useful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason the inconsistency of how we get the two newlines printed out is bothering me. I will ignore that little nag, though. :)

return
elif args.bridges:
print(json.dumps(Packages().get_bridge_configs(), indent=2))
return
Expand Down
2 changes: 1 addition & 1 deletion src/cockpit/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class Document(NamedTuple):


class PackagesListener:
def packages_loaded(self):
def packages_loaded(self) -> None:
"""Called when the packages have been reloaded"""


Expand Down
16 changes: 16 additions & 0 deletions test/verify/check-connection
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,22 @@ UnixPath=/run/cockpit/session
b.wait_visible("#nav-system li:contains(Hackices)")
self.assertFalse(b.is_present("#nav-system li:contains(Services)"))

def testBridgeCLI(self):
m = self.machine

out = m.execute("cockpit-bridge --help")
# this needs to work with both the C and Python bridges
self.assertRegex(out, r"[uU]sage:")
self.assertIn("--packages", out)

version = m.execute("cockpit-bridge --version")
self.assertIn("Protocol: 1", version)
self.assertRegex(version, r"Version: \d{3,}")

packages = m.execute("cockpit-bridge --packages")
self.assertRegex(packages, r"(^|\n)base1\s+.*/usr/share/cockpit/base1")
self.assertRegex(packages, r"(^|\n)system\s+.*/usr/share/cockpit/systemd")


@testlib.skipDistroPackage()
class TestReverseProxy(testlib.MachineCase):
Expand Down