Merge pull request #86144 from tahsinrahman/dont-serialize-internal

Do not serialize internal types in ComponentConfig tests

pkg/proxy/apis/config/scheme/testdata/KubeProxyConfiguration/after/__internal.yaml

@@ -1,43 +0,0 @@
-BindAddress: ""
-ClientConnection:
-  AcceptContentTypes: ""
-  Burst: 0
-  ContentType: ""
-  Kubeconfig: ""
-  QPS: 0
-ClusterCIDR: ""
-ConfigSyncPeriod: 0s
-Conntrack:
-  MaxPerCore: null
-  Min: null
-  TCPCloseWaitTimeout: null
-  TCPEstablishedTimeout: null
-EnableProfiling: false
-FeatureGates: null
-HealthzBindAddress: ""
-HostnameOverride: ""
-IPTables:
-  MasqueradeAll: false
-  MasqueradeBit: null
-  MinSyncPeriod: 0s
-  SyncPeriod: 0s
-IPVS:
-  ExcludeCIDRs: null
-  MinSyncPeriod: 0s
-  Scheduler: ""
-  StrictARP: false
-  SyncPeriod: 0s
-  TCPFinTimeout: 0s
-  TCPTimeout: 0s
-  UDPTimeout: 0s
-MetricsBindAddress: ""
-Mode: ""
-NodePortAddresses: null
-OOMScoreAdj: null
-PortRange: ""
-ShowHiddenMetricsForVersion: ""
-UDPIdleTimeout: 0s
-Winkernel:
-  EnableDSR: false
-  NetworkName: ""
-  SourceVip: ""

pkg/proxy/apis/config/scheme/testdata/KubeProxyConfiguration/before/v1alpha1.yaml

@@ -0,0 +1,2 @@
+apiVersion: kubeproxy.config.k8s.io/v1alpha1
+kind: KubeProxyConfiguration

pkg/proxy/apis/config/scheme/testdata/KubeProxyConfiguration/v1alpha1To__internal/empty.yaml.after_roundtrip

@@ -1,43 +0,0 @@
-BindAddress: 0.0.0.0
-ClientConnection:
-  AcceptContentTypes: ""
-  Burst: 10
-  ContentType: application/vnd.kubernetes.protobuf
-  Kubeconfig: ""
-  QPS: 5
-ClusterCIDR: ""
-ConfigSyncPeriod: 15m0s
-Conntrack:
-  MaxPerCore: 32768
-  Min: 131072
-  TCPCloseWaitTimeout: 1h0m0s
-  TCPEstablishedTimeout: 24h0m0s
-EnableProfiling: false
-FeatureGates: {}
-HealthzBindAddress: 0.0.0.0:10256
-HostnameOverride: ""
-IPTables:
-  MasqueradeAll: false
-  MasqueradeBit: 14
-  MinSyncPeriod: 0s
-  SyncPeriod: 30s
-IPVS:
-  ExcludeCIDRs: null
-  MinSyncPeriod: 0s
-  Scheduler: ""
-  StrictARP: false
-  SyncPeriod: 30s
-  TCPFinTimeout: 0s
-  TCPTimeout: 0s
-  UDPTimeout: 0s
-MetricsBindAddress: 127.0.0.1:10249
-Mode: ""
-NodePortAddresses: null
-OOMScoreAdj: -999
-PortRange: ""
-ShowHiddenMetricsForVersion: ""
-UDPIdleTimeout: 250ms
-Winkernel:
-  EnableDSR: false
-  NetworkName: ""
-  SourceVip: ""

staging/src/k8s.io/component-base/config/testing/BUILD

@@ -13,11 +13,11 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/github.com/google/go-cmp/cmp:go_default_library",
     ],

staging/src/k8s.io/component-base/config/testing/defaulting.go

