From 6acef2b94c289d37afda6ec97e652d5dcf38e12f Mon Sep 17 00:00:00 2001
From: obitech <ajknipping@mailbox.org>
Date: Mon, 23 Sep 2019 18:04:10 +0200
Subject: [PATCH] Enable strict serializer with codec factory

- Enabling strict serializer will throw errors on e.g. duplicate
  or unknown fields in YAML configs
- Add test cases for duplicate and unknown fields
---
 cmd/kube-scheduler/app/options/BUILD          |  1 +
 .../app/options/options_test.go               | 59 +++++++++++++++++--
 pkg/scheduler/apis/config/scheme/scheme.go    |  2 +-
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/cmd/kube-scheduler/app/options/BUILD b/cmd/kube-scheduler/app/options/BUILD
index cb094d3ff64..78518923455 100644
--- a/cmd/kube-scheduler/app/options/BUILD
+++ b/cmd/kube-scheduler/app/options/BUILD
@@ -75,5 +75,6 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/rand:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/server/options: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-scheduler/app/options/options_test.go b/cmd/kube-scheduler/app/options/options_test.go
index fa4f4f36258..1b7133c7224 100644
--- a/cmd/kube-scheduler/app/options/options_test.go
+++ b/cmd/kube-scheduler/app/options/options_test.go
@@ -24,10 +24,11 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"strings"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/diff"
@@ -112,7 +113,7 @@ leaderElection:
 		t.Fatal(err)
 	}
 
-	invalidconfigFile := filepath.Join(tmpDir, "scheduler_invalid.yaml")
+	invalidconfigFile := filepath.Join(tmpDir, "scheduler_invalid_wrong_api_version.yaml")
 	if err := ioutil.WriteFile(invalidconfigFile, []byte(fmt.Sprintf(`
 apiVersion: componentconfig/v1alpha2
 kind: KubeSchedulerConfiguration
@@ -123,6 +124,30 @@ leaderElection:
 		t.Fatal(err)
 	}
 
+	unknownFieldConfig := filepath.Join(tmpDir, "scheduler_invalid_unknown_field.yaml")
+	if err := ioutil.WriteFile(unknownFieldConfig, []byte(fmt.Sprintf(`
+apiVersion: kubescheduler.config.k8s.io/v1alpha1
+kind: KubeSchedulerConfiguration
+clientConnection:
+  kubeconfig: "%s"
+leaderElection:
+  leaderElect: true
+foo: bar`, configKubeconfig)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+
+	duplicateFieldConfig := filepath.Join(tmpDir, "scheduler_invalid_duplicate_fields.yaml")
+	if err := ioutil.WriteFile(duplicateFieldConfig, []byte(fmt.Sprintf(`
+apiVersion: kubescheduler.config.k8s.io/v1alpha1
+kind: KubeSchedulerConfiguration
+clientConnection:
+  kubeconfig: "%s"
+leaderElection:
+  leaderElect: true
+  leaderElect: false`, configKubeconfig)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+
 	// flag-specified kubeconfig
 	flagKubeconfig := filepath.Join(tmpDir, "flag.kubeconfig")
 	if err := ioutil.WriteFile(flagKubeconfig, []byte(fmt.Sprintf(`
@@ -189,6 +214,7 @@ pluginConfig:
 		expectedUsername string
 		expectedError    string
 		expectedConfig   kubeschedulerconfig.KubeSchedulerConfiguration
+		checkErrFn       func(err error) bool
 	}{
 		{
 			name: "config file",
@@ -433,6 +459,22 @@ pluginConfig:
 			options:       &Options{},
 			expectedError: "no configuration has been provided",
 		},
+		{
+			name: "unknown field",
+			options: &Options{
+				ConfigFile: unknownFieldConfig,
+			},
+			expectedError: "found unknown field: foo",
+			checkErrFn:    runtime.IsStrictDecodingError,
+		},
+		{
+			name: "duplicate fields",
+			options: &Options{
+				ConfigFile: duplicateFieldConfig,
+			},
+			expectedError: `key "leaderElect" already set`,
+			checkErrFn:    runtime.IsStrictDecodingError,
+		},
 	}
 
 	for _, tc := range testcases {
@@ -442,11 +484,16 @@ pluginConfig:
 
 			// handle errors
 			if err != nil {
-				if tc.expectedError == "" {
-					t.Error(err)
-				} else if !strings.Contains(err.Error(), tc.expectedError) {
-					t.Errorf("expected %q, got %q", tc.expectedError, err.Error())
+				if tc.expectedError != "" || tc.checkErrFn != nil {
+					if tc.expectedError != "" {
+						assert.Contains(t, err.Error(), tc.expectedError, tc.name)
+					}
+					if tc.checkErrFn != nil {
+						assert.True(t, tc.checkErrFn(err), "got error: %v", err)
+					}
+					return
 				}
+				assert.NoError(t, err)
 				return
 			}
 
diff --git a/pkg/scheduler/apis/config/scheme/scheme.go b/pkg/scheduler/apis/config/scheme/scheme.go
index 69aac55d380..1a6ac1ca4d0 100644
--- a/pkg/scheduler/apis/config/scheme/scheme.go
+++ b/pkg/scheduler/apis/config/scheme/scheme.go
@@ -29,7 +29,7 @@ var (
 	Scheme = runtime.NewScheme()
 
 	// Codecs provides access to encoding and decoding for the scheme.
-	Codecs = serializer.NewCodecFactory(Scheme)
+	Codecs = serializer.NewCodecFactory(Scheme, serializer.EnableStrict)
 )
 
 func init() {