Skip to content

Commit

Permalink
Validate that ClusterIP service range type matches the configuration
Browse files Browse the repository at this point in the history 
and update documentation
  • Loading branch information
@thomasferrandiz @aauren
thomasferrandiz authored and aauren committed Jan 23, 2023
1 parent 9aa7bcd commit 1433bee
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 40 deletions.
2 changes: 1 addition & 1 deletion docs/user-guide.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Usage of kube-router:
--run-router Enables Pod Networking -- Advertises and learns the routes to Pods via iBGP. (default true)
--run-service-proxy Enables Service Proxy -- sets up IPVS for Kubernetes Services. (default true)
--runtime-endpoint string Path to CRI compatible container runtime socket (used for DSR mode). Currently known working with containerd.
--service-cluster-ip-range string CIDR value from which service cluster IPs are assigned. Default: 10.96.0.0/12 (default "10.96.0.0/12")
--service-cluster-ip-range string CIDR value from which service cluster IPs are assigned. If dual-stack is used, this can be a comma-separated list of CIDR value. Default: 10.96.0.0/12 (default "10.96.0.0/12")
--service-external-ip-range strings Specify external IP CIDRs that are used for inter-cluster communication (can be specified multiple times)
--service-node-port-range string NodePort range specified with either a hyphen or colon (default "30000-32767")
-v, --v string log level for V logs (default "0")
Expand Down
35 changes: 29 additions & 6 deletions pkg/controllers/netpol/network_policy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,26 +715,49 @@ func NewNetworkPolicyController(clientset kubernetes.Interface,

// Validate and parse ClusterIP service range
if config.ClusterIPCIDR == "" {
return nil, fmt.Errorf("parameter --service-cluster-ip is empty")
return nil, fmt.Errorf("parameter --service-cluster-ip-range is empty")
}
clusterIPCIDRList := strings.Split(config.ClusterIPCIDR, ",")

if len(clusterIPCIDRList) == 0 {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter, the list is empty")
}

_, primaryIpnet, err := net.ParseCIDR(clusterIPCIDRList[0])
_, primaryIpnet, err := net.ParseCIDR(strings.TrimSpace(clusterIPCIDRList[0]))
if err != nil {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: %w", err)
}
npc.primaryServiceClusterIPRange = primaryIpnet

//Validate that ClusterIP service range type matches the configuration
if config.EnableIPv4 && !config.EnableIPv6 {
if !netutils.IsIPv4CIDR(npc.primaryServiceClusterIPRange) {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: IPv4 is enabled but only IPv6 address is provided")
}
}
if !config.EnableIPv4 && config.EnableIPv6 {
if !netutils.IsIPv6CIDR(npc.primaryServiceClusterIPRange) {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: IPv6 is enabled but only IPv4 address is provided")
}
}

if len(clusterIPCIDRList) > 1 {
_, secondaryIpnet, err := net.ParseCIDR(clusterIPCIDRList[1])
if err != nil {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: %v", err)
if config.EnableIPv4 && config.EnableIPv6 {
_, secondaryIpnet, err := net.ParseCIDR(strings.TrimSpace(clusterIPCIDRList[1]))
if err != nil {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: %v", err)
}
npc.secondaryServiceClusterIPRange = secondaryIpnet

ipv4Provided := netutils.IsIPv4CIDR(npc.primaryServiceClusterIPRange) || netutils.IsIPv4CIDR(npc.secondaryServiceClusterIPRange)
ipv6Provided := netutils.IsIPv6CIDR(npc.primaryServiceClusterIPRange) || netutils.IsIPv6CIDR(npc.secondaryServiceClusterIPRange)
if !(ipv4Provided && ipv6Provided) {
return nil, fmt.Errorf("failed to get parse --service-cluster-ip-range parameter: dual-stack is enabled, both IPv4 and IPv6 addresses should be provided")
}
} else {
return nil, fmt.Errorf("too many CIDRs provided in --service-cluster-ip-range parameter: " +
"dual-stack must be enabled to provide two addresses")
}
npc.secondaryServiceClusterIPRange = secondaryIpnet
}
if len(clusterIPCIDRList) > 2 {
return nil, fmt.Errorf("too many CIDRs provided in --service-cluster-ip-range parameter, only two " +
Expand Down
71 changes: 39 additions & 32 deletions pkg/controllers/netpol/network_policy_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,20 @@ func tCreateFakePods(t *testing.T, podInformer cache.SharedIndexInformer, nsInfo
}

// newFakeNode is a helper function for creating Nodes for testing.
func newFakeNode(name string, addr string) *v1.Node {
func newFakeNode(name string, addrs []string) *v1.Node {
addresses := make([]v1.NodeAddress, len(addrs))
for i, addr := range addrs {
addresses[i] = v1.NodeAddress{Type: v1.NodeExternalIP, Address: addr}
}

return &v1.Node{
ObjectMeta: metav1.ObjectMeta{Name: name},
Status: v1.NodeStatus{
Capacity: v1.ResourceList{
v1.ResourceCPU: resource.MustParse("1"),
v1.ResourceMemory: resource.MustParse("1G"),
},
Addresses: []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: addr}},
Addresses: addresses,
},
}
}
Expand Down Expand Up @@ -270,7 +275,7 @@ func testForMissingOrUnwanted(t *testing.T, targetMsg string, got []podInfo, wan
}
}

func newMinimalKubeRouterConfig(clusterIPCIDR string, nodePortRange string, hostNameOverride string, externalIPs []string) *options.KubeRouterConfig {
func newMinimalKubeRouterConfig(clusterIPCIDR string, nodePortRange string, hostNameOverride string, externalIPs []string, enableIPv6 bool) *options.KubeRouterConfig {
kubeConfig := options.NewKubeRouterConfig()
if clusterIPCIDR != "" {
kubeConfig.ClusterIPCIDR = clusterIPCIDR
Expand All @@ -284,6 +289,8 @@ func newMinimalKubeRouterConfig(clusterIPCIDR string, nodePortRange string, host
if externalIPs != nil {
kubeConfig.ExternalIPCIDRs = externalIPs
}
kubeConfig.EnableIPv4 = true
kubeConfig.EnableIPv6 = enableIPv6
return kubeConfig
}

Expand Down Expand Up @@ -388,7 +395,7 @@ func TestNewNetworkPolicySelectors(t *testing.T) {
},
}

client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*newFakeNode("node", "10.10.10.10")}})
client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*newFakeNode("node", []string{"10.10.10.10"})}})
informerFactory, podInformer, nsInformer, netpolInformer := newFakeInformersFromClient(client)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand Down Expand Up @@ -544,7 +551,7 @@ func TestNetworkPolicyBuilder(t *testing.T) {
},
}

