From 504d1c274e9668a1f8edabd74e22333f88177405 Mon Sep 17 00:00:00 2001 From: Jeremy Audet Date: Thu, 5 Jul 2018 12:01:06 -0400 Subject: [PATCH] Make settings machinery support Pulp 3 When applied, this commit will redefine what a "valid" configuration file is. Existing configuration files will continue to be valid. But in addition, entirely new (and similar-looking) configuration files may also be written. The old configuration files are suitable for use when testing Pulp 2. They let one declare information like "host pulp.example.com is running rabbitmq." The new configuration files are suitable for testing Pulp 3. They lew one declare information like "host pulp.example.com is running redis." In addition, this commit will update two pieces of machinery that are related to the configuration file. Specifically, this commit will overhaul the entire CLI interface (as accessed by the `pulp-smash` executable) and re-implement the `PulpSmashConfig.get_services` method. Fix: https://github.com/PulpQE/pulp-smash/issues/965 Fix: https://github.com/PulpQE/pulp-smash/issues/980 --- docs/usage.rst | 3 +- pulp_smash/cli.py | 14 +- pulp_smash/config.py | 336 ++++++++++++++++++++++------------- pulp_smash/exceptions.py | 13 +- pulp_smash/pulp_smash_cli.py | 289 ++++++++++++++++++++---------- tests/test_cli.py | 6 +- tests/test_config.py | 24 +-- tests/test_pulp_smash_cli.py | 117 ++++++------ 8 files changed, 492 insertions(+), 310 deletions(-) diff --git a/docs/usage.rst b/docs/usage.rst index 6cd8c073b..c60dfbbde 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -89,7 +89,8 @@ you declare properties of the individual hosts that comprise the Pulp application. Each host must fulfill the "shell" role. In addition, the hosts must -collectively fulfill the :obj:`pulp_smash.config.REQUIRED_ROLES`. +collectively fulfill either :obj:`pulp_smash.config.P2_REQUIRED_ROLES` or +:obj:`pulp_smash.config.P3_REQUIRED_ROLES`. Not all roles requires additional information. Currently, only the ``amqp broker``, ``api`` and ``shell`` roles do. The ``amqp broker`` object must have a diff --git a/pulp_smash/cli.py b/pulp_smash/cli.py index 94627e203..3ab91257b 100644 --- a/pulp_smash/cli.py +++ b/pulp_smash/cli.py @@ -7,6 +7,7 @@ from urllib.parse import urlsplit import plumbum +from packaging.version import Version from pulp_smash import exceptions @@ -176,8 +177,10 @@ class Client(): # pylint:disable=too-few-public-methods the host on which commands will be executed. :param response_handler: A callback function. Defaults to :func:`pulp_smash.cli.code_handler`. - :param pulp_smash.config.PulpHost pulp_host: A specific host to target, - instead of the first host with the ``pulp cli`` role. + :param pulp_smash.config.PulpHost pulp_host: A specific host to target. + Defaults to the first host with the ``pulp cli`` role when targeting + Pulp 2, and the first host with the ``shell`` role when targeting Pulp + 3. If Pulp 3 gets a CLI, this latter default may change. .. _Plumbum: http://plumbum.readthedocs.io/en/latest/index.html """ @@ -186,7 +189,10 @@ def __init__(self, cfg, response_handler=None, pulp_host=None): """Initialize this object with needed instance attributes.""" # How do we make requests? if not pulp_host: - pulp_host = cfg.get_hosts('pulp cli')[0] + if cfg.pulp_version < Version('3'): + pulp_host = cfg.get_hosts('pulp cli')[0] + else: + pulp_host = cfg.get_hosts('shell')[0] self.pulp_host = pulp_host hostname = pulp_host.hostname transport = pulp_host.roles.get('shell', {}).get('transport') @@ -510,7 +516,7 @@ class ServiceManager(BaseServiceManager): >>> from pulp_smash import cli, config >>> cfg = config.get_config() - >>> pulp_host = cfg.get_get_services(('api',))[0] + >>> pulp_host = cfg.get_services(('api',))[0] >>> svc_mgr = cli.ServiceManager(cfg, pulp_host) >>> completed_process_list = svc_mgr.stop(['httpd']) >>> completed_process_list = svc_mgr.start(['httpd']) diff --git a/pulp_smash/config.py b/pulp_smash/config.py index 4cf9cd5ef..d6272b727 100644 --- a/pulp_smash/config.py +++ b/pulp_smash/config.py @@ -8,7 +8,6 @@ eases the task of managing that information. """ import collections -import itertools import json import os import warnings @@ -22,12 +21,45 @@ from pulp_smash import exceptions +def _get_pulp_2_api_role(): + """Return a schema definition for the Pulp 2 "api" role.""" + return { + 'required': ['scheme'], + 'type': 'object', + 'properties': { + 'port': { + 'type': 'integer', + 'minimum': 0, + 'maximum': 65535, + }, + 'scheme': { + 'enum': ['http', 'https'], + 'type': 'string', + }, + 'service': { + 'enum': ['httpd', 'nginx'], + 'type': 'string', + }, + 'verify': { + 'type': ['boolean', 'string'], + }, + } + } + + +def _get_pulp_3_api_role(): + """Return a schema definition for the Pulp 3 "api" role.""" + api_role = _get_pulp_2_api_role() + api_role['required'].append('service') + return api_role + + # `get_config` uses this as a cache. It is intentionally a global. This design # lets us do interesting things like flush the cache at run time or completely # avoid a config file by fetching values from the UI. _CONFIG = None -REQUIRED_ROLES = { +P2_REQUIRED_ROLES = { 'amqp broker', 'api', 'mongod', @@ -36,25 +68,60 @@ 'pulp workers', 'shell', } -"""The set of roles that must be present in a functional Pulp application.""" +"""The set of roles that must be present in a functional Pulp 2 application.""" -OPTIONAL_ROLES = { +P2_OPTIONAL_ROLES = { 'pulp cli', 'squid', } -"""Additional roles that can be present in a Pulp application.""" +"""Additional roles that can be present in a Pulp 2 application.""" -ROLES = REQUIRED_ROLES.union(OPTIONAL_ROLES) -"""The set of all roles that may be present in a Pulp application.""" +P2_ROLES = P2_REQUIRED_ROLES.union(P2_OPTIONAL_ROLES) +"""The set of all roles that can be present in a Pulp 2 application.""" -AMQP_SERVICES = {'qpidd', 'rabbitmq'} +P2_AMQP_SERVICES = {'qpidd', 'rabbitmq'} """The set of services that can fulfill the ``amqp broker`` role.""" -CONFIG_JSON_SCHEMA = { - 'additionalProperties': False, - 'required': ['pulp', 'hosts'], - 'type': 'object', - 'properties': { +P3_REQUIRED_ROLES = { + 'api', + 'pulp resource manager', + 'pulp workers', + 'redis', + 'shell', +} +"""The set of roles that must be present in a functional Pulp 3 application.""" + +JSON_CONFIG_SCHEMA = { + 'title': 'Pulp Smash configuration file', + 'anyOf': [ + { + 'additionalProperties': False, + 'required': ['pulp', 'hosts'], + 'type': 'object', + 'properties': { + 'hosts': { + 'type': 'array', + 'minItems': 1, + 'items': {'$ref': '#/definitions/pulp 2 host'}, + }, + 'pulp': {'$ref': '#/definitions/pulp'} + } + }, + { + 'additionalProperties': False, + 'required': ['pulp', 'hosts'], + 'type': 'object', + 'properties': { + 'hosts': { + 'type': 'array', + 'minItems': 1, + 'items': {'$ref': '#/definitions/pulp 3 host'}, + }, + 'pulp': {'$ref': '#/definitions/pulp'} + } + } + ], + 'definitions': { 'pulp': { 'additionalProperties': False, 'required': ['auth', 'version'], @@ -65,14 +132,7 @@ 'selinux enabled': {'type': 'boolean'}, } }, - 'hosts': { - 'type': 'array', - 'minItems': 1, - 'items': {'$ref': '#/definitions/host'}, - } - }, - 'definitions': { - 'host': { + 'pulp 2 host': { 'additionalProperties': False, 'required': ['hostname', 'roles'], 'type': 'object', @@ -83,69 +143,90 @@ }, 'roles': { 'additionalProperties': False, + # We do *not* want to use P2_REQUIRED_ROLES here. See + # validate_config(). 'required': ['shell'], 'type': 'object', 'properties': { 'amqp broker': { - 'required': ['service'], - 'type': 'object', - 'properties': { - 'service': { - 'enum': list(AMQP_SERVICES), - 'type': 'string', - }, - } - }, - 'api': { - 'required': ['scheme'], - 'type': 'object', - 'properties': { - 'port': { - 'type': 'integer', - 'minimum': 0, - 'maximum': 65535, - }, - 'scheme': { - 'enum': ['http', 'https'], - 'type': 'string', - }, - 'verify': { - 'type': ['boolean', 'string'], - }, - } - }, - 'mongod': { - 'type': 'object', - }, - 'pulp cli': { - 'type': 'object', + '$ref': '#/definitions/amqp broker role' }, + 'api': {'$ref': '#/definitions/pulp 2 api role'}, + 'mongod': {'$ref': '#/definitions/mongod role'}, 'pulp celerybeat': { - 'type': 'object', + '$ref': '#/definitions/pulp celerybeat role' }, + 'pulp cli': {'$ref': '#/definitions/pulp cli role'}, 'pulp resource manager': { - 'type': 'object', + '$ref': '#/definitions/pulp resource manager role' }, 'pulp workers': { - 'type': 'object', + '$ref': '#/definitions/pulp workers role' }, - 'shell': { - 'type': 'object', - 'properties': { - 'transport': { - 'enum': ['local', 'ssh'], - 'type': 'string', - } - } + 'shell': {'$ref': '#/definitions/shell role'}, + 'squid': {'$ref': '#/definitions/squid role'}, + } + } + } + }, + 'pulp 3 host': { + 'additionalProperties': False, + 'required': ['hostname', 'roles'], + 'type': 'object', + 'properties': { + 'hostname': { + 'type': 'string', + 'format': 'hostname', + }, + 'roles': { + 'additionalProperties': False, + # We do *not* want to use P3_REQUIRED_ROLES here. See + # validate_config(). + 'required': ['shell'], + 'type': 'object', + 'properties': { + 'api': {'$ref': '#/definitions/pulp 3 api role'}, + 'pulp resource manager': { + '$ref': '#/definitions/pulp resource manager role' }, - 'squid': { - 'type': 'object', + 'pulp workers': { + '$ref': '#/definitions/pulp workers role' }, + 'redis': {'$ref': '#/definitions/redis role'}, + 'shell': {'$ref': '#/definitions/shell role'}, } } } - } - }, + }, + 'amqp broker role': { + 'required': ['service'], + 'type': 'object', + 'properties': { + 'service': { + 'enum': list(P2_AMQP_SERVICES), + 'type': 'string', + }, + } + }, + 'pulp 2 api role': _get_pulp_2_api_role(), + 'pulp 3 api role': _get_pulp_3_api_role(), + 'mongod role': {'type': 'object'}, + 'pulp celerybeat role': {'type': 'object'}, + 'pulp cli role': {'type': 'object'}, + 'pulp resource manager role': {'type': 'object'}, + 'pulp workers role': {'type': 'object'}, + 'redis role': {'type': 'object'}, + 'shell role': { + 'type': 'object', + 'properties': { + 'transport': { + 'enum': ['local', 'ssh'], + 'type': 'string', + } + } + }, + 'squid role': {'type': 'object'}, + } } """The schema for Pulp Smash's configuration file.""" @@ -175,53 +256,35 @@ def get_config(): def validate_config(config_dict): - """Validate an in-memory configuration file. + """Validate a config against :data:`pulp_smash.config.JSON_CONFIG_SCHEMA`. - Given an in-memory configuration file, verify its sanity by validating it - against a schema and performing several semantic checks. - - :param config_dict: A dictionary returned by ``json.load`` or - ``json.loads`` after loading the config file. + :param config_dict: A dict, such as one returned by calling ``json.load`` + on a configuration file, or one generated by the user-facing CLI. + :returns: Nothing. :raises pulp_smash.exceptions.ConfigValidationError: If the any validation error is found. """ - validator_cls = jsonschema.validators.validator_for(CONFIG_JSON_SCHEMA) - validator = validator_cls( - schema=CONFIG_JSON_SCHEMA, format_checker=jsonschema.FormatChecker()) - validator.check_schema(CONFIG_JSON_SCHEMA) - messages = [] - - if not validator.is_valid(config_dict): - for error in validator.iter_errors(config_dict): - # jsonschema returns messages where the first letter is uppercase, - # make sure to lower the first letter case to fit better on our - # message. - error_message = error.message[:1].lower() + error.message[1:] - if error.relative_path: - config_path = '[{}]'.format( - ']['.join([repr(i) for i in error.relative_path]) - ) - else: - config_path = '' - messages.append( - 'Failed to validate config{} because {}.'.format( - config_path, - error_message, - ) - ) - if messages: - raise exceptions.ConfigValidationError(messages) - - # Now that the schema is valid, let's check if all the REQUIRED_ROLES are - # defined - config_roles = set(itertools.chain(*[ - list(host['roles'].keys()) for host in config_dict['hosts']])) - if not REQUIRED_ROLES.issubset(config_roles): - raise exceptions.ConfigValidationError([ - 'The following roles are missing: {}'.format( - ', '.join(sorted(REQUIRED_ROLES.difference(config_roles))) - ) - ]) + try: + jsonschema.validate(config_dict, JSON_CONFIG_SCHEMA) + except jsonschema.exceptions.ValidationError as err: + raise exceptions.ConfigValidationError(err.message) from err + + # The schema is capable of defining what roles must be fulfilled by *every* + # host in a Pulp deployment. But it's not capable of defining which roles + # must be fulfilled by the roles in a Pulp deployment *in aggregate*. The + # latter is done here. + config_roles = set() + for host in config_dict['hosts']: + config_roles.update(set(host['roles'].keys())) + if Version(config_dict['pulp']['version']) < Version('3'): + required_roles = P2_REQUIRED_ROLES + else: + required_roles = P3_REQUIRED_ROLES + if not required_roles.issubset(config_roles): + raise exceptions.ConfigValidationError( + 'The following roles are not fulfilled by any hosts: {}' + .format(', '.join(sorted(required_roles - config_roles))) + ) # Representation of a host and its roles.""" @@ -299,7 +362,7 @@ class PulpSmashConfig(): Configuration files contain JSON data structured in a way that resembles what is accepted by this class's constructor. For exact details on the structure of configuration files, see - :data:`pulp_smash.config.CONFIG_JSON_SCHEMA`. + :data:`pulp_smash.config.JSON_CONFIG_SCHEMA`. :param pulp_auth: A two-tuple. Credentials to use when communicating with the server. For example: ``('username', 'password')``. @@ -339,37 +402,54 @@ def get_hosts(self, role): """Return a list of hosts fulfilling the given role. :param role: The role to filter the available hosts, see - `pulp_smash.config.ROLES` for more information. + `pulp_smash.config.P2_ROLES` for more information. """ - if role not in ROLES: + if role not in P2_ROLES: raise ValueError( 'The given role, {}, is not recognized. Valid roles are: {}' - .format(role, ROLES) + .format(role, P2_ROLES) ) return [host for host in self.hosts if role in host.roles] @staticmethod def get_services(roles): - """Translate role names to init system service names.""" - services = [] + """Translate role names to init system service names. + + Sample usage: + + >>> from pulp_smash import config + >>> host = config.PulpHost('example.com', { + ... 'amqp broker': {'service': 'qpidd'}, + ... 'pulp celerybeat': {}, + ... }) + >>> services = config.PulpSmashConfig.get_services(host.roles) + >>> services == {'pulp_celerybeat', 'qpidd'} + True + + :param roles: The ``roles`` attribute of a + :class:`pulp_smash.config.PulpHost`. + :returns: A set. The services that back the named roles. + """ + services = set() for role in roles: if role == 'amqp broker': - service = roles[role].get('service') - if service in AMQP_SERVICES: - services.append(service) + services.add(roles[role]['service']) elif role == 'api': - services.append('httpd') + # The "api" role only requires a "service" key for Pulp 3 + # hosts. For Pulp 2 hosts, the "service" key is optional. + services.add(roles[role].get('service', 'httpd')) elif role in ( 'pulp celerybeat', 'pulp resource manager', - 'pulp workers', - ): - services.append(role.replace(' ', '_')) - elif role in ('mongod', 'squid'): - services.append(role) + 'pulp workers'): + services.add(role.replace(' ', '_')) + elif role in ('mongod', 'redis', 'squid'): + services.add(role) else: + # Some roles, such as "pulp cli" and "shell", don't have any + # backing services. continue - return set(services) + return services def get_base_url(self, pulp_host=None): """Generate the base URL for a given ``pulp_host``. @@ -417,7 +497,7 @@ def get_requests_kwargs(self, pulp_host=None): pulp_host = self.get_hosts('api')[0] kwargs = deepcopy(pulp_host.roles['api']) kwargs['auth'] = tuple(self.pulp_auth) - for key in ('port', 'scheme'): + for key in ('port', 'scheme', 'service'): kwargs.pop(key, None) return kwargs diff --git a/pulp_smash/exceptions.py b/pulp_smash/exceptions.py index f6506912c..9b5085d2b 100644 --- a/pulp_smash/exceptions.py +++ b/pulp_smash/exceptions.py @@ -60,17 +60,14 @@ class ConfigValidationError(Exception): configuration validation is handled. """ - def __init__(self, error_messages, *args, **kwargs): - """Require that the validation messages list is defined.""" - super().__init__(error_messages, *args, **kwargs) - self.error_messages = error_messages + def __init__(self, message, *args, **kwargs): + """Require that an error message be provided.""" + super().__init__(message, *args, **kwargs) + self.message = message def __str__(self): """Provide a human-friendly string representation of this exception.""" - return ( - 'Configuration file is not valid:\n\n' - '{}' - ).format('\n'.join(self.error_messages)) + return 'Pulp Smash configuration is invalid: {}'.format(self.message) class ConfigFileSectionNotFoundError(Exception): diff --git a/pulp_smash/pulp_smash_cli.py b/pulp_smash/pulp_smash_cli.py index 214177598..59d5b7549 100644 --- a/pulp_smash/pulp_smash_cli.py +++ b/pulp_smash/pulp_smash_cli.py @@ -1,8 +1,10 @@ # coding=utf-8 """The entry point for Pulp Smash's command line interface.""" import json +import sys import click +from packaging.version import Version from pulp_smash import config, exceptions from pulp_smash.config import PulpSmashConfig @@ -39,109 +41,220 @@ def settings(ctx): @settings.command('create') @click.pass_context -def settings_create(ctx): # pylint:disable=too-many-locals +def settings_create(ctx): """Create a settings file.""" + # Choose where and whether to save the configuration file. path = ctx.obj['load_path'] if path: - click.echo( - 'A settings file already exists. Continuing will override it.' + click.confirm( + 'A settings file already exists. Continuing will override it. ' + 'Do you want to continue?', + abort=True, ) - click.confirm('Do you want to continue?', abort=True) else: path = ctx.obj['save_path'] - pulp_username = click.prompt('Pulp admin username', default='admin') - pulp_password = click.prompt('Pulp admin password', default='admin') - pulp_version = click.prompt('Pulp version') - pulp_selinux_enabled = click.confirm( - 'Is SELinux supported in the test environment?', - default=True + + # Get information about Pulp. + pulp_config = {'pulp': _get_pulp_properties()} + pulp_config['hosts'] = [ + _get_host_properties(pulp_config['pulp']['version']) + ] + pulp_config['pulp']['version'] = str(pulp_config['pulp']['version']) + try: + config.validate_config(pulp_config) # This should NEVER fail! + except exceptions.ConfigValidationError: + print( + 'An internal error has occurred. Please report this to the Pulp ' + 'Smash developers at https://github.com/PulpQE/pulp-smash/issues', + file=sys.stderr, + ) + raise + + # Write the config to disk. + with open(path, 'w') as handler: + handler.write(json.dumps(pulp_config, indent=2, sort_keys=True)) + click.echo('Settings written to {}.'.format(path)) + + +def _get_pulp_properties(): + """Get information about the Pulp application as a whole.""" + version = click.prompt( + 'Which version of Pulp is under test?', + type=PulpVersionType() ) - host_hostname = click.prompt('Host hostname') - if click.confirm( - "Is Pulp's API available over HTTPS (no for HTTP)?", default=True): - host_api_scheme = 'https' - else: - host_api_scheme = 'http' + username = click.prompt( + "What is the Pulp administrative user's username?", + default='admin', + type=click.STRING, + ) + password = click.prompt( + "What is the Pulp administrative user's password?", + default='admin', + type=click.STRING, + ) + # We could make this default to "false" if version >= 3, but it seems + # better to assume that Pulp is secure by default, and to annoy everyone + # about Pulp 3's lack of security. + selinux_enabled = click.confirm( + 'Is SELinux supported on the Pulp hosts?', + default=True, + ) + return { + 'auth': [username, password], + 'selinux enabled': selinux_enabled, + 'version': version, + } + + +def _get_host_properties(pulp_version): + """Get information about a Pulp host.""" + if pulp_version < Version('3'): + return _get_v2_host_properties(pulp_version) + return _get_v3_host_properties(pulp_version) + + +def _get_v2_host_properties(pulp_version): + """Get information about a Pulp 2 host.""" + hostname = _get_hostname() + amqp_broker = _get_amqp_broker_role() + api_role = _get_api_role(pulp_version) + shell_role = _get_shell_role(hostname) + return { + 'hostname': hostname, + 'roles': { + 'amqp broker': {'service': amqp_broker}, + 'api': api_role, + 'mongod': {}, + 'pulp celerybeat': {}, + 'pulp cli': {}, + 'pulp resource manager': {}, + 'pulp workers': {}, + 'shell': shell_role, + 'squid': {}, + } + } + + +def _get_v3_host_properties(pulp_version): + """Get information about a Pulp 3 host.""" + hostname = _get_hostname() + api_role = _get_api_role(pulp_version) + shell_role = _get_shell_role(hostname) + return { + 'hostname': hostname, + 'roles': { + 'api': api_role, + 'pulp resource manager': {}, + 'pulp workers': {}, + 'redis': {}, + 'shell': shell_role, + } + } + + +def _get_hostname(): + """Get the Pulp host's hostname.""" + return click.prompt("What is the Pulp host's hostname?", type=click.STRING) + - if (host_api_scheme == 'https' and +def _get_amqp_broker_role(): + """Get information for the "amqp broker" role.""" + return click.prompt( + "What service backs Pulp's AMQP broker?", + default='qpidd', + type=click.Choice(('qpidd', 'rabbitmq')), + ) + + +def _get_api_role(pulp_version): + """Get information for the "api" role.""" + api_role = {} + + # Get "scheme" + api_role['scheme'] = click.prompt( + "What scheme should be used when communicating with Pulp's API?", + default='https', + type=click.Choice(('https', 'http')), + ) + + # Get "verify" + if (api_role['scheme'] == 'https' and click.confirm('Verify HTTPS?', default=True)): - certificate_path = click.prompt('SSL certificate path', default='') - if not certificate_path: - host_api_verify = True # pragma: no cover - else: - host_api_verify = certificate_path + certificate_path = click.prompt( + 'SSL certificate path', + default='', + type=click.Path(), + ) + api_role['verify'] = certificate_path if certificate_path else True else: - host_api_verify = False + api_role['verify'] = False + # Get "port" click.echo( "By default, Pulp Smash will communicate with Pulp's API on the port " "number implied by the scheme. For example, if Pulp's API is " - 'available over HTTPS, then Pulp Smash will communicate on port 443.' - "If Pulp's API is avaialable on a non-standard port, like 8000, then " + 'available over HTTPS, then Pulp Smash will communicate on port 443. ' + "If Pulp's API is available on a non-standard port, like 8000, then " 'Pulp Smash needs to know about that.' ) - host_api_port = click.prompt('Pulp API port number', type=int, default=0) + port = click.prompt('Pulp API port number', default=0, type=click.INT) + if port: + api_role['port'] = port - if click.confirm( - 'Is Pulp\'s message broker Qpid (no for RabbitMQ)?', default=True): - amqp_broker = 'qpidd' - else: - amqp_broker = 'rabbitmq' - using_ssh = not click.confirm( - 'Are you running Pulp Smash on the Pulp host?') - if using_ssh: - click.echo( - 'Pulp Smash will be configured to access the Pulp host using ' - 'SSH. Because of that, some additional information is required.' - ) - ssh_user = click.prompt('SSH username to use', default='root') - click.echo( - 'Ensure ~/.ssh/controlmasters/ exists, and ensure the following ' - 'is present in your ~/.ssh/config file:' - '\n\n' - ' Host {host_hostname}\n' - ' StrictHostKeyChecking no\n' - ' User {ssh_user}\n' - ' UserKnownHostsFile /dev/null\n' - ' ControlMaster auto\n' - ' ControlPersist 10m\n' - ' ControlPath ~/.ssh/controlmasters/%C\n' - .format(host_hostname=host_hostname, ssh_user=ssh_user) - ) + # Get "service" + api_role['service'] = click.prompt( + "What web server service backs Pulp's API?", + default='httpd' if pulp_version < Version('3') else 'nginx', + type=click.Choice(('httpd', 'nginx')) + ) - click.echo('Creating the settings file at {}...'.format(path)) - config_dict = { - 'pulp': { - 'auth': [pulp_username, pulp_password], - 'version': pulp_version, - 'selinux enabled': pulp_selinux_enabled, - }, - 'hosts': [{ - 'hostname': host_hostname, - 'roles': { - 'amqp broker': {'service': amqp_broker}, - 'api': { - 'scheme': host_api_scheme, - 'verify': host_api_verify, - }, - 'mongod': {}, - 'pulp celerybeat': {}, - 'pulp cli': {}, - 'pulp resource manager': {}, - 'pulp workers': {}, - 'shell': {'transport': 'ssh' if using_ssh else 'local'}, - 'squid': {}, - } - }] - } - if host_api_port: - config_dict['hosts'][0]['roles']['api']['port'] = host_api_port - with open(path, 'w') as handler: - handler.write(json.dumps(config_dict, indent=2, sort_keys=True)) + return api_role + + +def _get_shell_role(hostname): + if click.confirm('Is Pulp Smash installed on the same host as Pulp?'): + click.echo('Pulp Smash will access the Pulp host using a local shell.') + return {'transport': 'local'} + click.echo('Pulp Smash will access the Pulp host using SSH.') + ssh_user = click.prompt('SSH username', default='root') click.echo( - 'Settings file created, run `pulp-smash settings show` to show its ' - 'contents.' + 'Ensure the SSH user has passwordless sudo access, ensure ' + '~/.ssh/controlmasters/ exists, and ensure the following is ' + 'present in your ~/.ssh/config file:' + '\n\n' + ' Host {}\n' + ' StrictHostKeyChecking no\n' + ' User {}\n' + ' UserKnownHostsFile /dev/null\n' + ' ControlMaster auto\n' + ' ControlPersist 10m\n' + ' ControlPath ~/.ssh/controlmasters/%C\n' + .format(hostname, ssh_user) ) + return {'transport': 'ssh'} + + +class PulpVersionType(click.ParamType): + """Define the possible values for a Pulp version string. + + A Pulp version string is valid if it can be cast to a + ``packaging.version.Version`` object, if it is at least 2, and if it is + less than 4. + """ + + name = 'Pulp version' + + def convert(self, value, param, ctx): + """Convert a version string to a ``Version`` object.""" + converted_ver = Version(value) + if converted_ver < Version('2') or converted_ver >= Version('4'): + self.fail( + "Pulp Smash can test Pulp version 2.y and 3.y. It can't test " + 'Pulp version {}.'.format(converted_ver), + param, + ctx + ) + return converted_ver @settings.command('path') @@ -188,7 +301,6 @@ def settings_show(ctx): path = ctx.obj['load_path'] if not path: _raise_settings_not_found() - with open(path) as handle: click.echo(json.dumps(json.load(handle), indent=2, sort_keys=True)) @@ -205,12 +317,9 @@ def settings_validate(ctx): try: config.validate_config(config_dict) except exceptions.ConfigValidationError as err: - message = ('Invalid settings file: {}\n' .format(path)) - for error_message in err.error_messages: - message += error_message - result = click.ClickException(message) - result.exit_code = -1 - raise result + raise click.ClickException( + '{} is invalid: '.format(path) + err.message + ) from err if __name__ == '__main__': diff --git a/tests/test_cli.py b/tests/test_cli.py index 22468fb3b..a8f981811 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -102,7 +102,7 @@ def test_implicit_local_transport(self): cfg = _get_pulp_smash_config(hosts=[ config.PulpHost( hostname=socket.getfqdn(), - roles={'pulp cli': {}}, + roles={'shell': {}}, ) ]) self.assertIsInstance(cli.Client(cfg).machine, LocalMachine) @@ -133,11 +133,11 @@ def test_implicit_pulp_host(self): cfg = _get_pulp_smash_config(hosts=[ config.PulpHost( hostname=utils.uuid4(), - roles={'pulp cli': {}}, + roles={'shell': {}}, ), config.PulpHost( hostname=utils.uuid4(), - roles={'pulp cli': {}}, + roles={'shell': {}}, ) ]) with mock.patch('pulp_smash.cli.plumbum') as plumbum: diff --git a/tests/test_config.py b/tests/test_config.py index dbf369520..51a45029e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -153,14 +153,8 @@ def test_invalid_config(self): config_dict = json.loads(PULP_SMASH_CONFIG) config_dict['pulp']['auth'] = [] config_dict['hosts'][0]['hostname'] = '' - with self.assertRaises(exceptions.ConfigValidationError) as err: + with self.assertRaises(exceptions.ConfigValidationError): config.validate_config(config_dict) - self.assertEqual(sorted(err.exception.error_messages), sorted([ - 'Failed to validate config[\'pulp\'][\'auth\'] because [] is too ' - 'short.', - 'Failed to validate config[\'hosts\'][0][\'hostname\'] because ' - '\'\' is not a \'hostname\'.', - ])) def test_config_missing_roles(self): """Missing required roles in config raises an exception.""" @@ -171,8 +165,9 @@ def test_config_missing_roles(self): with self.assertRaises(exceptions.ConfigValidationError) as err: config.validate_config(config_dict) self.assertEqual( - err.exception.error_messages, - ['The following roles are missing: api, pulp workers'] + err.exception.message, + 'The following roles are not fulfilled by any hosts: api, pulp ' + 'workers', ) @@ -282,7 +277,10 @@ def test_get_hosts(self): def test_get_services(self): """``get_services`` returns proper result.""" - roles = {role: {} for role in config.ROLES} + # If set, the "amqp broker" role must have a "service" attribute. + roles = {role: {} for role in config.P2_ROLES} + del roles['amqp broker'] + expected_roles = { 'httpd', 'mongod', @@ -296,14 +294,16 @@ def test_get_services(self): self.cfg.get_services(roles), expected_roles ) + + roles['amqp broker'] = {'service': 'qpidd'} with self.subTest('qpidd amqp broker service'): - roles['amqp broker']['service'] = 'qpidd' self.assertEqual( self.cfg.get_services(roles), expected_roles.union({'qpidd'}) ) + + roles['amqp broker'] = {'service': 'rabbitmq'} with self.subTest('rabbitmq amqp broker service'): - roles['amqp broker']['service'] = 'rabbitmq' self.assertEqual( self.cfg.get_services(roles), expected_roles.union({'rabbitmq'}) diff --git a/tests/test_pulp_smash_cli.py b/tests/test_pulp_smash_cli.py index 684d56e80..c49aa7d70 100644 --- a/tests/test_pulp_smash_cli.py +++ b/tests/test_pulp_smash_cli.py @@ -40,11 +40,6 @@ def test_missing_settings_file(self): [self.settings_subcommand], ) self.assertNotEqual(result.exit_code, 0, result.output) - self.assertIn( - 'there is no settings file. Use `pulp-smash settings create` ' - 'to create one.', - result.output, - ) class SettingsCreateTestCase(BasePulpSmashCliTestCase): @@ -56,8 +51,8 @@ def setUp(self): self.expected_config_dict = { 'pulp': { 'auth': ['admin', 'admin'], - 'version': '2.13', 'selinux enabled': True, + 'version': '2.13', }, 'hosts': [{ 'hostname': 'pulp.example.com', @@ -65,6 +60,7 @@ def setUp(self): 'amqp broker': {'service': 'qpidd'}, 'api': { 'scheme': 'https', + 'service': 'httpd', 'verify': True, }, 'mongod': {}, @@ -103,12 +99,6 @@ def _test_common_logic(self, create_input, glp_return_value=None): input=create_input, ) self.assertEqual(result.exit_code, 0, result.output) - self.assertIn( - 'Creating the settings file at settings.json...\nSettings ' - 'file created, run `pulp-smash settings show` to show its ' - 'contents.\n', - result.output, - ) self.assertTrue(os.path.isfile('settings.json')) with open('settings.json') as handler: return handler.read() @@ -116,86 +106,94 @@ def _test_common_logic(self, create_input, glp_return_value=None): def test_create_with_defaults(self): """Create settings file with default values values.""" create_input = ( - '\n' # admin username - '\n' # admin password '2.13\n' # pulp version - '\n' # pulp_selinux_enabled + '\n' # pulp admin username + '\n' # pulp admin password + '\n' # pulp selinux enabled 'pulp.example.com\n' # host hostname - '\n' # published via HTTPS - '\n' # verify HTTPS - '\n' # API port - '\n' # using qpidd + '\n' # host amqp broker + '\n' # host api scheme + '\n' # host api ssl verification + '\n' # host api ssl cert path + '\n' # host api port + '\n' # host api web server '\n' # running on Pulp host ) - generated_settings = self._test_common_logic(create_input) - self.assertEqual( - json.loads(generated_settings), self.expected_config_dict) + generated_settings = json.loads(self._test_common_logic(create_input)) + self.assertEqual(generated_settings, self.expected_config_dict) def test_settings_already_exists(self): """Create settings file by overriding existing one.""" create_input = ( - 'y\n' # settings exists, continue - '\n' # admin username - '\n' # admin password + 'y\n' # overwrite existing settings file? '2.13\n' # pulp version - '\n' # pulp_selinux_enabled + '\n' # pulp admin username + '\n' # pulp admin password + '\n' # pulp selinux enabled 'pulp.example.com\n' # host hostname - '\n' # published via HTTPS - '\n' # verify HTTPS - '\n' # API port - '\n' # using qpidd + '\n' # host amqp broker + '\n' # host api scheme + '\n' # host api ssl verification + '\n' # host api ssl cert path + '\n' # host api port + '\n' # host api web server '\n' # running on Pulp host ) - generated_settings = self._test_common_logic( - create_input, 'settings.json') - self.assertEqual( - json.loads(generated_settings), self.expected_config_dict) + generated_settings = json.loads( + self._test_common_logic(create_input, 'settings.json') + ) + self.assertEqual(generated_settings, self.expected_config_dict) def test_create_defaults_and_verify(self): """Create settings file with defaults and custom SSL certificate.""" create_input = ( - '\n' # admin username - '\n' # admin password '2.13\n' # pulp version - '\n' # pulp_selinux_enabled + '\n' # pulp admin username + '\n' # pulp admin password + '\n' # pulp selinux enabled 'pulp.example.com\n' # host hostname - '\n' # published via HTTPS - 'y\n' # verify HTTPS - '/path/to/ssl/certificate\n' # SSL certificate path - '\n' # API port - '\n' # using qpidd + '\n' # host amqp broker + '\n' # host api scheme + '\n' # host api ssl verification + '/path/to/cert\n' # host api ssl cert path + '\n' # host api port + '\n' # host api web server '\n' # running on Pulp host ) - generated_settings = self._test_common_logic(create_input) + generated_settings = json.loads(self._test_common_logic(create_input)) self.expected_config_dict['hosts'][0]['roles']['api']['verify'] = ( - '/path/to/ssl/certificate' + '/path/to/cert' ) - self.assertEqual( - json.loads(generated_settings), self.expected_config_dict) + self.assertEqual(generated_settings, self.expected_config_dict) def test_create_other_values(self): """Create settings file with custom values.""" create_input = ( - 'username\n' # admin username - 'password\n' # admin password '2.13\n' # pulp version - 'n\n' # pulp_selinux_enabled + 'username\n' # pulp admin username + 'password\n' # pulp admin password + 'n\n' # pulp selinux enabled 'pulp.example.com\n' # host hostname - 'n\n' # published via HTTPS - '\n' # API port - 'n\n' # using qpidd + 'rabbitmq\n' # host amqp broker + 'http\n' # host api scheme + 'n\n' # host api ssl verification + '1234\n' # host api port + 'nginx\n' # host api web server 'y\n' # running on Pulp host ) - generated_settings = self._test_common_logic(create_input) + generated_settings = json.loads(self._test_common_logic(create_input)) + self.expected_config_dict['pulp']['auth'] = ['username', 'password'] self.expected_config_dict['pulp']['selinux enabled'] = False host_roles = self.expected_config_dict['hosts'][0]['roles'] host_roles['amqp broker']['service'] = 'rabbitmq' + host_roles['api']['port'] = 1234 host_roles['api']['scheme'] = 'http' + host_roles['api']['service'] = 'nginx' host_roles['api']['verify'] = False host_roles['shell']['transport'] = 'local' - self.assertEqual( - json.loads(generated_settings), self.expected_config_dict) + + self.assertEqual(generated_settings, self.expected_config_dict) class SettingsPathTestCase(BasePulpSmashCliTestCase, MissingSettingsFileMixin): @@ -319,12 +317,3 @@ def test_invalid_config(self): ['validate'], ) self.assertNotEqual(result.exit_code, 0) - self.assertIn( - 'Invalid settings file: settings.json', - result.output, - ) - self.assertIn( - "Failed to validate config['pulp'] because 'auth' is a " - 'required property.', - result.output, - )