Unit tests for microversion, initial change for consumers

Add unit tests for microversions. This also changes the default
microversion to 1.1, which tells Barbican to return secret
consumers. Because of that, this patch adds consumers to the
Secret object in barbicanclient/v1/secrets.py , so that the
API response is correctly interpreted.

Co-Authored-By: Ade Lee <alee@redhat.com>
Co-Authored-By: Andre Aranha <afariasa@redhat.com>
Co-Authored-By: Grzegorz Grasza <xek@redhat.com>

Change-Id: Ibfaea3fe9e394f6b1286d92437629d0400305968

6 files changed, 409 insertions, 105 deletions

diff --git a/barbicanclient/client.py b/barbicanclient/client.py
index 24e271f..309432e 100644
--- a/barbicanclient/client.py
+++ b/barbicanclient/client.py
@@ -30,14 +30,12 @@ LOG = logging.getLogger(__name__)
 _DEFAULT_SERVICE_TYPE = 'key-manager'
 _DEFAULT_SERVICE_INTERFACE = 'public'
 _DEFAULT_API_VERSION = 'v1'
-# TODO(dmendiza) Default to '1.1'
-_DEFAULT_API_MICROVERSION = '1.0'
 _SUPPORTED_API_VERSION_MAP = {'v1': 'barbicanclient.v1.client.Client'}
 
 
 class _HTTPClient(adapter.Adapter):
 
-    def __init__(self, session, project_id=None, **kwargs):
+    def __init__(self, session, microversion, project_id=None, **kwargs):
         endpoint = kwargs.pop('endpoint', None)
         if endpoint:
             kwargs['endpoint_override'] = "{}/{}/".format(
@@ -46,6 +44,7 @@ class _HTTPClient(adapter.Adapter):
             )
 
         super().__init__(session, **kwargs)
+        self.microversion = microversion
 
         if project_id is None:
             self._default_headers = dict()
@@ -180,7 +179,6 @@ def Client(version=None, session=None, *args, **kwargs):
     kwargs['version'] = version or _DEFAULT_API_VERSION
     kwargs.setdefault('service_type', _DEFAULT_SERVICE_TYPE)
     kwargs.setdefault('interface', _DEFAULT_SERVICE_INTERFACE)
-    kwargs.setdefault('microversion', _DEFAULT_API_MICROVERSION)
 
     try:
         client_path = _SUPPORTED_API_VERSION_MAP[kwargs['version']]
diff --git a/barbicanclient/tests/test_barbican.py b/barbicanclient/tests/test_barbican.py
index ded8ca1..545580d 100644
--- a/barbicanclient/tests/test_barbican.py
+++ b/barbicanclient/tests/test_barbican.py
@@ -49,24 +49,19 @@ class WhenTestingBarbicanCLI(testtools.TestCase):
         self.responses.get(
             'http://localhost:9311/',
             json={
-                "versions": {
-                    "values": [{
-                        "id": "v1",
-                        "status": "stable",
-                        "links": [{
-                            "rel": "self",
-                            "href": "http://localhost:9311/v1/"
-                        }, {
-                            "rel": "describedby",
-                            "type": "text/html",
-                            "href": "https://docs.openstack.org/"
-                        }],
-                        "media-types": [{
-                            "type": "application/vnd.openstack.key-manager-v1"
-                                    "+json",
-                            "base": "application/json",
-                        }]}]}}
-        )
+                "versions": [{
+                    "id": "v1",
+                    "status": "CURRENT",
+                    "min_version": "1.0",
+                    "max_version": "1.1",
+                    "links": [{
+                        "rel": "self",
+                        "href": "http://localhost:9311/v1/"
+                    }, {
+                        "rel": "describedby",
+                        "type": "text/html",
+                        "href": "https://docs.openstack.org/"}]}]})
+
         self.captured_stdout = io.StringIO()
         self.captured_stderr = io.StringIO()
         self.barbican = Barbican(
diff --git a/barbicanclient/tests/test_client.py b/barbicanclient/tests/test_client.py
index 6548f0a..46b5e11 100644
--- a/barbicanclient/tests/test_client.py
+++ b/barbicanclient/tests/test_client.py
@@ -21,6 +21,15 @@ import testtools
 
 from barbicanclient import client
 from barbicanclient import exceptions
+from barbicanclient.exceptions import UnsupportedVersion
+from barbicanclient.tests.utils import get_server_supported_versions
+from barbicanclient.tests.utils import get_version_endpoint
+from barbicanclient.tests.utils import mock_session
+from barbicanclient.tests.utils import mock_session_get
+from barbicanclient.tests.utils import mock_session_get_endpoint
+
+
+_DEFAULT_MICROVERSION = (1, 1)
 
 
 class TestClient(testtools.TestCase):
@@ -47,30 +56,26 @@ class TestClient(testtools.TestCase):
         self.responses.get(
             'http://localhost:9311/',
             json={
-                "versions": {
-                    "values": [{
-                        "id": "v1",
-                        "status": "stable",
-                        "links": [{
-                            "rel": "self",
-                            "href": "http://localhost:9311/v1/"
-                        }, {
-                            "rel": "describedby",
-                            "type": "text/html",
-                            "href": "https://docs.openstack.org/"
-                        }],
-                        "media-types": [{
-                            "type": "application/vnd.openstack.key-manager-v1"
-                                    "+json",
-                            "base": "application/json",
-                        }]}]}}
-        )
+                "versions": [{
+                    "id": "v1",
+                    "status": "CURRENT",
+                    "min_version": "1.0",
+                    "max_version": "1.1",
+                    "links": [{
+                        "rel": "self",
+                        "href": "http://localhost:9311/v1/"
+                    }, {
+                        "rel": "describedby",
+                        "type": "text/html",
+                        "href": "https://docs.openstack.org/"}]}]})
 
         self.project_id = 'project_id'
         self.session = session.Session()
