Fix secret repr #2517
Closed
Fix secret repr #2517
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Could you take a look? Thanks. |
|
|
||
| def __repr__(self): | ||
| return "<%s: '%s'>" % (self.__class__.__name__, self.name) | ||
| return "<%s: '%s'>" % (self.__class__.__name__, self.id) |
There was a problem hiding this comment.
Why changing the attribute here?
There was a problem hiding this comment.
It's because List Secrets doesn't return the name attribute.
Issue #2025
There was a problem hiding this comment.
Well...
The API says it does. That's in the very link you mentioned. If not, it's a bug
There was a problem hiding this comment.
Name is nested in the Spec in the response, so not at the same level as id;
[
{
"ID": "ktnbjxoalbkvbvedmg1urrz8h",
"Version": {
"Index": 11
},
"CreatedAt": "2016-11-05T01:20:17.327670065Z",
"UpdatedAt": "2016-11-05T01:20:17.327670065Z",
"Spec": {
"Name": "app-dev.crt"
}
}
]
There was a problem hiding this comment.
Hi @ulyssessouza @thaJeztah the secret creation endpoint just return the ID value, so the method __repr__ raise an exception:
12 @property
13 def name(self):
---> 14 return self.attrs['Spec']['Name']
15
16 def remove(self):
KeyError: 'Spec'
|
Please sign your commits following these rules: $ git clone -b "fix-secret-repr" git@github.com:feliperuhland/docker-py.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354382592
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
feliperuhland
force-pushed
the
fix-secret-repr
branch
from
191da62
to
3626329
Compare
- a post fake secret - added to fake api - create a unit test Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>
- Change id_attribute to Id - add new property id - add name fallback Fixes docker#2025 Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>
Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>
Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>
feliperuhland
force-pushed
the
fix-secret-repr
branch
from
3626329
to
5e39c88
Compare
|
Hi @ulyssessouza @thaJeztah @shin- Could you please take a look? |
|
Hi @ulyssessouza @thaJeztah @shin- @aiordache Thanks! |
feliperuhland
added a commit
to feliperuhland/docker-py
that referenced
this pull request
Mar 24, 2021
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/ruhland/projects/github.com/feliperuhland/docker-py/docker/models/secrets.py", line 10, in __repr__
return "<%s: '%s'>" % (self.__class__.__name__, self.name)
File "/home/ruhland/projects/github.com/feliperuhland/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
feliperuhland
added a commit
to feliperuhland/docker-py
that referenced
this pull request
Mar 24, 2021
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/ruhland/projects/github.com/feliperuhland/docker-py/docker/models/secrets.py", line 10, in __repr__
return "<%s: '%s'>" % (self.__class__.__name__, self.name)
File "/home/ruhland/projects/github.com/feliperuhland/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
added a commit
to feliperuhland/docker-py
that referenced
this pull request
Mar 24, 2021
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes issue #2025