Repository: zalando-stups/pierone-cli

Issue:
Use zign.api.get_token
https://github.com/zalando-stups/zign/pull/41

Discussion:

Patch:
diff --git a/pierone/api.py b/pierone/api.py
index 01523d7..06a97d0 100644
--- a/pierone/api.py
+++ b/pierone/api.py
@@ -8,7 +8,7 @@ import time
 
 import requests
 from clickclick import Action
-from zign.api import get_named_token
+from zign.api import get_token
 
 adapter = requests.adapters.HTTPAdapter(pool_connections=10, pool_maxsize=10)
 session = requests.Session()
@@ -52,23 +52,10 @@ class DockerImage(collections.namedtuple('DockerImage', 'registry team artifact
         return '{}/{}/{}:{}'.format(*tuple(self))
 
 
+# all the other paramaters are deprecated, but still here for compatibility
 def docker_login(url, realm, name, user, password, token_url=None, use_keyring=True, prompt=False):
-    token = None  # Make linters happy
-    with Action('Getting OAuth2 token "{}"..'.format(name)) as action:
-        try:
-            token = get_named_token(['uid', 'application.write'],
-                                    realm, name, user, password, url=token_url,
-                                    use_keyring=use_keyring, prompt=prompt)
-        except requests.HTTPError as error:
-            status_code = error.response.status_code
-            if 400 <= status_code < 500:
-                action.fatal_error(
-                    'Authentication Failed ({} Client Error). Check your configuration.'.format(status_code))
-            if 500 <= status_code < 600:
-                action.fatal_error('Authentication Failed ({} Server Error).'.format(status_code))
-            else:
-                action.fatal_error('Authentication Failed ({})'.format(status_code))
-    access_token = token.get('access_token')
+    with Action('Getting OAuth2 token "{}"..'.format(name)):
+        access_token = get_token(name, ['uid', 'application.write'])
     docker_login_with_token(url, access_token)
 
 


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index f1ea9c9..1b99794 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -12,10 +12,7 @@ import requests.exceptions
 
 def test_docker_login(monkeypatch, tmpdir):
     monkeypatch.setattr('os.path.expanduser', lambda x: x.replace('~', str(tmpdir)))
-    response = MagicMock()
-    response.status_code = 200
-    response.json.return_value = {'access_token': '12377'}
-    monkeypatch.setattr('requests.get', MagicMock(return_value=response))
+    monkeypatch.setattr('pierone.api.get_token', MagicMock(return_value='12377'))
     docker_login('https://pierone.example.org', 'services', 'mytok',
                  'myuser', 'mypass', 'https://token.example.org', use_keyring=False)
     path = os.path.expanduser('~/.docker/config.json')
@@ -36,44 +33,9 @@ def test_docker_login_service_token(monkeypatch, tmpdir):
                 'email': 'no-mail-required@example.org'} == data.get('auths').get('https://pierone.example.org')
 
 
-@pytest.mark.parametrize(
-    "status_code",
-    [
-        (400),
-        (404),
-        (500),
-        (502),
-        (700),  # nonsense status code that should be handled all the same
-    ])
-def test_docker_login_error(monkeypatch, status_code):
-    mock_get = MagicMock()
-    response = MagicMock()
-    response.status_code = status_code
-    mock_get.side_effect = requests.exceptions.HTTPError(response=response)
-    monkeypatch.setattr('tokens.get', mock_get)
-
-    mock_action = MagicMock()
-    mock_action.side_effect = SystemExit(1)
-    monkeypatch.setattr('pierone.api.Action.fatal_error', mock_action)
-    with pytest.raises(SystemExit):
-        docker_login('https://pierone.example.org', None, 'mytok', 'myuser', 'mypass', 'https://token.example.org')
-    mock_action.assert_called_once_with(ANY)
-    call = mock_action.call_args[0]
-    argument = call[0]  # type: str
-    assert argument.startswith("Authentication Failed")
-    assert str(status_code) in argument
-    if 400 <= status_code < 500:
-        assert "Client Error" in argument
-    if 500 <= status_code < 600:
-        assert "Server Error" in argument
-
-
 def test_keep_dockercfg_entries(monkeypatch, tmpdir):
     monkeypatch.setattr('os.path.expanduser', lambda x: x.replace('~', str(tmpdir)))
-    response = MagicMock()
-    response.status_code = 200
-    response.json.return_value = {'access_token': '12377'}
-    monkeypatch.setattr('requests.get', MagicMock(return_value=response))
+    monkeypatch.setattr('pierone.api.get_token', MagicMock(return_value='12377'))
     path = os.path.expanduser('~/.docker/config.json')
 
     key = 'https://old.example.org'
diff --git a/tests/test_cli.py b/tests/test_cli.py
index da9efb6..e3eecea 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -28,7 +28,7 @@ def test_login(monkeypatch, tmpdir):
     runner = CliRunner()
 
     monkeypatch.setattr('stups_cli.config.load_config', lambda x: {})
-    monkeypatch.setattr('pierone.api.get_named_token', MagicMock(return_value={'access_token': 'tok123'}))
+    monkeypatch.setattr('pierone.api.get_token', MagicMock(return_value='tok123'))
     monkeypatch.setattr('os.path.expanduser', lambda x: x.replace('~', str(tmpdir)))
 
     with runner.isolated_filesystem():
@@ -45,7 +45,7 @@ def test_invalid_url_for_login(monkeypatch, tmpdir):
     response = MagicMock()
 
     monkeypatch.setattr('stups_cli.config.load_config', lambda x: {})
-    monkeypatch.setattr('pierone.api.get_named_token', MagicMock(return_value={'access_token': 'tok123'}))
+    monkeypatch.setattr('pierone.api.get_token', MagicMock(return_value='tok123'))
     monkeypatch.setattr('os.path.expanduser', lambda x: x.replace('~', str(tmpdir)))
 
     # Missing Pier One header
@@ -138,7 +138,7 @@ def test_login_given_url_option(monkeypatch, tmpdir):
 
     monkeypatch.setattr('stups_cli.config.load_config', lambda x: {})
     monkeypatch.setattr('stups_cli.config.store_config', store)
-    monkeypatch.setattr('pierone.api.get_named_token', MagicMock(return_value={'access_token': 'tok123'}))
+    monkeypatch.setattr('pierone.api.get_token', MagicMock(return_value='tok123'))
     monkeypatch.setattr('os.path.expanduser', lambda x: x.replace('~', str(tmpdir)))
 
     with runner.isolated_filesystem():