-        self.httpclient = client._HTTPClient(session=self.session,
-                                             endpoint=self.endpoint,
-                                             project_id=self.project_id)
+        self.httpclient = client._HTTPClient(
+            session=self.session,
+            microversion=_DEFAULT_MICROVERSION,
+            endpoint=self.endpoint,
+            project_id=self.project_id)
 
 
 class WhenTestingClientInit(TestClient):
@@ -83,12 +88,14 @@ class WhenTestingClientInit(TestClient):
                          c.client.endpoint_override)
 
     def test_default_headers_are_empty(self):
-        c = client._HTTPClient(session=self.session, endpoint=self.endpoint)
+        c = client._HTTPClient(
+            session=self.session, microversion='1.1', endpoint=self.endpoint)
         self.assertIsInstance(c._default_headers, dict)
         self.assertFalse(bool(c._default_headers))
 
     def test_project_id_is_added_to_default_headers(self):
         c = client._HTTPClient(session=self.session,
+                               microversion=_DEFAULT_MICROVERSION,
                                endpoint=self.endpoint,
                                project_id=self.project_id)
         self.assertIn('X-Project-Id', c._default_headers.keys())
@@ -121,9 +128,11 @@ class WhenTestingClientPost(TestClient):
 
     def setUp(self):
         super(WhenTestingClientPost, self).setUp()
-        self.httpclient = client._HTTPClient(session=self.session,
-                                             endpoint=self.endpoint,
-                                             version='v1')
+        self.httpclient = client._HTTPClient(
+            session=self.session,
+            microversion=_DEFAULT_MICROVERSION,
+            endpoint=self.endpoint,
+            version='v1')
         self.href = self.endpoint + '/v1/secrets/'
         self.post_mock = self.responses.post(self.href, json={})
 
@@ -153,8 +162,10 @@ class WhenTestingClientPut(TestClient):
 
     def setUp(self):
         super(WhenTestingClientPut, self).setUp()
-        self.httpclient = client._HTTPClient(session=self.session,
-                                             endpoint=self.endpoint)
+        self.httpclient = client._HTTPClient(
+            session=self.session,
+            microversion=_DEFAULT_MICROVERSION,
+            endpoint=self.endpoint)
         self.href = 'http://test_href/'
         self.put_mock = self.responses.put(self.href, status_code=204)
 
@@ -184,8 +195,10 @@ class WhenTestingClientGet(TestClient):
 
     def setUp(self):
         super(WhenTestingClientGet, self).setUp()
