From d25757fba7d98b4535629de9dccf3f32b3e03487 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Thu, 29 Jun 2017 18:54:30 +0200 Subject: [refactor] use VPNManagment in composition instead of inheritance We make an ugly step for that moving the VPNStatus into VPNManagement. Let's iterate on it a second time to clean up that. - Related: #8942 --- src/leap/bitmask/vpn/_management.py | 79 ++++++++++++++++--------------------- src/leap/bitmask/vpn/_status.py | 16 ++++---- src/leap/bitmask/vpn/process.py | 42 +++++++++++--------- 3 files changed, 67 insertions(+), 70 deletions(-) (limited to 'src') diff --git a/src/leap/bitmask/vpn/_management.py b/src/leap/bitmask/vpn/_management.py index e86b3e5..c06c1d9 100644 --- a/src/leap/bitmask/vpn/_management.py +++ b/src/leap/bitmask/vpn/_management.py @@ -53,9 +53,7 @@ class VPNManagement(object): self._host = None self._port = None - self._last_state = None - self._last_status = None - self._status = None + self._watcher = None self._logs = {} def set_connection(self, host, port): @@ -75,24 +73,6 @@ class VPNManagement(object): def is_connected(self): return bool(self._tn) - def connect(self): - if not self._host or not self._port: - raise ImproperlyConfigured('Connection is not configured') - - try: - self._tn = UDSTelnet(self._host, self._port) - self._tn.read_eager() - - except Exception as e: - self.log.warn('Could not connect to OpenVPN yet: %r' % (e,)) - self._tn = None - - if self._tn: - return True - else: - self.log.error('Error while connecting to management!') - return False - def connect_retry(self, retry=0, max_retries=None): """ Attempts to connect to a management interface, and retries @@ -109,11 +89,29 @@ class VPNManagement(object): return if not self.aborted and not self.is_connected(): - self.connect() + self._connect() reactor.callLater( self.CONNECTION_RETRY_TIME, self.connect_retry, retry + 1, max_retries) + def _connect(self): + if not self._host or not self._port: + raise ImproperlyConfigured('Connection is not configured') + + try: + self._tn = UDSTelnet(self._host, self._port) + self._tn.read_eager() + + except Exception as e: + self.log.warn('Could not connect to OpenVPN yet: %r' % (e,)) + self._tn = None + + if self._tn: + return True + else: + self.log.error('Error while connecting to management!') + return False + def process_log(self): if not self._watcher or not self._tn: return @@ -191,10 +189,9 @@ class VPNManagement(object): self._tn.get_socket().close() self._tn = None - def _parse_state_and_notify(self, output): + def _parse_state(self, output): """ - Parses the output of the state command, and trigger a state transition - when the state changes. + Parses the output of the state command. :param output: list of lines that the state command printed as its output @@ -216,17 +213,11 @@ class VPNManagement(object): except ValueError: self.log.debug('Could not parse %s' % parts) - state = status_step - if state != self._last_state: - # XXX this status object is the vpn status observer - if self._status: - self._status.set_status(state, None) - self._last_state = state + return status_step - def _parse_status_and_notify(self, output): + def _parse_status(self, output): """ - Parses the output of the status command and emits - status_changed signal when the status changes. + Parses the output of the status command. :param output: list of lines that the status command printed as its output @@ -260,27 +251,25 @@ class VPNManagement(object): elif text == "TUN/TAP write bytes": tun_tap_write = value # upload - traffic_status = (tun_tap_read, tun_tap_write) - if traffic_status != self._last_status: - if self._status: - self._status.set_traffic_status(traffic_status) - self._last_status = traffic_status + return (tun_tap_read, tun_tap_write) def get_state(self): """ Notifies the gui of the output of the state command over the openvpn management interface. """ - if self.is_connected(): - return self._parse_state_and_notify(self._send_command("state")) + if not self.is_connected(): + return "" + return self._parse_state(self._send_command("state")) - def get_status(self): + def get_traffic_status(self): """ Notifies the gui of the output of the status command over the openvpn management interface. """ - if self.is_connected(): - return self._parse_status_and_notify(self._send_command("status")) + if not self.is_connected(): + return (None, None) + return self._parse_status(self._send_command("status")) def terminate(self, shutdown=False): """ @@ -380,7 +369,7 @@ class VPNManagement(object): port = cmdline[index + 2] self.log.debug("Trying to connect to %s:%s" % (host, port)) - self.connect() + self._connect() # XXX this has a problem with connections to different # remotes. So the reconnection will only work when we are diff --git a/src/leap/bitmask/vpn/_status.py b/src/leap/bitmask/vpn/_status.py index 6bd9c7c..5d41a53 100644 --- a/src/leap/bitmask/vpn/_status.py +++ b/src/leap/bitmask/vpn/_status.py @@ -74,9 +74,10 @@ class VPNStatus(object): elif status in self._CONNECTED: status = "on" - self._status = status - self.errcode = errcode - emit_async(catalog.VPN_STATUS_CHANGED) + if self._status != status: + self._status = status + self.errcode = errcode + emit_async(catalog.VPN_STATUS_CHANGED) def get_traffic_status(self): down = None @@ -87,7 +88,8 @@ class VPNStatus(object): up = bytes2human(self._traffic_up) return {'down': down, 'up': up} - def set_traffic_status(self, status): - up, down = status - self._traffic_up = up - self._traffic_down = down + def set_traffic_status(self, up, down): + if up != self._traffic_up or down != self._traffic_down: + self._traffic_up = up + self._traffic_down = down + emit_async(catalog.VPN_STATUS_CHANGED) diff --git a/src/leap/bitmask/vpn/process.py b/src/leap/bitmask/vpn/process.py index daf3576..aaf990c 100644 --- a/src/leap/bitmask/vpn/process.py +++ b/src/leap/bitmask/vpn/process.py @@ -29,8 +29,8 @@ from twisted.internet import error as internet_error from twisted.logger import Logger from leap.bitmask.vpn.utils import get_vpn_launcher -from leap.bitmask.vpn import _status -from leap.bitmask.vpn import _management +from leap.bitmask.vpn._status import VPNStatus +from leap.bitmask.vpn._management import VPNManagement from leap.bitmask.vpn.launchers import darwin from leap.bitmask.vpn.constants import IS_MAC, IS_LINUX @@ -40,7 +40,7 @@ from leap.bitmask.vpn.constants import IS_MAC, IS_LINUX OPENVPN_VERBOSITY = 1 -class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): +class _VPNProcess(protocol.ProcessProtocol): """ A ProcessProtocol class that can be used to spawn a process that will @@ -76,16 +76,15 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): openvpn invocation :type openvpn_verb: int """ - # TODO handle management as a component - _management.VPNManagement.__init__(self) - self.set_connection(socket_host, socket_port) + self._management = VPNManagement() + self._management.set_connection(socket_host, socket_port) + self._host = socket_host + self._port = socket_port self._vpnconfig = vpnconfig self._providerconfig = providerconfig self._launcher = get_vpn_launcher() - self._last_state = None - self._last_status = None self._alive = False # XXX use flags, maybe, instead of passing @@ -93,8 +92,8 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): self._openvpn_verb = openvpn_verb self._restartfun = restartfun - self._status = _status.VPNStatus() - self.set_watcher(self._status) + self._status = VPNStatus() + self._management.set_watcher(self._status) self.restarting = True self._remotes = remotes @@ -110,10 +109,6 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): def traffic_status(self): return self._status.get_traffic_status() - @traffic_status.setter - def traffic_status(self, value): - self._status.set_traffic_status(value) - # processProtocol methods def connectionMade(self): @@ -122,7 +117,7 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): """ self._alive = True self.aborted = False - self.connect_retry(max_retries=10) + self._management.connect_retry(max_retries=10) def outReceived(self, data): """ @@ -183,18 +178,20 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): Polls connection status. """ if self._alive: - self.get_status() + up, down = self._management.get_traffic_status() + self._status.set_traffic_status(up, down) def pollState(self): """ Polls connection state. """ if self._alive: - self.get_state() + state = self._management.get_state() + self._status.set_status(state, None) def pollLog(self): if self._alive: - self.process_log() + self._management.process_log() # launcher @@ -243,8 +240,17 @@ class _VPNProcess(protocol.ProcessProtocol, _management.VPNManagement): # filter out ports since we don't need that info return [gateway for gateway, port in gateways_ports] + def get_openvpn_process(self): + return self._management.get_openvpn_process() + # shutdown + def stop_if_already_running(self): + return self._management.stop_if_already_running() + + def terminate(self, shutdown=False): + self._management.terminate(shutdown) + def killProcess(self): """ Sends the KILL signal to the running process. -- cgit v1.2.3