Skip to content

Commit

Permalink
pkg/cluster: raise error if host-ports are configured again with same…
Browse files Browse the repository at this point in the history 
… addr and protocol

Signed-off-by: Daman <aroradaman@gmail.com>
  • Loading branch information
@aroradaman
aroradaman committed Apr 29, 2023
1 parent 49c8845 commit 0af59a5
Show file tree
Hide file tree
Showing 2 changed files with 181 additions and 0 deletions.
63 changes: 63 additions & 0 deletions pkg/internal/apis/config/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"strings"

"sigs.k8s.io/kind/pkg/errors"
"sigs.k8s.io/kind/pkg/internal/sets"
)

// similar to valid docker container names, but since we will prefix
Expand Down Expand Up @@ -118,18 +119,80 @@ func (n *Node) Validate() error {
if err := validatePort(mapping.HostPort); err != nil {
errs = append(errs, errors.Wrapf(err, "invalid hostPort"))
}

if err := validatePort(mapping.ContainerPort); err != nil {
errs = append(errs, errors.Wrapf(err, "invalid containerPort"))
}
}

if err := validatePortMappings(n.ExtraPortMappings); err != nil {
errs = append(errs, errors.Wrapf(err, "invalid portMapping"))
}

if len(errs) > 0 {
return errors.NewAggregate(errs)
}

return nil
}

func validatePortMappings(portMappings []PortMapping) error {
errMsg := "port mapping with same listen address, port and protocol already configured"

wildcardAddrIPv4 := net.ParseIP("0.0.0.0")
wildcardAddrIPv6 := net.ParseIP("::")

// bindMap has the following key-value structure
// PORT/PROTOCOL: [ IP ]
// { 80/TCP: [ 127.0.0.1, 192.168.2.3 ], 80/UDP: [ 0.0.0.0 ] }
bindMap := make(map[string]sets.String)

formatPortProtocol := func(port int32, protocol PortMappingProtocol) string {
return fmt.Sprintf("%d/%s", port, protocol)
}

for _, portMapping := range portMappings {
addr := net.ParseIP(portMapping.ListenAddress)
addrString := addr.String()

portProtocol := formatPortProtocol(portMapping.HostPort, portMapping.Protocol)
possibleErr := fmt.Errorf("%s: %s:%s", errMsg, addrString, portProtocol)

if _, ok := bindMap[portProtocol]; ok {
// in golang 0.0.0.0 and [::] are equivalent, for any wild card address if there
// exists any other address for same port & protocol immediately return error.
// https://github.com/golang/go/issues/48723
if addr.Equal(wildcardAddrIPv4) || addr.Equal(wildcardAddrIPv6) {
if bindMap[portProtocol].Len() > 0 {
return possibleErr
}
}

// direct duplicate check:
// return error if same combination of ip, port and protocol already exists in bindMap.
if bindMap[portProtocol].Has(addrString) {
return possibleErr
}

// wildcard address already present check:
// return error if wildcard address is already present for same port & protocol
for _, otherAddrString := range bindMap[portProtocol].UnsortedList() {
if otherAddrString == wildcardAddrIPv4.String() || otherAddrString == wildcardAddrIPv6.String() {
return possibleErr
}
}

} else {
// initialize the set
bindMap[portProtocol] = sets.NewString()
}

// add the entry to bindMap
bindMap[portProtocol].Insert(addrString)
}
return nil
}

func validatePort(port int32) error {
// NOTE: -1 is a special value for auto-selecting the port in the container
// backend where possible as opposed to in kind itself.
Expand Down
118 changes: 118 additions & 0 deletions pkg/internal/apis/config/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package config

import (
"fmt"
"sigs.k8s.io/kind/pkg/internal/assert"
"testing"

"sigs.k8s.io/kind/pkg/errors"
Expand Down Expand Up @@ -428,3 +430,119 @@ func TestPortValidate(t *testing.T) {
})
}
}

