Add no-op list support to token credential request

This allows us to keep all of our resources in the pinniped category
while not having kubectl return errors for calls such as:

kubectl get pinniped -A

Signed-off-by: Monis Khan <mok@vmware.com>

internal/concierge/apiserver/apiserver.go

@@ -71,7 +71,7 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 	}
 
 	gvr := c.ExtraConfig.GroupVersion.WithResource("tokencredentialrequests")
-	storage := credentialrequest.NewREST(c.ExtraConfig.Authenticator, c.ExtraConfig.Issuer)
+	storage := credentialrequest.NewREST(c.ExtraConfig.Authenticator, c.ExtraConfig.Issuer, gvr.GroupResource())
 	if err := s.GenericAPIServer.InstallAPIGroup(&genericapiserver.APIGroupInfo{
 		PrioritizedVersions:          []schema.GroupVersion{gvr.GroupVersion()},
 		VersionedResourcesStorageMap: map[string]map[string]rest.Storage{gvr.Version: {gvr.Resource: storage}},

internal/registry/credentialrequest/rest.go

@@ -11,8 +11,10 @@ import (
 	"time"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/registry/rest"
@@ -32,16 +34,18 @@ type TokenCredentialRequestAuthenticator interface {
 	AuthenticateTokenCredentialRequest(ctx context.Context, req *loginapi.TokenCredentialRequest) (user.Info, error)
 }
 
-func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer CertIssuer) *REST {
+func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer CertIssuer, resource schema.GroupResource) *REST {
 	return &REST{
-		authenticator: authenticator,
+		authenticator:  authenticator,
-		issuer:        issuer,
+		issuer:         issuer,
+		tableConvertor: rest.NewDefaultTableConvertor(resource),
 	}
 }
 
 type REST struct {
-	authenticator TokenCredentialRequestAuthenticator
+	authenticator  TokenCredentialRequestAuthenticator
-	issuer        CertIssuer
+	issuer         CertIssuer
+	tableConvertor rest.TableConvertor
 }
 
 // Assert that our *REST implements all the optional interfaces that we expect it to implement.
@@ -51,12 +55,30 @@ var _ interface {
 	rest.Scoper
 	rest.Storage
 	rest.CategoriesProvider
+	rest.Lister
 } = (*REST)(nil)
 
 func (*REST) New() runtime.Object {
 	return &loginapi.TokenCredentialRequest{}
 }
 
+func (*REST) NewList() runtime.Object {
+	return &loginapi.TokenCredentialRequestList{}
+}
+
+func (*REST) List(_ context.Context, _ *metainternalversion.ListOptions) (runtime.Object, error) {
+	return &loginapi.TokenCredentialRequestList{
+		ListMeta: metav1.ListMeta{
+			ResourceVersion: "0", // this resource version means "from the API server cache"
+		},
+		Items: []loginapi.TokenCredentialRequest{}, // avoid sending nil items list
+	}, nil
+}
+
+func (r *REST) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
+	return r.tableConvertor.ConvertToTable(ctx, obj, tableOptions)
+}
+
 func (*REST) NamespaceScoped() bool {
 	return true
 }

internal/registry/credentialrequest/rest_test.go

@@ -17,6 +17,7 @@ import (
 	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"
 	"k8s.io/apiserver/pkg/authentication/user"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
@@ -28,11 +29,34 @@ import (
 )
 
 func TestNew(t *testing.T) {
-	r := NewREST(nil, nil)
+	r := NewREST(nil, nil, schema.GroupResource{Group: "bears", Resource: "panda"})
 	require.NotNil(t, r)
 	require.True(t, r.NamespaceScoped())
 	require.Equal(t, []string{"pinniped"}, r.Categories())
 	require.IsType(t, &loginapi.TokenCredentialRequest{}, r.New())
+	require.IsType(t, &loginapi.TokenCredentialRequestList{}, r.NewList())
+
+	ctx := context.Background()
+
+	// check the simple invariants of our no-op list
+	list, err := r.List(ctx, nil)
+	require.NoError(t, err)
+	require.NotNil(t, list)
+	require.IsType(t, &loginapi.TokenCredentialRequestList{}, list)
+	require.Equal(t, "0", list.(*loginapi.TokenCredentialRequestList).ResourceVersion)
+	require.NotNil(t, list.(*loginapi.TokenCredentialRequestList).Items)
+	require.Len(t, list.(*loginapi.TokenCredentialRequestList).Items, 0)
+
+	// make sure we can turn lists into tables if needed
+	table, err := r.ConvertToTable(ctx, list, nil)
+	require.NoError(t, err)
+	require.NotNil(t, table)
+	require.Equal(t, "0", table.ResourceVersion)
+	require.Nil(t, table.Rows)
+
+	// exercise group resource - force error by passing a runtime.Object that does not have an embedded object meta
+	_, err = r.ConvertToTable(ctx, &metav1.APIGroup{}, nil)
+	require.Error(t, err, "the resource panda.bears does not support being converted to a Table")
 }
 
 func TestCreate(t *testing.T) {
@@ -73,7 +97,7 @@ func TestCreate(t *testing.T) {
 				5*time.Minute,
 			).Return([]byte("test-cert"), []byte("test-key"), nil)
 
-			storage := NewREST(requestAuthenticator, issuer)
+			storage := NewREST(requestAuthenticator, issuer, schema.GroupResource{})
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -112,7 +136,7 @@ func TestCreate(t *testing.T) {
 				IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()).
 				Return(nil, nil, fmt.Errorf("some certificate authority error"))
 
-			storage := NewREST(requestAuthenticator, issuer)
+			storage := NewREST(requestAuthenticator, issuer, schema.GroupResource{})
 
 			response, err := callCreate(context.Background(), storage, req)
 			requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
@@ -125,7 +149,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl)
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).Return(nil, nil)
 
-			storage := NewREST(requestAuthenticator, nil)
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{})
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -140,7 +164,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
 				Return(nil, errors.New("some webhook error"))
 
