Skip to content

Commit 370ce12

Browse files
committed
refactor(webhook): Refine schema validation
1 parent 5c6469a commit 370ce12

File tree

9 files changed

+184
-117
lines changed

9 files changed

+184
-117
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ require (
2222
github.com/onsi/gomega v1.10.3
2323
github.com/pelletier/go-toml v1.8.1 // indirect
2424
github.com/prometheus/client_golang v1.8.0 // indirect
25+
github.com/santhosh-tekuri/jsonschema/v2 v2.2.0
2526
github.com/spf13/cobra v1.1.1
2627
github.com/spf13/jwalterweatherman v1.1.0 // indirect
2728
github.com/spf13/viper v1.7.1
2829
github.com/tommy351/goldga v0.2.0
29-
github.com/xeipuuv/gojsonschema v1.2.0
3030
github.com/zenazn/goji v1.0.1
3131
go.uber.org/atomic v1.7.0 // indirect
3232
go.uber.org/zap v1.16.0

go.sum

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,8 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR
608608
github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
609609
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
610610
github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da/go.mod h1:gi+0XIa01GRL2eRQVjQkKGqKF3SF9vZR/HnPullcV2E=
611+
github.com/santhosh-tekuri/jsonschema/v2 v2.2.0 h1:72xCpK0g27Y1is2lreGNcZhIX3ZCtRpkHvvHrHD+5y4=
612+
github.com/santhosh-tekuri/jsonschema/v2 v2.2.0/go.mod h1:yzJzKUGV4RbWqWIBBP4wSOBqavX5saE02yirLS0OTyg=
611613
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
612614
github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
613615
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
680682
github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
681683
github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
682684
github.com/vektah/gqlparser v1.1.2/go.mod h1:1ycwN7Ij5njmMkPPAOaRFY4rET2Enx7IkVv3vaXspKw=
683-
github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c=
684-
github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU=
685-
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0=
686-
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
687-
github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=
688-
github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=
689685
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
690686
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
691687
github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

internal/httputil/response.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import (
44
"encoding/json"
55
"fmt"
66
"net/http"
7-
8-
"github.com/xeipuuv/gojsonschema"
97
)
108

119
type Response struct {
@@ -54,18 +52,6 @@ func JSON(w http.ResponseWriter, status int, data interface{}) error {
5452
return nil
5553
}
5654

57-
func NewErrorsForJSONSchema(errors []gojsonschema.ResultError) (result []Error) {
58-
for _, err := range errors {
59-
result = append(result, Error{
60-
Type: err.Type(),
61-
Description: err.Description(),
62-
Field: err.Field(),
63-
})
64-
}
65-
66-
return
67-
}
68-
6955
func NewValidationErrors(field string, errors []string) (result []Error) {
7056
for _, err := range errors {
7157
result = append(result, Error{

internal/webhook/hookutil/error.go

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,11 @@ import (
55
"fmt"
66
"strings"
77

8-
"github.com/xeipuuv/gojsonschema"
98
"k8s.io/apimachinery/pkg/types"
109
)
1110

1211
var ErrInvalidAction = errors.New("invalid action")
1312

14-
type JSONSchemaValidationErrors []gojsonschema.ResultError
15-
16-
func (v JSONSchemaValidationErrors) Error() string {
17-
details := make([]string, len(v))
18-
19-
for i, e := range v {
20-
details[i] = "- " + e.Description()
21-
}
22-
23-
return "validation errors:\n" + strings.Join(details, "\n")
24-
}
25-
2613
type ValidationErrors []string
2714

2815
func (v ValidationErrors) Error() string {
@@ -35,18 +22,6 @@ func (v ValidationErrors) Error() string {
3522
return "validation errors:\n" + strings.Join(details, "\n")
3623
}
3724

38-
type JSONSchemaValidateError struct {
39-
err error
40-
}
41-
42-
func (j JSONSchemaValidateError) Error() string {
43-
return fmt.Sprintf("json schema validate failed: %s", j.err.Error())
44-
}
45-
46-
func (j JSONSchemaValidateError) Unwrap() error {
47-
return j.err
48-
}
49-
5025
type TriggerNotFoundError struct {
5126
key types.NamespacedName
5227
err error

internal/webhook/hookutil/handler.go

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,46 @@ package hookutil
33
import (
44
"errors"
55
"net/http"
6+
"strings"
67

78
"github.com/go-logr/logr"
9+
"github.com/santhosh-tekuri/jsonschema/v2"
810
"github.com/tommy351/pullup/internal/httputil"
911
)
1012

13+
func formatJSONSchemaValidationError(err *jsonschema.ValidationError) (output []httputil.Error) {
14+
causes := err.Causes
15+
16+
if len(causes) == 0 {
17+
causes = append(causes, err)
18+
}
19+
20+
for _, cause := range causes {
21+
var field string
22+
23+
if cause.InstancePtr != "#" {
24+
field = strings.TrimPrefix(cause.InstancePtr, "#/")
25+
}
26+
27+
output = append(output, httputil.Error{
28+
Description: cause.Message,
29+
Field: field,
30+
})
31+
}
32+
33+
return
34+
}
35+
1136
func NewHandler(handler httputil.Handler) http.Handler {
1237
return httputil.NewHandler(func(w http.ResponseWriter, r *http.Request) error {
1338
logger := logr.FromContextOrDiscard(r.Context())
1439

1540
if err := handler(w, r); err != nil {
1641
var (
17-
jsve JSONSchemaValidationErrors
1842
ve ValidationErrors
19-
jse JSONSchemaValidateError
2043
tnfe TriggerNotFoundError
44+
jsse *jsonschema.SchemaError
45+
jsve *jsonschema.ValidationError
2146
)
2247

2348
switch {
@@ -29,34 +54,34 @@ func NewHandler(handler httputil.Handler) http.Handler {
2954
},
3055
}
3156

32-
case errors.As(err, &jsve):
57+
case errors.As(err, &ve):
3358
return httputil.Response{
3459
StatusCode: http.StatusBadRequest,
35-
Errors: httputil.NewErrorsForJSONSchema(jsve),
60+
Errors: httputil.NewValidationErrors("", ve),
3661
}
3762

38-
case errors.As(err, &ve):
63+
case errors.As(err, &tnfe):
3964
return httputil.Response{
4065
StatusCode: http.StatusBadRequest,
41-
Errors: httputil.NewValidationErrors("", ve),
66+
Errors: []httputil.Error{
67+
{Description: "Trigger not found"},
68+
},
4269
}
4370

44-
case errors.As(err, &jse):
45-
logger.Error(err, "JSON schema validation error")
71+
case errors.As(err, &jsse):
72+
logger.Error(err, "Invalid JSON schema")
4673

4774
return httputil.Response{
4875
StatusCode: http.StatusBadRequest,
4976
Errors: []httputil.Error{
50-
{Description: "Failed to validate against JSON schema"},
77+
{Description: "Invalid JSON schema"},
5178
},
5279
}
5380

54-
case errors.As(err, &tnfe):
81+
case errors.As(err, &jsve):
5582
return httputil.Response{
5683
StatusCode: http.StatusBadRequest,
57-
Errors: []httputil.Error{
58-
{Description: "Trigger not found"},
59-
},
84+
Errors: formatJSONSchemaValidationError(jsve),
6085
}
6186

6287
default:

internal/webhook/hookutil/handler_test.go

Lines changed: 116 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -61,48 +61,119 @@ var _ = Describe("NewHandler", func() {
6161
})
6262
})
6363

64-
When("error is JSONSchemaValidationErrors", func() {
64+
When("error is jsonschema.ValidateError", func() {
6565
var recorder *httptest.ResponseRecorder
6666

67-
BeforeEach(func() {
68-
recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
69-
_, err := ValidateJSONSchema(
70-
&extv1.JSON{
71-
Raw: testutil.MustMarshalJSON(map[string]interface{}{
72-
"type": "object",
73-
"properties": map[string]interface{}{
74-
"foo": map[string]interface{}{
75-
"type": "string",
76-
},
77-
"bar": map[string]interface{}{
78-
"type": "string",
67+
Context("single error", func() {
68+
BeforeEach(func() {
69+
recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
70+
_, err := ValidateJSONSchema(
71+
&extv1.JSON{
72+
Raw: testutil.MustMarshalJSON(map[string]interface{}{
73+
"type": "object",
74+
"properties": map[string]interface{}{
75+
"foo": map[string]interface{}{
76+
"type": "string",
77+
},
7978
},
80-
},
81-
}),
82-
},
83-
&extv1.JSON{
84-
Raw: testutil.MustMarshalJSON(map[string]interface{}{
85-
"foo": 3,
86-
"bar": 4,
87-
}),
88-
},
89-
)
79+
}),
80+
},
81+
&extv1.JSON{
82+
Raw: testutil.MustMarshalJSON(map[string]interface{}{
83+
"foo": true,
84+
}),
85+
},
86+
)
9087

91-
return err
88+
return err
89+
})
90+
})
91+
92+
It("should respond 400", func() {
93+
Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
94+
})
95+
96+
It("should respond errors", func() {
97+
var res httputil.Response
98+
Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
99+
Expect(res.Errors).To(ConsistOf([]httputil.Error{
100+
{Field: "foo", Description: "expected string, but got boolean"},
101+
}))
92102
})
93103
})
94104

95-
It("should respond 400", func() {
96-
Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
105+
Context("multi errors", func() {
106+
BeforeEach(func() {
107+
recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
108+
_, err := ValidateJSONSchema(
109+
&extv1.JSON{
110+
Raw: testutil.MustMarshalJSON(map[string]interface{}{
111+
"type": "object",
112+
"properties": map[string]interface{}{
113+
"foo": map[string]interface{}{
114+
"type": "string",
115+
},
116+
"bar": map[string]interface{}{
117+
"type": "string",
118+
},
119+
},
120+
}),
121+
},
122+
&extv1.JSON{
123+
Raw: testutil.MustMarshalJSON(map[string]interface{}{
124+
"foo": 3,
125+
"bar": 4,
126+
}),
127+
},
128+
)
129+
130+
return err
131+
})
132+
})
133+
134+
It("should respond 400", func() {
135+
Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
136+
})
137+
138+
It("should respond errors", func() {
139+
var res httputil.Response
140+
Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
141+
Expect(res.Errors).To(ConsistOf([]httputil.Error{
142+
{Field: "foo", Description: "expected string, but got number"},
143+
{Field: "bar", Description: "expected string, but got number"},
144+
}))
145+
})
97146
})
98147

99-
It("should respond errors", func() {
100-
var res httputil.Response
101-
Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
102-
Expect(res.Errors).To(ConsistOf([]httputil.Error{
103-
{Type: "invalid_type", Field: "foo", Description: "Invalid type. Expected: string, given: integer"},
104-
{Type: "invalid_type", Field: "bar", Description: "Invalid type. Expected: string, given: integer"},
105-
}))
148+
Context("root error", func() {
149+
BeforeEach(func() {
150+
recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
151+
_, err := ValidateJSONSchema(
152+
&extv1.JSON{
153+
Raw: testutil.MustMarshalJSON(map[string]interface{}{
154+
"type": "object",
155+
}),
156+
},
157+
&extv1.JSON{
158+
Raw: []byte("null"),
159+
},
160+
)
161+
162+
return err
163+
})
164+
})
165+
166+
It("should respond 400", func() {
167+
Expect(recorder).To(HaveHTTPStatus(http.StatusBadRequest))
168+
})
169+
170+
It("should respond errors", func() {
171+
var res httputil.Response
172+
Expect(json.NewDecoder(recorder.Body).Decode(&res)).To(Succeed())
173+
Expect(res.Errors).To(ConsistOf([]httputil.Error{
174+
{Description: "expected object, but got null"},
175+
}))
176+
})
106177
})
107178
})
108179

@@ -129,12 +200,21 @@ var _ = Describe("NewHandler", func() {
129200
})
130201
})
131202

132-
When("error is JSONSchemaValidateError", func() {
203+
When("error is jsonschema.SchemaError", func() {
133204
var recorder *httptest.ResponseRecorder
134205

135206
BeforeEach(func() {
136207
recorder = testHandler(func(w http.ResponseWriter, r *http.Request) error {
137-
return JSONSchemaValidateError{}
208+
_, err := ValidateJSONSchema(
209+
&extv1.JSON{
210+
Raw: testutil.MustMarshalJSON(map[string]interface{}{
211+
"type": "what",
212+
}),
213+
},
214+
&extv1.JSON{},
215+
)
216+
217+
return err
138218
})
139219
})
140220

@@ -145,7 +225,7 @@ var _ = Describe("NewHandler", func() {
145225
It("should respond errors", func() {
146226
Expect(recorder.Body).To(MatchJSON(testutil.MustMarshalJSON(httputil.Response{
147227
Errors: []httputil.Error{
148-
{Description: "Failed to validate against JSON schema"},
228+
{Description: "Invalid JSON schema"},
149229
},
150230
})))
151231
})

0 commit comments

Comments
 (0)