Merge pull request #8766 from deads2k/project-request-limit

Merged by openshift-bot

Author: OpenShift Bot

pkg/project/admission/requestlimit/admission.go

@@ -10,6 +10,7 @@ import (
 	kapi "k8s.io/kubernetes/pkg/api"
 	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/labels"
+	"k8s.io/kubernetes/pkg/serviceaccount"
 
 	"github.com/openshift/origin/pkg/client"
 	oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
@@ -18,6 +19,7 @@ import (
 	requestlimitapivalidation "github.com/openshift/origin/pkg/project/admission/requestlimit/api/validation"
 	projectapi "github.com/openshift/origin/pkg/project/api"
 	projectcache "github.com/openshift/origin/pkg/project/cache"
+	uservalidation "github.com/openshift/origin/pkg/user/api/validation"
 )
 
 // allowedTerminatingProjects is the number of projects that are owned by a user, are in terminating state,
@@ -98,6 +100,24 @@ func (o *projectRequestLimit) Admit(a admission.Attributes) (err error) {
 // maxProjectsByRequester returns the maximum number of projects allowed for a given user, whether a limit exists, and an error
 // if an error occurred. If a limit doesn't exist, the maximum number should be ignored.
 func (o *projectRequestLimit) maxProjectsByRequester(userName string) (int, bool, error) {
+	// service accounts have a different ruleset, check them
+	if _, _, err := serviceaccount.SplitUsername(userName); err == nil {
+		if o.config.MaxProjectsForServiceAccounts == nil {
+			return 0, false, nil
+		}
+
+		return *o.config.MaxProjectsForServiceAccounts, true, nil
+	}
+
+	// if we aren't a valid username, we came in as cert user for certain, use our cert user rules
+	if valid, _ := uservalidation.ValidateUserName(userName, false); !valid {
+		if o.config.MaxProjectsForSystemUsers == nil {
+			return 0, false, nil
+		}
+
+		return *o.config.MaxProjectsForSystemUsers, true, nil
+	}
+
 	// prevent a user lookup if no limits are configured
 	if len(o.config.Limits) == 0 {
 		return 0, false, nil

pkg/project/admission/requestlimit/api/types.go

@@ -10,6 +10,15 @@ import (
 type ProjectRequestLimitConfig struct {
 	unversioned.TypeMeta
 	Limits []ProjectLimitBySelector
+
+	// MaxProjectsForSystemUsers controls how many projects a certificate user may have.  Certificate
+	// users do not have any labels associated with them for more fine grained control
+	MaxProjectsForSystemUsers *int
+
+	// MaxProjectsForServiceAccounts controls how many projects a service account may have.  Service
+	// accounts can't create projects by default, but if they are allowed to create projects, you cannot
+	// trust any labels placed on them since project editors can manipulate those labels
+	MaxProjectsForServiceAccounts *int
 }
 
 // ProjectLimitBySelector specifies the maximum number of projects allowed for a given user label selector

pkg/project/admission/requestlimit/api/v1/swagger_doc.go

@@ -16,8 +16,10 @@ func (ProjectLimitBySelector) SwaggerDoc() map[string]string {
 }
 
 var map_ProjectRequestLimitConfig = map[string]string{
-	"":       "ProjectRequestLimitConfig is the configuration for the project request limit plug-in It contains an ordered list of limits based on user label selectors. Selectors will be checked in order and the first one that applies will be used as the limit.",
-	"limits": "Limits are the project request limits",
+	"":                              "ProjectRequestLimitConfig is the configuration for the project request limit plug-in It contains an ordered list of limits based on user label selectors. Selectors will be checked in order and the first one that applies will be used as the limit.",
+	"limits":                        "Limits are the project request limits",
+	"maxProjectsForSystemUsers":     "MaxProjectsForSystemUsers controls how many projects a certificate user may have.  Certificate users do not have any labels associated with them for more fine grained control",
+	"maxProjectsForServiceAccounts": "MaxProjectsForServiceAccounts controls how many projects a service account may have.  Service accounts can't create projects by default, but if they are allowed to create projects, you cannot trust any labels placed on them since project editors can manipulate those labels",
 }
 
 func (ProjectRequestLimitConfig) SwaggerDoc() map[string]string {

pkg/project/admission/requestlimit/api/v1/types.go

@@ -12,6 +12,15 @@ type ProjectRequestLimitConfig struct {
 
 	// Limits are the project request limits
 	Limits []ProjectLimitBySelector `json:"limits",description:"project request limits"`
+
+	// MaxProjectsForSystemUsers controls how many projects a certificate user may have.  Certificate
+	// users do not have any labels associated with them for more fine grained control
+	MaxProjectsForSystemUsers *int `json:"maxProjectsForSystemUsers"`
+
+	// MaxProjectsForServiceAccounts controls how many projects a service account may have.  Service
+	// accounts can't create projects by default, but if they are allowed to create projects, you cannot
+	// trust any labels placed on them since project editors can manipulate those labels
+	MaxProjectsForServiceAccounts *int `json:"maxProjectsForServiceAccounts"`
 }
 
 // ProjectLimitBySelector specifies the maximum number of projects allowed for a given user label selector

pkg/project/admission/requestlimit/api/validation/validation.go

@@ -12,6 +12,12 @@ func ValidateProjectRequestLimitConfig(config *api.ProjectRequestLimitConfig) fi
 	for i, projectLimit := range config.Limits {
 		allErrs = append(allErrs, ValidateProjectLimitBySelector(projectLimit, field.NewPath("limits").Index(i))...)
 	}
+	if config.MaxProjectsForSystemUsers != nil && *config.MaxProjectsForSystemUsers < 0 {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("maxProjectsForSystemUsers"), *config.MaxProjectsForSystemUsers, "cannot be a negative number"))
+	}
+	if config.MaxProjectsForServiceAccounts != nil && *config.MaxProjectsForServiceAccounts < 0 {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("maxProjectsForServiceAccounts"), *config.MaxProjectsForServiceAccounts, "cannot be a negative number"))
+	}
 	return allErrs
 }
 

test/integration/project_reqlimit_test.go

@@ -111,6 +111,8 @@ func projectRequestLimitSingleDefaultConfig() *requestlimit.ProjectRequestLimitC
 				MaxProjects: intPointer(1),
 			},
 		},
