From 4606677df485bbf76ce2850f36d799070947d4d7 Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Tue, 17 May 2016 17:42:54 +0200 Subject: [PATCH 1/8] Adding ignore patterns --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index e69de29..ee20f29 100644 --- a/.gitignore +++ b/.gitignore @@ -0,0 +1,4 @@ +.idea +.cover +coverage +webhook From 673e84a7ea21ea37ea1594d6dd8c9e56b366bdd8 Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Tue, 17 May 2016 17:43:31 +0200 Subject: [PATCH 2/8] Adding support for env var naming --- hook/hook.go | 13 ++++++-- hook/hook_test.go | 76 +++++++++++++++++++++++------------------------ 2 files changed, 48 insertions(+), 41 deletions(-) diff --git a/hook/hook.go b/hook/hook.go index d23993e..ac9d3c9 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -196,8 +196,9 @@ func ExtractParameterAsString(s string, params interface{}) (string, bool) { // Argument type specifies the parameter key name and the source it should // be extracted from type Argument struct { - Source string `json:"source,omitempty"` - Name string `json:"name,omitempty"` + Source string `json:"source,omitempty"` + Name string `json:"name,omitempty"` + EnvName string `json:"envname,omitempty"` } // Get Argument method returns the value for the Argument's key name @@ -364,7 +365,13 @@ func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string for i := range h.PassEnvironmentToCommand { if arg, ok := h.PassEnvironmentToCommand[i].Get(headers, query, payload); ok { - args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg) + if h.PassEnvironmentToCommand[i].EnvName != "" { + // first try to use the EnvName if specified + args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].EnvName+"="+arg) + } else { + // then fallbask on the name + args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg) + } } else { return args, &ArgumentError{h.PassEnvironmentToCommand[i]} } diff --git a/hook/hook_test.go b/hook/hook_test.go index f4d5b8f..e273665 100644 --- a/hook/hook_test.go +++ b/hook/hook_test.go @@ -80,7 +80,7 @@ var argumentGetTests = []struct { func TestArgumentGet(t *testing.T) { for _, tt := range argumentGetTests { - a := Argument{tt.source, tt.name} + a := Argument{tt.source, tt.name, ""} value, ok := a.Get(tt.headers, tt.query, tt.payload) if ok != 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) @@ -94,14 +94,14 @@ var hookParseJSONParametersTests = []struct { rheaders, rquery, rpayload *map[string]interface{} ok bool }{ - {[]Argument{Argument{"header", "a"}}, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, nil, true}, - {[]Argument{Argument{"url", "a"}}, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true}, - {[]Argument{Argument{"payload", "a"}}, nil, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true}, - {[]Argument{Argument{"header", "z"}}, &map[string]interface{}{"z": `{}`}, nil, nil, &map[string]interface{}{"z": map[string]interface{}{}}, nil, nil, true}, + {[]Argument{Argument{"header", "a", ""}}, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, nil, true}, + {[]Argument{Argument{"url", "a", ""}}, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true}, + {[]Argument{Argument{"payload", "a", ""}}, nil, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true}, + {[]Argument{Argument{"header", "z", ""}}, &map[string]interface{}{"z": `{}`}, nil, nil, &map[string]interface{}{"z": map[string]interface{}{}}, nil, nil, true}, // failures - {[]Argument{Argument{"header", "z"}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false}, // empty string - {[]Argument{Argument{"header", "y"}}, &map[string]interface{}{"X": `{}`}, nil, nil, &map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter - {[]Argument{Argument{"string", "z"}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false}, // invalid argument source + {[]Argument{Argument{"header", "z", ""}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false}, // empty string + {[]Argument{Argument{"header", "y", ""}}, &map[string]interface{}{"X": `{}`}, nil, nil, &map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter + {[]Argument{Argument{"string", "z", ""}}, &map[string]interface{}{"z": ``}, nil, nil, &map[string]interface{}{"z": ``}, nil, nil, false}, // invalid argument source } func TestHookParseJSONParameters(t *testing.T) { @@ -121,9 +121,9 @@ var hookExtractCommandArgumentsTests = []struct { value []string ok bool }{ - {"test", []Argument{Argument{"header", "a"}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"test", "z"}, true}, + {"test", []Argument{Argument{"header", "a", ""}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"test", "z"}, true}, // failures - {"fail", []Argument{Argument{"payload", "a"}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"fail", ""}, false}, + {"fail", []Argument{Argument{"payload", "a", ""}}, &map[string]interface{}{"a": "z"}, nil, nil, []string{"fail", ""}, false}, } func TestHookExtractCommandArguments(t *testing.T) { @@ -182,16 +182,16 @@ var matchRuleTests = []struct { ok bool err bool }{ - {"value", "", "", "z", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false}, - {"regex", "^z", "", "z", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false}, - {"payload-hash-sha1", "", "secret", "", Argument{"header", "a"}, &map[string]interface{}{"a": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), true, false}, + {"value", "", "", "z", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false}, + {"regex", "^z", "", "z", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false}, + {"payload-hash-sha1", "", "secret", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), true, false}, // failures - {"value", "", "", "X", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false}, - {"regex", "^X", "", "", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false}, - {"value", "", "2", "X", Argument{"header", "a"}, &map[string]interface{}{"y": "z"}, nil, nil, []byte{}, false, false}, // reference invalid header + {"value", "", "", "X", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false}, + {"regex", "^X", "", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false}, + {"value", "", "2", "X", Argument{"header", "a", ""}, &map[string]interface{}{"y": "z"}, nil, nil, []byte{}, false, false}, // reference invalid header // errors - {"regex", "*", "", "", Argument{"header", "a"}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, true}, // invalid regex - {"payload-hash-sha1", "", "secret", "", Argument{"header", "a"}, &map[string]interface{}{"a": ""}, nil, nil, []byte{}, false, true}, // invalid hmac + {"regex", "*", "", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, true}, // invalid regex + {"payload-hash-sha1", "", "secret", "", Argument{"header", "a", ""}, &map[string]interface{}{"a": ""}, nil, nil, []byte{}, false, true}, // invalid hmac } func TestMatchRule(t *testing.T) { @@ -215,8 +215,8 @@ var andRuleTests = []struct { { "(a=z, b=y): a=z && b=y", AndRule{ - {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, - {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}}, + {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, + {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}}, }, &map[string]interface{}{"a": "z", "b": "y"}, nil, nil, []byte{}, true, false, @@ -224,8 +224,8 @@ var andRuleTests = []struct { { "(a=z, b=Y): a=z && b=y", AndRule{ - {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, - {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}}, + {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, + {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}}, }, &map[string]interface{}{"a": "z", "b": "Y"}, nil, nil, []byte{}, false, false, @@ -234,22 +234,22 @@ var andRuleTests = []struct { { "(a=z, b=y, c=x, d=w=, e=X, f=X): a=z && (b=y && c=x) && (d=w || e=v) && !f=u", AndRule{ - {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, + {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, { And: &AndRule{ - {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}}, - {Match: &MatchRule{"value", "", "", "x", Argument{"header", "c"}}}, + {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}}, + {Match: &MatchRule{"value", "", "", "x", Argument{"header", "c", ""}}}, }, }, { Or: &OrRule{ - {Match: &MatchRule{"value", "", "", "w", Argument{"header", "d"}}}, - {Match: &MatchRule{"value", "", "", "v", Argument{"header", "e"}}}, + {Match: &MatchRule{"value", "", "", "w", Argument{"header", "d", ""}}}, + {Match: &MatchRule{"value", "", "", "v", Argument{"header", "e", ""}}}, }, }, { Not: &NotRule{ - Match: &MatchRule{"value", "", "", "u", Argument{"header", "f"}}, + Match: &MatchRule{"value", "", "", "u", Argument{"header", "f", ""}}, }, }, }, @@ -260,7 +260,7 @@ var andRuleTests = []struct { // failures { "invalid rule", - AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a"}}}}, + AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", ""}}}}, &map[string]interface{}{"y": "z"}, nil, nil, nil, false, false, }, @@ -286,8 +286,8 @@ var orRuleTests = []struct { { "(a=z, b=X): a=z || b=y", OrRule{ - {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, - {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}}, + {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, + {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}}, }, &map[string]interface{}{"a": "z", "b": "X"}, nil, nil, []byte{}, true, false, @@ -295,8 +295,8 @@ var orRuleTests = []struct { { "(a=X, b=y): a=z || b=y", OrRule{ - {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, - {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}}, + {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, + {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}}, }, &map[string]interface{}{"a": "X", "b": "y"}, nil, nil, []byte{}, true, false, @@ -304,8 +304,8 @@ var orRuleTests = []struct { { "(a=Z, b=Y): a=z || b=y", OrRule{ - {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, - {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b"}}}, + {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, + {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", ""}}}, }, &map[string]interface{}{"a": "Z", "b": "Y"}, nil, nil, []byte{}, false, false, @@ -314,7 +314,7 @@ var orRuleTests = []struct { { "invalid rule", OrRule{ - {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, + {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, }, &map[string]interface{}{"y": "Z"}, nil, nil, []byte{}, false, false, @@ -338,8 +338,8 @@ var notRuleTests = []struct { ok bool err bool }{ - {"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a"}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false}, - {"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a"}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false}, + {"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", ""}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, true, false}, + {"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", ""}}}, &map[string]interface{}{"a": "z"}, nil, nil, []byte{}, false, false}, } func TestNotRule(t *testing.T) { From 982687228757d82b71d5b0161fcdd4c61081c2a6 Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Mon, 23 May 2016 12:27:08 +0200 Subject: [PATCH 3/8] Fixed typo in docstring --- hook/hook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hook/hook.go b/hook/hook.go index ac9d3c9..d0115e8 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -369,7 +369,7 @@ func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string // first try to use the EnvName if specified args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].EnvName+"="+arg) } else { - // then fallbask on the name + // then fallback on the name args = append(args, EnvNamespace+h.PassEnvironmentToCommand[i].Name+"="+arg) } } else { From dcec0f7e5be00514b5c0d71837ed2fec20e19273 Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Mon, 23 May 2016 12:28:05 +0200 Subject: [PATCH 4/8] Adding tests for the env var extraction w & w/o explicit naming --- hook/hook_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/hook/hook_test.go b/hook/hook_test.go index e273665..472787a 100644 --- a/hook/hook_test.go +++ b/hook/hook_test.go @@ -136,6 +136,67 @@ func TestHookExtractCommandArguments(t *testing.T) { } } +// Here we test the extraction of env variables when the user defined a hook +// with the "pass-environment-to-command" directive +// we test both cases where the name of the data is used as the name of the +// env key & the case where the hook definition sets the env var name to a +// fixed value using the envname construct like so:: +// [ +// { +// "id": "push", +// "execute-command": "bb2mm", +// "command-working-directory": "/tmp", +// "pass-environment-to-command": +// [ +// { +// "source": "entire-payload", +// "envname": "PAYLOAD" +// }, +// ] +// } +// ] +var hookExtractCommandArgumentsForEnvTests = []struct { + exec string + args []Argument + headers, query, payload *map[string]interface{} + value []string + ok bool +}{ + // successes + { + "test", + []Argument{Argument{"header", "a", ""}}, + &map[string]interface{}{"a": "z"}, nil, nil, + []string{"HOOK_a=z"}, + true, + }, + { + "test", + []Argument{Argument{"header", "a", "MYKEY"}}, + &map[string]interface{}{"a": "z"}, nil, nil, + []string{"HOOK_MYKEY=z"}, + true, + }, + // failures + { + "fail", + []Argument{Argument{"payload", "a", ""}}, + &map[string]interface{}{"a": "z"}, nil, nil, + []string{}, + false, + }, +} + +func TestHookExtractCommandArgumentsForEnv(t *testing.T) { + for _, tt := range hookExtractCommandArgumentsForEnvTests { + h := &Hook{ExecuteCommand: tt.exec, PassEnvironmentToCommand: tt.args} + value, err := h.ExtractCommandArgumentsForEnv(tt.headers, tt.query, tt.payload) + if (err == nil) != tt.ok || !reflect.DeepEqual(value, tt.value) { + t.Errorf("failed to extract args for env {cmd=%q, args=%v}:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.exec, tt.args, tt.value, tt.ok, value, (err == nil)) + } + } +} + var hooksLoadFromFileTests = []struct { path string ok bool From 52dc8e0edcd6d7b4d3c23bc212af3d8eea001ff2 Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Mon, 23 May 2016 12:29:33 +0200 Subject: [PATCH 5/8] remove coverage script from ignore patterns --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index ee20f29..08af0ee 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ .idea .cover -coverage webhook From 7e9bfaa351a9a1ef79de552f06905b5f3368f1c9 Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Mon, 23 May 2016 12:30:45 +0200 Subject: [PATCH 6/8] Adding the coverage script to help see which code is tested and which is not --- coverage | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100755 coverage diff --git a/coverage b/coverage new file mode 100755 index 0000000..80940df --- /dev/null +++ b/coverage @@ -0,0 +1,40 @@ +#!/bin/sh +# +# this file is from: +# https://raw.githubusercontent.com/mlafeldt/chef-runner/v0.7.0/script/coverage +# it is covered by the Apache License v2.0 +# modified for our purpose +# +# Generate test coverage statistics for Go packages. +# +# Works around the fact that `go test -coverprofile` currently does not work +# with multiple packages, see https://code.google.com/p/go/issues/detail?id=6909 +# +# Usage: script/coverage +# + +#set -e + +workdir=.cover +profile="$workdir/cover.out" +mode=count + +generate_cover_data() { + echo "Cleaning" + rm -rf "$workdir" + mkdir "$workdir" + + for pkg in "$@"; do + echo $pkg + f="$workdir/$(echo $pkg | tr / -).cover" + go test -covermode="$mode" -coverprofile="$f" "$pkg" + done + + echo "Building coverfile" + echo "mode: $mode" >"$profile" + grep -h -v "^mode:" "$workdir"/*.cover >>"$profile" +} + +generate_cover_data $(go list ./...) +echo "Showing report" +go tool cover -html="$profile" From 142f4bed05a1fb3d062d1e30c633ba0438bbaddc Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Tue, 24 May 2016 09:21:54 +0200 Subject: [PATCH 7/8] remove coverage script from sources --- coverage | 40 ---------------------------------------- 1 file changed, 40 deletions(-) delete mode 100755 coverage diff --git a/coverage b/coverage deleted file mode 100755 index 80940df..0000000 --- a/coverage +++ /dev/null @@ -1,40 +0,0 @@ -#!/bin/sh -# -# this file is from: -# https://raw.githubusercontent.com/mlafeldt/chef-runner/v0.7.0/script/coverage -# it is covered by the Apache License v2.0 -# modified for our purpose -# -# Generate test coverage statistics for Go packages. -# -# Works around the fact that `go test -coverprofile` currently does not work -# with multiple packages, see https://code.google.com/p/go/issues/detail?id=6909 -# -# Usage: script/coverage -# - -#set -e - -workdir=.cover -profile="$workdir/cover.out" -mode=count - -generate_cover_data() { - echo "Cleaning" - rm -rf "$workdir" - mkdir "$workdir" - - for pkg in "$@"; do - echo $pkg - f="$workdir/$(echo $pkg | tr / -).cover" - go test -covermode="$mode" -coverprofile="$f" "$pkg" - done - - echo "Building coverfile" - echo "mode: $mode" >"$profile" - grep -h -v "^mode:" "$workdir"/*.cover >>"$profile" -} - -generate_cover_data $(go list ./...) -echo "Showing report" -go tool cover -html="$profile" From f7e573f3dfaaa1e029a1f70623e2395ad8bb7647 Mon Sep 17 00:00:00 2001 From: Florent AIDE Date: Tue, 24 May 2016 09:57:30 +0200 Subject: [PATCH 8/8] Ignore coverage script from sources tree --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 08af0ee..ee20f29 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .idea .cover +coverage webhook