-        self.httpclient = client._HTTPClient(session=self.session,
-                                             endpoint=self.endpoint)
+        self.httpclient = client._HTTPClient(
+            session=self.session,
+            microversion=_DEFAULT_MICROVERSION,
+            endpoint=self.endpoint)
         self.headers = dict()
         self.href = 'http://test_href/'
         self.get_mock = self.responses.get(self.href, json={})
@@ -242,8 +255,10 @@ class WhenTestingClientDelete(TestClient):
 
     def setUp(self):
         super(WhenTestingClientDelete, self).setUp()
-        self.httpclient = client._HTTPClient(session=self.session,
-                                             endpoint=self.endpoint)
+        self.httpclient = client._HTTPClient(
+            session=self.session,
+            microversion=_DEFAULT_MICROVERSION,
+            endpoint=self.endpoint)
         self.href = 'http://test_href/'
         self.del_mock = self.responses.delete(self.href, status_code=204)
 
@@ -328,3 +343,140 @@ class BaseEntityResource(TestClient):
 
         self.client = client.Client(endpoint=self.endpoint,
                                     project_id=self.project_id)
+
+
+class WhenTestingClientMicroversion(TestClient):
+    def _create_mock_session(
+            self, requested_version, server_max_version, server_min_version,
+            endpoint):
+        sess = mock_session()
+
+        mock_session_get_endpoint(sess, get_version_endpoint(endpoint))
+        mock_session_get(
+            sess, get_server_supported_versions(
+                server_min_version, server_max_version))
+
+        return sess
+
+    def _test_client_creation_with_endpoint(
+            self, requested_version, server_max_version, server_min_version,
+            endpoint):
+        sess = self._create_mock_session(
+            requested_version, server_max_version, server_min_version,
+            endpoint)
+
+        client.Client(session=sess, microversion=requested_version)
+
+        headers = {
+            'Accept': 'application/json',
+            'OpenStack-API-Version': 'key-manager 1.1'
+        }
+
+        sess.get.assert_called_with(
+            get_version_endpoint(endpoint), headers=headers,
+            authenticated=None)
+
+    def _mock_session_and_get_client(
+            self, requested_version, server_max_version, server_min_version,
+            endpoint=None):
+        sess = self._create_mock_session(
+            requested_version, server_max_version, server_min_version,
+            endpoint)
+
+        return client.Client(session=sess, microversion=requested_version)
+
+    def test_fails_when_requesting_invalid_microversion(self):
+        self.assertRaises(TypeError,
+                          client.Client, session=self.session,
+                          endpoint=self.endpoint, project_id=self.project_id,
+                          microversion="a")
+
+    def test_fails_when_requesting_unsupported_microversion(self):
+        self.assertRaises(UnsupportedVersion,
+                          client.Client, session=self.session,
+                          endpoint=self.endpoint, project_id=self.project_id,
+                          microversion="1.9")
+
+    def test_fails_when_requesting_unsupported_version(self):
+        self.assertRaises(UnsupportedVersion,
+                          client.Client, session=self.session,
+                          endpoint=self.endpoint, project_id=self.project_id,
+                          version="v0")
+
+    def test_passes_without_providing_endpoint(self):
+        requested_version = None
+        server_max_version = (1, 1)
+        server_min_version = (1, 0)
+        endpoint = None
+
+        self._test_client_creation_with_endpoint(
+            requested_version, server_max_version, server_min_version,
+            endpoint)
+
+    def test_passes_with_custom_endpoint(self):
+        requested_version = None
+        server_max_version = (1, 1)
+        server_min_version = (1, 0)
+        endpoint = self.endpoint
+
+        self._test_client_creation_with_endpoint(
+            requested_version, server_max_version, server_min_version,
+            endpoint)
+
+    def test_passes_with_default_microversion_as_1_1(self):
+        requested_version = None
+        server_max_version = (1, 1)
+        server_min_version = (1, 0)
+
+        c = self._mock_session_and_get_client(
+            requested_version, server_max_version, server_min_version)
+
+        self.assertEqual("1.1", c.client.microversion)
+
+    def test_passes_with_default_microversion_as_1_0(self):
+        requested_version = None
+        server_max_version = (1, 0)
+        server_min_version = (1, 0)
+
+        c = self._mock_session_and_get_client(
+            requested_version, server_max_version, server_min_version)
+
+        self.assertEqual("1.0", c.client.microversion)
+
+    def test_fails_requesting_higher_microversion_than_supported_by_server(
+            self):
+        requested_version = "1.1"
+        server_max_version = (1, 0)
+        server_min_version = (1, 0)
+
+        sess = self._create_mock_session(
+            requested_version, server_max_version, server_min_version,
+            self.endpoint)
+
+        self.assertRaises(
+            UnsupportedVersion, client.Client, session=sess,
+            endpoint=self.endpoint, microversion=requested_version)
+
+    def test_fails_requesting_lower_microversion_than_supported_by_server(
+            self):
+        requested_version = "1.0"
+        server_max_version = (1, 1)
+        server_min_version = (1, 1)
+
+        sess = self._create_mock_session(
+            requested_version, server_max_version, server_min_version,
+            self.endpoint)
+
+        self.assertRaises(
+            UnsupportedVersion, client.Client, session=sess,
+            endpoint=self.endpoint, microversion=requested_version)
+
+    def test_passes_with_stable_server_version(self):
+        requested_version = "1.0"
+        server_max_version = None
+        server_min_version = None
+
+        c = self._mock_session_and_get_client(
+            requested_version, server_max_version, server_min_version)
+
+        self.assertEqual(requested_version, c.client.microversion)
diff --git a/barbicanclient/tests/utils.py b/barbicanclient/tests/utils.py
new file mode 100644
index 0000000..5e2c067
--- /dev/null
+++ b/barbicanclient/tests/utils.py
@@ -0,0 +1,112 @@
+"""
+Copyright 2022 Red Hat Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+
+from unittest import mock
+
+from keystoneauth1 import identity
+from keystoneauth1 import session
+
+
+_DEFAULT_ENDPOINT = "http://192.168.1.23/key-manager/"
+
+STABLE_RESPONSE = {
+    'version': {
+        'id': 'v1',
+        'status': 'stable',
+        'links': [
+            {
+                'rel': 'self',
+                'href': 'http://192.168.1.23/key-manager/v1/'
+            }, {
+                'rel': 'describedby',
+                'type': 'text/html',
+                'href': 'https://docs.openstack.org/'
+            }],
+        'media-types': [{
+            'base': 'application/json',
+            'type': 'application/vnd.openstack.key-manager-v1+json'
+        }]
+    }
+}
+
+
+def get_custom_current_response(min_version="1.0", max_version="1.1"):
+    return {
+        'version': {
+            'id': 'v1',
+            'status': 'CURRENT',
+            'min_version': min_version,
+            'max_version': max_version,
+            'links': [
+                {
+                    'rel': 'self',
+                    'href': 'http://192.168.1.23/key-manager/v1/'
+                }, {
+                    'rel': 'describedby',
+                    'type': 'text/html',
+                    'href': 'https://docs.openstack.org/'
+                }
+            ]
+        }
+    }
+
+
+def mock_microversion_response(response=STABLE_RESPONSE):
+    response_mock = mock.MagicMock()
+    response_mock.json.return_value = response
+    return response_mock
+
+
+def get_version_endpoint(endpoint=None):
+    return "{}/v1/".format(endpoint or _DEFAULT_ENDPOINT)
+
+
+def mock_session():
+    auth = identity.Password(
+        auth_url="http://localhost/identity/v3",
+        username="username",
+        password="password",
+        project_name="project_name",
+        default_domain_id='default')
+    sess = session.Session(auth=auth)
+    return sess
+
+
+def mock_session_get_endpoint(sess, endpoint_response):
+    sess.get_endpoint = mock.MagicMock()
+    sess.get_endpoint.return_value = endpoint_response
+
+
+def mock_session_get(sess, get_response):
+    response_mock = mock.MagicMock()
+    response_mock.json.return_value = get_response
+
+    sess.get = mock.MagicMock()
+    sess.get.return_value = response_mock
+
+
+def mock_session_with_get_and_get_endpoint(endpoint_response, get_response):
+    sess = mock_session()
+    mock_session_get(get_response)
+    mock_session_get_endpoint(endpoint_response)
+
+    return sess
+
+
+def get_server_supported_versions(min_version, max_version):
+    if min_version and max_version:
+        return get_custom_current_response(min_version, max_version)
+    return STABLE_RESPONSE
diff --git a/barbicanclient/v1/client.py b/barbicanclient/v1/client.py
index 008f99a..f0e91ca 100644
--- a/barbicanclient/v1/client.py
+++ b/barbicanclient/v1/client.py
@@ -16,6 +16,7 @@
 import logging
 
 from keystoneauth1 import discover
+from keystoneauth1.exceptions.http import NotAcceptable
 
 from barbicanclient import client as base_client
 from barbicanclient.v1 import acls
@@ -41,25 +42,27 @@ class Client(object):
         `barbicanclient.client.Client`.  It's recommended to use that
         function instead of making instances of this class directly.
         """
