Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix KeyError when creating a new secret #2793

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

feliperuhland
Copy link
Contributor

@feliperuhland feliperuhland commented Mar 24, 2021

How to reproduce the issue:

>>> 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.

def __repr__(self):
    return "<%s: '%s'>" % (self.__class__.__name__, self.name)
@property
def name(self):
    return self.attrs['Spec']['Name']

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

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.

>>> 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

EDIT: It will save a new request to get the name.

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 docker#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 docker#2025

Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>
@feliperuhland feliperuhland force-pushed the create-secret-missing-name branch from 2e22b6e to d4310b2 Compare March 24, 2021 17:04
Copy link
Contributor

@ulyssessouza ulyssessouza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@aiordache aiordache left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Couldn't create secret object
3 participants