-			storage := NewREST(requestAuthenticator, nil)
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{})
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -155,7 +179,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
 				Return(&user.DefaultInfo{Name: ""}, nil)
 
-			storage := NewREST(requestAuthenticator, nil)
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{})
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -165,7 +189,7 @@ func TestCreate(t *testing.T) {
 
 		it("CreateFailsWhenGivenTheWrongInputType", func() {
 			notACredentialRequest := runtime.Unknown{}
-			response, err := NewREST(nil, nil).Create(
+			response, err := NewREST(nil, nil, schema.GroupResource{}).Create(
 				genericapirequest.NewContext(),
 				&notACredentialRequest,
 				rest.ValidateAllObjectFunc,
@@ -176,7 +200,7 @@ func TestCreate(t *testing.T) {
 		})
 
 		it("CreateFailsWhenTokenValueIsEmptyInRequest", func() {
-			storage := NewREST(nil, nil)
+			storage := NewREST(nil, nil, schema.GroupResource{})
 			response, err := callCreate(context.Background(), storage, credentialRequest(loginapi.TokenCredentialRequestSpec{
 				Token: "",
 			}))
@@ -187,7 +211,7 @@ func TestCreate(t *testing.T) {
 		})
 
 		it("CreateFailsWhenValidationFails", func() {
-			storage := NewREST(nil, nil)
+			storage := NewREST(nil, nil, schema.GroupResource{})
 			response, err := storage.Create(
 				context.Background(),
 				validCredentialRequest(),
@@ -207,7 +231,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req.DeepCopy()).
 				Return(&user.DefaultInfo{Name: "test-user"}, nil)
 
-			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl))
+			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{})
 			response, err := storage.Create(
 				context.Background(),
 				req,
@@ -228,7 +252,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req.DeepCopy()).
 				Return(&user.DefaultInfo{Name: "test-user"}, nil)
 
-			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl))
+			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{})
 			validationFunctionWasCalled := false
 			var validationFunctionSawTokenValue string
 			response, err := storage.Create(
@@ -248,7 +272,7 @@ func TestCreate(t *testing.T) {
 		})
 
 		it("CreateFailsWhenRequestOptionsDryRunIsNotEmpty", func() {
-			response, err := NewREST(nil, nil).Create(
+			response, err := NewREST(nil, nil, schema.GroupResource{}).Create(
 				genericapirequest.NewContext(),
 				validCredentialRequest(),
 				rest.ValidateAllObjectFunc,

test/integration/category_test.go

@@ -0,0 +1,96 @@
+// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package integration
+
+import (
+	"bytes"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.pinniped.dev/test/library"
+)
+
+func TestGetPinnipedCategory(t *testing.T) {
+	env := library.IntegrationEnv(t)
+	dotSuffix := "." + env.APIGroupSuffix
+
+	t.Run("category, no special params", func(t *testing.T) {
+		var stdOut, stdErr bytes.Buffer
+
+		cmd := exec.Command("kubectl", "get", "pinniped", "-A")
+		cmd.Stdout = &stdOut
+		cmd.Stderr = &stdErr
+		err := cmd.Run()
+		require.NoError(t, err, stdErr.String(), stdOut.String())
+		require.Empty(t, stdErr.String())
+
+		require.NotContains(t, stdOut.String(), "MethodNotAllowed")
+		require.Contains(t, stdOut.String(), dotSuffix)
+	})
+
+	t.Run("category, table params", func(t *testing.T) {
+		var stdOut, stdErr bytes.Buffer
+
+		cmd := exec.Command("kubectl", "get", "pinniped", "-A", "-o", "wide", "-v", "10")
+		cmd.Stdout = &stdOut
+		cmd.Stderr = &stdErr
+		err := cmd.Run()
+		require.NoError(t, err, stdErr.String(), stdOut.String())
+
+		require.NotContains(t, stdOut.String(), "MethodNotAllowed")
+		require.Contains(t, stdOut.String(), dotSuffix)
+
+		require.Contains(t, stdErr.String(), `"kind":"Table"`)
+		require.Contains(t, stdErr.String(), `"resourceVersion":"0"`)
+	})
+
+	t.Run("list, no special params", func(t *testing.T) {
+		var stdOut, stdErr bytes.Buffer
+
+		//nolint: gosec  // input is part of test env
+		cmd := exec.Command("kubectl", "get", "tokencredentialrequests.login.concierge"+dotSuffix, "-A")
+		cmd.Stdout = &stdOut
+		cmd.Stderr = &stdErr
+		err := cmd.Run()
+		require.NoError(t, err, stdErr.String(), stdOut.String())
+		require.Empty(t, stdOut.String())
+
+		require.NotContains(t, stdErr.String(), "MethodNotAllowed")
+		require.Contains(t, stdErr.String(), `No resources found`)
+	})
+
+	t.Run("list, table params", func(t *testing.T) {
+		var stdOut, stdErr bytes.Buffer
+
+		//nolint: gosec  // input is part of test env
+		cmd := exec.Command("kubectl", "get", "tokencredentialrequests.login.concierge"+dotSuffix, "-A", "-o", "wide", "-v", "10")
+		cmd.Stdout = &stdOut
+		cmd.Stderr = &stdErr
+		err := cmd.Run()
+		require.NoError(t, err, stdErr.String(), stdOut.String())
+		require.Empty(t, stdOut.String())
+
+		require.NotContains(t, stdErr.String(), "MethodNotAllowed")
+		require.Contains(t, stdErr.String(), `"kind":"Table"`)
+		require.Contains(t, stdErr.String(), `"resourceVersion":"0"`)
+	})
+
+	t.Run("raw request to see body", func(t *testing.T) {
+		var stdOut, stdErr bytes.Buffer
+
+		//nolint: gosec  // input is part of test env
+		cmd := exec.Command("kubectl", "get", "--raw", "/apis/login.concierge"+dotSuffix+"/v1alpha1/tokencredentialrequests")
+		cmd.Stdout = &stdOut
+		cmd.Stderr = &stdErr
+		err := cmd.Run()
+		require.NoError(t, err, stdErr.String(), stdOut.String())
+		require.Empty(t, stdErr.String())
+
+		require.NotContains(t, stdOut.String(), "MethodNotAllowed")
+		require.Contains(t, stdOut.String(), `{"kind":"TokenCredentialRequestList","apiVersion":"login.concierge`+
+			dotSuffix+`/v1alpha1","metadata":{"resourceVersion":"0"},"items":[]}`)
+	})
+}

test/integration/kube_api_discovery_test.go

@@ -72,7 +72,7 @@ func TestGetAPIResourceList(t *testing.T) {
 					{
 						Name:       "tokencredentialrequests",
 						Kind:       "TokenCredentialRequest",
-						Verbs:      []string{"create"},
+						Verbs:      []string{"create", "list"},
 						Namespaced: true,
 						Categories: []string{"pinniped"},
 					},