diff --git a/landscape/client/broker/tests/test_transport.py b/landscape/client/broker/tests/test_transport.py index 7aaa3c66..dd65ba43 100644 --- a/landscape/client/broker/tests/test_transport.py +++ b/landscape/client/broker/tests/test_transport.py @@ -67,7 +67,7 @@ def request_with_payload(self, payload): payload, computer_id="34", exchange_token="abcd-efgh", - message_api="X.Y", + message_api=b"X.Y", ) def got_result(ignored): @@ -141,7 +141,7 @@ def test_ssl_verification_positive(self): transport.exchange, "HI", computer_id="34", - message_api="X.Y", + message_api=b"X.Y", ) def got_result(ignored): @@ -190,7 +190,7 @@ def test_ssl_verification_negative(self): transport.exchange, "HI", computer_id="34", - message_api="X.Y", + message_api=b"X.Y", ) def got_result(ignored): diff --git a/landscape/client/broker/transport.py b/landscape/client/broker/transport.py index e4c6e007..3b59a84c 100644 --- a/landscape/client/broker/transport.py +++ b/landscape/client/broker/transport.py @@ -1,18 +1,12 @@ """Low-level server communication.""" -import logging -import pprint -import time +from dataclasses import asdict import uuid - -import pycurl +from typing import Optional +from typing import Union from landscape import SERVER_API -from landscape import VERSION -from landscape.lib import bpickle -from landscape.lib.compat import _PY3 +from landscape.client.exchange import exchange_messages from landscape.lib.compat import unicode -from landscape.lib.fetch import fetch -from landscape.lib.format import format_delta class HTTPTransport: @@ -35,96 +29,37 @@ def set_url(self, url): """Set the URL of the remote message system.""" self._url = url - def _curl(self, payload, computer_id, exchange_token, message_api): - # There are a few "if _PY3" checks below, because for Python 3 we - # want to convert a number of values from bytes to string, before - # assigning them to the headers. - if _PY3 and isinstance(message_api, bytes): - message_api = message_api.decode("ascii") - headers = { - "X-Message-API": message_api, - "User-Agent": f"landscape-client/{VERSION}", - "Content-Type": "application/octet-stream", - } - if computer_id: - if _PY3 and isinstance(computer_id, bytes): - computer_id = computer_id.decode("ascii") - headers["X-Computer-ID"] = computer_id - if exchange_token: - if _PY3 and isinstance(exchange_token, bytes): - exchange_token = exchange_token.decode("ascii") - headers["X-Exchange-Token"] = str(exchange_token) - curl = pycurl.Curl() - return ( - curl, - fetch( - self._url, - post=True, - data=payload, - headers=headers, - cainfo=self._pubkey, - curl=curl, - ), - ) - def exchange( self, - payload, - computer_id=None, - exchange_token=None, - message_api=SERVER_API, - ): + payload: dict, + computer_id: Optional[str] = None, + exchange_token: Optional[str] = None, + message_api: bytes = SERVER_API, + ) -> Union[dict, None]: """Exchange message data with the server. - @param payload: The object to send, it must be L{bpickle}-compatible. - @param computer_id: The computer ID to send the message as (see - also L{Identity}). - @param exchange_token: The token that the server has given us at the - last exchange. It's used to prove that we are still the same - client. + :param payload: The object to send. It must be `bpickle`-compatible. + :param computer_id: The computer ID to send the message as. + :param exchange_token: Token included in the exchange to prove client - @type: C{dict} - @return: The server's response to sent message or C{None} in case - of error. - - @note: This code is thread safe (HOPEFULLY). + :return: The server's response to the sent message or `None` if there + was an error. + :note: This code is thread safe (HOPEFULLY). """ - spayload = bpickle.dumps(payload) - start_time = time.time() - if logging.getLogger().getEffectiveLevel() <= logging.DEBUG: - logging.debug("Sending payload:\n%s", pprint.pformat(payload)) try: - curly, data = self._curl( - spayload, - computer_id, - exchange_token, - message_api, - ) - except Exception: - logging.exception(f"Error contacting the server at {self._url}.") - raise - else: - logging.info( - "Sent %d bytes and received %d bytes in %s.", - len(spayload), - len(data), - format_delta(time.time() - start_time), + response = exchange_messages( + payload, + self._url, + cainfo=self._pubkey, + computer_id=computer_id, + exchange_token=exchange_token, + server_api=message_api.decode(), ) - - try: - response = bpickle.loads(data) except Exception: - logging.exception(f"Server returned invalid data: {data!r}") return None - else: - if logging.getLogger().getEffectiveLevel() <= logging.DEBUG: - logging.debug( - "Received payload:\n%s", - pprint.pformat(response), - ) - return response + return asdict(response) class FakeTransport: diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 3ddb80b3..a546fc86 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -5,38 +5,37 @@ """ import getpass import io +import logging import os import pwd import re import shlex import sys import textwrap -from functools import partial from urllib.parse import urlparse from landscape.client import GROUP from landscape.client import IS_SNAP from landscape.client import USER -from landscape.client.broker.amp import RemoteBrokerConnector from landscape.client.broker.config import BrokerConfiguration from landscape.client.broker.registration import Identity -from landscape.client.broker.registration import RegistrationError from landscape.client.broker.service import BrokerService -from landscape.client.reactor import LandscapeReactor +from landscape.client.registration import ClientRegistrationInfo +from landscape.client.registration import register +from landscape.client.registration import RegistrationException from landscape.client.serviceconfig import ServiceConfig from landscape.client.serviceconfig import ServiceConfigException from landscape.lib import base64 -from landscape.lib.amp import MethodCallError from landscape.lib.bootstrap import BootstrapDirectory from landscape.lib.bootstrap import BootstrapList from landscape.lib.compat import input from landscape.lib.fetch import fetch from landscape.lib.fetch import FetchError from landscape.lib.fs import create_binary_file +from landscape.lib.logging import init_app_logging from landscape.lib.network import get_fqdn from landscape.lib.persist import Persist from landscape.lib.tag import is_valid_tag -from landscape.lib.twisted_util import gather_results EXIT_NOT_REGISTERED = 5 @@ -631,13 +630,15 @@ def decode_base64_ssl_public_certificate(config): config.ssl_public_key = store_public_key_data(config, decoded_cert) -def setup(config): +def setup(config) -> Identity: """ Perform steps to ensure that landscape-client is correctly configured before we attempt to register it with a landscape server. If we are not configured to be silent then interrogate the user to provide necessary details for registration. + + :returns: a registration identity representing the configuration. """ bootstrap_tree(config) @@ -657,6 +658,15 @@ def setup(config): decode_base64_ssl_public_certificate(config) config.write() + persist = Persist( + filename=os.path.join( + config.data_path, + f"{BrokerService.service_name}.bpickle", + ), + ) + + return Identity(config, persist) + def restart_client(config): """Restart the client to ensure that it's using the new configuration.""" @@ -668,11 +678,6 @@ def restart_client(config): ServiceConfig.restart_landscape() except ServiceConfigException as exc: print_text(str(exc), error=True) - print_text( - "This machine will be registered with the provided " - "details when the client runs.", - error=True, - ) exit_code = 2 if config.ok_no_register: exit_code = 0 @@ -711,180 +716,42 @@ def store_public_key_data(config, certificate_data): return key_filename -def failure(add_result, reason=None): - """Handle a failed communication by recording the kind of failure.""" - if reason: - add_result(reason) - - -def exchange_failure(add_result, ssl_error=False): - """Handle a failed call by recording if the failure was SSL-related.""" - if ssl_error: - add_result("ssl-error") - else: - add_result("non-ssl-error") +def attempt_registration( + identity: Identity, + config: LandscapeSetupConfiguration, + retries: int = 14, +) -> int: + """Attempts to send a registration message, reporting the result to stdout + or stderr. Retries `retries` times. - -def handle_registration_errors(add_result, failure, connector): - """Handle registration errors. - - The connection to the broker succeeded but the registration itself - failed, because of invalid credentials or excessive pending computers. - We need to trap the exceptions so they don't stacktrace (we know what is - going on), and try to cleanly disconnect from the broker. - - Note: "results" contains a failure indication already (or will shortly) - since the registration-failed signal will fire.""" - error = failure.trap(RegistrationError, MethodCallError) - if error is RegistrationError: - add_result(str(failure.value)) - connector.disconnect() - - -def success(add_result): - """Handle a successful communication by recording the fact.""" - add_result("success") - - -def done(ignored_result, connector, reactor): - """Clean up after communicating with the server.""" - connector.disconnect() - reactor.stop() - - -def got_connection(add_result, connector, reactor, remote): - """Handle becoming connected to a broker.""" - handlers = { - "registration-done": partial(success, add_result), - "registration-failed": partial(failure, add_result), - "exchange-failed": partial(exchange_failure, add_result), - } - deferreds = [ - remote.call_on_event(handlers), - remote.register().addErrback( - partial(handle_registration_errors, add_result), - connector, - ), - ] - results = gather_results(deferreds) - results.addCallback(done, connector, reactor) - return results - - -def got_error(failure, reactor, add_result, print=print): - """Handle errors contacting broker.""" - print(failure.getTraceback(), file=sys.stderr) - # Can't just raise SystemExit; it would be ignored by the reactor. - add_result(SystemExit()) - reactor.stop() - - -def register( - config, - reactor=None, - connector_factory=RemoteBrokerConnector, - got_connection=got_connection, - max_retries=14, - on_error=None, - results=None, -): - """Instruct the Landscape Broker to register the client. - - The broker will be instructed to reload its configuration and then to - attempt a registration. - - @param reactor: The reactor to use. This parameter is optional because - the client charm does not pass it. - @param connector_factory: A callable that accepts a reactor and a - configuration object and returns a new remote broker connection. Used - primarily for dependency injection. - @param got_connection: The handler to trigger when the remote broker - connects. Used primarily for dependency injection. - @param max_retries: The number of times to retry connecting to the - landscape client service. The delay between retries is calculated - by Twisted and increases geometrically. - @param on_error: A callable that will be passed a non-zero positive - integer argument in the case that some error occurs. This is a legacy - API provided for use by the client charm. - @param results: This parameter provides a mechanism to pre-seed the result - of registering. Used for testing. + :returns: an exit code based on the registration result. """ - if reactor is None: - reactor = LandscapeReactor() + client_info = ClientRegistrationInfo.from_identity(identity) - if results is None: - results = [] - add_result = results.append + for retry in range(retries): + if retry > 0: + print(f"Retrying... (attempt {retry + 1} of {retries})") - connector = connector_factory(reactor, config) - connection = connector.connect(max_retries=max_retries, quiet=True) - connection.addCallback( - partial(got_connection, add_result, connector, reactor), - ) - connection.addErrback( - partial(got_error, reactor=reactor, add_result=add_result), - ) - reactor.run() - - assert len(results) == 1, "We expect exactly one result." - # Results will be things like "success" or "ssl-error". - result = results[0] - - # If there was an error and the caller requested that errors be reported - # to the on_error callable, then do so. - if result != "success" and on_error is not None: - on_error(1) - - if isinstance(result, SystemExit): - raise result - - set_secure_id(config, "registering") - - return result - - -def report_registration_outcome(what_happened, print=print): - """Report the registration interaction outcome to the user in - human-readable form. - """ - messages = { - "registration-skipped": "Registration skipped.", - "success": "Registration request sent successfully.", - "unknown-account": "Invalid account name or registration key.", - "max-pending-computers": ( - "Maximum number of computers pending approval reached. ", - "Login to your Landscape server account page to manage " - "pending computer approvals.", - ), - "ssl-error": ( - "\nThe server's SSL information is incorrect, or fails " - "signature verification!\n" - "If the server is using a self-signed certificate, " - "please ensure you supply it with the --ssl-public-key " - "parameter." - ), - "non-ssl-error": ( - "\nWe were unable to contact the server.\n" - "Your internet connection may be down. " - "The landscape client will continue to try and contact " - "the server periodically." - ), - } - message = messages.get(what_happened) - use_std_out = what_happened in {"success", "registration-skipped"} - if message: - fd = sys.stdout if use_std_out else sys.stderr - print(message, file=fd) + try: + registration_info = register(client_info, config.url) + break + except RegistrationException as e: + # This is unlikely to be resolved by the time we retry, so we fail + # immediately. + logging.error(f"Registration failed: {e}") + return 2 + except Exception: + # Retrying... + logging.exception("Registration failed") + else: + # We're finished retrying and haven't succeeded yet. + return 2 + set_secure_id(config, registration_info.secure_id) + print("Registration request sent successfully") + restart_client(config) -def determine_exit_code(what_happened): - """Return what the application's exit code should be depending on the - registration result. - """ - if what_happened in {"success", "registration-skipped"}: - return 0 - else: - return 2 # An error happened + return 0 def is_registered(config): @@ -950,7 +817,6 @@ def get_secure_id(config): def main(args, print=print): """Interact with the user and the server to set up client configuration.""" - config = LandscapeSetupConfiguration() try: config.load(args) @@ -958,6 +824,13 @@ def main(args, print=print): print_text(str(error), error=True) sys.exit(1) + init_app_logging( + config.log_dir, + config.log_level, + "landscape-config", + config.quiet + ) + if config.skip_registration and config.force_registration: sys.exit( "Do not set both skip registration " @@ -992,14 +865,13 @@ def main(args, print=print): # Setup client configuration. try: - setup(config) + identity = setup(config) except Exception as e: print_text(str(e)) sys.exit("Aborting Landscape configuration") if config.skip_registration: - result = "registration-skipped" - report_registration_outcome(result, print=print) + print("Registration skipped.") return should_register = False @@ -1017,17 +889,13 @@ def main(args, print=print): default=default_answer, ) - if should_register or config.silent: - restart_client(config) + exit_code = 0 if should_register: - # Attempt to register the client. - reactor = LandscapeReactor() - result = register( - config, - reactor, - on_error=lambda _: set_secure_id(config, None), - ) + exit_code = attempt_registration(identity, config) + restart_client(config) + elif config.silent: + restart_client(config) else: - result = "registration-skipped" - report_registration_outcome(result, print=print) - sys.exit(determine_exit_code(result)) + print("Registration skipped.") + + sys.exit(exit_code) diff --git a/landscape/client/deployment.py b/landscape/client/deployment.py index ef4a4fca..913f9666 100644 --- a/landscape/client/deployment.py +++ b/landscape/client/deployment.py @@ -7,6 +7,7 @@ from datetime import timezone from logging import debug from optparse import SUPPRESS_HELP +from typing import Sequence from twisted.logger import globalLogBeginner @@ -51,7 +52,7 @@ class BaseConfiguration(_BaseConfiguration): default_config_filename = DEFAULT_CONFIG if _is_script(): - default_config_filenames = ( + default_config_filenames: Sequence[str] = ( "landscape-client.conf", default_config_filename, ) diff --git a/landscape/client/exchange.py b/landscape/client/exchange.py new file mode 100644 index 00000000..b9301241 --- /dev/null +++ b/landscape/client/exchange.py @@ -0,0 +1,94 @@ +"""Utility functions for exchanging messages synchronously with a Landscape +Server instance. +""" +from dataclasses import dataclass +import logging +from pprint import pformat +import time +from typing import Any +from typing import Dict +from typing import List +from typing import Optional + +import pycurl + +from landscape import SERVER_API +from landscape import VERSION +from landscape.lib import bpickle +from landscape.lib.fetch import fetch +from landscape.lib.format import format_delta + + +@dataclass +class ServerResponse: + """The HTTP response from the server after a message exchange.""" + + server_uuid: str + messages: List[Dict[str, Any]] + + +def exchange_messages( + payload: dict, + server_url: str, + *, + cainfo: Optional[str] = None, + computer_id: Optional[str] = None, + exchange_token: Optional[str] = None, + server_api: str = SERVER_API.decode(), +) -> ServerResponse: + """Sends `payload` via HTTP(S) to `server_url`, parsing and returning the + response. + + :param payload: The object to send. It must be `bpickle`-compatible. + :param server_url: The URL to which the payload will be sent. + :param cainfo: Any additional certificate authority information to be used + to verify an HTTPS connection. + :param computer_id: The computer ID to send the message as. + :param exchange_token: Token included in the exchange to prove client + identity. + """ + start_time = time.time() + logging.debug(f"Sending payload:\n{pformat(payload)}") + + data = bpickle.dumps(payload) + headers = { + "X-Message-API": server_api, + "User-Agent": f"landscape-client/{VERSION}", + "Content-Type": "application/octet-stream", + } + + if computer_id: + headers["X-Computer-ID"] = computer_id + + if exchange_token: + headers["X-Exchange-Token"] = exchange_token + + curl = pycurl.Curl() + + try: + response_bytes = fetch( + server_url, + post=True, + data=data, + headers=headers, + cainfo=cainfo, + curl=curl, + ) + except Exception: + logging.exception(f"Error contacting the server at {server_url}.") + raise + + logging.info( + f"Sent {len(data)} bytes and received {len(response_bytes)} bytes in " + f"{format_delta(time.time() - start_time)}" + ) + + try: + response = bpickle.loads(response_bytes) + except Exception: + logging.exception(f"Server returned invalid data: {response_bytes!r}") + raise + + logging.debug(f"Received payload:\n{pformat(response)}") + + return ServerResponse(response["server-uuid"], response["messages"]) diff --git a/landscape/client/manager/plugin.py b/landscape/client/manager/plugin.py index f4ab1621..2f79144d 100644 --- a/landscape/client/manager/plugin.py +++ b/landscape/client/manager/plugin.py @@ -1,5 +1,6 @@ import logging from pathlib import Path +from typing import Optional from twisted.internet.defer import maybeDeferred @@ -82,7 +83,7 @@ class DataWatcherManager(ManagerPlugin): a get_data method """ - message_type = None + message_type: Optional[str] = None def __init__(self): super().__init__() diff --git a/landscape/client/registration.py b/landscape/client/registration.py new file mode 100644 index 00000000..9ecf013f --- /dev/null +++ b/landscape/client/registration.py @@ -0,0 +1,182 @@ +"""Utility functions for sending a registration message to a server. + +These are intended to be used directly and synchronously - without involving +other machinery, i.e. the Broker, and therefore exist outside of the usual +message exchange scheduling system. Callers are responsible for ensuring +exchange state is consistent when using these functions. +""" +from dataclasses import asdict +from dataclasses import dataclass +import json +from typing import Any +from typing import Dict +from typing import List +from typing import Optional +from typing import Tuple +from typing import Type +from typing import Union + +from landscape.client.broker.registration import Identity +from landscape.client.exchange import exchange_messages +from landscape.client.manager.ubuntuproinfo import get_ubuntu_pro_info + +from landscape.lib.fetch import HTTPCodeError +from landscape.lib.fetch import PyCurlError +from landscape.lib.network import get_fqdn +from landscape.lib.vm_info import get_container_info +from landscape.lib.vm_info import get_vm_info + + +@dataclass +class ClientRegistrationInfo: + """The information required by the server to register a client.""" + + access_group: str + account_name: str + computer_title: str + + container_info: Optional[str] = None + hostagent_uid: Optional[str] = None + hostname: Optional[str] = None + juju_info: None = None # We don't send Juju info currently. + registration_password: Optional[str] = None + tags: Optional[str] = None + ubuntu_pro_info: Optional[str] = None + vm_info: Optional[bytes] = None + + @classmethod + def from_identity( + cls: Type["ClientRegistrationInfo"], + identity: Identity, + ) -> "ClientRegistrationInfo": + return cls( + identity.access_group, + identity.account_name, + identity.computer_title, + container_info=get_container_info(), + hostagent_uid=identity.hostagent_uid, + hostname=get_fqdn(), + registration_password=identity.registration_key, + tags=identity.tags, + ubuntu_pro_info=json.dumps(get_ubuntu_pro_info()), + vm_info=get_vm_info(), + ) + + +class RegistrationException(Exception): + """Exception raised when registration fails for any reason.""" + + +@dataclass +class RegistrationInfo: + """The persistable information returned from the server after a + registration message is accepted. + """ + + insecure_id: int + secure_id: str + server_uuid: str + + +def register( + client_info: ClientRegistrationInfo, server_url: str +) -> RegistrationInfo: + """Sends a registration message to the server at `server_url`, returning + registration info if successful. + + :raises RegistrationException: if the registration fails for any reason. + """ + message = _create_message(client_info) + + try: + response = exchange_messages(message, server_url) + except HTTPCodeError as e: + if e.http_code == 404: + # Most likely cause is that we are trying to speak to a server with + # an API version that it does not support. + raise RegistrationException( + "\nWe were unable to contact the server or it is " + "an incompatible server version.\n" + "Please check your server URL and version." + ) from e + + raise # Other exceptions are unexpected and should propagate. + except PyCurlError as e: + if e.error_code == 60: + raise RegistrationException( + "\nThe server's SSL information is incorrect or fails " + "signature verification!\n" + "If the server is using a self-signed certificate, please " + "ensure you supply it with the --ssl-public-key parameter." + ) from e + + raise # Other exceptions are unexpected and should propagate. + + if not response.messages: + raise RegistrationException("No messages in registration response.") + + # Iterate over the response messages to extract the insecure and secure + # IDs. + client_ids = None + for response_message in response.messages: + client_ids = _handle_message(response_message) + + if client_ids is not None: + break + else: + raise RegistrationException( + "Did not receive ID information in registration response." + ) + + secure_id, insecure_id = client_ids + + return RegistrationInfo( + insecure_id=insecure_id, + secure_id=secure_id, + server_uuid=response.server_uuid, + ) + + +def _create_message( + client_info: ClientRegistrationInfo, +) -> Dict[str, List[Dict[str, Any]]]: + """Serializes `client_info` into a registration message suitable for + message exchange. Values that are `None` are stripped. + """ + message = {k: v for k, v in asdict(client_info).items() if v is not None} + message["type"] = "register" + + return {"messages": [message]} + + +def _handle_message(message: Dict[str, Any]) -> Union[Tuple[str, int], None]: + """Parses a single message in the server's response to the registration + message. + + :returns: Pair of insecure ID and secure ID for the registered client. + :raises RegistrationException: If the message implies registration did not + succeed. + """ + # Swap this out with (IMO) a reasonable `match` once we drop 3.8 support. + message_type = message.get("type") + + if message_type == "registration": + info = message.get("info") + + if info == "unknown-account": + raise RegistrationException( + "Invalid account name or registration key." + ) + elif info == "max-pending-computers": + raise RegistrationException( + "Maximum number of computers pending approval reached. " + "Log in to your Landscape server account page to manage " + "pending computer approvals." + ) + elif ( + message_type == "set-id" + and "id" in message and "insecure-id" in message + ): + return message["id"], message["insecure-id"] + + return None diff --git a/landscape/client/snap_utils.py b/landscape/client/snap_utils.py index 5772a519..77de9adf 100644 --- a/landscape/client/snap_utils.py +++ b/landscape/client/snap_utils.py @@ -60,7 +60,11 @@ def get_assertions(assertion_type: str): # signature # extract the assertion headers + their signatures as separate assertions + if not isinstance(response.result, bytes): + return + assertions = [] + result = response.result.decode() if result: sections = result.split("\n\n") diff --git a/landscape/client/tests/test_configuration.py b/landscape/client/tests/test_configuration.py index 3ca37493..0c160619 100644 --- a/landscape/client/tests/test_configuration.py +++ b/landscape/client/tests/test_configuration.py @@ -1,29 +1,16 @@ import os -import sys import textwrap import unittest -from functools import partial from unittest import mock -from twisted.internet.defer import Deferred -from twisted.internet.defer import fail from twisted.internet.defer import succeed from landscape.client.broker.registration import Identity -from landscape.client.broker.registration import RegistrationError from landscape.client.broker.tests.helpers import BrokerConfigurationHelper -from landscape.client.broker.tests.helpers import RemoteBrokerHelper from landscape.client.configuration import bootstrap_tree from landscape.client.configuration import ConfigurationError -from landscape.client.configuration import determine_exit_code -from landscape.client.configuration import done -from landscape.client.configuration import exchange_failure from landscape.client.configuration import EXIT_NOT_REGISTERED -from landscape.client.configuration import failure from landscape.client.configuration import get_secure_id -from landscape.client.configuration import got_connection -from landscape.client.configuration import got_error -from landscape.client.configuration import handle_registration_errors from landscape.client.configuration import ImportOptionError from landscape.client.configuration import is_registered from landscape.client.configuration import LandscapeSetupConfiguration @@ -31,19 +18,14 @@ from landscape.client.configuration import main from landscape.client.configuration import print_text from landscape.client.configuration import prompt_yes_no -from landscape.client.configuration import register from landscape.client.configuration import registration_info_text -from landscape.client.configuration import report_registration_outcome from landscape.client.configuration import restart_client from landscape.client.configuration import set_secure_id from landscape.client.configuration import setup from landscape.client.configuration import show_help from landscape.client.configuration import store_public_key_data -from landscape.client.configuration import success from landscape.client.serviceconfig import ServiceConfigException -from landscape.client.tests.helpers import FakeBrokerServiceHelper from landscape.client.tests.helpers import LandscapeTest -from landscape.lib.amp import MethodCallError from landscape.lib.compat import ConfigParser from landscape.lib.compat import StringIO from landscape.lib.fetch import HTTPCodeError @@ -71,182 +53,6 @@ def get_config(self, args, data_path=None): return config -class SuccessTests(unittest.TestCase): - def test_success(self): - """The success handler records the success.""" - results = [] - success(results.append) - self.assertEqual(["success"], results) - - -class FailureTests(unittest.TestCase): - def test_failure(self): - """The failure handler records the failure and returns non-zero.""" - results = [] - self.assertNotEqual(0, failure(results.append, "an-error")) - self.assertEqual(["an-error"], results) - - -class ExchangeFailureTests(unittest.TestCase): - def test_exchange_failure_ssl(self): - """The exchange_failure() handler records whether or not the failure - involved SSL or not and returns non-zero.""" - results = [] - self.assertNotEqual( - 0, - exchange_failure(results.append, ssl_error=True), - ) - self.assertEqual(["ssl-error"], results) - - def test_exchange_failure_non_ssl(self): - """ - The exchange_failure() handler records whether or not the failure - involved SSL or not and returns non-zero. - """ - results = [] - self.assertNotEqual( - 0, - exchange_failure(results.append, ssl_error=False), - ) - self.assertEqual(["non-ssl-error"], results) - - -class HandleRegistrationErrorsTests(unittest.TestCase): - def test_handle_registration_errors_traps(self): - """ - The handle_registration_errors() function traps RegistrationError - and MethodCallError errors. - """ - - class FauxFailure: - def trap(self, *trapped): - self.trapped_exceptions = trapped - - faux_connector = FauxConnector() - faux_failure = FauxFailure() - - results = [] - add_result = results.append - - self.assertNotEqual( - 0, - handle_registration_errors( - add_result, - faux_failure, - faux_connector, - ), - ) - self.assertTrue( - [RegistrationError, MethodCallError], - faux_failure.trapped_exceptions, - ) - - def test_handle_registration_errors_disconnects_cleanly(self): - """ - The handle_registration_errors function disconnects the broker - connector cleanly. - """ - - class FauxFailure: - def trap(self, *trapped): - pass - - faux_connector = FauxConnector() - faux_failure = FauxFailure() - - results = [] - add_result = results.append - - self.assertNotEqual( - 0, - handle_registration_errors( - add_result, - faux_failure, - faux_connector, - ), - ) - self.assertTrue(faux_connector.was_disconnected) - - def test_handle_registration_errors_as_errback(self): - """ - The handle_registration_errors functions works as an errback. - - This test was put in place to assert the parameters passed to the - function when used as an errback are in the correct order. - """ - faux_connector = FauxConnector() - calls = [] - - def i_raise(result): - calls.append(True) - return RegistrationError("Bad mojo") - - results = [] - add_result = results.append - - deferred = Deferred() - deferred.addCallback(i_raise) - deferred.addErrback( - partial(handle_registration_errors, add_result), - faux_connector, - ) - deferred.callback("") # This kicks off the callback chain. - - self.assertEqual([True], calls) - - -class DoneTests(unittest.TestCase): - def test_done(self): - """The done() function handles cleaning up.""" - - class FauxConnector: - was_disconnected = False - - def disconnect(self): - self.was_disconnected = True - - class FauxReactor: - was_stopped = False - - def stop(self): - self.was_stopped = True - - faux_connector = FauxConnector() - faux_reactor = FauxReactor() - - done(None, faux_connector, faux_reactor) - self.assertTrue(faux_connector.was_disconnected) - self.assertTrue(faux_reactor.was_stopped) - - -class GotErrorTests(unittest.TestCase): - def test_got_error(self): - """The got_error() function handles displaying errors and exiting.""" - - class FauxFailure: - def getTraceback(self): # noqa: N802 - return "traceback" - - results = [] - printed = [] - - def faux_print(text, file): - printed.append((text, file)) - - mock_reactor = mock.Mock() - - got_error( - FauxFailure(), - reactor=mock_reactor, - add_result=results.append, - print=faux_print, - ) - mock_reactor.stop.assert_called_once_with() - - self.assertIsInstance(results[0], SystemExit) - self.assertEqual([("traceback", sys.stderr)], printed) - - class PrintTextTest(LandscapeTest): @mock.patch("sys.stdout", new_callable=StringIO) def test_default(self, stdout): @@ -786,23 +592,17 @@ class ConfigurationFunctionsTest(LandscapeConfigurationTest): def setUp(self): super().setUp() - getuid_patcher = mock.patch("os.getuid", return_value=0) - bootstrap_tree_patcher = mock.patch( - "landscape.client.configuration.bootstrap_tree", - ) - set_secure_id_patch = mock.patch( - "landscape.client.configuration.set_secure_id", - ) - self.mock_getuid = getuid_patcher.start() - self.mock_bootstrap_tree = bootstrap_tree_patcher.start() - set_secure_id_patch.start() - def cleanup(): - getuid_patcher.stop() - bootstrap_tree_patcher.stop() - set_secure_id_patch.stop() + self.mock_getuid = mock.patch("os.getuid", return_value=0).start() + patches = mock.patch.multiple( + "landscape.client.configuration", + bootstrap_tree=mock.DEFAULT, + init_app_logging=mock.DEFAULT, + set_secure_id=mock.DEFAULT, + ).start() + self.mock_bootstrap_tree = patches["bootstrap_tree"] - self.addCleanup(cleanup) + self.addCleanup(mock.patch.stopall) def get_content(self, config): """Write C{config} to a file and return it's contents as a string.""" @@ -1269,7 +1069,7 @@ def test_setup_prefers_proxies_from_config_over_environment( @mock.patch("landscape.client.configuration.sys.exit") @mock.patch("landscape.client.configuration.input", return_value="n") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.setup") def test_main_no_registration( self, @@ -1286,7 +1086,7 @@ def test_main_no_registration( @mock.patch("landscape.client.configuration.sys.exit") @mock.patch("landscape.client.configuration.input") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.setup") def test_skip_registration( self, @@ -1306,7 +1106,7 @@ def test_skip_registration( mock_register.assert_not_called() mock_input.assert_not_called() - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.setup") def test_main_no_registration_silent(self, mock_setup, mock_register): """Skip registration works in silent mode""" @@ -1324,7 +1124,7 @@ def test_main_no_registration_silent(self, mock_setup, mock_register): @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.sys.exit") @mock.patch("landscape.client.configuration.input") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.setup") def test_main_force_registration_silent( self, @@ -1351,8 +1151,8 @@ def test_main_force_registration_silent( @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") @mock.patch( - "landscape.client.configuration.register", - return_value="success", + "landscape.client.configuration.attempt_registration", + return_value=0, ) @mock.patch("landscape.client.configuration.setup") def test_main_register_if_needed_silent( @@ -1372,7 +1172,6 @@ def test_main_register_if_needed_silent( "--register-if-needed", "--silent", ], - print=noop_print, ) self.assertEqual(0, system_exit.code) mock_restart_client.assert_called_once() @@ -1385,7 +1184,7 @@ def test_main_register_if_needed_silent( ) @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.setup") def test_main_do_not_register_if_not_needed_silent( self, @@ -1414,8 +1213,8 @@ def test_main_do_not_register_if_not_needed_silent( @mock.patch("landscape.client.configuration.restart_client") @mock.patch( - "landscape.client.configuration.register", - return_value="success", + "landscape.client.configuration.attempt_registration", + return_value=0, ) @mock.patch("landscape.client.configuration.setup") def test_main_silent(self, mock_setup, mock_register, mock_restart_client): @@ -1434,7 +1233,6 @@ def test_main_silent(self, mock_setup, mock_register, mock_restart_client): SystemExit, main, ["-c", config_filename, "--silent"], - print=noop_print, ) self.assertEqual(0, exception.code) mock_setup.assert_called_once() @@ -1444,8 +1242,8 @@ def test_main_silent(self, mock_setup, mock_register, mock_restart_client): @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input", return_value="y") @mock.patch( - "landscape.client.configuration.register", - return_value="success", + "landscape.client.configuration.attempt_registration", + return_value=0, ) @mock.patch("landscape.client.configuration.setup") def test_main_user_interaction_success( @@ -1456,16 +1254,10 @@ def test_main_user_interaction_success( mock_restart_client, ): """The successful result of register() is communicated to the user.""" - printed = [] - - def faux_print(string, file=sys.stdout): - printed.append((string, file)) - exception = self.assertRaises( SystemExit, main, ["-c", self.make_working_config()], - print=faux_print, ) self.assertEqual(0, exception.code) mock_setup.assert_called_once() @@ -1474,18 +1266,12 @@ def faux_print(string, file=sys.stdout): mock_input.assert_called_once_with( "\nRequest a new registration for this computer now? [Y/n]: ", ) - self.assertEqual( - [ - ("Registration request sent successfully.", sys.stdout), - ], - printed, - ) @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input", return_value="y") @mock.patch( - "landscape.client.configuration.register", - return_value="unknown-account", + "landscape.client.configuration.attempt_registration", + return_value=2, ) @mock.patch("landscape.client.configuration.setup") def test_main_user_interaction_failure( @@ -1496,16 +1282,10 @@ def test_main_user_interaction_failure( mock_restart_client, ): """The failed result of register() is communicated to the user.""" - printed = [] - - def faux_print(string, file=sys.stdout): - printed.append((string, file)) - exception = self.assertRaises( SystemExit, main, ["-c", self.make_working_config()], - print=faux_print, ) self.assertEqual(2, exception.code) mock_setup.assert_called_once() @@ -1515,19 +1295,11 @@ def faux_print(string, file=sys.stdout): "\nRequest a new registration for this computer now? [Y/n]: ", ) - # Note that the error is output via sys.stderr. - self.assertEqual( - [ - ("Invalid account name or registration key.", sys.stderr), - ], - printed, - ) - @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") @mock.patch( - "landscape.client.configuration.register", - return_value="success", + "landscape.client.configuration.attempt_registration", + return_value=0, ) @mock.patch("landscape.client.configuration.setup") def test_main_user_interaction_success_silent( @@ -1538,16 +1310,10 @@ def test_main_user_interaction_success_silent( mock_restart_client, ): """Successful result is communicated to the user even with --silent.""" - printed = [] - - def faux_print(string, file=sys.stdout): - printed.append((string, file)) - exception = self.assertRaises( SystemExit, main, ["--silent", "-c", self.make_working_config()], - print=faux_print, ) self.assertEqual(0, exception.code) mock_setup.assert_called_once() @@ -1555,18 +1321,11 @@ def faux_print(string, file=sys.stdout): mock_register.assert_called_once() mock_input.assert_not_called() - self.assertEqual( - [ - ("Registration request sent successfully.", sys.stdout), - ], - printed, - ) - @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") @mock.patch( - "landscape.client.configuration.register", - return_value="unknown-account", + "landscape.client.configuration.attempt_registration", + return_value=2, ) @mock.patch("landscape.client.configuration.setup") def test_main_user_interaction_failure_silent( @@ -1579,29 +1338,16 @@ def test_main_user_interaction_failure_silent( """ A failure result is communicated to the user even with --silent. """ - printed = [] - - def faux_print(string, file=sys.stdout): - printed.append((string, file)) - exception = self.assertRaises( SystemExit, main, ["--silent", "-c", self.make_working_config()], - print=faux_print, ) self.assertEqual(2, exception.code) mock_setup.assert_called_once() mock_restart_client.assert_called_once() mock_register.assert_called_once() mock_input.assert_not_called() - # Note that the error is output via sys.stderr. - self.assertEqual( - [ - ("Invalid account name or registration key.", sys.stderr), - ], - printed, - ) def make_working_config(self): data_path = self.makeFile() @@ -1617,7 +1363,7 @@ def make_working_config(self): ) @mock.patch("landscape.client.configuration.input", return_value="") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.LandscapeSetupScript") @mock.patch("landscape.client.configuration.ServiceConfig") def test_register_system_exit( @@ -1669,11 +1415,6 @@ def test_errors_from_restart_landscape( "Couldn't restart the Landscape client.", error=True, ) - mock_print_text.assert_called_with( - "This machine will be registered with the provided details when " - "the client runs.", - error=True, - ) @mock.patch("landscape.client.configuration.print_text") @mock.patch("landscape.client.configuration.ServiceConfig") @@ -1702,15 +1443,10 @@ def test_errors_from_restart_landscape_ok_no_register( "Couldn't restart the Landscape client.", error=True, ) - mock_print_text.assert_called_with( - "This machine will be registered with the provided details when " - "the client runs.", - error=True, - ) @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input", return_value="") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.setup") def test_main_with_register( self, @@ -1734,7 +1470,7 @@ def test_main_with_register( @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch("landscape.client.configuration.setup") def test_register_silent( self, @@ -1759,7 +1495,7 @@ def test_register_silent( mock_input.assert_not_called() @mock.patch("landscape.client.configuration.input") - @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.attempt_registration") @mock.patch( "landscape.client.configuration.stop_client_and_disable_init_script", ) @@ -2438,48 +2174,6 @@ def disconnect(self): return succeed(None) -class RegisterRealFunctionTest(LandscapeConfigurationTest): - - helpers = [FakeBrokerServiceHelper] - - def setUp(self): - super().setUp() - self.config = LandscapeSetupConfiguration() - self.config.load(["-c", self.config_filename]) - - def test_register_success(self): - self.reactor.call_later(0, self.reactor.fire, "registration-done") - connector_factory = FakeConnectorFactory(self.remote) - result = register( - self.config, - self.reactor, - connector_factory, - max_retries=99, - ) - self.assertEqual("success", result) - - def test_register_registration_error(self): - """ - If we get a registration error, the register() function returns the - error from the registration response message. - """ - self.reactor.call_later(0, self.reactor.fire, "registration-failed") - - def fail_register(): - return fail(RegistrationError("max-pending-computers")) - - self.remote.register = fail_register - - connector_factory = FakeConnectorFactory(self.remote) - result = register( - config=self.config, - reactor=self.reactor, - connector_factory=connector_factory, - max_retries=99, - ) - self.assertEqual("max-pending-computers", result) - - class FauxConnection: def __init__(self): self.callbacks = [] @@ -2509,252 +2203,6 @@ def disconnect(self): self.was_disconnected = True -class RegisterFunctionTest(LandscapeConfigurationTest): - - helpers = [RemoteBrokerHelper] - - def setUp(self): - super().setUp() - self.config = LandscapeSetupConfiguration() - self.config.load(["-c", self.config_filename]) - - def test_register(self): - """Is the async machinery wired up properly?""" - - class FauxFailure: - def getTraceback(self): # noqa: N802 - return "traceback" - - class FauxReactor: - def run(self): - self.was_run = True - - def stop(self, *args): - self.was_stopped = True - - reactor = FauxReactor() - connector = FauxConnector(reactor, self.config) - - def connector_factory(reactor, config): - return connector - - # We pre-seed a success because no actual result will be generated. - register( - self.config, - reactor, - connector_factory, - max_retries=99, - results=["success"], - ) - self.assertTrue(reactor.was_run) - # Only a single callback is registered, it does the real work when a - # connection is established. - self.assertTrue(1, len(connector.connection.callbacks)) - self.assertEqual( - "got_connection", - connector.connection.callbacks[0].func.__name__, - ) - # Should something go wrong, there is an error handler registered. - self.assertTrue(1, len(connector.connection.errbacks)) - self.assertEqual( - "got_error", - connector.connection.errbacks[0].func.__name__, - ) - # We ask for retries because networks aren't reliable. - self.assertEqual(99, connector.max_retries) - - def test_register_got_error(self): - """If there is an error from the connection, raises `SystemExit`.""" - reactor = mock.Mock() - connector_factory = mock.Mock() - results = [] - - def add_error(): - results.append(SystemExit()) - - reactor.run.side_effect = add_error - - self.assertRaises( - SystemExit, - register, - self.config, - reactor, - connector_factory, - max_retries=2, - results=results, - ) - - @mock.patch("landscape.client.configuration.LandscapeReactor") - def test_register_without_reactor(self, mock_reactor): - """If no reactor is passed, a LandscapeReactor will be instantiated. - - This behaviour is exclusively for compatability with the client charm - which does not pass in a reactor. - """ - - def connector_factory(reactor, config): - return FauxConnector(reactor, self.config) - - # We pre-seed a success because no actual result will be generated. - register( - self.config, - connector_factory=connector_factory, - results=["success"], - ) - mock_reactor.assert_called_once_with() - mock_reactor().run.assert_called_once_with() - - def test_got_connection(self): - """got_connection() adds deferreds and callbacks.""" - - def faux_got_connection(add_result, remote, connector, reactor): - pass - - class FauxRemote: - handlers = None - deferred = None - - def call_on_event(self, handlers): - assert not self.handlers, "Called twice" - self.handlers = handlers - self.call_on_event_deferred = FauxCallOnEventDeferred() - return self.call_on_event_deferred - - def register(self): - assert not self.deferred, "Called twice" - self.register_deferred = FauxRegisterDeferred() - return self.register_deferred - - class FauxCallOnEventDeferred: - def __init__(self): - self.callbacks = [] - self.errbacks = [] - - def addCallbacks(self, *funcs, **kws): # noqa: N802 - self.callbacks.extend(funcs) - - class FauxRegisterDeferred: - def __init__(self): - self.callbacks = [] - self.errbacks = [] - - def addCallback(self, func): # noqa: N802 - assert func.__name__ == "got_connection", "Wrong callback." - self.callbacks.append(faux_got_connection) - self.gather_results_deferred = GatherResultsDeferred() - return self.gather_results_deferred - - def addCallbacks(self, *funcs, **kws): # noqa: N802 - self.callbacks.extend(funcs) - - def addErrback(self, func, *args, **kws): # noqa: N802 - self.errbacks.append(func) - return self - - class GatherResultsDeferred: - def __init__(self): - self.callbacks = [] - self.errbacks = [] - - def addCallbacks(self, *funcs, **kws): # noqa: N802 - self.callbacks.extend(funcs) - - faux_connector = FauxConnector(self.reactor, self.config) - - status_results = [] - faux_remote = FauxRemote() - results = got_connection( - status_results.append, - faux_connector, - self.reactor, - faux_remote, - ) - # We set up two deferreds, one for the RPC call and one for event - # handlers. - self.assertEqual(2, len(results.resultList)) - # Handlers are registered for the events we are interested in. - self.assertCountEqual( - ["registration-failed", "exchange-failed", "registration-done"], - faux_remote.handlers.keys(), - ) - self.assertCountEqual( - ["failure", "exchange_failure", "success"], - [ - handler.func.__name__ - for handler in faux_remote.handlers.values() - ], - ) - # We include a single error handler to react to exchange errors. - self.assertTrue(1, len(faux_remote.register_deferred.errbacks)) - # the handle_registration_errors is wrapped in a partial() - self.assertEqual( - "handle_registration_errors", - faux_remote.register_deferred.errbacks[0].func.__name__, - ) - - def test_register_with_on_error_and_an_error(self): - """A caller-provided on_error callable will be called if errors occur. - - The on_error parameter is provided for the client charm which calls - register() directly and provides on_error as a keyword argument. - """ - - def faux_got_connection(add_result, remote, connector, reactor): - add_result("something bad") - - on_error_was_called = [] - - def on_error(status): - # A positive number is provided for the status. - self.assertGreater(status, 0) - on_error_was_called.append(True) - - self.reactor.call_later(1, self.reactor.stop) - register( - self.config, - reactor=self.reactor, - on_error=on_error, - got_connection=faux_got_connection, - ) - self.assertTrue(on_error_was_called) - - def test_register_with_on_error_and_no_error(self): - """Caller-provided on_error callable will not be called if no error.""" - - def faux_got_connection(add_result, remote, connector, reactor): - add_result("success") - - on_error_was_called = [] - - def on_error(status): - on_error_was_called.append(True) - - self.reactor.call_later(1, self.reactor.stop) - register( - self.config, - reactor=self.reactor, - on_error=on_error, - got_connection=faux_got_connection, - ) - self.assertFalse(on_error_was_called) - - def test_register_happy_path(self): - """A successful result provokes no exceptions.""" - - def faux_got_connection(add_result, remote, connector, reactor): - add_result("success") - - self.reactor.call_later(1, self.reactor.stop) - self.assertEqual( - "success", - register( - self.config, - reactor=self.reactor, - got_connection=faux_got_connection, - ), - ) - - class SSLCertificateDataTest(LandscapeConfigurationTest): @mock.patch("landscape.client.configuration.print_text") def test_store_public_key_data(self, mock_print_text): @@ -2780,117 +2228,6 @@ def test_store_public_key_data(self, mock_print_text): ) -class ReportRegistrationOutcomeTest(unittest.TestCase): - def setUp(self): - self.result = [] - self.output = [] - - def record_result(self, result, file=sys.stdout): - self.result.append(result) - self.output.append(file.name) - - def test_success_case(self): - report_registration_outcome("success", print=self.record_result) - self.assertIn("Registration request sent successfully.", self.result) - self.assertIn(sys.stdout.name, self.output) - - def test_registration_skipped_case(self): - report_registration_outcome( - "registration-skipped", - print=self.record_result, - ) - self.assertIn("Registration skipped.", self.result) - self.assertIn(sys.stdout.name, self.output) - - def test_unknown_account_case(self): - """ - If the unknown-account error is found, an appropriate message is - returned. - """ - report_registration_outcome( - "unknown-account", - print=self.record_result, - ) - self.assertIn("Invalid account name or registration key.", self.result) - self.assertIn(sys.stderr.name, self.output) - - def test_max_pending_computers_case(self): - """ - If the max-pending-computers error is found, an appropriate message is - returned. - """ - report_registration_outcome( - "max-pending-computers", - print=self.record_result, - ) - messages = ( - "Maximum number of computers pending approval reached. ", - "Login to your Landscape server account page to manage pending " - "computer approvals.", - ) - self.assertIn(messages, self.result) - self.assertIn(sys.stderr.name, self.output) - - def test_ssl_error_case(self): - report_registration_outcome("ssl-error", print=self.record_result) - self.assertIn( - ( - "\nThe server's SSL information is incorrect, or fails " - "signature verification!\n" - "If the server is using a self-signed certificate, " - "please ensure you supply it with the --ssl-public-key " - "parameter." - ), - self.result, - ) - self.assertIn(sys.stderr.name, self.output) - - def test_non_ssl_error_case(self): - report_registration_outcome("non-ssl-error", print=self.record_result) - self.assertIn( - ( - "\nWe were unable to contact the server.\n" - "Your internet connection may be down. " - "The landscape client will continue to try and contact " - "the server periodically." - ), - self.result, - ) - self.assertIn(sys.stderr.name, self.output) - - -class DetermineExitCodeTest(unittest.TestCase): - def test_success_means_exit_code_0(self): - """ - When passed "success" the determine_exit_code function returns 0. - """ - result = determine_exit_code("success") - self.assertEqual(0, result) - - def test_registration_skipped_means_exit_code_0(self): - """ - When passed "registration-skipped" the determine_exit_code - function returns 0. - """ - result = determine_exit_code("registration-skipped") - self.assertEqual(0, result) - - def test_a_failure_means_exit_code_2(self): - """ - When passed a failure result, the determine_exit_code function returns - 2. - """ - failure_codes = [ - "unknown-account", - "max-computers-count", - "ssl-error", - "non-ssl-error", - ] - for code in failure_codes: - result = determine_exit_code(code) - self.assertEqual(2, result) - - class IsRegisteredTest(LandscapeTest): helpers = [BrokerConfigurationHelper] @@ -2921,6 +2258,7 @@ class RegistrationInfoTest(LandscapeTest): def setUp(self): super().setUp() + self.custom_args = ["hello.py"] # Fake python script name self.account_name = "world" self.data_path = self.makeDir() @@ -2936,6 +2274,10 @@ def setUp(self): ), ) + mock.patch("landscape.client.configuration.init_app_logging").start() + + self.addCleanup(mock.patch.stopall) + def test_not_registered(self): """False when client is not registered""" config_filename = self.config.default_config_filenames[0] diff --git a/landscape/client/tests/test_exchange.py b/landscape/client/tests/test_exchange.py new file mode 100644 index 00000000..36e64e3b --- /dev/null +++ b/landscape/client/tests/test_exchange.py @@ -0,0 +1,107 @@ +"""Tests for the `landscape.client.exchange` utility functions.""" +from unittest import TestCase +from unittest import mock + +from landscape import SERVER_API +from landscape import VERSION +from landscape.lib import bpickle + +from landscape.client.exchange import exchange_messages + + +class ExchangeMessagesTestCase(TestCase): + """Tests for the `exchange_messages` function.""" + + def setUp(self): + super().setUp() + + self.fetch_mock = mock.patch("landscape.client.exchange.fetch").start() + self.logging_mock = mock.patch( + "landscape.client.exchange.logging" + ).start() + + self.addCleanup(mock.patch.stopall) + + def test_success(self): + """A successful exchange results in a response and appropriate + logging. + """ + payload = {"messages": [{"type": "my-message-type", "some-value": 5}]} + + mock_response = { + "server-uuid": "my-server-uuid", + "messages": [{"type": "my-server-message-type", "other-value": 6}] + } + + self.fetch_mock.return_value = bpickle.dumps(mock_response) + + server_response = exchange_messages( + payload, + "https://my-server.local/message-system", + cainfo="mycainfo", + computer_id="my-secure-id", + exchange_token="my-exchange-token", + ) + + self.assertEqual(server_response.server_uuid, "my-server-uuid") + self.assertEqual( + server_response.messages, + [{"type": "my-server-message-type", "other-value": 6}] + ) + self.fetch_mock.assert_called_once_with( + "https://my-server.local/message-system", + post=True, + data=bpickle.dumps(payload), + headers={ + "X-Message-API": SERVER_API.decode(), + "User-Agent": f"landscape-client/{VERSION}", + "Content-Type": "application/octet-stream", + "X-Computer-ID": "my-secure-id", + "X-Exchange-Token": "my-exchange-token", + }, + cainfo="mycainfo", + curl=mock.ANY, + ) + self.assertEqual(self.logging_mock.debug.call_count, 2) + self.logging_mock.info.assert_called_once() + self.logging_mock.exception.assert_not_called() + + def test_fetch_exception(self): + """If the HTTP `fetch` raises an exception, it is logged and raised.""" + payload = {"messages": [{"type": "my-message-type", "some-value": 6}]} + + self.fetch_mock.side_effect = Exception("OOPS") + + with self.assertRaises(Exception) as exc_context: + exchange_messages( + payload, + "https://my-server.local/message-system" + ) + + self.assertIn("OOPS", str(exc_context.exception)) + self.fetch_mock.assert_called_once() + self.logging_mock.debug.assert_called_once() + self.logging_mock.exception.assert_called_once_with( + "Error contacting the server at https://my-server.local/message" + "-system." + ) + + def test_bpickle_exception(self): + """If the deserialization of the server response raises an exception, + it is logged and raised. + """ + payload = {"messages": [{"type": "my-message-type", "some-value": 7}]} + + self.fetch_mock.return_value = b"thisisnotbpickled" + + with self.assertRaises(ValueError): + exchange_messages( + payload, + "https://my-server.local/message-system", + ) + + self.fetch_mock.assert_called_once() + self.logging_mock.debug.assert_called_once() + self.logging_mock.exception.assert_called_once_with( + "Server returned invalid data: b'thisisnotbpickled'" + ) diff --git a/landscape/client/tests/test_registration.py b/landscape/client/tests/test_registration.py new file mode 100644 index 00000000..f11d0539 --- /dev/null +++ b/landscape/client/tests/test_registration.py @@ -0,0 +1,159 @@ +"""Unit tests for registration utility functions.""" +from unittest import mock, TestCase + +from landscape.lib.fetch import HTTPCodeError +from landscape.lib.fetch import PyCurlError + +from landscape.client.exchange import ServerResponse +from landscape.client.registration import ( + ClientRegistrationInfo, + RegistrationException, + register, +) + + +class RegisterTestCase(TestCase): + """Tests for the `register` function.""" + + def setUp(self): + super().setUp() + + self.exchange_messages_mock = mock.patch( + "landscape.client.registration.exchange_messages" + ).start() + + self.addCleanup(mock.patch.stopall) + + def test_success(self): + """A successful registration call results in a `RegistationInfo` + object. + """ + client_info = ClientRegistrationInfo( + access_group="", + account_name="testy", + computer_title="Test Computer", + ) + + self.exchange_messages_mock.return_value = ServerResponse( + "thisisaserveruuid", + [{"type": "set-id", "id": "mysecureid", "insecure-id": 1}], + ) + registration_info = register( + client_info, + "https://my-server.local/message-system", + ) + + self.assertEqual(registration_info.insecure_id, 1) + self.assertEqual(registration_info.secure_id, "mysecureid") + self.assertEqual(registration_info.server_uuid, "thisisaserveruuid") + + def test_exchange_http_code_error_404(self): + """If a 404 is raised during the message exchange, a + `RegistrationException` is raised. + """ + client_info = ClientRegistrationInfo( + access_group="", + account_name="testy", + computer_title="Test Computer", + ) + + self.exchange_messages_mock.side_effect = HTTPCodeError( + http_code=404, + body="" + ) + + with self.assertRaises(RegistrationException): + register(client_info, "https://my-server.local/message-system") + + def test_exchange_http_code_error_non_404(self): + """If a non-404 is raised during the message exchange, it is re-raised. + """ + client_info = ClientRegistrationInfo( + access_group="", + account_name="testy", + computer_title="Test Computer", + ) + + self.exchange_messages_mock.side_effect = HTTPCodeError( + http_code=400, + body="" + ) + + with self.assertRaises(HTTPCodeError): + register(client_info, "https://my-server.local/message-system") + + def test_exchange_pycurl_error_ssl(self): + """If a pycurl SSL exception is raised during the message exchange, a + `RegistrationException` is raised. + """ + client_info = ClientRegistrationInfo( + access_group="", + account_name="testy", + computer_title="Test Computer", + ) + + self.exchange_messages_mock.side_effect = PyCurlError( + error_code=60, + message="" + ) + + with self.assertRaises(RegistrationException): + register(client_info, "https://my-server.local/message-system") + + def test_exchange_pycurl_error_non_ssl(self): + """If a pycurl non-SSL exception is raised during the message exchange, + it is re-raised. + """ + client_info = ClientRegistrationInfo( + access_group="", + account_name="testy", + computer_title="Test Computer", + ) + + self.exchange_messages_mock.side_effect = PyCurlError( + error_code=61, + message="" + ) + + with self.assertRaises(PyCurlError): + register(client_info, "https://my-server.local/message-system") + + def test_no_messages(self): + """If there are no messages in the server's response, a + `RegistrationException` is raised. + """ + client_info = ClientRegistrationInfo( + access_group="", + account_name="testy", + computer_title="Test Computer", + ) + + self.exchange_messages_mock.return_value = ServerResponse( + server_uuid="thisisaserveruuid", + messages=[], + ) + + with self.assertRaises(RegistrationException) as exc_context: + register(client_info, "https://my-server.local/message-system") + + self.assertIn("No messages", str(exc_context.exception)) + + def test_no_set_id_message(self): + """If there's no 'set-id' message in the server's response, a + `RegistrationException` is raised. + """ + client_info = ClientRegistrationInfo( + access_group="", + account_name="testy", + computer_title="Test Computer", + ) + + self.exchange_messages_mock.return_value = ServerResponse( + server_uuid="thisisaserveruuid", + messages=[{"type": "unknown-message-type"}], + ) + + with self.assertRaises(RegistrationException) as exc_context: + register(client_info, "https://my-server.local/message-system") + + self.assertIn("Did not receive ID", str(exc_context.exception)) diff --git a/landscape/lib/bpickle.py b/landscape/lib/bpickle.py index 4e7976c0..1858f8d8 100644 --- a/landscape/lib/bpickle.py +++ b/landscape/lib/bpickle.py @@ -31,11 +31,15 @@ This file is modified from the original to work with python3, but should be wire compatible and behave the same way (bugs notwithstanding). """ +from typing import Dict +from typing import Callable + from landscape.lib.compat import _PY3 from landscape.lib.compat import long +from landscape.lib.compat import unicode -dumps_table = {} -loads_table = {} +dumps_table: Dict[type, Callable] = {} +loads_table: Dict[bytes, Callable] = {} def dumps(obj, _dt=dumps_table): diff --git a/landscape/lib/compat.py b/landscape/lib/compat.py index 0bdcf0a5..347a6e73 100644 --- a/landscape/lib/compat.py +++ b/landscape/lib/compat.py @@ -1,36 +1,19 @@ # flake8: noqa # TiCS: disabled -_PY3 = str != bytes +_PY3 = True +import pickle as cPickle +from configparser import ConfigParser, NoOptionError -if _PY3: - import _pickle as cPickle - from configparser import ConfigParser, NoOptionError +SafeConfigParser = ConfigParser - SafeConfigParser = ConfigParser +import _thread as thread - import _thread as thread +from io import StringIO - from io import StringIO +stringio = cstringio = StringIO +from builtins import input - stringio = cstringio = StringIO - from builtins import input - - unicode = str - long = int - -else: - import cPickle - from ConfigParser import ConfigParser, NoOptionError, SafeConfigParser - - import thread - - from StringIO import StringIO - - stringio = StringIO - from cStringIO import StringIO as cstringio - - input = raw_input - long = long - unicode = unicode +unicode = str +long = int diff --git a/landscape/lib/config.py b/landscape/lib/config.py index 6b0fe6b0..8f93a81c 100644 --- a/landscape/lib/config.py +++ b/landscape/lib/config.py @@ -2,6 +2,8 @@ import sys from logging import getLogger from optparse import OptionParser +from typing import Optional +from typing import Sequence from configobj import ConfigObj from configobj import ConfigObjError @@ -29,8 +31,6 @@ def add_cli_options(parser, filename=None): class ConfigSpecOptionParser(OptionParser): - _config_spec_definitions = {} - def __init__(self, unsaved_options=None): OptionParser.__init__(self, unsaved_options) @@ -55,13 +55,13 @@ class BaseConfiguration: Default values for supported options are set as in make_parser. """ - version = None + version: Optional[str] = None required_options = () unsaved_options = () - default_config_filenames = () - default_data_dir = None - config_section = None + default_config_filenames: Sequence[str] = () + default_data_dir: Optional[str] = None + config_section: Optional[str] = None def __init__(self): self._set_options = {}