Merge pull request #55835 from smarterclayton/table_printer_meta

Automatic merge from submit-queue (batch tested with PRs 55642, 55897, 55835, 55496, 55313). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Table printers and server generation should always copy ListMeta

Tables should be a mapping from lists, so if the incoming object has these add them to the table. Paging over server side tables was broken without this. Add tests on the generic creater and on the resttest compatibility.

@deads2k

Kubernetes-commit: 941c6aa1db828c9f687780bea21f1d94319a1abe

pkg/endpoints/apiserver_test.go

@@ -441,6 +441,10 @@ func (storage *SimpleRESTStorage) ConvertToTable(ctx request.Context, obj runtim
 func (storage *SimpleRESTStorage) List(ctx request.Context, options *metainternalversion.ListOptions) (runtime.Object, error) {
 	storage.checkContext(ctx)
 	result := &genericapitesting.SimpleList{
+		ListMeta: metav1.ListMeta{
+			ResourceVersion: "10",
+			SelfLink:        "/test/link",
+		},
 		Items: storage.list,
 	}
 	storage.requestedLabelSelector = labels.Everything()
@@ -1832,24 +1836,10 @@ func TestGetPretty(t *testing.T) {
 
 func TestGetTable(t *testing.T) {
 	now := metav1.Now()
-	storage := map[string]rest.Storage{}
 	obj := genericapitesting.Simple{
-		ObjectMeta: metav1.ObjectMeta{Name: "foo1", Namespace: "ns1", CreationTimestamp: now, UID: types.UID("abcdef0123")},
+		ObjectMeta: metav1.ObjectMeta{Name: "foo1", Namespace: "ns1", ResourceVersion: "10", SelfLink: "/blah", CreationTimestamp: now, UID: types.UID("abcdef0123")},
 		Other:      "foo",
 	}
-	simpleStorage := SimpleRESTStorage{
-		item: obj,
-	}
-	selfLinker := &setTestSelfLinker{
-		t:           t,
-		expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id",
-		name:        "id",
-		namespace:   "default",
-	}
-	storage["simple"] = &simpleStorage
-	handler := handleLinker(storage, selfLinker)
-	server := httptest.NewServer(handler)
-	defer server.Close()
 
 	m, err := meta.Accessor(&obj)
 	if err != nil {
@@ -1872,15 +1862,34 @@ func TestGetTable(t *testing.T) {
 		pretty     bool
 		expected   *metav1alpha1.Table
 		statusCode int
+		item       bool
 	}{
 		{
 			accept:     runtime.ContentTypeJSON + ";as=Table;v=v1;g=meta.k8s.io",
 			statusCode: http.StatusNotAcceptable,
 		},
 		{
+			item:   true,
 			accept: runtime.ContentTypeJSON + ";as=Table;v=v1alpha1;g=meta.k8s.io",
 			expected: &metav1alpha1.Table{
 				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1alpha1"},
+				ListMeta: metav1.ListMeta{ResourceVersion: "10", SelfLink: "/blah"},
+				ColumnDefinitions: []metav1alpha1.TableColumnDefinition{
+					{Name: "Name", Type: "string", Format: "name", Description: metaDoc["name"]},
+					{Name: "Created At", Type: "date", Description: metaDoc["creationTimestamp"]},
+				},
+				Rows: []metav1alpha1.TableRow{
+					{Cells: []interface{}{"foo1", now.Time.UTC().Format(time.RFC3339)}, Object: runtime.RawExtension{Raw: encodedBody}},
+				},
+			},
+		},
+		{
+			item:   true,
+			accept: runtime.ContentTypeJSON + ";as=Table;v=v1alpha1;g=meta.k8s.io",
+			params: url.Values{"includeObject": []string{"Metadata"}},
+			expected: &metav1alpha1.Table{
+				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1alpha1"},
+				ListMeta: metav1.ListMeta{ResourceVersion: "10", SelfLink: "/blah"},
 				ColumnDefinitions: []metav1alpha1.TableColumnDefinition{
 					{Name: "Name", Type: "string", Format: "name", Description: metaDoc["name"]},
 					{Name: "Created At", Type: "date", Description: metaDoc["creationTimestamp"]},
@@ -1895,6 +1904,7 @@ func TestGetTable(t *testing.T) {
 			params: url.Values{"includeObject": []string{"Metadata"}},
 			expected: &metav1alpha1.Table{
 				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1alpha1"},
+				ListMeta: metav1.ListMeta{ResourceVersion: "10", SelfLink: "/test/link"},
 				ColumnDefinitions: []metav1alpha1.TableColumnDefinition{
 					{Name: "Name", Type: "string", Format: "name", Description: metaDoc["name"]},
 					{Name: "Created At", Type: "date", Description: metaDoc["creationTimestamp"]},
@@ -1906,7 +1916,31 @@ func TestGetTable(t *testing.T) {
 		},
 	}
 	for i, test := range tests {
-		u, err := url.Parse(server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/id")
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			storage := map[string]rest.Storage{}
+			simpleStorage := SimpleRESTStorage{
+				item: obj,
+				list: []genericapitesting.Simple{obj},
+			}
+			selfLinker := &setTestSelfLinker{
+				t:           t,
+				expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple",
+				namespace:   "default",
+			}
+			if test.item {
+				selfLinker.expectedSet += "/id"
+				selfLinker.name = "id"
+			}
+			storage["simple"] = &simpleStorage
+			handler := handleLinker(storage, selfLinker)
+			server := httptest.NewServer(handler)
+			defer server.Close()
+
+			var id string
+			if test.item {
+				id = "/id"
+			}
+			u, err := url.Parse(server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple" + id)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1924,14 +1958,13 @@ func TestGetTable(t *testing.T) {
 				}
 				obj, _, err := extractBodyObject(resp, unstructured.UnstructuredJSONScheme)
 				if err != nil {
-				t.Errorf("%d: unexpected body read error: %v", err)
-				continue
+					t.Fatalf("%d: unexpected body read error: %v", err)
 				}
 				gvk := schema.GroupVersionKind{Version: "v1", Kind: "Status"}
 				if obj.GetObjectKind().GroupVersionKind() != gvk {
-				t.Errorf("%d: unexpected error body: %#v", obj)
+					t.Fatalf("%d: unexpected error body: %#v", obj)
 				}
-			continue
+				return
 			}
 			if resp.StatusCode != http.StatusOK {
 				t.Errorf("%d: unexpected response: %#v", i, resp)
@@ -1945,6 +1978,7 @@ func TestGetTable(t *testing.T) {
 				t.Log(body)
 				t.Errorf("%d: did not match: %s", i, diff.ObjectReflectDiff(test.expected, &itemOut))
 			}
+		})
 	}
 }
 

pkg/registry/rest/resttest/resttest.go

@@ -1291,10 +1291,21 @@ func (t *Tester) testListTableConversion(obj runtime.Object, assignFn AssignFunc
 		t.Errorf("expected: %#v, got: %#v", objs, items)
 	}
 
+	m, err := meta.ListAccessor(listObj)
+	if err != nil {
+		t.Fatalf("list should support ListMeta %T: %v", listObj, err)
+	}
+	m.SetContinue("continuetoken")
+	m.SetResourceVersion("11")
+	m.SetSelfLink("/list/link")
+
 	table, err := t.storage.(rest.TableConvertor).ConvertToTable(ctx, listObj, nil)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
+	if table.ResourceVersion != "11" || table.SelfLink != "/list/link" || table.Continue != "continuetoken" {
+		t.Errorf("printer lost list meta: %#v", table.ListMeta)
+	}
 	if len(table.Rows) != len(items) {
 		t.Errorf("unexpected number of rows: %v", len(table.Rows))
 	}

pkg/registry/rest/table.go

@@ -63,6 +63,16 @@ func (c defaultTableConvertor) ConvertToTable(ctx genericapirequest.Context, obj
 			return nil, err
 		}
 	}
+	if m, err := meta.ListAccessor(object); err == nil {
+		table.ResourceVersion = m.GetResourceVersion()
+		table.SelfLink = m.GetSelfLink()
+		table.Continue = m.GetContinue()
+	} else {
+		if m, err := meta.CommonAccessor(object); err == nil {
+			table.ResourceVersion = m.GetResourceVersion()
+			table.SelfLink = m.GetSelfLink()
+		}
+	}
 	table.ColumnDefinitions = []metav1alpha1.TableColumnDefinition{
 		{Name: "Name", Type: "string", Format: "name", Description: swaggerMetadataDescriptions["name"]},
 		{Name: "Created At", Type: "date", Description: swaggerMetadataDescriptions["creationTimestamp"]},