splits rolebindingscontroller per capability

pkg/authorization/defaultrolebindings/defaultrolebindings.go

@@ -116,7 +116,7 @@ func (c *RoleBindingController) syncNamespace(namespaceName string) error {
 	}
 
 	errs := []error{}
-	desiredRoleBindings := c.roleBindingsFunc(namespaceName)
+	desiredRoleBindings := GetRoleBindingsForController(c.name)(namespaceName)
 
 	for i := range desiredRoleBindings {
 		desiredRoleBinding := desiredRoleBindings[i]

pkg/authorization/defaultrolebindings/defaultrolebindings_test.go

@@ -16,25 +16,24 @@ import (
 	"k8s.io/kubernetes/pkg/controller"
 )
 
-type tests struct {
-	name                      string
-	controller                string
-	startingNamespaces        []*corev1.Namespace
-	startingRoleBindings      []*rbacv1.RoleBinding
-	namespaceToSync           string
-	expectedRoleBindingsNames []string
-}
-
 var controllerNames = []string{
 	"DefaultRoleBindingController",
 	"BuilderRoleBindingController",
+	"ImagePullerRoleBindingController",
 	"DeployerRoleBindingController",
 }
 
 func TestSync(t *testing.T) {
-	tests := []tests{
+	tests := []struct {
+		name                      string
+		controller                string
+		startingNamespaces        []*corev1.Namespace
+		startingRoleBindings      []*rbacv1.RoleBinding
+		namespaceToSync           string
+		expectedRoleBindingsNames []string
+	}{
 		{
-			name:       "create-default",
+			name:       "create-default-all",
 			controller: "DefaultRoleBindingController",
 			startingNamespaces: []*corev1.Namespace{
 				{ObjectMeta: metav1.ObjectMeta{Name: "foo"}},
@@ -43,7 +42,7 @@ func TestSync(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"}},
 			},
 			namespaceToSync:           "foo",
-			expectedRoleBindingsNames: []string{"system:image-pullers"},
+			expectedRoleBindingsNames: []string{"system:image-pullers", "system:image-builders", "system:deployers"},
 		},
 		{
 			name:       "create-builder",
@@ -52,7 +51,7 @@ func TestSync(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "foo"}},
 			},
 			startingRoleBindings: []*rbacv1.RoleBinding{
-				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"}},
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "system:image-pullers"}},
 			},
 			namespaceToSync:           "foo",
 			expectedRoleBindingsNames: []string{"system:image-builders"},
@@ -64,11 +63,24 @@ func TestSync(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "foo"}},
 			},
 			startingRoleBindings: []*rbacv1.RoleBinding{
-				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"}},
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "system:image-builders"}},
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "system:image-pullers"}},
 			},
 			namespaceToSync:           "foo",
 			expectedRoleBindingsNames: []string{"system:deployers"},
 		},
+		{
+			name:       "create-image-puller",
+			controller: "ImagePullerRoleBindingController",
+			startingNamespaces: []*corev1.Namespace{
+				{ObjectMeta: metav1.ObjectMeta{Name: "foo"}},
+			},
+			startingRoleBindings: []*rbacv1.RoleBinding{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"}},
+			},
+			namespaceToSync:           "foo",
+			expectedRoleBindingsNames: []string{"system:image-pullers"},
+		},
 		{
 			name:       "create-default-missing",
 			controller: "DefaultRoleBindingController",
@@ -77,11 +89,11 @@ func TestSync(t *testing.T) {
 				{ObjectMeta: metav1.ObjectMeta{Name: "new"}},
 			},
 			startingRoleBindings: []*rbacv1.RoleBinding{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "system:image-builders"}},
 				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"}},
-				{ObjectMeta: metav1.ObjectMeta{Namespace: "new", Name: "system:image-pullers"}},
 			},
 			namespaceToSync:           "foo",
-			expectedRoleBindingsNames: []string{"system:image-pullers"},
+			expectedRoleBindingsNames: []string{"system:image-pullers", "system:deployers"},
 		},
 		{
 			name:       "create-default-none",
@@ -91,6 +103,8 @@ func TestSync(t *testing.T) {
 			},
 			startingRoleBindings: []*rbacv1.RoleBinding{
 				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "system:image-pullers"}},
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "system:image-builders"}},
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "system:deployers"}},
 			},
 			namespaceToSync: "foo",
 		},

pkg/authorization/defaultrolebindings/project_policy.go

