template broker should use SAR, not impersonation

pkg/authorization/authorizer/subjects_test.go

@@ -49,6 +49,7 @@ func TestSubjects(t *testing.T) {
 			"system:serviceaccount:openshift-infra:build-controller",
 			"system:serviceaccount:openshift-infra:deployer-controller",
 			"system:serviceaccount:openshift-infra:deploymentconfig-controller",
+			"system:serviceaccount:openshift-infra:template-instance-controller",
 		),
 		expectedGroups: sets.NewString("RootUsers", "system:cluster-admins", "system:cluster-readers", "system:masters", "system:nodes"),
 	}

pkg/authorization/util/util.go

@@ -0,0 +1,41 @@
+package util
+
+import (
+	"errors"
+
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/kubernetes/pkg/apis/authorization"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion"
+)
+
+func AddUserToSAR(user user.Info, sar *authorization.SubjectAccessReview) *authorization.SubjectAccessReview {
+	sar.Spec.User = user.GetName()
+	copy(sar.Spec.Groups, user.GetGroups())
+	sar.Spec.Extra = map[string]authorization.ExtraValue{}
+
+	for k, v := range user.GetExtra() {
+		sar.Spec.Extra[k] = authorization.ExtraValue(v)
+	}
+
+	return sar
+}
+
+func Authorize(sarClient internalversion.SubjectAccessReviewInterface, user user.Info, resourceAttributes *authorization.ResourceAttributes) error {
+	sar := AddUserToSAR(user, &authorization.SubjectAccessReview{
+		Spec: authorization.SubjectAccessReviewSpec{
+			ResourceAttributes: resourceAttributes,
+		},
+	})
+
+	resp, err := sarClient.Create(sar)
+	if err == nil && resp != nil && resp.Status.Allowed {
+		return nil
+	}
+
+	if err == nil {
+		err = errors.New(resp.Status.Reason)
+	}
+	return kerrors.NewForbidden(schema.GroupResource{Group: resourceAttributes.Group, Resource: resourceAttributes.Resource}, resourceAttributes.Name, err)
+}

pkg/client/cache/index.go

@@ -7,12 +7,10 @@ import (
 	buildutil "github.com/openshift/origin/pkg/build/util"
 	deployapi "github.com/openshift/origin/pkg/deploy/api"
 	imageapi "github.com/openshift/origin/pkg/image/api"
-	templateapi "github.com/openshift/origin/pkg/template/api"
 )
 
 const (
 	ImageStreamReferenceIndex = "imagestreamref"
-	TemplateUIDIndex          = "templateuid"
 )
 
 // ImageStreamReferenceIndexFunc is a default index function that indexes based on image stream references.
@@ -75,7 +73,3 @@ func ImageStreamReferenceIndexFunc(obj interface{}) ([]string, error) {
 	}
 	return nil, fmt.Errorf("image stream reference index not implemented for %#v", obj)
 }
-
-func TemplateUIDIndexFunc(obj interface{}) ([]string, error) {
-	return []string{string(obj.(*templateapi.Template).UID)}, nil
-}

pkg/cmd/server/bootstrappolicy/constants.go

