Issue1317 - Added error logging to webhook controller

Author: soltysh

pkg/build/webhook/controller.go

@@ -5,6 +5,8 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/golang/glog"
+
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/openshift/origin/pkg/build/api"
 	buildclient "github.com/openshift/origin/pkg/build/client"
@@ -54,23 +56,27 @@ func NewController(buildConfigGetter buildclient.BuildConfigGetter, buildCreator
 func (c *controller) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	uv, err := parseURL(req)
 	if err != nil {
+		glog.V(4).Infof("Failed parsing request URL: %v", err)
 		notFound(w, err.Error())
 		return
 	}
 
 	buildCfg, err := c.buildConfigGetter.Get(uv.namespace, uv.buildConfigName)
 	if err != nil {
+		glog.V(4).Infof("Failed getting BuildConfig: %v", err)
 		badRequest(w, err.Error())
 		return
 	}
 
 	plugin, ok := c.plugins[uv.plugin]
 	if !ok {
+		glog.V(4).Infof("Plugin %s not found", uv.plugin)
 		notFound(w, "Plugin ", uv.plugin, " not found")
 		return
 	}
 	revision, proceed, err := plugin.Extract(buildCfg, uv.secret, uv.path, req)
 	if err != nil {
+		glog.V(4).Infof("Failed extracting information from webhook: %v", err)
 		badRequest(w, err.Error())
 		return
 	}
@@ -79,10 +85,12 @@ func (c *controller) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	}
 	build, err := buildutil.GenerateBuildWithImageTag(buildCfg, revision, c.imageRepoGetter)
 	if err != nil {
+		glog.V(4).Infof("Failed generating new build: %v", err)
 		badRequest(w, err.Error())
 		return
 	}
 	if err := c.buildCreator.Create(uv.namespace, build); err != nil {
+		glog.V(4).Infof("Failed creating new build: %v", err)
 		badRequest(w, err.Error())
 	}
 }

pkg/build/webhook/generic/generic.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/golang/glog"
+
 	"github.com/openshift/origin/pkg/build/api"
 	"github.com/openshift/origin/pkg/build/webhook"
 )
@@ -40,11 +41,12 @@ func (p *WebHookPlugin) Extract(buildCfg *api.BuildConfig, secret, path string,
 			return nil, false, err
 		}
 		if len(body) == 0 {
-			return revision, true, nil
+			return nil, true, nil
 		}
 		var data api.GenericWebHookEvent
 		if err = json.Unmarshal(body, &data); err != nil {
-			return nil, false, err
+			glog.V(4).Infof("Error unmarshaling json %v, but continuing", err)
+			return nil, true, nil
 		}
 		if !webhook.GitRefMatches(data.Git.Ref, buildCfg.Parameters.Source.Git.Ref) {
 			glog.V(2).Infof("Skipping build for '%s'.  Branch reference from '%s' does not match configuration", buildCfg, data)

pkg/build/webhook/generic/generic_test.go

@@ -2,6 +2,7 @@ package generic
 
 import (
 	"bytes"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"strings"
@@ -191,3 +192,37 @@ func TestExtractWithGitPayload(t *testing.T) {
 		t.Error("Expected the 'revision' return value to not be nil")
 	}
 }
+
+type errJSON struct{}
+
+func (*errJSON) Read(p []byte) (n int, err error) {
+	p = []byte("{")
+	return len(p), io.EOF
+}
+
+func TestExtractWithUnmarshalError(t *testing.T) {
+	req, _ := http.NewRequest("POST", "http://someurl.com", &errJSON{})
+	req.Header.Add("User-Agent", "Some User Agent")
+	req.Header.Add("Content-Type", "application/json")
+	buildConfig := &api.BuildConfig{
+		Triggers: []api.BuildTriggerPolicy{
+			{
+				Type: api.GenericWebHookBuildTriggerType,
+				GenericWebHook: &api.WebHookTrigger{
+					Secret: "secret100",
+				},
+			},
+		},
+	}
+	plugin := New()
+	revision, proceed, err := plugin.Extract(buildConfig, "secret100", "", req)
+	if err != nil {
+		t.Errorf("Expected to be able to trigger a build without a payload error: %s", err)
+	}
+	if !proceed {
+		t.Error("Expected 'proceed' return value to be 'true'")
+	}
+	if revision != nil {
+		t.Error("Expected the 'revision' return value to be nil")
+	}
+}