diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index a27aeb72..584a84b6 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -110,7 +110,7 @@ func (a *App) runServer(ctx context.Context) error { } // Initialize the cache of active authenticators. - authenticators := authncache.New(*cfg.APIGroupSuffix) + authenticators := authncache.New() // This cert provider will provide certs to the API server and will // be mutated by a controller to keep the certs up to date with what diff --git a/internal/controller/authenticator/authncache/cache.go b/internal/controller/authenticator/authncache/cache.go index 7d629eb5..87f317df 100644 --- a/internal/controller/authenticator/authncache/cache.go +++ b/internal/controller/authenticator/authncache/cache.go @@ -15,7 +15,6 @@ import ( "k8s.io/klog/v2" loginapi "go.pinniped.dev/generated/1.20/apis/concierge/login" - "go.pinniped.dev/internal/groupsuffix" "go.pinniped.dev/internal/plog" ) @@ -27,8 +26,7 @@ var ( // Cache implements the authenticator.Token interface by multiplexing across a dynamic set of authenticators // loaded from authenticator resources. type Cache struct { - cache sync.Map - apiGroupSuffix string + cache sync.Map } type Key struct { @@ -43,8 +41,8 @@ type Value interface { } // New returns an empty cache. -func New(apiGroupSuffix string) *Cache { - return &Cache{apiGroupSuffix: apiGroupSuffix} +func New() *Cache { + return &Cache{} } // Get an authenticator by key. @@ -92,12 +90,7 @@ func (c *Cache) AuthenticateTokenCredentialRequest(ctx context.Context, req *log Kind: req.Spec.Authenticator.Kind, } if req.Spec.Authenticator.APIGroup != nil { - // The key must always be API group pinniped.dev because that's what the cache filler will always use. - apiGroup, replaced := groupsuffix.Unreplace(*req.Spec.Authenticator.APIGroup, c.apiGroupSuffix) - if !replaced { - return nil, ErrNoSuchAuthenticator - } - key.APIGroup = apiGroup + key.APIGroup = *req.Spec.Authenticator.APIGroup } val := c.Get(key) diff --git a/internal/controller/authenticator/authncache/cache_test.go b/internal/controller/authenticator/authncache/cache_test.go index fdb0fd03..d48a1951 100644 --- a/internal/controller/authenticator/authncache/cache_test.go +++ b/internal/controller/authenticator/authncache/cache_test.go @@ -28,7 +28,7 @@ func TestCache(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - cache := New("pinniped.dev") + cache := New() require.NotNil(t, cache) key1 := Key{Namespace: "foo", Name: "authenticator-one"} @@ -57,7 +57,7 @@ func TestCache(t *testing.T) { {APIGroup: "b", Kind: "b", Namespace: "b", Name: "b"}, } for tries := 0; tries < 10; tries++ { - cache := New("pinniped.dev") + cache := New() for _, i := range rand.Perm(len(keysInExpectedOrder)) { cache.Store(keysInExpectedOrder[i], nil) } @@ -91,48 +91,46 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) { Name: validRequest.Spec.Authenticator.Name, } - mockCache := func(t *testing.T, apiGroupSuffix string, expectAuthenticatorToBeCalled bool, res *authenticator.Response, authenticated bool, err error) *Cache { + mockCache := func(t *testing.T, res *authenticator.Response, authenticated bool, err error) *Cache { ctrl := gomock.NewController(t) t.Cleanup(ctrl.Finish) m := mocktokenauthenticator.NewMockToken(ctrl) - if expectAuthenticatorToBeCalled { - m.EXPECT().AuthenticateToken(audienceFreeContext{}, validRequest.Spec.Token).Return(res, authenticated, err) - } - c := New(apiGroupSuffix) + m.EXPECT().AuthenticateToken(audienceFreeContext{}, validRequest.Spec.Token).Return(res, authenticated, err) + c := New() c.Store(validRequestKey, m) return c } t.Run("no such authenticator", func(t *testing.T) { - c := New("pinniped.dev") + c := New() res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) require.EqualError(t, err, "no such authenticator") require.Nil(t, res) }) t.Run("authenticator returns error", func(t *testing.T) { - c := mockCache(t, "pinniped.dev", true, nil, false, fmt.Errorf("some authenticator error")) + c := mockCache(t, nil, false, fmt.Errorf("some authenticator error")) res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) require.EqualError(t, err, "some authenticator error") require.Nil(t, res) }) t.Run("authenticator returns unauthenticated without error", func(t *testing.T) { - c := mockCache(t, "pinniped.dev", true, &authenticator.Response{}, false, nil) + c := mockCache(t, &authenticator.Response{}, false, nil) res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) require.NoError(t, err) require.Nil(t, res) }) t.Run("authenticator returns nil response without error", func(t *testing.T) { - c := mockCache(t, "pinniped.dev", true, nil, true, nil) + c := mockCache(t, nil, true, nil) res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) require.NoError(t, err) require.Nil(t, res) }) t.Run("authenticator returns response with nil user", func(t *testing.T) { - c := mockCache(t, "pinniped.dev", true, &authenticator.Response{}, true, nil) + c := mockCache(t, &authenticator.Response{}, true, nil) res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) require.NoError(t, err) require.Nil(t, res) @@ -153,7 +151,7 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) { } }, ) - c := New("pinniped.dev") + c := New() c.Store(validRequestKey, m) ctx, cancel := context.WithCancel(context.Background()) @@ -173,7 +171,7 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) { Groups: []string{"test-group-1", "test-group-2"}, Extra: map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, } - c := mockCache(t, "pinniped.dev", true, &authenticator.Response{User: &userInfo}, true, nil) + c := mockCache(t, &authenticator.Response{User: &userInfo}, true, nil) audienceCtx := authenticator.WithAudiences(context.Background(), authenticator.Audiences{"test-audience-1"}) res, err := c.AuthenticateTokenCredentialRequest(audienceCtx, validRequest.DeepCopy()) @@ -184,50 +182,6 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) { require.Equal(t, []string{"test-group-1", "test-group-2"}, res.GetGroups()) require.Equal(t, map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, res.GetExtra()) }) - - t.Run("using a non-default API group suffix still performs the cache lookup using the pinniped.dev suffix", func(t *testing.T) { - authenticationGroupWithCustomSuffix := "authentication.concierge.custom-suffix.com" - validRequestForAlternateAPIGroup := loginapi.TokenCredentialRequest{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - }, - Spec: loginapi.TokenCredentialRequestSpec{ - Authenticator: corev1.TypedLocalObjectReference{ - APIGroup: &authenticationGroupWithCustomSuffix, - Kind: "WebhookAuthenticator", - Name: "test-name", - }, - Token: "test-token", - }, - Status: loginapi.TokenCredentialRequestStatus{}, - } - - userInfo := user.DefaultInfo{ - Name: "test-user", - UID: "test-uid", - Groups: []string{"test-group-1", "test-group-2"}, - Extra: map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, - } - c := mockCache(t, "custom-suffix.com", true, &authenticator.Response{User: &userInfo}, true, nil) - - audienceCtx := authenticator.WithAudiences(context.Background(), authenticator.Audiences{"test-audience-1"}) - res, err := c.AuthenticateTokenCredentialRequest(audienceCtx, validRequestForAlternateAPIGroup.DeepCopy()) - require.NoError(t, err) - require.NotNil(t, res) - require.Equal(t, "test-user", res.GetName()) - require.Equal(t, "test-uid", res.GetUID()) - require.Equal(t, []string{"test-group-1", "test-group-2"}, res.GetGroups()) - require.Equal(t, map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, res.GetExtra()) - }) - - t.Run("using a non-default API group suffix and the incoming request mentions a different API group, results in no such authenticator", func(t *testing.T) { - c := mockCache(t, "custom-suffix.com", false, &authenticator.Response{User: &user.DefaultInfo{Name: "someone"}}, true, nil) - - // Note that the validRequest.Spec.Authenticator.APIGroup value uses "pinniped.dev", not "custom-suffix.com" - res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) - require.EqualError(t, err, "no such authenticator") - require.Nil(t, res) - }) } type audienceFreeContext struct{} diff --git a/internal/controller/authenticator/cachecleaner/cachecleaner_test.go b/internal/controller/authenticator/cachecleaner/cachecleaner_test.go index e4a9cad6..a735900b 100644 --- a/internal/controller/authenticator/cachecleaner/cachecleaner_test.go +++ b/internal/controller/authenticator/cachecleaner/cachecleaner_test.go @@ -153,7 +153,7 @@ func TestController(t *testing.T) { fakeClient := pinnipedfake.NewSimpleClientset(tt.objects...) informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) - cache := authncache.New("pinniped.dev") + cache := authncache.New() if tt.initialCache != nil { tt.initialCache(t, cache) } diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index b1aee0d8..17b8073d 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -327,7 +327,7 @@ func TestController(t *testing.T) { fakeClient := pinnipedfake.NewSimpleClientset(tt.jwtAuthenticators...) informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) - cache := authncache.New("pinniped.dev") + cache := authncache.New() testLog := testlogger.New(t) if tt.cache != nil { diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index 27dd1637..6b0bec0b 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -90,7 +90,7 @@ func TestController(t *testing.T) { fakeClient := pinnipedfake.NewSimpleClientset(tt.webhooks...) informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0) - cache := authncache.New("pinniped.dev") + cache := authncache.New() testLog := testlogger.New(t) controller := New(cache, informers.Authentication().V1alpha1().WebhookAuthenticators(), testLog)