Skip to content

Commit

Permalink
Correct port range logic for port generation
Browse files Browse the repository at this point in the history
The existing logic (Range > 0) always triggered, because range is
guaranteed to be at least 1 (a single port has a range of 1, a
two port range (e.g. 80-81) has a range of 2, and so on). As such
this could cause ports that had a host port assigned to them by
the user to randomly assign one instead.

Fixes containers#8650

Signed-off-by: Matthew Heon <mheon@redhat.com>
  • Loading branch information
mheon committed Dec 8, 2020
1 parent 7caef9c commit 8eb8d73
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 4 deletions.
12 changes: 8 additions & 4 deletions pkg/specgen/generate/ports.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,11 @@ func parsePortMapping(portMappings []specgen.PortMapping) ([]ocicni.PortMapping,
var index uint16
for index = 0; index < len; index++ {
cPort := containerPort + index
hPort := hostPort + index
hPort := hostPort
// Only increment host port if it's not 0.
if hostPort != 0 {
hPort += index
}

if cPort == 0 {
return nil, nil, nil, errors.Errorf("container port cannot be 0")
Expand Down Expand Up @@ -162,8 +166,8 @@ func parsePortMapping(portMappings []specgen.PortMapping) ([]ocicni.PortMapping,
tempMappings,
tempMapping{
mapping: cniPort,
startOfRange: port.Range > 0 && index == 0,
isInRange: port.Range > 0,
startOfRange: port.Range > 1 && index == 0,
isInRange: port.Range > 1,
},
)
}
Expand All @@ -183,7 +187,7 @@ func parsePortMapping(portMappings []specgen.PortMapping) ([]ocicni.PortMapping,
for _, tmp := range tempMappings {
p := tmp.mapping

if p.HostPort != 0 && !tmp.isInRange {
if p.HostPort != 0 {
remadeMappings = append(remadeMappings, p)
continue
}
Expand Down
42 changes: 42 additions & 0 deletions test/e2e/run_networking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,48 @@ var _ = Describe("Podman run networking", func() {
Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal(""))
})

It("podman run -p 80-82 -p 8080:8080", func() {
name := "testctr"
session := podmanTest.Podman([]string{"create", "-t", "-p", "80-82", "-p", "8080:8080", "--name", name, ALPINE, "/bin/sh"})
session.WaitWithDefaultTimeout()
inspectOut := podmanTest.InspectContainer(name)
Expect(len(inspectOut)).To(Equal(1))
Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(4))
Expect(len(inspectOut[0].NetworkSettings.Ports["80/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort).To(Not(Equal("80")))
Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal(""))
Expect(len(inspectOut[0].NetworkSettings.Ports["81/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["81/tcp"][0].HostPort).To(Not(Equal("81")))
Expect(inspectOut[0].NetworkSettings.Ports["81/tcp"][0].HostIP).To(Equal(""))
Expect(len(inspectOut[0].NetworkSettings.Ports["82/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["82/tcp"][0].HostPort).To(Not(Equal("82")))
Expect(inspectOut[0].NetworkSettings.Ports["82/tcp"][0].HostIP).To(Equal(""))
Expect(len(inspectOut[0].NetworkSettings.Ports["8080/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostPort).To(Equal("8080"))
Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostIP).To(Equal(""))
})

It("podman run -p 80 -p 8080-8082:8080-8082", func() {
name := "testctr"
session := podmanTest.Podman([]string{"create", "-t", "-p", "80", "-p", "8080-8082:8080-8082", "--name", name, ALPINE, "/bin/sh"})
session.WaitWithDefaultTimeout()
inspectOut := podmanTest.InspectContainer(name)
Expect(len(inspectOut)).To(Equal(1))
Expect(len(inspectOut[0].NetworkSettings.Ports)).To(Equal(4))
Expect(len(inspectOut[0].NetworkSettings.Ports["80/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostPort).To(Not(Equal("80")))
Expect(inspectOut[0].NetworkSettings.Ports["80/tcp"][0].HostIP).To(Equal(""))
Expect(len(inspectOut[0].NetworkSettings.Ports["8080/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostPort).To(Equal("8080"))
Expect(inspectOut[0].NetworkSettings.Ports["8080/tcp"][0].HostIP).To(Equal(""))
Expect(len(inspectOut[0].NetworkSettings.Ports["8081/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["8081/tcp"][0].HostPort).To(Equal("8081"))
Expect(inspectOut[0].NetworkSettings.Ports["8081/tcp"][0].HostIP).To(Equal(""))
Expect(len(inspectOut[0].NetworkSettings.Ports["8082/tcp"])).To(Equal(1))
Expect(inspectOut[0].NetworkSettings.Ports["8082/tcp"][0].HostPort).To(Equal("8082"))
Expect(inspectOut[0].NetworkSettings.Ports["8082/tcp"][0].HostIP).To(Equal(""))
})

It("podman run -p 8080:80", func() {
name := "testctr"
session := podmanTest.Podman([]string{"create", "-t", "-p", "8080:80", "--name", name, ALPINE, "/bin/sh"})
Expand Down

0 comments on commit 8eb8d73

Please sign in to comment.