Validate stack-names for empty values#1088
Merged
vdemeester merged 1 commit intodocker:masterfrom May 28, 2018
Merged
Conversation
mat007
approved these changes
May 28, 2018
cli/command/stack/swarm/common.go
Outdated
| case '"', '\'': | ||
| return true | ||
| } | ||
| return false |
Member
There was a problem hiding this comment.
nit: wouldn't this be more readable
return unicode.IsSpace(r) || r == '"' || r == '\''
?
Member
There was a problem hiding this comment.
(which might optionally then be inlined into validateStackName)
Member
Author
There was a problem hiding this comment.
Good point; I initially had more stuff in the switch, but now it's not really needed anymore. I'll update
Add validation for stack names to prevent an empty name resulting in _all_
stacks to be returned after filtering, which can result in removal of services
for all stacks if `--prune`, or `docker stack rm` is used.
Before this change;
docker stack deploy -c docker-compose.yml one
docker stack deploy -c docker-compose.yml two
docker stack deploy -c docker-compose.yml three
docker stack deploy -c docker-compose.yml --prune ''
Removing service one_web
Removing service two_web
Removing service three_web
After this change:
docker stack deploy -c docker-compose.yml one
docker stack deploy -c docker-compose.yml two
docker stack deploy -c docker-compose.yml three
docker stack deploy -c docker-compose.yml --prune ''
invalid stack name: ""
Other stack commands were updated as well:
Before this change;
docker stack deploy -c docker-compose.yml ''
Creating network _default
failed to create network _default: Error response from daemon: rpc error: code = InvalidArgument desc = name must be valid as a DNS name component
docker stack ps ''
nothing found in stack:
docker stack rm ''
Removing service one_web
Removing service three_web
Removing service two_web
After this change:
docker stack deploy -c docker-compose.yml ''
invalid stack name: ""
docker stack ps ''
invalid stack name: ""
docker stack rm ''
invalid stack name: ""
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
fix_empty_stackname_validation
branch
from
f354695 to
d38f397
Compare
Member
Author
|
Updated 👍
I decided not to inline, as I thought having a |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Fixes moby/moby#36776
Add validation for stack names to prevent an empty name resulting in all
stacks to be returned after filtering, which can result in removal of services
for all stacks if
--prune, ordocker stack rmis used.Before this change;
After this change:
Other stack commands were updated as well:
Before this change;
After this change:
Signed-off-by: Sebastiaan van Stijn github@gone.nl
- Description for the changelog