Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExtraPortMapping Validation #3302

Closed
wants to merge 2 commits into from
Closed

ExtraPortMapping Validation #3302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9912a3
move extra port mapping defaulting to API level
aroradaman Nov 30, 2023
6fa6758
fix extra port mapping validation
aroradaman Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions pkg/apis/config/v1alpha4/default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,14 @@ 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 to listening on 127.0.0.1:randomPort on ipv4
// and [::1]:randomPort on ipv6
Expand Down Expand Up @@ -79,12 +80,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 = "::"
} else {
obj.ExtraPortMappings[i].ListenAddress = "0.0.0.0"
}
}

if string(obj.ExtraPortMappings[i].Protocol) == "" {
obj.ExtraPortMappings[i].Protocol = PortMappingProtocolTCP
}
}
}
40 changes: 12 additions & 28 deletions pkg/cluster/internal/providers/docker/provision.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,18 @@ func planCreation(cfg *config.Cluster, networkName string) (createContainerFuncs
ListenAddress: apiServerAddress,
HostPort: apiServerPort,
ContainerPort: common.APIServerInternalPort,
Protocol: config.PortMappingProtocolTCP,
},
)
args, err := runArgsForNode(node, cfg.Networking.IPFamily, name, genericArgs)
args, err := runArgsForNode(node, name, genericArgs)
if err != nil {
return err
}
return createContainerWithWaitUntilSystemdReachesMultiUserSystem(name, args)
})
case config.WorkerRole:
createContainerFuncs = append(createContainerFuncs, func() error {
args, err := runArgsForNode(node, cfg.Networking.IPFamily, name, genericArgs)
args, err := runArgsForNode(node, name, genericArgs)
if err != nil {
return err
}
Expand Down Expand Up @@ -212,7 +213,7 @@ func commonArgs(cluster string, cfg *config.Cluster, networkName string, nodeNam
return args, nil
}

func runArgsForNode(node *config.Node, clusterIPFamily config.ClusterIPFamily, name string, args []string) ([]string, error) {
func runArgsForNode(node *config.Node, name string, args []string) ([]string, error) {
args = append([]string{
"--hostname", name, // make hostname match container name
// label the node with the role ID
Expand Down Expand Up @@ -244,7 +245,7 @@ func runArgsForNode(node *config.Node, clusterIPFamily config.ClusterIPFamily, n

// convert mounts and port mappings to container run args
args = append(args, generateMountBindings(node.ExtraMounts...)...)
mappingArgs, err := generatePortMappings(clusterIPFamily, node.ExtraPortMappings...)
mappingArgs, err := generatePortMappings(node.ExtraPortMappings...)
if err != nil {
return nil, err
}
Expand All @@ -269,13 +270,12 @@ func runArgsForLoadBalancer(cfg *config.Cluster, name string, args []string) ([]
)

// load balancer port mapping
mappingArgs, err := generatePortMappings(cfg.Networking.IPFamily,
config.PortMapping{
ListenAddress: cfg.Networking.APIServerAddress,
HostPort: cfg.Networking.APIServerPort,
ContainerPort: common.APIServerInternalPort,
},
)
mappingArgs, err := generatePortMappings(config.PortMapping{
ListenAddress: cfg.Networking.APIServerAddress,
HostPort: cfg.Networking.APIServerPort,
ContainerPort: common.APIServerInternalPort,
Protocol: config.PortMappingProtocolTCP,
})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -357,25 +357,9 @@ func generateMountBindings(mounts ...config.Mount) []string {
}

// generatePortMappings converts the portMappings list to a list of args for docker
func generatePortMappings(clusterIPFamily config.ClusterIPFamily, portMappings ...config.PortMapping) ([]string, error) {
func generatePortMappings(portMappings ...config.PortMapping) ([]string, error) {
args := make([]string, 0, len(portMappings))
for _, pm := range portMappings {
// do provider internal defaulting
// in a future API revision we will handle this at the API level and remove this
if pm.ListenAddress == "" {
switch clusterIPFamily {
case config.IPv4Family, config.DualStackFamily:
pm.ListenAddress = "0.0.0.0" // this is the docker default anyhow
case config.IPv6Family:
pm.ListenAddress = "::"
default:
return nil, errors.Errorf("unknown cluster IP family: %v", clusterIPFamily)
}
}
if string(pm.Protocol) == "" {
pm.Protocol = config.PortMappingProtocolTCP // TCP is the default
}

// validate that the provider can handle this binding
switch pm.Protocol {
case config.PortMappingProtocolTCP:
Expand Down
40 changes: 12 additions & 28 deletions pkg/cluster/internal/providers/podman/provision.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,18 @@ func planCreation(cfg *config.Cluster, networkName string) (createContainerFuncs
ListenAddress: apiServerAddress,
HostPort: apiServerPort,
ContainerPort: common.APIServerInternalPort,
Protocol: config.PortMappingProtocolTCP,
},
)
args, err := runArgsForNode(node, cfg.Networking.IPFamily, name, genericArgs)
args, err := runArgsForNode(node, name, genericArgs)
if err != nil {
return err
}
return createContainerWithWaitUntilSystemdReachesMultiUserSystem(name, args)
})
case config.WorkerRole:
createContainerFuncs = append(createContainerFuncs, func() error {
args, err := runArgsForNode(node, cfg.Networking.IPFamily, name, genericArgs)
args, err := runArgsForNode(node, name, genericArgs)
if err != nil {
return err
}
Expand Down Expand Up @@ -173,7 +174,7 @@ func commonArgs(cfg *config.Cluster, networkName string, nodeNames []string) ([]
return args, nil
}

func runArgsForNode(node *config.Node, clusterIPFamily config.ClusterIPFamily, name string, args []string) ([]string, error) {
func runArgsForNode(node *config.Node, name string, args []string) ([]string, error) {
// Pre-create anonymous volumes to enable specifying mount options
// during container run time
varVolume, err := createAnonymousVolume(name)
Expand Down Expand Up @@ -214,7 +215,7 @@ func runArgsForNode(node *config.Node, clusterIPFamily config.ClusterIPFamily, n

// convert mounts and port mappings to container run args
args = append(args, generateMountBindings(node.ExtraMounts...)...)
mappingArgs, err := generatePortMappings(clusterIPFamily, node.ExtraPortMappings...)
mappingArgs, err := generatePortMappings(node.ExtraPortMappings...)
if err != nil {
return nil, err
}
Expand All @@ -240,13 +241,12 @@ func runArgsForLoadBalancer(cfg *config.Cluster, name string, args []string) ([]
)

// load balancer port mapping
mappingArgs, err := generatePortMappings(cfg.Networking.IPFamily,
config.PortMapping{
ListenAddress: cfg.Networking.APIServerAddress,
HostPort: cfg.Networking.APIServerPort,
ContainerPort: common.APIServerInternalPort,
},
)
mappingArgs, err := generatePortMappings(config.PortMapping{
ListenAddress: cfg.Networking.APIServerAddress,
HostPort: cfg.Networking.APIServerPort,
ContainerPort: common.APIServerInternalPort,
Protocol: config.PortMappingProtocolTCP,
})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -370,25 +370,9 @@ func generateMountBindings(mounts ...config.Mount) []string {
}

// generatePortMappings converts the portMappings list to a list of args for podman
func generatePortMappings(clusterIPFamily config.ClusterIPFamily, portMappings ...config.PortMapping) ([]string, error) {
func generatePortMappings(portMappings ...config.PortMapping) ([]string, error) {
args := make([]string, 0, len(portMappings))
for _, pm := range portMappings {
// do provider internal defaulting
// in a future API revision we will handle this at the API level and remove this
if pm.ListenAddress == "" {
switch clusterIPFamily {
case config.IPv4Family, config.DualStackFamily:
pm.ListenAddress = "0.0.0.0"
case config.IPv6Family:
pm.ListenAddress = "::"
default:
return nil, errors.Errorf("unknown cluster IP family: %v", clusterIPFamily)
}
}
if string(pm.Protocol) == "" {
pm.Protocol = config.PortMappingProtocolTCP // TCP is the default
}

// validate that the provider can handle this binding
switch pm.Protocol {
case config.PortMappingProtocolTCP:
Expand Down
25 changes: 20 additions & 5 deletions pkg/internal/apis/config/default.go
View file
Open in desktop

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
Open in desktop
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
Loading
Loading