Fix secret repr by feliperuhland · Pull Request #2517 · docker/docker-py

feliperuhland · 2020-02-19T17:07:39Z

This PR fixes issue #2025

feliperuhland · 2020-02-19T17:24:37Z

@shin- @ulyssessouza

Could you take a look?

Thanks.

ulyssessouza

I have some considerations on why the replacement of name by id

docker/models/secrets.py

ulyssessouza · 2020-02-26T14:42:01Z

docker/models/secrets.py


    def __repr__(self):
-        return "<%s: '%s'>" % (self.__class__.__name__, self.name)
+        return "<%s: '%s'>" % (self.__class__.__name__, self.id)


Why changing the attribute here?

It's because List Secrets doesn't return the name attribute.

Issue #2025

Well...
The API says it does. That's in the very link you mentioned. If not, it's a bug

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" } } ]

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'

GordonTheTurtle · 2020-02-27T22:35:22Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

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

Amending updates the existing PR. You DO NOT need to open a new one.

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

feliperuhland · 2021-03-02T19:30:10Z

Hi @ulyssessouza @thaJeztah @shin-

Could you please take a look?
Thanks!

feliperuhland · 2021-03-10T18:38:56Z

Hi @ulyssessouza @thaJeztah @shin- @aiordache
Could you please take a look?

Thanks!

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

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>

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>

ulyssessouza reviewed Feb 26, 2020

View reviewed changes

GordonTheTurtle added the dco/no label Feb 27, 2020

feliperuhland force-pushed the fix-secret-repr branch from 191da62 to 3626329 Compare February 27, 2020 22:38

GordonTheTurtle removed the dco/no label Feb 27, 2020

feliperuhland added 4 commits February 25, 2021 20:17

Create a unit test to reproduce the repr issue

d29122a

- a post fake secret - added to fake api - create a unit test Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>

Fix secrets repr issue

ed8bb55

- Change id_attribute to Id - add new property id - add name fallback Fixes docker#2025 Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>

Change repr and return id instead name attribute

8a4022c

Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>

Revert id to uppercase

5e39c88

Signed-off-by: Felipe Ruhland <felipe.ruhland@gmail.com>

feliperuhland force-pushed the fix-secret-repr branch from 3626329 to 5e39c88 Compare February 25, 2021 19:19

feliperuhland requested a review from aiordache as a code owner February 25, 2021 19:19

feliperuhland closed this Mar 24, 2021

feliperuhland mentioned this pull request Mar 24, 2021

Fix KeyError when creating a new secret #2793

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix secret repr#2517

Fix secret repr#2517
feliperuhland wants to merge 4 commits intodocker:masterfrom
feliperuhland:fix-secret-repr

feliperuhland commented Feb 19, 2020

Uh oh!

feliperuhland commented Feb 19, 2020

Uh oh!

ulyssessouza left a comment

Uh oh!

Uh oh!

ulyssessouza Feb 26, 2020

Uh oh!

feliperuhland Feb 27, 2020

Uh oh!

ulyssessouza Feb 28, 2020

Uh oh!

thaJeztah Feb 28, 2020

Uh oh!

feliperuhland Feb 25, 2021

Uh oh!

GordonTheTurtle commented Feb 27, 2020

Uh oh!

feliperuhland commented Mar 2, 2021

Uh oh!

feliperuhland commented Mar 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

feliperuhland commented Feb 19, 2020

Uh oh!

feliperuhland commented Feb 19, 2020

Uh oh!

ulyssessouza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ulyssessouza Feb 26, 2020

Choose a reason for hiding this comment

Uh oh!

feliperuhland Feb 27, 2020

Choose a reason for hiding this comment

Uh oh!

ulyssessouza Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

thaJeztah Feb 28, 2020

Choose a reason for hiding this comment

Uh oh!

feliperuhland Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

GordonTheTurtle commented Feb 27, 2020

Uh oh!

feliperuhland commented Mar 2, 2021

Uh oh!

feliperuhland commented Mar 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants