Skip to content

Commit

Permalink
fix-extra-port-mapping
Browse files Browse the repository at this point in the history 
Signed-off-by: Daman Arora <[email protected]>
  • Loading branch information
@aroradaman
aroradaman committed Jul 14, 2023
1 parent decbc46 commit 9ba8845
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 43 deletions.
6 changes: 6 additions & 0 deletions pkg/apis/config/defaults/networking.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,9 @@ const ServiceSubnetIPv6 = "fd00:10:96::/112"

// ServiceSubnetDualStack is the default DualStack subnet for the Networking.ServiceSubnet field
const ServiceSubnetDualStack = "10.96.0.0/16,fd00:10:96::/112"

// ExtraPortMappingListenAddressIPv4 is the default IPv4/DualStack listen address for ExtraPortMappings
const ExtraPortMappingListenAddressIPv4 = "0.0.0.0"

// ExtraPortMappingListenAddressIPv6 is the default IPv6 listen address for ExtraPortMappings
const ExtraPortMappingListenAddressIPv6 = "::"
27 changes: 22 additions & 5 deletions pkg/apis/config/v1alpha4/default.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,17 @@ func SetDefaultsCluster(obj *Cluster) {
},
}
}

if obj.Networking.IPFamily == "" {
obj.Networking.IPFamily = IPv4Family
}

// default the nodes
for i := range obj.Nodes {
a := &obj.Nodes[i]
SetDefaultsNode(a)
}
if obj.Networking.IPFamily == "" {
obj.Networking.IPFamily = IPv4Family
SetDefaultsNode(a, obj.Networking.IPFamily)
}

// default the API server address
if obj.Networking.APIServerAddress == "" {
obj.Networking.APIServerAddress = defaults.APIServerAddressIPv4
Expand Down Expand Up @@ -75,12 +78,26 @@ func SetDefaultsCluster(obj *Cluster) {
}

// SetDefaultsNode sets uninitialized fields to their default value.
func SetDefaultsNode(obj *Node) {
func SetDefaultsNode(obj *Node, ipFamily ClusterIPFamily) {
if obj.Image == "" {
obj.Image = defaults.Image
}

if obj.Role == "" {
obj.Role = ControlPlaneRole
}

for i := 0; i < len(obj.ExtraPortMappings); i++ {
if obj.ExtraPortMappings[i].ListenAddress == "" {
if ipFamily == IPv6Family {
obj.ExtraPortMappings[i].ListenAddress = defaults.ExtraPortMappingListenAddressIPv6
} else {
obj.ExtraPortMappings[i].ListenAddress = defaults.ExtraPortMappingListenAddressIPv4
}
}

if string(obj.ExtraPortMappings[i].Protocol) == "" {
obj.ExtraPortMappings[i].Protocol = PortMappingProtocolTCP
}
}
}
25 changes: 20 additions & 5 deletions pkg/internal/apis/config/default.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions pkg/internal/apis/config/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"net"
"regexp"
"strconv"
"strings"

"sigs.k8s.io/kind/pkg/errors"
Expand Down Expand Up @@ -137,7 +138,7 @@ func (n *Node) Validate() error {
}

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

wildcardAddrIPv4 := net.ParseIP("0.0.0.0")
wildcardAddrIPv6 := net.ParseIP("::")
Expand All @@ -152,11 +153,16 @@ func validatePortMappings(portMappings []PortMapping) error {
}

for _, portMapping := range portMappings {
// skipping validation if host port is not defined
if portMapping.HostPort == 0 {
continue
}

addr := net.ParseIP(portMapping.ListenAddress)
addrString := addr.String()

portProtocol := formatPortProtocol(portMapping.HostPort, portMapping.Protocol)
possibleErr := fmt.Errorf("%s: %s:%s", errMsg, addrString, portProtocol)
possibleErr := fmt.Errorf("%s: %s/%s", errMsg, net.JoinHostPort(addrString, strconv.Itoa(int(portMapping.HostPort))), portMapping.Protocol)

// in golang 0.0.0.0 and [::] are equivalent, convert [::] -> 0.0.0.0
// https://github.com/golang/go/issues/48723
Expand Down
91 changes: 60 additions & 31 deletions pkg/internal/apis/config/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package config

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

"sigs.k8s.io/kind/pkg/errors"
Expand Down Expand Up @@ -432,45 +431,66 @@ func TestPortValidate(t *testing.T) {
}

func TestValidatePortMappings(t *testing.T) {
newPortMapping := func(addr string, port int, protocol string) PortMapping {
newPortMapping := func(addr string, hostPort, containerPort int, protocol string) PortMapping {
return PortMapping{
HostPort: int32(port),
HostPort: int32(hostPort),
ContainerPort: int32(containerPort),
ListenAddress: addr,
Protocol: PortMappingProtocol(protocol),
}
}
errMsg := "port mapping with same listen address, port and protocol already configured"
errMsg := "port mapping with same listen address, host port and protocol already configured"
cases := []struct {
testName string
portMappings []PortMapping
expectErr string
}{
{
testName: "unique container ports",
portMappings: []PortMapping{
newPortMapping("", 0, 1000, ""),
newPortMapping("", 0, 2000, ""),
newPortMapping("", 0, 3000, ""),
newPortMapping("", 0, 4000, ""),
},
expectErr: "",
},
{
testName: "duplicate container ports",
portMappings: []PortMapping{
newPortMapping("", 0, 1000, ""),
newPortMapping("", 0, 2000, ""),
newPortMapping("", 0, 3000, ""),
newPortMapping("", 0, 3000, ""),
},
expectErr: "",
},
{
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"),
newPortMapping("127.0.0.1", 80, 5000, "UDP"),
newPortMapping("127.0.0.1", 80, 5000, "TCP"),
newPortMapping("0.0.0.0", 3000, 8000, "UDP"),
newPortMapping("0.0.0.0", 5000, 8000, "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"),
newPortMapping("::1", 80, 5000, "UDP"),
newPortMapping("::1", 80, 5000, "TCP"),
newPortMapping("1e3d:6e85:424d:a011:a72e:9780:5f6f:a6fc", 3000, 8000, "UDP"),
newPortMapping("6516:944d:e070:a1d1:2e91:8437:a6b3:edf9", 5000, 8000, "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"),
newPortMapping("127.0.0.1", 80, 3000, "TCP"),
newPortMapping("127.0.0.1", 80, 5000, "UDP"),
newPortMapping("127.0.0.1", 80, 8000, "TCP"),
},
// error expected: exact duplicate
expectErr: fmt.Sprintf("%s: 127.0.0.1:80/TCP", errMsg),
Expand All @@ -479,20 +499,20 @@ func TestValidatePortMappings(t *testing.T) {
{
testName: "exact duplicate port mappings ipv6",
portMappings: []PortMapping{
newPortMapping("::1", 80, "TCP"),
newPortMapping("::1", 80, "UDP"),
newPortMapping("::1", 80, "TCP"),
newPortMapping("::1", 80, 3000, "TCP"),
newPortMapping("::1", 80, 5000, "UDP"),
newPortMapping("::1", 80, 8000, "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"),
newPortMapping("127.0.0.1", 80, 3000, "TCP"),
newPortMapping("0.0.0.0", 80, 5000, "UDP"),
newPortMapping("::1", 80, 3000, "TCP"),
newPortMapping("::", 80, 5000, "UDP"),
},
// error expected: 0.0.0.0 & [::] are same in golang
// https://github.com/golang/go/issues/48723
Expand All @@ -501,35 +521,35 @@ func TestValidatePortMappings(t *testing.T) {
{
testName: "subset already configured ipv4",
portMappings: []PortMapping{
newPortMapping("127.0.0.1", 80, "TCP"),
newPortMapping("0.0.0.0", 80, "TCP"),
newPortMapping("127.0.0.1", 80, 3000, "TCP"),
newPortMapping("0.0.0.0", 80, 5000, "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"),
newPortMapping("::1", 80, 3000, "TCP"),
newPortMapping("::", 80, 5000, "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"),
newPortMapping("0.0.0.0", 80, 5000, "TCP"),
newPortMapping("127.0.0.1", 80, 5000, "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"),
newPortMapping("::", 80, 5000, "SCTP"),
newPortMapping("::1", 80, 5000, "SCTP"),
},
// error expected: port mapping is already defined for wildcard interface - ::
expectErr: fmt.Sprintf("%s: [::1]:80/SCTP", errMsg),
Expand All @@ -542,7 +562,16 @@ func TestValidatePortMappings(t *testing.T) {
t.Parallel()

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

// the error can be:
// - nil, in which case we should expect no errors or fail
if err == nil && len(tc.expectErr) > 0 {
t.Errorf("Test failed, unexpected error: %s", tc.expectErr)
}

if err != nil && err.Error() != tc.expectErr {
t.Errorf("Test failed, error: %s expected error: %s", err, tc.expectErr)
}
})
}
}

0 comments on commit 9ba8845

Please sign in to comment.