Merge pull request #90391 from johscheuer/improve-error-message-svc-cidr

Improve the error message for the service cidr check
This commit is contained in:
Kubernetes Prow Robot 2020-05-18 11:05:37 -07:00 committed by GitHub
commit 7dafbe3ff3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 97 additions and 9 deletions

View File

@ -35,6 +35,8 @@ import (
// validateClusterIPFlags is expected to be called after Complete()
func validateClusterIPFlags(options *ServerRunOptions) []error {
var errs []error
// maxCIDRBits is used to define the maximum CIDR size for the cluster ip(s)
const maxCIDRBits = 20
// validate that primary has been processed by user provided values or it has been defaulted
if options.PrimaryServiceClusterIPRange.IP == nil {
@ -48,9 +50,8 @@ func validateClusterIPFlags(options *ServerRunOptions) []error {
// Complete() expected to have set Primary* and Secondary*
// primary CIDR validation
var ones, bits = options.PrimaryServiceClusterIPRange.Mask.Size()
if bits-ones > 20 {
errs = append(errs, errors.New("specified --service-cluster-ip-range is too large"))
if err := validateMaxCIDRRange(options.PrimaryServiceClusterIPRange, maxCIDRBits, "--service-cluster-ip-range"); err != nil {
errs = append(errs, err)
}
// Secondary IP validation
@ -74,18 +75,26 @@ func validateClusterIPFlags(options *ServerRunOptions) []error {
errs = append(errs, errors.New("--service-cluster-ip-range and --secondary-service-cluster-ip-range must be of different IP family"))
}
// Should be smallish sized cidr, this thing is kept in etcd
// bigger cidr (specially those offered by IPv6) will add no value
// significantly increase snapshotting time.
var ones, bits = options.SecondaryServiceClusterIPRange.Mask.Size()
if bits-ones > 20 {
errs = append(errs, errors.New("specified --secondary-service-cluster-ip-range is too large"))
if err := validateMaxCIDRRange(options.SecondaryServiceClusterIPRange, maxCIDRBits, "--secondary-service-cluster-ip-range"); err != nil {
errs = append(errs, err)
}
}
return errs
}
func validateMaxCIDRRange(cidr net.IPNet, maxCIDRBits int, cidrFlag string) error {
// Should be smallish sized cidr, this thing is kept in etcd
// bigger cidr (specially those offered by IPv6) will add no value
// significantly increase snapshotting time.
var ones, bits = cidr.Mask.Size()
if bits-ones > maxCIDRBits {
return fmt.Errorf("specified %s is too large; for %d-bit addresses, the mask must be >= %d", cidrFlag, bits, bits-maxCIDRBits)
}
return nil
}
func validateServiceNodePort(options *ServerRunOptions) []error {
var errs []error

View File

@ -95,6 +95,18 @@ func TestClusterSerivceIPRange(t *testing.T) {
options: makeOptionsWithCIDRs("10.0.0.0/16", "3000::/108"),
enableDualStack: false,
},
{
name: "service cidr to big",
expectErrors: true,
options: makeOptionsWithCIDRs("10.0.0.0/8", ""),
enableDualStack: true,
},
{
name: "dual-stack secondary cidr to big",
expectErrors: true,
options: makeOptionsWithCIDRs("10.0.0.0/16", "3000::/64"),
enableDualStack: true,
},
/* success cases */
{
name: "valid primary",
@ -130,3 +142,70 @@ func TestClusterSerivceIPRange(t *testing.T) {
})
}
}
func getIPnetFromCIDR(cidr string) *net.IPNet {
_, ipnet, _ := net.ParseCIDR(cidr)
return ipnet
}
func TestValidateMaxCIDRRange(t *testing.T) {
testCases := []struct {
// tc.cidr, tc.maxCIDRBits, tc.cidrFlag) tc.expectedErrorMessage
name string
cidr net.IPNet
maxCIDRBits int
cidrFlag string
expectedErrorMessage string
expectErrors bool
}{
{
name: "valid ipv4 cidr",
cidr: *getIPnetFromCIDR("10.92.0.0/12"),
maxCIDRBits: 20,
cidrFlag: "--service-cluster-ip-range",
expectedErrorMessage: "",
expectErrors: false,
},
{
name: "valid ipv6 cidr",
cidr: *getIPnetFromCIDR("3000::/108"),
maxCIDRBits: 20,
cidrFlag: "--service-cluster-ip-range",
expectedErrorMessage: "",
expectErrors: false,
},
{
name: "ipv4 cidr to big",
cidr: *getIPnetFromCIDR("10.92.0.0/8"),
maxCIDRBits: 20,
cidrFlag: "--service-cluster-ip-range",
expectedErrorMessage: "specified --service-cluster-ip-range is too large; for 32-bit addresses, the mask must be >= 12",
expectErrors: true,
},
{
name: "ipv6 cidr to big",
cidr: *getIPnetFromCIDR("3000::/64"),
maxCIDRBits: 20,
cidrFlag: "--service-cluster-ip-range",
expectedErrorMessage: "specified --service-cluster-ip-range is too large; for 128-bit addresses, the mask must be >= 108",
expectErrors: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := validateMaxCIDRRange(tc.cidr, tc.maxCIDRBits, tc.cidrFlag)
if err != nil && !tc.expectErrors {
t.Errorf("expected no errors, error found %+v", err)
}
if err == nil && tc.expectErrors {
t.Errorf("expected errors, no errors found")
}
if err != nil && tc.expectErrors && err.Error() != tc.expectedErrorMessage {
t.Errorf("Expected error message: \"%s\"\nGot: \"%s\"", tc.expectedErrorMessage, err.Error())
}
})
}
}