client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*newFakeNode("node", "10.10.10.10")}})
client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*newFakeNode("node", []string{"10.10.10.10"})}})
informerFactory, podInformer, nsInformer, netpolInformer := newFakeInformersFromClient(client)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand Down Expand Up @@ -743,150 +750,150 @@ func TestNetworkPolicyController(t *testing.T) {
testCases := []tNetPolConfigTestCase{
{
"Default options are successful",
newMinimalKubeRouterConfig("", "", "node", nil),
newMinimalKubeRouterConfig("", "", "node", nil, false),
false,
"",
},
{
"Missing nodename fails appropriately",
newMinimalKubeRouterConfig("", "", "", nil),
newMinimalKubeRouterConfig("", "", "", nil, false),
true,
"failed to identify the node by NODE_NAME, hostname or --hostname-override",
},
{
"Test bad cluster CIDR (not properly formatting ip address)",
newMinimalKubeRouterConfig("10.10.10", "", "node", nil),
newMinimalKubeRouterConfig("10.10.10", "", "node", nil, false),
true,
"failed to get parse --service-cluster-ip-range parameter: invalid CIDR address: 10.10.10",
},
{
"Test bad cluster CIDR (not using an ip address)",
newMinimalKubeRouterConfig("foo", "", "node", nil),
newMinimalKubeRouterConfig("foo", "", "node", nil, false),
true,
"failed to get parse --service-cluster-ip-range parameter: invalid CIDR address: foo",
},
{
"Test bad cluster CIDR (using an ip address that is not a CIDR)",
newMinimalKubeRouterConfig("10.10.10.10", "", "node", nil),
newMinimalKubeRouterConfig("10.10.10.10", "", "node", nil, false),
true,
"failed to get parse --service-cluster-ip-range parameter: invalid CIDR address: 10.10.10.10",
},
{
"Test bad cluster CIDRs (using more than 2 ip addresses, including 2 ipv4)",
newMinimalKubeRouterConfig("10.96.0.0/12,10.244.0.0/16,2001:db8:42:1::/112", "", "node", nil),
newMinimalKubeRouterConfig("10.96.0.0/12,10.244.0.0/16,2001:db8:42:1::/112", "", "node", nil, false),
true,
"too many CIDRs provided in --service-cluster-ip-range parameter, only two addresses are allowed at once for dual-stack",
"too many CIDRs provided in --service-cluster-ip-range parameter: dual-stack must be enabled to provide two addresses",
},
{
"Test bad cluster CIDRs (using more than 2 ip addresses, including 2 ipv6)",
newMinimalKubeRouterConfig("10.96.0.0/12,2001:db8:42:0::/56,2001:db8:42:1::/112", "", "node", nil),
newMinimalKubeRouterConfig("10.96.0.0/12,2001:db8:42:0::/56,2001:db8:42:1::/112", "", "node", nil, false),
true,
"too many CIDRs provided in --service-cluster-ip-range parameter, only two addresses are allowed at once for dual-stack",
"too many CIDRs provided in --service-cluster-ip-range parameter: dual-stack must be enabled to provide two addresses",
},
{
"Test good cluster CIDR (using single IP with a /32)",
newMinimalKubeRouterConfig("10.10.10.10/32", "", "node", nil),
newMinimalKubeRouterConfig("10.10.10.10/32", "", "node", nil, false),
false,
"",
},
{
"Test good cluster CIDR (using normal range with /24)",
newMinimalKubeRouterConfig("10.10.10.0/24", "", "node", nil),
newMinimalKubeRouterConfig("10.10.10.0/24", "", "node", nil, false),
false,
"",
},
{
"Test good cluster CIDR (using ipv6)",
newMinimalKubeRouterConfig("2001:db8:42:1::/112", "", "node", nil),
newMinimalKubeRouterConfig("2001:db8:42:1::/112", "", "node", []string{"2001:db8:42:1::/112"}, true),
false,
"",
},
{
"Test good cluster CIDRs (with dual-stack)",
newMinimalKubeRouterConfig("10.96.0.0/12,2001:db8:42:1::/112", "", "node", nil),
newMinimalKubeRouterConfig("10.96.0.0/12,2001:db8:42:1::/112", "", "node", []string{"10.96.0.0/12", "2001:db8:42:1::/112"}, true),
false,
"",
},
{
"Test bad node port specification (using commas)",
newMinimalKubeRouterConfig("", "8080,8081", "node", nil),
newMinimalKubeRouterConfig("", "8080,8081", "node", nil, false),
true,
"failed to parse node port range given: '8080,8081' please see specification in help text",
},
{
"Test bad node port specification (not using numbers)",
newMinimalKubeRouterConfig("", "foo:bar", "node", nil),
newMinimalKubeRouterConfig("", "foo:bar", "node", nil, false),
true,
"failed to parse node port range given: 'foo:bar' please see specification in help text",
},
{
"Test bad node port specification (using anything in addition to range)",
newMinimalKubeRouterConfig("", "8080,8081-8090", "node", nil),
newMinimalKubeRouterConfig("", "8080,8081-8090", "node", nil, false),
true,
"failed to parse node port range given: '8080,8081-8090' please see specification in help text",
},
{
"Test bad node port specification (using reversed range)",
newMinimalKubeRouterConfig("", "8090-8080", "node", nil),
newMinimalKubeRouterConfig("", "8090-8080", "node", nil, false),
true,
"port 1 is greater than or equal to port 2 in range given: '8090-8080'",
},
{
"Test bad node port specification (port out of available range)",
newMinimalKubeRouterConfig("", "132000-132001", "node", nil),
newMinimalKubeRouterConfig("", "132000-132001", "node", nil, false),
true,
"could not parse first port number from range given: '132000-132001'",
},
{
"Test good node port specification (using colon separator)",
newMinimalKubeRouterConfig("", "8080:8090", "node", nil),
newMinimalKubeRouterConfig("", "8080:8090", "node", nil, false),
false,
"",
},
{
"Test good node port specification (using hyphen separator)",
newMinimalKubeRouterConfig("", "8080-8090", "node", nil),
newMinimalKubeRouterConfig("", "8080-8090", "node", nil, false),
false,
"",
},
{
"Test bad external IP CIDR (not properly formatting ip address)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10"}),
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10"}, false),
true,
"failed to get parse --service-external-ip-range parameter: '199.10.10'. Error: invalid CIDR address: 199.10.10",
},
{
"Test bad external IP CIDR (not using an ip address)",
newMinimalKubeRouterConfig("", "", "node", []string{"foo"}),
newMinimalKubeRouterConfig("", "", "node", []string{"foo"}, false),
true,
"failed to get parse --service-external-ip-range parameter: 'foo'. Error: invalid CIDR address: foo",
},
{
"Test bad external IP CIDR (using an ip address that is not a CIDR)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10"}),
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10"}, false),
true,
"failed to get parse --service-external-ip-range parameter: '199.10.10.10'. Error: invalid CIDR address: 199.10.10.10",
},
{
"Test bad external IP CIDR (making sure that it processes all items in the list)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/32", "199.10.10.11"}),
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/32", "199.10.10.11"}, false),
true,
"failed to get parse --service-external-ip-range parameter: '199.10.10.11'. Error: invalid CIDR address: 199.10.10.11",
},
{
"Test good external IP CIDR (using single IP with a /32)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/32"}),
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/32"}, false),
false,
"",
},
{
"Test good external IP CIDR (using normal range with /24)",
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/24"}),
newMinimalKubeRouterConfig("", "", "node", []string{"199.10.10.10/24"}, false),
false,
"",
},
}
client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*newFakeNode("node", "10.10.10.10")}})
client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*newFakeNode("node", []string{"10.10.10.10", "2001:0db8:0042:0001:0000:0000:0000:0000"})}})
_, podInformer, nsInformer, netpolInformer := newFakeInformersFromClient(client)
for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion pkg/options/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ func (s *KubeRouterConfig) AddFlags(fs *pflag.FlagSet) {
"Path to CRI compatible container runtime socket (used for DSR mode). Currently known working with "+
"containerd.")
fs.StringVar(&s.ClusterIPCIDR, "service-cluster-ip-range", s.ClusterIPCIDR,
"CIDR value from which service cluster IPs are assigned. Default: 10.96.0.0/12")
"CIDR value from which service cluster IPs are assigned. "+
"If dual-stack is used, this can be a comma-separated list of CIDR value. Default: 10.96.0.0/12")
fs.StringSliceVar(&s.ExternalIPCIDRs, "service-external-ip-range", s.ExternalIPCIDRs,
"Specify external IP CIDRs that are used for inter-cluster communication "+
"(can be specified multiple times)")
Expand Down

0 comments on commit 1433bee

Please sign in to comment.