From 2905e9b7160dfb75dbfa33f1d877d67eeb38fb74 Mon Sep 17 00:00:00 2001 From: zhangshipei Date: Tue, 26 May 2026 11:52:47 +0800 Subject: [PATCH] feat: render CRD additionalPrinterColumns for custom resources Custom resources were served to `kubectl get` as unstructured objects and rendered with the built-in table generator, which only knows core/apps/batch types. As a result any CR (e.g. Volcano PodGroup, Velero BackupStorageLocation) showed only NAME/AGE and `-o wide` added nothing, unlike a live apiserver. When the bundle ships the matching CustomResourceDefinition, look up its additionalPrinterColumns and render the table via the apiextensions tableconvertor, so `kubectl get ` and `-o wide` show the same columns a live cluster would. Falls back to the existing NAME/AGE table when no CRD matches or it declares no extra columns. Only v1 CRDs are supported; v1beta1 bundles fall through unchanged. Adds an integration test exercising the velero.io BackupStorageLocation CRD fixture (Phase / Last Validated / Default columns). --- pkg/api/server.go | 109 +++++++++++++++--- tests/backupstoragelocations_test.go | 35 ++++++ ...ckupstoragelocations_velero_namespace.json | 1 + .../velero.json | 20 ++++ 4 files changed, 151 insertions(+), 14 deletions(-) create mode 100644 tests/backupstoragelocations_test.go create mode 100644 tests/results/backupstoragelocations_velero_namespace.json create mode 100644 tests/support-bundle/cluster-resources/custom-resources/backupstoragelocations.velero.io/velero.json diff --git a/pkg/api/server.go b/pkg/api/server.go index 4516809..2fb0bbc 100644 --- a/pkg/api/server.go +++ b/pkg/api/server.go @@ -31,6 +31,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" storagev1 "k8s.io/api/storage/v1" extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -393,7 +394,7 @@ func (h handler) getAPIV1ClusterResources(w http.ResponseWriter, r *http.Request } if asTable { - table, err := toTable(result, r) + table, err := h.toTable(result, r) if err != nil { log.Error("could not convert to table: ", err) } else { @@ -519,7 +520,7 @@ func (h handler) getAPIV1NamespaceResources(w http.ResponseWriter, r *http.Reque } if asTable { - table, err := toTable(decoded, r) + table, err := h.toTable(decoded, r) if err != nil { log.Warn("could not convert to table: ", err) } else { @@ -629,7 +630,7 @@ func (h handler) getAPIV1NamespaceResource(w http.ResponseWriter, r *http.Reques } if asTable { - table, err := toTable(result, r) + table, err := h.toTable(result, r) if err != nil { log.Warn("could not convert to table: ", err) } else { @@ -998,7 +999,7 @@ func (h handler) getAPIsClusterResources(w http.ResponseWriter, r *http.Request) decoded = list } - table, err := toTable(decoded, r) + table, err := h.toTable(decoded, r) if err != nil { log.Warn("could not convert to table:", err) } else { @@ -1065,7 +1066,7 @@ func (h handler) getAPIsClusterResources(w http.ResponseWriter, r *http.Request) result = list } - table, err := toTable(result, r) + table, err := h.toTable(result, r) if err != nil { log.Warn("could not convert to table:", err) } else { @@ -1085,7 +1086,7 @@ func (h handler) getAPIsClusterResource(w http.ResponseWriter, r *http.Request) asTable := strings.Contains(r.Header.Get("Accept"), "as=Table") // who needs parsing setResponse := func(d runtime.Object) { if asTable { - table, err := toTable(d, r) + table, err := h.toTable(d, r) if err != nil { log.Warn("could not convert to table: ", err) } else { @@ -1193,7 +1194,7 @@ func (h handler) getAPIsNamespaceResources(w http.ResponseWriter, r *http.Reques } if asTable { - table, err := toTable(decoded, r) + table, err := h.toTable(decoded, r) if err != nil { log.Warn("could not convert to table: ", err) } else { @@ -1218,7 +1219,7 @@ func (h handler) getAPIsNamespaceResource(w http.ResponseWriter, r *http.Request setResponse := func(d runtime.Object) { if asTable { - table, err := toTable(d, r) + table, err := h.toTable(d, r) if err != nil { log.Warn("could not convert to table: ", err) } else { @@ -1538,7 +1539,62 @@ func podToSelectableFields(pod *corev1.Pod) fields.Set { return generic.MergeFieldsSets(specificFieldsSet, objectMetaFieldsSet) } -func toTable(object runtime.Object, r *http.Request) (runtime.Object, error) { +// crdPrinterColumns returns the additionalPrinterColumns for the CRD that matches the +// request's group/version/resource, when the bundle ships its CustomResourceDefinition. +// Returns ok=false when no CRD matches or it declares no extra columns, so the caller +// falls back to the built-in NAME/AGE table. +func (h handler) crdPrinterColumns(r *http.Request) ([]extensionsv1.CustomResourceColumnDefinition, bool) { + group := mux.Vars(r)["group"] + version := mux.Vars(r)["version"] + resource := mux.Vars(r)["resource"] + if group == "" || resource == "" { + return nil, false + } + + fileName := filepath.Join( + h.clusterData.ClusterResourcesDir, + fmt.Sprintf("%s.json", sbctlutil.GetSBCompatibleResourceName("customresourcedefinitions")), + ) + data, err := readFileAndLog(fileName) + if err != nil { + // A bundle without any CRDs is normal; only surface real read errors. + if !os.IsNotExist(err) { + log.Warn("could not read CRD definitions: ", err) + } + return nil, false + } + + // Only v1 CRDs carry per-version additionalPrinterColumns; v1beta1 bundles + // fall through to the built-in NAME/AGE table. + var crdList extensionsv1.CustomResourceDefinitionList + if err := json.Unmarshal(data, &crdList); err != nil { + log.Warn("could not unmarshal CRD definitions: ", err) + return nil, false + } + + for i := range crdList.Items { + crd := &crdList.Items[i] + if crd.Spec.Group != group { + continue + } + if crd.Spec.Names.Plural != resource && crd.Spec.Names.Singular != resource { + continue + } + for j := range crd.Spec.Versions { + v := &crd.Spec.Versions[j] + if v.Name != version { + continue + } + if len(v.AdditionalPrinterColumns) == 0 { + return nil, false + } + return v.AdditionalPrinterColumns, true + } + } + return nil, false +} + +func (h handler) toTable(object runtime.Object, r *http.Request) (runtime.Object, error) { switch o := object.(type) { case *corev1.PodList: converted := &apicore.PodList{} @@ -1740,12 +1796,37 @@ func toTable(object runtime.Object, r *http.Request) (runtime.Object, error) { ctx := context.TODO() tableOptions := &metav1.TableOptions{} - tableConvertor := printerstorage.TableConvertor{ - TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers), + + var table *metav1.Table + var err error + + // Custom resources arrive as unstructured objects, which the built-in table + // generator renders with only NAME/AGE. When the bundle ships the matching CRD + // definition, render with its additionalPrinterColumns so `kubectl get -o wide` + // shows the same columns a live apiserver would. + switch object.(type) { + case *unstructured.Unstructured, *unstructured.UnstructuredList: + if cols, ok := h.crdPrinterColumns(r); ok { + if conv, cerr := tableconvertor.New(cols); cerr == nil { + table, err = conv.ConvertToTable(ctx, object, tableOptions) + if err != nil { + log.Warn("could not convert custom resource to table via CRD columns: ", err) + table = nil + } + } else { + log.Warn("could not build CRD table convertor: ", cerr) + } + } } - table, err := tableConvertor.ConvertToTable(ctx, object, tableOptions) - if err != nil { - return nil, err + + if table == nil { + tableConvertor := printerstorage.TableConvertor{ + TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers), + } + table, err = tableConvertor.ConvertToTable(ctx, object, tableOptions) + if err != nil { + return nil, err + } } // TODO: github.com/golang/gddo is no longer maintained. We should diff --git a/tests/backupstoragelocations_test.go b/tests/backupstoragelocations_test.go new file mode 100644 index 0000000..13b04f3 --- /dev/null +++ b/tests/backupstoragelocations_test.go @@ -0,0 +1,35 @@ +package tests + +import ( + _ "embed" + "fmt" + "net/http" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +//go:embed results/backupstoragelocations_velero_namespace.json +var expectedGetBackupStorageLocationsResult string + +var _ = Describe("GET /apis/velero.io/v1/namespaces/{namespace}/backupstoragelocations", func() { + Context("When getting a custom resource whose CRD defines additionalPrinterColumns", func() { + It("Renders the CRD-defined columns instead of only NAME/AGE", func() { + resp, statusCode, err := HTTPExec( + "GET", + fmt.Sprintf("%s/apis/velero.io/v1/namespaces/velero/backupstoragelocations", apiServerEndpoint), + getHeaders, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(statusCode).To(Equal(http.StatusOK)) + + // The CRD's additionalPrinterColumns must surface as table columns. Without + // them a custom resource falls back to the built-in NAME/AGE table. + Expect(resp).To(ContainSubstring(`"name":"Phase"`)) + Expect(resp).To(ContainSubstring(`"name":"Last Validated"`)) + Expect(resp).To(ContainSubstring(`"name":"Default"`)) + + Expect(resp).To(Similar(expectedGetBackupStorageLocationsResult)) + }) + }) +}) diff --git a/tests/results/backupstoragelocations_velero_namespace.json b/tests/results/backupstoragelocations_velero_namespace.json new file mode 100644 index 0000000..4bb1899 --- /dev/null +++ b/tests/results/backupstoragelocations_velero_namespace.json @@ -0,0 +1 @@ +{"kind":"Table","apiVersion":"meta.k8s.io/v1","metadata":{},"columnDefinitions":[{"name":"Name","type":"string","format":"name","description":"Name must be unique within a namespace. Is required when creating resources, although some resources may allow a client to request the generation of an appropriate name automatically. Name is primarily intended for creation idempotence and configuration definition. Cannot be updated. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names#names","priority":0},{"name":"Phase","type":"string","format":"","description":"Backup Storage Location status such as Available/Unavailable","priority":0},{"name":"Last Validated","type":"date","format":"","description":"LastValidationTime is the last time the backup store location was validated","priority":0},{"name":"Age","type":"date","format":"","description":"Custom resource definition column (in JSONPath format): .metadata.creationTimestamp","priority":0},{"name":"Default","type":"boolean","format":"","description":"Default backup storage location","priority":0}],"rows":[{"cells":["default","Available","4y44d","4y45d",true],"object":{"kind":"PartialObjectMetadata","apiVersion":"meta.k8s.io/v1","metadata":{"name":"default","namespace":"velero","creationTimestamp":"2022-04-11T22:51:33Z"}}}]} \ No newline at end of file diff --git a/tests/support-bundle/cluster-resources/custom-resources/backupstoragelocations.velero.io/velero.json b/tests/support-bundle/cluster-resources/custom-resources/backupstoragelocations.velero.io/velero.json new file mode 100644 index 0000000..8925df0 --- /dev/null +++ b/tests/support-bundle/cluster-resources/custom-resources/backupstoragelocations.velero.io/velero.json @@ -0,0 +1,20 @@ +[ + { + "apiVersion": "velero.io/v1", + "kind": "BackupStorageLocation", + "metadata": { + "name": "default", + "namespace": "velero", + "creationTimestamp": "2022-04-11T22:51:33Z" + }, + "spec": { + "default": true, + "provider": "aws", + "objectStorage": { "bucket": "velero-backups" } + }, + "status": { + "phase": "Available", + "lastValidationTime": "2022-04-12T10:00:00Z" + } + } +]