From ecdc1638f6557d8d10d72ebc821e182ead2f0cdc Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 9 Mar 2018 18:47:53 +0100 Subject: [PATCH] apiextensions-apiserver: add columns to CRD spec --- .../pkg/apis/apiextensions/fuzzer/fuzzer.go | 8 + .../pkg/apis/apiextensions/types.go | 24 +++ .../apis/apiextensions/v1beta1/defaults.go | 8 + .../pkg/apis/apiextensions/v1beta1/types.go | 24 +++ .../apiextensions/validation/validation.go | 39 ++++ .../pkg/apiserver/customresource_handler.go | 5 +- .../pkg/registry/customresource/etcd_test.go | 102 +++++++++- .../tableconvertor/tableconvertor.go | 136 +++++++++---- .../tableconvertor/tableconvertor_test.go | 68 +++++++ .../test/integration/table_test.go | 185 ++++++++++++++++++ .../apimachinery/pkg/api/meta/table/table.go | 6 +- .../pkg/registry/rest/resttest/resttest.go | 5 +- 12 files changed, 556 insertions(+), 54 deletions(-) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor_test.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/test/integration/table_test.go diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go index 0fe919ab64e..ff8cc033469 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer/fuzzer.go @@ -23,9 +23,12 @@ import ( "github.com/google/gofuzz" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" ) +var swaggerMetadataDescriptions = metav1.ObjectMeta{}.SwaggerDoc() + // Funcs returns the fuzzer functions for the apiextensions apis. func Funcs(codecs runtimeserializer.CodecFactory) []interface{} { return []interface{}{ @@ -53,6 +56,11 @@ func Funcs(codecs runtimeserializer.CodecFactory) []interface{} { } else if len(obj.Versions) != 0 { obj.Version = obj.Versions[0].Name } + if len(obj.AdditionalPrinterColumns) == 0 { + obj.AdditionalPrinterColumns = []apiextensions.CustomResourceColumnDefinition{ + {Name: "Age", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"], JSONPath: ".metadata.creationTimestamp"}, + } + } }, func(obj *apiextensions.CustomResourceDefinition, c fuzz.Continue) { c.FuzzNoCustom(obj) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go index debe74a5b0c..6fc75154fac 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go @@ -49,6 +49,8 @@ type CustomResourceDefinitionSpec struct { // major version, then minor version. An example sorted list of versions: // v10, v2, v1, v11beta2, v10beta3, v3beta1, v12alpha1, v11alpha2, foo1, foo10. Versions []CustomResourceDefinitionVersion + // AdditionalPrinterColumns are additional columns shown e.g. in kubectl next to the name. Defaults to a created-at column. + AdditionalPrinterColumns []CustomResourceColumnDefinition } type CustomResourceDefinitionVersion struct { @@ -61,6 +63,28 @@ type CustomResourceDefinitionVersion struct { Storage bool } +// CustomResourceColumnDefinition specifies a column for server side printing. +type CustomResourceColumnDefinition struct { + // name is a human readable name for the column. + Name string + // type is an OpenAPI type definition for this column. + // See https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types for more. + Type string + // format is an optional OpenAPI type definition for this column. The 'name' format is applied + // to the primary identifier column to assist in clients identifying column is the resource name. + // See https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types for more. + Format string + // description is a human readable description of this column. + Description string + // priority is an integer defining the relative importance of this column compared to others. Lower + // numbers are considered higher priority. Columns that may be omitted in limited space scenarios + // should be given a higher priority. + Priority int32 + + // JSONPath is a simple JSON path, i.e. without array notation. + JSONPath string +} + // CustomResourceDefinitionNames indicates the names to serve this CustomResourceDefinition type CustomResourceDefinitionNames struct { // Plural is the plural name of the resource to serve. It must match the name of the CustomResourceDefinition-registration diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go index 1984e229778..e3235e8702c 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go @@ -19,9 +19,12 @@ package v1beta1 import ( "strings" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) +var swaggerMetadataDescriptions = metav1.ObjectMeta{}.SwaggerDoc() + func addDefaultingFuncs(scheme *runtime.Scheme) error { scheme.AddTypeDefaultingFunc(&CustomResourceDefinition{}, func(obj interface{}) { SetDefaults_CustomResourceDefinition(obj.(*CustomResourceDefinition)) }) // TODO figure out why I can't seem to get my defaulter generated @@ -63,4 +66,9 @@ func SetDefaults_CustomResourceDefinitionSpec(obj *CustomResourceDefinitionSpec) if len(obj.Version) == 0 && len(obj.Versions) != 0 { obj.Version = obj.Versions[0].Name } + if len(obj.AdditionalPrinterColumns) == 0 { + obj.AdditionalPrinterColumns = []CustomResourceColumnDefinition{ + {Name: "Age", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"], JSONPath: ".metadata.creationTimestamp"}, + } + } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go index 9d8d1cd80d1..2080cc8217e 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go @@ -52,6 +52,8 @@ type CustomResourceDefinitionSpec struct { // major version, then minor version. An example sorted list of versions: // v10, v2, v1, v11beta2, v10beta3, v3beta1, v12alpha1, v11alpha2, foo1, foo10. Versions []CustomResourceDefinitionVersion `json:"versions,omitempty" protobuf:"bytes,7,rep,name=versions"` + // AdditionalPrinterColumns are additional columns shown e.g. in kubectl next to the name. Defaults to a created-at column. + AdditionalPrinterColumns []CustomResourceColumnDefinition `json:"additionalPrinterColumns,omitempty" protobuf:"bytes,8,rep,name=additionalPrinterColumns"` } type CustomResourceDefinitionVersion struct { @@ -64,6 +66,28 @@ type CustomResourceDefinitionVersion struct { Storage bool `json:"storage" protobuf:"varint,3,opt,name=storage"` } +// CustomResourceColumnDefinition specifies a column for server side printing. +type CustomResourceColumnDefinition struct { + // name is a human readable name for the column. + Name string `json:"name" protobuf:"bytes,1,opt,name=name"` + // type is an OpenAPI type definition for this column. + // See https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types for more. + Type string `json:"type" protobuf:"bytes,2,opt,name=type"` + // format is an optional OpenAPI type definition for this column. The 'name' format is applied + // to the primary identifier column to assist in clients identifying column is the resource name. + // See https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types for more. + Format string `json:"format,omitempty" protobuf:"bytes,3,opt,name=format"` + // description is a human readable description of this column. + Description string `json:"description,omitempty" protobuf:"bytes,4,opt,name=description"` + // priority is an integer defining the relative importance of this column compared to others. Lower + // numbers are considered higher priority. Columns that may be omitted in limited space scenarios + // should be given a higher priority. + Priority int32 `json:"priority,omitempty" protobuf:"bytes,5,opt,name=priority"` + + // JSONPath is a simple JSON path, i.e. with array notation. + JSONPath string `json:"JSONPath" protobuf:"bytes,6,opt,name=JSONPath"` +} + // CustomResourceDefinitionNames indicates the names to serve this CustomResourceDefinition type CustomResourceDefinitionNames struct { // Plural is the plural name of the resource to serve. It must match the name of the CustomResourceDefinition-registration diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index eb9acc79e8e..40baf42351a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -22,6 +22,7 @@ import ( "strings" genericvalidation "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/util/sets" validationutil "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -31,6 +32,11 @@ import ( apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" ) +var ( + printerColumnDatatypes = sets.NewString("integer", "number", "string", "boolean", "date") + customResourceColumnDefinitionFormats = sets.NewString("int32", "int64", "float", "double", "byte", "date", "date-time", "password") +) + // ValidateCustomResourceDefinition statically validates func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinition) field.ErrorList { nameValidationFn := func(name string, prefix bool) []string { @@ -175,6 +181,12 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi allErrs = append(allErrs, field.Forbidden(fldPath.Child("subresources"), "disabled by feature-gate CustomResourceSubresources")) } + for i := range spec.AdditionalPrinterColumns { + if errs := ValidateCustomResourceColumnDefinition(&spec.AdditionalPrinterColumns[i], fldPath.Child("columns").Index(i)); len(errs) > 0 { + allErrs = append(allErrs, errs...) + } + } + return allErrs } @@ -238,6 +250,33 @@ func ValidateCustomResourceDefinitionNames(names *apiextensions.CustomResourceDe return allErrs } +// ValidateCustomResourceColumnDefinition statically validates a printer column. +func ValidateCustomResourceColumnDefinition(col *apiextensions.CustomResourceColumnDefinition, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if len(col.Name) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("header"), "")) + } + + if len(col.Type) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("type"), fmt.Sprintf("must be one of %s", strings.Join(printerColumnDatatypes.List(), ",")))) + } else if !printerColumnDatatypes.Has(col.Type) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), col.Type, fmt.Sprintf("must be one of %s", strings.Join(printerColumnDatatypes.List(), ",")))) + } + + if len(col.Format) > 0 && !customResourceColumnDefinitionFormats.Has(col.Format) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("format"), col.Format, fmt.Sprintf("must be one of %s", strings.Join(customResourceColumnDefinitionFormats.List(), ",")))) + } + + if len(col.JSONPath) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("path"), "")) + } else if errs := validateSimpleJSONPath(col.JSONPath, fldPath.Child("path")); len(errs) > 0 { + allErrs = append(allErrs, errs...) + } + + return allErrs +} + // specStandardValidator applies validations for different OpenAPI specification versions. type specStandardValidator interface { validate(spec *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 20231bdf866..231bffdbf6d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -370,6 +370,8 @@ func (r *crdHandler) GetCustomResourceListerCollectionDeleter(crd *apiextensions return info.storages[info.storageVersion].CustomResource, nil } +var swaggerMetadataDescriptions = metav1.ObjectMeta{}.SwaggerDoc() + func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResourceDefinition) (*crdInfo, error) { storageMap := r.customStorage.Load().(crdStorageMap) if ret, ok := storageMap[crd.UID]; ok { @@ -439,8 +441,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource scaleSpec = crd.Spec.Subresources.Scale } - // TODO: identify how to pass printer specification from the CRD - table, err := tableconvertor.New(nil) + table, err := tableconvertor.New(crd.Spec.AdditionalPrinterColumns) if err != nil { glog.V(2).Infof("The CRD for %v has an invalid printer specification, falling back to default printing: %v", kind, err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go index 9a2c8320505..15a242e4493 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go @@ -21,12 +21,15 @@ import ( "reflect" "strings" "testing" + "time" autoscalingv1 "k8s.io/api/autoscaling/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" + metainternal "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/diff" @@ -72,8 +75,19 @@ func newStorage(t *testing.T) (customresource.CustomResourceStorage, *etcdtestin status := &apiextensions.CustomResourceSubresourceStatus{} - // TODO: identify how to pass printer specification from the CRD - table, _ := tableconvertor.New(nil) + headers := []apiextensions.CustomResourceColumnDefinition{ + {Name: "Age", Type: "date", JSONPath: ".metadata.creationTimestamp"}, + {Name: "Replicas", Type: "integer", JSONPath: ".spec.replicas"}, + {Name: "Missing", Type: "string", JSONPath: ".spec.missing"}, + {Name: "Invalid", Type: "integer", JSONPath: ".spec.string"}, + {Name: "String", Type: "string", JSONPath: ".spec.string"}, + {Name: "StringFloat64", Type: "string", JSONPath: ".spec.float64"}, + {Name: "StringInt64", Type: "string", JSONPath: ".spec.replicas"}, + {Name: "StringBool", Type: "string", JSONPath: ".spec.bool"}, + {Name: "Float64", Type: "number", JSONPath: ".spec.float64"}, + {Name: "Bool", Type: "boolean", JSONPath: ".spec.bool"}, + } + table, _ := tableconvertor.New(headers) storage := customresource.NewStorage( schema.GroupResource{Group: "mygroup.example.com", Resource: "noxus"}, @@ -112,11 +126,18 @@ func validNewCustomResource() *unstructured.Unstructured { "apiVersion": "mygroup.example.com/v1beta1", "kind": "Noxu", "metadata": map[string]interface{}{ - "namespace": "default", - "name": "foo", + "namespace": "default", + "name": "foo", + "creationTimestamp": time.Now().Add(-time.Hour*12 - 30*time.Minute).UTC().Format(time.RFC3339), }, "spec": map[string]interface{}{ - "replicas": int64(7), + "replicas": int64(7), + "string": "string", + "float64": float64(3.1415926), + "bool": true, + "stringList": []interface{}{"foo", "bar"}, + "mixedList": []interface{}{"foo", int64(42)}, + "nonPrimitiveList": []interface{}{"foo", []interface{}{int64(1), int64(2)}}, }, }, } @@ -225,6 +246,77 @@ func TestCategories(t *testing.T) { } } +func TestColumns(t *testing.T) { + storage, server := newStorage(t) + defer server.Terminate(t) + defer storage.CustomResource.Store.DestroyFunc() + + ctx := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault) + key := "/noxus/" + metav1.NamespaceDefault + "/foo" + validCustomResource := validNewCustomResource() + if err := storage.CustomResource.Storage.Create(ctx, key, validCustomResource, nil, 0); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + gottenList, err := storage.CustomResource.List(ctx, &metainternal.ListOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + tbl, err := storage.CustomResource.ConvertToTable(ctx, gottenList, &metav1beta1.TableOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectedColumns := []struct { + Name, Type string + }{ + {"Name", "string"}, + {"Age", "date"}, + {"Replicas", "integer"}, + {"Missing", "string"}, + {"Invalid", "integer"}, + {"String", "string"}, + {"StringFloat64", "string"}, + {"StringInt64", "string"}, + {"StringBool", "string"}, + {"Float64", "number"}, + {"Bool", "boolean"}, + } + if len(tbl.ColumnDefinitions) != len(expectedColumns) { + t.Fatalf("got %d columns, expected %d. Got: %+v", len(tbl.ColumnDefinitions), len(expectedColumns), tbl.ColumnDefinitions) + } + for i, d := range tbl.ColumnDefinitions { + if d.Name != expectedColumns[i].Name { + t.Errorf("got column %d name %q, expected %q", i, d.Name, expectedColumns[i].Name) + } + if d.Type != expectedColumns[i].Type { + t.Errorf("got column %d type %q, expected %q", i, d.Type, expectedColumns[i].Type) + } + } + + expectedRows := [][]interface{}{ + { + "foo", + "12h", + int64(7), + nil, + nil, + "string", + "3.1415926", + "7", + "true", + float64(3.1415926), + true, + }, + } + for i, r := range tbl.Rows { + if !reflect.DeepEqual(r.Cells, expectedRows[i]) { + t.Errorf("got row %d with cells %#v, expected %#v", i, r.Cells, expectedRows[i]) + } + } +} + func TestStatusUpdate(t *testing.T) { storage, server := newStorage(t) defer server.Terminate(t) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor.go index 8dbc0e77265..e1bed809d99 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor.go @@ -19,11 +19,11 @@ package tableconvertor import ( "bytes" "context" + "encoding/json" "fmt" - "strings" - - "github.com/go-openapi/spec" + "reflect" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apimachinery/pkg/api/meta" metatable "k8s.io/apimachinery/pkg/api/meta/table" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,56 +33,46 @@ import ( "k8s.io/client-go/util/jsonpath" ) -const printColumnsKey = "x-kubernetes-print-columns" - var swaggerMetadataDescriptions = metav1.ObjectMeta{}.SwaggerDoc() -// New creates a new table convertor for the provided OpenAPI schema. If the printer definition cannot be parsed, +// New creates a new table convertor for the provided CRD column definition. If the printer definition cannot be parsed, // error will be returned along with a default table convertor. -func New(extensions spec.Extensions) (rest.TableConvertor, error) { +func New(crdColumns []apiextensions.CustomResourceColumnDefinition) (rest.TableConvertor, error) { headers := []metav1beta1.TableColumnDefinition{ {Name: "Name", Type: "string", Format: "name", Description: swaggerMetadataDescriptions["name"]}, - {Name: "Age", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"]}, } c := &convertor{ headers: headers, } - format, ok := extensions.GetString(printColumnsKey) - if !ok { - return c, nil - } - // "x-kubernetes-print-columns": "custom-columns=NAME:.metadata.name,RSRC:.metadata.resourceVersion" - parts := strings.SplitN(format, "=", 2) - if len(parts) != 2 || parts[0] != "custom-columns" { - return c, fmt.Errorf("unrecognized column definition in 'x-kubernetes-print-columns', only support 'custom-columns=NAME=JSONPATH[,NAME=JSONPATH]'") - } - columnSpecs := strings.Split(parts[1], ",") - var columns []*jsonpath.JSONPath - for _, spec := range columnSpecs { - parts := strings.SplitN(spec, ":", 2) - if len(parts) != 2 || len(parts[0]) == 0 || len(parts[1]) == 0 { - return c, fmt.Errorf("unrecognized column definition in 'x-kubernetes-print-columns', must specify NAME=JSONPATH: %s", spec) - } - path := jsonpath.New(parts[0]) - if err := path.Parse(parts[1]); err != nil { - return c, fmt.Errorf("unrecognized column definition in 'x-kubernetes-print-columns': %v", spec) + + for _, col := range crdColumns { + path := jsonpath.New(col.Name) + if err := path.Parse(fmt.Sprintf("{%s}", col.JSONPath)); err != nil { + return c, fmt.Errorf("unrecognized column definition %q", col.JSONPath) } path.AllowMissingKeys(true) - columns = append(columns, path) - headers = append(headers, metav1beta1.TableColumnDefinition{ - Name: parts[0], - Type: "string", - Description: fmt.Sprintf("Custom resource definition column from OpenAPI (in JSONPath format): %s", parts[1]), + + desc := fmt.Sprintf("Custom resource definition column (in JSONPath format): %s", col.JSONPath) + if len(col.Description) > 0 { + desc = col.Description + } + + c.additionalColumns = append(c.additionalColumns, path) + c.headers = append(c.headers, metav1beta1.TableColumnDefinition{ + Name: col.Name, + Type: col.Type, + Format: col.Format, + Description: desc, + Priority: col.Priority, }) } - c.columns = columns - c.headers = headers + return c, nil } type convertor struct { - headers []metav1beta1.TableColumnDefinition - columns []*jsonpath.JSONPath + headers []metav1beta1.TableColumnDefinition + additionalColumns []*jsonpath.JSONPath } func (c *convertor) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1beta1.Table, error) { @@ -103,18 +93,80 @@ func (c *convertor) ConvertToTable(ctx context.Context, obj runtime.Object, tabl var err error buf := &bytes.Buffer{} table.Rows, err = metatable.MetaToTableRow(obj, func(obj runtime.Object, m metav1.Object, name, age string) ([]interface{}, error) { - cells := make([]interface{}, 2, 2+len(c.columns)) + cells := make([]interface{}, 1, 1+len(c.additionalColumns)) cells[0] = name - cells[1] = age - for _, column := range c.columns { - if err := column.Execute(buf, obj); err != nil { + customHeaders := c.headers[1:] + for i, column := range c.additionalColumns { + results, err := column.FindResults(obj.(runtime.Unstructured).UnstructuredContent()) + if err != nil || len(results) == 0 || len(results[0]) == 0 { cells = append(cells, nil) continue } - cells = append(cells, buf.String()) - buf.Reset() + + // as we only support simple JSON path, we can assume to have only one result (or none, filtered out above) + value := results[0][0].Interface() + if customHeaders[i].Type == "string" { + if err := column.PrintResults(buf, []reflect.Value{reflect.ValueOf(value)}); err == nil { + cells = append(cells, buf.String()) + buf.Reset() + } else { + cells = append(cells, nil) + } + } else { + cells = append(cells, cellForJSONValue(customHeaders[i].Type, value)) + } } return cells, nil }) return table, err } + +func cellForJSONValue(headerType string, value interface{}) interface{} { + if value == nil { + return nil + } + + switch headerType { + case "integer": + switch typed := value.(type) { + case int64: + return typed + case float64: + return int64(typed) + case json.Number: + if i64, err := typed.Int64(); err == nil { + return i64 + } + } + case "number": + switch typed := value.(type) { + case int64: + return float64(typed) + case float64: + return typed + case json.Number: + if f, err := typed.Float64(); err == nil { + return f + } + } + case "boolean": + if b, ok := value.(bool); ok { + return b + } + case "string": + if s, ok := value.(string); ok { + return s + } + case "date": + if typed, ok := value.(string); ok { + var timestamp metav1.Time + err := timestamp.UnmarshalQueryParameter(typed) + if err != nil { + return "" + } + return metatable.ConvertToHumanReadableDateType(timestamp) + } + } + + return nil +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor_test.go new file mode 100644 index 00000000000..179aabb8ab7 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tableconvertor + +import ( + "fmt" + "reflect" + "testing" + "time" +) + +func Test_cellForJSONValue(t *testing.T) { + tests := []struct { + headerType string + value interface{} + want interface{} + }{ + {"integer", int64(42), int64(42)}, + {"integer", float64(3.14), int64(3)}, + {"integer", true, nil}, + {"integer", "foo", nil}, + + {"number", int64(42), float64(42)}, + {"number", float64(3.14), float64(3.14)}, + {"number", true, nil}, + {"number", "foo", nil}, + + {"boolean", int64(42), nil}, + {"boolean", float64(3.14), nil}, + {"boolean", true, true}, + {"boolean", "foo", nil}, + + {"string", int64(42), nil}, + {"string", float64(3.14), nil}, + {"string", true, nil}, + {"string", "foo", "foo"}, + + {"date", int64(42), nil}, + {"date", float64(3.14), nil}, + {"date", true, nil}, + {"date", time.Now().Add(-time.Hour*12 - 30*time.Minute).UTC().Format(time.RFC3339), "12h"}, + {"date", time.Now().Add(+time.Hour*12 + 30*time.Minute).UTC().Format(time.RFC3339), ""}, + {"date", "", ""}, + + {"unknown", "foo", nil}, + } + for _, tt := range tests { + t.Run(fmt.Sprintf("%#v of type %s", tt.value, tt.headerType), func(t *testing.T) { + if got := cellForJSONValue(tt.headerType, tt.value); !reflect.DeepEqual(got, tt.want) { + t.Errorf("cellForJSONValue() = %#v, want %#v", got, tt.want) + } + }) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/table_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/table_test.go new file mode 100644 index 00000000000..0c3ac609a9c --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/table_test.go @@ -0,0 +1,185 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "fmt" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" + + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apiextensions-apiserver/test/integration/testserver" +) + +func newTableCRD() *apiextensionsv1beta1.CustomResourceDefinition { + return &apiextensionsv1beta1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "tables.mygroup.example.com"}, + Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{ + Group: "mygroup.example.com", + Version: "v1beta1", + Names: apiextensionsv1beta1.CustomResourceDefinitionNames{ + Plural: "tables", + Singular: "table", + Kind: "Table", + ListKind: "TablemList", + }, + Scope: apiextensionsv1beta1.ClusterScoped, + AdditionalPrinterColumns: []apiextensionsv1beta1.CustomResourceColumnDefinition{ + {Name: "Age", Type: "date", JSONPath: ".metadata.creationTimestamp"}, + {Name: "Alpha", Type: "string", JSONPath: ".spec.alpha"}, + {Name: "Beta", Type: "integer", Description: "the beta field", Format: "int64", Priority: 42, JSONPath: ".spec.beta"}, + {Name: "Gamma", Type: "integer", Description: "a column with wrongly typed values", JSONPath: ".spec.gamma"}, + }, + }, + } +} + +func newTableInstance(name string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "mygroup.example.com/v1beta1", + "kind": "Table", + "metadata": map[string]interface{}{ + "name": name, + }, + "spec": map[string]interface{}{ + "alpha": "foo_123", + "beta": 10, + "gamma": "bar", + "delta": "hello", + }, + }, + } +} + +func TestTableGet(t *testing.T) { + stopCh, config, err := testserver.StartDefaultServer() + if err != nil { + t.Fatal(err) + } + defer close(stopCh) + + apiExtensionClient, err := clientset.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + + crd := newTableCRD() + crd, err = testserver.CreateNewCustomResourceDefinition(crd, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + + crd, err = apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(crd.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + t.Logf("table crd created: %#v", crd) + + crClient := newNamespacedCustomResourceClient("", dynamicClient, crd) + foo, err := crClient.Create(newTableInstance("foo")) + if err != nil { + t.Fatalf("unable to create noxu instance: %v", err) + } + t.Logf("foo created: %#v", foo.UnstructuredContent()) + + gv := schema.GroupVersion{Group: crd.Spec.Group, Version: crd.Spec.Version} + gvk := gv.WithKind(crd.Spec.Names.Kind) + + scheme := runtime.NewScheme() + codecs := serializer.NewCodecFactory(scheme) + parameterCodec := runtime.NewParameterCodec(scheme) + metav1.AddToGroupVersion(scheme, gv) + scheme.AddKnownTypes(gv, &metav1beta1.Table{}, &metav1beta1.TableOptions{}) + scheme.AddKnownTypes(metav1beta1.SchemeGroupVersion, &metav1beta1.Table{}, &metav1beta1.TableOptions{}) + + crConfig := *config + crConfig.GroupVersion = &gv + crConfig.APIPath = "/apis" + crConfig.NegotiatedSerializer = serializer.DirectCodecFactory{CodecFactory: codecs} + crRestClient, err := rest.RESTClientFor(&crConfig) + if err != nil { + t.Fatal(err) + } + + ret, err := crRestClient.Get(). + Resource(crd.Spec.Names.Plural). + SetHeader("Accept", fmt.Sprintf("application/json;as=Table;v=%s;g=%s, application/json", metav1beta1.SchemeGroupVersion.Version, metav1beta1.GroupName)). + VersionedParams(&metav1beta1.TableOptions{}, parameterCodec). + Do(). + Get() + if err != nil { + t.Fatalf("failed to list %v resources: %v", gvk, err) + } + + tbl, ok := ret.(*metav1beta1.Table) + if !ok { + t.Fatalf("expected metav1beta1.Table, got %T", ret) + } + t.Logf("%v table list: %#v", gvk, tbl) + + if got, expected := len(tbl.ColumnDefinitions), 5; got != expected { + t.Errorf("expected %d headers, got %d", expected, got) + } else { + alpha := metav1beta1.TableColumnDefinition{Name: "Alpha", Type: "string", Format: "", Description: "Custom resource definition column (in JSONPath format): .spec.alpha", Priority: 0} + if got, expected := tbl.ColumnDefinitions[2], alpha; got != expected { + t.Errorf("expected column definition %#v, got %#v", expected, got) + } + + beta := metav1beta1.TableColumnDefinition{Name: "Beta", Type: "integer", Format: "int64", Description: "the beta field", Priority: 42} + if got, expected := tbl.ColumnDefinitions[3], beta; got != expected { + t.Errorf("expected column definition %#v, got %#v", expected, got) + } + + gamma := metav1beta1.TableColumnDefinition{Name: "Gamma", Type: "integer", Description: "a column with wrongly typed values"} + if got, expected := tbl.ColumnDefinitions[4], gamma; got != expected { + t.Errorf("expected column definition %#v, got %#v", expected, got) + } + } + if got, expected := len(tbl.Rows), 1; got != expected { + t.Errorf("expected %d rows, got %d", expected, got) + } else if got, expected := len(tbl.Rows[0].Cells), 5; got != expected { + t.Errorf("expected %d cells, got %d", expected, got) + } else { + if got, expected := tbl.Rows[0].Cells[0], "foo"; got != expected { + t.Errorf("expected cell[0] to equal %q, got %q", expected, got) + } + if got, expected := tbl.Rows[0].Cells[2], "foo_123"; got != expected { + t.Errorf("expected cell[2] to equal %q, got %q", expected, got) + } + if got, expected := tbl.Rows[0].Cells[3], int64(10); got != expected { + t.Errorf("expected cell[3] to equal %#v, got %#v", expected, got) + } + if got, expected := tbl.Rows[0].Cells[4], interface{}(nil); got != expected { + t.Errorf("expected cell[3] to equal %#v although the type does not match the column, got %#v", expected, got) + } + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/table/table.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/table/table.go index a0097a4e26d..2144a77cb19 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/table/table.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/table/table.go @@ -53,7 +53,7 @@ func MetaToTableRow(obj runtime.Object, rowFn func(obj runtime.Object, m metav1. row := metav1beta1.TableRow{ Object: runtime.RawExtension{Object: obj}, } - row.Cells, err = rowFn(obj, m, m.GetName(), translateTimestamp(m.GetCreationTimestamp())) + row.Cells, err = rowFn(obj, m, m.GetName(), ConvertToHumanReadableDateType(m.GetCreationTimestamp())) if err != nil { return nil, err } @@ -61,9 +61,9 @@ func MetaToTableRow(obj runtime.Object, rowFn func(obj runtime.Object, m metav1. return rows, nil } -// translateTimestamp returns the elapsed time since timestamp in +// ConvertToHumanReadableDateType returns the elapsed time since timestamp in // human-readable approximation. -func translateTimestamp(timestamp metav1.Time) string { +func ConvertToHumanReadableDateType(timestamp metav1.Time) string { if timestamp.IsZero() { return "" } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go index 64d792a607a..647b9401b1e 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/resttest/resttest.go @@ -1320,7 +1320,7 @@ func (t *Tester) testListTableConversion(obj runtime.Object, assignFn AssignFunc t.Errorf("column %d has no name", j) } switch column.Type { - case "string", "date", "integer": + case "string", "date", "integer", "number", "boolean": default: t.Errorf("column %d has unexpected type: %q", j, column.Type) } @@ -1342,13 +1342,14 @@ func (t *Tester) testListTableConversion(obj runtime.Object, assignFn AssignFunc } for i, row := range table.Rows { if len(row.Cells) != len(table.ColumnDefinitions) { - t.Errorf("row %d did not have the correct number of cells: %d in %v", i, len(table.ColumnDefinitions), row.Cells) + t.Errorf("row %d did not have the correct number of cells: %d in %v, expected %d", i, len(row.Cells), row.Cells, len(table.ColumnDefinitions)) } for j, cell := range row.Cells { // do not add to this test without discussion - may break clients switch cell.(type) { case float64, int64, int32, int, string, bool: case []interface{}: + case nil: default: t.Errorf("row %d, cell %d has an unrecognized type, only JSON serialization safe types are allowed: %T ", i, j, cell) }