From 2e37611da809525f9c456c2336e7159d1676875d Mon Sep 17 00:00:00 2001 From: smruthi2187 <34555664+smruthi2187@users.noreply.github.com> Date: Mon, 4 Jun 2018 19:30:27 -0700 Subject: [PATCH] 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 | 1 - .../templates/pre-upgrade-job.yaml | 33 +++ charts/fission-all/values.yaml | 3 + .../templates/pre-upgrade-job.yaml | 33 +++ charts/fission-core/values.yaml | 3 + hack/release.sh | 29 +++ preupgradechecks/Dockerfile | 3 + preupgradechecks/build.sh | 18 ++ preupgradechecks/main.go | 66 ++++++ preupgradechecks/preupgradechecks.go | 207 ++++++++++++++++++ test/build_and_test.sh | 5 +- test/test_utils.sh | 20 +- 12 files changed, 417 insertions(+), 4 deletions(-) create mode 100644 charts/fission-all/templates/pre-upgrade-job.yaml create mode 100644 charts/fission-core/templates/pre-upgrade-job.yaml create mode 100644 preupgradechecks/Dockerfile create mode 100755 preupgradechecks/build.sh create mode 100644 preupgradechecks/main.go create mode 100644 preupgradechecks/preupgradechecks.go diff --git a/charts/fission-all/templates/deployment.yaml b/charts/fission-all/templates/deployment.yaml index 4f44b4e9..5f6d7c7f 100644 --- a/charts/fission-all/templates/deployment.yaml +++ b/charts/fission-all/templates/deployment.yaml @@ -95,7 +95,6 @@ kind: ServiceAccount metadata: name: fission-fetcher namespace: {{ .Values.functionNamespace }} - --- apiVersion: v1 kind: ServiceAccount diff --git a/charts/fission-all/templates/pre-upgrade-job.yaml b/charts/fission-all/templates/pre-upgrade-job.yaml new file mode 100644 index 00000000..199c954e --- /dev/null +++ b/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 \ No newline at end of file diff --git a/charts/fission-all/values.yaml b/charts/fission-all/values.yaml index 8541f58f..dcb08252 100644 --- a/charts/fission-all/values.yaml +++ b/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 \ No newline at end of file diff --git a/charts/fission-core/templates/pre-upgrade-job.yaml b/charts/fission-core/templates/pre-upgrade-job.yaml new file mode 100644 index 00000000..199c954e --- /dev/null +++ b/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 \ No newline at end of file diff --git a/charts/fission-core/values.yaml b/charts/fission-core/values.yaml index a3cae997..1145f737 100644 --- a/charts/fission-core/values.yaml +++ b/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 \ No newline at end of file diff --git a/hack/release.sh b/hack/release.sh index e2557170..7693e5dd 100755 --- a/hack/release.sh +++ b/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() { diff --git a/preupgradechecks/Dockerfile b/preupgradechecks/Dockerfile new file mode 100644 index 00000000..52187eb0 --- /dev/null +++ b/preupgradechecks/Dockerfile @@ -0,0 +1,3 @@ +FROM alpine:3.4 +RUN apk add --update ca-certificates +ADD pre-upgrade-checks / diff --git a/preupgradechecks/build.sh b/preupgradechecks/build.sh new file mode 100755 index 00000000..5918577e --- /dev/null +++ b/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 diff --git a/preupgradechecks/main.go b/preupgradechecks/main.go new file mode 100644 index 00000000..dd8610d7 --- /dev/null +++ b/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= --envbuilder-namespace= +Options: + --fn-pod-namespace= Namespace where function pods get deployed. + --envbuilder-namespace= 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() +} diff --git a/preupgradechecks/preupgradechecks.go b/preupgradechecks/preupgradechecks.go new file mode 100644 index 00000000..5a044298 --- /dev/null +++ b/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 +} diff --git a/test/build_and_test.sh b/test/build_and_test.sh index f49e962e..ef8ab338 100755 --- a/test/build_and_test.sh +++ b/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 diff --git a/test/test_utils.sh b/test/test_utils.sh index 8cbd8544..4d6c8be9 100755 --- a/test/test_utils.sh +++ b/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