Add --subnet validation

[spowelljr]

If the IP/CIDR passed in is not a private IP it should fail during validation as the way the incrementing and retries work it's impossible to start with a public IP and for it to increment into a private IP. Also, we should require that if a CIDR is passed in that the mask is no greater than /30. A mask of /30 gives a range of 4 IPs, anything larger reduces that number and is guaranteed to fail as minikube requires at least 4 IPs.

[shubhbapna]

Hi there! Can I take a shot at this?

[shubhbapna]

/assign

[spowelljr]

Hi @shubhbapna, for sure!

[shubhbapna]

@spowelljr I have been going through the codebase and it seems like we might already have the functions that can perform these checks. I am thinking of exposing two functions from the network package - isSubnetPrivate and inspect (inspect might be an overkill so maybe I can break it down into 2 functions and just expose what is required) and then simply call them inside a validator while parsing the subnet flag in start_flags.go

What do you think? If I am going in a completely different direction pls let me know

[spowelljr]

@shubhbapna Golang's net package has an isPrivate func that was added in go 1.17. isSubnetPrivate should be deleted and all references should be replaced with this new function. This will simplify the private check in the validation.

I don't think that importing the inspect func is necessary. The only parts we should need from it are:

	ip, network, err := net.ParseCIDR(addr)
	if err != nil {
		ip = net.ParseIP(addr)
		if ip == nil {
			return nil, fmt.Errorf("failed parsing address %s: %w", addr, err)
		}
	}

And if ParseCIDR is successful just check the mask and make sure's it's less than 30.

[shubhbapna]

Gotcha, thank you! For deleting isSubnetPrivate, should I do that in a separate PR?

[spowelljr]

You can if you'd like, even including it in this PR I'm assuming the PR will be fairly small, but completely up to you

[shubhbapna]

I think I found an issue in network_test while cleaning up isSubnetPrivate. So I think it will be better if I do this clean up in a separate PR

It seems like TestFreeSubnet is not restoring the mocks which has an effect on the subsequent tests. For eg: NonPrivateSubnet test ends up using the mocked inspect from the FirstSubnetIPV6NetworkFound test

What do you think @spowelljr ?

[spowelljr]

Feel free to make two PRs then, you can create an issue for what you discovered and assign yourself