refactor(webhook): Refine schema validation

go.mod

@@ -22,11 +22,11 @@ require (
 	github.com/onsi/gomega v1.10.3
 	github.com/pelletier/go-toml v1.8.1 // indirect
 	github.com/prometheus/client_golang v1.8.0 // indirect
+	github.com/santhosh-tekuri/jsonschema/v2 v2.2.0
 	github.com/spf13/cobra v1.1.1
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect
 	github.com/spf13/viper v1.7.1
 	github.com/tommy351/goldga v0.2.0
-	github.com/xeipuuv/gojsonschema v1.2.0
 	github.com/zenazn/goji v1.0.1
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/zap v1.16.0

go.sum

@@ -608,6 +608,8 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
 github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da/go.mod h1:gi+0XIa01GRL2eRQVjQkKGqKF3SF9vZR/HnPullcV2E=
+github.com/santhosh-tekuri/jsonschema/v2 v2.2.0 h1:72xCpK0g27Y1is2lreGNcZhIX3ZCtRpkHvvHrHD+5y4=
+github.com/santhosh-tekuri/jsonschema/v2 v2.2.0/go.mod h1:yzJzKUGV4RbWqWIBBP4wSOBqavX5saE02yirLS0OTyg=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
 github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
@@ -680,12 +682,6 @@ github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljT
 github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
 github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
 github.com/vektah/gqlparser v1.1.2/go.mod h1:1ycwN7Ij5njmMkPPAOaRFY4rET2Enx7IkVv3vaXspKw=
-github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c=
-github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
-github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0=
-github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
-github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=
-github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

internal/httputil/response.go

@@ -4,8 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-
-	"github.com/xeipuuv/gojsonschema"
 )
 
 type Response struct {
@@ -54,18 +52,6 @@ func JSON(w http.ResponseWriter, status int, data interface{}) error {
 	return nil
 }
 
-func NewErrorsForJSONSchema(errors []gojsonschema.ResultError) (result []Error) {
-	for _, err := range errors {
-		result = append(result, Error{
-			Type:        err.Type(),
-			Description: err.Description(),
-			Field:       err.Field(),
-		})
-	}
-
-	return
-}
-
 func NewValidationErrors(field string, errors []string) (result []Error) {
 	for _, err := range errors {
 		result = append(result, Error{

internal/webhook/hookutil/error.go

@@ -5,24 +5,11 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/xeipuuv/gojsonschema"
 	"k8s.io/apimachinery/pkg/types"
 )
 
 var ErrInvalidAction = errors.New("invalid action")
 
-type JSONSchemaValidationErrors []gojsonschema.ResultError
-
-func (v JSONSchemaValidationErrors) Error() string {
-	details := make([]string, len(v))
-
-	for i, e := range v {
-		details[i] = "- " + e.Description()
-	}
-
-	return "validation errors:\n" + strings.Join(details, "\n")
-}
-
 type ValidationErrors []string
 
 func (v ValidationErrors) Error() string {
@@ -35,18 +22,6 @@ func (v ValidationErrors) Error() string {
 	return "validation errors:\n" + strings.Join(details, "\n")
 }
 
-type JSONSchemaValidateError struct {
-	err error
-}
-
-func (j JSONSchemaValidateError) Error() string {
-	return fmt.Sprintf("json schema validate failed: %s", j.err.Error())
-}
-
-func (j JSONSchemaValidateError) Unwrap() error {
-	return j.err
-}
-
 type TriggerNotFoundError struct {
 	key types.NamespacedName
 	err error

internal/webhook/hookutil/handler.go

@@ -3,21 +3,46 @@ package hookutil
 import (
 	"errors"
 	"net/http"
+	"strings"
 
 	"github.com/go-logr/logr"
+	"github.com/santhosh-tekuri/jsonschema/v2"
 	"github.com/tommy351/pullup/internal/httputil"
 )
 
+func formatJSONSchemaValidationError(err *jsonschema.ValidationError) (output []httputil.Error) {
+	causes := err.Causes
+
+	if len(causes) == 0 {
+		causes = append(causes, err)
+	}
+
+	for _, cause := range causes {
+		var field string
+
+		if cause.InstancePtr != "#" {
+			field = strings.TrimPrefix(cause.InstancePtr, "#/")
+		}
+
+		output = append(output, httputil.Error{
+			Description: cause.Message,
+			Field:       field,
+		})
+	}
+
+	return
+}
+
 func NewHandler(handler httputil.Handler) http.Handler {
 	return httputil.NewHandler(func(w http.ResponseWriter, r *http.Request) error {
 		logger := logr.FromContextOrDiscard(r.Context())
 
 		if err := handler(w, r); err != nil {
 			var (
-				jsve JSONSchemaValidationErrors
 				ve   ValidationErrors
-				jse  JSONSchemaValidateError
 				tnfe TriggerNotFoundError
+				jsse *jsonschema.SchemaError
+				jsve *jsonschema.ValidationError
 			)
 
 			switch {
@@ -29,34 +54,34 @@ func NewHandler(handler httputil.Handler) http.Handler {
 					},
 				}
 
-			case errors.As(err, &jsve):
+			case errors.As(err, &ve):
 				return httputil.Response{
 					StatusCode: http.StatusBadRequest,
-					Errors:     httputil.NewErrorsForJSONSchema(jsve),
+					Errors:     httputil.NewValidationErrors("", ve),
 				}
 
-			case errors.As(err, &ve):
+			case errors.As(err, &tnfe):
 				return httputil.Response{
 					StatusCode: http.StatusBadRequest,
-					Errors:     httputil.NewValidationErrors("", ve),
+					Errors: []httputil.Error{
+						{Description: "Trigger not found"},
+					},
 				}
 
-			case errors.As(err, &jse):
-				logger.Error(err, "JSON schema validation error")
+			case errors.As(err, &jsse):
+				logger.Error(err, "Invalid JSON schema")
 
 				return httputil.Response{
 					StatusCode: http.StatusBadRequest,
 					Errors: []httputil.Error{
-						{Description: "Failed to validate against JSON schema"},
+						{Description: "Invalid JSON schema"},
 					},
 				}
 
-			case errors.As(err, &tnfe):
+			case errors.As(err, &jsve):
 				return httputil.Response{
 					StatusCode: http.StatusBadRequest,
-					Errors: []httputil.Error{
-						{Description: "Trigger not found"},
-					},
+					Errors:     formatJSONSchemaValidationError(jsve),
 				}
 
 			default:

internal/webhook/hookutil/handler_test.go

@@ -61,48 +61,119 @@ var _ = Describe("NewHandler", func() {
 		})
 	})
 
-	When("error is JSONSchemaValidationErrors", func() {
+	When("error is jsonschema.ValidateError", func() {
 		var recorder *httptest.ResponseRecorder
 
-		BeforeEach(func() {
-			recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
-				_, err := ValidateJSONSchema(
-					&extv1.JSON{
-						Raw: testutil.MustMarshalJSON(map[string]interface{}{
-							"type": "object",
-							"properties": map[string]interface{}{
-								"foo": map[string]interface{}{
-									"type": "string",
-								},
-								"bar": map[string]interface{}{
-									"type": "string",
+		Context("single error", func() {
+			BeforeEach(func() {
+				recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
+					_, err := ValidateJSONSchema(
+						&extv1.JSON{
+							Raw: testutil.MustMarshalJSON(map[string]interface{}{
+								"type": "object",
+								"properties": map[string]interface{}{
+									"foo": map[string]interface{}{
+										"type": "string",
+									},
 								},
-							},
-						}),
-					},
-					&extv1.JSON{
-						Raw: testutil.MustMarshalJSON(map[string]interface{}{
-							"foo": 3,
-							"bar": 4,
-						}),
-					},
-				)
+							}),
+						},
+						&extv1.JSON{
+							Raw: testutil.MustMarshalJSON(map[string]interface{}{
+								"foo": true,
+							}),
+						},
+					)
 
-				return err
+					return err
+				})
+			})
+
+			It("should respond 400", func() {
+				Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
+			})
+
+			It("should respond errors", func() {
+				var res httputil.Response
+				Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
+				Expect(res.Errors).To(ConsistOf([]httputil.Error{
+					{Field: "foo", Description: "expected string, but got boolean"},
+				}))
 			})
 		})
 
-		It("should respond 400", func() {
-			Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
+		Context("multi errors", func() {
+			BeforeEach(func() {
+				recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
+					_, err := ValidateJSONSchema(
+						&extv1.JSON{
+							Raw: testutil.MustMarshalJSON(map[string]interface{}{
+								"type": "object",
+								"properties": map[string]interface{}{
+									"foo": map[string]interface{}{
+										"type": "string",
+									},
+									"bar": map[string]interface{}{
+										"type": "string",
+									},
+								},
+							}),
+						},
+						&extv1.JSON{
+							Raw: testutil.MustMarshalJSON(map[string]interface{}{
+								"foo": 3,
+								"bar": 4,
+							}),
+						},
+					)
+
+					return err
+				})
+			})
+
+			It("should respond 400", func() {
+				Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
+			})
+
+			It("should respond errors", func() {
+				var res httputil.Response
+				Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
+				Expect(res.Errors).To(ConsistOf([]httputil.Error{
+					{Field: "foo", Description: "expected string, but got number"},
+					{Field: "bar", Description: "expected string, but got number"},
+				}))
+			})
 		})
 
-		It("should respond errors", func() {
-			var res httputil.Response
-			Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
-			Expect(res.Errors).To(ConsistOf([]httputil.Error{
-				{Type: "invalid_type", Field: "foo", Description: "Invalid type. Expected: string, given: integer"},
-				{Type: "invalid_type", Field: "bar", Description: "Invalid type. Expected: string, given: integer"},
-			}))
+		Context("root error", func() {
+			BeforeEach(func() {
+				recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
+					_, err := ValidateJSONSchema(
+						&extv1.JSON{
+							Raw: testutil.MustMarshalJSON(map[string]interface{}{
+								"type": "object",
+							}),
+						},
+						&extv1.JSON{
+							Raw: []byte("null"),
+						},
+					)
+
+					return err
+				})
+			})
+
+			It("should respond 400", func() {
+				Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
+			})
+
+			It("should respond errors", func() {
+				var res httputil.Response
+				Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
+				Expect(res.Errors).To(ConsistOf([]httputil.Error{
+					{Description: "expected object, but got null"},
+				}))
+			})
 		})
 	})
 
@@ -129,12 +200,21 @@ var _ = Describe("NewHandler", func() {
 		})
 	})
 
-	When("error is JSONSchemaValidateError", func() {
+	When("error is jsonschema.SchemaError", func() {
 		var recorder *httptest.ResponseRecorder
 
 		BeforeEach(func() {
 			recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
-				return JSONSchemaValidateError{}
+				_, err := ValidateJSONSchema(
+					&extv1.JSON{
+						Raw: testutil.MustMarshalJSON(map[string]interface{}{
+							"type": "what",
+						}),
+					},
+					&extv1.JSON{},
+				)
+
+				return err
 			})
 		})
 
@@ -145,7 +225,7 @@ var _ = Describe("NewHandler", func() {
 		It("should respond errors", func() {
 			Expect(recorder.Body).To(MatchJSON(testutil.MustMarshalJSON(httputil.Response{
 				Errors: []httputil.Error{
-					{Description: "Failed to validate against JSON schema"},
+					{Description: "Invalid JSON schema"},
 				},
 			})))
 		})