Skip to content

Commit

Permalink
change ContainerPort and ContainerPortRange to non-references
Browse files Browse the repository at this point in the history
  • Loading branch information
@singholt
singholt committed Nov 28, 2022
1 parent 7f8f7b5 commit dd11268
Show file tree
Hide file tree
Showing 19 changed files with 84 additions and 94 deletions.
7 changes: 3 additions & 4 deletions agent/api/container/port_binding.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (

apierrors "github.com/aws/amazon-ecs-agent/agent/api/errors"

"github.com/aws/aws-sdk-go/aws"
"github.com/docker/go-connections/nat"
)

Expand All @@ -32,9 +31,9 @@ const (
// PortBinding represents a port binding for a container
type PortBinding struct {
// ContainerPort is the port inside the container
ContainerPort *uint16
ContainerPort uint16
// ContainerPortRange is a range of ports exposed inside the container
ContainerPortRange *string
ContainerPortRange string
// HostPort is the port exposed on the host
HostPort uint16
// BindIP is the IP address to which the port is bound
Expand Down Expand Up @@ -64,7 +63,7 @@ func PortBindingFromDockerPortBinding(dockerPortBindings nat.PortMap) ([]PortBin
return nil, &apierrors.DefaultNamedError{Name: UnparseablePortErrorName, Err: "Error parsing port binding as int " + err.Error()}
}
portBindings = append(portBindings, PortBinding{
ContainerPort: aws.Uint16(uint16(containerPort)),
ContainerPort: uint16(containerPort),
HostPort: uint16(hostPort),
BindIP: binding.HostIP,
Protocol: protocol,
Expand Down
7 changes: 3 additions & 4 deletions agent/api/container/port_binding_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

apierrors "github.com/aws/amazon-ecs-agent/agent/api/errors"

"github.com/aws/aws-sdk-go/aws"
"github.com/docker/go-connections/nat"
)

Expand All @@ -41,7 +40,7 @@ func TestPortBindingFromDockerPortBinding(t *testing.T) {
{
BindIP: "1.2.3.4",
HostPort: 55,
ContainerPort: aws.Uint16(53),
ContainerPort: 53,
Protocol: TransportProtocolUDP,
},
},
Expand All @@ -57,13 +56,13 @@ func TestPortBindingFromDockerPortBinding(t *testing.T) {
{
BindIP: "2.3.4.5",
HostPort: 8080,
ContainerPort: aws.Uint16(80),
ContainerPort: 80,
Protocol: TransportProtocolTCP,
},
{
BindIP: "5.6.7.8",
HostPort: 80,
ContainerPort: aws.Uint16(80),
ContainerPort: 80,
Protocol: TransportProtocolTCP,
},
},
Expand Down
2 changes: 1 addition & 1 deletion agent/api/ecsclient/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ func getNetworkBindings(change api.ContainerStateChange, shouldExcludeIPv6PortBi
}

hostPort := int64(binding.HostPort)
containerPort := int64(aws.Uint16Value(binding.ContainerPort))
containerPort := int64(binding.ContainerPort)
bindIP := binding.BindIP
protocol := binding.Protocol.String()

Expand Down
32 changes: 16 additions & 16 deletions agent/api/ecsclient/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,24 +226,24 @@ func TestSubmitContainerStateChange(t *testing.T) {
PortBindings: []apicontainer.PortBinding{
{
BindIP: "1.2.3.4",
ContainerPort: aws.Uint16(1),
ContainerPort: 1,
HostPort: 2,
},
{
BindIP: "2.2.3.4",
ContainerPort: aws.Uint16(3),
ContainerPort: 3,
HostPort: 4,
Protocol: apicontainer.TransportProtocolUDP,
},
{
BindIP: "5.6.7.8",
ContainerPort: aws.Uint16(11),
ContainerPort: 11,
HostPort: 11,
Protocol: apicontainer.TransportProtocolUDP,
},
{
BindIP: "5.6.7.8",
ContainerPort: aws.Uint16(12),
ContainerPort: 12,
HostPort: 12,
Protocol: apicontainer.TransportProtocolUDP,
},
Expand Down Expand Up @@ -1195,25 +1195,25 @@ func getTestContainerStateChange() api.ContainerStateChange {
NetworkModeUnsafe: testNetworkName,
Ports: []apicontainer.PortBinding{
{
ContainerPort: aws.Uint16(10),
ContainerPort: 10,
HostPort: 10,
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPort: aws.Uint16(12),
ContainerPort: 12,
HostPort: 12,
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPort: aws.Uint16(15),
ContainerPort: 15,
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPortRange: aws.String("21-22"),
ContainerPortRange: "21-22",
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPortRange: aws.String("96-97"),
ContainerPortRange: "96-97",
Protocol: apicontainer.TransportProtocolTCP,
},
},
Expand All @@ -1236,43 +1236,43 @@ func getTestContainerStateChange() api.ContainerStateChange {
Container: testContainer,
PortBindings: []apicontainer.PortBinding{
{
ContainerPort: aws.Uint16(10),
ContainerPort: 10,
HostPort: 10,
BindIP: "0.0.0.0",
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPort: aws.Uint16(12),
ContainerPort: 12,
HostPort: 12,
BindIP: "1.2.3.4",
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPort: aws.Uint16(15),
ContainerPort: 15,
HostPort: 20,
BindIP: "5.6.7.8",
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPort: aws.Uint16(21),
ContainerPort: 21,
HostPort: 60001,
BindIP: "::",
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPort: aws.Uint16(22),
ContainerPort: 22,
HostPort: 60002,
BindIP: "::",
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPort: aws.Uint16(96),
ContainerPort: 96,
HostPort: 47001,
BindIP: "0.0.0.0",
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPort: aws.Uint16(97),
ContainerPort: 97,
HostPort: 47002,
BindIP: "0.0.0.0",
Protocol: apicontainer.TransportProtocolTCP,
Expand Down
9 changes: 4 additions & 5 deletions agent/api/statechange_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
apitaskstatus "github.com/aws/amazon-ecs-agent/agent/api/task/status"
execcmd "github.com/aws/amazon-ecs-agent/agent/engine/execcmd"

"github.com/aws/aws-sdk-go/aws"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -161,7 +160,7 @@ func TestNewUncheckedContainerStateChangeEvent(t *testing.T) {
SteadyStateStatusUnsafe: &steadyStateStatus,
KnownExitCodeUnsafe: &exitCode,
KnownPortBindingsUnsafe: []apicontainer.PortBinding{{
ContainerPort: aws.Uint16(1),
ContainerPort: 1,
HostPort: 2,
BindIP: "1.2.3.4",
Protocol: 3,
Expand All @@ -177,7 +176,7 @@ func TestNewUncheckedContainerStateChangeEvent(t *testing.T) {
Status: apicontainerstatus.ContainerRunning,
ExitCode: &exitCode,
PortBindings: []apicontainer.PortBinding{{
ContainerPort: aws.Uint16(1),
ContainerPort: 1,
HostPort: 2,
BindIP: "1.2.3.4",
Protocol: 3,
Expand Down Expand Up @@ -219,7 +218,7 @@ func TestNewUncheckedContainerStateChangeEvent_SCBridge(t *testing.T) {
addPauseContainer: true,
pauseContainerName: fmt.Sprintf("%s-%s", apitask.NetworkPauseContainerName, testContainerName),
pauseContainerPortBindings: []apicontainer.PortBinding{{
ContainerPort: aws.Uint16(1),
ContainerPort: 1,
HostPort: 2,
BindIP: "1.2.3.4",
Protocol: 3,
Expand Down Expand Up @@ -252,7 +251,7 @@ func TestNewUncheckedContainerStateChangeEvent_SCBridge(t *testing.T) {
SteadyStateStatusUnsafe: &steadyStateStatus,
KnownExitCodeUnsafe: &exitCode,
KnownPortBindingsUnsafe: []apicontainer.PortBinding{{
ContainerPort: aws.Uint16(8080), // we get this from task definition
ContainerPort: 8080, // we get this from task definition
}},
ImageDigest: "image",
},
Expand Down
18 changes: 9 additions & 9 deletions agent/api/task/task.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ func (task *Task) initServiceConnectEphemeralPorts() error {
// to avoid port conflicts.
for _, c := range task.Containers {
for _, p := range c.Ports {
utilizedPorts = append(utilizedPorts, aws.Uint16Value(p.ContainerPort))
utilizedPorts = append(utilizedPorts, p.ContainerPort)
}
}

Expand Down Expand Up @@ -1785,12 +1785,12 @@ func (task *Task) dockerExposedPorts(container *apicontainer.Container) (dockerE
for _, portBinding := range containerToCheck.Ports {
protocol := portBinding.Protocol.String()
// per port binding config, either one of ContainerPort or ContainerPortRange is set
if portBinding.ContainerPort != nil {
dockerPort := nat.Port(strconv.Itoa(int(aws.Uint16Value(portBinding.ContainerPort))) + "/" + protocol)
if portBinding.ContainerPort != 0 {
dockerPort := nat.Port(strconv.Itoa(int(portBinding.ContainerPort)) + "/" + protocol)
dockerExposedPorts[dockerPort] = struct{}{}
} else if portBinding.ContainerPortRange != nil {
} else if portBinding.ContainerPortRange != "" {
// we supply containerPortRange here in case we did not assign a host port range and ask docker to do so
dockerPortRange, err := nat.NewPort(protocol, aws.StringValue(portBinding.ContainerPortRange))
dockerPortRange, err := nat.NewPort(protocol, portBinding.ContainerPortRange)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -2349,18 +2349,18 @@ func (task *Task) dockerPortMap(container *apicontainer.Container) (nat.PortMap,
containerPortRangeMap := make(map[string]string)
for _, portBinding := range containerToCheck.Ports {
// for each port binding config, either one of containerPort or containerPortRange is set
if portBinding.ContainerPort != nil {
containerPort := int(aws.Uint16Value(portBinding.ContainerPort))
if portBinding.ContainerPort != 0 {
containerPort := int(portBinding.ContainerPort)

dockerPort := nat.Port(strconv.Itoa(containerPort) + "/" + portBinding.Protocol.String())
dockerPortMap[dockerPort] = append(dockerPortMap[dockerPort], nat.PortBinding{HostPort: strconv.Itoa(int(portBinding.HostPort))})

// append non-range, singular container port to the containerPortSet
containerPortSet[containerPort] = struct{}{}
} else if portBinding.ContainerPortRange != nil {
} else if portBinding.ContainerPortRange != "" {
containerToCheck.SetContainerHasPortRange(true)

containerPortRange := aws.StringValue(portBinding.ContainerPortRange)
containerPortRange := portBinding.ContainerPortRange
// nat.ParsePortRangeToInt validates a port range; if valid, it returns start and end ports as integers
startContainerPort, endContainerPort, err := nat.ParsePortRangeToInt(containerPortRange)
if err != nil {
Expand Down
28 changes: 14 additions & 14 deletions agent/api/task/task_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,24 +87,24 @@ func TestDockerConfigPortBinding(t *testing.T) {
Name: "c1",
Ports: []apicontainer.PortBinding{
{
ContainerPort: aws.Uint16(10),
ContainerPort: 10,
HostPort: 10,
BindIP: "",
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPort: aws.Uint16(20),
ContainerPort: 20,
HostPort: 20,
BindIP: "",
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPortRange: aws.String("99-999"),
ContainerPortRange: "99-999",
BindIP: "",
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPortRange: aws.String("121-221"),
ContainerPortRange: "121-221",
BindIP: "",
Protocol: apicontainer.TransportProtocolUDP,
},
Expand Down Expand Up @@ -221,13 +221,13 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
Name: "c1",
Ports: []apicontainer.PortBinding{
{
ContainerPort: aws.Uint16(10),
ContainerPort: 10,
HostPort: 10,
BindIP: "",
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPort: aws.Uint16(20),
ContainerPort: 20,
HostPort: 20,
BindIP: "",
Protocol: apicontainer.TransportProtocolUDP,
Expand All @@ -243,12 +243,12 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
Name: "c1",
Ports: []apicontainer.PortBinding{
{
ContainerPortRange: aws.String("999-1000"),
ContainerPortRange: "999-1000",
BindIP: "",
Protocol: apicontainer.TransportProtocolTCP,
},
{
ContainerPortRange: aws.String("1-3"),
ContainerPortRange: "1-3",
BindIP: "",
Protocol: apicontainer.TransportProtocolUDP,
},
Expand All @@ -263,12 +263,12 @@ func TestDockerHostConfigPortBinding(t *testing.T) {
Name: "c1",
Ports: []apicontainer.PortBinding{
{
ContainerPortRange: aws.String("55-57"),
ContainerPortRange: "55-57",
BindIP: "",
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPort: aws.Uint16(80),
ContainerPort: 80,
BindIP: "",
Protocol: apicontainer.TransportProtocolTCP,
},
Expand Down Expand Up @@ -384,8 +384,8 @@ func getTestTaskServiceConnectBridgeMode() *Task {
{
Name: "C1",
Ports: []apicontainer.PortBinding{
{ContainerPort: aws.Uint16(SCTaskContainerPort1), HostPort: 0, BindIP: "", Protocol: apicontainer.TransportProtocolTCP},
{ContainerPort: aws.Uint16(SCTaskContainerPort2), HostPort: 0, BindIP: "", Protocol: apicontainer.TransportProtocolTCP},
{ContainerPort: SCTaskContainerPort1, HostPort: 0, BindIP: "", Protocol: apicontainer.TransportProtocolTCP},
{ContainerPort: SCTaskContainerPort2, HostPort: 0, BindIP: "", Protocol: apicontainer.TransportProtocolTCP},
},
NetworkModeUnsafe: "", // should later be overridden to container mode
},
Expand Down Expand Up @@ -1723,11 +1723,11 @@ func TestTaskFromACS(t *testing.T) {
Ports: []apicontainer.PortBinding{
{
HostPort: 800,
ContainerPort: aws.Uint16(900),
ContainerPort: 900,
Protocol: apicontainer.TransportProtocolUDP,
},
{
ContainerPortRange: strptr("99-199"),
ContainerPortRange: "99-199",
Protocol: apicontainer.TransportProtocolTCP,
},
},
Expand Down
8 changes: 4 additions & 4 deletions agent/api/testutils/container_equal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestContainerEqual(t *testing.T) {
{apicontainer.Container{Memory: 1}, apicontainer.Container{Memory: 1}, true},
{apicontainer.Container{Links: []string{"1", "2"}}, apicontainer.Container{Links: []string{"1", "2"}}, true},
{apicontainer.Container{Links: []string{"1", "2"}}, apicontainer.Container{Links: []string{"2", "1"}}, true},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, true},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, true},
{apicontainer.Container{Essential: true}, apicontainer.Container{Essential: true}, true},
{apicontainer.Container{EntryPoint: nil}, apicontainer.Container{EntryPoint: nil}, true},
{apicontainer.Container{EntryPoint: &[]string{"1", "2"}}, apicontainer.Container{EntryPoint: &[]string{"1", "2"}}, true},
Expand All @@ -66,9 +66,9 @@ func TestContainerEqual(t *testing.T) {
{apicontainer.Container{CPU: 1}, apicontainer.Container{CPU: 2e2}, false},
{apicontainer.Container{Memory: 1}, apicontainer.Container{Memory: 2e2}, false},
{apicontainer.Container{Links: []string{"1", "2"}}, apicontainer.Container{Links: []string{"1", "二"}}, false},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 2, BindIP: "二", Protocol: apicontainer.TransportProtocolTCP}}}, false},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 22, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, false},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: aws.Uint16(1), HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolUDP}}}, false},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 2, BindIP: "二", Protocol: apicontainer.TransportProtocolTCP}}}, false},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 22, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, false},
{apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolTCP}}}, apicontainer.Container{Ports: []apicontainer.PortBinding{{ContainerPort: 1, HostPort: 2, BindIP: "1", Protocol: apicontainer.TransportProtocolUDP}}}, false},
{apicontainer.Container{Essential: true}, apicontainer.Container{Essential: false}, false},
{apicontainer.Container{EntryPoint: nil}, apicontainer.Container{EntryPoint: &[]string{"nonnil"}}, false},
{apicontainer.Container{EntryPoint: &[]string{"1", "2"}}, apicontainer.Container{EntryPoint: &[]string{"2", "1"}}, false},
Expand Down
Loading

0 comments on commit dd11268

Please sign in to comment.