Enabling multi-tenancy for fission objects. (#655)

This feature allows creation of fission objects in different namespaces, in addition to retaining the existing behavior of creating fission objects in default namespace if user doesnt provide one. 
It also removes cluster admin roles for fission-fetcher and fission-builder Service Accounts and grants them only those privileges that they need.

executor/poolmgr/funcwatcher.go

@@ -26,7 +26,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes"
 	apiv1 "k8s.io/client-go/pkg/api/v1"
-	"k8s.io/client-go/rest"
 	k8sCache "k8s.io/client-go/tools/cache"
 
 	"github.com/fission/fission"
@@ -39,15 +38,15 @@ func getIstioServiceLabels(fnName string) map[string]string {
 	}
 }
 
-func makeFuncIstioServiceRegister(crdClient *rest.RESTClient,
-	kubernetesClient *kubernetes.Clientset, fnNamespace string) k8sCache.Controller {
+func (gpm *GenericPoolManager) makeFuncController(fissionClient *crd.FissionClient,
+	kubernetesClient *kubernetes.Clientset, fissionfnNamespace string, istioEnabled bool) (k8sCache.Store, k8sCache.Controller) {
 
 	resyncPeriod := 30 * time.Second
-	lw := k8sCache.NewListWatchFromClient(crdClient, "functions", metav1.NamespaceDefault, fields.Everything())
-	_, controller := k8sCache.NewInformer(lw, &crd.Function{}, resyncPeriod,
+	lw := k8sCache.NewListWatchFromClient(fissionClient.GetCrdClient(), "functions", metav1.NamespaceAll, fields.Everything())
+
+	funcStore, controller := k8sCache.NewInformer(lw, &crd.Function{}, resyncPeriod,
 		k8sCache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
-
 				fn := obj.(*crd.Function)
 
 				// Since istio only allows accessing pod through k8s service,
@@ -56,70 +55,140 @@ func makeFuncIstioServiceRegister(crdClient *rest.RESTClient,
 				// Functions with executor type "Newdeploy" is specialized at
 				// pod starts. In this case, just ignore such functions.
 				fnExecutorType := fn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType
-				if fnExecutorType != fission.ExecutorTypePoolmgr {
+
+				// In some cases, user may not enter the executorType explicitly, for example in his spec.yaml.
+				// we assume it to be of type poolmgr
+				if fnExecutorType != "" && fnExecutorType != fission.ExecutorTypePoolmgr {
 					return
 				}
 
-				// create a same name service for function
-				// since istio only allows the traffic to service
-				sel := map[string]string{
-					"functionName": fn.Metadata.Name,
-					"functionUid":  string(fn.Metadata.UID),
+				// create or update role-binding
+				envNs := fissionfnNamespace
+				if fn.Spec.Environment.Namespace != metav1.NamespaceDefault {
+					envNs = fn.Spec.Environment.Namespace
 				}
 
-				svcName := fission.GetFunctionIstioServiceName(fn.Metadata.Name, fn.Metadata.Namespace)
+				// TODO : Just bring to your attention during review :
+				// setup rolebinding is tried, if it fails, we dont return. we just log an error and move on, because :
+				// 1. not all functions have secrets and/or configmaps, so things will work without this rolebinding in that case.
+				// 2. on the contrary, when the route is tried, the env fetcher logs will show a 403 forbidden message and same will be relayed to executor.
+				err := fission.SetupRoleBinding(kubernetesClient, fission.SecretConfigMapGetterRB, fn.Metadata.Namespace, fission.SecretConfigMapGetterCR, fission.ClusterRole, fission.FissionFetcherSA, envNs)
+				if err != nil {
+					log.Printf("Error : %v creating %s RoleBinding", err, fission.SecretConfigMapGetterRB)
+				} else {
+					log.Printf("Successfully set up rolebinding for fetcher SA: %s.%s, in func's ns for func : %s", fission.FissionFetcherSA, envNs, fn.Metadata.Name, fn.Metadata.Namespace)
+				}
 
-				// service for accepting user traffic
-				svc := apiv1.Service{
-					ObjectMeta: metav1.ObjectMeta{
-						Namespace: fnNamespace,
-						Name:      svcName,
-						Labels:    getIstioServiceLabels(fn.Metadata.Name),
-					},
-					Spec: apiv1.ServiceSpec{
-						Type: apiv1.ServiceTypeClusterIP,
-						Ports: []apiv1.ServicePort{
-							// Service port name should begin with a recognized prefix, or the traffic will be
-							// treated as TCP traffic. (https://istio.io/docs/setup/kubernetes/sidecar-injection.html)
-							// Originally the ports' name are similar to "http-fetch" and "http-specialize".
-							// But for istio 0.5.1, istio-proxy return unexpected 431 error with such naming.
-							// https://github.com/istio/istio/issues/928
-							// Workaround: remove prefix
-							// TODO: prepend prefix once the bug fixed
-							{
-								Name:       "fetch",
-								Protocol:   apiv1.ProtocolTCP,
-								Port:       8000,
-								TargetPort: intstr.FromInt(8000),
-							},
-							{
-								Name:       "specialize",
-								Protocol:   apiv1.ProtocolTCP,
-								Port:       8888,
-								TargetPort: intstr.FromInt(8888),
-							},
+				if istioEnabled {
+					// create a same name service for function
+					// since istio only allows the traffic to service
+					sel := map[string]string{
+						"functionName": fn.Metadata.Name,
+						"functionUid":  string(fn.Metadata.UID),
+					}
+
+					svcName := fission.GetFunctionIstioServiceName(fn.Metadata.Name, fn.Metadata.Namespace)
+
+					// service for accepting user traffic
+					svc := apiv1.Service{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: envNs,
+							Name:      svcName,
+							Labels:    getIstioServiceLabels(fn.Metadata.Name),
 						},
-						Selector: sel,
-					},
-				}
+						Spec: apiv1.ServiceSpec{
+							Type: apiv1.ServiceTypeClusterIP,
+							Ports: []apiv1.ServicePort{
+								// Service port name should begin with a recognized prefix, or the traffic will be
+								// treated as TCP traffic. (https://istio.io/docs/setup/kubernetes/sidecar-injection.html)
+								// Originally the ports' name are similar to "http-fetch" and "http-specialize".
+								// But for istio 0.5.1, istio-proxy return unexpected 431 error with such naming.
+								// https://github.com/istio/istio/issues/928
+								// Workaround: remove prefix
+								// TODO: prepend prefix once the bug fixed
+								{
+									Name:       "fetch",
+									Protocol:   apiv1.ProtocolTCP,
+									Port:       8000,
+									TargetPort: intstr.FromInt(8000),
+								},
+								{
+									Name:       "specialize",
+									Protocol:   apiv1.ProtocolTCP,
+									Port:       8888,
+									TargetPort: intstr.FromInt(8888),
+								},
+							},
+							Selector: sel,
+						},
+					}
 
-				// create function istio service if it does not exist
-				_, err := kubernetesClient.CoreV1().Services(fnNamespace).Create(&svc)
-				if err != nil && !kerrors.IsAlreadyExists(err) {
-					log.Printf("Error creating function istio service: %v", err)
+					// create function istio service if it does not exist
+					_, err = kubernetesClient.CoreV1().Services(envNs).Create(&svc)
+					if err != nil && !kerrors.IsAlreadyExists(err) {
+						log.Printf("Error creating function istio service: %v", err)
+					}
 				}
 			},
+
 			DeleteFunc: func(obj interface{}) {
 				fn := obj.(*crd.Function)
-				svcName := fission.GetFunctionIstioServiceName(fn.Metadata.Name, fn.Metadata.Namespace)
-				// delete function istio service
-				err := kubernetesClient.CoreV1().Services(fnNamespace).Delete(svcName, nil)
-				if err != nil && !kerrors.IsNotFound(err) {
-					log.Printf("Error deleting function istio service: %v", err)
+
+				fnExecutorType := fn.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType
+				if fnExecutorType != "" && fnExecutorType != fission.ExecutorTypePoolmgr {
+					return
+				}
+
+				envNs := fissionfnNamespace
+				if fn.Spec.Environment.Namespace != metav1.NamespaceDefault {
+					envNs = fn.Spec.Environment.Namespace
+				}
+
+				if istioEnabled {
+					svcName := fission.GetFunctionIstioServiceName(fn.Metadata.Name, fn.Metadata.Namespace)
+					// delete function istio service
+					err := kubernetesClient.CoreV1().Services(envNs).Delete(svcName, nil)
+					if err != nil && !kerrors.IsNotFound(err) {
+						log.Printf("Error deleting function istio service: %v", err)
+					}
+				}
+			},
+
+			UpdateFunc: func(oldObj, newObj interface{}) {
+				oldFunc := oldObj.(*crd.Function)
+				newFunc := newObj.(*crd.Function)
+
+				if oldFunc.Metadata.ResourceVersion == newFunc.Metadata.ResourceVersion {
+					return
+				}
+
+				envChanged := (oldFunc.Spec.Environment.Namespace != newFunc.Spec.Environment.Namespace)
+
+				executorTypeChangedToPM := (oldFunc.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType != fission.ExecutorTypePoolmgr &&
+					newFunc.Spec.InvokeStrategy.ExecutionStrategy.ExecutorType == fission.ExecutorTypePoolmgr)
+
+				// if a func's env reference gets updated and the newly referenced env is in a different ns,
+				// we need to create a rolebinding in func's ns so that the fetcher-sa in env ns has access
+				// to fetch secrets and config maps from the func's ns.
+				// similarly if executorType changed to Pool Manager, we now need a rolebinding in the func ns for fetcher sa
+				// present in env ns because for newdeploy, the fetcher sa is in function namespace
+				if envChanged || executorTypeChangedToPM {
+					envNs := fissionfnNamespace
+					if newFunc.Spec.Environment.Namespace != metav1.NamespaceDefault {
+						envNs = newFunc.Spec.Environment.Namespace
+					}
+
+					err := fission.SetupRoleBinding(kubernetesClient, fission.SecretConfigMapGetterRB,
+						newFunc.Metadata.Namespace, fission.SecretConfigMapGetterCR, fission.ClusterRole,
+						fission.FissionFetcherSA, envNs)
+					if err != nil {
+						log.Printf("Error : %v creating GetSecretConfigMapRoleBinding", err)
+					} else {
+						log.Printf("Set up rolebinding for fetcher SA in func's env ns : %s, in func's ns : %s, for func : %s", newFunc.Spec.Environment.Namespace, newFunc.Metadata.Namespace, newFunc.Metadata.Name)
+					}
 				}
 			},
-			UpdateFunc: func(oldObj, newObj interface{}) {},
 		})
 
-	return controller
+	return funcStore, controller
 }

executor/poolmgr/gp.go

@@ -56,6 +56,7 @@ type (
 		replicas               int32                         // num idle pods
 		deployment             *v1beta1.Deployment           // kubernetes deployment
 		namespace              string                        // namespace to keep our resources
+		functionNamespace      string                        // fallback namespace for fission functions
 		podReadyTimeout        time.Duration                 // timeout for generic pods to become ready
 		idlePodReapTime        time.Duration                 // pods unused for idlePodReapTime are deleted
 		fsCache                *fscache.FunctionServiceCache // cache funcSvc's by function, address and podname
@@ -103,6 +104,7 @@ func MakeGenericPool(
 	env *crd.Environment,
 	initialReplicas int32,
 	namespace string,
+	functionNamespace string,
 	fsCache *fscache.FunctionServiceCache,
 	instanceId string,
 	enableIstio bool) (*GenericPool, error) {
@@ -125,23 +127,24 @@ func MakeGenericPool(
 	// TODO: in general we need to provide the user a way to configure pools.  Initial
 	// replicas, autoscaling params, various timeouts, etc.
 	gp := &GenericPool{
-		env:              env,
-		replicas:         initialReplicas, // TODO make this an env param instead?
-		requestChannel:   make(chan *choosePodRequest),
-		fissionClient:    fissionClient,
-		kubernetesClient: kubernetesClient,
-		namespace:        namespace,
-		podReadyTimeout:  5 * time.Minute, // TODO make this an env param?
-		idlePodReapTime:  3 * time.Minute, // TODO make this configurable
-		fsCache:          fsCache,
-		poolInstanceId:   uniuri.NewLen(8),
-		instanceId:       instanceId,
-		fetcherImage:     fetcherImage,
-		useSvc:           false,       // defaults off -- svc takes a second or more to become routable, slowing cold start
-		useIstio:         enableIstio, // defaults off -- istio integration requires pod relabeling and it takes a second or more to become routable, slowing cold start
-		sharedMountPath:  "/userfunc", // change this may break v1 compatibility, since most of the v1 environments have hard-coded "/userfunc" in loading path
-		sharedSecretPath: "/secrets",
-		sharedCfgMapPath: "/configs",
+		env:               env,
+		replicas:          initialReplicas, // TODO make this an env param instead?
+		requestChannel:    make(chan *choosePodRequest),
+		fissionClient:     fissionClient,
+		kubernetesClient:  kubernetesClient,
+		namespace:         namespace,
+		functionNamespace: functionNamespace,
+		podReadyTimeout:   5 * time.Minute, // TODO make this an env param?
+		idlePodReapTime:   3 * time.Minute, // TODO make this configurable
+		fsCache:           fsCache,
+		poolInstanceId:    uniuri.NewLen(8),
+		instanceId:        instanceId,
+		fetcherImage:      fetcherImage,
+		useSvc:            false,       // defaults off -- svc takes a second or more to become routable, slowing cold start
+		useIstio:          enableIstio, // defaults off -- istio integration requires pod relabeling and it takes a second or more to become routable, slowing cold start
+		sharedMountPath:   "/userfunc", // change this may break v1 compatibility, since most of the v1 environments have hard-coded "/userfunc" in loading path
+		sharedSecretPath:  "/secrets",
+		sharedCfgMapPath:  "/configs",
 	}
 
 	gp.runtimeImagePullPolicy = getImagePullPolicy(runtimeImagePullPolicy)
@@ -149,6 +152,13 @@ func MakeGenericPool(
 	gp.fetcherImagePullPolicy = getImagePullPolicy(fetcherImagePullPolicy)
 	log.Printf("fetcher image: %v, pull policy: %v", gp.fetcherImage, gp.fetcherImagePullPolicy)
 
+	// create fetcher SA in this ns, if not already created
+	_, err := fission.SetupSA(gp.kubernetesClient, fission.FissionFetcherSA, gp.namespace)
+	if err != nil {
+		log.Printf("Error : %v creating fetcher SA in ns : %s", err, gp.namespace)
+		return nil, err
+	}
+
 	// Labels for generic deployment/RS/pods.
 	gp.labelsForPool = map[string]string{
 		"environmentName":                 gp.env.Metadata.Name,
@@ -158,7 +168,7 @@ func MakeGenericPool(
 	}
 
 	// create the pool
-	err := gp.createPool()
+	err = gp.createPool()
 	if err != nil {
 		return nil, err
 	}

executor/poolmgr/gpm.go

@@ -52,8 +52,11 @@ type (
 		instanceId     string
 		requestChannel chan *request
 
-		enableIstio          bool
-		istioServiceRegister k8sCache.Controller
+		enableIstio    bool
+		funcStore      k8sCache.Store
+		funcController k8sCache.Controller
+		pkgStore       k8sCache.Store
+		pkgController  k8sCache.Controller
 	}
 	request struct {
 		requestType
@@ -92,20 +95,19 @@ func MakeGenericPoolManager(
 			log.Println("Failed to parse ENABLE_ISTIO")
 		}
 		gpm.enableIstio = istio
-
-		if gpm.enableIstio {
-			gpm.istioServiceRegister = makeFuncIstioServiceRegister(
-				gpm.fissionClient.GetCrdClient(), gpm.kubernetesClient, functionNamespace)
-		}
 	}
 
+	gpm.funcStore, gpm.funcController = gpm.makeFuncController(
+		gpm.fissionClient, gpm.kubernetesClient, gpm.namespace, gpm.enableIstio)
+
+	gpm.pkgStore, gpm.pkgController = gpm.makePkgController(gpm.fissionClient, gpm.kubernetesClient, gpm.namespace)
+
 	return gpm
 }
 
 func (gpm *GenericPoolManager) Run(ctx context.Context) {
-	if gpm.enableIstio && gpm.istioServiceRegister != nil {
-		go gpm.istioServiceRegister.Run(ctx.Done())
-	}
+	go gpm.funcController.Run(ctx.Done())
+	go gpm.pkgController.Run(ctx.Done())
 }
 
 func (gpm *GenericPoolManager) service() {
@@ -113,6 +115,7 @@ func (gpm *GenericPoolManager) service() {
 		req := <-gpm.requestChannel
 		switch req.requestType {
 		case GET_POOL:
+			// just because they are missing in the cache, we end up creating another duplicate pool.
 			var err error
 			pool, ok := gpm.pools[crd.CacheKey(&req.env.Metadata)]
 			if !ok {
@@ -122,9 +125,16 @@ func (gpm *GenericPoolManager) service() {
 					poolsize = 1
 				}
 
+				// To support backward compatibility, if envs are created in default ns, we go ahead
+				// and create pools in fission-function ns as earlier.
+				ns := gpm.namespace
+				if req.env.Metadata.Namespace != metav1.NamespaceDefault {
+					ns = req.env.Metadata.Namespace
+				}
+
 				pool, err = MakeGenericPool(
 					gpm.fissionClient, gpm.kubernetesClient, req.env, poolsize,
-					gpm.namespace, gpm.fsCache, gpm.instanceId, gpm.enableIstio)
+					ns, gpm.namespace, gpm.fsCache, gpm.instanceId, gpm.enableIstio)
 				if err != nil {
 					req.responseChannel <- &response{error: err}
 					continue
@@ -133,15 +143,12 @@ func (gpm *GenericPoolManager) service() {
 			}
 			req.responseChannel <- &response{pool: pool}
 		case CLEANUP_POOLS:
-			latestEnvSet := make(map[string]bool)
 			latestEnvPoolsize := make(map[string]int)
 			for _, env := range req.envList {
-				latestEnvSet[crd.CacheKey(&env.Metadata)] = true
 				latestEnvPoolsize[crd.CacheKey(&env.Metadata)] = int(gpm.getEnvPoolsize(&env))
 			}
 			for key, pool := range gpm.pools {
-				_, ok := latestEnvSet[key]
-				poolsize := latestEnvPoolsize[key]
+				poolsize, ok := latestEnvPoolsize[key]
 				if !ok || poolsize == 0 {
 					// Env no longer exists or pool size changed to zero
 

executor/poolmgr/packagewatcher.go

@@ -0,0 +1,91 @@
+/*
+Copyright 2018 The Fission Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package poolmgr
+
+import (
+	"time"
+
+	log "github.com/sirupsen/logrus"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/client-go/kubernetes"
+	k8sCache "k8s.io/client-go/tools/cache"
+
+	"github.com/fission/fission"
+	"github.com/fission/fission/crd"
+)
+
+// TODO : It may make sense to make each of add, update, delete funcs run as separate go routines.
+func (gpm *GenericPoolManager) makePkgController(fissionClient *crd.FissionClient,
+	kubernetesClient *kubernetes.Clientset, fissionfnNamespace string) (k8sCache.Store, k8sCache.Controller) {
+
+	resyncPeriod := 30 * time.Second
+	lw := k8sCache.NewListWatchFromClient(fissionClient.GetCrdClient(), "packages", metav1.NamespaceAll, fields.Everything())
+	pkgStore, controller := k8sCache.NewInformer(lw, &crd.Package{}, resyncPeriod,
+		k8sCache.ResourceEventHandlerFuncs{
+			AddFunc: func(obj interface{}) {
+				pkg := obj.(*crd.Package)
+				log.Printf("List watch for package reported a new package addition: %s.%s", pkg.Metadata.Name, pkg.Metadata.Namespace)
+
+				// create or update role-binding for fetcher sa in env ns to be able to get the pkg contents from pkg namespace
+				envNs := fissionfnNamespace
+				if pkg.Spec.Environment.Namespace != metav1.NamespaceDefault {
+					envNs = pkg.Spec.Environment.Namespace
+				}
+
+				// here, we return if we hit an error during rolebinding setup. this is because this rolebinding is mandatory for
+				// every function's package to be loaded into its env. without that, there's no point to move forward.
+				err := fission.SetupRoleBinding(kubernetesClient, fission.PackageGetterRB, pkg.Metadata.Namespace, fission.PackageGetterCR, fission.ClusterRole, fission.FissionFetcherSA, envNs)
+				if err != nil {
+					log.Printf("Error creating %s for package: %s.%s, err: %v", fission.PackageGetterRB, pkg.Metadata.Name, pkg.Metadata.Namespace, err)
+					return
+				}
+
+				log.Printf("Successfully set up rolebinding for fetcher SA: %s.%s, in packages's ns : %s, for pkg : %s", fission.FissionFetcherSA, envNs, pkg.Metadata.Namespace, pkg.Metadata.Name)
+			},
+
+			UpdateFunc: func(oldObj, newObj interface{}) {
+				oldPkg := oldObj.(*crd.Package)
+				newPkg := newObj.(*crd.Package)
+
+				if oldPkg.Metadata.ResourceVersion == newPkg.Metadata.ResourceVersion {
+					return
+				}
+
+				// if a pkg's env reference gets updated and the newly referenced env is in a different ns,
+				// we need to update the role-binding in pkg ns to grant permissions to the fetcher-sa in env ns
+				// to do a get on pkg
+				if oldPkg.Spec.Environment.Namespace != newPkg.Spec.Environment.Namespace {
+					envNs := fissionfnNamespace
+					if newPkg.Spec.Environment.Namespace != metav1.NamespaceDefault {
+						envNs = newPkg.Spec.Environment.Namespace
+					}
+
+					err := fission.SetupRoleBinding(kubernetesClient, fission.PackageGetterRB,
+						newPkg.Metadata.Namespace, fission.PackageGetterCR, fission.ClusterRole,
+						fission.FissionFetcherSA, envNs)
+					if err != nil {
+						log.Printf("Error : %v updating %s for package: %s.%s", err, fission.PackageGetterRB, newPkg.Metadata.Name, newPkg.Metadata.Namespace)
+						return
+					}
+					log.Printf("Updated rolebinding for fetcher SA: %s.%s, in packages's ns : %s, for pkg : %s", fission.FissionFetcherSA, envNs, newPkg.Metadata.Namespace, newPkg.Metadata.Name)
+				}
+			},
+		})
+
+	return pkgStore, controller
+}