@@ -51,17 +51,17 @@ func (w *WebHook) ServeHTTP(writer http.ResponseWriter, req *http.Request, ctx k

 	}

 	revision, envvars, proceed, err := plugin.Extract(config, secret, "", req)

-	switch err {

-	case webhook.ErrSecretMismatch, webhook.ErrHookNotEnabled:

-		return errors.NewUnauthorized(fmt.Sprintf("the webhook %q for %q did not accept your secret", hookType, name))

-	case nil:

-	default:

-		return errors.NewInternalError(fmt.Errorf("hook failed: %v", err))

-	}

-

 	if !proceed {

-		return nil

+		switch err {

+		case webhook.ErrSecretMismatch, webhook.ErrHookNotEnabled:

+			return errors.NewUnauthorized(fmt.Sprintf("the webhook %q for %q did not accept your secret", hookType, name))

+		case nil:

+			return nil

+		default:

+			return errors.NewInternalError(fmt.Errorf("hook failed: %v", err))

+		}

 	}

+	warning := err

 	buildTriggerCauses := generateBuildTriggerInfo(revision, hookType, secret)

 	request := &buildapi.BuildRequest{

@@ -73,7 +73,7 @@ func (w *WebHook) ServeHTTP(writer http.ResponseWriter, req *http.Request, ctx k

 	if _, err := w.instantiator.Instantiate(config.Namespace, request); err != nil {

 		return errors.NewInternalError(fmt.Errorf("could not generate a build: %v", err))

 	}

-	return nil

+	return warning

 }

 func generateBuildTriggerInfo(revision *buildapi.SourceRevision, hookType, secret string) (buildTriggerCauses []buildapi.BuildTriggerCause) {

@@ -36,18 +36,19 @@ type plugin struct {

 	Secret, Path string

 	Err          error

 	Env          []kapi.EnvVar

+	Proceed      bool

 }

 func (p *plugin) Extract(buildCfg *api.BuildConfig, secret, path string, req *http.Request) (*api.SourceRevision, []kapi.EnvVar, bool, error) {

 	p.Secret, p.Path = secret, path

-	return nil, p.Env, true, p.Err

+	return nil, p.Env, p.Proceed, p.Err

 }

 func newStorage() (*rest.WebHook, *buildConfigInstantiator, *test.BuildConfigRegistry) {

 	mockRegistry := &test.BuildConfigRegistry{}

 	bci := &buildConfigInstantiator{}

 	hook := NewWebHookREST(mockRegistry, bci, map[string]webhook.Plugin{

-		"ok": &plugin{},

+		"ok": &plugin{Proceed: true},

 		"okenv": &plugin{

 			Env: []kapi.EnvVar{

 				{

@@ -55,6 +56,7 @@ func newStorage() (*rest.WebHook, *buildConfigInstantiator, *test.BuildConfigReg

 					Value: "bar",

 				},

 			},

+			Proceed: true,

 		},

 		"errsecret": &plugin{Err: webhook.ErrSecretMismatch},

 		"errhook":   &plugin{Err: webhook.ErrHookNotEnabled},

@@ -239,7 +241,7 @@ func (p *pathPlugin) Extract(buildCfg *api.BuildConfig, secret, path string, req

 type errPlugin struct{}

 func (*errPlugin) Extract(buildCfg *api.BuildConfig, secret, path string, req *http.Request) (*api.SourceRevision, []kapi.EnvVar, bool, error) {

-	return nil, []kapi.EnvVar{}, true, errors.New("Plugin error!")

+	return nil, []kapi.EnvVar{}, false, errors.New("Plugin error!")

 }

 var testBuildConfig = &api.BuildConfig{

@@ -42,68 +42,74 @@ func (p *WebHookPlugin) Extract(buildCfg *api.BuildConfig, secret, path string,

 		return revision, envvars, false, err

 	}

-	if buildCfg.Spec.Source.Git == nil {

-		glog.V(4).Infof("No git source defined for BuildConfig %s/%s, but triggering anyway", buildCfg.Namespace, buildCfg.Name)

-		return revision, envvars, true, err

-	}

-

 	contentType := req.Header.Get("Content-Type")

 	if len(contentType) != 0 {

 		contentType, _, err = mime.ParseMediaType(contentType)

 		if err != nil {

-			return nil, envvars, false, fmt.Errorf("non-parseable Content-Type %s (%s)", contentType, err)

+			return revision, envvars, false, fmt.Errorf("error parsing Content-Type: %s", err)

 		}

 	}

-	if req.Body != nil && (contentType == "application/json" || contentType == "application/yaml") {

-		body, err := ioutil.ReadAll(req.Body)

-		if err != nil {

-			return nil, envvars, false, err

-		}

+	if req.Body == nil {

+		return revision, envvars, true, nil

+	}

-		if len(body) == 0 {

-			return nil, envvars, true, nil

-		}

+	if contentType != "application/json" && contentType != "application/yaml" {

+		warning := webhook.NewWarning("invalid Content-Type on payload, ignoring payload and continuing with build")

+		return revision, envvars, true, warning

+	}

-		var data api.GenericWebHookEvent

-		if contentType == "application/yaml" {

-			body, err = yaml.ToJSON(body)

-			if err != nil {

-				glog.V(4).Infof("Error converting payload to json %v, but continuing with build", err)

-				return nil, envvars, true, nil

-			}

-		}

-		if err = json.Unmarshal(body, &data); err != nil {

-			glog.V(4).Infof("Error unmarshalling payload %v, but continuing with build", err)

-			return nil, envvars, true, nil

-		}

-		if len(data.Env) > 0 && trigger.AllowEnv {

-			envvars = data.Env

-		}

-		if data.Git == nil {

-			glog.V(4).Infof("No git information for the generic webhook found in %s/%s", buildCfg.Namespace, buildCfg.Name)

-			return nil, envvars, true, nil

+	body, err := ioutil.ReadAll(req.Body)

+	if err != nil {

+		return revision, envvars, false, err

+	}

+

+	if len(body) == 0 {

+		return revision, envvars, true, nil

+	}

+

+	var data api.GenericWebHookEvent

+	if contentType == "application/yaml" {

+		body, err = yaml.ToJSON(body)

+		if err != nil {

+			warning := webhook.NewWarning(fmt.Sprintf("error converting payload to json: %v, ignoring payload and continuing with build", err))

+			return revision, envvars, true, warning

 		}

+	}

+	if err = json.Unmarshal(body, &data); err != nil {

+		warning := webhook.NewWarning(fmt.Sprintf("error unmarshalling payload: %v, ignoring payload and continuing with build", err))

+		return revision, envvars, true, warning

+	}

+	if len(data.Env) > 0 && trigger.AllowEnv {

+		envvars = data.Env

+	}

+	if buildCfg.Spec.Source.Git == nil {

+		// everything below here is specific to git-based builds

+		return revision, envvars, true, nil

+	}

+	if data.Git == nil {

+		warning := webhook.NewWarning("no git information found in payload, ignoring and continuing with build")

+		return revision, envvars, true, warning

+	}

-		if data.Git.Refs != nil {

-			for _, ref := range data.Git.Refs {

-				if webhook.GitRefMatches(ref.Ref, webhook.DefaultConfigRef, &buildCfg.Spec.Source) {

-					revision = &api.SourceRevision{

-						Git: &ref.GitSourceRevision,

-					}

-					return revision, envvars, true, nil

+	if data.Git.Refs != nil {

+		for _, ref := range data.Git.Refs {

+			if webhook.GitRefMatches(ref.Ref, webhook.DefaultConfigRef, &buildCfg.Spec.Source) {

+				revision = &api.SourceRevision{

+					Git: &ref.GitSourceRevision,

 				}

+				return revision, envvars, true, nil

 			}

-			glog.V(2).Infof("Skipping build for BuildConfig %s/%s. None of the supplied refs matched %q", buildCfg.Namespace, buildCfg, buildCfg.Spec.Source.Git.Ref)

-			return nil, envvars, false, nil

-		}

-		if !webhook.GitRefMatches(data.Git.Ref, webhook.DefaultConfigRef, &buildCfg.Spec.Source) {

-			glog.V(2).Infof("Skipping build for BuildConfig %s/%s. Branch reference from %q does not match configuration", buildCfg.Namespace, buildCfg.Name, data.Git.Ref)

-			return nil, envvars, false, nil

-		}

-		revision = &api.SourceRevision{

-			Git: &data.Git.GitSourceRevision,

 		}

+		warning := webhook.NewWarning(fmt.Sprintf("skipping build. None of the supplied refs matched %q", buildCfg.Spec.Source.Git.Ref))

+		return revision, envvars, false, warning

+	}

+	if !webhook.GitRefMatches(data.Git.Ref, webhook.DefaultConfigRef, &buildCfg.Spec.Source) {

+		warning := webhook.NewWarning(fmt.Sprintf("skipping build. Branch reference from %q does not match configuration", data.Git.Ref))

+		return revision, envvars, false, warning

+	}

+	revision = &api.SourceRevision{

+		Git: &data.Git.GitSourceRevision,

 	}

 	return revision, envvars, true, nil

 }

@@ -11,6 +11,8 @@ import (

 	"github.com/openshift/origin/pkg/build/api"

 	"github.com/openshift/origin/pkg/build/webhook"

 	kapi "k8s.io/kubernetes/pkg/api"

+	"k8s.io/kubernetes/pkg/api/errors"

+	"k8s.io/kubernetes/pkg/api/unversioned"

 )

 var mockBuildStrategy = api.BuildStrategy{

@@ -52,6 +54,24 @@ func GivenRequestWithRefsPayload(t *testing.T) *http.Request {

 	return req

 }

+func matchWarning(t *testing.T, err error, message string) {

+	status, ok := err.(*errors.StatusError)

+	if !ok {

+		t.Errorf("Expected %v to be a StatusError object", err)

+		return

+	}

+

+	if status.ErrStatus.Status != unversioned.StatusSuccess {

+		t.Errorf("Unexpected response status %v, expected %v", status.ErrStatus.Status, unversioned.StatusSuccess)

+	}

+	if status.ErrStatus.Code != http.StatusOK {

+		t.Errorf("Unexpected response code %v, expected %v", status.ErrStatus.Code, http.StatusOK)

+	}

+	if status.ErrStatus.Message != message {

+		t.Errorf("Unexpected response message %v, expected %v", status.ErrStatus.Message, message)

+	}

+}

+

 func TestVerifyRequestForMethod(t *testing.T) {

 	req := GivenRequest("GET")

 	buildConfig := &api.BuildConfig{

@@ -327,9 +347,7 @@ func TestExtractWithUnmatchedRefGitPayload(t *testing.T) {

 	plugin := New()

 	build, _, proceed, err := plugin.Extract(buildConfig, "secret100", "", req)

-	if err != nil {

-		t.Errorf("Unexpected error when triggering build: %v", err)

-	}

+	matchWarning(t, err, `skipping build. Branch reference from "refs/heads/master" does not match configuration`)

 	if proceed {

 		t.Error("Expected 'proceed' return value to be 'false' for unmatched refs")

 	}

@@ -471,9 +489,7 @@ func TestExtractWithUnmatchedGitRefsPayload(t *testing.T) {

 	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: %v", err)

-	}

+	matchWarning(t, err, `skipping build. None of the supplied refs matched "other"`)

 	if proceed {

 		t.Error("Expected 'proceed' return value to be 'false'")

 	}

@@ -627,9 +643,7 @@ func TestGitlabPush(t *testing.T) {

 	plugin := New()

 	_, _, proceed, err := plugin.Extract(buildConfig, "secret100", "", req)

-	if err != nil {

-		t.Errorf("Expected to be able to trigger a build without a payload error: %v", err)

-	}

+	matchWarning(t, err, "no git information found in payload, ignoring and continuing with build")

 	if !proceed {

 		t.Error("Expected 'proceed' return value to be 'true'")

 	}

@@ -699,9 +713,75 @@ func TestExtractWithUnmarshalError(t *testing.T) {

 	}

 	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: %v", err)

+	matchWarning(t, err, `error unmarshalling payload: invalid character '\x00' looking for beginning of value, ignoring payload and continuing with build`)

+	if !proceed {

+		t.Error("Expected 'proceed' return value to be 'true'")

+	}

+	if revision != nil {

+		t.Error("Expected the 'revision' return value to be nil")

+	}

+}

+

+func TestExtractWithUnmarshalErrorYAML(t *testing.T) {

+	req, _ := http.NewRequest("POST", "http://someurl.com", errJSON{})

+	req.Header.Add("Content-Type", "application/yaml")

+	buildConfig := &api.BuildConfig{

+		Spec: api.BuildConfigSpec{

+			Triggers: []api.BuildTriggerPolicy{

+				{

+					Type: api.GenericWebHookBuildTriggerType,

+					GenericWebHook: &api.WebHookTrigger{

+						Secret: "secret100",

+					},

+				},

+			},

+			CommonSpec: api.CommonSpec{

+				Source: api.BuildSource{

+					Git: &api.GitBuildSource{

+						Ref: "other",

+					},

+				},

+				Strategy: mockBuildStrategy,

+			},

+		},

+	}

+	plugin := New()

+	revision, _, proceed, err := plugin.Extract(buildConfig, "secret100", "", req)

+	matchWarning(t, err, "error converting payload to json: yaml: control characters are not allowed, ignoring payload and continuing with build")

+	if !proceed {

+		t.Error("Expected 'proceed' return value to be 'true'")

+	}

+	if revision != nil {

+		t.Error("Expected the 'revision' return value to be nil")

+	}

+}

+

+func TestExtractWithBadContentType(t *testing.T) {

+	req, _ := http.NewRequest("POST", "http://someurl.com", errJSON{})

+	req.Header.Add("Content-Type", "bad")

+	buildConfig := &api.BuildConfig{

+		Spec: api.BuildConfigSpec{

+			Triggers: []api.BuildTriggerPolicy{

+				{

+					Type: api.GenericWebHookBuildTriggerType,

+					GenericWebHook: &api.WebHookTrigger{

+						Secret: "secret100",

+					},

+				},

+			},

+			CommonSpec: api.CommonSpec{

+				Source: api.BuildSource{

+					Git: &api.GitBuildSource{

+						Ref: "other",

+					},

+				},

+				Strategy: mockBuildStrategy,

+			},

+		},

 	}

+	plugin := New()

+	revision, _, proceed, err := plugin.Extract(buildConfig, "secret100", "", req)

+	matchWarning(t, err, "invalid Content-Type on payload, ignoring payload and continuing with build")

 	if !proceed {

 		t.Error("Expected 'proceed' return value to be 'true'")

 	}

@@ -709,3 +789,39 @@ func TestExtractWithUnmarshalError(t *testing.T) {

 		t.Error("Expected the 'revision' return value to be nil")

 	}

 }

+

+func TestExtractWithUnparseableContentType(t *testing.T) {

+	req, _ := http.NewRequest("POST", "http://someurl.com", errJSON{})

+	req.Header.Add("Content-Type", "bad//bad")

+	buildConfig := &api.BuildConfig{

+		Spec: api.BuildConfigSpec{

+			Triggers: []api.BuildTriggerPolicy{

+				{

+					Type: api.GenericWebHookBuildTriggerType,

+					GenericWebHook: &api.WebHookTrigger{

+						Secret: "secret100",

+					},

+				},

+			},

+			CommonSpec: api.CommonSpec{

+				Source: api.BuildSource{

+					Git: &api.GitBuildSource{

+						Ref: "other",

+					},

+				},

+				Strategy: mockBuildStrategy,

+			},

+		},

+	}

+	plugin := New()

+	revision, _, proceed, err := plugin.Extract(buildConfig, "secret100", "", req)

+	if err == nil || err.Error() != "error parsing Content-Type: mime: expected token after slash" {

+		t.Errorf("Unexpected error %v", err)

+	}

+	if proceed {

+		t.Error("Expected 'proceed' return value to be 'false'")

+	}

+	if revision != nil {

+		t.Error("Expected the 'revision' return value to be nil")

+	}

+}

@@ -7,6 +7,8 @@ import (

 	"strings"

 	kapi "k8s.io/kubernetes/pkg/api"

+	kerrors "k8s.io/kubernetes/pkg/api/errors"

+	"k8s.io/kubernetes/pkg/api/unversioned"

 	buildapi "github.com/openshift/origin/pkg/build/api"

 )

@@ -76,3 +78,12 @@ func ValidateWebHookSecret(webHookTriggers []buildapi.BuildTriggerPolicy, secret

 	}

 	return nil, ErrSecretMismatch

 }

+

+// NewWarning returns an StatusError object with a http.StatusOK (200) code.

+func NewWarning(message string) *kerrors.StatusError {

+	return &kerrors.StatusError{ErrStatus: unversioned.Status{

+		Status:  unversioned.StatusSuccess,

+		Code:    http.StatusOK,

+		Message: message,

+	}}

+}