Show failed parameter node lookups

When attempting to match a JSON path for initial setup, it would be
helpful to know where the path failed. This change logs the failed
parameter node. For example, if you are trying to match path "a.b.d.e",
but you failed to include the "c" node, webhook will log an error
"parameter node not found: d.e" to assist in troubleshooting.

internal/hook/hook.go

@@ -46,6 +46,28 @@ const (
 	EnvNamespace string = "HOOK_"
 )
 
+// ParameterNodeError describes an error walking a parameter node.
+type ParameterNodeError struct {
+	key string
+}
+
+func (e *ParameterNodeError) Error() string {
+	if e == nil {
+		return "<nil>"
+	}
+	return fmt.Sprintf("parameter node not found: %s", e.key)
+}
+
+// IsParameterNodeError returns whether err is of type ParameterNodeError.
+func IsParameterNodeError(err error) bool {
+	switch err.(type) {
+	case *ParameterNodeError:
+		return true
+	default:
+		return false
+	}
+}
+
 // SignatureError describes an invalid payload signature passed to Hook.
 type SignatureError struct {
 	Signature string
@@ -274,9 +296,9 @@ func ReplaceParameter(s string, params interface{}, value interface{}) bool {
 }
 
 // GetParameter extracts interface{} value based on the passed string
-func GetParameter(s string, params interface{}) (interface{}, bool) {
+func GetParameter(s string, params interface{}) (interface{}, error) {
 	if params == nil {
-		return nil, false
+		return nil, errors.New("no parameters")
 	}
 
 	paramsValue := reflect.ValueOf(params)
@@ -290,7 +312,7 @@ func GetParameter(s string, params interface{}) (interface{}, bool) {
 				index, err := strconv.ParseUint(p[0], 10, 64)
 
 				if err != nil || paramsValueSliceLength <= int(index) {
-					return nil, false
+					return nil, &ParameterNodeError{s}
 				}
 
 				return GetParameter(p[1], params.([]interface{})[index])
@@ -299,18 +321,18 @@ func GetParameter(s string, params interface{}) (interface{}, bool) {
 			index, err := strconv.ParseUint(s, 10, 64)
 
 			if err != nil || paramsValueSliceLength <= int(index) {
-				return nil, false
+				return nil, &ParameterNodeError{s}
 			}
 
-			return params.([]interface{})[index], true
+			return params.([]interface{})[index], nil
 		}
 
-		return nil, false
+		return nil, &ParameterNodeError{s}
 
 	case reflect.Map:
 		// Check for raw key
 		if v, ok := params.(map[string]interface{})[s]; ok {
-			return v, true
+			return v, nil
 		}
 
 		// Checked for dotted references
@@ -320,19 +342,21 @@ func GetParameter(s string, params interface{}) (interface{}, bool) {
 				return GetParameter(p[1], pValue)
 			}
 
-			return pValue, true
+			return pValue, nil
 		}
 	}
 
-	return nil, false
+	return nil, &ParameterNodeError{s}
 }
 
 // ExtractParameterAsString extracts value from interface{} as string based on the passed string
-func ExtractParameterAsString(s string, params interface{}) (string, bool) {
+func ExtractParameterAsString(s string, params interface{}) (string, error) {
-	if pValue, ok := GetParameter(s, params); ok {
+	pValue, err := GetParameter(s, params)
-		return fmt.Sprintf("%v", pValue), true
+	if err != nil {
+		return "", err
 	}
-	return "", false
+
+	return fmt.Sprintf("%v", pValue), nil
 }
 
 // Argument type specifies the parameter key name and the source it should
@@ -346,7 +370,7 @@ type Argument struct {
 
 // Get Argument method returns the value for the Argument's key name
 // based on the Argument's source
-func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string, bool) {
+func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string, error) {
 	var source *map[string]interface{}
 	key := ha.Name
 
@@ -359,35 +383,35 @@ func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string
 	case SourcePayload:
 		source = payload
 	case SourceString:
-		return ha.Name, true
+		return ha.Name, nil
 	case SourceEntirePayload:
 		r, err := json.Marshal(payload)
 		if err != nil {
-			return "", false
+			return "", err
 		}
 
-		return string(r), true
+		return string(r), nil
 	case SourceEntireHeaders:
 		r, err := json.Marshal(headers)
 		if err != nil {
-			return "", false
+			return "", err
 		}
 
-		return string(r), true
+		return string(r), nil
 	case SourceEntireQuery:
 		r, err := json.Marshal(query)
 		if err != nil {
-			return "", false
+			return "", err
 		}
 
-		return string(r), true
+		return string(r), nil
 	}
 
 	if source != nil {
 		return ExtractParameterAsString(key, *source)
 	}
 
-	return "", false
+	return "", errors.New("no source for value retrieval")
 }
 
 // Header is a structure containing header name and it's value
@@ -469,7 +493,10 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface
 	errors := make([]error, 0)
 
 	for i := range h.JSONStringParameters {
-		if arg, ok := h.JSONStringParameters[i].Get(headers, query, payload); ok {
+		arg, err := h.JSONStringParameters[i].Get(headers, query, payload)
+		if err != nil {
+			errors = append(errors, &ArgumentError{h.JSONStringParameters[i]})
+		} else {
 			var newArg map[string]interface{}
 
 			decoder := json.NewDecoder(strings.NewReader(string(arg)))
@@ -503,8 +530,6 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface
 			} else {
 				errors = append(errors, &SourceError{h.JSONStringParameters[i]})
 			}
-		} else {
-			errors = append(errors, &ArgumentError{h.JSONStringParameters[i]})
 		}
 	}
 
@@ -524,12 +549,14 @@ func (h *Hook) ExtractCommandArguments(headers, query, payload *map[string]inter
 	args = append(args, h.ExecuteCommand)
 
 	for i := range h.PassArgumentsToCommand {
-		if arg, ok := h.PassArgumentsToCommand[i].Get(headers, query, payload); ok {
+		arg, err := h.PassArgumentsToCommand[i].Get(headers, query, payload)
-			args = append(args, arg)
+		if err != nil {
-		} else {
 			args = append(args, "")
 			errors = append(errors, &ArgumentError{h.PassArgumentsToCommand[i]})
+			continue
 		}
+
+		args = append(args, arg)
 	}
 
 	if len(errors) > 0 {
@@ -546,7 +573,12 @@ func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string
 	args := make([]string, 0)
 	errors := make([]error, 0)
 	for i := range h.PassEnvironmentToCommand {
-		if arg, ok := h.PassEnvironmentToCommand[i].Get(headers, query, payload); ok {
+		arg, err := h.PassEnvironmentToCommand[i].Get(headers, query, payload)
+		if err != nil {
+			errors = append(errors, &ArgumentError{h.PassEnvironmentToCommand[i]})
+			continue
+		}
+
 		if h.PassEnvironmentToCommand[i].EnvName != "" {
 			// first try to use the EnvName if specified
 			args = append(args, h.PassEnvironmentToCommand[i].EnvName+"="+arg)
@@ -554,9 +586,6 @@ func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string
 			// then fallback on the name
 			args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg)
 		}
-		} else {
-			errors = append(errors, &ArgumentError{h.PassEnvironmentToCommand[i]})
-		}
 	}
 
 	if len(errors) > 0 {
@@ -580,7 +609,11 @@ func (h *Hook) ExtractCommandArgumentsForFile(headers, query, payload *map[strin
 	args := make([]FileParameter, 0)
 	errors := make([]error, 0)
 	for i := range h.PassFileToCommand {
-		if arg, ok := h.PassFileToCommand[i].Get(headers, query, payload); ok {
+		arg, err := h.PassFileToCommand[i].Get(headers, query, payload)
+		if err != nil {
+			errors = append(errors, &ArgumentError{h.PassFileToCommand[i]})
+			continue
+		}
 
 		if h.PassFileToCommand[i].EnvName == "" {
 			// if no environment-variable name is set, fall-back on the name
@@ -600,10 +633,6 @@ func (h *Hook) ExtractCommandArgumentsForFile(headers, query, payload *map[strin
 		}
 
 		args = append(args, FileParameter{EnvName: h.PassFileToCommand[i].EnvName, Data: fileContent})
-
-		} else {
-			errors = append(errors, &ArgumentError{h.PassFileToCommand[i]})
-		}
 	}
 
 	if len(errors) > 0 {
@@ -785,7 +814,8 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod
 		return CheckScalrSignature(*headers, *body, r.Secret, true)
 	}
 
-	if arg, ok := r.Parameter.Get(headers, query, payload); ok {
+	arg, err := r.Parameter.Get(headers, query, payload)
+	if err == nil {
 		switch r.Type {
 		case MatchValue:
 			return compare(arg, r.Value), nil
@@ -802,7 +832,7 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod
 			return err == nil, err
 		}
 	}
-	return false, nil
+	return false, err
 }
 
 // compare is a helper function for constant time string comparisons.

internal/hook/hook_test.go

@@ -28,9 +28,9 @@ func TestGetParameter(t *testing.T) {
 		{"z.b", map[string]interface{}{"a": map[string]interface{}{"z": 2}}, nil, false},
 		{"a.2", map[string]interface{}{"a": []interface{}{"a", "b"}}, nil, false},
 	} {
-		res, ok := GetParameter(test.key, test.val)
+		res, err := GetParameter(test.key, test.val)
-		if ok != test.ok {
+		if (err == nil) != test.ok {
-			t.Errorf("unexpected result given {%q, %q}: %t\n", test.key, test.val, ok)
+			t.Errorf("unexpected result given {%q, %q}: %s\n", test.key, test.val, err)
 		}
 
 		if !reflect.DeepEqual(res, test.expect) {
@@ -225,9 +225,9 @@ var extractParameterTests = []struct {
 
 func TestExtractParameter(t *testing.T) {
 	for _, tt := range extractParameterTests {
-		value, ok := ExtractParameterAsString(tt.s, tt.params)
+		value, err := ExtractParameterAsString(tt.s, tt.params)
-		if ok != tt.ok || value != tt.value {
+		if (err == nil) != tt.ok || value != tt.value {
-			t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, ok:%#v}", tt.s, tt.value, tt.ok, value, ok)
+			t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.s, tt.value, tt.ok, value, err)
 		}
 	}
 }
@@ -252,9 +252,9 @@ var argumentGetTests = []struct {
 func TestArgumentGet(t *testing.T) {
 	for _, tt := range argumentGetTests {
 		a := Argument{tt.source, tt.name, "", false}
-		value, ok := a.Get(tt.headers, tt.query, tt.payload)
+		value, err := a.Get(tt.headers, tt.query, tt.payload)
-		if ok != tt.ok || value != tt.value {
+		if (err == nil) != tt.ok || value != tt.value {
-			t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, ok:%#v}", tt.source, tt.name, tt.value, tt.ok, value, ok)
+			t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.source, tt.name, tt.value, tt.ok, value, err)
 		}
 	}
 }
@@ -452,7 +452,7 @@ var matchRuleTests = []struct {
 	// failures
 	{"value", "", "", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false},
 	{"regex", "^X", "", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false},
-	{"value", "", "2", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, false}, // reference invalid header
+	{"value", "", "2", "X", "", Argument{"header", "a", "", false}, &map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, true}, // reference invalid header
 	// errors
 	{"regex", "*", "", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, true},                   // invalid regex
 	{"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true},   // invalid hmac
@@ -477,7 +477,7 @@ func TestMatchRule(t *testing.T) {
 		r := MatchRule{tt.typ, tt.regex, tt.secret, tt.value, tt.param, tt.ipRange}
 		ok, err := r.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, tt.remoteAddr)
 		if ok != tt.ok || (err != nil) != tt.err {
-			t.Errorf("%d failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", i, r, tt.ok, tt.err, ok, (err != nil))
+			t.Errorf("%d failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", i, r, tt.ok, tt.err, ok, err)
 		}
 	}
 }
@@ -540,7 +540,7 @@ var andRuleTests = []struct {
 		"invalid rule",
 		AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}},
 		&map[string]interface{}{"Y": "z"}, nil, nil, nil,
-		false, false,
+		false, true,
 	},
 }
 
@@ -595,7 +595,7 @@ var orRuleTests = []struct {
 			{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}},
 		},
 		&map[string]interface{}{"Y": "Z"}, nil, nil, []byte{},
-		false, false,
+		false, true,
 	},
 }
 

webhook.go

@@ -473,12 +473,16 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 	} else {
 		ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body, r.RemoteAddr)
 		if err != nil {
+			if !hook.IsParameterNodeError(err) {
 				msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err)
 				log.Println(msg)
 				w.WriteHeader(http.StatusInternalServerError)
 				fmt.Fprint(w, "Error occurred while evaluating hook rules.")
 				return
 			}
+
+			log.Printf("[%s] %v", rid, err)
+		}
 	}
 
 	if ok {