Partial extension API server store + control over printed columns (#432)

* Checkpoint

* Add support for custom columns

* Remove old Store and Delegate abstraction

* Fix nits and rewording

* Remove unused mock file

* Update documentation for extension api server

* Remove the need for scheme for ConvertListOptions

* Rename store to utils

* fixup! Remove the need for scheme for ConvertListOptions

* Move watch helper to tests

* Add convertError at a few places

* Ignore misspell on creater

* Fix comments and remove unused params

* Add convertError to missing error returns

* Fix watcher implementation

* Document request.UserFrom and request.NamespaceFrom

pkg/ext/apiserver_test.go

@@ -11,11 +11,11 @@ import (
 	"net/http/httptest"
 	"sort"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -24,6 +24,7 @@ import (
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
+	regrest "k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/rest"
 )
@@ -41,83 +42,6 @@ func authzAllowAll(ctx context.Context, a authorizer.Attributes) (authorizer.Dec
 	return authorizer.DecisionAllow, "", nil
 }
 
-type mapStore struct {
-	items  map[string]*TestType
-	events chan WatchEvent[*TestType]
-}
-
-func newMapStore() *mapStore {
-	return &mapStore{
-		items:  make(map[string]*TestType),
-		events: make(chan WatchEvent[*TestType], 100),
-	}
-}
-
-func (t *mapStore) Create(ctx Context, obj *TestType, opts *metav1.CreateOptions) (*TestType, error) {
-	if _, found := t.items[obj.Name]; found {
-		return nil, apierrors.NewAlreadyExists(ctx.GroupVersionResource.GroupResource(), obj.Name)
-	}
-	t.items[obj.Name] = obj
-	t.events <- WatchEvent[*TestType]{
-		Event:  watch.Added,
-		Object: obj,
-	}
-	return obj, nil
-}
-
-func (t *mapStore) Update(ctx Context, obj *TestType, opts *metav1.UpdateOptions) (*TestType, error) {
-	if _, found := t.items[obj.Name]; !found {
-		return nil, apierrors.NewNotFound(ctx.GroupVersionResource.GroupResource(), obj.Name)
-	}
-	obj.ManagedFields = []metav1.ManagedFieldsEntry{}
-	t.items[obj.Name] = obj
-	t.events <- WatchEvent[*TestType]{
-		Event:  watch.Modified,
-		Object: obj,
-	}
-	return obj, nil
-}
-
-func (t *mapStore) Get(ctx Context, name string, opts *metav1.GetOptions) (*TestType, error) {
-	obj, found := t.items[name]
-	if !found {
-		return nil, apierrors.NewNotFound(ctx.GroupVersionResource.GroupResource(), name)
-	}
-	return obj, nil
-}
-
-func (t *mapStore) List(ctx Context, opts *metav1.ListOptions) (*TestTypeList, error) {
-	items := []TestType{}
-	for _, obj := range t.items {
-		items = append(items, *obj)
-	}
-	sort.Slice(items, func(i, j int) bool {
-		return items[i].Name > items[j].Name
-	})
-	list := &TestTypeList{
-		Items: items,
-	}
-	return list, nil
-}
-
-func (t *mapStore) Watch(ctx Context, opts *metav1.ListOptions) (<-chan WatchEvent[*TestType], error) {
-	return t.events, nil
-}
-
-func (t *mapStore) Delete(ctx Context, name string, opts *metav1.DeleteOptions) error {
-	obj, found := t.items[name]
-	if !found {
-		return apierrors.NewNotFound(ctx.GroupVersionResource.GroupResource(), name)
-	}
-
-	delete(t.items, name)
-	t.events <- WatchEvent[*TestType]{
-		Event:  watch.Deleted,
-		Object: obj,
-	}
-	return nil
-}
-
 func TestStore(t *testing.T) {
 	scheme := runtime.NewScheme()
 	AddToScheme(scheme)
@@ -128,8 +52,10 @@ func TestStore(t *testing.T) {
 	ln, err := (&net.ListenConfig{}).Listen(ctx, "tcp", ":0")
 	require.NoError(t, err)
 
-	store := newMapStore()
-	extensionAPIServer, cleanup, err := setupExtensionAPIServer(t, scheme, &TestType{}, &TestTypeList{}, store, func(opts *ExtensionAPIServerOptions) {
+	store := newDefaultTestStore()
+	store.items = make(map[string]*TestType)
+
+	extensionAPIServer, cleanup, err := setupExtensionAPIServer(t, scheme, store, func(opts *ExtensionAPIServerOptions) {
 		opts.Listener = ln
 		opts.Authorizer = authorizer.AuthorizerFunc(authzAllowAll)
 		opts.Authenticator = authenticator.RequestFunc(authAsAdmin)
@@ -298,36 +224,48 @@ func TestStore(t *testing.T) {
 	}
 }
 
-var _ Store[*TestTypeOther, *TestTypeOtherList] = (*testStoreOther)(nil)
-
-// This store is meant to be able to test many stores
-type testStoreOther struct {
+// This store tests when there's only a subset of verbs supported
+type partialStorage struct {
+	gvk schema.GroupVersionKind
 }
 
-func (t *testStoreOther) Create(ctx Context, obj *TestTypeOther, opts *metav1.CreateOptions) (*TestTypeOther, error) {
-	return &testTypeOtherFixture, nil
+// New implements [regrest.Storage]
+func (t *partialStorage) New() runtime.Object {
+	obj := &TestType{}
+	obj.GetObjectKind().SetGroupVersionKind(t.gvk)
+	return obj
 }
 
-func (t *testStoreOther) Update(ctx Context, obj *TestTypeOther, opts *metav1.UpdateOptions) (*TestTypeOther, error) {
-	return &testTypeOtherFixture, nil
+// Destroy implements [regrest.Storage]
+func (t *partialStorage) Destroy() {
 }
 
-func (t *testStoreOther) Get(ctx Context, name string, opts *metav1.GetOptions) (*TestTypeOther, error) {
-	return &testTypeOtherFixture, nil
+// GetSingularName implements [regrest.SingularNameProvider]
+func (t *partialStorage) GetSingularName() string {
+	return "testtype"
 }
 
-func (t *testStoreOther) List(ctx Context, opts *metav1.ListOptions) (*TestTypeOtherList, error) {
-	return &testTypeOtherListFixture, nil
+// NamespaceScoped implements [regrest.Scoper]
+func (t *partialStorage) NamespaceScoped() bool {
+	return false
 }
 
-func (t *testStoreOther) Watch(ctx Context, opts *metav1.ListOptions) (<-chan WatchEvent[*TestTypeOther], error) {
+// GroupVersionKind implements [regrest.GroupVersionKindProvider]
+func (t *partialStorage) GroupVersionKind(_ schema.GroupVersion) schema.GroupVersionKind {
+	return t.gvk
+}
+
+func (s *partialStorage) Create(ctx context.Context, obj runtime.Object, createValidation regrest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
+	if createValidation != nil {
+		err := createValidation(ctx, obj)
+		if err != nil {
+			return obj, err
+		}
+	}
+
 	return nil, nil
 }
 
-func (t *testStoreOther) Delete(ctx Context, name string, opts *metav1.DeleteOptions) error {
-	return nil
-}
-
 // The POC had a bug where multiple resources couldn't be installed so we're
 // testing this here
 func TestDiscoveryAndOpenAPI(t *testing.T) {
@@ -343,35 +281,67 @@ func TestDiscoveryAndOpenAPI(t *testing.T) {
 		Group:   "ext2.cattle.io",
 		Version: "v3",
 	}
+
+	partialGroupVersion := schema.GroupVersion{
+		Group:   "ext.cattle.io",
+		Version: "v4",
+	}
 	scheme.AddKnownTypes(differentVersion, &TestType{}, &TestTypeList{})
 	scheme.AddKnownTypes(differentGroupVersion, &TestType{}, &TestTypeList{})
+	scheme.AddKnownTypes(partialGroupVersion, &TestType{}, &TestTypeList{})
 	metav1.AddToGroupVersion(scheme, differentVersion)
 	metav1.AddToGroupVersion(scheme, differentGroupVersion)
+	metav1.AddToGroupVersion(scheme, partialGroupVersion)
 
 	ln, err := (&net.ListenConfig{}).Listen(context.Background(), "tcp", ":0")
 	require.NoError(t, err)
 
-	store := &testStore{}
-	extensionAPIServer, cleanup, err := setupExtensionAPIServer(t, scheme, &TestType{}, &TestTypeList{}, store, func(opts *ExtensionAPIServerOptions) {
+	store := newDefaultTestStore()
+	extensionAPIServer, cleanup, err := setupExtensionAPIServer(t, scheme, store, func(opts *ExtensionAPIServerOptions) {
 		opts.Listener = ln
 		opts.Authorizer = authorizer.AuthorizerFunc(authzAllowAll)
 		opts.Authenticator = authenticator.RequestFunc(authAsAdmin)
 	}, func(s *ExtensionAPIServer) error {
-		store := &testStoreOther{}
-		err := InstallStore(s, &TestTypeOther{}, &TestTypeOtherList{}, "testtypeothers", "testtypeother", testTypeGV.WithKind("TestTypeOther"), store)
+		err = s.Install("testtypeothers", testTypeGV.WithKind("TestTypeOther"), &testStore[*TestTypeOther, *TestTypeOtherList]{
+			singular: "testtypeother",
+			objT:     &TestTypeOther{},
+			objListT: &TestTypeOtherList{},
+			gvk:      testTypeGV.WithKind("TestTypeOther"),
+			gvr:      schema.GroupVersionResource{Group: testTypeGV.Group, Version: testTypeGV.Version, Resource: "testtypes"},
+		})
 		if err != nil {
 			return err
 		}
 
-		err = InstallStore(s, &TestType{}, &TestTypeList{}, "testtypes", "testtype", differentVersion.WithKind("TestType"), &testStore{})
+		err = s.Install("testtypes", differentVersion.WithKind("TestType"), &testStore[*TestType, *TestTypeList]{
+			singular: "testtype",
+			objT:     &TestType{},
+			objListT: &TestTypeList{},
+			gvk:      differentVersion.WithKind("TestType"),
+			gvr:      schema.GroupVersionResource{Group: differentVersion.Group, Version: differentVersion.Version, Resource: "testtypes"},
+		})
 		if err != nil {
 			return err
 		}
 
-		err = InstallStore(s, &TestType{}, &TestTypeList{}, "testtypes", "testtype", differentGroupVersion.WithKind("TestType"), &testStore{})
+		err = s.Install("testtypes", differentGroupVersion.WithKind("TestType"), &testStore[*TestType, *TestTypeList]{
+			singular: "testtype",
+			objT:     &TestType{},
+			objListT: &TestTypeList{},
+			gvk:      differentGroupVersion.WithKind("TestType"),
+			gvr:      schema.GroupVersionResource{Group: differentGroupVersion.Group, Version: differentVersion.Version, Resource: "testtypes"},
+		})
 		if err != nil {
 			return err
 		}
+
+		err = s.Install("testtypes", partialGroupVersion.WithKind("TestType"), &partialStorage{
+			gvk: partialGroupVersion.WithKind("TestType"),
+		})
+		if err != nil {
+			return err
+		}
+
 		return nil
 	})
 	require.NoError(t, err)
@@ -401,6 +371,10 @@ func TestDiscoveryAndOpenAPI(t *testing.T) {
 						{
 							Name: "ext.cattle.io",
 							Versions: []metav1.GroupVersionForDiscovery{
+								{
+									GroupVersion: "ext.cattle.io/v4",
+									Version:      "v4",
+								},
 								{
 									GroupVersion: "ext.cattle.io/v2",
 									Version:      "v2",
@@ -450,6 +424,10 @@ func TestDiscoveryAndOpenAPI(t *testing.T) {
 						GroupVersion: "ext.cattle.io/v2",
 						Version:      "v2",
 					},
+					{
+						GroupVersion: "ext.cattle.io/v4",
+						Version:      "v4",
+					},
 					{
 						GroupVersion: "ext.cattle.io/v1",
 						Version:      "v1",
@@ -569,6 +547,32 @@ func TestDiscoveryAndOpenAPI(t *testing.T) {
 				},
 			},
 		},
+		{
+			path:               "/apis/ext.cattle.io/v4",
+			got:                &metav1.APIResourceList{},
+			expectedStatusCode: http.StatusOK,
+			expectedBody: &metav1.APIResourceList{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "APIResourceList",
+					APIVersion: "v1",
+				},
+				GroupVersion: "ext.cattle.io/v4",
+				APIResources: []metav1.APIResource{
+					{
+						Name:         "testtypes",
+						SingularName: "testtype",
+						Namespaced:   false,
+						Kind:         "TestType",
+						Group:        "ext.cattle.io",
+						Version:      "v4",
+						// Only the create verb is supported for this store
+						Verbs: metav1.Verbs{
+							"create",
+						},
+					},
+				},
+			},
+		},
 		{
 			path:               "/openapi/v2",
 			expectedStatusCode: http.StatusOK,
@@ -664,15 +668,29 @@ func TestNoStore(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func setupExtensionAPIServer[
-	T runtime.Object,
-	TList runtime.Object,
-](
+func setupExtensionAPIServer(
+	t *testing.T,
+	scheme *runtime.Scheme,
+	store regrest.Storage,
+	optionSetter func(*ExtensionAPIServerOptions),
+	extensionAPIServerSetter func(*ExtensionAPIServer) error,
+) (*ExtensionAPIServer, func(), error) {
+	fn := func(e *ExtensionAPIServer) error {
+		err := e.Install("testtypes", testTypeGV.WithKind("TestType"), store)
+		if err != nil {
+			return fmt.Errorf("InstallStore: %w", err)
+		}
+		if extensionAPIServerSetter != nil {
+			return extensionAPIServerSetter(e)
+		}
+		return nil
+	}
+	return setupExtensionAPIServerNoStore(t, scheme, optionSetter, fn)
+}
+
+func setupExtensionAPIServerNoStore(
 	t *testing.T,
 	scheme *runtime.Scheme,
-	objT T,
-	objTList TList,
-	store Store[T, TList],
 	optionSetter func(*ExtensionAPIServerOptions),
 	extensionAPIServerSetter func(*ExtensionAPIServer) error,
 ) (*ExtensionAPIServer, func(), error) {
@@ -694,11 +712,6 @@ func setupExtensionAPIServer[
 		return nil, func() {}, err
 	}
 
-	err = InstallStore(extensionAPIServer, objT, objTList, "testtypes", "testtype", testTypeGV.WithKind("TestType"), store)
-	if err != nil {
-		return nil, func() {}, fmt.Errorf("InstallStore: %w", err)
-	}
-
 	if extensionAPIServerSetter != nil {
 		err = extensionAPIServerSetter(extensionAPIServer)
 		if err != nil {
@@ -768,3 +781,222 @@ func createRecordingWatcher(scheme *runtime.Scheme, gvr schema.GroupVersionResou
 		stop: myWatch.Stop,
 	}, nil
 }
+
+// This store tests the printed columns functionality
+type customColumnsStore struct {
+	*testStore[*TestType, *TestTypeList]
+
+	lock      sync.Mutex
+	columns   []metav1.TableColumnDefinition
+	convertFn func(obj *TestType) []string
+}
+
+func (s *customColumnsStore) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+	return ConvertToTable(ctx, object, tableOptions, s.testStore.gvr.GroupResource(), s.columns, s.convertFn)
+}
+
+func (s *customColumnsStore) Set(columns []metav1.TableColumnDefinition, convertFn func(obj *TestType) []string) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+	s.columns = columns
+	s.convertFn = convertFn
+}
+
+func TestCustomColumns(t *testing.T) {
+	scheme := runtime.NewScheme()
+	AddToScheme(scheme)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	ln, err := (&net.ListenConfig{}).Listen(ctx, "tcp", ":0")
+	require.NoError(t, err)
+
+	store := &customColumnsStore{
+		testStore: newDefaultTestStore(),
+	}
+
+	extensionAPIServer, cleanup, err := setupExtensionAPIServerNoStore(t, scheme, func(opts *ExtensionAPIServerOptions) {
+		opts.Listener = ln
+		opts.Authorizer = authorizer.AuthorizerFunc(authzAllowAll)
+		opts.Authenticator = authenticator.RequestFunc(authAsAdmin)
+	}, func(s *ExtensionAPIServer) error {
+		err := s.Install("testtypes", testTypeGV.WithKind("TestType"), store)
+		if err != nil {
+			return err
+		}
+		return nil
+	})
+
+	require.NoError(t, err)
+	defer cleanup()
+
+	ts := httptest.NewServer(extensionAPIServer)
+	defer ts.Close()
+
+	createRequest := func(path string) *http.Request {
+		req := httptest.NewRequest(http.MethodGet, path, nil)
+		// This asks the apiserver to give back a metav1.Table for List and Get operations
+		req.Header.Add("Accept", "application/json;as=Table;v=v1;g=meta.k8s.io")
+		return req
+	}
+
+	columns := []metav1.TableColumnDefinition{
+		{
+			Name: "Name",
+			Type: "name",
+		},
+		{
+			Name: "Foo",
+			Type: "string",
+		},
+		{
+			Name: "Bar",
+			Type: "number",
+		},
+	}
+	convertFn := func(obj *TestType) []string {
+		return []string{
+			"the name is " + obj.GetName(),
+			"the foo value",
+			"the bar value",
+		}
+	}
+
+	tests := []struct {
+		name               string
+		requests           []*http.Request
+		columns            []metav1.TableColumnDefinition
+		convertFn          func(obj *TestType) []string
+		expectedStatusCode int
+		expectedBody       any
+	}{
+		{
+			name: "default",
+			requests: []*http.Request{
+				createRequest("/apis/ext.cattle.io/v1/testtypes"),
+				createRequest("/apis/ext.cattle.io/v1/testtypes/foo"),
+			},
+			expectedStatusCode: http.StatusOK,
+			expectedBody: &metav1.Table{
+				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1"},
+				ColumnDefinitions: []metav1.TableColumnDefinition{
+					{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"},
+					{Name: "Created At", Type: "date", Description: "CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC.\n\nPopulated by the system. Read-only. Null for lists. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata"},
+				},
+				Rows: []metav1.TableRow{
+					{
+						Cells: []any{"foo", "0001-01-01T00:00:00Z"},
+						Object: runtime.RawExtension{
+							Raw: []byte(`{"kind":"PartialObjectMetadata","apiVersion":"meta.k8s.io/v1","metadata":{"name":"foo","creationTimestamp":null}}`),
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "custom include object default and metadata",
+			requests: []*http.Request{
+				createRequest("/apis/ext.cattle.io/v1/testtypes"),
+				createRequest("/apis/ext.cattle.io/v1/testtypes/foo"),
+				createRequest("/apis/ext.cattle.io/v1/testtypes?includeObject=Metadata"),
+				createRequest("/apis/ext.cattle.io/v1/testtypes/foo?includeObject=Metadata"),
+			},
+			columns:            columns,
+			convertFn:          convertFn,
+			expectedStatusCode: http.StatusOK,
+			expectedBody: &metav1.Table{
+				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1"},
+				ColumnDefinitions: []metav1.TableColumnDefinition{
+					{Name: "Name", Type: "name"},
+					{Name: "Foo", Type: "string"},
+					{Name: "Bar", Type: "number"},
+				},
+				Rows: []metav1.TableRow{
+					{
+						Cells: []any{"the name is foo", "the foo value", "the bar value"},
+						Object: runtime.RawExtension{
+							Raw: []byte(`{"kind":"PartialObjectMetadata","apiVersion":"meta.k8s.io/v1","metadata":{"name":"foo","creationTimestamp":null}}`),
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "custom include object None",
+			requests: []*http.Request{
+				createRequest("/apis/ext.cattle.io/v1/testtypes?includeObject=None"),
+				createRequest("/apis/ext.cattle.io/v1/testtypes/foo?includeObject=None"),
+			},
+			columns:            columns,
+			convertFn:          convertFn,
+			expectedStatusCode: http.StatusOK,
+			expectedBody: &metav1.Table{
+				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1"},
+				ColumnDefinitions: []metav1.TableColumnDefinition{
+					{Name: "Name", Type: "name"},
+					{Name: "Foo", Type: "string"},
+					{Name: "Bar", Type: "number"},
+				},
+				Rows: []metav1.TableRow{
+					{
+						Cells: []any{"the name is foo", "the foo value", "the bar value"},
+					},
+				},
+			},
+		},
+		{
+			name: "custom include object Object",
+			requests: []*http.Request{
+				createRequest("/apis/ext.cattle.io/v1/testtypes?includeObject=Object"),
+				createRequest("/apis/ext.cattle.io/v1/testtypes/foo?includeObject=Object"),
+			},
+			columns:            columns,
+			convertFn:          convertFn,
+			expectedStatusCode: http.StatusOK,
+			expectedBody: &metav1.Table{
+				TypeMeta: metav1.TypeMeta{Kind: "Table", APIVersion: "meta.k8s.io/v1"},
+				ColumnDefinitions: []metav1.TableColumnDefinition{
+					{Name: "Name", Type: "name"},
+					{Name: "Foo", Type: "string"},
+					{Name: "Bar", Type: "number"},
+				},
+				Rows: []metav1.TableRow{
+					{
+						Cells: []any{"the name is foo", "the foo value", "the bar value"},
+						Object: runtime.RawExtension{
+							Raw: []byte(`{"kind":"TestType","apiVersion":"ext.cattle.io/v1","metadata":{"name":"foo","creationTimestamp":null}}`),
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.columns != nil {
+				store.Set(test.columns, test.convertFn)
+			}
+
+			for _, req := range test.requests {
+				w := httptest.NewRecorder()
+
+				extensionAPIServer.ServeHTTP(w, req)
+
+				resp := w.Result()
+				body, _ := io.ReadAll(resp.Body)
+
+				require.Equal(t, test.expectedStatusCode, resp.StatusCode)
+				if test.expectedBody != nil {
+					table := &metav1.Table{}
+					err = json.Unmarshal(body, table)
+					require.NoError(t, err)
+					require.Equal(t, test.expectedBody, table)
+				}
+			}
+		})
+	}
+
+}