@@ -27,24 +27,31 @@ import (
 
 // DefaultingTest run defaulting tests for given scheme
 func DefaultingTest(t *testing.T, scheme *runtime.Scheme, codecs serializer.CodecFactory) {
-	tc := GetDefaultingTestCases(scheme)
-	RunTestsOnYAMLData(t, scheme, tc, codecs)
+	cases := GetDefaultingTestCases(t, scheme, codecs)
+	RunTestsOnYAMLData(t, cases)
 }
 
 // GetDefaultingTestCases returns defaulting testcases for given scheme
-func GetDefaultingTestCases(scheme *runtime.Scheme) []TestCase {
+func GetDefaultingTestCases(t *testing.T, scheme *runtime.Scheme, codecs serializer.CodecFactory) []TestCase {
 	cases := []TestCase{}
 	for gvk := range scheme.AllKnownTypes() {
+		if gvk.Version == runtime.APIVersionInternal {
+			continue
+		}
 		beforeDir := fmt.Sprintf("testdata/%s/before", gvk.Kind)
 		afterDir := fmt.Sprintf("testdata/%s/after", gvk.Kind)
 		filename := fmt.Sprintf("%s.yaml", gvk.Version)
 
+		codec, err := getCodecForGV(codecs, gvk.GroupVersion())
+		if err != nil {
+			t.Fatal(err)
+		}
+
 		cases = append(cases, TestCase{
 			name:  fmt.Sprintf("default_%s", gvk.Version),
 			in:    filepath.Join(beforeDir, filename),
-			inGVK: gvk,
 			out:   filepath.Join(afterDir, filename),
-			outGV: gvk.GroupVersion(),
+			codec: codec,
 		})
 	}
 	return cases

staging/src/k8s.io/component-base/config/testing/helpers.go

@@ -18,6 +18,7 @@ package testing
 
 import (
 	"bytes"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -28,69 +29,77 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 )
 
+// TestCase defines a testcase for roundtrip and defaulting tests
+type TestCase struct {
+	name, in, out string
+	codec         runtime.Codec
+}
+
 // RunTestsOnYAMLData decodes the yaml file from specified path, encodes the object and matches
 // with expected yaml in specified path
-func RunTestsOnYAMLData(t *testing.T, scheme *runtime.Scheme, tests []TestCase, codecs serializer.CodecFactory) {
-	for _, rt := range tests {
-		t.Run(rt.name, func(t *testing.T) {
-			obj, err := decodeTestData(rt.in, scheme, rt.inGVK, codecs)
+func RunTestsOnYAMLData(t *testing.T, tests []TestCase) {
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			roundTrip(t, tc)
+		})
+	}
+}
+
+func decodeYAML(t *testing.T, path string, codec runtime.Codec) runtime.Object {
+	content, err := ioutil.ReadFile(path)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-			const mediaType = runtime.ContentTypeYAML
-			info, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), mediaType)
-			if !ok {
-				t.Errorf("unable to locate encoder -- %q is not a supported media type", mediaType)
-			}
-
-			encoder := codecs.EncoderForVersion(info.Serializer, rt.outGV)
-
-			actual, err := runtime.Encode(encoder, obj)
+	// decode to internal type
+	object, err := runtime.Decode(codec, content)
 	if err != nil {
-				t.Fatalf("failed to encode object: %v", err)
+		t.Fatal(err)
 	}
 
-			expected, err := ioutil.ReadFile(rt.out)
+	return object
+}
+
+func getCodecForGV(codecs serializer.CodecFactory, gv schema.GroupVersion) (runtime.Codec, error) {
+	mediaType := runtime.ContentTypeYAML
+	serializerInfo, ok := runtime.SerializerInfoForMediaType(codecs.SupportedMediaTypes(), mediaType)
+	if !ok {
+		return nil, fmt.Errorf("unable to locate encoder -- %q is not a supported media type", mediaType)
+	}
+	codec := codecs.CodecForVersions(serializerInfo.Serializer, codecs.UniversalDeserializer(), gv, nil)
+	return codec, nil
+}
+
+func matchOutputFile(t *testing.T, actual []byte, expectedFilePath string) {
+	expected, err := ioutil.ReadFile(expectedFilePath)
 	if err != nil && !os.IsNotExist(err) {
 		t.Fatalf("couldn't read test data: %v", err)
 	}
 
 	needsUpdate := false
+	const updateEnvVar = "UPDATE_COMPONENTCONFIG_FIXTURE_DATA"
+
 	if os.IsNotExist(err) {
 		needsUpdate = true
+		if os.Getenv(updateEnvVar) != "true" {
 			t.Error("couldn't find test data")
+		}
 	} else {
 		if !bytes.Equal(expected, actual) {
-					t.Errorf("Output does not match expected, diff (- want, + got):\n\tin: %s\n\tout: %s\n\tgroupversion: %s\n\tdiff: \n%s\n",
-						rt.in, rt.out, rt.outGV.String(), cmp.Diff(string(expected), string(actual)))
+			t.Errorf("Output does not match expected, diff (- want, + got):\n%s\n",
+				cmp.Diff(string(expected), string(actual)))
 			needsUpdate = true
 		}
 	}
 	if needsUpdate {
-				const updateEnvVar = "UPDATE_COMPONENTCONFIG_FIXTURE_DATA"
 		if os.Getenv(updateEnvVar) == "true" {
-					if err := ioutil.WriteFile(rt.out, actual, 0755); err != nil {
+			if err := ioutil.WriteFile(expectedFilePath, actual, 0644); err != nil {
 				t.Fatal(err)
 			}
-					t.Logf("wrote expected test data... verify, commit, and rerun tests")
+			t.Error("wrote expected test data... verify, commit, and rerun tests")
 		} else {
-					t.Logf("if the diff is expected because of a new type or a new field, re-run with %s=true to update the compatibility data", updateEnvVar)
+			t.Errorf("if the diff is expected because of a new type or a new field, "+
+				"re-run with %s=true to update the compatibility data or generate missing files", updateEnvVar)
 		}
 	}
-		})
-	}
-}
-
-func decodeTestData(path string, scheme *runtime.Scheme, gvk schema.GroupVersionKind, codecs serializer.CodecFactory) (runtime.Object, error) {
-	content, err := ioutil.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-
-	obj, _, err := codecs.DecoderToVersion(codecs.UniversalDecoder(), gvk.GroupVersion()).Decode(content, &gvk, nil)
-	if err != nil {
-		return nil, err
-	}
-	return obj, nil
 }

