From f64a9f638fbab51292c03c583cf374252036dd03 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 19 Sep 2017 21:10:12 -0700 Subject: [PATCH] oci_discovery/fetch_json: Return the final URI along with the JSON From [1]: Note that if the retrieval was the result of a redirected request, the last URI used (i.e., the URI that resulted in the actual retrieval of the representation) is the base URI. This fills in remaining Python functionality left off from 5b75a01 (*-template: Cover relative references, 2017-09-18, #21). Ref-engine discovery and ref-engine APIs now also return dicts that include a 'uri' key representing this URI, which allows their consumers to also perform reference resolution. For example, tools consuming the Merkle roots may need the URI from which those roots were extracted in order to resolve a CAS-engine URI reference. [1]: https://tools.ietf.org/html/rfc3986#section-5.1.3 --- README.md | 33 +++++++------- oci_discovery/fetch_json/__init__.py | 27 ++++++++---- oci_discovery/fetch_json/test.py | 44 +++++++++++++++---- oci_discovery/ref_engine/dummy.py | 3 +- .../ref_engine/oci_index_template.py | 8 +++- .../ref_engine/test_oci_index_template.py | 30 ++++++++++--- .../ref_engine_discovery/__init__.py | 5 ++- oci_discovery/ref_engine_discovery/test.py | 34 +++++++++++--- 8 files changed, 136 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index db012d4..70f52ec 100644 --- a/README.md +++ b/README.md @@ -44,22 +44,25 @@ $ python3 -m oci_discovery.ref_engine_discovery -l debug example.com/app#1.0 2>/ "example.com/app#1.0": { "roots": [ { - "annotations": { - "org.opencontainers.image.ref.name": "1.0" + "root": { + "annotations": { + "org.opencontainers.image.ref.name": "1.0" + }, + "casEngines": [ + { + "protocol": "oci-cas-template-v1", + "uri": "https://a.example.com/cas/{algorithm}/{encoded:2}/{encoded}" + } + ], + "digest": "sha256:e9770a03fbdccdd4632895151a93f9af58bbe2c91fdfaaf73160648d250e6ec3", + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "platform": { + "architecture": "ppc64le", + "os": "linux" + }, + "size": 799 }, - "casEngines": [ - { - "protocol": "oci-cas-template-v1", - "uri": "https://a.example.com/cas/{algorithm}/{encoded:2}/{encoded}" - } - ], - "digest": "sha256:e9770a03fbdccdd4632895151a93f9af58bbe2c91fdfaaf73160648d250e6ec3", - "mediaType": "application/vnd.oci.image.manifest.v1+json", - "platform": { - "architecture": "ppc64le", - "os": "linux" - }, - "size": 799 + "uri": "http://example.com/oci-index/app" } ] } diff --git a/oci_discovery/fetch_json/__init__.py b/oci_discovery/fetch_json/__init__.py index 713023a..4a7625e 100644 --- a/oci_discovery/fetch_json/__init__.py +++ b/oci_discovery/fetch_json/__init__.py @@ -13,18 +13,26 @@ # limitations under the License. import json as _json +import logging as _logging import urllib.request as _urllib_request +_LOGGER = _logging.getLogger(__name__) + + def fetch(uri, media_type='application/json'): """Fetch a JSON resource.""" - response = _urllib_request.urlopen(uri) - content_type = response.headers.get_content_type() - if content_type != media_type: - raise ValueError( - '{} returned {}, not {}'.format(uri, content_type, media_type)) - body_bytes = response.read() - charset = response.headers.get_content_charset() + with _urllib_request.urlopen(uri) as response: + content_type = response.headers.get_content_type() + if content_type != media_type: + raise ValueError( + '{} returned {}, not {}'.format(uri, content_type, media_type)) + body_bytes = response.read() + charset = response.headers.get_content_charset() + finalURI = response.geturl() + if finalURI != uri: + _LOGGER.debug('redirects lead from {} to {}'.format(uri, finalURI)) + uri = finalURI if charset is None: raise ValueError('{} does not declare a charset'.format(uri)) try: @@ -34,6 +42,9 @@ def fetch(uri, media_type='application/json'): '{} returned content which did not match the declared {} charset' .format(uri, charset)) from error try: - return _json.loads(body) + return { + 'uri': uri, + 'json': _json.loads(body), + } except ValueError as error: raise ValueError('{} returned invalid JSON'.format(uri)) from error diff --git a/oci_discovery/fetch_json/test.py b/oci_discovery/fetch_json/test.py index 9f628f9..bd1ede1 100644 --- a/oci_discovery/fetch_json/test.py +++ b/oci_discovery/fetch_json/test.py @@ -19,24 +19,47 @@ from . import fetch +class ContextManager(object): + def __init__(self, target, return_value): + self._target = target + self._return_value = return_value + + def __enter__(self): + context = unittest.mock.MagicMock() + context.__enter__ = lambda a: self._return_value + context.__exit__ = lambda a, b, c, d: None + self._patch = unittest.mock.patch( + target=self._target, return_value=context) + return self._patch.__enter__() + + def __exit__(self, *args, **kwargs): + self._patch.__exit__(*args, **kwargs) + + class HTTPResponse(object): - def __init__(self, code=200, body=None, headers=None): + def __init__(self, url, code=200, body=None, headers=None): + self._url = url self.code = code self._body = body self.headers = email.message.Message() for key, value in headers.items(): self.headers[key] = value + def geturl(self): + return self._url + def read(self): return self._body or '' class TestFetchJSON(unittest.TestCase): def test_good(self): + uri = 'https://example.com' for name, response, expected in [ ( 'empty object', HTTPResponse( + url=uri, body=b'{}', headers={ 'Content-Type': 'application/json; charset=UTF-8', @@ -47,6 +70,7 @@ def test_good(self): ( 'basic object', HTTPResponse( + url=uri, body=b'{"a": "b", "c": 1}', headers={ 'Content-Type': 'application/json; charset=UTF-8', @@ -56,18 +80,19 @@ def test_good(self): ), ]: with self.subTest(name=name): - with unittest.mock.patch( - target='oci_discovery.fetch_json._urllib_request.urlopen', - return_value=response - ) as patch_context: - json = fetch(uri='https://example.com') - self.assertEqual(json, expected) + with ContextManager( + target='oci_discovery.fetch_json._urllib_request.urlopen', + return_value=response): + fetched = fetch(uri=uri) + self.assertEqual(fetched, {'uri': uri, 'json': expected}) def test_bad(self): + uri = 'https://example.com' for name, response, error, regex in [ ( 'no charset', HTTPResponse( + url=uri, body=b'{}', headers={ 'Content-Type': 'application/json', @@ -79,6 +104,7 @@ def test_bad(self): ( 'declared charset does not match body', HTTPResponse( + url=uri, body=b'\xff', headers={ 'Content-Type': 'application/json; charset=UTF-8', @@ -90,6 +116,7 @@ def test_bad(self): ( 'invalid JSON', HTTPResponse( + url=uri, body=b'{', headers={ 'Content-Type': 'application/json; charset=UTF-8', @@ -101,6 +128,7 @@ def test_bad(self): ( 'unexpected media type', HTTPResponse( + url=uri, body=b'{}', headers={ 'Content-Type': 'text/plain; charset=UTF-8', @@ -111,7 +139,7 @@ def test_bad(self): ), ]: with self.subTest(name=name): - with unittest.mock.patch( + with ContextManager( target='oci_discovery.fetch_json._urllib_request.urlopen', return_value=response): self.assertRaisesRegex( diff --git a/oci_discovery/ref_engine/dummy.py b/oci_discovery/ref_engine/dummy.py index ea10e30..b3c47fd 100644 --- a/oci_discovery/ref_engine/dummy.py +++ b/oci_discovery/ref_engine/dummy.py @@ -26,8 +26,9 @@ def __str__(self): self.__class__.__name__, self._response) - def __init__(self, response): + def __init__(self, response, base=None): self._response = response + self.base = base def resolve(self, name): return _copy.deepcopy(self._response) diff --git a/oci_discovery/ref_engine/oci_index_template.py b/oci_discovery/ref_engine/oci_index_template.py index bb0699b..61ccb35 100644 --- a/oci_discovery/ref_engine/oci_index_template.py +++ b/oci_discovery/ref_engine/oci_index_template.py @@ -45,9 +45,10 @@ def resolve(self, name): if self.base: uri = _urllib_parse.urljoin(base=self.base, url=uri) _LOGGER.debug('fetching an OCI index for {} from {}'.format(name, uri)) - index = _fetch_json.fetch( + fetched = _fetch_json.fetch( uri=uri, media_type='application/vnd.oci.image.index.v1+json') + index = fetched['json'] _LOGGER.debug('received OCI index object:\n{}'.format( _pprint.pformat(index))) if not isinstance(index, dict): @@ -72,4 +73,7 @@ def resolve(self, name): 'org.opencontainers.image.ref.name', None) if (name_parts['fragment'] == '' or name_parts['fragment'] == entry_name): - yield entry + yield { + 'uri': fetched['uri'], + 'root': entry, + } diff --git a/oci_discovery/ref_engine/test_oci_index_template.py b/oci_discovery/ref_engine/test_oci_index_template.py index c5f6e66..0125c8a 100644 --- a/oci_discovery/ref_engine/test_oci_index_template.py +++ b/oci_discovery/ref_engine/test_oci_index_template.py @@ -94,14 +94,24 @@ def test_good(self): ), ]: engine = oci_index_template.Engine(uri='https://example.com/index') + responseURI = 'https://x.example.com/y' with self.subTest(label=label): with unittest.mock.patch( target='oci_discovery.ref_engine.oci_index_template._fetch_json.fetch', - return_value=response): + return_value={ + 'uri': responseURI, + 'json': response, + }): resolved = list(engine.resolve(name=name)) - self.assertEqual(resolved, expected) + self.assertEqual( + resolved, + [ + {'uri': responseURI, 'root': root} + for root in expected + ]) def test_bad(self): + uri = 'https://example.com/index' for label, response, error, regex in [ ( 'index is not a JSON object', @@ -128,11 +138,14 @@ def test_bad(self): "https://example.com/index claimed to return application/vnd.oci.image.index.v1\+json, but actually returned \{'manifests': \[\{'annotations': None}]}", ), ]: - engine = oci_index_template.Engine(uri='https://example.com/index') + engine = oci_index_template.Engine(uri=uri) with self.subTest(label=label): with unittest.mock.patch( target='oci_discovery.ref_engine.oci_index_template._fetch_json.fetch', - return_value=response): + return_value={ + 'uri': uri, + 'json': response, + }): generator = engine.resolve(name='example.com/a') self.assertRaisesRegex(error, regex, list, generator) @@ -193,9 +206,14 @@ def test_reference_expansion(self): engine = oci_index_template.Engine(uri=uri, base=base) with unittest.mock.patch( target='oci_discovery.ref_engine.oci_index_template._fetch_json.fetch', - return_value=response) as mock: + return_value={ + 'uri': expected, + 'json': response + }) as mock: resolved = list(engine.resolve(name='example.com/a#1.0')) mock.assert_called_with( uri=expected, media_type='application/vnd.oci.image.index.v1+json') - self.assertEqual(resolved, response['manifests']) + self.assertEqual( + resolved, + [{'uri': expected, 'root': root} for root in response['manifests']]) diff --git a/oci_discovery/ref_engine_discovery/__init__.py b/oci_discovery/ref_engine_discovery/__init__.py index 2e09580..e113ffc 100644 --- a/oci_discovery/ref_engine_discovery/__init__.py +++ b/oci_discovery/ref_engine_discovery/__init__.py @@ -40,7 +40,7 @@ def resolve(name, protocols=('https', 'http'), port=None): protocol, host) _LOGGER.debug('discovering ref engines via {}'.format(uri)) try: - ref_engines_object = _fetch_json.fetch( + fetched = _fetch_json.fetch( uri=uri, media_type='application/vnd.oci.ref-engines.v1+json') except (_ssl.CertificateError, @@ -48,6 +48,7 @@ def resolve(name, protocols=('https', 'http'), port=None): _urllib_error.URLError) as error: _LOGGER.warning('failed to fetch {} ({})'.format(uri, error)) continue + ref_engines_object = fetched['json'] _LOGGER.debug('received ref-engine discovery object:\n{}'.format( _pprint.pformat(ref_engines_object))) if not isinstance(ref_engines_object, dict): @@ -58,7 +59,7 @@ def resolve(name, protocols=('https', 'http'), port=None): continue for ref_engine_object in ref_engines_object.get('refEngines', []): try: - ref_engine = _ref_engine.new(**ref_engine_object) + ref_engine = _ref_engine.new(base=fetched['uri'], **ref_engine_object) except KeyError as error: _LOGGER.warning(error) continue diff --git a/oci_discovery/ref_engine_discovery/test.py b/oci_discovery/ref_engine_discovery/test.py index 4c64efd..cd52a91 100644 --- a/oci_discovery/ref_engine_discovery/test.py +++ b/oci_discovery/ref_engine_discovery/test.py @@ -36,26 +36,48 @@ def test(self): { 'protocol': '_dummy', 'response': [ - {'name': 'dummy Merkle root 1'}, - {'name': 'dummy Merkle root 2'}, + { + 'uri': 'https://x.example.com/y', + 'root': {'name': 'dummy Merkle root 1'}, + }, + { + 'uri': 'https://x.example.com/z', + 'root': {'name': 'dummy Merkle root 2'}, + }, ], } ] }, { 'roots': [ - {'name': 'dummy Merkle root 1'}, - {'name': 'dummy Merkle root 2'}, + { + 'uri': 'https://x.example.com/y', + 'root': {'name': 'dummy Merkle root 1'}, + }, + { + 'uri': 'https://x.example.com/z', + 'root': {'name': 'dummy Merkle root 2'}, + }, ], } ), ]: + responseURI = 'https://x.example.com/y' with self.subTest(label=label): with unittest.mock.patch( target='oci_discovery.ref_engine_discovery._fetch_json.fetch', - return_value=response): + return_value={ + 'uri': responseURI, + 'json': response, + }): resolved = resolve(name=name) - self.assertEqual(resolved, expected) + self.assertEqual( + resolved, + [ + {'uri': responseURI, 'root': root} + for root in expected + ]) + def test_bad(self): for label, name, response, error, regex in [