Skip to content

Commit

Permalink
fix: account for tls termination in exposed port validation (#5549)
Browse files Browse the repository at this point in the history
Addresses #5536 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
  • Loading branch information
CaptainCarpensir authored Dec 11, 2023
1 parent 8de7d2b commit 048c067
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
7 changes: 7 additions & 0 deletions internal/pkg/manifest/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2226,6 +2226,7 @@ func validateAndPopulateNLBListenerPorts(listener NetworkLoadBalancerListener, p
if err != nil {
return err
}

port, err := strconv.ParseUint(aws.StringValue(nlbReceiverPort), 10, 16)
if err != nil {
return err
Expand All @@ -2242,6 +2243,11 @@ func validateAndPopulateNLBListenerPorts(listener NetworkLoadBalancerListener, p
targetProtocol = strings.ToUpper(aws.StringValue(nlbProtocol))
}

// Handle TLS termination of container exposed port protocol
if targetProtocol == TLS {
targetProtocol = TCP
}

// Prefer `nlb.target_container`, then existing exposed port mapping, then fallback on name of main container
targetContainer := mainContainerName
if existingContainerNameAndProtocol, ok := portExposedTo[targetPort]; ok {
Expand All @@ -2256,6 +2262,7 @@ func validateAndPopulateNLBListenerPorts(listener NetworkLoadBalancerListener, p

func validateAndPopulateExposedPortMapping(portExposedTo map[uint16]containerNameAndProtocol, targetPort uint16, targetProtocol string, targetContainer string) error {
exposedContainerAndProtocol, alreadyExposed := portExposedTo[targetPort]
targetProtocol = strings.ToUpper(targetProtocol)

// Port is not associated with container and protocol, populate map
if !alreadyExposed {
Expand Down
22 changes: 20 additions & 2 deletions internal/pkg/manifest/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4196,7 +4196,25 @@ func TestValidateExposedPorts(t *testing.T) {
},
wanted: nil,
},
"should not error out when nlb target_port is same as that of sidecar container port but sidecar uses non default protocol": {
"should not error out when tls is terminated exposing a tcp port": {
in: validateExposedPortsOpts{
mainContainerName: "mockMainContainer",
mainContainerPort: aws.Uint16(8080),
sidecarConfig: map[string]*SidecarConfig{
"foo": {
Port: aws.String("80/tcp"),
},
},
nlb: &NetworkLoadBalancerConfiguration{
Listener: NetworkLoadBalancerListener{
Port: aws.String("8080/tls"),
TargetPort: aws.Int(80),
},
},
},
wanted: nil,
},
"should return an error when nlb target_port is same as that of sidecar container port but sidecar uses non default protocol": {
in: validateExposedPortsOpts{
mainContainerName: "mockMainContainer",
mainContainerPort: aws.Uint16(8080),
Expand All @@ -4212,7 +4230,7 @@ func TestValidateExposedPorts(t *testing.T) {
},
},
},
wanted: fmt.Errorf(`validate "nlb": container "foo" is exposing the same port 80 with protocol TCP and udp`),
wanted: fmt.Errorf(`validate "nlb": container "foo" is exposing the same port 80 with protocol TCP and UDP`),
},
"should return an error if alb target_port points to one sidecar container port and target_container points to another sidecar container": {
in: validateExposedPortsOpts{
Expand Down

0 comments on commit 048c067

Please sign in to comment.