staging/src/k8s.io/component-base/config/testing/roundtrip.go

@@ -18,75 +18,92 @@ package testing
 
 import (
 	"fmt"
-	"os"
+	"io/ioutil"
 	"path/filepath"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // RoundTripTest runs roundtrip tests for given scheme
 func RoundTripTest(t *testing.T, scheme *runtime.Scheme, codecs serializer.CodecFactory) {
-	tc := GetRoundtripTestCases(scheme, nil)
-	RunTestsOnYAMLData(t, scheme, tc, codecs)
-}
-
-// TestCase defines a testcase for roundtrip and defaulting tests
-type TestCase struct {
-	name, in, out string
-	inGVK         schema.GroupVersionKind
-	outGV         schema.GroupVersion
+	tc := GetRoundtripTestCases(t, scheme, codecs)
+	RunTestsOnYAMLData(t, tc)
 }
 
 // GetRoundtripTestCases returns the testcases for roundtrip testing for given scheme
-func GetRoundtripTestCases(scheme *runtime.Scheme, disallowMarshalGroupVersions sets.String) []TestCase {
+func GetRoundtripTestCases(t *testing.T, scheme *runtime.Scheme, codecs serializer.CodecFactory) []TestCase {
 	cases := []TestCase{}
 	versionsForKind := map[schema.GroupKind][]string{}
 	for gvk := range scheme.AllKnownTypes() {
+		if gvk.Version != runtime.APIVersionInternal {
 			versionsForKind[gvk.GroupKind()] = append(versionsForKind[gvk.GroupKind()], gvk.Version)
 		}
+	}
 
 	for gk, versions := range versionsForKind {
-		for _, vin := range versions {
-			if vin == runtime.APIVersionInternal {
-				continue // Don't try to deserialize the internal version
-			}
-			for _, vout := range versions {
-				inGVK := schema.GroupVersionKind{Group: gk.Group, Version: vin, Kind: gk.Kind}
-				marshalGV := schema.GroupVersion{Group: gk.Group, Version: vout}
-				if disallowMarshalGroupVersions.Has(marshalGV.String()) {
-					continue // Don't marshal a gv that is blacklisted
-				}
-				testdir := filepath.Join("testdata", gk.Kind, fmt.Sprintf("%sTo%s", vin, vout))
-				utilruntime.Must(filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error {
+		testdir := filepath.Join("testdata", gk.Kind, "roundtrip")
+		dirs, err := ioutil.ReadDir(testdir)
 		if err != nil {
-						return err
+			t.Fatalf("failed to read testdir %s: %v", testdir, err)
 		}
-					if info.IsDir() {
-						if info.Name() == fmt.Sprintf("%sTo%s", vin, vout) {
-							return nil
-						}
-						return filepath.SkipDir
-					}
-					if filepath.Ext(info.Name()) != ".yaml" {
-						return nil
-					}
-					cases = append(cases, TestCase{
-						name:  fmt.Sprintf("%sTo%s", vin, vout),
-						in:    filepath.Join(testdir, info.Name()),
-						inGVK: inGVK,
-						out:   filepath.Join(testdir, fmt.Sprintf("%s.after_roundtrip", info.Name())),
-						outGV: marshalGV,
-					})
 
-					return nil
-				}))
+		for _, dir := range dirs {
+			for _, vin := range versions {
+				for _, vout := range versions {
+					marshalGVK := gk.WithVersion(vout)
+					codec, err := getCodecForGV(codecs, marshalGVK.GroupVersion())
+					if err != nil {
+						t.Fatalf("failed to get codec for %v: %v", marshalGVK.GroupVersion().String(), err)
+					}
+
+					testname := dir.Name()
+					cases = append(cases, TestCase{
+						name:  fmt.Sprintf("%sTo%s_%s", vin, vout, testname),
+						in:    filepath.Join(testdir, testname, vin+".yaml"),
+						out:   filepath.Join(testdir, testname, vout+".yaml"),
+						codec: codec,
+					})
+				}
 			}
 		}
 	}
 	return cases
 }
+
+func roundTrip(t *testing.T, tc TestCase) {
+	object := decodeYAML(t, tc.in, tc.codec)
+
+	// original object of internal type
+	original := object
+
+	// encode (serialize) the object using the provided codec
+	data, err := runtime.Encode(tc.codec, object)
+	if err != nil {
+		t.Fatalf("failed to encode object: %v", err)
+	}
+
+	// ensure that the encoding should not alter the object
+	if !apiequality.Semantic.DeepEqual(original, object) {
+		t.Fatalf("encode altered the object, diff (- want, + got): \n%v", cmp.Diff(original, object))
+	}
+
+	// decode (deserialize) the encoded data back into an object
+	obj2, err := runtime.Decode(tc.codec, data)
+	if err != nil {
+		t.Fatalf("failed to decode: %v", err)
+	}
+
+	// ensure that the object produced from decoding the encoded data is equal
+	// to the original object
+	if !apiequality.Semantic.DeepEqual(original, obj2) {
+		t.Fatalf("object was not the same after roundtrip, diff (- want, + got):\n%v", cmp.Diff(object, obj2))
+	}
+
+	// match with the input file, checks if they're the same after roundtrip
+	matchOutputFile(t, data, tc.out)
+}