From a036577e2c54405e0ddc368271bb4642b7f362b9 Mon Sep 17 00:00:00 2001 From: "Christopher M. Luciano" Date: Fri, 18 Sep 2020 16:55:38 -0400 Subject: [PATCH] proxy: Restructure config validation tests to check errors The tests for most functions have also been revised to check the errors explicitly upon validating. This will properly catch occasions where we should be returning multiple errors if more error occurs or if just one block is failing. Signed-off-by: Christopher M. Luciano --- .../apis/config/validation/validation_test.go | 549 ++++++++---------- 1 file changed, 245 insertions(+), 304 deletions(-) diff --git a/pkg/proxy/apis/config/validation/validation_test.go b/pkg/proxy/apis/config/validation/validation_test.go index 3e3eda83ca8..cc301bf2642 100644 --- a/pkg/proxy/apis/config/validation/validation_test.go +++ b/pkg/proxy/apis/config/validation/validation_test.go @@ -17,9 +17,7 @@ limitations under the License. package validation import ( - "fmt" "runtime" - "strings" "testing" "time" @@ -188,13 +186,13 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { } } - errorCases := []struct { - config kubeproxyconfig.KubeProxyConfiguration - msg string + newPath := field.NewPath("KubeProxyConfiguration") + testCases := map[string]struct { + config kubeproxyconfig.KubeProxyConfiguration + expectedErrs field.ErrorList }{ - { + "invalid BindAddress": { config: kubeproxyconfig.KubeProxyConfiguration{ - // only BindAddress is invalid BindAddress: "10.10.12.11:2000", HealthzBindAddress: "0.0.0.0:10256", MetricsBindAddress: "127.0.0.1:10249", @@ -213,12 +211,11 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "not a valid textual representation of an IP address", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("BindAddress"), "10.10.12.11:2000", "not a valid textual representation of an IP address")}, }, - { + "invalid HealthzBindAddress": { config: kubeproxyconfig.KubeProxyConfiguration{ - BindAddress: "10.10.12.11", - // only HealthzBindAddress is invalid + BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0", MetricsBindAddress: "127.0.0.1:10249", ClusterCIDR: "192.168.59.0/24", @@ -236,13 +233,12 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "must be IP:port", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("HealthzBindAddress"), "0.0.0.0", "must be IP:port")}, }, - { + "invalid MetricsBindAddress": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0:12345", - // only MetricsBindAddress is invalid MetricsBindAddress: "127.0.0.1", ClusterCIDR: "192.168.59.0/24", UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, @@ -259,17 +255,16 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "must be IP:port", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("MetricsBindAddress"), "127.0.0.1", "must be IP:port")}, }, - { + "ClusterCIDR missing subset range": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0:12345", MetricsBindAddress: "127.0.0.1:10249", - // only ClusterCIDR is invalid - ClusterCIDR: "192.168.59.0", - UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, - ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + ClusterCIDR: "192.168.59.0", + UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ MasqueradeAll: true, SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, @@ -282,9 +277,9 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "must be a valid CIDR block (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterCIDR"), "192.168.59.0", "must be a valid CIDR block (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)")}, }, - { + "Two ClusterCIDR addresses provided without DualStack feature-enabled": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0:12345", @@ -306,9 +301,9 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "only one CIDR allowed (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterCIDR"), "192.168.59.0/24,fd00:192:168::/64", "only one CIDR allowed (e.g. 10.100.0.0/16 or fde4:8dba:82e1::/48)")}, }, - { + "DualStack feature-enabled but EndpointSlice feature disabled": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0:12345", @@ -330,58 +325,9 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "EndpointSlice feature flag must be turned on", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("FeatureGates"), map[string]bool{"EndpointSlice": false, "IPv6DualStack": true}, "EndpointSlice feature flag must be turned on when turning on DualStack")}, }, - - { - config: kubeproxyconfig.KubeProxyConfiguration{ - BindAddress: "10.10.12.11", - HealthzBindAddress: "0.0.0.0:12345", - MetricsBindAddress: "127.0.0.1:10249", - // DualStack with multiple CIDRs but only one IP family - FeatureGates: map[string]bool{"IPv6DualStack": true, "EndpointSlice": true}, - ClusterCIDR: "192.168.59.0/24,10.0.0.0/16", - UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, - ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, - IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ - MasqueradeAll: true, - SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, - MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, - }, - Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ - MaxPerCore: pointer.Int32Ptr(1), - Min: pointer.Int32Ptr(1), - TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, - TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, - }, - }, - msg: "must be a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)", - }, - { - config: kubeproxyconfig.KubeProxyConfiguration{ - BindAddress: "10.10.12.11", - HealthzBindAddress: "0.0.0.0:12345", - MetricsBindAddress: "127.0.0.1:10249", - // DualStack with an invalid subnet - FeatureGates: map[string]bool{"IPv6DualStack": true, "EndpointSlice": true}, - ClusterCIDR: "192.168.59.0/24,fd00:192:168::/64,a.b.c.d/f", - UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, - ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, - IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ - MasqueradeAll: true, - SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, - MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, - }, - Conntrack: kubeproxyconfig.KubeProxyConntrackConfiguration{ - MaxPerCore: pointer.Int32Ptr(1), - Min: pointer.Int32Ptr(1), - TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, - TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, - }, - }, - msg: "only one CIDR allowed or a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)", - }, - { + "Invalid number of ClusterCIDRs": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0:12345", @@ -402,17 +348,16 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "only one CIDR allowed or a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterCIDR"), "192.168.59.0/24,fd00:192:168::/64,10.0.0.0/16", "only one CIDR allowed or a valid DualStack CIDR (e.g. 10.100.0.0/16,fde4:8dba:82e1::/48)")}, }, - { + "UDPIdleTimeout must be > 0": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0:12345", MetricsBindAddress: "127.0.0.1:10249", ClusterCIDR: "192.168.59.0/24", - // only UDPIdleTimeout is invalid - UDPIdleTimeout: metav1.Duration{Duration: -1 * time.Second}, - ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, + UDPIdleTimeout: metav1.Duration{Duration: -1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: 1 * time.Second}, IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ MasqueradeAll: true, SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, @@ -425,17 +370,16 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "must be greater than 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("UDPIdleTimeout"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than 0")}, }, - { + "ConfigSyncPeriod must be > 0": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "10.10.12.11", HealthzBindAddress: "0.0.0.0:12345", MetricsBindAddress: "127.0.0.1:10249", ClusterCIDR: "192.168.59.0/24", UDPIdleTimeout: metav1.Duration{Duration: 1 * time.Second}, - // only ConfigSyncPeriod is invalid - ConfigSyncPeriod: metav1.Duration{Duration: -1 * time.Second}, + ConfigSyncPeriod: metav1.Duration{Duration: -1 * time.Second}, IPTables: kubeproxyconfig.KubeProxyIPTablesConfiguration{ MasqueradeAll: true, SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, @@ -448,9 +392,9 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "must be greater than 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ConfigSyncPeriod"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than 0")}, }, - { + "IPVS mode selected without providing required SyncPeriod": { config: kubeproxyconfig.KubeProxyConfiguration{ BindAddress: "192.168.59.103", HealthzBindAddress: "0.0.0.0:10256", @@ -472,295 +416,276 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, }, - msg: "must be greater than 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeProxyIPVSConfiguration.SyncPeriod"), metav1.Duration{Duration: 0}, "must be greater than 0")}, }, } - for _, errorCase := range errorCases { - if errs := Validate(&errorCase.config); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + for _, testCase := range testCases { + errs := Validate(&testCase.config) + if len(testCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Fatalf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error()) + } } } } func TestValidateKubeProxyIPTablesConfiguration(t *testing.T) { - valid := int32(5) - successCases := []kubeproxyconfig.KubeProxyIPTablesConfiguration{ - { - MasqueradeAll: true, - SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, - MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, - }, - { - MasqueradeBit: &valid, - MasqueradeAll: true, - SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, - MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, - }, - } newPath := field.NewPath("KubeProxyConfiguration") - for _, successCase := range successCases { - if errs := validateKubeProxyIPTablesConfiguration(successCase, newPath.Child("KubeProxyIPTablesConfiguration")); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } - } - invalid := int32(-10) - errorCases := []struct { - config kubeproxyconfig.KubeProxyIPTablesConfiguration - msg string + testCases := map[string]struct { + config kubeproxyconfig.KubeProxyIPTablesConfiguration + expectedErrs field.ErrorList }{ - { + "valid iptables config": { + config: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + expectedErrs: field.ErrorList{}, + }, + "valid custom MasqueradeBit": { + config: kubeproxyconfig.KubeProxyIPTablesConfiguration{ + MasqueradeBit: pointer.Int32Ptr(5), + MasqueradeAll: true, + SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, + }, + expectedErrs: field.ErrorList{}, + }, + "SyncPeriod must be > 0": { config: kubeproxyconfig.KubeProxyIPTablesConfiguration{ MasqueradeAll: true, SyncPeriod: metav1.Duration{Duration: -5 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, }, - msg: "must be greater than 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPTablesConfiguration.SyncPeriod"), metav1.Duration{Duration: -5 * time.Second}, "must be greater than 0"), + field.Invalid(newPath.Child("KubeIPTablesConfiguration.SyncPeriod"), metav1.Duration{Duration: 2 * time.Second}, "must be greater than or equal to KubeProxyConfiguration.KubeIPTablesConfiguration.MinSyncPeriod")}, }, - { + "MinSyncPeriod must be > 0": { config: kubeproxyconfig.KubeProxyIPTablesConfiguration{ - MasqueradeBit: &valid, + MasqueradeBit: pointer.Int32Ptr(5), MasqueradeAll: true, SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: -1 * time.Second}, }, - msg: "must be greater than or equal to 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPTablesConfiguration.MinSyncPeriod"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than or equal to 0")}, }, - { + "MasqueradeBit cannot be < 0": { config: kubeproxyconfig.KubeProxyIPTablesConfiguration{ - MasqueradeBit: &invalid, + MasqueradeBit: pointer.Int32Ptr(-10), MasqueradeAll: true, SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, }, - msg: "must be within the range [0, 31]", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPTablesConfiguration.MasqueradeBit"), -10, "must be within the range [0, 31]")}, }, - // SyncPeriod must be >= MinSyncPeriod - { + "SyncPeriod must be >= MinSyncPeriod": { config: kubeproxyconfig.KubeProxyIPTablesConfiguration{ - MasqueradeBit: &valid, + MasqueradeBit: pointer.Int32Ptr(5), MasqueradeAll: true, SyncPeriod: metav1.Duration{Duration: 1 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 5 * time.Second}, }, - msg: fmt.Sprintf("must be greater than or equal to %s", newPath.Child("KubeProxyIPTablesConfiguration").Child("MinSyncPeriod").String()), + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPTablesConfiguration.SyncPeriod"), metav1.Duration{Duration: 5 * time.Second}, "must be greater than or equal to KubeProxyConfiguration.KubeIPTablesConfiguration.MinSyncPeriod")}, }, } - for _, errorCase := range errorCases { - if errs := validateKubeProxyIPTablesConfiguration(errorCase.config, newPath.Child("KubeProxyIPTablesConfiguration")); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + for _, testCase := range testCases { + errs := validateKubeProxyIPTablesConfiguration(testCase.config, newPath.Child("KubeIPTablesConfiguration")) + if len(testCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Errorf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error()) + } } } } func TestValidateKubeProxyIPVSConfiguration(t *testing.T) { newPath := field.NewPath("KubeProxyConfiguration") - testCases := []struct { - config kubeproxyconfig.KubeProxyIPVSConfiguration - expectErr bool - reason string + testCases := map[string]struct { + config kubeproxyconfig.KubeProxyIPVSConfiguration + expectedErrs field.ErrorList }{ - { + "SyncPeriod is not greater than 0": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: -5 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 2 * time.Second}, }, - expectErr: true, - reason: "SyncPeriod must be greater than 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPVSConfiguration.SyncPeriod"), metav1.Duration{Duration: -5 * time.Second}, "must be greater than 0"), + field.Invalid(newPath.Child("KubeIPVSConfiguration.SyncPeriod"), metav1.Duration{Duration: 2 * time.Second}, "must be greater than or equal to KubeProxyConfiguration.KubeIPVSConfiguration.MinSyncPeriod")}, }, - { + "SyncPeriod cannot be 0": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 0 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 10 * time.Second}, }, - expectErr: true, - reason: "SyncPeriod must be greater than 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPVSConfiguration.SyncPeriod"), metav1.Duration{Duration: 0}, "must be greater than 0"), + field.Invalid(newPath.Child("KubeIPVSConfiguration.SyncPeriod"), metav1.Duration{Duration: 10 * time.Second}, "must be greater than or equal to KubeProxyConfiguration.KubeIPVSConfiguration.MinSyncPeriod")}, }, - { + "MinSyncPeriod cannot be less than 0": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: -1 * time.Second}, }, - expectErr: true, - reason: "MinSyncPeriod must be greater than or equal to 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPVSConfiguration.MinSyncPeriod"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than or equal to 0")}, }, - { + "SyncPeriod must be greater than MinSyncPeriod": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 1 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 5 * time.Second}, }, - expectErr: true, - reason: "SyncPeriod must be greater than or equal to MinSyncPeriod", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPVSConfiguration.SyncPeriod"), metav1.Duration{Duration: 5 * time.Second}, "must be greater than or equal to KubeProxyConfiguration.KubeIPVSConfiguration.MinSyncPeriod")}, }, - // SyncPeriod == MinSyncPeriod - { + "SyncPeriod == MinSyncPeriod": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 10 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 10 * time.Second}, }, - expectErr: false, + expectedErrs: field.ErrorList{}, }, - // SyncPeriod > MinSyncPeriod - { + "SyncPeriod should be > MinSyncPeriod": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 10 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 5 * time.Second}, }, - expectErr: false, + expectedErrs: field.ErrorList{}, }, - // SyncPeriod can be 0 - { + "MinSyncPeriod can be 0": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, MinSyncPeriod: metav1.Duration{Duration: 0 * time.Second}, }, - expectErr: false, + expectedErrs: field.ErrorList{}, }, - // IPVS Timeout can be 0 - { + "IPVS Timeout can be 0": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, TCPTimeout: metav1.Duration{Duration: 0 * time.Second}, TCPFinTimeout: metav1.Duration{Duration: 0 * time.Second}, UDPTimeout: metav1.Duration{Duration: 0 * time.Second}, }, - expectErr: false, + expectedErrs: field.ErrorList{}, }, - // IPVS Timeout > 0 - { + "IPVS Timeout > 0": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, TCPTimeout: metav1.Duration{Duration: 1 * time.Second}, TCPFinTimeout: metav1.Duration{Duration: 2 * time.Second}, UDPTimeout: metav1.Duration{Duration: 3 * time.Second}, }, - expectErr: false, + expectedErrs: field.ErrorList{}, }, - // TCPTimeout Timeout < 0 - { - config: kubeproxyconfig.KubeProxyIPVSConfiguration{ - SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, - TCPTimeout: metav1.Duration{Duration: -1 * time.Second}, - }, - expectErr: true, - reason: "TCPTimeout must be greater than or equal to 0", - }, - // TCPFinTimeout Timeout < 0 - { + "TCP,TCPFin,UDP Timeouts < 0": { config: kubeproxyconfig.KubeProxyIPVSConfiguration{ SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + TCPTimeout: metav1.Duration{Duration: -1 * time.Second}, + UDPTimeout: metav1.Duration{Duration: -1 * time.Second}, TCPFinTimeout: metav1.Duration{Duration: -1 * time.Second}, }, - expectErr: true, - reason: "TCPFinTimeout must be greater than or equal to 0", - }, - // UDPTimeout Timeout < 0 - { - config: kubeproxyconfig.KubeProxyIPVSConfiguration{ - SyncPeriod: metav1.Duration{Duration: 5 * time.Second}, - UDPTimeout: metav1.Duration{Duration: -1 * time.Second}, - }, - expectErr: true, - reason: "UDPTimeout must be greater than or equal to 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeIPVSConfiguration.TCPTimeout"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than or equal to 0"), + field.Invalid(newPath.Child("KubeIPVSConfiguration.TCPFinTimeout"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than or equal to 0"), + field.Invalid(newPath.Child("KubeIPVSConfiguration.UDPTimeout"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than or equal to 0")}, }, } - - for _, test := range testCases { - errs := validateKubeProxyIPVSConfiguration(test.config, newPath.Child("KubeProxyIPVSConfiguration")) - if len(errs) == 0 && test.expectErr { - t.Errorf("Expect error, got nil, reason: %s", test.reason) + for _, testCase := range testCases { + errs := validateKubeProxyIPVSConfiguration(testCase.config, newPath.Child("KubeIPVSConfiguration")) + if len(testCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) } - if len(errs) > 0 && !test.expectErr { - t.Errorf("Unexpected error: %v", errs) + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Errorf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error()) + } } } } func TestValidateKubeProxyConntrackConfiguration(t *testing.T) { - successCases := []kubeproxyconfig.KubeProxyConntrackConfiguration{ - { - MaxPerCore: pointer.Int32Ptr(1), - Min: pointer.Int32Ptr(1), - TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, - TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, - }, - { - MaxPerCore: pointer.Int32Ptr(0), - Min: pointer.Int32Ptr(0), - TCPEstablishedTimeout: &metav1.Duration{Duration: 0 * time.Second}, - TCPCloseWaitTimeout: &metav1.Duration{Duration: 0 * time.Second}, - }, - } newPath := field.NewPath("KubeProxyConfiguration") - for _, successCase := range successCases { - if errs := validateKubeProxyConntrackConfiguration(successCase, newPath.Child("KubeProxyConntrackConfiguration")); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } - } - - errorCases := []struct { - config kubeproxyconfig.KubeProxyConntrackConfiguration - msg string + testCases := map[string]struct { + config kubeproxyconfig.KubeProxyConntrackConfiguration + expectedErrs field.ErrorList }{ - { + "valid 5 second timeouts": { + config: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, + }, + expectedErrs: field.ErrorList{}, + }, + "valid duration equal to 0 second timeout": { + config: kubeproxyconfig.KubeProxyConntrackConfiguration{ + MaxPerCore: pointer.Int32Ptr(1), + Min: pointer.Int32Ptr(1), + TCPEstablishedTimeout: &metav1.Duration{Duration: 0 * time.Second}, + TCPCloseWaitTimeout: &metav1.Duration{Duration: 0 * time.Second}, + }, + expectedErrs: field.ErrorList{}, + }, + "invalid MaxPerCore < 0": { config: kubeproxyconfig.KubeProxyConntrackConfiguration{ MaxPerCore: pointer.Int32Ptr(-1), Min: pointer.Int32Ptr(1), TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, - msg: "must be greater than or equal to 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeConntrackConfiguration.MaxPerCore"), -1, "must be greater than or equal to 0")}, }, - { + "invalid minimum < 0": { config: kubeproxyconfig.KubeProxyConntrackConfiguration{ MaxPerCore: pointer.Int32Ptr(1), Min: pointer.Int32Ptr(-1), TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, - msg: "must be greater than or equal to 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeConntrackConfiguration.Min"), -1, "must be greater than or equal to 0")}, }, - { + "invalid EstablishedTimeout < 0": { config: kubeproxyconfig.KubeProxyConntrackConfiguration{ MaxPerCore: pointer.Int32Ptr(1), - Min: pointer.Int32Ptr(3), + Min: pointer.Int32Ptr(1), TCPEstablishedTimeout: &metav1.Duration{Duration: -5 * time.Second}, TCPCloseWaitTimeout: &metav1.Duration{Duration: 5 * time.Second}, }, - msg: "must be greater than or equal to 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeConntrackConfiguration.TCPEstablishedTimeout"), metav1.Duration{Duration: -5 * time.Second}, "must be greater than or equal to 0")}, }, - { + "invalid CloseWaitTimeout < 0": { config: kubeproxyconfig.KubeProxyConntrackConfiguration{ MaxPerCore: pointer.Int32Ptr(1), - Min: pointer.Int32Ptr(3), + Min: pointer.Int32Ptr(1), TCPEstablishedTimeout: &metav1.Duration{Duration: 5 * time.Second}, TCPCloseWaitTimeout: &metav1.Duration{Duration: -5 * time.Second}, }, - msg: "must be greater than or equal to 0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("KubeConntrackConfiguration.TCPCloseWaitTimeout"), metav1.Duration{Duration: -5 * time.Second}, "must be greater than or equal to 0")}, }, } - for _, errorCase := range errorCases { - if errs := validateKubeProxyConntrackConfiguration(errorCase.config, newPath.Child("KubeProxyConntrackConfiguration")); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + for _, testCase := range testCases { + errs := validateKubeProxyConntrackConfiguration(testCase.config, newPath.Child("KubeConntrackConfiguration")) + if len(testCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Errorf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error()) + } } } } func TestValidateProxyMode(t *testing.T) { newPath := field.NewPath("KubeProxyConfiguration") - successCases := []kubeproxyconfig.ProxyMode{ - kubeproxyconfig.ProxyModeUserspace, - kubeproxyconfig.ProxyMode(""), - } + successCases := []kubeproxyconfig.ProxyMode{""} if runtime.GOOS == "windows" { successCases = append(successCases, kubeproxyconfig.ProxyModeKernelspace) @@ -774,21 +699,32 @@ func TestValidateProxyMode(t *testing.T) { } } - errorCases := []struct { - mode kubeproxyconfig.ProxyMode - msg string + testCases := map[string]struct { + mode kubeproxyconfig.ProxyMode + expectedErrs field.ErrorList }{ - { - mode: kubeproxyconfig.ProxyMode("non-existing"), - msg: "or blank (blank means the", + "valid Userspace mode": { + mode: kubeproxyconfig.ProxyModeUserspace, + expectedErrs: field.ErrorList{}, + }, + "blank mode should default": { + mode: kubeproxyconfig.ProxyMode(""), + expectedErrs: field.ErrorList{}, + }, + "invalid mode non-existent": { + mode: kubeproxyconfig.ProxyMode("non-existing"), + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ProxyMode"), "non-existing", "must be iptables,ipvs,userspace or blank (blank means the best-available proxy [currently iptables])")}, }, } - - for _, errorCase := range errorCases { - if errs := validateProxyMode(errorCase.mode, newPath.Child("ProxyMode")); len(errs) == 0 { - t.Errorf("expected failure %s for %v", errorCase.msg, errorCase.mode) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + for _, testCase := range testCases { + errs := validateProxyMode(testCase.mode, newPath) + if len(testCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Errorf("Expected error: %s, got %v", testCase.expectedErrs[i], err.Error()) + } } } } @@ -796,36 +732,33 @@ func TestValidateProxyMode(t *testing.T) { func TestValidateClientConnectionConfiguration(t *testing.T) { newPath := field.NewPath("KubeProxyConfiguration") - successCases := []componentbaseconfig.ClientConnectionConfiguration{ - { - Burst: 0, - }, - { - Burst: 5, - }, - } - - for _, successCase := range successCases { - if errs := validateClientConnectionConfiguration(successCase, newPath.Child("Burst")); len(errs) != 0 { - t.Errorf("expected success: %v", errs) - } - } - - errorCases := []struct { - ccc componentbaseconfig.ClientConnectionConfiguration - msg string + testCases := map[string]struct { + ccc componentbaseconfig.ClientConnectionConfiguration + expectedErrs field.ErrorList }{ - { - ccc: componentbaseconfig.ClientConnectionConfiguration{Burst: -5}, - msg: "must be greater than or equal to 0", + "successful 0 value": { + ccc: componentbaseconfig.ClientConnectionConfiguration{Burst: 0}, + expectedErrs: field.ErrorList{}, + }, + "successful 5 value": { + ccc: componentbaseconfig.ClientConnectionConfiguration{Burst: 5}, + expectedErrs: field.ErrorList{}, + }, + "burst < 0": { + ccc: componentbaseconfig.ClientConnectionConfiguration{Burst: -5}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("Burst"), -5, "must be greater than or equal to 0")}, }, } - for _, errorCase := range errorCases { - if errs := validateClientConnectionConfiguration(errorCase.ccc, newPath.Child("Burst")); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + for _, testCase := range testCases { + errs := validateClientConnectionConfiguration(testCase.ccc, newPath) + if len(testCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(testCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != testCase.expectedErrs[i].Error() { + t.Errorf("Expected error: %s, got %s", testCase.expectedErrs[i], err.Error()) + } } } } @@ -845,37 +778,41 @@ func TestValidateHostPort(t *testing.T) { } } - errorCases := []struct { - ccc string - msg string + errorCases := map[string]struct { + ip string + expectedErrs field.ErrorList }{ - { - ccc: "10.10.10.10", - msg: "must be IP:port", + "missing port": { + ip: "10.10.10.10", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("HealthzBindAddress"), "10.10.10.10", "must be IP:port")}, }, - { - ccc: "123.456.789.10:12345", - msg: "must be a valid IP", + "digits outside of 1-255": { + ip: "123.456.789.10:12345", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("HealthzBindAddress"), "123.456.789.10", "must be a valid IP")}, }, - { - ccc: "10.10.10.10:foo", - msg: "must be a valid port", + "invalid named-port": { + ip: "10.10.10.10:foo", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("HealthzBindAddress"), "foo", "must be a valid port")}, }, - { - ccc: "10.10.10.10:0", - msg: "must be a valid port", + "port cannot be 0": { + ip: "10.10.10.10:0", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("HealthzBindAddress"), "0", "must be a valid port")}, }, - { - ccc: "10.10.10.10:65536", - msg: "must be a valid port", + "port is greater than allowed range": { + ip: "10.10.10.10:65536", + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("HealthzBindAddress"), "65536", "must be a valid port")}, }, } for _, errorCase := range errorCases { - if errs := validateHostPort(errorCase.ccc, newPath.Child("HealthzBindAddress")); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + errs := validateHostPort(errorCase.ip, newPath.Child("HealthzBindAddress")) + if len(errorCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(errorCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != errorCase.expectedErrs[i].Error() { + t.Errorf("Expected error: %s, got %s", errorCase.expectedErrs[i], err.Error()) + } } } } @@ -903,21 +840,25 @@ func TestValidateIPVSSchedulerMethod(t *testing.T) { } } - errorCases := []struct { - mode kubeproxyconfig.IPVSSchedulerMethod - msg string + errorCases := map[string]struct { + mode kubeproxyconfig.IPVSSchedulerMethod + expectedErrs field.ErrorList }{ - { - mode: kubeproxyconfig.IPVSSchedulerMethod("non-existing"), - msg: "blank means the default algorithm method (currently rr)", + "non-existent scheduler method": { + mode: kubeproxyconfig.IPVSSchedulerMethod("non-existing"), + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ProxyMode.Scheduler"), "non-existing", "must be in [rr wrr lc wlc lblc lblcr sh dh sed nq ], blank means the default algorithm method (currently rr)")}, }, } for _, errorCase := range errorCases { - if errs := validateIPVSSchedulerMethod(errorCase.mode, newPath.Child("ProxyMode")); len(errs) == 0 { - t.Errorf("expected failure for %s", errorCase.msg) - } else if !strings.Contains(errs[0].Error(), errorCase.msg) { - t.Errorf("unexpected error: %v, expected: %s", errs[0], errorCase.msg) + errs := validateIPVSSchedulerMethod(errorCase.mode, newPath.Child("ProxyMode")) + if len(errorCase.expectedErrs) != len(errs) { + t.Fatalf("Expected %d errors, got %d errors: %v", len(errorCase.expectedErrs), len(errs), errs) + } + for i, err := range errs { + if err.Error() != errorCase.expectedErrs[i].Error() { + t.Fatalf("Expected error: %s, got %s", errorCase.expectedErrs[i], err.Error()) + } } } }