diff --git a/devstack/lib/ironic b/devstack/lib/ironic index e8bb33b09b..b5a00b99d1 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1113,7 +1113,7 @@ function configure_ironic_conductor { # TODO(pas-ha) this block is for transition period only, # after all clients are moved to use keystoneauth adapters, # it will be deleted - local sections_with_adapter="service_catalog" + local sections_with_adapter="service_catalog glance" for conf_section in $sections_with_adapter; do configure_adapter_for $conf_section done diff --git a/doc/source/install/enabling-https.rst b/doc/source/install/enabling-https.rst index 38da170a28..66b7e2adc3 100644 --- a/doc/source/install/enabling-https.rst +++ b/doc/source/install/enabling-https.rst @@ -79,13 +79,13 @@ To enable secure HTTPS communication between Bare Metal service and Image servic served by Image service. #. If not using the keystone service catalog for the Image service API endpoint - discovery, also edit the ``glance_api_servers`` option to point to HTTPS URL + discovery, also edit the ``endpoint_override`` option to point to HTTPS URL of image service (replace ```` with hostname[:port][path] of the Image service endpoint):: [glance] ... - glance_api_servers = https:// + endpoint_override = https:// #. Restart ironic-conductor service:: diff --git a/doc/source/install/include/configure-ironic-conductor.rst b/doc/source/install/include/configure-ironic-conductor.rst index f187157ae3..2279a63072 100644 --- a/doc/source/install/include/configure-ironic-conductor.rst +++ b/doc/source/install/include/configure-ironic-conductor.rst @@ -131,7 +131,7 @@ Configuring ironic-conductor service .. code-block:: ini [glance] - glance_api_servers = + endpoint_override = #. Notes for configuring the Network service access diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index c11b6cd0b5..b32d0a6e75 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1507,9 +1507,14 @@ # Authentication URL (string value) #auth_url = -# Authentication strategy to use when connecting to glance. -# (string value) +# DEPRECATED: Authentication strategy to use when connecting +# to glance. (string value) # Allowed values: keystone, noauth +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: To configure glance in noauth mode, set +# [glance]/auth_type=none and +# [glance]/endpoint_override= instead. #auth_strategy = keystone # Authentication type to load (string value) @@ -1539,15 +1544,24 @@ # Domain name to scope to (string value) #domain_name = -# Allow to perform insecure SSL (https) requests to glance. -# (boolean value) +# Always use this endpoint URL for requests for this client. +# (string value) +#endpoint_override = + +# DEPRECATED: Allow to perform insecure SSL (https) requests +# to glance. (boolean value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Use [glance]/insecure option instead. #glance_api_insecure = false -# A list of the glance api servers available to ironic. Prefix -# with https:// for SSL-based glance API servers. Format is -# [hostname|IP]:port. If this option is not set, the service -# catalog is used. It is recommended to rely on the service -# catalog, if possible. (list value) +# DEPRECATED: A list of the glance api servers available to +# ironic. Prefix with https:// for SSL-based glance API +# servers. Format is [hostname|IP]:port. (list value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Use [glance]/endpoint_override option to set the +# full load-balanced glance API URL instead. #glance_api_servers = # DEPRECATED: Glance API version (1 or 2) to use. (integer @@ -1560,9 +1574,13 @@ # in the Queens release. #glance_api_version = 2 -# Optional path to a CA certificate bundle to be used to -# validate the SSL certificate served by glance. It is used -# when glance_api_insecure is set to False. (string value) +# DEPRECATED: Optional path to a CA certificate bundle to be +# used to validate the SSL certificate served by glance. It is +# used when glance_api_insecure is set to False. (string +# value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Use [glance]/cafile option instead. #glance_cafile = # Number of retries when downloading an image from glance. @@ -1575,6 +1593,18 @@ # PEM encoded client certificate key file (string value) #keyfile = +# The maximum major version of a given API, intended to be +# used as the upper bound of a range with min_version. +# Mutually exclusive with version. (string value) +#max_version = + +# The minimum major version of a given API, intended to be +# used as the lower bound of a range with max_version. +# Mutually exclusive with version. If min_version is given +# with no max_version it is as if max version is "latest". +# (string value) +#min_version = + # User's password (string value) #password = @@ -1592,6 +1622,18 @@ # Deprecated group/name - [glance]/tenant_name #project_name = +# The default region_name for endpoint URL discovery. (string +# value) +#region_name = + +# The default service_name for endpoint URL discovery. (string +# value) +#service_name = + +# The default service_type for endpoint URL discovery. (string +# value) +#service_type = image + # The account that Glance uses to communicate with Swift. The # format is "AUTH_uuid". "uuid" is the UUID for the account # configured in the glance-api.conf. Required for temporary @@ -1689,6 +1731,15 @@ # Deprecated group/name - [glance]/user_name #username = +# List of interfaces, in order of preference, for endpoint +# URL. (list value) +#valid_interfaces = internal,public + +# Minimum Major API version within a given Major API version +# for endpoint URL discovery. Mutually exclusive with +# min_version and max_version (string value) +#version = + [ilo] diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 05e8533f7a..c8d925c45a 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -487,7 +487,7 @@ class UnsupportedDriverExtension(Invalid): class GlanceConnectionFailed(IronicException): - _msg_fmt = _("Connection to glance host %(endpoint)s failed: " + _msg_fmt = _("Connection to glance endpoint %(endpoint)s failed: " "%(reason)s") diff --git a/ironic/common/glance_service/base_image_service.py b/ironic/common/glance_service/base_image_service.py index 7cd47eafb7..1768b2a118 100644 --- a/ironic/common/glance_service/base_image_service.py +++ b/ironic/common/glance_service/base_image_service.py @@ -21,7 +21,6 @@ import time from glanceclient import client from glanceclient import exc as glance_exc -from oslo_config import cfg from oslo_log import log import sendfile import six @@ -29,10 +28,20 @@ import six.moves.urllib.parse as urlparse from ironic.common import exception from ironic.common.glance_service import service_utils +from ironic.common import keystone +from ironic.conf import CONF LOG = log.getLogger(__name__) -CONF = cfg.CONF + +_GLANCE_SESSION = None + + +def _get_glance_session(**session_kwargs): + global _GLANCE_SESSION + if not _GLANCE_SESSION: + _GLANCE_SESSION = keystone.get_session('glance', **session_kwargs) + return _GLANCE_SESSION def _translate_image_exception(image_id, exc_value): @@ -46,6 +55,10 @@ def _translate_image_exception(image_id, exc_value): return exc_value +# NOTE(pas-ha) while looking very ugly currently, this will be simplified +# in Rocky after all deprecated [glance] options are removed and +# keystone catalog is always used with 'keystone' auth strategy +# together with session always loaded from config options def check_image_service(func): """Creates a glance client if doesn't exists and calls the function.""" @six.wraps(func) @@ -58,19 +71,54 @@ def check_image_service(func): if self.client: return func(self, *args, **kwargs) - image_href = kwargs.get('image_href') - _id, self.endpoint, use_ssl = service_utils.parse_image_ref(image_href) + # TODO(pas-ha) remove in Rocky + session_params = {} + if CONF.glance.glance_api_insecure and not CONF.glance.insecure: + session_params['insecure'] = CONF.glance.glance_api_insecure + if CONF.glance.glance_cafile and not CONF.glance.cafile: + session_params['cacert'] = CONF.glance.glance_cafile + # NOTE(pas-ha) glanceclient uses Adapter-based SessionClient, + # so we can pass session and auth separately, makes things easier + session = _get_glance_session(**session_params) - params = {} - params['insecure'] = CONF.glance.glance_api_insecure - if (not params['insecure'] and CONF.glance.glance_cafile - and use_ssl): - params['cacert'] = CONF.glance.glance_cafile - if CONF.glance.auth_strategy == 'keystone': - params['token'] = self.context.auth_token - self.client = client.Client(self.version, - self.endpoint, **params) + # TODO(pas-ha) remove in Rocky + # NOTE(pas-ha) new option must win if configured + if (CONF.glance.glance_api_servers and + not CONF.glance.endpoint_override): + # NOTE(pas-ha) all the 2 methods have image_href as the first + # positional arg, but check in kwargs too + image_href = args[0] if args else kwargs.get('image_href') + url = service_utils.get_glance_api_server(image_href) + CONF.set_override('endpoint_override', url, group='glance') + + # TODO(pas-ha) remove in Rocky + if CONF.glance.auth_strategy == 'noauth': + CONF.set_override('auth_type', 'none', group='glance') + + service_auth = keystone.get_auth('glance') + + # TODO(pas-ha) remove in Rocky + adapter_params = {} + if CONF.keystone.region_name and not CONF.glance.region_name: + adapter_params['region_name'] = CONF.keystone.region_name + + adapter = keystone.get_adapter('glance', session=session, + auth=service_auth, **adapter_params) + self.endpoint = adapter.get_endpoint() + + user_auth = None + # NOTE(pas-ha) our ContextHook removes context.auth_token in noauth + # case, so when ironic is in noauth but glance is not, we will not + # enter the next if-block and use auth from [glance] config section + if self.context.auth_token: + user_auth = keystone.get_service_auth(self.context, self.endpoint, + service_auth) + self.client = client.Client(self.version, session=session, + auth=user_auth or service_auth, + endpoint_override=self.endpoint, + global_request_id=self.context.global_id) return func(self, *args, **kwargs) + return wrapper @@ -110,16 +158,16 @@ class BaseImageService(object): try: return getattr(self.client.images, method)(*args, **kwargs) except retry_excs as e: - error_msg = ("Error contacting glance server " - "'%(endpoint)s' for '%(method)s', attempt" - " %(attempt)s of %(num_attempts)s failed.") + error_msg = ("Error contacting glance endpoint " + "%(endpoint)s for '%(method)s', attempt " + "%(attempt)s of %(num_attempts)s failed.") LOG.exception(error_msg, {'endpoint': self.endpoint, 'num_attempts': num_attempts, 'attempt': attempt, 'method': method}) if attempt == num_attempts: raise exception.GlanceConnectionFailed( - endpoint=self.endpoint, reason=str(e)) + endpoint=self.endpoint, reason=e) time.sleep(1) except image_excs as e: exc_type, exc_value, exc_trace = sys.exc_info() @@ -131,14 +179,14 @@ class BaseImageService(object): def _show(self, image_href, method='get'): """Returns a dict with image data for the given opaque image id. - :param image_id: The opaque image identifier. + :param image_href: The opaque image identifier. :returns: A dict containing image metadata. :raises: ImageNotFound """ LOG.debug("Getting image metadata from glance. Image: %s", image_href) - image_id = service_utils.parse_image_ref(image_href)[0] + image_id = service_utils.parse_image_id(image_href) image = self.call(method, image_id) @@ -152,10 +200,10 @@ class BaseImageService(object): def _download(self, image_href, data=None, method='data'): """Calls out to Glance for data and writes data. - :param image_id: The opaque image identifier. + :param image_href: The opaque image identifier. :param data: (Optional) File object to write data to. """ - image_id = service_utils.parse_image_ref(image_href)[0] + image_id = service_utils.parse_image_id(image_href) if (self.version == 2 and 'file' in CONF.glance.allowed_direct_url_schemes): diff --git a/ironic/common/glance_service/service_utils.py b/ironic/common/glance_service/service_utils.py index b23650d7dc..f8c3303214 100644 --- a/ironic/common/glance_service/service_utils.py +++ b/ironic/common/glance_service/service_utils.py @@ -18,7 +18,7 @@ import copy import itertools import random -from oslo_config import cfg +from oslo_log import log from oslo_serialization import jsonutils from oslo_utils import timeutils from oslo_utils import uuidutils @@ -27,15 +27,17 @@ import six.moves.urllib.parse as urlparse from ironic.common import exception from ironic.common import image_service -from ironic.common import keystone +from ironic.conf import CONF -CONF = cfg.CONF + +LOG = log.getLogger(__name__) _GLANCE_API_SERVER = None """ iterator that cycles (indefinitely) over glance API servers. """ def _extract_attributes(image): + # TODO(pas-ha) in Queens unify these once GlanceV1 is no longer supported IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', 'container_format', 'checksum', 'id', 'name', 'created_at', 'updated_at', @@ -90,73 +92,47 @@ def _convert(metadata): return metadata -def _get_api_server_iterator(): - """Return iterator over shuffled API servers. - - Shuffle a list of CONF.glance.glance_api_servers and return an iterator - that will cycle through the list, looping around to the beginning if - necessary. - - If CONF.glance.glance_api_servers isn't set, fetch the endpoint from the - service catalog. - - :returns: iterator that cycles (indefinitely) over shuffled glance API - servers. - """ - api_servers = [] - - if not CONF.glance.glance_api_servers: - session = keystone.get_session('glance', - auth=keystone.get_auth('glance')) - api_servers = [keystone.get_service_url(session, service_type='image', - endpoint_type='public')] - else: - api_servers = random.sample(CONF.glance.glance_api_servers, - len(CONF.glance.glance_api_servers)) - return itertools.cycle(api_servers) - - -def _get_api_server(): - """Return a Glance API server. - - :returns: for an API server, the tuple (host-or-IP, port, use_ssl), where - use_ssl is True to use the 'https' scheme, and False to use 'http'. - """ - global _GLANCE_API_SERVER - - if not _GLANCE_API_SERVER: - _GLANCE_API_SERVER = _get_api_server_iterator() - return six.next(_GLANCE_API_SERVER) - - -def parse_image_ref(image_href): - """Parse an image href. +def parse_image_id(image_href): + """Parse an image id from image href. :param image_href: href of an image - :returns: a tuple (image ID, glance URL, whether to use SSL) + :returns: image id parsed from image_href - :raises ValueError: when input image href is invalid + :raises InvalidImageRef: when input image href is invalid """ - if '/' not in six.text_type(image_href): - endpoint = _get_api_server() - return (image_href, endpoint, endpoint.startswith('https')) - else: - try: - url = urlparse.urlparse(image_href) - if url.scheme == 'glance': - endpoint = _get_api_server() - image_id = image_href.split('/')[-1] - return (image_id, endpoint, endpoint.startswith('https')) - else: - glance_port = url.port or 80 - glance_host = url.netloc.split(':', 1)[0] - image_id = url.path.split('/')[-1] - use_ssl = (url.scheme == 'https') - endpoint = '%s://%s:%s' % (url.scheme, glance_host, - glance_port) - return (image_id, endpoint, use_ssl) - except ValueError: + image_href = six.text_type(image_href) + if uuidutils.is_uuid_like(image_href): + image_id = image_href + elif image_href.startswith('glance://'): + image_id = image_href.split('/')[-1] + if not uuidutils.is_uuid_like(image_id): raise exception.InvalidImageRef(image_href=image_href) + else: + raise exception.InvalidImageRef(image_href=image_href) + return image_id + + +# TODO(pas-ha) remove in Rocky +def get_glance_api_server(image_href): + """Construct a glance API url from config options + + Returns a random server from the CONF.glance.glance_api_servers list + of servers. + + :param image_href: href of an image + :returns: glance API URL + + :raises InvalidImageRef: when input image href is invalid + """ + image_href = six.text_type(image_href) + if not is_glance_image(image_href): + raise exception.InvalidImageRef(image_href=image_href) + global _GLANCE_API_SERVER + if not _GLANCE_API_SERVER: + _GLANCE_API_SERVER = itertools.cycle( + random.sample(CONF.glance.glance_api_servers, + len(CONF.glance.glance_api_servers))) + return six.next(_GLANCE_API_SERVER) def translate_from_glance(image): @@ -176,7 +152,9 @@ def is_image_available(context, image): # request and we need not handle the noauth use-case. if hasattr(context, 'auth_token') and context.auth_token: return True - if image.is_public or context.is_admin: + + if ((getattr(image, 'is_public', None) or + getattr(image, 'visibility', None) == 'public') or context.is_admin): return True properties = image.properties if context.project_id and ('owner_id' in properties): diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index 10ff11ff98..462a76367a 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -20,7 +20,9 @@ import datetime import os import shutil +from oslo_log import log from oslo_utils import importutils +from oslo_utils import uuidutils import requests import sendfile import six @@ -29,23 +31,15 @@ import six.moves.urllib.parse as urlparse from ironic.common import exception from ironic.common.i18n import _ -from ironic.common import keystone from ironic.common import utils from ironic.conf import CONF IMAGE_CHUNK_SIZE = 1024 * 1024 # 1mb - -_GLANCE_SESSION = None - - -def _get_glance_session(): - global _GLANCE_SESSION - if not _GLANCE_SESSION: - auth = keystone.get_auth('glance') - _GLANCE_SESSION = keystone.get_session('glance', auth=auth) - return _GLANCE_SESSION +LOG = log.getLogger(__name__) +# TODO(pas-ha) in Queens change default to '2', +# but keep the versioned import in place (less work for possible Glance v3) def GlanceImageService(client=None, version=None, context=None): module_str = 'ironic.common.glance_service' if version is None: @@ -54,9 +48,6 @@ def GlanceImageService(client=None, version=None, context=None): module = importutils.import_versioned_module(module_str, version, 'image_service') service_class = getattr(module, 'GlanceImageService') - if (context is not None and CONF.glance.auth_strategy == 'keystone' - and not context.auth_token): - context.auth_token = _get_glance_session().get_token() return service_class(client, version, context) @@ -279,14 +270,21 @@ def get_image_service(image_href, client=None, version=None, context=None): specified image. """ scheme = urlparse.urlparse(image_href).scheme.lower() - try: - cls = protocol_mapping[scheme or 'glance'] - except KeyError: - raise exception.ImageRefValidationFailed( - image_href=image_href, - reason=_('Image download protocol ' - '%s is not supported.') % scheme - ) + + if not scheme: + if uuidutils.is_uuid_like(six.text_type(image_href)): + cls = GlanceImageService + else: + raise exception.ImageRefValidationFailed( + image_href=image_href, + reason=_('Scheme-less image href is not a UUID.')) + else: + cls = protocol_mapping.get(scheme) + if not cls: + raise exception.ImageRefValidationFailed( + image_href=image_href, + reason=_('Image download protocol %s is not supported.' + ) % scheme) if cls == GlanceImageService: return cls(client, version, context) diff --git a/ironic/common/keystone.py b/ironic/common/keystone.py index bc97ac35ef..53f2637169 100644 --- a/ironic/common/keystone.py +++ b/ironic/common/keystone.py @@ -16,6 +16,8 @@ from keystoneauth1 import exceptions as kaexception from keystoneauth1 import loading as kaloading +from keystoneauth1 import service_token +from keystoneauth1 import token_endpoint from oslo_log import log as logging import six @@ -102,7 +104,23 @@ def get_adapter(group, **adapter_kwargs): **adapter_kwargs) -# NOTE(pas-ha) Used by neutronclient and glanceclient only +def get_service_auth(context, endpoint, service_auth): + """Create auth plugin wrapping both user and service auth. + + When properly configured and using auth_token middleware, + requests with valid service auth will not fail + if the user token is expired. + + Ideally we would use the plugin provided by auth_token middleware + however this plugin isn't serialized yet. + """ + # TODO(pas-ha) use auth plugin from context when it is available + user_auth = token_endpoint.Token(endpoint, context.auth_token) + return service_token.ServiceTokenAuthWrapper(user_auth=user_auth, + service_auth=service_auth) + + +# NOTE(pas-ha) Used by neutronclient only # FIXME(pas-ha) remove this while moving to kesytoneauth adapters @ks_exceptions def get_service_url(session, **kwargs): diff --git a/ironic/conf/glance.py b/ironic/conf/glance.py index 7d5393b659..089bf839d2 100644 --- a/ironic/conf/glance.py +++ b/ironic/conf/glance.py @@ -104,14 +104,17 @@ opts = [ 'multiple containers to store images, and this value ' 'will determine how many containers are created.')), cfg.ListOpt('glance_api_servers', + deprecated_for_removal=True, + deprecated_reason=_("Use [glance]/endpoint_override option " + "to set the full load-balanced glance API " + "URL instead."), help=_('A list of the glance api servers available to ironic. ' 'Prefix with https:// for SSL-based glance API ' - 'servers. Format is [hostname|IP]:port. ' - 'If this option is not set, the service ' - 'catalog is used. It is recommended to rely on the ' - 'service catalog, if possible.')), + 'servers. Format is [hostname|IP]:port.')), cfg.BoolOpt('glance_api_insecure', default=False, + deprecated_for_removal=True, + deprecated_reason=_("Use [glance]/insecure option instead."), help=_('Allow to perform insecure SSL (https) requests to ' 'glance.')), cfg.IntOpt('glance_num_retries', @@ -121,9 +124,16 @@ opts = [ cfg.StrOpt('auth_strategy', default='keystone', choices=['keystone', 'noauth'], + deprecated_for_removal=True, + deprecated_reason=_("To configure glance in noauth mode, " + "set [glance]/auth_type=none and " + "[glance]/endpoint_override=" + " instead."), help=_('Authentication strategy to use when connecting to ' 'glance.')), cfg.StrOpt('glance_cafile', + deprecated_for_removal=True, + deprecated_reason=_("Use [glance]/cafile option instead."), help=_('Optional path to a CA certificate bundle to be used to ' 'validate the SSL certificate served by glance. It is ' 'used when glance_api_insecure is set to False.')), @@ -138,8 +148,8 @@ opts = [ def register_opts(conf): conf.register_opts(opts, group='glance') - auth.register_auth_opts(conf, 'glance') + auth.register_auth_opts(conf, 'glance', service_type='image') def list_opts(): - return auth.add_auth_opts(opts) + return auth.add_auth_opts(opts, service_type='image') diff --git a/ironic/drivers/modules/image_cache.py b/ironic/drivers/modules/image_cache.py index 0a5f8723fd..0b3133c8f0 100644 --- a/ironic/drivers/modules/image_cache.py +++ b/ironic/drivers/modules/image_cache.py @@ -92,7 +92,7 @@ class ImageCache(object): # NOTE(vdrok): File name is converted to UUID if it's not UUID already, # so that two images with same file names do not collide if service_utils.is_glance_image(href): - master_file_name = service_utils.parse_image_ref(href)[0] + master_file_name = service_utils.parse_image_id(href) else: # NOTE(vdrok): Doing conversion of href in case it's unicode # string, UUID cannot be generated for unicode strings on python 2. diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index f8f5e1a88c..b449ee7592 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -19,6 +19,7 @@ import time from glanceclient import client as glance_client from glanceclient import exc as glance_exc +from keystoneauth1 import loading as kaloading import mock from oslo_config import cfg from oslo_utils import uuidutils @@ -98,15 +99,7 @@ class TestGlanceImageService(base.TestCase): self.service = service.GlanceImageService(client, 1, self.context) self.config(glance_api_servers=['http://localhost'], group='glance') - try: - self.config(auth_strategy='keystone', group='glance') - except Exception: - opts = [ - cfg.StrOpt('auth_strategy', default='keystone'), - ] - CONF.register_opts(opts) - - return + self.config(auth_strategy='keystone', group='glance') @staticmethod def _make_fixture(**kwargs): @@ -196,7 +189,7 @@ class TestGlanceImageService(base.TestCase): stub_context.user_id = 'fake' stub_context.project_id = 'fake' stub_service = service.GlanceImageService(stub_client, 1, stub_context) - image_id = 1 # doesn't matter + image_id = uuidutils.generate_uuid() writer = NullWriter() # When retries are disabled, we should get an exception @@ -233,7 +226,7 @@ class TestGlanceImageService(base.TestCase): stub_service = service.GlanceImageService(stub_client, context=stub_context, version=2) - image_id = 1 # doesn't matter + image_id = uuidutils.generate_uuid() self.config(allowed_direct_url_schemes=['file'], group='glance') @@ -268,7 +261,7 @@ class TestGlanceImageService(base.TestCase): stub_context.user_id = 'fake' stub_context.project_id = 'fake' stub_service = service.GlanceImageService(stub_client, 1, stub_context) - image_id = 1 # doesn't matter + image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotAuthorized, stub_service.download, image_id, writer) @@ -284,7 +277,7 @@ class TestGlanceImageService(base.TestCase): stub_context.user_id = 'fake' stub_context.project_id = 'fake' stub_service = service.GlanceImageService(stub_client, 1, stub_context) - image_id = 1 # doesn't matter + image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotAuthorized, stub_service.download, image_id, writer) @@ -300,7 +293,7 @@ class TestGlanceImageService(base.TestCase): stub_context.user_id = 'fake' stub_context.project_id = 'fake' stub_service = service.GlanceImageService(stub_client, 1, stub_context) - image_id = 1 # doesn't matter + image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotFound, stub_service.download, image_id, writer) @@ -316,12 +309,44 @@ class TestGlanceImageService(base.TestCase): stub_context.user_id = 'fake' stub_context.project_id = 'fake' stub_service = service.GlanceImageService(stub_client, 1, stub_context) - image_id = 1 # doesn't matter + image_id = uuidutils.generate_uuid() writer = NullWriter() self.assertRaises(exception.ImageNotFound, stub_service.download, image_id, writer) - def test_check_image_service_client_set(self): + +@mock.patch('ironic.common.keystone.get_auth', autospec=True, + return_value=mock.sentinel.auth) +@mock.patch('ironic.common.keystone.get_service_auth', autospec=True, + return_value=mock.sentinel.sauth) +@mock.patch('ironic.common.keystone.get_adapter', autospec=True) +@mock.patch('ironic.common.keystone.get_session', autospec=True, + return_value=mock.sentinel.session) +@mock.patch.object(glance_client, 'Client', autospec=True) +class CheckImageServiceTestCase(base.TestCase): + def setUp(self): + super(CheckImageServiceTestCase, self).setUp() + self.context = context.RequestContext(global_request_id='global') + self.service = service.GlanceImageService(None, 1, self.context) + # NOTE(pas-ha) register keystoneauth dynamic options manually + plugin = kaloading.get_plugin_loader('password') + opts = kaloading.get_auth_plugin_conf_options(plugin) + self.cfg_fixture.register_opts(opts, group='glance') + self.config(auth_type='password', + auth_url='viking', + username='spam', + password='ham', + project_name='parrot', + service_type='image', + region_name='SomeRegion', + interface='internal', + auth_strategy='keystone', + group='glance') + base_image_service._GLANCE_SESSION = None + + def test_check_image_service_client_already_set(self, mock_gclient, + mock_sess, mock_adapter, + mock_sauth, mock_auth): def func(self): return True @@ -329,70 +354,118 @@ class TestGlanceImageService(base.TestCase): wrapped_func = base_image_service.check_image_service(func) self.assertTrue(wrapped_func(self.service)) + self.assertEqual(0, mock_gclient.call_count) + self.assertEqual(0, mock_sess.call_count) + self.assertEqual(0, mock_adapter.call_count) + self.assertEqual(0, mock_auth.call_count) + self.assertEqual(0, mock_sauth.call_count) - @mock.patch.object(glance_client, 'Client', autospec=True) - def test_check_image_service__no_client_set_http(self, mock_gclient): - def func(service, *args, **kwargs): - return (self.endpoint, args, kwargs) - - endpoint = 'http://123.123.123.123:9292' - mock_gclient.return_value.endpoint = endpoint - self.service.client = None - - params = {'image_href': '%s/image_uuid' % endpoint} - self.config(auth_strategy='keystone', group='glance') - wrapped_func = base_image_service.check_image_service(func) - self.assertEqual((endpoint, (), params), - wrapped_func(self.service, **params)) + def _assert_client_call(self, mock_gclient, url, user=False): mock_gclient.assert_called_once_with( - 1, endpoint, - **{'insecure': CONF.glance.glance_api_insecure, - 'token': self.context.auth_token}) + 1, + session=mock.sentinel.session, + global_request_id='global', + auth=mock.sentinel.sauth if user else mock.sentinel.auth, + endpoint_override=url) - @mock.patch.object(glance_client, 'Client', autospec=True) - def test_get_image_service__no_client_set_https_insecure(self, - mock_gclient): + def test_check_image_service__config_auth(self, mock_gclient, mock_sess, + mock_adapter, mock_sauth, + mock_auth): def func(service, *args, **kwargs): - return (self.endpoint, args, kwargs) + return args, kwargs - endpoint = 'https://123.123.123.123:9292' - mock_gclient.return_value.endpoint = endpoint - self.service.client = None + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'glance_url' + uuid = uuidutils.generate_uuid() + params = {'image_href': uuid} - params = {'image_href': '%s/image_uuid' % endpoint} - self.config(auth_strategy='keystone', group='glance') - self.config(glance_api_insecure=True, group='glance') wrapped_func = base_image_service.check_image_service(func) + self.assertEqual(((), params), wrapped_func(self.service, **params)) + self._assert_client_call(mock_gclient, 'glance_url') + mock_auth.assert_called_once_with('glance') + mock_sess.assert_called_once_with('glance') + mock_adapter.assert_called_once_with('glance', + session=mock.sentinel.session, + auth=mock.sentinel.auth) + adapter.get_endpoint.assert_called_once_with() + self.assertEqual(0, mock_sauth.call_count) - self.assertEqual((endpoint, (), params), - wrapped_func(self.service, **params)) - mock_gclient.assert_called_once_with( - 1, endpoint, - **{'insecure': CONF.glance.glance_api_insecure, - 'token': self.context.auth_token}) - - @mock.patch.object(glance_client, 'Client', autospec=True) - def test_get_image_service__no_client_set_https_secure(self, mock_gclient): + def test_check_image_service__token_auth(self, mock_gclient, mock_sess, + mock_adapter, mock_sauth, + mock_auth): def func(service, *args, **kwargs): - return (self.endpoint, args, kwargs) + return args, kwargs - endpoint = 'https://123.123.123.123:9292' - mock_gclient.return_value.endpoint = endpoint - self.service.client = None + self.service.context = context.RequestContext( + auth_token='token', global_request_id='global') + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'glance_url' + uuid = uuidutils.generate_uuid() + params = {'image_href': uuid} - params = {'image_href': '%s/image_uuid' % endpoint} - self.config(auth_strategy='keystone', group='glance') - self.config(glance_api_insecure=False, group='glance') - self.config(glance_cafile='/path/to/certfile', group='glance') wrapped_func = base_image_service.check_image_service(func) + self.assertEqual(((), params), wrapped_func(self.service, **params)) + self._assert_client_call(mock_gclient, 'glance_url', user=True) + mock_sess.assert_called_once_with('glance') + mock_adapter.assert_called_once_with('glance', + session=mock.sentinel.session, + auth=mock.sentinel.auth) + mock_sauth.assert_called_once_with(self.service.context, 'glance_url', + mock.sentinel.auth) + mock_auth.assert_called_once_with('glance') - self.assertEqual((endpoint, (), params), - wrapped_func(self.service, **params)) - mock_gclient.assert_called_once_with( - 1, endpoint, - **{'cacert': CONF.glance.glance_cafile, - 'insecure': CONF.glance.glance_api_insecure, - 'token': self.context.auth_token}) + def test_check_image_service__deprecated_opts(self, mock_gclient, + mock_sess, mock_adapter, + mock_sauth, mock_auth): + def func(service, *args, **kwargs): + return args, kwargs + + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'glance_url' + uuid = uuidutils.generate_uuid() + params = {'image_href': uuid} + self.config(glance_api_servers='https://localhost:1234', + glance_api_insecure=True, + glance_cafile='cafile', + region_name=None, + group='glance') + self.config(region_name='OtherRegion', group='keystone') + + wrapped_func = base_image_service.check_image_service(func) + self.assertEqual(((), params), wrapped_func(self.service, **params)) + self.assertEqual('https://localhost:1234', + base_image_service.CONF.glance.endpoint_override) + self._assert_client_call(mock_gclient, 'glance_url') + mock_sess.assert_called_once_with('glance', insecure=True, + cacert='cafile') + mock_adapter.assert_called_once_with( + 'glance', session=mock.sentinel.session, + auth=mock.sentinel.auth, region_name='OtherRegion') + self.assertEqual(0, mock_sauth.call_count) + mock_auth.assert_called_once_with('glance') + + def test_check_image_service__no_auth(self, mock_gclient, mock_sess, + mock_adapter, mock_sauth, mock_auth): + def func(service, *args, **kwargs): + return args, kwargs + + self.config(endpoint_override='foo', + auth_strategy='noauth', + group='glance') + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'foo' + uuid = uuidutils.generate_uuid() + params = {'image_href': uuid} + + wrapped_func = base_image_service.check_image_service(func) + self.assertEqual(((), params), wrapped_func(self.service, **params)) + self.assertEqual('none', base_image_service.CONF.glance.auth_type) + self._assert_client_call(mock_gclient, 'foo') + mock_sess.assert_called_once_with('glance') + mock_adapter.assert_called_once_with('glance', + session=mock.sentinel.session, + auth=mock.sentinel.auth) + self.assertEqual(0, mock_sauth.call_count) def _create_failing_glance_client(info): @@ -811,19 +884,43 @@ class TestSwiftTempUrlCache(base.TestCase): class TestServiceUtils(base.TestCase): - def test_parse_image_ref_no_ssl(self): - image_href = u'http://127.0.0.1:9292/image_path/'\ - u'image_\u00F9\u00FA\u00EE\u0111' - parsed_href = service_utils.parse_image_ref(image_href) - self.assertEqual((u'image_\u00F9\u00FA\u00EE\u0111', - 'http://127.0.0.1:9292', False), parsed_href) + def setUp(self): + super(TestServiceUtils, self).setUp() + service_utils._GLANCE_API_SERVER = None - def test_parse_image_ref_ssl(self): - image_href = 'https://127.0.0.1:9292/image_path/'\ - u'image_\u00F9\u00FA\u00EE\u0111' - parsed_href = service_utils.parse_image_ref(image_href) - self.assertEqual((u'image_\u00F9\u00FA\u00EE\u0111', - 'https://127.0.0.1:9292', True), parsed_href) + def test_parse_image_id_from_uuid(self): + image_href = uuidutils.generate_uuid() + parsed_id = service_utils.parse_image_id(image_href) + self.assertEqual(image_href, parsed_id) + + def test_parse_image_id_from_glance(self): + uuid = uuidutils.generate_uuid() + image_href = u'glance://some-stuff/%s' % uuid + parsed_id = service_utils.parse_image_id(image_href) + self.assertEqual(uuid, parsed_id) + + def test_parse_image_id_from_glance_fail(self): + self.assertRaises(exception.InvalidImageRef, + service_utils.parse_image_id, u'glance://not-a-uuid') + + def test_parse_image_id_fail(self): + self.assertRaises(exception.InvalidImageRef, + service_utils.parse_image_id, + u'http://spam.ham/eggs') + + def test_get_glance_api_server_fail(self): + self.assertRaises(exception.InvalidImageRef, + service_utils.get_glance_api_server, + u'http://spam.ham/eggs') + + # TODO(pas-ha) remove in Rocky + def test_get_glance_api_server(self): + self.config(glance_api_servers='http://spam:1234, https://ham', + group='glance') + api_servers = {service_utils.get_glance_api_server( + uuidutils.generate_uuid()) for i in range(2)} + self.assertEqual({'http://spam:1234', 'https://ham'}, + api_servers) def test_is_glance_image(self): image_href = u'uui\u0111' @@ -850,51 +947,3 @@ class TestServiceUtils(base.TestCase): u'file://\u0111eploy_iso',): result = service_utils.is_image_href_ordinary_file_name(image) self.assertFalse(result) - - -class TestGlanceAPIServers(base.TestCase): - - def setUp(self): - super(TestGlanceAPIServers, self).setUp() - service_utils._GLANCE_API_SERVER = None - - @mock.patch.object(service_utils.keystone, 'get_service_url', - autospec=True) - @mock.patch.object(service_utils.keystone, 'get_session', autospec=True) - @mock.patch.object(service_utils.keystone, 'get_auth', autospec=True) - def test__get_api_servers_with_keystone(self, mock_auth, mock_session, - mock_service_url): - mock_service_url.return_value = 'https://example.com' - - endpoint = service_utils._get_api_server() - self.assertEqual('https://example.com', endpoint) - - mock_auth.assert_called_once_with('glance') - mock_session.assert_called_once_with('glance', - auth=mock_auth.return_value) - mock_service_url.assert_called_once_with(mock_session.return_value, - service_type='image', - endpoint_type='public') - - def test__get_api_servers_one(self): - CONF.set_override('glance_api_servers', ['https://10.0.0.1:9293'], - 'glance') - s1 = service_utils._get_api_server() - s2 = service_utils._get_api_server() - self.assertEqual('https://10.0.0.1:9293', s1) - - # Only one server, should always get the same one - self.assertEqual(s1, s2) - - def test__get_api_servers_two(self): - CONF.set_override('glance_api_servers', - ['http://10.0.0.1:9293', 'http://10.0.0.2:9294'], - 'glance') - s1 = service_utils._get_api_server() - s2 = service_utils._get_api_server() - s3 = service_utils._get_api_server() - - self.assertNotEqual(s1, s2) - - # 2 servers, so cycles to the first again - self.assertEqual(s1, s3) diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py index 6e7e522b60..c3a61432b7 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/ironic/tests/unit/common/test_image_service.py @@ -16,6 +16,7 @@ import shutil import mock from oslo_config import cfg +from oslo_utils import uuidutils import requests import sendfile import six @@ -265,73 +266,30 @@ class FileImageServiceTestCase(base.TestCase): class ServiceGetterTestCase(base.TestCase): - @mock.patch.object(image_service, '_get_glance_session', autospec=True) @mock.patch.object(glance_v2_service.GlanceImageService, '__init__', return_value=None, autospec=True) - def test_get_glance_image_service(self, glance_service_mock, - session_mock): - image_href = 'image-uuid' - self.context.auth_token = 'fake' + def test_get_glance_image_service(self, glance_service_mock): + image_href = uuidutils.generate_uuid() image_service.get_image_service(image_href, context=self.context) glance_service_mock.assert_called_once_with(mock.ANY, None, 2, self.context) - self.assertFalse(session_mock.called) - @mock.patch.object(image_service, '_get_glance_session', autospec=True) @mock.patch.object(glance_v1_service.GlanceImageService, '__init__', return_value=None, autospec=True) - def test_get_glance_image_service_default_v1(self, glance_service_mock, - session_mock): + def test_get_glance_image_service_default_v1(self, glance_service_mock): self.config(glance_api_version=1, group='glance') - image_href = 'image-uuid' - self.context.auth_token = 'fake' + image_href = uuidutils.generate_uuid() image_service.get_image_service(image_href, context=self.context) glance_service_mock.assert_called_once_with(mock.ANY, None, 1, self.context) - self.assertFalse(session_mock.called) - @mock.patch.object(image_service, '_get_glance_session', autospec=True) @mock.patch.object(glance_v2_service.GlanceImageService, '__init__', return_value=None, autospec=True) - def test_get_glance_image_service_url(self, glance_service_mock, - session_mock): - image_href = 'glance://image-uuid' - self.context.auth_token = 'fake' + def test_get_glance_image_service_url(self, glance_service_mock): + image_href = 'glance://%s' % uuidutils.generate_uuid() image_service.get_image_service(image_href, context=self.context) glance_service_mock.assert_called_once_with(mock.ANY, None, 2, self.context) - self.assertFalse(session_mock.called) - - @mock.patch.object(image_service, '_get_glance_session', autospec=True) - @mock.patch.object(glance_v2_service.GlanceImageService, '__init__', - return_value=None, autospec=True) - def test_get_glance_image_service_no_token(self, glance_service_mock, - session_mock): - image_href = 'image-uuid' - self.context.auth_token = None - sess = mock.Mock() - sess.get_token.return_value = 'admin-token' - session_mock.return_value = sess - image_service.get_image_service(image_href, context=self.context) - glance_service_mock.assert_called_once_with(mock.ANY, None, 2, - self.context) - sess.get_token.assert_called_once_with() - self.assertEqual('admin-token', self.context.auth_token) - - @mock.patch.object(image_service, '_get_glance_session', autospec=True) - @mock.patch.object(glance_v2_service.GlanceImageService, '__init__', - return_value=None, autospec=True) - def test_get_glance_image_service_token_not_needed(self, - glance_service_mock, - session_mock): - image_href = 'image-uuid' - self.context.auth_token = None - self.config(auth_strategy='noauth', group='glance') - image_service.get_image_service(image_href, context=self.context) - glance_service_mock.assert_called_once_with(mock.ANY, None, 2, - self.context) - self.assertFalse(session_mock.called) - self.assertIsNone(self.context.auth_token) @mock.patch.object(image_service.HttpImageService, '__init__', return_value=None, autospec=True) @@ -354,10 +312,13 @@ class ServiceGetterTestCase(base.TestCase): image_service.get_image_service(image_href) local_service_mock.assert_called_once_with() - def test_get_image_service_unknown_protocol(self): - image_href = 'usenet://alt.binaries.dvd/image.qcow2' - self.assertRaises(exception.ImageRefValidationFailed, - image_service.get_image_service, image_href) + def test_get_image_service_invalid_image_ref(self): + invalid_refs = ( + 'usenet://alt.binaries.dvd/image.qcow2', + 'no scheme, no uuid') + for image_ref in invalid_refs: + self.assertRaises(exception.ImageRefValidationFailed, + image_service.get_image_service, image_ref) def test_out_range_auth_strategy(self): self.assertRaises(ValueError, cfg.CONF.set_override, diff --git a/ironic/tests/unit/common/test_keystone.py b/ironic/tests/unit/common/test_keystone.py index 31c85baad0..9babe028c4 100644 --- a/ironic/tests/unit/common/test_keystone.py +++ b/ironic/tests/unit/common/test_keystone.py @@ -17,6 +17,7 @@ import mock from oslo_config import cfg from oslo_config import fixture +from ironic.common import context from ironic.common import exception from ironic.common import keystone from ironic.conf import auth as ironic_auth @@ -92,3 +93,14 @@ class KeystoneTestCase(base.TestCase): interface='admin') self.assertEqual('admin', adapter.interface) self.assertEqual(session, adapter.session) + + @mock.patch('keystoneauth1.service_token.ServiceTokenAuthWrapper') + @mock.patch('keystoneauth1.token_endpoint.Token') + def test_get_service_auth(self, token_mock, service_auth_mock): + ctxt = context.RequestContext(auth_token='spam') + mock_auth = mock.Mock() + self.assertEqual(service_auth_mock.return_value, + keystone.get_service_auth(ctxt, 'ham', mock_auth)) + token_mock.assert_called_once_with('ham', 'spam') + service_auth_mock.assert_called_once_with( + user_auth=token_mock.return_value, service_auth=mock_auth) diff --git a/ironic/tests/unit/drivers/modules/test_image_cache.py b/ironic/tests/unit/drivers/modules/test_image_cache.py index eb12cfbd32..b50d3fe40d 100644 --- a/ironic/tests/unit/drivers/modules/test_image_cache.py +++ b/ironic/tests/unit/drivers/modules/test_image_cache.py @@ -38,8 +38,6 @@ def touch(filename): open(filename, 'w').close() -@mock.patch('ironic.common.glance_service.service_utils.parse_image_ref', - lambda image: (image, 'example.com', True)) class TestImageCacheFetch(base.TestCase): def setUp(self): diff --git a/releasenotes/notes/deprecate-glance-url-scheme-ceff3008cf9cf590.yaml b/releasenotes/notes/deprecate-glance-url-scheme-ceff3008cf9cf590.yaml new file mode 100644 index 0000000000..f1cbdb3284 --- /dev/null +++ b/releasenotes/notes/deprecate-glance-url-scheme-ceff3008cf9cf590.yaml @@ -0,0 +1,7 @@ +--- +other: + - | + Support for parsing the glance API endpoint from the full REST path + to a glance image was removed as it was not working anyway. + The image service API is now always resolved from keystone catalog or + via the options in the ``[glance]`` section in ironic configuration file. diff --git a/releasenotes/notes/deprecated-glance-opts-4825f000d20c2932.yaml b/releasenotes/notes/deprecated-glance-opts-4825f000d20c2932.yaml new file mode 100644 index 0000000000..037851a2b1 --- /dev/null +++ b/releasenotes/notes/deprecated-glance-opts-4825f000d20c2932.yaml @@ -0,0 +1,31 @@ +--- +deprecations: + - | + Configuration option ``glance_api_servers`` from the ``[glance]`` + section in the configuration file is deprecated + and will be ignored in the Rocky release. + Instead, use ``[glance]/endpoint_override`` configuration option to set + a specific (possibly load-balanced) glance API address when automatic + discovery of glance API endpoint from keystone catalog is not desired. + This new option defaults to ``None`` and must be set explicitly if needed. + This new option is mostly suited for standalone ironic deployments without + keystone and its service catalog, and it is generally recommended to + rely on keystone service catalog for service endpoint discovery. + + - | + Configuration option ``[glance]/glance_api_insecure`` is deprecated + and will be ignored in the Rocky release. + Instead, use ``[glance]/insecure`` configuration option + (its default is ``False``). + + - | + Configuration option ``[glance]/glance_cafile`` is deprecated + and will be ignored in the Rocky release. + Instead, use ``[glance]/cafile`` configuration option + (its default is ``None``). + - | + Configuration option ``[glance]/auth_strategy`` is deprecated + and will be ignored in the Rocky release. + Instead, to setup glance in noauth mode set ``[glance]/auth_type`` + configuration option to ``none`` and provide glance API address as + ``[glance]/endpoint_override`` configuration option. diff --git a/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml b/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml new file mode 100644 index 0000000000..164fee987b --- /dev/null +++ b/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds the ability to set keystoneauth settings in the + ``[glance]`` configuration section for service automatic + discovery.