Merge pull request #139 from FabianKramm/master

refactor: performance improvements & watch fix

devspace.yaml

@@ -43,9 +43,12 @@ profiles:
       - op: add
         path: deployments[0].helm.values.kiosk.resources
         value:
+          requests:
+            memory: "0"
+            cpu: "0"
           limits:
-            memory: 4Gi
-            cpu: "4"
+            memory: "0"
+            cpu: "0"
     replace:
       dev:
         terminal:

pkg/apiserver/registry/account/rest.go

@@ -19,10 +19,10 @@ package account
 import (
 	"context"
 	"fmt"
-	"github.com/loft-sh/kiosk/kube/plugin/pkg/auth/authorizer/rbac"
 	config "github.com/loft-sh/kiosk/pkg/apis/config/v1alpha1"
 	"github.com/loft-sh/kiosk/pkg/apis/tenancy"
 	"github.com/loft-sh/kiosk/pkg/authorization"
+	"github.com/loft-sh/kiosk/pkg/authorization/rbac"
 	kioskwatch "github.com/loft-sh/kiosk/pkg/watch"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -46,8 +46,7 @@ type accountREST struct {
 
 // NewAccountREST creates a new account storage that implements the rest interface
 func NewAccountREST(cachedClient client.Client, uncachedClient client.Client, scheme *runtime.Scheme) rest.Storage {
-	ruleClient := authorization.NewRuleClient(cachedClient)
-	authorizer := rbac.New(ruleClient, ruleClient, ruleClient, ruleClient)
+	authorizer := rbac.New(cachedClient)
 	return &accountREST{
 		client:     uncachedClient,
 		authorizer: authorizer,
@@ -128,7 +127,8 @@ func (r *accountREST) List(ctx context.Context, options *metainternalversion.Lis
 	}
 
 	accountList := &tenancy.AccountList{
-		Items: []tenancy.Account{},
+		ListMeta: configAccountList.ListMeta,
+		Items:    []tenancy.Account{},
 	}
 	for _, n := range configAccountList.Items {
 		account, err := ConvertConfigAccount(&n)

pkg/apiserver/registry/account/rest_test.go

@@ -3,6 +3,8 @@ package account
 import (
 	"context"
 	tenancyv1alpha1 "github.com/loft-sh/kiosk/pkg/apis/tenancy/v1alpha1"
+	"github.com/loft-sh/kiosk/pkg/authorization"
+	"github.com/loft-sh/kiosk/pkg/authorization/rbac"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -115,7 +117,15 @@ func TestListAccount(t *testing.T) {
 		})
 	ctx := context.TODO()
 	userCtx := withRequestInfo(request.WithUser(ctx, &user.DefaultInfo{Name: "foo"}), "list", "")
-	accountStorage := NewAccountREST(fakeClient, fakeClient, scheme).(*accountREST)
+	a := &rbac.RBACAuthorizer{AuthorizationRuleResolver: &rbac.DefaultRuleResolver{
+		ListAll: true,
+		Client: fakeClient,
+	}}
+	accountStorage := &accountREST{
+		client:     fakeClient,
+		authorizer: a,
+		filter:     authorization.NewFilteredLister(fakeClient, a),
+	}
 
 	// Get empty list
 	obj, err := accountStorage.List(userCtx, &metainternalversion.ListOptions{})

pkg/apiserver/registry/space/rest.go

@@ -20,12 +20,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/loft-sh/kiosk/kube/plugin/pkg/auth/authorizer/rbac"
 	configv1alpha1 "github.com/loft-sh/kiosk/pkg/apis/config/v1alpha1"
 	"github.com/loft-sh/kiosk/pkg/apis/tenancy"
 	"github.com/loft-sh/kiosk/pkg/apis/tenancy/validation"
 	"github.com/loft-sh/kiosk/pkg/apiserver/registry/util"
 	"github.com/loft-sh/kiosk/pkg/authorization"
+	"github.com/loft-sh/kiosk/pkg/authorization/rbac"
 	"github.com/loft-sh/kiosk/pkg/constants"
 	"github.com/loft-sh/kiosk/pkg/util/loghelper"
 	kioskwatch "github.com/loft-sh/kiosk/pkg/watch"
@@ -56,8 +56,7 @@ type spaceStorage struct {
 
 // NewSpaceREST creates a new space storage that implements the rest interface
 func NewSpaceREST(cachedClient client.Client, uncachedClient client.Client, scheme *runtime.Scheme) rest.Storage {
-	ruleClient := authorization.NewRuleClient(cachedClient)
-	authorizer := rbac.New(ruleClient, ruleClient, ruleClient, ruleClient)
+	authorizer := rbac.New(cachedClient)
 	return &spaceStorage{
 		client:     uncachedClient,
 		authorizer: authorizer,
@@ -139,7 +138,8 @@ func (r *spaceStorage) List(ctx context.Context, options *metainternalversion.Li
 	}
 
 	spaceList := &tenancy.SpaceList{
-		Items: []tenancy.Space{},
+		ListMeta: namespaces.ListMeta,
+		Items:    []tenancy.Space{},
 	}
 	for _, n := range namespaces.Items {
 		spaceList.Items = append(spaceList.Items, *ConvertNamespace(&n))

pkg/apiserver/registry/space/rest_test.go

@@ -2,6 +2,8 @@ package space
 
 import (
 	tenancyv1alpha1 "github.com/loft-sh/kiosk/pkg/apis/tenancy/v1alpha1"
+	"github.com/loft-sh/kiosk/pkg/authorization"
+	"github.com/loft-sh/kiosk/pkg/authorization/rbac"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"testing"
@@ -41,6 +43,19 @@ var clusterAdminBinding = &rbacv1.ClusterRoleBinding{
 	},
 }
 
+func NewTestSpaceREST(fakeClient client.Client, scheme *runtime.Scheme) *spaceStorage {
+	a := &rbac.RBACAuthorizer{AuthorizationRuleResolver: &rbac.DefaultRuleResolver{
+		ListAll: true,
+		Client: fakeClient,
+	}}
+	return &spaceStorage{
+		client:     fakeClient,
+		authorizer: a,
+		scheme:     scheme,
+		filter:     authorization.NewFilteredLister(fakeClient, a),
+	}
+}
+
 func clientWithDefaultRoles(scheme *runtime.Scheme, objs ...runtime.Object) *testingutil.FakeIndexClient {
 	objs = append(objs, &rbacv1.ClusterRole{
 		ObjectMeta: metav1.ObjectMeta{
@@ -83,7 +98,7 @@ func TestGetSpace(t *testing.T) {
 	})
 	ctx := context.TODO()
 	userCtx := request.WithUser(ctx, &user.DefaultInfo{Name: "foo"})
-	spaceStorage := NewSpaceREST(fakeClient, fakeClient, scheme).(*spaceStorage)
+	spaceStorage := NewTestSpaceREST(fakeClient, scheme)
 
 	// We are not allowed to retrieve it so this should return a not found
 	_, err := spaceStorage.Get(withRequestInfo(userCtx, "get", "test"), "test", &metav1.GetOptions{})
@@ -135,7 +150,7 @@ func TestListSpaces(t *testing.T) {
 		})
 	ctx := context.TODO()
 	userCtx := withRequestInfo(request.WithUser(ctx, &user.DefaultInfo{Name: "foo"}), "list", "")
-	spaceStorage := NewSpaceREST(fakeClient, fakeClient, scheme).(*spaceStorage)
+	spaceStorage := NewTestSpaceREST(fakeClient, scheme)
 
 	// Get empty list
 	obj, err := spaceStorage.List(userCtx, &metainternalversion.ListOptions{})
@@ -232,7 +247,7 @@ func TestCreateSpace(t *testing.T) {
 		})
 	ctx := context.TODO()
 	userCtx := withRequestInfo(request.WithUser(ctx, &user.DefaultInfo{Name: "foo"}), "create", "")
-	spaceStorage := NewSpaceREST(fakeClient, fakeClient, scheme).(*spaceStorage)
+	spaceStorage := NewTestSpaceREST(fakeClient, scheme)
 
 	// Try to create if we are not allowed to
 	_, err := spaceStorage.Create(userCtx, &tenancy.Space{
@@ -330,7 +345,7 @@ func TestSpaceUpdate(t *testing.T) {
 		})
 	ctx := context.TODO()
 	userCtx := withRequestInfo(request.WithUser(ctx, &user.DefaultInfo{Name: "foo"}), "update", "test")
-	spaceStorage := NewSpaceREST(fakeClient, fakeClient, scheme).(*spaceStorage)
+	spaceStorage := NewTestSpaceREST(fakeClient, scheme)
 
 	_, updated, err := spaceStorage.Update(userCtx, "test", &fakeUpdater{out: &tenancy.Space{
 		ObjectMeta: metav1.ObjectMeta{
@@ -377,7 +392,7 @@ func TestSpaceDelete(t *testing.T) {
 		})
 	ctx := context.TODO()
 	userCtx := withRequestInfo(request.WithUser(ctx, &user.DefaultInfo{Name: "foo"}), "delete", "test")
-	spaceStorage := NewSpaceREST(fakeClient, fakeClient, scheme).(*spaceStorage)
+	spaceStorage := NewTestSpaceREST(fakeClient, scheme)
 
 	_, deleted, err := spaceStorage.Delete(userCtx, "test", fakeDeleteValidation, &metav1.DeleteOptions{})
 	if err == nil || kerrors.IsForbidden(err) == false || deleted == true {

kube/plugin/pkg/auth/authorizer/rbac/rbac.go → pkg/authorization/rbac/authorizer.go

@@ -19,32 +19,24 @@ import (
 	"context"
 	"fmt"
 	"k8s.io/klog"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	rbacv1helpers "github.com/loft-sh/kiosk/kube/pkg/apis/rbac/v1"
-	rbacregistryvalidation "github.com/loft-sh/kiosk/kube/pkg/registry/rbac/validation"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	rbaclisters "k8s.io/client-go/listers/rbac/v1"
 )
 
-type RequestToRuleMapper interface {
-	// RulesFor returns all known PolicyRules and any errors that happened while locating those rules.
-	// Any rule returned is still valid, since rules are deny by default.  If you can pass with the rules
-	// supplied, you do not have to fail the request.  If you cannot, you should indicate the error along
-	// with your denial.
-	RulesFor(subject user.Info, namespace string) ([]rbacv1.PolicyRule, error)
-
-	// VisitRulesFor invokes visitor() with each rule that applies to a given user in a given namespace,
-	// and each error encountered resolving those rules. Rule may be nil if err is non-nil.
-	// If visitor() returns false, visiting is short-circuited.
-	VisitRulesFor(user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool)
+type RBACAuthorizer struct {
+	AuthorizationRuleResolver *DefaultRuleResolver
 }
 
-type RBACAuthorizer struct {
-	authorizationRuleResolver RequestToRuleMapper
+func New(client client.Client) *RBACAuthorizer {
+	return &RBACAuthorizer{
+		AuthorizationRuleResolver: NewDefaultRuleResolver(client),
+	}
 }
 
 // authorizingVisitor short-circuits once allowed, and collects any resolution errors encountered
@@ -71,7 +63,7 @@ func (v *authorizingVisitor) visit(source fmt.Stringer, rule *rbacv1.PolicyRule,
 func (r *RBACAuthorizer) Authorize(ctx context.Context, requestAttributes authorizer.Attributes) (authorizer.Decision, string, error) {
 	ruleCheckingVisitor := &authorizingVisitor{requestAttributes: requestAttributes}
 
-	r.authorizationRuleResolver.VisitRulesFor(requestAttributes.GetUser(), requestAttributes.GetNamespace(), ruleCheckingVisitor.visit)
+	r.AuthorizationRuleResolver.VisitRulesFor(ctx, requestAttributes.GetUser(), requestAttributes.GetNamespace(), ruleCheckingVisitor.visit)
 	if ruleCheckingVisitor.allowed {
 		return authorizer.DecisionAllow, ruleCheckingVisitor.reason, nil
 	}
@@ -122,55 +114,6 @@ func (r *RBACAuthorizer) Authorize(ctx context.Context, requestAttributes author
 	return authorizer.DecisionNoOpinion, reason, nil
 }
 
-func (r *RBACAuthorizer) RulesFor(user user.Info, namespace string) ([]authorizer.ResourceRuleInfo, []authorizer.NonResourceRuleInfo, bool, error) {
-	var (
-		resourceRules    []authorizer.ResourceRuleInfo
-		nonResourceRules []authorizer.NonResourceRuleInfo
-	)
-
-	policyRules, err := r.authorizationRuleResolver.RulesFor(user, namespace)
-	for _, policyRule := range policyRules {
-		if len(policyRule.Resources) > 0 {
-			r := authorizer.DefaultResourceRuleInfo{
-				Verbs:         policyRule.Verbs,
-				APIGroups:     policyRule.APIGroups,
-				Resources:     policyRule.Resources,
-				ResourceNames: policyRule.ResourceNames,
-			}
-			var resourceRule authorizer.ResourceRuleInfo = &r
-			resourceRules = append(resourceRules, resourceRule)
-		}
-		if len(policyRule.NonResourceURLs) > 0 {
-			r := authorizer.DefaultNonResourceRuleInfo{
-				Verbs:           policyRule.Verbs,
-				NonResourceURLs: policyRule.NonResourceURLs,
-			}
-			var nonResourceRule authorizer.NonResourceRuleInfo = &r
-			nonResourceRules = append(nonResourceRules, nonResourceRule)
-		}
-	}
-	return resourceRules, nonResourceRules, false, err
-}
-
-func New(roles rbacregistryvalidation.RoleGetter, roleBindings rbacregistryvalidation.RoleBindingLister, clusterRoles rbacregistryvalidation.ClusterRoleGetter, clusterRoleBindings rbacregistryvalidation.ClusterRoleBindingLister) *RBACAuthorizer {
-	authorizer := &RBACAuthorizer{
-		authorizationRuleResolver: rbacregistryvalidation.NewDefaultRuleResolver(
-			roles, roleBindings, clusterRoles, clusterRoleBindings,
-		),
-	}
-	return authorizer
-}
-
-func RulesAllow(requestAttributes authorizer.Attributes, rules ...rbacv1.PolicyRule) bool {
-	for i := range rules {
-		if RuleAllows(requestAttributes, &rules[i]) {
-			return true
-		}
-	}
-
-	return false
-}
-
 func RuleAllows(requestAttributes authorizer.Attributes, rule *rbacv1.PolicyRule) bool {
 	if requestAttributes.IsResourceRequest() {
 		combinedResource := requestAttributes.GetResource()