+
+		MaxProjectsForSystemUsers: intPointer(1),
 	}
 }
 
@@ -167,6 +169,23 @@ func TestProjectRequestLimitSingleConfig(t *testing.T) {
 	})
 }
 
+// we had a bug where this failed on ` uenxpected error: metadata.name: Invalid value: "system:admin": may not contain ":"`
+// make sure we never have that bug again and that project limits for them work
+func TestProjectRequestLimitAsSystemAdmin(t *testing.T) {
+	_, oclient, _ := setupProjectRequestLimitTest(t, projectRequestLimitSingleDefaultConfig())
+
+	if _, err := oclient.ProjectRequests().Create(&projectapi.ProjectRequest{
+		ObjectMeta: kapi.ObjectMeta{Name: "foo"},
+	}); err != nil {
+		t.Errorf("uenxpected error: %v", err)
+	}
+	if _, err := oclient.ProjectRequests().Create(&projectapi.ProjectRequest{
+		ObjectMeta: kapi.ObjectMeta{Name: "bar"},
+	}); !apierrors.IsForbidden(err) {
+		t.Errorf("missing error: %v", err)
+	}
+}
+
 func testProjectRequestLimitAdmission(t *testing.T, errorPrefix string, clientConfig *restclient.Config, tests map[string]bool) {
 	for user, expectSuccess := range tests {
 		oclient, _, _, err := testutil.GetClientForUser(*clientConfig, user)