From 1eadfb0e28f1b51d586d645eeb0dc22d0705d567 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 11 Nov 2023 15:28:26 +0100 Subject: [PATCH] opts: ValidateIPAddress: improve error, godoc, and tests - document accepted values - add test-coverage for the function's behavior (including whitespace handling), and use sub-tests. - improve error-message to use uppercase for "IP", and to use a common prefix. Signed-off-by: Sebastiaan van Stijn --- daemon/config/config_test.go | 4 +- opts/opts.go | 9 +++- opts/opts_test.go | 83 +++++++++++++++++++++++++++++------- 3 files changed, 76 insertions(+), 20 deletions(-) diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index 0e927c0023..afb8a1c15c 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -247,7 +247,7 @@ func TestValidateConfigurationErrors(t *testing.T) { }, }, }, - expectedErr: "1.1.1.1o is not an ip address", + expectedErr: "IP address is not correctly formatted: 1.1.1.1o", }, { name: "multiple DNS, invalid IP-address", @@ -258,7 +258,7 @@ func TestValidateConfigurationErrors(t *testing.T) { }, }, }, - expectedErr: "1.1.1.1o is not an ip address", + expectedErr: "IP address is not correctly formatted: 1.1.1.1o", }, { name: "single DNSSearch", diff --git a/opts/opts.go b/opts/opts.go index b431f2a3d7..40b19c4bd9 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -298,13 +298,18 @@ type ValidatorFctType func(val string) (string, error) // ValidatorFctListType defines a validator function that returns a validated list of string and/or an error type ValidatorFctListType func(val string) ([]string, error) -// ValidateIPAddress validates an Ip address. +// ValidateIPAddress validates if the given value is a correctly formatted +// IP address, and returns the value in normalized form. Leading and trailing +// whitespace is allowed, but it does not allow IPv6 addresses surrounded by +// square brackets ("[::1]"). +// +// Refer to [net.ParseIP] for accepted formats. func ValidateIPAddress(val string) (string, error) { ip := net.ParseIP(strings.TrimSpace(val)) if ip != nil { return ip.String(), nil } - return "", fmt.Errorf("%s is not an ip address", val) + return "", fmt.Errorf("IP address is not correctly formatted: %s", val) } // ValidateDNSSearch validates domain for resolvconf search configuration. diff --git a/opts/opts_test.go b/opts/opts_test.go index 49ceea8676..3f7f33a246 100644 --- a/opts/opts_test.go +++ b/opts/opts_test.go @@ -10,24 +10,75 @@ import ( ) func TestValidateIPAddress(t *testing.T) { - if ret, err := ValidateIPAddress(`1.2.3.4`); err != nil || ret == "" { - t.Fatalf("ValidateIPAddress(`1.2.3.4`) got %s %s", ret, err) + tests := []struct { + doc string + input string + expectedOut string + expectedErr string + }{ + { + doc: "IPv4 loopback", + input: `127.0.0.1`, + expectedOut: `127.0.0.1`, + }, + { + doc: "IPv4 loopback with whitespace", + input: ` 127.0.0.1 `, + expectedOut: `127.0.0.1`, + }, + { + doc: "IPv6 loopback long form", + input: `0:0:0:0:0:0:0:1`, + expectedOut: `::1`, + }, + { + doc: "IPv6 loopback", + input: `::1`, + expectedOut: `::1`, + }, + { + doc: "IPv6 loopback with whitespace", + input: ` ::1 `, + expectedOut: `::1`, + }, + { + doc: "IPv6 lowercase", + input: `2001:db8::68`, + expectedOut: `2001:db8::68`, + }, + { + doc: "IPv6 uppercase", + input: `2001:DB8::68`, + expectedOut: `2001:db8::68`, + }, + { + doc: "IPv6 with brackets", + input: `[::1]`, + expectedErr: `IP address is not correctly formatted: [::1]`, + }, + { + doc: "IPv4 partial", + input: `127`, + expectedErr: `IP address is not correctly formatted: 127`, + }, + { + doc: "random invalid string", + input: `random invalid string`, + expectedErr: `IP address is not correctly formatted: random invalid string`, + }, } - if ret, err := ValidateIPAddress(`127.0.0.1`); err != nil || ret == "" { - t.Fatalf("ValidateIPAddress(`127.0.0.1`) got %s %s", ret, err) - } - - if ret, err := ValidateIPAddress(`::1`); err != nil || ret == "" { - t.Fatalf("ValidateIPAddress(`::1`) got %s %s", ret, err) - } - - if ret, err := ValidateIPAddress(`127`); err == nil || ret != "" { - t.Fatalf("ValidateIPAddress(`127`) got %s %s", ret, err) - } - - if ret, err := ValidateIPAddress(`random invalid string`); err == nil || ret != "" { - t.Fatalf("ValidateIPAddress(`random invalid string`) got %s %s", ret, err) + for _, tc := range tests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + actualOut, actualErr := ValidateIPAddress(tc.input) + assert.Check(t, is.Equal(tc.expectedOut, actualOut)) + if tc.expectedErr == "" { + assert.Check(t, actualErr) + } else { + assert.Check(t, is.Error(actualErr, tc.expectedErr)) + } + }) } }