From 32b2eea50d6457b4b1abf50e8e03be52e81d30c2 Mon Sep 17 00:00:00 2001
From: Sean Sullivan <seans3@gmail.com>
Date: Fri, 11 Oct 2024 16:17:09 -0700
Subject: [PATCH] EgressSelectorConfiguration now uses strict validation

---
 .../pkg/server/egressselector/config.go       | 17 +++------
 .../pkg/server/egressselector/config_test.go  | 37 ++++++++++++++++++-
 .../apiserver/tracing/tracing_test.go         |  2 +-
 3 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go
index ce9a3691a96..0513b282236 100644
--- a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go
+++ b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config.go
@@ -22,13 +22,12 @@ import (
 	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/apis/apiserver"
 	"k8s.io/apiserver/pkg/apis/apiserver/install"
-	"k8s.io/apiserver/pkg/apis/apiserver/v1beta1"
 	"k8s.io/utils/path"
-	"sigs.k8s.io/yaml"
 )
 
 var cfgScheme = runtime.NewScheme()
@@ -55,19 +54,13 @@ func ReadEgressSelectorConfiguration(configFilePath string) (*apiserver.EgressSe
 	if err != nil {
 		return nil, fmt.Errorf("unable to read egress selector configuration from %q [%v]", configFilePath, err)
 	}
-	var decodedConfig v1beta1.EgressSelectorConfiguration
-	err = yaml.Unmarshal(data, &decodedConfig)
+	config, gvk, err := serializer.NewCodecFactory(cfgScheme, serializer.EnableStrict).UniversalDecoder().Decode(data, nil, nil)
 	if err != nil {
-		// we got an error where the decode wasn't related to a missing type
 		return nil, err
 	}
-	if decodedConfig.Kind != "EgressSelectorConfiguration" {
-		return nil, fmt.Errorf("invalid service configuration object %q", decodedConfig.Kind)
-	}
-	internalConfig := &apiserver.EgressSelectorConfiguration{}
-	if err := cfgScheme.Convert(&decodedConfig, internalConfig, nil); err != nil {
-		// we got an error where the decode wasn't related to a missing type
-		return nil, err
+	internalConfig, ok := config.(*apiserver.EgressSelectorConfiguration)
+	if !ok {
+		return nil, fmt.Errorf("unexpected config type: %v", gvk)
 	}
 	return internalConfig, nil
 }
diff --git a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go
index 6effe442c4e..5709b6aebc9 100644
--- a/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/server/egressselector/config_test.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 
 	utiltesting "k8s.io/client-go/util/testing"
@@ -52,7 +53,39 @@ func TestReadEgressSelectorConfiguration(t *testing.T) {
 			createFile:     false,
 			contents:       ``,
 			expectedResult: nil,
-			expectedError:  strptr("unable to read egress selector configuration from \"test-egress-selector-config-absent\" [open test-egress-selector-config-absent: no such file or directory]"),
+			expectedError:  strptr("errors.errorString{s:\"unable to read egress selector configuration"),
+		},
+		{
+			name:       "unknown field causes error",
+			createFile: false,
+			contents: `
+apiVersion: apiserver.k8s.io/v1beta1
+kind: EgressSelectorConfiguration
+egressSelections:
+- name: "etcd"
+  connection:
+    proxyProtocol: "Direct"
+  foo:
+    bar: "baz"
+`,
+			expectedResult: nil,
+			expectedError:  strptr("runtime.strictDecodingError"),
+		},
+		{
+			name:       "duplicate field causes error",
+			createFile: false,
+			contents: `
+apiVersion: apiserver.k8s.io/v1beta1
+kind: EgressSelectorConfiguration
+egressSelections:
+- name: "etcd"
+  connection:
+    proxyProtocol: "Direct"
+  connection:
+    proxyProtocol: "Indirect"
+`,
+			expectedResult: nil,
+			expectedError:  strptr("runtime.strictDecodingError"),
 		},
 		{
 			name:       "v1beta1",
@@ -295,7 +328,7 @@ spec:
 			if err != nil && tc.expectedError == nil {
 				t.Errorf("unexpected error calling ReadEgressSelectorConfiguration got: %#v", err)
 			}
-			if err != nil && tc.expectedError != nil && err.Error() != *tc.expectedError {
+			if err != nil && tc.expectedError != nil && strings.Contains(err.Error(), *tc.expectedError) {
 				t.Errorf("calling ReadEgressSelectorConfiguration expected error: %s, got %#v", *tc.expectedError, err)
 			}
 			if !reflect.DeepEqual(config, tc.expectedResult) {
diff --git a/test/integration/apiserver/tracing/tracing_test.go b/test/integration/apiserver/tracing/tracing_test.go
index 823913d3102..565d8ef5cf6 100644
--- a/test/integration/apiserver/tracing/tracing_test.go
+++ b/test/integration/apiserver/tracing/tracing_test.go
@@ -208,7 +208,7 @@ func TestAPIServerTracingWithEgressSelector(t *testing.T) {
 	defer os.Remove(egressSelectorConfigFile.Name())
 
 	if err := os.WriteFile(egressSelectorConfigFile.Name(), []byte(`
-apiVersion: apiserver.config.k8s.io/v1beta1
+apiVersion: apiserver.k8s.io/v1beta1
 kind: EgressSelectorConfiguration
 egressSelections:
 - name: cluster