func TestValidatePortMappings(t *testing.T) {
newPortMapping := func(addr string, port int, protocol string) PortMapping {
return PortMapping{
HostPort: int32(port),
ListenAddress: addr,
Protocol: PortMappingProtocol(protocol),
}
}
errMsg := "port mapping with same listen address, port and protocol already configured"
cases := []struct {
testName string
portMappings []PortMapping
expectErr string
}{
{
testName: "unique port mappings ipv4",
portMappings: []PortMapping{
newPortMapping("127.0.0.1", 80, "UDP"),
newPortMapping("127.0.0.1", 80, "TCP"),
newPortMapping("0.0.0.0", 3000, "UDP"),
newPortMapping("0.0.0.0", 5000, "TCP"),
},
expectErr: "",
},
{
testName: "unique port mappings ipv6",
portMappings: []PortMapping{
newPortMapping("::1", 80, "UDP"),
newPortMapping("::1", 80, "TCP"),
newPortMapping("1e3d:6e85:424d:a011:a72e:9780:5f6f:a6fc", 3000, "UDP"),
newPortMapping("6516:944d:e070:a1d1:2e91:8437:a6b3:edf9", 5000, "TCP"),
},
expectErr: "",
},
{
testName: "exact duplicate port mappings ipv4",
portMappings: []PortMapping{
newPortMapping("127.0.0.1", 80, "TCP"),
newPortMapping("127.0.0.1", 80, "UDP"),
newPortMapping("127.0.0.1", 80, "TCP"),
},
// error expected: exact duplicate
expectErr: fmt.Sprintf("%s: 127.0.0.1:80/TCP", errMsg),
},

{
testName: "exact duplicate port mappings ipv6",
portMappings: []PortMapping{
newPortMapping("::1", 80, "TCP"),
newPortMapping("::1", 80, "UDP"),
newPortMapping("::1", 80, "TCP"),
},
// error expected: exact duplicate
expectErr: fmt.Sprintf("%s: [::1]:80/TCP", errMsg),
},
{
testName: "wildcard ipv4 & ipv6",
portMappings: []PortMapping{
newPortMapping("127.0.0.1", 80, "TCP"),
newPortMapping("0.0.0.0", 80, "UDP"),
newPortMapping("::1", 80, "TCP"),
newPortMapping("::", 80, "UDP"),
},
// error expected: 0.0.0.0 & [::] are same in golang
// https://github.com/golang/go/issues/48723
expectErr: fmt.Sprintf("%s: [::]:80/UDP", errMsg),
},
{
testName: "subset already configured ipv4",
portMappings: []PortMapping{
newPortMapping("127.0.0.1", 80, "TCP"),
newPortMapping("0.0.0.0", 80, "TCP"),
},
// error expected: subset of 0.0.0.0 -> 127.0.0.1 is already defined for same port and protocol
expectErr: fmt.Sprintf("%s: 0.0.0.0:80/TCP", errMsg),
},
{
testName: "subset already configured ipv6",
portMappings: []PortMapping{
newPortMapping("::1", 80, "TCP"),
newPortMapping("::", 80, "TCP"),
},
// error expected: subset of :: -> ::1 is already defined for same port and protocol
expectErr: fmt.Sprintf("%s: [::]:80/TCP", errMsg),
},
{
testName: "port mapping already configured via wildcard ipv4",
portMappings: []PortMapping{
newPortMapping("0.0.0.0", 80, "TCP"),
newPortMapping("127.0.0.1", 80, "TCP"),
},
// error expected: port mapping is already defined for wildcard interface - 0.0.0.0
expectErr: fmt.Sprintf("%s: 127.0.0.1:80/TCP", errMsg),
},
{
testName: "port mapping already configured via wildcard ipv6",
portMappings: []PortMapping{
newPortMapping("::", 80, "SCTP"),
newPortMapping("::1", 80, "SCTP"),
},
// error expected: port mapping is already defined for wildcard interface - ::
expectErr: fmt.Sprintf("%s: [::1]:80/SCTP", errMsg),
},
}

for _, tc := range cases {
tc := tc //capture loop variable
t.Run(tc.testName, func(t *testing.T) {
t.Parallel()

err := validatePortMappings(tc.portMappings)
assert.ExpectError(t, len(tc.expectErr) > 0, err)
})
}
}

0 comments on commit 0af59a5

Please sign in to comment.