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()) + } } } }