@@ -62,7 +62,7 @@ const (
 	RegistryViewerRoleName = "registry-viewer"
 	RegistryEditorRoleName = "registry-editor"
 
-	TemplateServiceBrokerClientRoleName = "templateservicebroker-client"
+	TemplateServiceBrokerClientRoleName = "system:openshift:templateservicebroker-client"
 
 	BuildStrategyDockerRoleName          = "system:build-strategy-docker"
 	BuildStrategyCustomRoleName          = "system:build-strategy-custom"
@@ -129,4 +129,6 @@ const (
 	BuildStrategyJenkinsPipelineRoleBindingName = BuildStrategyJenkinsPipelineRoleName + "-binding"
 
 	OpenshiftSharedResourceViewRoleBindingName = OpenshiftSharedResourceViewRoleName + "s"
+
+	TemplateInstanceControllerRoleBindingName = TemplateInstanceControllerRoleName
 )

pkg/cmd/server/bootstrappolicy/infra_sa_policy.go

@@ -6,17 +6,17 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	kapi "k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/apis/authorization"
 	"k8s.io/kubernetes/pkg/apis/certificates"
 	"k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/apis/storage"
 
-	// we need the conversions registered for our init block
 	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
-	_ "github.com/openshift/origin/pkg/authorization/api/install"
 	authorizationapiv1 "github.com/openshift/origin/pkg/authorization/api/v1"
 	buildapi "github.com/openshift/origin/pkg/build/api"
 	deployapi "github.com/openshift/origin/pkg/deploy/api"
 	imageapi "github.com/openshift/origin/pkg/image/api"
+	templateapi "github.com/openshift/origin/pkg/template/api"
 
 	// we need the conversions registered for our init block
 	_ "github.com/openshift/origin/pkg/authorization/api/install"
@@ -56,6 +56,17 @@ const (
 
 	InfraNodeBootstrapServiceAccountName = "node-bootstrapper"
 	NodeBootstrapRoleName                = "system:node-bootstrapper"
+
+	// template instance controller watches for TemplateInstance object creation
+	// and instantiates templates as a result.
+	InfraTemplateInstanceControllerServiceAccountName = "template-instance-controller"
+	TemplateInstanceControllerRoleName                = "system:openshift:controller:template-instance-controller"
+
+	// template service broker is an open service broker-compliant API
+	// implementation which serves up OpenShift templates.  It uses the
+	// TemplateInstance backend for most of the heavy lifting.
+	InfraTemplateServiceBrokerServiceAccountName = "template-service-broker"
+	TemplateServiceBrokerControllerRoleName      = "system:openshift:template-service-broker"
 )
 
 type InfraServiceAccounts struct {
@@ -588,4 +599,74 @@ func init() {
 		panic(err)
 	}
 
+	err = InfraSAs.addServiceAccount(
+		InfraTemplateInstanceControllerServiceAccountName,
+		authorizationapi.ClusterRole{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: TemplateInstanceControllerRoleName,
+			},
+			Rules: []authorizationapi.PolicyRule{
+				{
+					APIGroups: []string{authorization.GroupName},
+					Verbs:     sets.NewString("create"),
+					Resources: sets.NewString("subjectaccessreviews"),
+				},
+				{
+					APIGroups: []string{templateapi.GroupName},
+					Verbs:     sets.NewString("get", "list", "watch"),
+					Resources: sets.NewString("templateinstances"),
+				},
+				{
+					APIGroups: []string{templateapi.GroupName},
+					Verbs:     sets.NewString("update"),
+					Resources: sets.NewString("templateinstances/status"),
+				},
+			},
+		},
+	)
+	if err != nil {
+		panic(err)
+	}
+
+	err = InfraSAs.addServiceAccount(
+		InfraTemplateServiceBrokerServiceAccountName,
+		authorizationapi.ClusterRole{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: TemplateServiceBrokerControllerRoleName,
+			},
+			Rules: []authorizationapi.PolicyRule{
+				{
+					APIGroups: []string{authorization.GroupName},
+					Verbs:     sets.NewString("create"),
+					Resources: sets.NewString("subjectaccessreviews"),
+				},
+				{
+					APIGroups: []string{templateapi.GroupName},
+					Verbs:     sets.NewString("get", "create", "update", "delete"),
+					Resources: sets.NewString("brokertemplateinstances"),
+				},
+				{
+					APIGroups: []string{templateapi.GroupName},
+					// "assign" is required for the API server to accept creation of
+					// TemplateInstance objects with the requester username set to an
+					// identity which is not the API caller.
+					Verbs:     sets.NewString("get", "create", "delete", "assign"),
+					Resources: sets.NewString("templateinstances"),
+				},
+				{
+					APIGroups: []string{kapi.GroupName},
+					Verbs:     sets.NewString("get", "list", "create", "delete"),
+					Resources: sets.NewString("secrets"),
+				},
+				{
+					APIGroups: []string{kapi.GroupName},
+					Verbs:     sets.NewString("list"),
+					Resources: sets.NewString("services"),
+				},
+			},
+		},
+	)
+	if err != nil {
+		panic(err)
+	}
 }

pkg/cmd/server/bootstrappolicy/policy.go

@@ -1185,6 +1185,22 @@ func GetOpenshiftBootstrapClusterRoleBindings() []authorizationapi.ClusterRoleBi
 			RoleRef:    kapi.ObjectReference{Name: BuildStrategyJenkinsPipelineRoleName},
 			Subjects:   []kapi.ObjectReference{{Kind: authorizationapi.SystemGroupKind, Name: AuthenticatedGroup}},
 		},
