From 6d3d277cb998452955bafca149b59435454c36ad Mon Sep 17 00:00:00 2001
From: Ta-Ching Chen <contact@tachingchen.com>
Date: Mon, 16 Sep 2019 11:03:23 +0800
Subject: [PATCH] Fix potential nil pointer problem when using multierror pkg
 (#1311)

---
 cmd/preupgradechecks/preupgradechecks.go   |  2 +-
 pkg/apis/fission.io/v1/typeDeepcopyFunc.go | 30 ++++++-------
 pkg/apis/fission.io/v1/validation.go       | 50 +++++++++++-----------
 pkg/executor/newdeploy/newdeploy.go        | 19 ++++----
 pkg/executor/newdeploy/newdeploymgr.go     |  2 +-
 pkg/executor/poolmgr/gp.go                 |  2 +-
 pkg/executor/util/merge.go                 |  8 ++--
 pkg/fission-cli/cmd/spec/spec.go           |  2 +-
 pkg/fission-cli/package.go                 |  2 +-
 pkg/fission-cli/spec.go                    |  2 +-
 10 files changed, 59 insertions(+), 60 deletions(-)

diff --git a/cmd/preupgradechecks/preupgradechecks.go b/cmd/preupgradechecks/preupgradechecks.go
index c14c3597..fc28742f 100644
--- a/cmd/preupgradechecks/preupgradechecks.go
+++ b/cmd/preupgradechecks/preupgradechecks.go
@@ -86,7 +86,7 @@ func (client *PreUpgradeTaskClient) IsFissionReInstall() bool {
 func (client *PreUpgradeTaskClient) VerifyFunctionSpecReferences() {
 	client.logger.Info("verifying function spec references for all functions in the cluster")
 
-	var result *multierror.Error
+	result := &multierror.Error{}
 	var err error
 	var fList *fv1.FunctionList
 
diff --git a/pkg/apis/fission.io/v1/typeDeepcopyFunc.go b/pkg/apis/fission.io/v1/typeDeepcopyFunc.go
index 6b33a245..df81a99b 100644
--- a/pkg/apis/fission.io/v1/typeDeepcopyFunc.go
+++ b/pkg/apis/fission.io/v1/typeDeepcopyFunc.go
@@ -153,7 +153,7 @@ func validateMetadata(field string, m metav1.ObjectMeta) error {
 }
 
 func (p *Package) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("Package", p.Metadata),
@@ -164,7 +164,7 @@ func (p *Package) Validate() error {
 }
 
 func (pl *PackageList) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	// not validate ListMeta
 	for _, p := range pl.Items {
 		result = multierror.Append(result, p.Validate())
@@ -173,7 +173,7 @@ func (pl *PackageList) Validate() error {
 }
 
 func (f *Function) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("Function", f.Metadata),
@@ -183,7 +183,7 @@ func (f *Function) Validate() error {
 }
 
 func (fl *FunctionList) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	for _, f := range fl.Items {
 		result = multierror.Append(result, f.Validate())
 	}
@@ -191,7 +191,7 @@ func (fl *FunctionList) Validate() error {
 }
 
 func (e *Environment) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("Environment", e.Metadata),
@@ -201,7 +201,7 @@ func (e *Environment) Validate() error {
 }
 
 func (el *EnvironmentList) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	for _, e := range el.Items {
 		result = multierror.Append(result, e.Validate())
 	}
@@ -209,7 +209,7 @@ func (el *EnvironmentList) Validate() error {
 }
 
 func (h *HTTPTrigger) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("HTTPTrigger", h.Metadata),
@@ -219,7 +219,7 @@ func (h *HTTPTrigger) Validate() error {
 }
 
 func (hl *HTTPTriggerList) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	for _, h := range hl.Items {
 		result = multierror.Append(result, h.Validate())
 	}
@@ -227,7 +227,7 @@ func (hl *HTTPTriggerList) Validate() error {
 }
 
 func (k *KubernetesWatchTrigger) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("KubernetesWatchTrigger", k.Metadata),
@@ -237,7 +237,7 @@ func (k *KubernetesWatchTrigger) Validate() error {
 }
 
 func (kl *KubernetesWatchTriggerList) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	for _, k := range kl.Items {
 		result = multierror.Append(result, k.Validate())
 	}
@@ -245,7 +245,7 @@ func (kl *KubernetesWatchTriggerList) Validate() error {
 }
 
 func (t *TimeTrigger) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("TimeTrigger", t.Metadata),
@@ -255,7 +255,7 @@ func (t *TimeTrigger) Validate() error {
 }
 
 func (tl *TimeTriggerList) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	for _, t := range tl.Items {
 		result = multierror.Append(result, t.Validate())
 	}
@@ -263,7 +263,7 @@ func (tl *TimeTriggerList) Validate() error {
 }
 
 func (m *MessageQueueTrigger) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("MessageQueueTrigger", m.Metadata),
@@ -273,7 +273,7 @@ func (m *MessageQueueTrigger) Validate() error {
 }
 
 func (ml *MessageQueueTriggerList) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	for _, m := range ml.Items {
 		result = multierror.Append(result, m.Validate())
 	}
@@ -281,7 +281,7 @@ func (ml *MessageQueueTriggerList) Validate() error {
 }
 
 func (r *Recorder) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		validateMetadata("Recorder", r.Metadata),
diff --git a/pkg/apis/fission.io/v1/validation.go b/pkg/apis/fission.io/v1/validation.go
index 61926de4..8db61ff1 100644
--- a/pkg/apis/fission.io/v1/validation.go
+++ b/pkg/apis/fission.io/v1/validation.go
@@ -89,7 +89,7 @@ func (e ValidationError) Error() string {
 }
 
 func AggregateValidationErrors(objName string, err error) error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result, err)
 
@@ -114,7 +114,7 @@ func MakeValidationErr(errType ValidationErrorType, field string, val interface{
 }
 
 func ValidateKubeLabel(field string, labels map[string]string) error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	for k, v := range labels {
 		// Example: XXX -> YYY
@@ -129,7 +129,7 @@ func ValidateKubeLabel(field string, labels map[string]string) error {
 }
 
 func ValidateKubePort(field string, port int) error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	e := validation.IsValidPortNum(port)
 	if len(e) > 0 {
@@ -140,7 +140,7 @@ func ValidateKubePort(field string, port int) error {
 }
 
 func ValidateKubeName(field string, val string) error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	e := validation.IsDNS1123Label(val)
 	if len(e) > 0 {
@@ -151,7 +151,7 @@ func ValidateKubeName(field string, val string) error {
 }
 
 func ValidateKubeReference(refName string, name string, namespace string) error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result,
 		ValidateKubeName(fmt.Sprintf("%v.Name", refName), name),
@@ -197,7 +197,7 @@ func IsValidCronSpec(spec string) error {
 /* Resource validation function */
 
 func (checksum Checksum) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	switch checksum.Type {
 	case ChecksumTypeSHA256: // no op
@@ -209,7 +209,7 @@ func (checksum Checksum) Validate() error {
 }
 
 func (archive Archive) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	if len(archive.Type) > 0 {
 		switch archive.Type {
@@ -227,25 +227,25 @@ func (archive Archive) Validate() error {
 }
 
 func (ref EnvironmentReference) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	result = multierror.Append(result, ValidateKubeReference("EnvironmentReference", ref.Name, ref.Namespace))
 	return result.ErrorOrNil()
 }
 
 func (ref SecretReference) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	result = multierror.Append(result, ValidateKubeReference("SecretReference", ref.Name, ref.Namespace))
 	return result.ErrorOrNil()
 }
 
 func (ref ConfigMapReference) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	result = multierror.Append(result, ValidateKubeReference("ConfigMapReference", ref.Name, ref.Namespace))
 	return result.ErrorOrNil()
 }
 
 func (spec PackageSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result, spec.Environment.Validate())
 
@@ -259,7 +259,7 @@ func (spec PackageSpec) Validate() error {
 }
 
 func (sts PackageStatus) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	switch sts.BuildStatus {
 	case BuildStatusPending, BuildStatusRunning, BuildStatusSucceeded, BuildStatusFailed, BuildStatusNone: // no op
@@ -271,19 +271,19 @@ func (sts PackageStatus) Validate() error {
 }
 
 func (ref PackageRef) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	result = multierror.Append(result, ValidateKubeReference("PackageRef", ref.Name, ref.Namespace))
 	return result.ErrorOrNil()
 }
 
 func (ref FunctionPackageRef) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 	result = multierror.Append(result, ref.PackageRef.Validate())
 	return result.ErrorOrNil()
 }
 
 func (spec FunctionSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	if spec.Environment != (EnvironmentReference{}) {
 		result = multierror.Append(result, spec.Environment.Validate())
@@ -313,7 +313,7 @@ func (spec FunctionSpec) Validate() error {
 }
 
 func (is InvokeStrategy) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	switch is.StrategyType {
 	case StrategyTypeExecution: // no op
@@ -327,7 +327,7 @@ func (is InvokeStrategy) Validate() error {
 }
 
 func (es ExecutionStrategy) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	switch es.ExecutorType {
 	case ExecutorTypeNewdeploy, ExecutorTypePoolmgr: // no op
@@ -362,7 +362,7 @@ func (es ExecutionStrategy) Validate() error {
 }
 
 func (ref FunctionReference) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	switch ref.Type {
 	case FunctionReferenceTypeFunctionName: // no op
@@ -379,7 +379,7 @@ func (ref FunctionReference) Validate() error {
 }
 
 func (runtime Runtime) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	if runtime.LoadEndpointPort > 0 {
 		result = multierror.Append(result, ValidateKubePort("Runtime.LoadEndpointPort", int(runtime.LoadEndpointPort)))
@@ -398,7 +398,7 @@ func (builder Builder) Validate() error {
 }
 
 func (spec EnvironmentSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	if spec.Version < 1 || spec.Version > 3 {
 		result = multierror.Append(result, MakeValidationErr(ErrorInvalidValue, "EnvironmentSpec.Version", spec.Version, "not a valid environment version"))
@@ -426,7 +426,7 @@ func (spec EnvironmentSpec) Validate() error {
 }
 
 func (spec HTTPTriggerSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	switch spec.Method {
 	case http.MethodGet, http.MethodHead, http.MethodPost, http.MethodPut, http.MethodPatch,
@@ -448,7 +448,7 @@ func (spec HTTPTriggerSpec) Validate() error {
 }
 
 func (spec KubernetesWatchTriggerSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	switch strings.ToUpper(spec.Type) {
 	case "POD", "SERVICE", "REPLICATIONCONTROLLER", "JOB":
@@ -465,7 +465,7 @@ func (spec KubernetesWatchTriggerSpec) Validate() error {
 }
 
 func (spec MessageQueueTriggerSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	result = multierror.Append(result, spec.FunctionReference.Validate())
 
@@ -487,7 +487,7 @@ func (spec MessageQueueTriggerSpec) Validate() error {
 }
 
 func (spec RecorderSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	// TODO: Function validation
 	//if len(spec.Function.Name) != 0 {
@@ -514,7 +514,7 @@ func (spec RecorderSpec) Validate() error {
 }
 
 func (spec TimeTriggerSpec) Validate() error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	err := IsValidCronSpec(spec.Cron)
 	if err != nil {
diff --git a/pkg/executor/newdeploy/newdeploy.go b/pkg/executor/newdeploy/newdeploy.go
index 25adffe7..1e92b087 100644
--- a/pkg/executor/newdeploy/newdeploy.go
+++ b/pkg/executor/newdeploy/newdeploy.go
@@ -21,8 +21,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/fission/fission/pkg/types"
-	"github.com/fission/fission/pkg/utils"
 	multierror "github.com/hashicorp/go-multierror"
 	"go.uber.org/zap"
 	asv1 "k8s.io/api/autoscaling/v1"
@@ -35,6 +33,8 @@ import (
 
 	fv1 "github.com/fission/fission/pkg/apis/fission.io/v1"
 	"github.com/fission/fission/pkg/executor/util"
+	"github.com/fission/fission/pkg/types"
+	"github.com/fission/fission/pkg/utils"
 )
 
 const (
@@ -70,9 +70,7 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *fv1.Function, env *fv1.Enviro
 			}
 		}
 		return existingDepl, err
-	}
-
-	if err != nil && k8s_err.IsNotFound(err) {
+	} else if k8s_err.IsNotFound(err) {
 		err := deploy.setupRBACObjs(deployNamespace, fn)
 		if err != nil {
 			return nil, err
@@ -442,7 +440,7 @@ func (deploy *NewDeploy) waitForDeploy(depl *v1beta1.Deployment, replicas int32,
 
 // cleanupNewdeploy cleans all kubernetes objects related to function
 func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
-	var multierr *multierror.Error
+	result := &multierror.Error{}
 
 	err := deploy.deleteSvc(ns, name)
 	if err != nil {
@@ -450,7 +448,7 @@ func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
 			zap.Error(err),
 			zap.String("function_name", name),
 			zap.String("function_namespace", ns))
-		multierror.Append(multierr, err)
+		result = multierror.Append(result, err)
 	}
 
 	err = deploy.deleteHpa(ns, name)
@@ -459,7 +457,7 @@ func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
 			zap.Error(err),
 			zap.String("function_name", name),
 			zap.String("function_namespace", ns))
-		multierror.Append(multierr, err)
+		result = multierror.Append(result, err)
 	}
 
 	err = deploy.deleteDeployment(ns, name)
@@ -468,7 +466,8 @@ func (deploy *NewDeploy) cleanupNewdeploy(ns string, name string) error {
 			zap.Error(err),
 			zap.String("function_name", name),
 			zap.String("function_namespace", ns))
-		multierror.Append(multierr, err)
+		result = multierror.Append(result, err)
 	}
-	return multierr.ErrorOrNil()
+
+	return result.ErrorOrNil()
 }
diff --git a/pkg/executor/newdeploy/newdeploymgr.go b/pkg/executor/newdeploy/newdeploymgr.go
index e8579b51..91263a41 100644
--- a/pkg/executor/newdeploy/newdeploymgr.go
+++ b/pkg/executor/newdeploy/newdeploymgr.go
@@ -558,7 +558,7 @@ func (deploy *NewDeploy) updateFuncDeployment(fn *fv1.Function, env *fv1.Environ
 }
 
 func (deploy *NewDeploy) fnDelete(fn *fv1.Function) error {
-	var multierr *multierror.Error
+	multierr := &multierror.Error{}
 
 	// GetByFunction uses resource version as part of cache key, however,
 	// the resource version in function metadata will be changed when a function
diff --git a/pkg/executor/poolmgr/gp.go b/pkg/executor/poolmgr/gp.go
index f6f1c848..6fe7d741 100644
--- a/pkg/executor/poolmgr/gp.go
+++ b/pkg/executor/poolmgr/gp.go
@@ -471,7 +471,7 @@ func (gp *GenericPool) waitForReadyPod() error {
 
 			// Since even single pod is not ready, choosing the first pod to inspect is a good approximation. In future this can be done better
 			pod := podList.Items[0]
-			var multierr *multierror.Error
+			multierr := &multierror.Error{}
 			for _, cStatus := range pod.Status.ContainerStatuses {
 				if cStatus.Ready != true {
 					multierr = multierror.Append(multierr, errors.New(fmt.Sprintf("%v: %v", cStatus.State.Waiting.Reason, cStatus.State.Waiting.Message)))
diff --git a/pkg/executor/util/merge.go b/pkg/executor/util/merge.go
index 7b0aab28..ecf58520 100644
--- a/pkg/executor/util/merge.go
+++ b/pkg/executor/util/merge.go
@@ -73,7 +73,7 @@ func MergePodSpec(srcPodSpec *apiv1.PodSpec, targetPodSpec *apiv1.PodSpec) error
 		return nil
 	}
 
-	var multierr *multierror.Error
+	multierr := &multierror.Error{}
 
 	// Get item from spec, if they exist in deployment - merge, else append
 	// Same pattern for all lists (Mergo can not handle lists)
@@ -155,7 +155,7 @@ func mergeContainerLists(srcPodSpec *apiv1.PodSpec, targetPodSpec *apiv1.PodSpec
 		targetContainers[c.Name] = c
 	}
 
-	var multierr *multierror.Error
+	multierr := &multierror.Error{}
 	for _, c := range srcPodSpec.Containers {
 		container, ok := targetContainers[c.Name]
 		if ok {
@@ -179,7 +179,7 @@ func mergeInitContainerList(srcPodSpec *apiv1.PodSpec, targetPodSpec *apiv1.PodS
 		targetContainers[c.Name] = c
 	}
 
-	var multierr *multierror.Error
+	multierr := &multierror.Error{}
 	for _, c := range srcPodSpec.InitContainers {
 		container, ok := targetContainers[c.Name]
 		if ok {
@@ -202,7 +202,7 @@ func mergeVolumeLists(srcPodSpec *apiv1.PodSpec, targetPodSpec *apiv1.PodSpec) e
 		specVolumes[vol.Name] = vol
 	}
 
-	var multierr *multierror.Error
+	multierr := &multierror.Error{}
 	for _, vol := range srcPodSpec.Volumes {
 		_, ok := specVolumes[vol.Name]
 		if ok {
diff --git a/pkg/fission-cli/cmd/spec/spec.go b/pkg/fission-cli/cmd/spec/spec.go
index 98dceea4..83729b54 100644
--- a/pkg/fission-cli/cmd/spec/spec.go
+++ b/pkg/fission-cli/cmd/spec/spec.go
@@ -297,7 +297,7 @@ func (fr *FissionResources) validateFunctionReference(functions map[string]bool,
 }
 
 func (fr *FissionResources) Validate(c *cli.Context) error {
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	// check references: both dangling refs + garbage
 	//   packages -> archives
diff --git a/pkg/fission-cli/package.go b/pkg/fission-cli/package.go
index 3ce23f2a..4677f413 100644
--- a/pkg/fission-cli/package.go
+++ b/pkg/fission-cli/package.go
@@ -488,7 +488,7 @@ func fileChecksum(fileName string) (*fv1.Checksum, error) {
 // includeFiles, but is ignored if there's more than one includeFile.
 func createArchive(client *client.Client, includeFiles []string, noZip bool, specDir string, specFile string) *fv1.Archive {
 
-	var errs *multierror.Error
+	errs := &multierror.Error{}
 
 	// check files existence
 	for _, path := range includeFiles {
diff --git a/pkg/fission-cli/spec.go b/pkg/fission-cli/spec.go
index 4d835b14..8c473253 100644
--- a/pkg/fission-cli/spec.go
+++ b/pkg/fission-cli/spec.go
@@ -160,7 +160,7 @@ func readSpecs(specDir string) (*spec.FissionResources, error) {
 		},
 	}
 
-	var result *multierror.Error
+	result := &multierror.Error{}
 
 	// Users can organize the specdir into subdirs if they want to.
 	err := filepath.Walk(specDir, func(path string, info os.FileInfo, err error) error {