-        microversion = kwargs.pop('microversion', None)
-        if microversion:
-            if not self._validate_microversion(
-                session,
-                kwargs.get('endpoint'),
-                kwargs.get('version'),
-                kwargs.get('service_type'),
-                kwargs.get('service_name'),
-                kwargs.get('interface'),
-                kwargs.get('region_name'),
-                microversion
-            ):
-                raise ValueError(
-                    "Endpoint does not support microversion {}".format(
-                        microversion))
-            kwargs['default_microversion'] = microversion
+        microversion = self._get_normalized_microversion(
+            kwargs.pop('microversion', None))
+        normalized_microversion = self._get_max_supported_version(
+            session,
+            kwargs.get('endpoint'),
+            kwargs.get('version'),
+            kwargs.get('service_type'),
+            kwargs.get('service_name'),
+            kwargs.get('interface'),
+            kwargs.get('region_name'),
+            microversion)
+
+        if normalized_microversion is None:
+            raise ValueError(
+                "Endpoint does not support selected microversion"
+            )
+        kwargs['default_microversion'] = normalized_microversion
 
         # TODO(dmendiza): This should be a private member
-        self.client = base_client._HTTPClient(session=session, *args, **kwargs)
+        self.client = base_client._HTTPClient(
+            session, normalized_microversion, *args, **kwargs)
 
         self.secrets = secrets.SecretManager(self.client)
         self.orders = orders.OrderManager(self.client)
@@ -67,15 +70,43 @@ class Client(object):
         self.cas = cas.CAManager(self.client)
         self.acls = acls.ACLManager(self.client)
 
-    def _validate_microversion(self, session, endpoint, version, service_type,
-                               service_name, interface, region_name,
-                               microversion):
-        # first we make sure that the microversion is something we understand
+    def _get_normalized_microversion(self, microversion):
+        if microversion is None:
+            return
+
+        # We need to make sure that the microversion is something we understand
         normalized = discover.normalize_version_number(microversion)
         if normalized not in _SUPPORTED_MICROVERSIONS:
-            raise ValueError("Invalid microversion {}".format(microversion))
-        microversion = discover.version_to_string(normalized)
-
+            raise ValueError(
+                "Invalid microversion {}: Microversion requested is not "
+                "supported by the client".format(microversion))
+        return discover.version_to_string(normalized)
+
+    def _get_max_supported_version(self, session, endpoint, version,
+                                   service_type, service_name, interface,
+                                   region_name, microversion):
+        min_ver, max_ver = self._get_min_max_server_supported_microversion(
+            session, endpoint, version, service_type, service_name, interface,
+            region_name)
+
+        if microversion is None:
+            for client_version in _SUPPORTED_MICROVERSIONS[::-1]:
+                if discover.version_between(min_ver, max_ver, client_version):
+                    return self._get_normalized_microversion(client_version)
+            raise ValueError(
+                "Couldn't find a version supported by both client and server")
+
+        if discover.version_between(min_ver, max_ver, microversion):
+            return microversion
+
+        raise ValueError(
+            "Invalid microversion {}: Microversion requested is not "
+            "supported by the server".format(microversion))
+
+    def _get_min_max_server_supported_microversion(self, session, endpoint,
+                                                   version, service_type,
+                                                   service_name, interface,
+                                                   region_name):
         if not endpoint:
             endpoint = session.get_endpoint(
                 service_type=service_type,
@@ -85,27 +116,31 @@ class Client(object):
                 version=version
             )
 
