diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 92329d1439..200f58c3a6 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -47,7 +47,7 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command { flags.Var(newListOptsVar(), flagLabelRemove, "Remove a label by its key") flags.Var(newListOptsVar(), flagContainerLabelRemove, "Remove a container label by its key") flags.Var(newListOptsVar(), flagMountRemove, "Remove a mount by its target path") - flags.Var(newListOptsVar(), flagPublishRemove, "Remove a published port by its target port") + flags.Var(newListOptsVar().WithValidator(validatePublishRemove), flagPublishRemove, "Remove a published port by its target port") flags.MarkHidden(flagPublishRemove) flags.Var(newListOptsVar(), flagPortRemove, "Remove a port(target-port mandatory)") flags.Var(newListOptsVar(), flagConstraintRemove, "Remove a constraint") @@ -645,6 +645,23 @@ func portConfigToString(portConfig *swarm.PortConfig) string { return fmt.Sprintf("%v:%v/%s/%s", portConfig.PublishedPort, portConfig.TargetPort, protocol, mode) } +// This validation is only used for `--publish-rm`. +// The `--publish-rm` takes: +// [/] (e.g., 80, 80/tcp, 53/udp) +func validatePublishRemove(val string) (string, error) { + proto, port := nat.SplitProtoPort(val) + if proto != "tcp" && proto != "udp" { + return "", fmt.Errorf("invalid protocol '%s' for %s", proto, val) + } + if strings.Contains(port, ":") { + return "", fmt.Errorf("invalid port format: '%s', should be [/] (e.g., 80, 80/tcp, 53/udp)", port) + } + if _, err := nat.ParsePort(port); err != nil { + return "", err + } + return val, nil +} + func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) error { // The key of the map is `port/protocol`, e.g., `80/tcp` portSet := map[string]swarm.PortConfig{} diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 998d06d3bd..bb2e9c1073 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -345,3 +345,43 @@ func TestUpdateHosts(t *testing.T) { assert.Equal(t, hosts[1], "2001:db8:abc8::1 ipv6.net") assert.Equal(t, hosts[2], "4.3.2.1 example.org") } + +func TestUpdatePortsRmWithProtocol(t *testing.T) { + flags := newUpdateCommand(nil).Flags() + flags.Set("publish-add", "8081:81") + flags.Set("publish-add", "8082:82") + flags.Set("publish-rm", "80") + flags.Set("publish-rm", "81/tcp") + flags.Set("publish-rm", "82/udp") + + portConfigs := []swarm.PortConfig{ + {TargetPort: 80, PublishedPort: 8080, Protocol: swarm.PortConfigProtocolTCP}, + } + + err := updatePorts(flags, &portConfigs) + assert.Equal(t, err, nil) + assert.Equal(t, len(portConfigs), 1) + assert.Equal(t, portConfigs[0].TargetPort, uint32(82)) +} + +func TestValidatePort(t *testing.T) { + validPorts := []string{"80/tcp", "80", "80/udp"} + invalidPorts := map[string]string{ + "9999999": "out of range", + "80:80/tcp": "invalid port format", + "53:53/udp": "invalid port format", + "80:80": "invalid port format", + "80/xyz": "invalid protocol", + "tcp": "invalid syntax", + "udp": "invalid syntax", + "": "invalid protocol", + } + for _, port := range validPorts { + _, err := validatePublishRemove(port) + assert.Equal(t, err, nil) + } + for port, e := range invalidPorts { + _, err := validatePublishRemove(port) + assert.Error(t, err, e) + } +} diff --git a/opts/opts.go b/opts/opts.go index ae851537ec..9f66c039e7 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -108,6 +108,12 @@ func (opts *ListOpts) Type() string { return "list" } +// WithValidator returns the ListOpts with validator set. +func (opts *ListOpts) WithValidator(validator ValidatorFctType) *ListOpts { + opts.validator = validator + return opts +} + // NamedOption is an interface that list and map options // with names implement. type NamedOption interface {