pre-upgrade job to verify function references and restricted privileges for fetcher and builder SA (#717)

It is mandatory (from this release onwards) for function to refer to secrets, config-maps and packages in its own namespace to ensure isolation of users sharing the same cluster.
This change runs a pre-upgrade job to verify function references for all functions created prior to this release and fails the upgrade by printing a list of functions that violate this restriction.

charts/fission-all/templates/deployment.yaml

@@ -95,7 +95,6 @@ kind: ServiceAccount
 metadata:
   name: fission-fetcher
   namespace: {{ .Values.functionNamespace }}
-
 ---
 apiVersion: v1
 kind: ServiceAccount

charts/fission-all/templates/pre-upgrade-job.yaml

@@ -0,0 +1,33 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: {{ template "fullname" . }}-{{ .Chart.Version }}-{{ randNumeric 3 }}
+  labels:
+    # The "release" convention makes it easy to tie a release to all of the
+    # Kubernetes resources that were created as part of that release.
+    release: "{{ .Release.Name }}"
+    # This makes it easy to audit chart usage.
+    chart: {{ .Chart.Name }}-{{ .Chart.Version }}
+    app: {{ template "name" . }}
+  annotations:
+    # This is what defines this resource as a hook. Without this line, the
+    # job is considered part of the release.
+    "helm.sh/hook": pre-upgrade
+    "helm.sh/hook-delete-policy": hook-succeeded
+spec:
+  backoffLimit: 0
+  template:
+    metadata:
+      name: {{ template "fullname" . }}
+      labels:
+        release: "{{ .Release.Name }}"
+        app: {{ template "name" . }}
+    spec:
+      restartPolicy: Never
+      containers:
+      - name: pre-upgrade-job
+        image: {{ .Values.preUpgradeChecksImage }}:{{ .Values.imageTag }}
+        imagePullPolicy: {{ .Values.pullPolicy }}
+        command: [ "/pre-upgrade-checks" ]
+        args: ["--fn-pod-namespace", "{{ .Values.functionNamespace }}", "--envbuilder-namespace", "{{ .Values.builderNamespace }}"]
+      serviceAccount: fission-svc

charts/fission-all/values.yaml

@@ -91,3 +91,6 @@ heapster: false
 ## This interval configures the frequency at which it runs inside the storagesvc pod.
 ## The value is in minutes.
 pruneInterval: 60
+
+## Fission pre-install/pre-upgrade checks live in this image
+preUpgradeChecksImage: fission/pre-upgrade-checks

charts/fission-core/templates/pre-upgrade-job.yaml

@@ -0,0 +1,33 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: {{ template "fullname" . }}-{{ .Chart.Version }}-{{ randNumeric 3 }}
+  labels:
+    # The "release" convention makes it easy to tie a release to all of the
+    # Kubernetes resources that were created as part of that release.
+    release: "{{ .Release.Name }}"
+    # This makes it easy to audit chart usage.
+    chart: {{ .Chart.Name }}-{{ .Chart.Version }}
+    app: {{ template "name" . }}
+  annotations:
+    # This is what defines this resource as a hook. Without this line, the
+    # job is considered part of the release.
+    "helm.sh/hook": pre-upgrade
+    "helm.sh/hook-delete-policy": hook-succeeded
+spec:
+  backoffLimit: 0
+  template:
+    metadata:
+      name: {{ template "fullname" . }}
+      labels:
+        release: "{{ .Release.Name }}"
+        app: {{ template "name" . }}
+    spec:
+      restartPolicy: Never
+      containers:
+      - name: pre-upgrade-job
+        image: {{ .Values.preUpgradeChecksImage }}:{{ .Values.imageTag }}
+        imagePullPolicy: {{ .Values.pullPolicy }}
+        command: [ "/pre-upgrade-checks" ]
+        args: ["--fn-pod-namespace", "{{ .Values.functionNamespace }}", "--envbuilder-namespace", "{{ .Values.builderNamespace }}"]
+      serviceAccount: fission-svc

charts/fission-core/values.yaml

@@ -62,3 +62,6 @@ analytics: true
 ## This interval configures the frequency at which it runs inside the storagesvc pod.
 ## The value is in minutes.
 pruneInterval: 60
+
+## Fission pre-install/pre-upgrade checks live in this image
+preUpgradeChecksImage: fission/pre-upgrade-checks

hack/release.sh

@@ -250,6 +250,31 @@ build_charts() {
     popd
 }
 
+
+# Build pre-upgrade-checks image
+build_pre_upgrade_checks_image() {
+    local version=$1
+    local date=$2
+    local gitcommit=$3
+
+    local tag=fission/pre-upgrade-checks:$version
+
+    pushd $DIR/preupgradechecks
+
+    ./build.sh $version $date $gitcommit
+    docker build -t $tag .
+    docker tag $tag fission/pre-upgrade-checks:latest
+
+    popd
+}
+
+# Push pre-upgrade-checks image
+push_pre_upgrade_checks_image() {
+    local version=$1
+    local tag=fission/pre-upgrade-checks:$version
+    docker push $tag
+}
+
 build_all() {
     local version=$1
 
@@ -289,6 +314,7 @@ build_all() {
     build_logger_image $version
     build_all_cli $version $date $gitcommit
     build_charts $version
+    build_pre_upgrade_checks_image $version $date $gitcommit
 }
 
 push_all() {
@@ -304,6 +330,9 @@ push_all() {
 
     push_logger_image $version
     push_logger_image latest
+
+    push_pre_upgrade_checks_image $version
+    push_pre_upgrade_checks_image latest
 }
 
 tag_and_release() {

preupgradechecks/Dockerfile

@@ -0,0 +1,3 @@
+FROM alpine:3.4
+RUN apk add --update ca-certificates
+ADD pre-upgrade-checks /

preupgradechecks/build.sh

@@ -0,0 +1,18 @@
+#!/bin/sh
+version=$1
+if [ -z $version ]; then
+    version=$(git rev-parse HEAD)
+fi
+
+date=$2
+if [ -z $date ]; then
+    date=$(date -u +'%Y-%m-%dT%H:%M:%SZ')
+fi
+
+gitcommit=$3
+if [ -z $gitcommit ]; then
+    gitcommit=$(git rev-parse HEAD)
+fi
+
+
+CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -gcflags=-trimpath=$GOPATH -asmflags=-trimpath=$GOPATH -ldflags "-X github.com/fission/fission.GitCommit=$gitcommit -X github.com/fission/fission.BuildDate=$date -X github.com/fission/fission.Version=$version" -o pre-upgrade-checks

preupgradechecks/main.go

@@ -0,0 +1,66 @@
+/*
+Copyright 2016 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 main
+
+import (
+	"os"
+
+	"github.com/docopt/docopt-go"
+	log "github.com/sirupsen/logrus"
+
+	"github.com/fission/fission"
+)
+
+func getStringArgWithDefault(arg interface{}, defaultValue string) string {
+	if arg != nil {
+		return arg.(string)
+	} else {
+		return defaultValue
+	}
+}
+
+func main() {
+	usage := `Package to perform operations needed prior to fission installation
+Usage:
+  pre-upgrade-checks --fn-pod-namespace=<podNamespace> --envbuilder-namespace=<envBuilderNamespace>
+Options:
+  --fn-pod-namespace=<podNamespace>                        Namespace where function pods get deployed.
+  --envbuilder-namespace=<envBuilderNamespace>             Namespace where builder env pods are deployed.`
+
+	arguments, err := docopt.Parse(usage, nil, true, fission.BuildInfo().String(), false)
+	if err != nil {
+		log.Fatalf("Error: %v", err)
+	}
+
+	functionPodNs := getStringArgWithDefault(arguments["--fn-pod-namespace"], "fission-function")
+	envBuilderNs := getStringArgWithDefault(arguments["--envbuilder-namespace"], "fission-builder")
+
+	crdBackedClient, err := makePreUpgradeTaskClient(functionPodNs, envBuilderNs)
+	if err != nil {
+		log.Printf("Error creating a crd client : %v, please retry helm upgrade", err)
+		os.Exit(1)
+	}
+
+	if !crdBackedClient.IsFissionReInstall() {
+		log.Printf("Nothing to do since CRDs are not present on the cluster")
+		return
+	}
+
+	crdBackedClient.VerifyFunctionSpecReferences()
+	crdBackedClient.RemoveClusterAdminRolesForFissionSAs()
+	crdBackedClient.SetupRoleBindings()
+}

preupgradechecks/preupgradechecks.go

@@ -0,0 +1,207 @@
+/*
+Copyright 2016 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 main
+
+import (
+	"fmt"
+	"os"
+
+	multierror "github.com/hashicorp/go-multierror"
+	log "github.com/sirupsen/logrus"
+	apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+
+	"github.com/fission/fission"
+	"github.com/fission/fission/crd"
+)
+
+type (
+	PreUpgradeTaskClient struct {
+		fissionClient *crd.FissionClient
+		k8sClient     *kubernetes.Clientset
+		apiExtClient  *apiextensionsclient.Clientset
+		fnPodNs       string
+		envBuilderNs  string
+	}
+)
+
+const (
+	MaxRetries  = 5
+	FunctionCRD = "functions.fission.io"
+)
+
+func fatal(msg string) {
+	os.Stderr.WriteString(msg + "\n")
+	os.Exit(1)
+}
+
+func makePreUpgradeTaskClient(fnPodNs, envBuilderNs string) (*PreUpgradeTaskClient, error) {
+	fissionClient, k8sClient, apiExtClient, err := crd.MakeFissionClient()
+	if err != nil {
+		log.Errorf("Error making fission client")
+		return nil, err
+	}
+
+	return &PreUpgradeTaskClient{
+		fissionClient: fissionClient,
+		k8sClient:     k8sClient,
+		fnPodNs:       fnPodNs,
+		envBuilderNs:  envBuilderNs,
+		apiExtClient:  apiExtClient,
+	}, nil
+}
+
+// IsFissionReInstall checks if there is atleast one fission CRD, i.e. function in this case, on this cluster.
+// We need this to find out if fission had been previously installed on this cluster
+func (client *PreUpgradeTaskClient) IsFissionReInstall() bool {
+	for i := 0; i < MaxRetries; i++ {
+		_, err := client.apiExtClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(FunctionCRD, metav1.GetOptions{})
+		if err != nil && k8serrors.IsNotFound(err) {
+			return false
+		}
+		if err == nil {
+			return true
+		}
+	}
+
+	return false
+}
+
+// VerifyFunctionSpecReferences verifies that a function references secrets, configmaps, pkgs in its own namespace and
+// outputs a list of functions that don't adhere to this requirement.
+func (client *PreUpgradeTaskClient) VerifyFunctionSpecReferences() {
+	log.Printf("Verifying Function spec references for all functions in the cluster")
+
+	var result *multierror.Error
+	var err error
+	var fList *crd.FunctionList
+
+	for i := 0; i < MaxRetries; i++ {
+		fList, err = client.fissionClient.Functions(metav1.NamespaceAll).List(metav1.ListOptions{})
+		if err == nil {
+			break
+		}
+	}
+
+	if err != nil {
+		fatal(fmt.Sprintf("Error: %v listing functions even after %d retries", err, MaxRetries))
+	}
+
+	// check that all secrets, configmaps, packages are in the same namespace
+	for _, fn := range fList.Items {
+		secrets := fn.Spec.Secrets
+		for _, secret := range secrets {
+			if secret.Namespace != fn.Metadata.Namespace {
+				result = multierror.Append(result, fmt.Errorf("function : %s.%s cannot reference a secret : %s in namespace : %s", fn.Metadata.Name, fn.Metadata.Namespace, secret.Name, secret.Namespace))
+			}
+		}
+
+		configmaps := fn.Spec.ConfigMaps
+		for _, configmap := range configmaps {
+			if configmap.Namespace != fn.Metadata.Namespace {
+				result = multierror.Append(result, fmt.Errorf("function : %s.%s cannot reference a configmap : %s in namespace : %s", fn.Metadata.Name, fn.Metadata.Namespace, configmap.Name, configmap.Namespace))
+			}
+		}
+
+		if fn.Spec.Package.PackageRef.Namespace != fn.Metadata.Namespace {
+			result = multierror.Append(result, fmt.Errorf("function : %s.%s cannot reference a package : %s in namespace : %s", fn.Metadata.Name, fn.Metadata.Namespace, fn.Spec.Package.PackageRef.Name, fn.Spec.Package.PackageRef.Namespace))
+		}
+	}
+
+	if result != nil {
+		log.Printf("Installation failed due to the following errors :")
+		log.Printf("Summary : A function cannot reference secrets, configmaps and packages outside it's own namespace")
+		fatal(result.Error())
+	}
+
+	log.Printf("Function Spec References verified")
+}
+
+// deleteClusterRoleBinding deletes the clusterRoleBinding passed as an argument to it.
+// If its not present, it just ignores and returns no errors
+func (client *PreUpgradeTaskClient) deleteClusterRoleBinding(clusterRoleBinding string) (err error) {
+	for i := 0; i < MaxRetries; i++ {
+		err = client.k8sClient.RbacV1beta1().ClusterRoleBindings().Delete(clusterRoleBinding, &metav1.DeleteOptions{})
+		if err != nil && k8serrors.IsNotFound(err) || err == nil {
+			return nil
+		}
+	}
+
+	return err
+}
+
+// RemoveClusterAdminRolesForFissionSAs deletes the clusterRoleBindings previously created on this cluster
+func (client *PreUpgradeTaskClient) RemoveClusterAdminRolesForFissionSAs() {
+	clusterRoleBindings := []string{"fission-builder-crd", "fission-fetcher-crd"}
+	for _, clusterRoleBinding := range clusterRoleBindings {
+		err := client.deleteClusterRoleBinding(clusterRoleBinding)
+		if err != nil {
+			fatal(fmt.Sprintf("Error deleting rolebinding : %s, err : %v", clusterRoleBinding, err))
+		}
+	}
+
+	log.Println("Removed cluster admin privileges for fission-builder and fission-fetcher Service Accounts")
+}
+
+// NeedRoleBindings checks if there is atleast one package or function in default namespace.
+// It is needed to find out if package-getter-rb and secret-configmap-getter-rb needs to be created for fission-fetcher
+// and fission-builder service accounts.
+// This is because, we just deleted the ClusterRoleBindings for these service accounts in the previous function and
+// for the existing functions to work, we need to give these SAs the right privileges
+func (client *PreUpgradeTaskClient) NeedRoleBindings() bool {
+	pkgList, err := client.fissionClient.Packages(metav1.NamespaceDefault).List(metav1.ListOptions{})
+	if err == nil && len(pkgList.Items) > 0 {
+		return true
+	}
+
+	fnList, err := client.fissionClient.Functions(metav1.NamespaceDefault).List(metav1.ListOptions{})
+	if err == nil && len(fnList.Items) > 0 {
+		return true
+	}
+
+	return false
+}
+
+// Setup appropriate role bindings for fission-fetcher and fission-builder SAs
+func (client *PreUpgradeTaskClient) SetupRoleBindings() {
+	if !client.NeedRoleBindings() {
+		log.Printf("No fission objects found, so no role-bindings to create")
+		return
+	}
+
+	// the fact that we're here implies that there had been a prior installation of fission and objects are present still
+	// so, we go ahead and create the role-bindings necessary for the fission-fetcher and fission-builder Service Accounts.
+	err := fission.SetupRoleBinding(client.k8sClient, fission.PackageGetterRB, metav1.NamespaceDefault, fission.PackageGetterCR, fission.ClusterRole, fission.FissionFetcherSA, client.fnPodNs)
+	if err != nil {
+		fatal(fmt.Sprintf("Error setting up rolebinding %s for %s.%s service account", fission.PackageGetterRB, fission.FissionFetcherSA, client.fnPodNs))
+	}
+
+	err = fission.SetupRoleBinding(client.k8sClient, fission.PackageGetterRB, metav1.NamespaceDefault, fission.PackageGetterCR, fission.ClusterRole, fission.FissionBuilderSA, client.envBuilderNs)
+	if err != nil {
+		fatal(fmt.Sprintf("Error setting up rolebinding %s for %s.%s service account", fission.PackageGetterRB, fission.FissionBuilderSA, client.envBuilderNs))
+	}
+
+	err = fission.SetupRoleBinding(client.k8sClient, fission.SecretConfigMapGetterRB, metav1.NamespaceDefault, fission.SecretConfigMapGetterCR, fission.ClusterRole, fission.FissionFetcherSA, client.fnPodNs)
+	if err != nil {
+		fatal(fmt.Sprintf("Error setting up rolebinding %s for %s.%s service account", fission.SecretConfigMapGetterRB, fission.FissionFetcherSA, client.fnPodNs))
+	}
+
+	log.Printf("Created role-bindings : %s and %s in default namespace", fission.PackageGetterRB, fission.SecretConfigMapGetterRB)
+	return
+}

test/build_and_test.sh

@@ -26,11 +26,14 @@ TAG=test
 PRUNE_INTERVAL=1 # this variable controls the interval to run archivePruner. The unit is in minutes.
 ROUTER_SERVICE_TYPE=LoadBalancer
 SERVICE_TYPE=LoadBalancer
+PRE_UPGRADE_CHECK_IMAGE=$REPO/pre-upgrade-checks
 
 dump_system_info
 
 build_and_push_fission_bundle $IMAGE:$TAG
 
+build_and_push_pre_upgrade_check_image $PRE_UPGRADE_CHECK_IMAGE:$TAG
+
 build_and_push_fetcher $FETCHER_IMAGE:$TAG
 
 build_and_push_builder $BUILDER_IMAGE:$TAG
@@ -45,4 +48,4 @@ build_and_push_fluentd $FLUENTD_IMAGE:$TAG
 
 build_fission_cli
 
-install_and_test $IMAGE $TAG $FETCHER_IMAGE $TAG $FLUENTD_IMAGE $TAG $PRUNE_INTERVAL $ROUTER_SERVICE_TYPE $SERVICE_TYPE
+install_and_test $IMAGE $TAG $FETCHER_IMAGE $TAG $FLUENTD_IMAGE $TAG $PRUNE_INTERVAL $ROUTER_SERVICE_TYPE $SERVICE_TYPE $PRE_UPGRADE_CHECK_IMAGE

test/test_utils.sh

@@ -54,6 +54,19 @@ gcloud_login() {
     gcloud auth activate-service-account --key-file $KEY
 }
 
+build_and_push_pre_upgrade_check_image() {
+    image_tag=$1
+
+    pushd $ROOT/preupgradechecks
+    ./build.sh
+    docker build -q -t $image_tag .
+
+    gcloud_login
+
+    gcloud docker -- push $image_tag
+    popd
+}
+
 build_and_push_fission_bundle() {
     image_tag=$1
 
@@ -177,11 +190,12 @@ helm_install_fission() {
     pruneInterval="${10}"
     routerServiceType=${11}
     serviceType=${12}
+    preUpgradeCheckImage=${13}
 
     ns=f-$id
     fns=f-func-$id
 
-    helmVars=image=$image,imageTag=$imageTag,fetcherImage=$fetcherImage,fetcherImageTag=$fetcherImageTag,functionNamespace=$fns,controllerPort=$controllerNodeport,routerPort=$routerNodeport,pullPolicy=Always,analytics=false,logger.fluentdImage=$fluentdImage,logger.fluentdImageTag=$fluentdImageTag,pruneInterval=$pruneInterval,routerServiceType=$routerServiceType,serviceType=$serviceType
+    helmVars=image=$image,imageTag=$imageTag,fetcherImage=$fetcherImage,fetcherImageTag=$fetcherImageTag,functionNamespace=$fns,controllerPort=$controllerNodeport,routerPort=$routerNodeport,pullPolicy=Always,analytics=false,logger.fluentdImage=$fluentdImage,logger.fluentdImageTag=$fluentdImageTag,pruneInterval=$pruneInterval,routerServiceType=$routerServiceType,serviceType=$serviceType,preUpgradeChecksImage=$preUpgradeCheckImage
 
     timeout 30 bash -c "helm_setup"
 
@@ -484,6 +498,8 @@ install_and_test() {
     pruneInterval=$7
     routerServiceType=$8
     serviceType=$9
+    preUpgradeCheckImage=${10}
+
 
     controllerPort=31234
     routerPort=31235
@@ -492,7 +508,7 @@ install_and_test() {
     
     id=$(generate_test_id)
     trap "helm_uninstall_fission $id" EXIT
-    helm_install_fission $id $image $imageTag $fetcherImage $fetcherImageTag $controllerPort $routerPort $fluentdImage $fluentdImageTag $pruneInterval $routerServiceType $serviceType
+    helm_install_fission $id $image $imageTag $fetcherImage $fetcherImageTag $controllerPort $routerPort $fluentdImage $fluentdImageTag $pruneInterval $routerServiceType $serviceType $preUpgradeCheckImage
     helm status $id | grep STATUS | grep -i deployed
     if [ $? -ne 0 ]; then
         describe_all_pods $id