-        resp = discover.get_version_data(
-            session, endpoint,
-            version_header='key-manager ' + microversion)
-        if resp:
-            resp = resp[0]
-            status = resp['status'].upper()
-
-            if status == _STABLE:
-                # status is only set to STABLE in two cases
-                # 1. when the server is older and is ignoring the microversion
-                #    header
-                # 2. when we ask for microversion 1.0 and the server
-                #    undertsands the header
-                # in either case min/max will be 1.0
-                min_ver = '1.0'
-                max_ver = '1.0'
-            else:
-                # any other status will have a min/max
-                min_ver = resp['version']['min_version']
-                max_ver = resp['version']['max_version']
-            return discover.version_between(min_ver, max_ver, microversion)
-
-        # TODO(afariasa) What should be returned? error?
-        return False
+        return self._get_min_max_version(session, endpoint, '1.1')
+
+    def _get_min_max_version(self, session, endpoint, microversion):
+        try:
+            # If the microversion requested in the version_header is outside of
+            # the range of microversions supported, return 406 Not Acceptable.
+            resp = discover.get_version_data(
+                session, endpoint,
+                version_header='key-manager ' + microversion)
+        except NotAcceptable:
+            return None, None
+
+        resp = resp[0]
+        status = resp['status'].upper()
+        if status == _STABLE:
+            # status is only set to STABLE in two cases
+            # 1. when the server is older and is ignoring the microversion
+            #    header
+            # 2. when we ask for microversion 1.0 and the server
+            #    understands the header
+            # in either case min/max will be 1.0
+            min_ver = '1.0'
+            max_ver = '1.0'
+        else:
+            # any other status will have a min/max
+            min_ver = resp['min_version']
+            max_ver = resp['max_version']
+        return min_ver, max_ver
diff --git a/barbicanclient/v1/secrets.py b/barbicanclient/v1/secrets.py
index 8cd883e..e09a92f 100644
--- a/barbicanclient/v1/secrets.py
+++ b/barbicanclient/v1/secrets.py
@@ -88,7 +88,7 @@ class Secret(SecretFormatter):
                  payload_content_type=None, payload_content_encoding=None,
                  secret_ref=None, created=None, updated=None,
                  content_types=None, status=None, secret_type=None,
-                 creator_id=None):
+                 creator_id=None, consumers=None):
         """Secret objects should not be instantiated directly.
 
         You should use the `create` or `get` methods of the
@@ -110,7 +110,8 @@ class Secret(SecretFormatter):
             updated=updated,
             content_types=content_types,
             status=status,
-            creator_id=creator_id
+            creator_id=creator_id,
+            consumers=consumers
         )
         self._acl_manager = acl_manager.ACLManager(api)
         self._acls = None
@@ -202,6 +203,15 @@ class Secret(SecretFormatter):
             self._acls = self._acl_manager.get(self.secret_ref)
         return self._acls
 
+    @property
+    @lazy
+    def consumers(self):
+        return self._consumers
+
+    @consumers.setter
+    def consumers(self, value):
+        self._consumers = value
+
     @name.setter
     @immutable_after_save
     def name(self, value):
@@ -375,7 +385,7 @@ class Secret(SecretFormatter):
                         payload=None, payload_content_type=None,
                         payload_content_encoding=None, created=None,
                         updated=None, content_types=None, status=None,
-                        creator_id=None):
+                        creator_id=None, consumers=None):
         self._name = name
         self._algorithm = algorithm
         self._bit_length = bit_length
@@ -385,6 +395,7 @@ class Secret(SecretFormatter):
         self._payload_content_encoding = payload_content_encoding
         self._expiration = expiration
         self._creator_id = creator_id
+        self._consumers = consumers or list()
         if not self._secret_type:
             self._secret_type = "opaque"
         if self._expiration:
@@ -428,7 +439,8 @@ class Secret(SecretFormatter):
                 created=result.get('created'),
                 updated=result.get('updated'),
                 content_types=result.get('content_types'),
-                status=result.get('status')
+                status=result.get('status'),
+                consumers=result.get('consumers', [])
             )
 
     def __repr__(self):