diff --git a/pkg/api/conversion.go b/pkg/api/conversion.go index 5599db3460f..e0605fa098e 100644 --- a/pkg/api/conversion.go +++ b/pkg/api/conversion.go @@ -41,6 +41,7 @@ func init() { Convert_unversioned_ListMeta_To_unversioned_ListMeta, Convert_intstr_IntOrString_To_intstr_IntOrString, Convert_unversioned_Time_To_unversioned_Time, + Convert_string_slice_To_unversioned_Time, Convert_string_To_labels_Selector, Convert_string_To_fields_Selector, Convert_bool_ref_To_bool, @@ -116,6 +117,16 @@ func Convert_unversioned_Time_To_unversioned_Time(in *unversioned.Time, out *unv *out = *in return nil } + +// Convert_string_slice_To_unversioned_Time allows converting a URL query parameter value +func Convert_string_slice_To_unversioned_Time(input *[]string, out *unversioned.Time, s conversion.Scope) error { + str := "" + if len(*input) > 0 { + str = (*input)[0] + } + return out.UnmarshalQueryParameter(str) +} + func Convert_string_To_labels_Selector(in *string, out *labels.Selector, s conversion.Scope) error { selector, err := labels.Parse(*in) if err != nil { diff --git a/pkg/api/unversioned/time.go b/pkg/api/unversioned/time.go index 4072833d965..1180e6bd14f 100644 --- a/pkg/api/unversioned/time.go +++ b/pkg/api/unversioned/time.go @@ -97,6 +97,27 @@ func (t *Time) UnmarshalJSON(b []byte) error { return nil } +// UnmarshalQueryParameter converts from a URL query parameter value to an object +func (t *Time) UnmarshalQueryParameter(str string) error { + if len(str) == 0 { + t.Time = time.Time{} + return nil + } + // Tolerate requests from older clients that used JSON serialization to build query params + if len(str) == 4 && str == "null" { + t.Time = time.Time{} + return nil + } + + pt, err := time.Parse(time.RFC3339, str) + if err != nil { + return err + } + + t.Time = pt.Local() + return nil +} + // MarshalJSON implements the json.Marshaler interface. func (t Time) MarshalJSON() ([]byte, error) { if t.IsZero() { @@ -107,6 +128,16 @@ func (t Time) MarshalJSON() ([]byte, error) { return json.Marshal(t.UTC().Format(time.RFC3339)) } +// MarshalQueryParameter converts to a URL query parameter value +func (t Time) MarshalQueryParameter() (string, error) { + if t.IsZero() { + // Encode unset/nil objects as an empty string + return "", nil + } + + return t.UTC().Format(time.RFC3339), nil +} + // Fuzz satisfies fuzz.Interface. func (t *Time) Fuzz(c fuzz.Continue) { if t == nil { diff --git a/pkg/api/v1/conversion_test.go b/pkg/api/v1/conversion_test.go index 8cc2f047429..cb64f47d802 100644 --- a/pkg/api/v1/conversion_test.go +++ b/pkg/api/v1/conversion_test.go @@ -17,13 +17,105 @@ limitations under the License. package v1_test import ( + "net/url" + "reflect" "testing" + "time" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/resource" + "k8s.io/kubernetes/pkg/api/unversioned" versioned "k8s.io/kubernetes/pkg/api/v1" + "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util" ) +func TestPodLogOptions(t *testing.T) { + sinceSeconds := int64(1) + sinceTime := unversioned.NewTime(time.Date(2000, 1, 1, 12, 34, 56, 0, time.UTC).Local()) + tailLines := int64(2) + limitBytes := int64(3) + + versionedLogOptions := &versioned.PodLogOptions{ + Container: "mycontainer", + Follow: true, + Previous: true, + SinceSeconds: &sinceSeconds, + SinceTime: &sinceTime, + Timestamps: true, + TailLines: &tailLines, + LimitBytes: &limitBytes, + } + unversionedLogOptions := &api.PodLogOptions{ + Container: "mycontainer", + Follow: true, + Previous: true, + SinceSeconds: &sinceSeconds, + SinceTime: &sinceTime, + Timestamps: true, + TailLines: &tailLines, + LimitBytes: &limitBytes, + } + expectedParameters := url.Values{ + "container": {"mycontainer"}, + "follow": {"true"}, + "previous": {"true"}, + "sinceSeconds": {"1"}, + "sinceTime": {"2000-01-01T12:34:56Z"}, + "timestamps": {"true"}, + "tailLines": {"2"}, + "limitBytes": {"3"}, + } + + codec := runtime.NewParameterCodec(api.Scheme) + + // unversioned -> query params + { + actualParameters, err := codec.EncodeParameters(unversionedLogOptions, versioned.SchemeGroupVersion) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(actualParameters, expectedParameters) { + t.Fatalf("Expected\n%#v\ngot\n%#v", expectedParameters, actualParameters) + } + } + + // versioned -> query params + { + actualParameters, err := codec.EncodeParameters(versionedLogOptions, versioned.SchemeGroupVersion) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(actualParameters, expectedParameters) { + t.Fatalf("Expected\n%#v\ngot\n%#v", expectedParameters, actualParameters) + } + } + + // query params -> versioned + { + convertedLogOptions := &versioned.PodLogOptions{} + err := codec.DecodeParameters(expectedParameters, versioned.SchemeGroupVersion, convertedLogOptions) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(convertedLogOptions, versionedLogOptions) { + t.Fatalf("Unexpected deserialization:\n%s", util.ObjectGoPrintSideBySide(versionedLogOptions, convertedLogOptions)) + } + } + + // query params -> unversioned + { + convertedLogOptions := &api.PodLogOptions{} + err := codec.DecodeParameters(expectedParameters, versioned.SchemeGroupVersion, convertedLogOptions) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(convertedLogOptions, unversionedLogOptions) { + t.Fatalf("Unexpected deserialization:\n%s", util.ObjectGoPrintSideBySide(unversionedLogOptions, convertedLogOptions)) + } + } +} + // TestPodSpecConversion tests that ServiceAccount is an alias for // ServiceAccountName. func TestPodSpecConversion(t *testing.T) { diff --git a/pkg/conversion/queryparams/convert.go b/pkg/conversion/queryparams/convert.go index 0f04e7bb4cb..63c5456976e 100644 --- a/pkg/conversion/queryparams/convert.go +++ b/pkg/conversion/queryparams/convert.go @@ -23,6 +23,16 @@ import ( "strings" ) +// Marshaler converts an object to a query parameter string representation +type Marshaler interface { + MarshalQueryParameter() (string, error) +} + +// Unmarshaler converts a string representation to an object +type Unmarshaler interface { + UnmarshalQueryParameter(string) error +} + func jsonTag(field reflect.StructField) (string, bool) { structTag := field.Tag.Get("json") if len(structTag) == 0 { @@ -72,6 +82,31 @@ func zeroValue(value reflect.Value) bool { return reflect.DeepEqual(reflect.Zero(value.Type()).Interface(), value.Interface()) } +func customMarshalValue(value reflect.Value) (reflect.Value, bool) { + // Return unless we implement a custom query marshaler + if !value.CanInterface() { + return reflect.Value{}, false + } + + marshaler, ok := value.Interface().(Marshaler) + if !ok { + return reflect.Value{}, false + } + + // Don't invoke functions on nil pointers + // If the type implements MarshalQueryParameter, AND the tag is not omitempty, AND the value is a nil pointer, "" seems like a reasonable response + if isPointerKind(value.Kind()) && zeroValue(value) { + return reflect.ValueOf(""), true + } + + // Get the custom marshalled value + v, err := marshaler.MarshalQueryParameter() + if err != nil { + return reflect.Value{}, false + } + return reflect.ValueOf(v), true +} + func addParam(values url.Values, tag string, omitempty bool, value reflect.Value) { if omitempty && zeroValue(value) { return @@ -128,7 +163,8 @@ func convertStruct(result url.Values, st reflect.Type, sv reflect.Value) { kind := ft.Kind() if isPointerKind(kind) { - kind = ft.Elem().Kind() + ft = ft.Elem() + kind = ft.Kind() if !field.IsNil() { field = reflect.Indirect(field) } @@ -142,7 +178,11 @@ func convertStruct(result url.Values, st reflect.Type, sv reflect.Value) { addListOfParams(result, tag, omitempty, field) } case isStructKind(kind) && !(zeroValue(field) && omitempty): - convertStruct(result, ft, field) + if marshalValue, ok := customMarshalValue(field); ok { + addParam(result, tag, omitempty, marshalValue) + } else { + convertStruct(result, ft, field) + } } } } diff --git a/pkg/conversion/queryparams/convert_test.go b/pkg/conversion/queryparams/convert_test.go index 405357557fa..cbeeeca7394 100644 --- a/pkg/conversion/queryparams/convert_test.go +++ b/pkg/conversion/queryparams/convert_test.go @@ -20,6 +20,7 @@ import ( "net/url" "reflect" "testing" + "time" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/conversion/queryparams" @@ -61,6 +62,19 @@ type baz struct { func (obj *baz) GetObjectKind() unversioned.ObjectKind { return unversioned.EmptyObjectKind } +// childStructs tests some of the types we serialize to query params for log API calls +// notably, the nested time struct +type childStructs struct { + Container string `json:"container,omitempty"` + Follow bool `json:"follow,omitempty"` + Previous bool `json:"previous,omitempty"` + SinceSeconds *int64 `json:"sinceSeconds,omitempty"` + SinceTime *unversioned.Time `json:"sinceTime,omitempty"` + EmptyTime *unversioned.Time `json:"emptyTime"` +} + +func (obj *childStructs) GetObjectKind() unversioned.ObjectKind { return unversioned.EmptyObjectKind } + func validateResult(t *testing.T, input interface{}, actual, expected url.Values) { local := url.Values{} for k, v := range expected { @@ -73,7 +87,6 @@ func validateResult(t *testing.T, input interface{}, actual, expected url.Values } else { t.Errorf("%#v: values don't match: actual: %#v, expected: %#v", input, v, ev) } - break } delete(local, k) } @@ -83,6 +96,9 @@ func validateResult(t *testing.T, input interface{}, actual, expected url.Values } func TestConvert(t *testing.T) { + sinceSeconds := int64(123) + sinceTime := unversioned.Date(2000, 1, 1, 12, 34, 56, 0, time.UTC) + tests := []struct { input interface{} expected url.Values @@ -158,6 +174,27 @@ func TestConvert(t *testing.T) { }, expected: url.Values{"ptr": {"5"}}, }, + { + input: &childStructs{ + Container: "mycontainer", + Follow: true, + Previous: true, + SinceSeconds: &sinceSeconds, + SinceTime: &sinceTime, // test a custom marshaller + EmptyTime: nil, // test a nil custom marshaller without omitempty + }, + expected: url.Values{"container": {"mycontainer"}, "follow": {"true"}, "previous": {"true"}, "sinceSeconds": {"123"}, "sinceTime": {"2000-01-01T12:34:56Z"}, "emptyTime": {""}}, + }, + { + input: &childStructs{ + Container: "mycontainer", + Follow: true, + Previous: true, + SinceSeconds: &sinceSeconds, + SinceTime: nil, // test a nil custom marshaller with omitempty + }, + expected: url.Values{"container": {"mycontainer"}, "follow": {"true"}, "previous": {"true"}, "sinceSeconds": {"123"}, "emptyTime": {""}}, + }, } for _, test := range tests { diff --git a/test/e2e_node/kubelet_test.go b/test/e2e_node/kubelet_test.go index 69240dad1b9..c8fa7227fb8 100644 --- a/test/e2e_node/kubelet_test.go +++ b/test/e2e_node/kubelet_test.go @@ -26,6 +26,7 @@ import ( "time" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/unversioned" client "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/stats" @@ -69,7 +70,8 @@ var _ = Describe("Kubelet", func() { It("it should print the output to logs", func() { Eventually(func() string { - rc, err := cl.Pods(api.NamespaceDefault).GetLogs("busybox", &api.PodLogOptions{}).Stream() + sinceTime := unversioned.NewTime(time.Now().Add(time.Duration(-1 * time.Hour))) + rc, err := cl.Pods(api.NamespaceDefault).GetLogs("busybox", &api.PodLogOptions{SinceTime: &sinceTime}).Stream() if err != nil { return "" }