Skip to content

Commit

Permalink
Fix KeyError when creating a new secret
Browse files Browse the repository at this point in the history
How to reproduce the issue:

```py
>>> import docker
>>> cli = docker.from_env()
>>> cli.secrets.create(name="any_name", data="1")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/docker-py/docker/models/secrets.py", line 10, in __repr__
    return "<%s: '%s'>" % (self.__class__.__name__, self.name)
  File "/home/docker-py/docker/models/secrets.py", line 14, in name
    return self.attrs['Spec']['Name']
KeyError: 'Spec'
```

The exception raises because create secrets API `/secrets/create` only
return the `id` attribute:
https://docs.docker.com/engine/api/v1.41/#operation/SecretCreate
The secret model is created using just the `id` attribute and fails
when looking for Spec.Name attribute.

```py
def __repr__(self):
    return "<%s: '%s'>" % (self.__class__.__name__, self.name)
```

```py
@Property
def name(self):
    return self.attrs['Spec']['Name']
```

I came up with a ugly solution but will prevent the problem to happen
again:

```py
def create(self, **kwargs):
    obj = self.client.api.create_secret(**kwargs)
+   obj.setdefault("Spec", {})["Name"] = kwargs.get("name")
    return self.prepare_model(obj)
```

After the API call, I added the name attribute to the right place to be
used on the property name.

```py
>>> import docker
>>> cli = docker.from_env()
>>> cli.secrets.create(name="any_name", data="1")
<Secret: 'any_name'>
```

It isn't the most elegant solution, but it will do the trick.
I had a previous PR #2517 when I propose using the `id` attribute
instead of `name` on the `__repr__` method, but I think this one will be better.

That fixes #2025

Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>
  • Loading branch information
feliperuhland committed Mar 24, 2021
1 parent 31775a1 commit d4310b2
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 0 deletions.
1 change: 1 addition & 0 deletions docker/models/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class SecretCollection(Collection):

def create(self, **kwargs):
obj = self.client.api.create_secret(**kwargs)
obj.setdefault("Spec", {})["Name"] = kwargs.get("name")
return self.prepare_model(obj)
create.__doc__ = APIClient.create_secret.__doc__

Expand Down
10 changes: 10 additions & 0 deletions tests/unit/fake_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
FAKE_PATH = '/path'
FAKE_VOLUME_NAME = 'perfectcherryblossom'
FAKE_NODE_ID = '24ifsmvkjbyhk'
FAKE_SECRET_ID = 'epdyrw4tsi03xy3deu8g8ly6o'
FAKE_SECRET_NAME = 'super_secret'

# Each method is prefixed with HTTP method (get, post...)
# for clarity and readability
Expand Down Expand Up @@ -512,6 +514,12 @@ def post_fake_network_disconnect():
return 200, None


def post_fake_secret():
status_code = 200
response = {'ID': FAKE_SECRET_ID}
return status_code, response


# Maps real api url to fake response callback
prefix = 'http+docker://localhost'
if constants.IS_WINDOWS_PLATFORM:
Expand Down Expand Up @@ -643,4 +651,6 @@ def post_fake_network_disconnect():
CURRENT_VERSION, prefix, FAKE_NETWORK_ID
), 'POST'):
post_fake_network_disconnect,
'{1}/{0}/secrets/create'.format(CURRENT_VERSION, prefix):
post_fake_secret,
}
1 change: 1 addition & 0 deletions tests/unit/fake_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def make_fake_api_client(overrides=None):
fake_api.post_fake_create_container()[1],
'create_host_config.side_effect': api_client.create_host_config,
'create_network.return_value': fake_api.post_fake_network()[1],
'create_secret.return_value': fake_api.post_fake_secret()[1],
'exec_create.return_value': fake_api.post_fake_exec_create()[1],
'exec_start.return_value': fake_api.post_fake_exec_start()[1],
'images.return_value': fake_api.get_fake_images()[1],
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/models_secrets_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import unittest

from .fake_api_client import make_fake_client
from .fake_api import FAKE_SECRET_NAME


class CreateServiceTest(unittest.TestCase):
def test_secrets_repr(self):
client = make_fake_client()
secret = client.secrets.create(name="super_secret", data="secret")
assert secret.__repr__() == "<Secret: '{}'>".format(FAKE_SECRET_NAME)

0 comments on commit d4310b2

Please sign in to comment.