+
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: TemplateInstanceControllerRoleBindingName,
+			},
+			RoleRef: kapi.ObjectReference{
+				Name: EditRoleName,
+			},
+			Subjects: []kapi.ObjectReference{
+				{
+					Kind:      authorizationapi.ServiceAccountKind,
+					Name:      InfraTemplateInstanceControllerServiceAccountName,
+					Namespace: DefaultOpenShiftInfraNamespace,
+				},
+			},
+		},
 	}
 }
 

pkg/cmd/server/origin/master.go

@@ -512,9 +512,9 @@ func (c *MasterConfig) InstallProtectedAPI(server *apiserver.GenericAPIServer) (
 			templateapi.ServiceBrokerRoot,
 			templateservicebroker.NewBroker(
 				c.PrivilegedLoopbackClientConfig,
-				c.PrivilegedLoopbackOpenShiftClient,
-				c.PrivilegedLoopbackKubernetesClientsetInternal.Core(),
-				c.Informers,
+				c.PrivilegedLoopbackKubernetesClientsetInternal,
+				c.Options.PolicyConfig.OpenShiftInfrastructureNamespace,
+				c.TemplateInformers.Template().InternalVersion().Templates(),
 				c.Options.TemplateServiceBrokerConfig.TemplateNamespaces,
 			),
 		)
@@ -895,12 +895,13 @@ func (c *MasterConfig) GetRestStorage() map[schema.GroupVersion]map[string]rest.
 	}
 
 	if c.Options.TemplateServiceBrokerConfig != nil {
-		templateInstanceStorage, err := templateinstanceetcd.NewREST(c.RESTOptionsGetter, c.PrivilegedLoopbackOpenShiftClient)
+		templateInstanceStorage, templateInstanceStatusStorage, err := templateinstanceetcd.NewREST(c.RESTOptionsGetter, c.PrivilegedLoopbackKubernetesClientsetInternal)
 		checkStorageErr(err)
 		brokerTemplateInstanceStorage, err := brokertemplateinstanceetcd.NewREST(c.RESTOptionsGetter)
 		checkStorageErr(err)
 
 		storage[templateapiv1.SchemeGroupVersion]["templateinstances"] = templateInstanceStorage
+		storage[templateapiv1.SchemeGroupVersion]["templateinstances/status"] = templateInstanceStatusStorage
 		storage[templateapiv1.SchemeGroupVersion]["brokertemplateinstances"] = brokerTemplateInstanceStorage
 	}
 

pkg/cmd/server/origin/master_config.go

