Update order of '--secret-rm' and '--secret-add'#29802
Conversation
cli/command/service/parse.go
Outdated
There was a problem hiding this comment.
Changed this to a more fine-grained interface; only SecretAPIClient interface was expected - made it a bit easier to create a mock 😄
759662f to
b349a93
Compare
|
Commit message has a typo |
When using both `--secret-rm` and `--secret-add` on `docker service update`,
`--secret-rm` was always performed last. This made it impossible to update
a secret that was already in use on a service (for example, to change
it's permissions, or mount-location inside the container).
This patch changes the order in which `rm` and `add` are performed,
allowing updating a secret in a single `docker service update`.
Before this change, the `rm` was always performed "last", so the secret
was always removed:
$ echo "foo" | docker secret create foo -f -
foo
$ docker service create --name myservice --secret foo nginx:alpine
62xjcr9sr0c2hvepdzqrn3ssn
$ docker service update --secret-rm foo --secret-add source=foo,target=foo2 myservice
myservice
$ docker service inspect --format '{{ json .Spec.TaskTemplate.ContainerSpec.Secrets }}' myservice | jq .
null
After this change, the `rm` is performed _first_, allowing users to
update a secret without updating the service _twice_;
$ echo "foo" | docker secret create foo -f -
1bllmvw3a1yaq3eixqw3f7bjl
$ docker service create --name myservice --secret foo nginx:alpine
lr6s3uoggli1x0hab78glpcxo
$ docker service update --secret-rm foo --secret-add source=foo,target=foo2 myservice
myservice
$ docker service inspect --format '{{ json .Spec.TaskTemplate.ContainerSpec.Secrets }}' myservice | jq .
[
{
"File": {
"Name": "foo2",
"UID": "0",
"GID": "0",
"Mode": 292
},
"SecretID": "tn9qiblgnuuut11eufquw5dev",
"SecretName": "foo"
}
]
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
b349a93 to
e919534
Compare
nice catch; updated 😄 |
|
I remember pointing this out on one of the earlier PRs, and getting the response that there would be a Anyway, LGTM. Just wondering if there was a reason for it to behave this way. cc @ehazlett |
I missed that particular comment I guess, but there's indeed an open PR for that; #28461. That's still an option for future enhancement, but the basic options should work as well, and having |
|
This can probably be merged unless there are outstanding concerns. |
|
LGTM |
Update order of '--secret-rm' and '--secret-add'

When using both
--secret-rmand--secret-addondocker service update,--secret-rmwas always performed last. This made it impossible to update a secret that was already in use on a service (for example, to change it's permissions, or mount-location inside the container).This patch changes the order in which
rmandaddare performed, allowing updating a secret in a singledocker service update.Before this change, the
rmwas always performed "last", so the secret was always removed:After this change, the
rmis performed first, allowing users to update a secret without updating the service twice;