diff --git a/cmd/kube-proxy/app/BUILD b/cmd/kube-proxy/app/BUILD index 081ac969874..a9a79c723d5 100644 --- a/cmd/kube-proxy/app/BUILD +++ b/cmd/kube-proxy/app/BUILD @@ -26,6 +26,7 @@ go_library( "//pkg/proxy/apis:go_default_library", "//pkg/proxy/apis/config:go_default_library", "//pkg/proxy/apis/config/scheme:go_default_library", + "//pkg/proxy/apis/config/v1alpha1:go_default_library", "//pkg/proxy/apis/config/validation:go_default_library", "//pkg/proxy/config:go_default_library", "//pkg/proxy/healthcheck:go_default_library", @@ -46,6 +47,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/selection:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", @@ -67,6 +69,7 @@ go_library( "//staging/src/k8s.io/component-base/version/verflag:go_default_library", "//staging/src/k8s.io/kube-proxy/config/v1alpha1:go_default_library", "//vendor/github.com/fsnotify/fsnotify:go_default_library", + "//vendor/github.com/pkg/errors:go_default_library", "//vendor/github.com/spf13/cobra:go_default_library", "//vendor/github.com/spf13/pflag:go_default_library", "//vendor/k8s.io/klog:go_default_library", @@ -167,7 +170,6 @@ go_test( "//pkg/proxy/apis/config:go_default_library", "//pkg/util/configz:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/component-base/config:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 0d94a88f00f..5e010d3e4e9 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -32,10 +32,12 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" + gerrors "github.com/pkg/errors" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/selection" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -65,6 +67,7 @@ import ( "k8s.io/kubernetes/pkg/proxy/apis" kubeproxyconfig "k8s.io/kubernetes/pkg/proxy/apis/config" proxyconfigscheme "k8s.io/kubernetes/pkg/proxy/apis/config/scheme" + kubeproxyconfigv1alpha1 "k8s.io/kubernetes/pkg/proxy/apis/config/v1alpha1" "k8s.io/kubernetes/pkg/proxy/apis/config/validation" "k8s.io/kubernetes/pkg/proxy/config" "k8s.io/kubernetes/pkg/proxy/healthcheck" @@ -369,6 +372,20 @@ func addressFromDeprecatedFlags(addr string, port int32) string { return proxyutil.AppendPortIfNeeded(addr, port) } +// newLenientSchemeAndCodecs returns a scheme that has only v1alpha1 registered into +// it and a CodecFactory with strict decoding disabled. +func newLenientSchemeAndCodecs() (*runtime.Scheme, *serializer.CodecFactory, error) { + lenientScheme := runtime.NewScheme() + if err := kubeproxyconfig.AddToScheme(lenientScheme); err != nil { + return nil, nil, fmt.Errorf("failed to add kube-proxy config API to lenient scheme: %v", err) + } + if err := kubeproxyconfigv1alpha1.AddToScheme(lenientScheme); err != nil { + return nil, nil, fmt.Errorf("failed to add kube-proxy config v1alpha1 API to lenient scheme: %v", err) + } + lenientCodecs := serializer.NewCodecFactory(lenientScheme, serializer.DisableStrict) + return lenientScheme, &lenientCodecs, nil +} + // loadConfigFromFile loads the contents of file and decodes it as a // KubeProxyConfiguration object. func (o *Options) loadConfigFromFile(file string) (*kubeproxyconfig.KubeProxyConfiguration, error) { @@ -380,12 +397,34 @@ func (o *Options) loadConfigFromFile(file string) (*kubeproxyconfig.KubeProxyCon return o.loadConfig(data) } -// loadConfig decodes data as a KubeProxyConfiguration object. +// loadConfig decodes a serialized KubeProxyConfiguration to the internal type. func (o *Options) loadConfig(data []byte) (*kubeproxyconfig.KubeProxyConfiguration, error) { + configObj, gvk, err := proxyconfigscheme.Codecs.UniversalDecoder().Decode(data, nil, nil) if err != nil { - return nil, err + // Try strict decoding first. If that fails decode with a lenient + // decoder, which has only v1alpha1 registered, and log a warning. + // The lenient path is to be dropped when support for v1alpha1 is dropped. + if !runtime.IsStrictDecodingError(err) { + return nil, gerrors.Wrap(err, "failed to decode") + } + + _, lenientCodecs, lenientErr := newLenientSchemeAndCodecs() + if lenientErr != nil { + return nil, lenientErr + } + + configObj, gvk, lenientErr = lenientCodecs.UniversalDecoder().Decode(data, nil, nil) + if lenientErr != nil { + // Lenient decoding failed with the current version, return the + // original strict error. + return nil, fmt.Errorf("failed lenient decoding: %v", err) + } + + // Continue with the v1alpha1 object that was decoded leniently, but emit a warning. + klog.Warningf("using lenient decoding as strict decoding failed: %v", err) } + proxyConfig, ok := configObj.(*kubeproxyconfig.KubeProxyConfiguration) if !ok { return nil, fmt.Errorf("got unexpected config type: %v", gvk) diff --git a/cmd/kube-proxy/app/server_test.go b/cmd/kube-proxy/app/server_test.go index e32fb23b1ca..5887eabbb36 100644 --- a/cmd/kube-proxy/app/server_test.go +++ b/cmd/kube-proxy/app/server_test.go @@ -33,7 +33,6 @@ import ( utilpointer "k8s.io/utils/pointer" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kuberuntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/diff" componentbaseconfig "k8s.io/component-base/config" kubeproxyconfig "k8s.io/kubernetes/pkg/proxy/apis/config" @@ -161,6 +160,7 @@ nodePortAddresses: clusterCIDR string healthzBindAddress string metricsBindAddress string + extraConfig string }{ { name: "iptables mode, IPv4 all-zeros bind address", @@ -219,6 +219,30 @@ nodePortAddresses: healthzBindAddress: "[fd00:1::5]:12345", metricsBindAddress: "[fd00:2::5]:23456", }, + { + // Test for unknown field within config. + // For v1alpha1 a lenient path is implemented and will throw a + // strict decoding warning instead of failing to load + name: "unknown field", + mode: "iptables", + bindAddress: "9.8.7.6", + clusterCIDR: "1.2.3.0/24", + healthzBindAddress: "1.2.3.4:12345", + metricsBindAddress: "2.3.4.5:23456", + extraConfig: "foo: bar", + }, + { + // Test for duplicate field within config. + // For v1alpha1 a lenient path is implemented and will throw a + // strict decoding warning instead of failing to load + name: "duplicate field", + mode: "iptables", + bindAddress: "9.8.7.6", + clusterCIDR: "1.2.3.0/24", + healthzBindAddress: "1.2.3.4:12345", + metricsBindAddress: "2.3.4.5:23456", + extraConfig: "bindAddress: 9.8.7.6", + }, } for _, tc := range testCases { @@ -268,11 +292,17 @@ nodePortAddresses: options := NewOptions() - yaml := fmt.Sprintf( + baseYAML := fmt.Sprintf( yamlTemplate, tc.bindAddress, tc.clusterCIDR, tc.healthzBindAddress, tc.metricsBindAddress, tc.mode) + + // Append additional configuration to the base yaml template + yaml := fmt.Sprintf("%s\n%s", baseYAML, tc.extraConfig) + config, err := options.loadConfig([]byte(yaml)) + assert.NoError(t, err, "unexpected error for %s: %v", tc.name, err) + if !reflect.DeepEqual(expected, config) { t.Fatalf("unexpected config for %s, diff = %s", tc.name, diff.ObjectDiff(config, expected)) } @@ -281,10 +311,15 @@ nodePortAddresses: // TestLoadConfigFailures tests failure modes for loadConfig() func TestLoadConfigFailures(t *testing.T) { - yamlTemplate := `bindAddress: 0.0.0.0 -clusterCIDR: "1.2.3.0/24" -configSyncPeriod: 15s -kind: KubeProxyConfiguration` + // TODO(phenixblue): Uncomment below template when v1alpha2+ of kube-proxy config is + // released with strict decoding. These associated tests will fail with + // the lenient codec and only one config API version. + /* + yamlTemplate := `bindAddress: 0.0.0.0 + clusterCIDR: "1.2.3.0/24" + configSyncPeriod: 15s + kind: KubeProxyConfiguration` + */ testCases := []struct { name string @@ -307,16 +342,21 @@ kind: KubeProxyConfiguration` config: "bindAddress: ::", expErr: "mapping values are not allowed in this context", }, - { - name: "Duplicate fields", - config: fmt.Sprintf("%s\nbindAddess: 1.2.3.4", yamlTemplate), - checkFn: kuberuntime.IsStrictDecodingError, - }, - { - name: "Unknown field", - config: fmt.Sprintf("%s\nfoo: bar", yamlTemplate), - checkFn: kuberuntime.IsStrictDecodingError, - }, + // TODO(phenixblue): Uncomment below tests when v1alpha2+ of kube-proxy config is + // released with strict decoding. These tests will fail with the + // lenient codec and only one config API version. + /* + { + name: "Duplicate fields", + config: fmt.Sprintf("%s\nbindAddress: 1.2.3.4", yamlTemplate), + checkFn: kuberuntime.IsStrictDecodingError, + }, + { + name: "Unknown field", + config: fmt.Sprintf("%s\nfoo: bar", yamlTemplate), + checkFn: kuberuntime.IsStrictDecodingError, + }, + */ } version := "apiVersion: kubeproxy.config.k8s.io/v1alpha1"