diff --git a/CHANGELOG.md b/CHANGELOG.md index 59e7a98eb7..d0cff95006 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,7 +79,7 @@ To manually remove all plugins and resolve this problem, take the following step ### Networking + Add `--attachable` network support to enable `docker run` to work in swarm-mode overlay network [#25962](https://github.com/docker/docker/pull/25962) -+ Add support for host port PublishMode in services using the `--port` option in `docker service create` [#27917](https://github.com/docker/docker/pull/27917) ++ Add support for host port PublishMode in services using the `--publish` option in `docker service create` [#27917](https://github.com/docker/docker/pull/27917) and [#28943](https://github.com/docker/docker/pull/28943) + Add support for Windows server 2016 overlay network driver (requires upcoming ws2016 update) [#28182](https://github.com/docker/docker/pull/28182) * Change the default `FORWARD` policy to `DROP` [#28257](https://github.com/docker/docker/pull/28257) + Add support for specifying static IP addresses for predefined network on windows [#22208](https://github.com/docker/docker/pull/22208) diff --git a/cli/command/service/create.go b/cli/command/service/create.go index ea078e43ad..a8382835a0 100644 --- a/cli/command/service/create.go +++ b/cli/command/service/create.go @@ -40,13 +40,11 @@ func newCreateCommand(dockerCli *command.DockerCli) *cobra.Command { flags.Var(&opts.networks, flagNetwork, "Network attachments") flags.Var(&opts.secrets, flagSecret, "Specify secrets to expose to the service") flags.VarP(&opts.endpoint.publishPorts, flagPublish, "p", "Publish a port as a node port") - flags.MarkHidden(flagPublish) flags.Var(&opts.groups, flagGroup, "Set one or more supplementary user groups for the container") flags.Var(&opts.dns, flagDNS, "Set custom DNS servers") flags.Var(&opts.dnsOption, flagDNSOption, "Set DNS options") flags.Var(&opts.dnsSearch, flagDNSSearch, "Set custom DNS search domains") flags.Var(&opts.hosts, flagHost, "Set one or more custom host-to-IP mappings (host:ip)") - flags.Var(&opts.endpoint.expandedPorts, flagPort, "Publish a port") flags.SetInterspersed(false) return cmd diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 023b922a15..c7518e5976 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -287,45 +287,17 @@ func convertNetworks(networks []string) []swarm.NetworkAttachmentConfig { } type endpointOptions struct { - mode string - publishPorts opts.ListOpts - expandedPorts opts.PortOpt + mode string + publishPorts opts.PortOpt } func (e *endpointOptions) ToEndpointSpec() *swarm.EndpointSpec { - portConfigs := []swarm.PortConfig{} - // We can ignore errors because the format was already validated by ValidatePort - ports, portBindings, _ := nat.ParsePortSpecs(e.publishPorts.GetAll()) - - for port := range ports { - portConfigs = append(portConfigs, ConvertPortToPortConfig(port, portBindings)...) - } - return &swarm.EndpointSpec{ Mode: swarm.ResolutionMode(strings.ToLower(e.mode)), - Ports: append(portConfigs, e.expandedPorts.Value()...), + Ports: e.publishPorts.Value(), } } -// ConvertPortToPortConfig converts ports to the swarm type -func ConvertPortToPortConfig( - port nat.Port, - portBindings map[nat.Port][]nat.PortBinding, -) []swarm.PortConfig { - ports := []swarm.PortConfig{} - - for _, binding := range portBindings[port] { - hostPort, _ := strconv.ParseUint(binding.HostPort, 10, 16) - ports = append(ports, swarm.PortConfig{ - //TODO Name: ? - Protocol: swarm.PortConfigProtocol(strings.ToLower(port.Proto())), - TargetPort: uint32(port.Int()), - PublishedPort: uint32(hostPort), - }) - } - return ports -} - type logDriverOptions struct { name string opts opts.ListOpts @@ -459,16 +431,13 @@ func newServiceOptions() *serviceOptions { containerLabels: opts.NewListOpts(runconfigopts.ValidateEnv), env: opts.NewListOpts(runconfigopts.ValidateEnv), envFile: opts.NewListOpts(nil), - endpoint: endpointOptions{ - publishPorts: opts.NewListOpts(ValidatePort), - }, - groups: opts.NewListOpts(nil), - logDriver: newLogDriverOptions(), - dns: opts.NewListOpts(opts.ValidateIPAddress), - dnsOption: opts.NewListOpts(nil), - dnsSearch: opts.NewListOpts(opts.ValidateDNSSearch), - hosts: opts.NewListOpts(runconfigopts.ValidateExtraHost), - networks: opts.NewListOpts(nil), + groups: opts.NewListOpts(nil), + logDriver: newLogDriverOptions(), + dns: opts.NewListOpts(opts.ValidateIPAddress), + dnsOption: opts.NewListOpts(nil), + dnsSearch: opts.NewListOpts(opts.ValidateDNSSearch), + hosts: opts.NewListOpts(runconfigopts.ValidateExtraHost), + networks: opts.NewListOpts(nil), } } @@ -649,9 +618,6 @@ const ( flagPublish = "publish" flagPublishRemove = "publish-rm" flagPublishAdd = "publish-add" - flagPort = "port" - flagPortAdd = "port-add" - flagPortRemove = "port-rm" flagReplicas = "replicas" flagReserveCPU = "reserve-cpu" flagReserveMemory = "reserve-memory" diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 92329d1439..4bbcf35a8d 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -24,7 +24,7 @@ import ( ) func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command { - opts := newServiceOptions() + serviceOpts := newServiceOptions() cmd := &cobra.Command{ Use: "update [OPTIONS] SERVICE", @@ -40,36 +40,33 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command { flags.String("args", "", "Service command args") flags.Bool("rollback", false, "Rollback to previous specification") flags.Bool("force", false, "Force update even if no changes require it") - addServiceFlags(cmd, opts) + addServiceFlags(cmd, serviceOpts) flags.Var(newListOptsVar(), flagEnvRemove, "Remove an environment variable") flags.Var(newListOptsVar(), flagGroupRemove, "Remove a previously added supplementary user group from the container") 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.MarkHidden(flagPublishRemove) - flags.Var(newListOptsVar(), flagPortRemove, "Remove a port(target-port mandatory)") + // flags.Var(newListOptsVar().WithValidator(validatePublishRemove), flagPublishRemove, "Remove a published port by its target port") + flags.Var(&opts.PortOpt{}, flagPublishRemove, "Remove a published port by its target port") flags.Var(newListOptsVar(), flagConstraintRemove, "Remove a constraint") flags.Var(newListOptsVar(), flagDNSRemove, "Remove a custom DNS server") flags.Var(newListOptsVar(), flagDNSOptionRemove, "Remove a DNS option") flags.Var(newListOptsVar(), flagDNSSearchRemove, "Remove a DNS search domain") flags.Var(newListOptsVar(), flagHostRemove, "Remove a custom host-to-IP mapping (host:ip)") - flags.Var(&opts.labels, flagLabelAdd, "Add or update a service label") - flags.Var(&opts.containerLabels, flagContainerLabelAdd, "Add or update a container label") - flags.Var(&opts.env, flagEnvAdd, "Add or update an environment variable") + flags.Var(&serviceOpts.labels, flagLabelAdd, "Add or update a service label") + flags.Var(&serviceOpts.containerLabels, flagContainerLabelAdd, "Add or update a container label") + flags.Var(&serviceOpts.env, flagEnvAdd, "Add or update an environment variable") flags.Var(newListOptsVar(), flagSecretRemove, "Remove a secret") - flags.Var(&opts.secrets, flagSecretAdd, "Add or update a secret on a service") - flags.Var(&opts.mounts, flagMountAdd, "Add or update a mount on a service") - flags.Var(&opts.constraints, flagConstraintAdd, "Add or update a placement constraint") - flags.Var(&opts.endpoint.publishPorts, flagPublishAdd, "Add or update a published port") - flags.MarkHidden(flagPublishAdd) - flags.Var(&opts.endpoint.expandedPorts, flagPortAdd, "Add or update a port") - flags.Var(&opts.groups, flagGroupAdd, "Add an additional supplementary user group to the container") - flags.Var(&opts.dns, flagDNSAdd, "Add or update a custom DNS server") - flags.Var(&opts.dnsOption, flagDNSOptionAdd, "Add or update a DNS option") - flags.Var(&opts.dnsSearch, flagDNSSearchAdd, "Add or update a custom DNS search domain") - flags.Var(&opts.hosts, flagHostAdd, "Add or update a custom host-to-IP mapping (host:ip)") + flags.Var(&serviceOpts.secrets, flagSecretAdd, "Add or update a secret on a service") + flags.Var(&serviceOpts.mounts, flagMountAdd, "Add or update a mount on a service") + flags.Var(&serviceOpts.constraints, flagConstraintAdd, "Add or update a placement constraint") + flags.Var(&serviceOpts.endpoint.publishPorts, flagPublishAdd, "Add or update a published port") + flags.Var(&serviceOpts.groups, flagGroupAdd, "Add an additional supplementary user group to the container") + flags.Var(&serviceOpts.dns, flagDNSAdd, "Add or update a custom DNS server") + flags.Var(&serviceOpts.dnsOption, flagDNSOptionAdd, "Add or update a DNS option") + flags.Var(&serviceOpts.dnsSearch, flagDNSSearchAdd, "Add or update a custom DNS search domain") + flags.Var(&serviceOpts.hosts, flagHostAdd, "Add or update a custom host-to-IP mapping (host:ip)") return cmd } @@ -276,7 +273,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { } } - if anyChanged(flags, flagPublishAdd, flagPublishRemove, flagPortAdd, flagPortRemove) { + if anyChanged(flags, flagPublishAdd, flagPublishRemove) { if spec.EndpointSpec == nil { spec.EndpointSpec = &swarm.EndpointSpec{} } @@ -633,73 +630,46 @@ func (r byPortConfig) Less(i, j int) bool { func portConfigToString(portConfig *swarm.PortConfig) string { protocol := portConfig.Protocol - if protocol == "" { - protocol = "tcp" - } - mode := portConfig.PublishMode - if mode == "" { - mode = "ingress" - } - return fmt.Sprintf("%v:%v/%s/%s", portConfig.PublishedPort, portConfig.TargetPort, protocol, mode) } +// FIXME(vdemeester) port to opts.PortOpt +// 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{} - // Check to see if there are any conflict in flags. - if flags.Changed(flagPublishAdd) { - values := flags.Lookup(flagPublishAdd).Value.(*opts.ListOpts).GetAll() - ports, portBindings, _ := nat.ParsePortSpecs(values) - for port := range ports { - newConfigs := ConvertPortToPortConfig(port, portBindings) - for _, entry := range newConfigs { - if v, ok := portSet[portConfigToString(&entry)]; ok && v != entry { - return fmt.Errorf("conflicting port mapping between %v:%v/%s and %v:%v/%s", entry.PublishedPort, entry.TargetPort, entry.Protocol, v.PublishedPort, v.TargetPort, v.Protocol) - } - portSet[portConfigToString(&entry)] = entry - } - } - } - - if flags.Changed(flagPortAdd) { - for _, entry := range flags.Lookup(flagPortAdd).Value.(*opts.PortOpt).Value() { - if v, ok := portSet[portConfigToString(&entry)]; ok && v != entry { - return fmt.Errorf("conflicting port mapping between %v:%v/%s and %v:%v/%s", entry.PublishedPort, entry.TargetPort, entry.Protocol, v.PublishedPort, v.TargetPort, v.Protocol) - } - portSet[portConfigToString(&entry)] = entry - } - } - - // Override previous PortConfig in service if there is any duplicate + // Build the current list of portConfig for _, entry := range *portConfig { if _, ok := portSet[portConfigToString(&entry)]; !ok { portSet[portConfigToString(&entry)] = entry } } - toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.ListOpts).GetAll() - removePortCSV := flags.Lookup(flagPortRemove).Value.(*opts.ListOpts).GetAll() - removePortOpts := &opts.PortOpt{} - for _, portCSV := range removePortCSV { - if err := removePortOpts.Set(portCSV); err != nil { - return err - } - } - newPorts := []swarm.PortConfig{} + + // Clean current ports + toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.PortOpt).Value() portLoop: for _, port := range portSet { - for _, rawTargetPort := range toRemove { - targetPort := nat.Port(rawTargetPort) - if equalPort(targetPort, port) { - continue portLoop - } - } - - for _, pConfig := range removePortOpts.Value() { + for _, pConfig := range toRemove { if equalProtocol(port.Protocol, pConfig.Protocol) && port.TargetPort == pConfig.TargetPort && equalPublishMode(port.PublishMode, pConfig.PublishMode) { @@ -710,6 +680,23 @@ portLoop: newPorts = append(newPorts, port) } + // Check to see if there are any conflict in flags. + if flags.Changed(flagPublishAdd) { + ports := flags.Lookup(flagPublishAdd).Value.(*opts.PortOpt).Value() + + for _, port := range ports { + if v, ok := portSet[portConfigToString(&port)]; ok { + if v != port { + fmt.Println("v", v) + return fmt.Errorf("conflicting port mapping between %v:%v/%s and %v:%v/%s", port.PublishedPort, port.TargetPort, port.Protocol, v.PublishedPort, v.TargetPort, v.Protocol) + } + continue + } + //portSet[portConfigToString(&port)] = port + newPorts = append(newPorts, port) + } + } + // Sort the PortConfig to avoid unnecessary updates sort.Sort(byPortConfig(newPorts)) *portConfig = newPorts diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 998d06d3bd..08fe248769 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -220,28 +220,18 @@ func TestUpdatePorts(t *testing.T) { assert.Equal(t, targetPorts[1], 1000) } -func TestUpdatePortsDuplicateEntries(t *testing.T) { +func TestUpdatePortsDuplicate(t *testing.T) { // Test case for #25375 flags := newUpdateCommand(nil).Flags() flags.Set("publish-add", "80:80") portConfigs := []swarm.PortConfig{ - {TargetPort: 80, PublishedPort: 80}, - } - - err := updatePorts(flags, &portConfigs) - assert.Equal(t, err, nil) - assert.Equal(t, len(portConfigs), 1) - assert.Equal(t, portConfigs[0].TargetPort, uint32(80)) -} - -func TestUpdatePortsDuplicateKeys(t *testing.T) { - // Test case for #25375 - flags := newUpdateCommand(nil).Flags() - flags.Set("publish-add", "80:80") - - portConfigs := []swarm.PortConfig{ - {TargetPort: 80, PublishedPort: 80}, + { + TargetPort: 80, + PublishedPort: 80, + Protocol: swarm.PortConfigProtocolTCP, + PublishMode: swarm.PortConfigPublishModeIngress, + }, } err := updatePorts(flags, &portConfigs) @@ -345,3 +335,50 @@ 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, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + } + + err := updatePorts(flags, &portConfigs) + assert.Equal(t, err, nil) + assert.Equal(t, len(portConfigs), 2) + assert.Equal(t, portConfigs[0].TargetPort, uint32(81)) + assert.Equal(t, portConfigs[1].TargetPort, uint32(82)) +} + +// FIXME(vdemeester) port to opts.PortOpt +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/cli/command/stack/deploy.go b/cli/command/stack/deploy.go index 1f41cb7d89..00a7634a0a 100644 --- a/cli/command/stack/deploy.go +++ b/cli/command/stack/deploy.go @@ -21,7 +21,6 @@ import ( "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" - servicecmd "github.com/docker/docker/cli/command/service" dockerclient "github.com/docker/docker/client" "github.com/docker/docker/opts" runconfigopts "github.com/docker/docker/runconfig/opts" @@ -745,7 +744,7 @@ func convertEndpointSpec(source []string) (*swarm.EndpointSpec, error) { for port := range ports { portConfigs = append( portConfigs, - servicecmd.ConvertPortToPortConfig(port, portBindings)...) + opts.ConvertPortToPortConfig(port, portBindings)...) } return &swarm.EndpointSpec{Ports: portConfigs}, nil diff --git a/contrib/completion/bash/docker b/contrib/completion/bash/docker index 7f9a13f710..82d5ef1ead 100644 --- a/contrib/completion/bash/docker +++ b/contrib/completion/bash/docker @@ -2756,7 +2756,7 @@ _docker_service_update() { --host --mode --name - --port + --publish --secret " @@ -2803,8 +2803,8 @@ _docker_service_update() { --host-add --host-rm --image - --port-add - --port-rm + --publish-add + --publish-rm --secret-add --secret-rm " diff --git a/contrib/completion/zsh/_docker b/contrib/completion/zsh/_docker index 229a0edb7f..8d00b13e6d 100644 --- a/contrib/completion/zsh/_docker +++ b/contrib/completion/zsh/_docker @@ -1797,7 +1797,7 @@ __docker_service_subcommand() { "($help)*--env-file=[Read environment variables from a file]:environment file:_files" \ "($help)--mode=[Service Mode]:mode:(global replicated)" \ "($help)--name=[Service name]:name: " \ - "($help)*--port=[Publish a port]:port: " \ + "($help)*--publish=[Publish a port]:port: " \ "($help -): :__docker_complete_images" \ "($help -):command: _command_names -e" \ "($help -)*::arguments: _normal" && ret=0 @@ -1870,8 +1870,8 @@ __docker_service_subcommand() { "($help)*--group-add=[Add additional supplementary user groups to the container]:group:_groups" \ "($help)*--group-rm=[Remove previously added supplementary user groups from the container]:group:_groups" \ "($help)--image=[Service image tag]:image:__docker_complete_repositories" \ - "($help)*--port-add=[Add or update a port]:port: " \ - "($help)*--port-rm=[Remove a port(target-port mandatory)]:port: " \ + "($help)*--publish-add=[Add or update a port]:port: " \ + "($help)*--publish-rm=[Remove a port(target-port mandatory)]:port: " \ "($help)--rollback[Rollback to previous specification]" \ "($help -)1:service:__docker_complete_services" && ret=0 ;; diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 634fb955a5..b0f2de7a0a 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -232,23 +232,51 @@ func (s *DockerSwarmSuite) TestSwarmNodeTaskListFilter(c *check.C) { func (s *DockerSwarmSuite) TestSwarmPublishAdd(c *check.C) { d := s.AddDaemon(c, true, true) - name := "top" - out, err := d.Cmd("service", "create", "--name", name, "--label", "x=y", "busybox", "top") - c.Assert(err, checker.IsNil) - c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") + testCases := []struct { + name string + publishAdd []string + ports string + }{ + { + name: "simple-syntax", + publishAdd: []string{ + "80:80", + "80:80", + "80:80", + "80:20", + }, + ports: "[{ tcp 80 80 ingress}]", + }, + { + name: "complex-syntax", + publishAdd: []string{ + "target=90,published=90,protocol=tcp,mode=ingress", + "target=90,published=90,protocol=tcp,mode=ingress", + "target=90,published=90,protocol=tcp,mode=ingress", + "target=30,published=90,protocol=tcp,mode=ingress", + }, + ports: "[{ tcp 90 90 ingress}]", + }, + } - out, err = d.Cmd("service", "update", "--publish-add", "80:80", name) - c.Assert(err, checker.IsNil) + for _, tc := range testCases { + out, err := d.Cmd("service", "create", "--name", tc.name, "--label", "x=y", "busybox", "top") + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "") - out, err = d.cmdRetryOutOfSequence("service", "update", "--publish-add", "80:80", name) - c.Assert(err, checker.IsNil) + out, err = d.cmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[0], tc.name) + c.Assert(err, checker.IsNil, check.Commentf(out)) - out, err = d.cmdRetryOutOfSequence("service", "update", "--publish-add", "80:80", "--publish-add", "80:20", name) - c.Assert(err, checker.NotNil) + out, err = d.cmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[1], tc.name) + c.Assert(err, checker.IsNil, check.Commentf(out)) - out, err = d.Cmd("service", "inspect", "--format", "{{ .Spec.EndpointSpec.Ports }}", name) - c.Assert(err, checker.IsNil) - c.Assert(strings.TrimSpace(out), checker.Equals, "[{ tcp 80 80 ingress}]") + out, err = d.cmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[2], "--publish-add", tc.publishAdd[3], tc.name) + c.Assert(err, checker.NotNil, check.Commentf(out)) + + out, err = d.Cmd("service", "inspect", "--format", "{{ .Spec.EndpointSpec.Ports }}", tc.name) + c.Assert(err, checker.IsNil) + c.Assert(strings.TrimSpace(out), checker.Equals, tc.ports) + } } func (s *DockerSwarmSuite) TestSwarmServiceWithGroup(c *check.C) { diff --git a/opts/port.go b/opts/port.go index d337cb1a43..020a5d1e1c 100644 --- a/opts/port.go +++ b/opts/port.go @@ -3,10 +3,12 @@ package opts import ( "encoding/csv" "fmt" + "regexp" "strconv" "strings" "github.com/docker/docker/api/types/swarm" + "github.com/docker/go-connections/nat" ) const ( @@ -23,59 +25,83 @@ type PortOpt struct { // Set a new port value func (p *PortOpt) Set(value string) error { - csvReader := csv.NewReader(strings.NewReader(value)) - fields, err := csvReader.Read() + longSyntax, err := regexp.MatchString(`\w+=\w+(,\w+=\w+)*`, value) if err != nil { return err } - - pConfig := swarm.PortConfig{} - for _, field := range fields { - parts := strings.SplitN(field, "=", 2) - if len(parts) != 2 { - return fmt.Errorf("invalid field %s", field) + if longSyntax { + csvReader := csv.NewReader(strings.NewReader(value)) + fields, err := csvReader.Read() + if err != nil { + return err } - key := strings.ToLower(parts[0]) - value := strings.ToLower(parts[1]) - - switch key { - case portOptProtocol: - if value != string(swarm.PortConfigProtocolTCP) && value != string(swarm.PortConfigProtocolUDP) { - return fmt.Errorf("invalid protocol value %s", value) + pConfig := swarm.PortConfig{} + for _, field := range fields { + parts := strings.SplitN(field, "=", 2) + if len(parts) != 2 { + return fmt.Errorf("invalid field %s", field) } - pConfig.Protocol = swarm.PortConfigProtocol(value) - case portOptMode: - if value != string(swarm.PortConfigPublishModeIngress) && value != string(swarm.PortConfigPublishModeHost) { - return fmt.Errorf("invalid publish mode value %s", value) - } + key := strings.ToLower(parts[0]) + value := strings.ToLower(parts[1]) - pConfig.PublishMode = swarm.PortConfigPublishMode(value) - case portOptTargetPort: - tPort, err := strconv.ParseUint(value, 10, 16) - if err != nil { - return err - } + switch key { + case portOptProtocol: + if value != string(swarm.PortConfigProtocolTCP) && value != string(swarm.PortConfigProtocolUDP) { + return fmt.Errorf("invalid protocol value %s", value) + } - pConfig.TargetPort = uint32(tPort) - case portOptPublishedPort: - pPort, err := strconv.ParseUint(value, 10, 16) - if err != nil { - return err - } + pConfig.Protocol = swarm.PortConfigProtocol(value) + case portOptMode: + if value != string(swarm.PortConfigPublishModeIngress) && value != string(swarm.PortConfigPublishModeHost) { + return fmt.Errorf("invalid publish mode value %s", value) + } - pConfig.PublishedPort = uint32(pPort) - default: - return fmt.Errorf("invalid field key %s", key) + pConfig.PublishMode = swarm.PortConfigPublishMode(value) + case portOptTargetPort: + tPort, err := strconv.ParseUint(value, 10, 16) + if err != nil { + return err + } + + pConfig.TargetPort = uint32(tPort) + case portOptPublishedPort: + pPort, err := strconv.ParseUint(value, 10, 16) + if err != nil { + return err + } + + pConfig.PublishedPort = uint32(pPort) + default: + return fmt.Errorf("invalid field key %s", key) + } } - } - if pConfig.TargetPort == 0 { - return fmt.Errorf("missing mandatory field %q", portOptTargetPort) - } + if pConfig.TargetPort == 0 { + return fmt.Errorf("missing mandatory field %q", portOptTargetPort) + } - p.ports = append(p.ports, pConfig) + if pConfig.PublishMode == "" { + pConfig.PublishMode = swarm.PortConfigPublishModeIngress + } + + if pConfig.Protocol == "" { + pConfig.Protocol = swarm.PortConfigProtocolTCP + } + + p.ports = append(p.ports, pConfig) + } else { + // short syntax + portConfigs := []swarm.PortConfig{} + // We can ignore errors because the format was already validated by ValidatePort + ports, portBindings, _ := nat.ParsePortSpecs([]string{value}) + + for port := range ports { + portConfigs = append(portConfigs, ConvertPortToPortConfig(port, portBindings)...) + } + p.ports = append(p.ports, portConfigs...) + } return nil } @@ -98,3 +124,23 @@ func (p *PortOpt) String() string { func (p *PortOpt) Value() []swarm.PortConfig { return p.ports } + +// ConvertPortToPortConfig converts ports to the swarm type +func ConvertPortToPortConfig( + port nat.Port, + portBindings map[nat.Port][]nat.PortBinding, +) []swarm.PortConfig { + ports := []swarm.PortConfig{} + + for _, binding := range portBindings[port] { + hostPort, _ := strconv.ParseUint(binding.HostPort, 10, 16) + ports = append(ports, swarm.PortConfig{ + //TODO Name: ? + Protocol: swarm.PortConfigProtocol(strings.ToLower(port.Proto())), + TargetPort: uint32(port.Int()), + PublishedPort: uint32(hostPort), + PublishMode: swarm.PortConfigPublishModeIngress, + }) + } + return ports +} diff --git a/opts/port_test.go b/opts/port_test.go new file mode 100644 index 0000000000..67bcf8f1d9 --- /dev/null +++ b/opts/port_test.go @@ -0,0 +1,259 @@ +package opts + +import ( + "testing" + + "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/pkg/testutil/assert" +) + +func TestPortOptValidSimpleSyntax(t *testing.T) { + testCases := []struct { + value string + expected []swarm.PortConfig + }{ + { + value: "80", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "80:8080", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 8080, + PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "8080:80/tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + PublishedPort: 8080, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "80:8080/udp", + expected: []swarm.PortConfig{ + { + Protocol: "udp", + TargetPort: 8080, + PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "80-81:8080-8081/tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 8080, + PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + { + Protocol: "tcp", + TargetPort: 8081, + PublishedPort: 81, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "80-82:8080-8082/udp", + expected: []swarm.PortConfig{ + { + Protocol: "udp", + TargetPort: 8080, + PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + { + Protocol: "udp", + TargetPort: 8081, + PublishedPort: 81, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + { + Protocol: "udp", + TargetPort: 8082, + PublishedPort: 82, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + } + for _, tc := range testCases { + var port PortOpt + assert.NilError(t, port.Set(tc.value)) + assert.Equal(t, len(port.Value()), len(tc.expected)) + for _, expectedPortConfig := range tc.expected { + assertContains(t, port.Value(), expectedPortConfig) + } + } +} + +func TestPortOptValidComplexSyntax(t *testing.T) { + testCases := []struct { + value string + expected []swarm.PortConfig + }{ + { + value: "target=80", + expected: []swarm.PortConfig{ + { + TargetPort: 80, + Protocol: "tcp", + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "target=80,protocol=tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "target=80,published=8080,protocol=tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + PublishedPort: 8080, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "published=80,target=8080,protocol=tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 8080, + PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, + }, + }, + }, + { + value: "target=80,published=8080,protocol=tcp,mode=host", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + PublishedPort: 8080, + PublishMode: "host", + }, + }, + }, + { + value: "target=80,published=8080,mode=host", + expected: []swarm.PortConfig{ + { + TargetPort: 80, + PublishedPort: 8080, + PublishMode: "host", + Protocol: "tcp", + }, + }, + }, + { + value: "target=80,published=8080,mode=ingress", + expected: []swarm.PortConfig{ + { + TargetPort: 80, + PublishedPort: 8080, + PublishMode: "ingress", + Protocol: "tcp", + }, + }, + }, + } + for _, tc := range testCases { + var port PortOpt + assert.NilError(t, port.Set(tc.value)) + assert.Equal(t, len(port.Value()), len(tc.expected)) + for _, expectedPortConfig := range tc.expected { + assertContains(t, port.Value(), expectedPortConfig) + } + } +} + +func TestPortOptInvalidComplexSyntax(t *testing.T) { + testCases := []struct { + value string + expectedError string + }{ + { + value: "invalid,target=80", + expectedError: "invalid field", + }, + { + value: "invalid=field", + expectedError: "invalid field", + }, + { + value: "protocol=invalid", + expectedError: "invalid protocol value", + }, + { + value: "target=invalid", + expectedError: "invalid syntax", + }, + { + value: "published=invalid", + expectedError: "invalid syntax", + }, + { + value: "mode=invalid", + expectedError: "invalid publish mode value", + }, + { + value: "published=8080,protocol=tcp,mode=ingress", + expectedError: "missing mandatory field", + }, + { + value: `target=80,protocol="tcp,mode=ingress"`, + expectedError: "non-quoted-field", + }, + { + value: `target=80,"protocol=tcp,mode=ingress"`, + expectedError: "invalid protocol value", + }, + } + for _, tc := range testCases { + var port PortOpt + assert.Error(t, port.Set(tc.value), tc.expectedError) + } +} + +func assertContains(t *testing.T, portConfigs []swarm.PortConfig, expected swarm.PortConfig) { + var contains = false + for _, portConfig := range portConfigs { + if portConfig == expected { + contains = true + break + } + } + if !contains { + t.Errorf("expected %v to contain %v, did not", portConfigs, expected) + } +}