From 96f7d91cc7664e00949fdb556b9a723bcf029e05 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Thu, 29 Apr 2021 08:17:43 -0700 Subject: [PATCH] Timestamp fuzzer in metav1 should not use negative values While debugging a similar issue in Cluster API, found that the Creation and Deletion timestamp were mostly `nil`. After a bit of digging it seems that using the Fuzz on an int64 can cause the Rfc3339Copy() to error out (silently, the error is actually dropped) and return a nil value. When using int64 large negative numbers can be generated randomly, which result in an invalid date. Signed-off-by: Vince Prignano --- .../k8s.io/apimachinery/pkg/apis/meta/fuzzer/fuzzer.go | 8 +++++--- .../pkg/apis/meta/v1/unstructured/unstructured_test.go | 9 --------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer/fuzzer.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer/fuzzer.go index bb5e8a2712d..6bc1356ed46 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer/fuzzer.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer/fuzzer.go @@ -186,15 +186,17 @@ func v1FuzzerFuncs(codecs runtimeserializer.CodecFactory) []interface{} { j.ResourceVersion = strconv.FormatUint(c.RandUint64(), 10) j.UID = types.UID(c.RandString()) - var sec, nsec int64 + // Fuzzing sec and nsec in a smaller range (uint32 instead of int64), + // so that the result Unix time is a valid date and can be parsed into RFC3339 format. + var sec, nsec uint32 c.Fuzz(&sec) c.Fuzz(&nsec) - j.CreationTimestamp = metav1.Unix(sec, nsec).Rfc3339Copy() + j.CreationTimestamp = metav1.Unix(int64(sec), int64(nsec)).Rfc3339Copy() if j.DeletionTimestamp != nil { c.Fuzz(&sec) c.Fuzz(&nsec) - t := metav1.Unix(sec, nsec).Rfc3339Copy() + t := metav1.Unix(int64(sec), int64(nsec)).Rfc3339Copy() j.DeletionTimestamp = &t } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go index fe981137653..7ee74a3eed0 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured_test.go @@ -64,15 +64,6 @@ func TestUnstructuredMetadataRoundTrip(t *testing.T) { } setObjectMetaUsingAccessors(u, uCopy) - // TODO: remove this special casing when creationTimestamp becomes a pointer. - // Right now, creationTimestamp is a struct (metav1.Time) so omitempty holds no meaning for it. - // However, the current behaviour is to remove the field if it holds an empty struct. - // This special casing exists here because custom marshallers for metav1.Time marshal - // an empty value to "null", which gets converted to nil when converting to an unstructured map by "ToUnstructured". - if err := unstructured.SetNestedField(uCopy.UnstructuredContent(), nil, "metadata", "creationTimestamp"); err != nil { - t.Fatalf("unexpected error setting creationTimestamp as nil: %v", err) - } - if !equality.Semantic.DeepEqual(u, uCopy) { t.Errorf("diff: %v", diff.ObjectReflectDiff(u, uCopy)) }