Merge pull request #14634 from jim-minter/issue14627

Merged by openshift-bot

Author: OpenShift Bot

pkg/cmd/server/bootstrappolicy/controller_policy.go

@@ -121,7 +121,7 @@ func init() {
 
 	// template-instance-controller
 	controllerRoleBindings = append(controllerRoleBindings,
-		rbac.NewClusterBinding(EditRoleName).SAs(DefaultOpenShiftInfraNamespace, InfraTemplateInstanceControllerServiceAccountName).BindingOrDie())
+		rbac.NewClusterBinding(AdminRoleName).SAs(DefaultOpenShiftInfraNamespace, InfraTemplateInstanceControllerServiceAccountName).BindingOrDie())
 
 	// origin-namespace-controller
 	addControllerRole(rbac.ClusterRole{

pkg/template/servicebroker/catalog.go

@@ -15,9 +15,6 @@ import (
 )
 
 const (
-	namespaceTitle       = "Template service broker: namespace"
-	namespaceDescription = "OpenShift namespace in which to provision service"
-
 	// the following should go away with catalog<->broker support for passing
 	// identity information.
 	requesterUsernameTitle       = "Template service broker: requester username"

pkg/template/servicebroker/test-scripts/provision.sh

@@ -14,8 +14,7 @@ req="{
   \"parameters\": {
     \"MYSQL_USER\": \"username\",
     \"template.openshift.io/requester-username\": \"$requesterUsername\"
-  },
-  \"accepts_incomplete\": true
+  }
 }"
 
 curl \
@@ -25,4 +24,4 @@ curl \
   -d "$req" \
   -v \
   $curlargs \
-  $endpoint/v2/service_instances/$instanceUUID
+  $endpoint/v2/service_instances/$instanceUUID'?accepts_incomplete=true'

test/extended/templates/templateinstance_security.go

@@ -9,9 +9,12 @@ import (
 	kerrors "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/apimachinery/pkg/util/wait"
 	kapi "k8s.io/kubernetes/pkg/api"
 	kapiv1 "k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/apis/storage"
+	storagev1 "k8s.io/kubernetes/pkg/apis/storage/v1"
 
 	"github.com/openshift/origin/pkg/api/latest"
 	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
@@ -54,6 +57,13 @@ var _ = g.Describe("[templates] templateinstance security tests", func() {
 				Name: bootstrappolicy.AdminRoleName,
 			},
 		}
+
+		storageclass = &storage.StorageClass{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "storageclass",
+			},
+			Provisioner: "no-provisioning",
+		}
 	)
 
 	g.BeforeEach(func() {
@@ -98,7 +108,7 @@ var _ = g.Describe("[templates] templateinstance security tests", func() {
 				},
 			},
 			{
-				by:              "checking edituser can't create a privileged object",
+				by:              "checking edituser can't create an object that requires admin",
 				user:            edituser,
 				namespace:       cli.Namespace(),
 				objects:         []runtime.Object{dummyrolebinding},
@@ -109,21 +119,32 @@ var _ = g.Describe("[templates] templateinstance security tests", func() {
 				},
 			},
 			{
-				// at the moment, an admin cannot create a privileged object
-				// via the template instance controller as the latter only has
-				// global edit permissions.
-				by:              "checking adminuser can't create a privileged object",
+				by:              "checking adminuser can't create an object that requires admin",
 				user:            adminuser,
 				namespace:       cli.Namespace(),
 				objects:         []runtime.Object{dummyrolebinding},
-				expectCondition: templateapi.TemplateInstanceInstantiateFailure,
+				expectCondition: templateapi.TemplateInstanceReady,
 				checkOK: func(namespace string) bool {
 					_, err := cli.AdminClient().RoleBindings(namespace).Get(dummyrolebinding.Name, metav1.GetOptions{})
+					return err == nil
+				},
+			},
+			{
+				by:              "checking adminuser can't create an object that requires more than admin",
+				user:            adminuser,
+				namespace:       cli.Namespace(),
+				objects:         []runtime.Object{storageclass},
+				expectCondition: templateapi.TemplateInstanceInstantiateFailure,
+				checkOK: func(namespace string) bool {
+					_, err := cli.AdminKubeClient().StorageV1().StorageClasses().Get(storageclass.Name, metav1.GetOptions{})
 					return err != nil && kerrors.IsNotFound(err)
 				},
 			},
 		}
 
+		targetVersions := []schema.GroupVersion{storagev1.SchemeGroupVersion}
+		targetVersions = append(targetVersions, latest.Versions...)
+
 		for _, test := range tests {
 			g.By(test.by)
 			cli.ChangeUser(test.user.Name)
@@ -161,7 +182,7 @@ var _ = g.Describe("[templates] templateinstance security tests", func() {
 				},
 			}
 
-			err = templateapi.AddObjectsToTemplate(&templateinstance.Spec.Template, test.objects, latest.Versions...)
+			err = templateapi.AddObjectsToTemplate(&templateinstance.Spec.Template, test.objects, targetVersions...)
 			o.Expect(err).NotTo(o.HaveOccurred())
 
 			templateinstance, err = cli.TemplateClient().Template().TemplateInstances(cli.Namespace()).Create(templateinstance)

test/integration/authorization_test.go

@@ -635,11 +635,12 @@ func TestAuthorizationResourceAccessReview(t *testing.T) {
 				Users:           sets.NewString("edgar"),
 				Groups:          sets.NewString(),
 				Namespace:       "mallet-project",
-				EvaluationError: `role.authorization.openshift.io "admin" not found`,
+				EvaluationError: `[role.authorization.openshift.io "admin" not found, role.authorization.openshift.io "admin" not found]`,
 			},
 		}
 		test.response.Users.Insert(globalClusterReaderUsers.List()...)
 		test.response.Users.Insert(globalDeploymentConfigGetterUsers.List()...)
+		test.response.Users.Delete("system:serviceaccount:openshift-infra:template-instance-controller")
 		test.response.Groups.Insert(globalClusterReaderGroups.List()...)
 		test.run(t)
 	}

test/testdata/bootstrappolicy/bootstrap_cluster_role_bindings.yaml

@@ -727,9 +727,9 @@ items:
   kind: ClusterRoleBinding
   metadata:
     creationTimestamp: null
-    name: edit
+    name: admin
   roleRef:
-    name: edit
+    name: admin
   subjects:
   - kind: ServiceAccount
     name: template-instance-controller