@@ -97,6 +97,8 @@ import (
 	"github.com/openshift/origin/pkg/service"
 	serviceadmit "github.com/openshift/origin/pkg/service/admission"
 	"github.com/openshift/origin/pkg/serviceaccounts"
+	templateinformer "github.com/openshift/origin/pkg/template/generated/informers/internalversion"
+	templateclient "github.com/openshift/origin/pkg/template/generated/internalclientset"
 	usercache "github.com/openshift/origin/pkg/user/cache"
 	groupregistry "github.com/openshift/origin/pkg/user/registry/group"
 	groupstorage "github.com/openshift/origin/pkg/user/registry/group/etcd"
@@ -185,6 +187,7 @@ type MasterConfig struct {
 	Informers shared.InformerFactory
 
 	AuthorizationInformers authorizationinformer.SharedInformerFactory
+	TemplateInformers      templateinformer.SharedInformerFactory
 }
 
 // BuildMasterConfig builds and returns the OpenShift master configuration based on the
@@ -225,6 +228,10 @@ func BuildMasterConfig(options configapi.MasterConfig) (*MasterConfig, error) {
 	if err != nil {
 		return nil, err
 	}
+	templateClient, err := templateclient.NewForConfig(privilegedLoopbackClientConfig)
+	if err != nil {
+		return nil, err
+	}
 
 	customListerWatchers := shared.DefaultListerWatcherOverrides{}
 	if err := addAuthorizationListerWatchers(customListerWatchers, restOptsGetter); err != nil {
@@ -236,6 +243,7 @@ func BuildMasterConfig(options configapi.MasterConfig) (*MasterConfig, error) {
 	internalkubeInformerFactory := kinternalinformers.NewSharedInformerFactory(privilegedLoopbackKubeClientsetInternal, defaultInformerResyncPeriod)
 	externalkubeInformerFactory := kinformers.NewSharedInformerFactory(privilegedLoopbackKubeClientsetExternal, defaultInformerResyncPeriod)
 	authorizationInformers := authorizationinformer.NewSharedInformerFactory(authorizationClient, defaultInformerResyncPeriod)
+	templateInformers := templateinformer.NewSharedInformerFactory(templateClient, defaultInformerResyncPeriod)
 	informerFactory := shared.NewInformerFactory(internalkubeInformerFactory, externalkubeInformerFactory, privilegedLoopbackKubeClientsetInternal, privilegedLoopbackOpenShiftClient, customListerWatchers, defaultInformerResyncPeriod)
 
 	imageTemplate := variable.NewDefaultImageTemplate()
@@ -346,6 +354,7 @@ func BuildMasterConfig(options configapi.MasterConfig) (*MasterConfig, error) {
 		PrivilegedLoopbackKubernetesClientsetExternal: privilegedLoopbackKubeClientsetExternal,
 		Informers:              informerFactory,
 		AuthorizationInformers: authorizationInformers,
+		TemplateInformers:      templateInformers,
 	}
 
 	// ensure that the limit range informer will be started

pkg/cmd/server/origin/run_components.go

@@ -37,6 +37,7 @@ import (
 	osclient "github.com/openshift/origin/pkg/client"
 	oscache "github.com/openshift/origin/pkg/client/cache"
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
+	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
 	deployclient "github.com/openshift/origin/pkg/deploy/generated/internalclientset/typed/apps/internalversion"
@@ -59,6 +60,7 @@ import (
 	servingcertcontroller "github.com/openshift/origin/pkg/service/controller/servingcert"
 	serviceaccountcontrollers "github.com/openshift/origin/pkg/serviceaccounts/controllers"
 	templatecontroller "github.com/openshift/origin/pkg/template/controller"
+	templateclient "github.com/openshift/origin/pkg/template/generated/internalclientset"
 	unidlingcontroller "github.com/openshift/origin/pkg/unidling/controller"
 )
 
@@ -569,7 +571,17 @@ func (c *MasterConfig) RunUnidlingController() {
 }
 
 func (c *MasterConfig) RunTemplateController() {
-	go templatecontroller.NewTemplateInstanceController(c.PrivilegedLoopbackClientConfig).Run(utilwait.NeverStop)
+	config, oc, kc, _, err := c.GetServiceAccountClients(bootstrappolicy.InfraTemplateInstanceControllerServiceAccountName)
+	if err != nil {
+		glog.Fatalf("Could not get client: %v", err)
+	}
+
+	templateClient, err := templateclient.NewForConfig(config)
+	if err != nil {
+		glog.Fatalf("Could not get client: %v", err)
+	}
+
+	go templatecontroller.NewTemplateInstanceController(oc, kc, templateClient.TemplateClient, c.TemplateInformers.Template().InternalVersion().TemplateInstances()).Run(5, utilwait.NeverStop)
 }
 
 func (c *MasterConfig) RunOriginToRBACSyncControllers() {

pkg/cmd/server/start/start_master.go

@@ -447,12 +447,14 @@ func (m *Master) Start() error {
 			openshiftConfig.Informers.Start(utilwait.NeverStop)
 			openshiftConfig.Informers.StartCore(utilwait.NeverStop)
 			openshiftConfig.AuthorizationInformers.Start(utilwait.NeverStop)
+			openshiftConfig.TemplateInformers.Start(utilwait.NeverStop)
 		}()
 	} else {
 		openshiftConfig.Informers.InternalKubernetesInformers().Start(utilwait.NeverStop)
 		openshiftConfig.Informers.KubernetesInformers().Start(utilwait.NeverStop)
 		openshiftConfig.Informers.Start(utilwait.NeverStop)
 		openshiftConfig.AuthorizationInformers.Start(utilwait.NeverStop)
+		openshiftConfig.TemplateInformers.Start(utilwait.NeverStop)
 	}
 
 	return nil