@@ -26,20 +26,29 @@ const (
 type projectRoleBindings func(namespace string) []rbacv1.RoleBinding
 type serviceAccountRoleBinding func(namespace string) rbacv1.RoleBinding
 
-func GetBootstrapServiceAccountProjectRoleBindings(namespace string) rbacv1.RoleBinding {
+// GetImagePullerProjectRoleBindings generates a role binding that allows all pods
+// to pull ImageStream images associated with given namespace.
+// These should only be created if the "ImageRegistry" capability is enabled on the cluster.
+func GetImagePullerProjectRoleBindings(namespace string) rbacv1.RoleBinding {
 	imagePuller := newOriginRoleBindingForClusterRoleWithGroup(ImagePullerRoleBindingName, ImagePullerRoleName, namespace, serviceaccount.MakeNamespaceGroupName(namespace))
 	imagePuller.Annotations[openShiftDescription] = "Allows all pods in this namespace to pull images from this namespace.  It is auto-managed by a controller; remove subjects to disable."
 
 	return imagePuller
 }
 
+// GetBuilderServiceAccountProjectRoleBindings generates the role bindings
+// specific to the "builder" service account of given namespace.
+// These should only be created if the "Build" capability is enabled on the cluster.
 func GetBuilderServiceAccountProjectRoleBindings(namespace string) rbacv1.RoleBinding {
 	imageBuilder := newOriginRoleBindingForClusterRoleWithSA(ImageBuilderRoleBindingName, ImageBuilderRoleName, namespace, BuilderServiceAccountName)
 	imageBuilder.Annotations[openShiftDescription] = "Allows builds in this namespace to push images to this namespace.  It is auto-managed by a controller; remove subjects to disable."
 
 	return imageBuilder
 }
 
+// GetDeployerServiceAccountProjectRoleBindings generates the role bindings
+// specific to the "builder" service account of given namespace.
+// These should only be created if the "DeploymentConfig" capability is enabled on the cluster.
 func GetDeployerServiceAccountProjectRoleBindings(namespace string) rbacv1.RoleBinding {
 	deployer := newOriginRoleBindingForClusterRoleWithSA(DeployerRoleBindingName, DeployerRoleName, namespace, DeployerServiceAccountName)
 	deployer.Annotations[openShiftDescription] = "Allows deploymentconfigs in this namespace to rollout pods in this namespace.  It is auto-managed by a controller; remove subjects to disable."
@@ -57,14 +66,18 @@ func composeRoleBindings(roleBindings ...serviceAccountRoleBinding) projectRoleB
 	}
 }
 
+// GetRoleBindingsForController returns the appropriate generator function for the
+// given named controller that will reconcile role bindings in a namespace.
 func GetRoleBindingsForController(controller string) projectRoleBindings {
 	switch controller {
 	case "BuilderRoleBindingController":
 		return composeRoleBindings(GetBuilderServiceAccountProjectRoleBindings)
 	case "DeployerRoleBindingController":
 		return composeRoleBindings(GetDeployerServiceAccountProjectRoleBindings)
+	case "ImagePullerRoleBindingController":
+		return composeRoleBindings(GetImagePullerProjectRoleBindings)
 	default:
-		return composeRoleBindings(GetBootstrapServiceAccountProjectRoleBindings,
+		return composeRoleBindings(GetImagePullerProjectRoleBindings,
 			GetBuilderServiceAccountProjectRoleBindings,
 			GetDeployerServiceAccountProjectRoleBindings,
 		)

pkg/authorization/defaultrolebindings/project_policy_test.go

@@ -0,0 +1,47 @@
+package defaultrolebindings
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+func TestRoleBindingsForController(t *testing.T) {
+	tests := map[string]struct {
+		controller                string
+		expectedRoleBindingsNames sets.Set[string]
+	}{
+		"test-builder": {
+			controller:                "BuilderRoleBindingController",
+			expectedRoleBindingsNames: sets.Set[string]{"system:image-builders": {}},
+		},
+		"test-deployer": {
+			controller:                "DeployerRoleBindingController",
+			expectedRoleBindingsNames: sets.Set[string]{"system:deployers": {}},
+		},
+		"test-image-puller": {
+			controller:                "ImagePullerRoleBindingController",
+			expectedRoleBindingsNames: sets.Set[string]{"system:image-pullers": {}},
+		},
+		"test-default": {
+			controller: "DefaultRoleBindingController",
+			expectedRoleBindingsNames: sets.Set[string]{"system:image-pullers": {},
+				"system:image-builders": {},
+				"system:deployers":      {},
+			},
+		},
+	}
+
+	for tName, tCase := range tests {
+		t.Run(tName, func(t *testing.T) {
+			controllerRoleBindings := GetRoleBindingsForController(tCase.controller)
+			roleBindingNames := GetBootstrapServiceAccountProjectRoleBindingNames(controllerRoleBindings)
+
+			if !cmp.Equal(roleBindingNames, tCase.expectedRoleBindingsNames) {
+				t.Fatalf("expected %v, got %#v", tCase.expectedRoleBindingsNames, roleBindingNames)
+			}
+
+		})
+	}
+}

pkg/cmd/controller/authorization.go

@@ -16,7 +16,7 @@ func RunDefaultRoleBindingController(ctx *ControllerContext) (bool, error) {
 
 func RunBuilderRoleBindingController(ctx *ControllerContext) (bool, error) {
 	// Role binding controllers currently share the same service account,
-	// as these are created by "boostrap" logic located elsewhere in Openshift.
+	// as these are created by "bootstrap" logic located elsewhere in Openshift.
 	// TODO: Refactor the controller service accounts to be managed by openshift-controller-manager-operator.
 	kubeClient, err := ctx.ClientBuilder.Client(infraDefaultRoleBindingsControllerServiceAccountName)
 	if err != nil {
@@ -28,7 +28,7 @@ func RunBuilderRoleBindingController(ctx *ControllerContext) (bool, error) {
 
 func RunDeployerRoleBindingController(ctx *ControllerContext) (bool, error) {
 	// Role binding controllers currently share the same service account,
-	// as these are created by "boostrap" logic located elsewhere in Openshift.
+	// as these are created by "bootstrap" logic located elsewhere in Openshift.
 	// TODO: Refactor the controller service accounts to be managed by openshift-controller-manager-operator.
 	kubeClient, err := ctx.ClientBuilder.Client(infraDefaultRoleBindingsControllerServiceAccountName)
 	if err != nil {
@@ -38,6 +38,18 @@ func RunDeployerRoleBindingController(ctx *ControllerContext) (bool, error) {
 	return runRoleBindingController(ctx, kubeClient, "DeployerRoleBindingController")
 }
 
+func RunImagePullerRoleBindingController(ctx *ControllerContext) (bool, error) {
+	// Role binding controllers currently share the same service account,
+	// as these are created by "bootstrap" logic located elsewhere in Openshift.
+	// TODO: Refactor the controller service accounts to be managed by openshift-controller-manager-operator.
+	kubeClient, err := ctx.ClientBuilder.Client(infraDefaultRoleBindingsControllerServiceAccountName)
+	if err != nil {
+		return true, err
+	}
+
+	return runRoleBindingController(ctx, kubeClient, "ImagePullerRoleBindingController")
+}
+
 func runRoleBindingController(cctx *ControllerContext, kubeClient kubernetes.Interface, controllerName string) (bool, error) {
 	go defaultrolebindings.NewRoleBindingsController(
 		cctx.KubernetesInformers.Rbac().V1().RoleBindings(),

pkg/cmd/controller/config.go

@@ -15,11 +15,9 @@ var ControllerInitializers = map[openshiftcontrolplanev1.OpenShiftControllerName
 	openshiftcontrolplanev1.OpenShiftBuilderServiceAccountController: RunBuilderServiceAccountController,
 	openshiftcontrolplanev1.OpenShiftBuildController:                 RunBuildController,
 	openshiftcontrolplanev1.OpenShiftBuildConfigChangeController:     RunBuildConfigChangeController,
-	openshiftcontrolplanev1.OpenShiftBuilderRoleBindingsController:   RunBuilderRoleBindingController,
 
 	openshiftcontrolplanev1.OpenShiftDeployerServiceAccountController: RunDeployerServiceAccountController,
 	openshiftcontrolplanev1.OpenShiftDeployerController:               RunDeployerController,
-	openshiftcontrolplanev1.OpenShiftDeployerRoleBindingsController:   RunDeployerRoleBindingController,
 	openshiftcontrolplanev1.OpenShiftDeploymentConfigController:       RunDeploymentConfigController,
 
 	openshiftcontrolplanev1.OpenShiftImageTriggerController:         RunImageTriggerController,