You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by ho...@apache.org on 2017/08/14 14:14:11 UTC

[incubator-openwhisk-wskdeploy] branch master updated: Fix Parameter Resolution (both schema) & documentation for the triggerrule use case (#298)

This is an automated email from the ASF dual-hosted git repository.

houshengbo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk-wskdeploy.git


The following commit(s) were added to refs/heads/master by this push:
     new f716b13  Fix Parameter Resolution (both schema) & documentation for the triggerrule use case (#298)
f716b13 is described below

commit f716b13025d953cced51198bd2c33aff55c5b0f7
Author: Matt Rutkowski <mr...@us.ibm.com>
AuthorDate: Mon Aug 14 09:14:10 2017 -0500

    Fix Parameter Resolution (both schema) & documentation for the triggerrule use case (#298)
    
    * More test and test documentation updates; adjust some Deployment files.
    
    * fix broken triggerrule usecase and incorrect documentation.
    
    * Steps towards fixing Parameter resolution logic.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
    
    * Add logic that recognizes single vs multiline param schema.
---
 README.md                                          |   2 +-
 deployers/manifestreader.go                        |   3 +-
 deployers/servicedeployer.go                       |   2 +-
 parsers/manifest_parser.go                         | 194 +++++++++++++++++++--
 parsers/yamlparser.go                              |   1 +
 tests/README.md                                    |   2 +-
 tests/dat/actions/dump_params.js                   |   3 +
 tests/dat/manifest_validate_multiline_params.yaml  |  39 +++++
 tests/dat/manifest_validate_singleline_params.yaml |  30 ++++
 tests/src/integration/triggerrule/manifest.yml     |   2 +-
 tests/usecases/triggerrule/README.md               |  39 ++++-
 tests/usecases/triggerrule/deployment.yml          |   4 +-
 tests/usecases/webhook/deployment.yml              |   2 +-
 utils/errorhandlers.go                             |   9 -
 utils/misc.go                                      |  46 ++++-
 utils/util_test.go                                 |   6 +-
 16 files changed, 339 insertions(+), 45 deletions(-)

diff --git a/README.md b/README.md
index 5274e74..db6c499 100644
--- a/README.md
+++ b/README.md
@@ -116,7 +116,7 @@ commands will start the wskdeploy cross compile for your specific OS platform in
 
 Wskdeploy uses the OpenWhisk Go Client to format and invoke OpenWhisk's APIs which has additional debug tracing available.
 
-To enable this trace, set the following environemt variable in Bash:
+To enable this trace, set the following environment variable in Bash:
 ```
 # set to any value > 0
 WSK_CLI_DEBUG=1
diff --git a/deployers/manifestreader.go b/deployers/manifestreader.go
index c31faf4..d137c20 100644
--- a/deployers/manifestreader.go
+++ b/deployers/manifestreader.go
@@ -190,8 +190,7 @@ func (reader *ManifestReader) SetActions(actions []utils.ActionRecord) error {
 				return errors.New("manifestReader. Error: Conflict detected for action named " + existAction.Action.Name + ". Found two locations for source file: " + existAction.Filepath + " and " + manifestAction.Filepath)
 			}
 		} else {
-			// not a new action so to actions in package
-
+			// not a new action so update the action in the package
 			err := reader.checkAction(manifestAction)
 			utils.Check(err)
 			reader.serviceDeployer.Deployment.Packages[manifestAction.Packagename].Actions[manifestAction.Action.Name] = manifestAction
diff --git a/deployers/servicedeployer.go b/deployers/servicedeployer.go
index a235774..b4b1ae4 100644
--- a/deployers/servicedeployer.go
+++ b/deployers/servicedeployer.go
@@ -180,7 +180,7 @@ func (deployer *ServiceDeployer) ConstructUnDeploymentPlan() (*DeploymentApplica
 }
 
 // Use reflect util to deploy everything in this service deployer
-// TODO(TBD): according some planning?
+// TODO(TBD): according to some planning?
 func (deployer *ServiceDeployer) Deploy() error {
 
 	if deployer.IsInteractive == true && !utils.Flags.WithinOpenWhisk {
diff --git a/parsers/manifest_parser.go b/parsers/manifest_parser.go
index ce78662..c3e7003 100644
--- a/parsers/manifest_parser.go
+++ b/parsers/manifest_parser.go
@@ -23,6 +23,7 @@ import (
 	"log"
 	"os"
 	"path"
+	"reflect"
 	"strings"
 
 	"encoding/base64"
@@ -31,6 +32,7 @@ import (
 	"github.com/apache/incubator-openwhisk-client-go/whisk"
 	"github.com/apache/incubator-openwhisk-wskdeploy/utils"
 	"gopkg.in/yaml.v2"
+	"fmt"
 )
 
 // Read existing manifest file or create new if none exists
@@ -90,6 +92,7 @@ func (dm *YAMLParser) ParseManifest(mani string) *ManifestYAML {
 
 func (dm *YAMLParser) ComposeDependencies(mani *ManifestYAML, projectPath string) (map[string]utils.DependencyRecord, error) {
 
+	var errorParser error
 	depMap := make(map[string]utils.DependencyRecord)
 	for key, dependency := range mani.Package.Dependencies {
 		version := dependency.Version
@@ -123,7 +126,11 @@ func (dm *YAMLParser) ComposeDependencies(mani *ManifestYAML, projectPath string
 			var keyVal whisk.KeyValue
 			keyVal.Key = name
 
-			keyVal.Value = ResolveParameter(&param)
+			keyVal.Value, errorParser = ResolveParameter(name, &param)
+
+			if errorParser != nil {
+				return nil, errorParser
+			}
 
 			if keyVal.Value != nil {
 				keyValArrParams = append(keyValArrParams, keyVal)
@@ -148,6 +155,8 @@ func (dm *YAMLParser) ComposeDependencies(mani *ManifestYAML, projectPath string
 
 // Is we consider multi pacakge in one yaml?
 func (dm *YAMLParser) ComposePackage(mani *ManifestYAML) (*whisk.Package, error) {
+	var errorParser error
+
 	//mani := dm.ParseManifest(manipath)
 	pag := &whisk.Package{}
 	pag.Name = mani.Package.Packagename
@@ -161,7 +170,11 @@ func (dm *YAMLParser) ComposePackage(mani *ManifestYAML) (*whisk.Package, error)
 		var keyVal whisk.KeyValue
 		keyVal.Key = name
 
-		keyVal.Value = ResolveParameter(&param)
+		keyVal.Value, errorParser = ResolveParameter(name, &param)
+
+		if errorParser != nil {
+			return nil, errorParser
+		}
 
 		if keyVal.Value != nil {
 			keyValArr = append(keyValArr, keyVal)
@@ -220,6 +233,7 @@ func (dm *YAMLParser) ComposeSequences(namespace string, mani *ManifestYAML) ([]
 
 func (dm *YAMLParser) ComposeActions(mani *ManifestYAML, manipath string) (ar []utils.ActionRecord, aub []*utils.ActionExposedURLBinding, err error) {
 
+	var errorParser error
 	var s1 []utils.ActionRecord = make([]utils.ActionRecord, 0)
 	var au []*utils.ActionExposedURLBinding = make([]*utils.ActionExposedURLBinding, 0)
 
@@ -238,7 +252,7 @@ func (dm *YAMLParser) ComposeActions(mani *ManifestYAML, manipath string) (ar []
 
 			if utils.IsDirectory(filePath) {
 				zipName := filePath + ".zip"
-				err := utils.NewZipWritter(filePath, zipName).Zip()
+				err = utils.NewZipWritter(filePath, zipName).Zip()
 				defer os.Remove(zipName)
 				utils.Check(err)
 				// To do: support docker and main entry as did by go cli?
@@ -284,7 +298,11 @@ func (dm *YAMLParser) ComposeActions(mani *ManifestYAML, manipath string) (ar []
 			var keyVal whisk.KeyValue
 			keyVal.Key = name
 
-			keyVal.Value = ResolveParameter(&param)
+			keyVal.Value, errorParser = ResolveParameter(name, &param)
+
+			if errorParser != nil {
+				return nil, nil, errorParser
+			}
 
 			if keyVal.Value != nil {
 				keyValArr = append(keyValArr, keyVal)
@@ -330,7 +348,7 @@ func (dm *YAMLParser) ComposeActions(mani *ManifestYAML, manipath string) (ar []
 }
 
 func (dm *YAMLParser) ComposeTriggers(manifest *ManifestYAML) ([]*whisk.Trigger, error) {
-
+	var errorParser error
 	var t1 []*whisk.Trigger = make([]*whisk.Trigger, 0)
 	pkg := manifest.Package
 	for _, trigger := range pkg.GetTriggerList() {
@@ -357,7 +375,11 @@ func (dm *YAMLParser) ComposeTriggers(manifest *ManifestYAML) ([]*whisk.Trigger,
 			var keyVal whisk.KeyValue
 			keyVal.Key = name
 
-			keyVal.Value = ResolveParameter(&param)
+			keyVal.Value, errorParser = ResolveParameter(name, &param)
+
+			if errorParser != nil {
+				return nil, errorParser
+			}
 
 			if keyVal.Value != nil {
 				keyValArr = append(keyValArr, keyVal)
@@ -403,19 +425,151 @@ func (action *Action) ComposeWskAction(manipath string) (*whisk.Action, error) {
 	return wskaction, err
 }
 
-// Resolve parameter input
-func ResolveParameter(param *Parameter) interface{} {
-	value := utils.GetEnvVar(param.Value)
+
+// TODO(): Support other valid Package Manifest types
+// TODO(): i.e., json (valid), timestamp, version, string256, string64, string16
+// TODO(): Support JSON schema validation for type: json
+// TODO(): Support OpenAPI schema validation
+
+var validParameterNameMap = map[string]string{
+	"string": "string",
+	"int": "integer",
+	"float": "float",
+	"bool": "boolean",
+	"int8": "integer",
+	"int16": "integer",
+	"int32": "integer",
+	"int64": "integer",
+	"float32": "float",
+	"float64": "float",
+}
+
+
+var typeDefaultValueMap = map[string]interface{} {
+	"string": "",
+	"integer": 0,
+	"float": 0.0,
+	"boolean": false,
+	// TODO() Support these types + their validation
+	// timestamp
+	// null
+	// version
+	// string256
+	// string64
+	// string16
+	// json
+	// scalar-unit
+	// schema
+	// object
+}
+
+func isValidParameterType(typeName string) bool {
+	_, isValid := typeDefaultValueMap[typeName]
+	return isValid
+}
+
+// TODO(): throw errors
+func getTypeDefaultValue(typeName string) interface{} {
+
+	if val, ok := typeDefaultValueMap[typeName]; ok {
+		return val
+	} else {
+		// TODO() throw an error "type not found"
+	}
+        return nil
+}
+
+func ResolveParamTypeFromValue(value interface{}) (string, error) {
+        // Note: string is the default type if not specified.
+	var paramType string = "string"
+	var err error = nil
+
+	if value != nil {
+		actualType := reflect.TypeOf(value).Kind().String()
+
+		// See if the actual type of the value is valid
+		if normalizedTypeName, found := validParameterNameMap[actualType]; found {
+			// use the full spec. name
+			paramType = normalizedTypeName
+
+		} else {
+			// raise an error if param is not a known type
+			err = utils.NewParserErr("",-1, "Parameter value is not a known type. [" + actualType + "]")
+		}
+	} else {
+
+		// TODO: The value may be supplied later, we need to support non-fatal warnings
+		// raise an error if param is nil
+		//err = utils.NewParserErr("",-1,"Paramter value is nil.")
+	}
+	return paramType, err
+}
+
+// Resolve input parameter (i.e., type, value, default)
+// Note: parameter values may set later (overriddNen) by an (optional) Deployment file
+func ResolveParameter(paramName string, param *Parameter) (interface{}, error) {
+
+	var errorParser error
+	var tempType string
+	// default parameter value to empty string
+	var value interface{} = ""
+
+	// Trace Parameter struct before any resolution
+	//dumpParameter(paramName, param, "BEFORE")
+
+	// Parameters can be single OR multi-line declarations which must be processed/validated differently
+	if !param.multiline {
+		// we have a single-line parameter declaration
+		// We need to identify parameter Type here for later validation
+		param.Type, errorParser = ResolveParamTypeFromValue(param.Value)
+
+	} else {
+		// we have a multi-line parameter declaration
+
+		// if we do not have a value, but have a default, use it for the value
+                if param.Value == nil && param.Default !=nil {
+			param.Value = param.Default
+		}
+
+		// if we also have a type at this point, verify value (and/or default) matches type, if not error
+		// Note: if either the value or default is in conflict with the type then this is an error
+		tempType, errorParser = ResolveParamTypeFromValue(param.Value)
+
+		// if we do not have a value or default, but have a type, find its default and use it for the value
+		if param.Type!="" && !isValidParameterType(param.Type) {
+		    return value, utils.NewParserErr("",-1, "Invalid Type for parameter. [" + param.Type + "]")
+		} else if param.Type == "" {
+			param.Type = tempType
+		}
+	}
+
+	// Make sure the parameter's value is a valid, non-empty string and startsWith '$" (dollar) sign
+	value = utils.GetEnvVar(param.Value)
 
 	typ := param.Type
+
+	// TODO(Priti): need to validate type is one of the supported primitive types with unit testing
+	// TODO(): with the new logic, when would the fpllowing Unmarhsall() call be used?
+	// if value is of type 'string' and its not empty <OR> if type is not 'string'
 	if str, ok := value.(string); ok && (len(typ) == 0 || typ != "string") {
 		var parsed interface{}
 		err := json.Unmarshal([]byte(str), &parsed)
 		if err == nil {
-			return parsed
+			return parsed, err
 		}
 	}
-	return value
+
+	// Default to an empty string, do NOT error/terminate as Value may be provided later bu a Deployment file.
+	if (value == nil) {
+		value = getTypeDefaultValue(param.Type)
+		// @TODO(): Need warning message here to warn of default usage, support for warnings (non-fatal)
+	}
+
+	// Trace Parameter struct after resolution
+	//dumpParameter(paramName, param, "AFTER")
+	//fmt.Printf("EXIT: value=[%v]\n", value)
+
+	return value, errorParser
 }
 
 // Provide custom Parameter marshalling and unmarshalling
@@ -424,7 +578,10 @@ type ParsedParameter Parameter
 
 func (n *Parameter) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	var aux ParsedParameter
+
+	// Attempt to unmarshall the multi-line schema
 	if err := unmarshal(&aux); err == nil {
+		n.multiline = true
 		n.Type = aux.Type
 		n.Description = aux.Description
 		n.Value = aux.Value
@@ -435,12 +592,14 @@ func (n *Parameter) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		return nil
 	}
 
+	// If we did not find the multi-line schema, assume in-line (or single-line) schema
 	var inline interface{}
 	if err := unmarshal(&inline); err != nil {
 		return err
 	}
 
 	n.Value = inline
+	n.multiline = false
 	return nil
 }
 
@@ -453,3 +612,16 @@ func (n *Parameter) MarshalYAML() (interface{}, error) {
 
 	return n, nil
 }
+
+// Provides debug/trace support for Parameter type
+func dumpParameter(paramName string, param *Parameter, separator string) {
+
+	fmt.Printf("%s:\n", separator)
+	fmt.Printf("\t%s: (%T)\n", paramName, param)
+	if(param!= nil) {
+		fmt.Printf("\t\tParameter.Descrption: [%s]\n", param.Description)
+		fmt.Printf("\t\tParameter.Type: [%s]\n", param.Type)
+		fmt.Printf("\t\tParameter.Value: [%v]\n", param.Value)
+		fmt.Printf("\t\tParameter.Default: [%v]\n", param.Default)
+	}
+}
diff --git a/parsers/yamlparser.go b/parsers/yamlparser.go
index 384c054..b768525 100644
--- a/parsers/yamlparser.go
+++ b/parsers/yamlparser.go
@@ -87,6 +87,7 @@ type Parameter struct {
 	Default     interface{} `yaml:"default,omitempty"`
 	Status      string      `yaml:"status,omitempty"`
 	Schema      interface{} `yaml:"schema,omitempty"`
+	multiline   bool
 }
 
 type Trigger struct {
diff --git a/tests/README.md b/tests/README.md
index 88fa527..8b7be57 100644
--- a/tests/README.md
+++ b/tests/README.md
@@ -46,7 +46,7 @@ $ go test -v ./... -tags integration
 
 All integration test cases are put under the folder 'tests/src/integration'.
 
-Unit tests are co-located in the same directory as the package they are testing (by Go convention). The test files would use the same basename as the file that containst the package they are providing tests for but with and add '_test' postfix.
+Unit tests are co-located in the same directory as the package they are testing (by Go convention). The test files would use the same basename as the file that contains the package they are providing tests for, but with the added '_test' postfix to the base file name.
 
 For example, the package 'deployers', which defines a 'DeploymentReader' service, is declared in the file 'deploymentreader.go'; its corresponding unit test is in the file named 'deploymentreader_test.go' in the same directory.
 
diff --git a/tests/dat/actions/dump_params.js b/tests/dat/actions/dump_params.js
new file mode 100644
index 0000000..75e5c7a
--- /dev/null
+++ b/tests/dat/actions/dump_params.js
@@ -0,0 +1,3 @@
+function main(params) {
+  return {payload:  params};
+}
\ No newline at end of file
diff --git a/tests/dat/manifest_validate_multiline_params.yaml b/tests/dat/manifest_validate_multiline_params.yaml
new file mode 100644
index 0000000..c5cf966
--- /dev/null
+++ b/tests/dat/manifest_validate_multiline_params.yaml
@@ -0,0 +1,39 @@
+package:
+    name: validate
+    actions:
+        validate_multiline_params:
+            location: actions/dump_params.js
+            runtime: nodejs:6
+            inputs:
+                # value only
+                param_string_value_only:
+                    value: foo
+                param_int_value_only:
+                    value: 123
+                param_float_value_only:
+                    value: 3.14
+                # type and value only
+                param_string_type_and_value_only:
+                    type: string
+                    value: foo
+                # type only
+                param_string_type_only:
+                    type: string
+                param_integer_type_only:
+                    type: integer
+                param_float_type_only:
+                    type: float
+                # No value, but with default value
+                param_string_with_default:
+                    type: string
+                    default: bar
+                param_integer_with_default:
+                    type: integer
+                    default: -1
+                param_float_with_default:
+                    type: float
+                    default: 2.9
+            outputs:
+                payload:
+                    type: string
+                    description: parameter dump
diff --git a/tests/dat/manifest_validate_singleline_params.yaml b/tests/dat/manifest_validate_singleline_params.yaml
new file mode 100644
index 0000000..8b51aa1
--- /dev/null
+++ b/tests/dat/manifest_validate_singleline_params.yaml
@@ -0,0 +1,30 @@
+package:
+    name: validate
+    actions:
+        validate_singleline_params:
+            location: actions/dump_params.js
+            runtime: nodejs:6
+            inputs:
+                # simple string
+                param_simple_string: foo
+                # simple integers
+                param_simple_integer_1: 1
+                param_simple_integer_2: 0
+                param_simple_integer_3: -1
+                param_simple_integer_4: 99999
+                param_simple_integer_5: -99999
+                # simple floats
+                param_simple_float_1: 1.1
+                param_simple_float_2: 0.0
+                param_simple_float_3: -1.1
+                # simple Environment variables
+                param_simple_env_var_1: $GOPATH
+                param_simple_invalid_env_var: $DollarSignNotInEnv
+                # Empty (string)
+                param_simple_implied_empty:
+                param_simple_explicit_empty_1: ''
+                param_simple_explicit_empty_2: ""
+            outputs:
+                payload:
+                    type: string
+                    description: parameter dump
diff --git a/tests/src/integration/triggerrule/manifest.yml b/tests/src/integration/triggerrule/manifest.yml
index b04596f..a707617 100644
--- a/tests/src/integration/triggerrule/manifest.yml
+++ b/tests/src/integration/triggerrule/manifest.yml
@@ -18,6 +18,6 @@ package:
     myRule:
       trigger: locationUpdate
       #the action name and the action file greeting.js should consistent.
-      #currently the implementation deside the action name consistent with action file name?
+      #currently the implementation decide the action name consistent with action file name?
       action: greeting
 
diff --git a/tests/usecases/triggerrule/README.md b/tests/usecases/triggerrule/README.md
index c1eed78..4fde518 100644
--- a/tests/usecases/triggerrule/README.md
+++ b/tests/usecases/triggerrule/README.md
@@ -2,35 +2,56 @@
 
 ### Package description
 
-The package includes:
-- An action named as "hello". It accepts two parameters "name" and "place" and will return a greeting message "Hello, name from place!"
+This Package named "helloworld" includes:
+- An action named "greeting". It accepts two parameters "name" and "place" and will return a greeting message "Hello, name from place!"
 - A trigger named as "locationUpdate"
-- A rule to associate the trigger with the action.
+- A rule named "myRule" to associate the trigger with the action.
 
 ### How to deploy and test
 
 Step 1. Deploy the package.
 ```
-$ wskdeploy -p /tests/usecases/helloworld
+$ wskdeploy -m tests/usecases/triggerrule/manifest.yml
 ```
 Step 2. Verify the action is installed.
 ```
 $ wsk action list
+
+{
+  "name": "greeting",
+  "publish": false,
+  "annotations": [{
+    "key": "exec",
+    "value": "nodejs:6"
+  }],
+  "version": "0.0.1",
+  "namespace": "<namespace>/helloworld"
+}
 ```
 
-Step 3. Verify the action is invoked.
+Step 3. Verify the action's last activation (ID) before invoking.
 ```
-$ wsk activation list --limit 1 hello
-$ wsk activation result <your action ID>
+$ wsk activation list --limit 1 greeting
+
+activations
+3b80ec50d934464faedc04bc991f8673 greeting
+
 ```
 
 Step 4. Invoke the trigger
 ``
 $ wsk trigger fire locationUpdate --param name Bernie --param place "Washington, D.C."
+
+ok: triggered /_/locationUpdate with id 5f8f26928c384afd85f47281bf85b302
 ```
 
 Step 5. Verify the action is invoked
 ```
-$ wsk activation list --limit 1 hello
-$ wsk activation result <your action ID>
+$ wsk activation list --limit 2 greeting
+
+activations
+a2b5cb1076b24c1885c018bb46f4e990 greeting
+3b80ec50d934464faedc04bc991f8673 greeting
 ```
+
+Indeed an new activation was listed.
diff --git a/tests/usecases/triggerrule/deployment.yml b/tests/usecases/triggerrule/deployment.yml
index 74b349c..d9e5e12 100644
--- a/tests/usecases/triggerrule/deployment.yml
+++ b/tests/usecases/triggerrule/deployment.yml
@@ -7,9 +7,9 @@ application:
   package:
     triggerrule:
       name: helloworld
-      namespace: guest
+     namespace: guest
       actions:
         greeting:
           inputs:
             name: Bernie
-            place: Vermont
+            place: DC
diff --git a/tests/usecases/webhook/deployment.yml b/tests/usecases/webhook/deployment.yml
index 1f6f4b6..6d6f772 100644
--- a/tests/usecases/webhook/deployment.yml
+++ b/tests/usecases/webhook/deployment.yml
@@ -10,7 +10,7 @@ application:
         webhook:
           #we treat the inputs as parameters besides --feed.
           inputs:
-            # TODO(dliu): Please fix "repsotiory value" and see if we can change/remove username
+            # TODO(dliu): Please fix "repository" value and see if we can change/remove username
             # as actual developer names should not be in these files when possible.
             username: daisy-ycguo
             repository: https://github.com/openwhisk/wsktool.git
diff --git a/utils/errorhandlers.go b/utils/errorhandlers.go
index 6e08cf6..2b87f49 100644
--- a/utils/errorhandlers.go
+++ b/utils/errorhandlers.go
@@ -24,15 +24,6 @@ import (
 	"os"
 )
 
-// ServerlessErr records errors from the Serverless binary
-type ServerlessErr struct {
-	Msg string
-}
-
-func (e *ServerlessErr) Error() string {
-	return e.Msg
-}
-
 // Check is a util function to panic when there is an error.
 func Check(e error) {
 	defer func() {
diff --git a/utils/misc.go b/utils/misc.go
index 44ea6db..823e4d4 100644
--- a/utils/misc.go
+++ b/utils/misc.go
@@ -130,18 +130,39 @@ func Ask(reader *bufio.Reader, question string, def string) string {
 	return answer[:len-1]
 }
 
+
+// Test if a string
+func isValidEnvironmentVar( value string ) bool {
+
+	// A valid Env. variable should start with '$' (dollar) char.
+	// AND have at least 1 additional character after it.
+	if value != "" && len(value) > 1 && strings.HasPrefix(value, "$") {
+		return true
+	}
+	return false
+}
+
 // Get the env variable value by key.
 // Get the env variable if the key is start by $
 func GetEnvVar(key interface{}) interface{} {
+	// Assure the key itself is not nil
+	if (key == nil ) {
+		return nil
+	}
+
 	if reflect.TypeOf(key).String() == "string" {
-		if strings.HasPrefix(key.(string), "$") {
+		if isValidEnvironmentVar(key.(string)) {
+			// retrieve the value of the env. var. from the host system.
 			envkey := strings.Split(key.(string), "$")[1]
 			value := os.Getenv(envkey)
-			if value != "" {
-				return value
+			if value == "" {
+				// TODO() We should issue a warning to the user (verbose) that env. var. was not found
+				// (i.e., and empty string was returned).
 			}
-			return envkey
+			return value
 		}
+
+		// The key was not a valid env. variable, simply return it as the value itself (of type string)
 		return key.(string)
 	}
 	return key
@@ -295,6 +316,23 @@ func javaEntryError() error {
 	return errors.New(errMsg)
 }
 
+// ParserErr records errors from parsing YAML against the wskdeploy spec.
+type ParserErr struct {
+	filneame string
+        lineNum int
+	message string
+}
+
+// Implement the error interface.
+func (e ParserErr) Error() string {
+	return fmt.Sprintf("%s [%d]: %s", e.filneame, e.lineNum, e.message)
+}
+
+func NewParserErr(fname string, line int, msg string) error {
+	var err = &ParserErr{"", -1, msg}
+	return err
+}
+
 //for web action support, code from wsk cli with tiny adjustments
 const WEB_EXPORT_ANNOT = "web-export"
 const RAW_HTTP_ANNOT = "raw-http"
diff --git a/utils/util_test.go b/utils/util_test.go
index 1a1b871..21c4b6a 100644
--- a/utils/util_test.go
+++ b/utils/util_test.go
@@ -56,7 +56,7 @@ func TestGetEnvVar(t *testing.T) {
 	fmt.Println(GetEnvVar("$WithDollar"))
 	fmt.Println(GetEnvVar("$5000"))
 	assert.Equal(t, "NoDollar", GetEnvVar("NoDollar"), "NoDollar should be no change.")
-	assert.Equal(t, "oh, dollars!", GetEnvVar("$WithDollar"), "dollar sign should be handled")
-	assert.Equal(t, "5000", GetEnvVar("5000"), "Should be no difference between integer and string")
-	assert.Equal(t, "WithDollarAgain", GetEnvVar("$WithDollarAgain"), "if not found, just return the env")
+	assert.Equal(t, "oh, dollars!", GetEnvVar("$WithDollar"), "dollar sign should be handled.")
+	assert.Equal(t, "5000", GetEnvVar("5000"), "Should be no difference between integer and string.")
+	assert.Equal(t, "", GetEnvVar("$WithDollarAgain"), "if not found in environemnt, return empty string.")
 }

-- 
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].