Skip to content

Commit

Permalink
Replace go-playground/validator with jellidator
Browse files Browse the repository at this point in the history
Also, remove all the go-playground validator dependencies and wiring

Co-authored-by: Danail Branekov <[email protected]>
  • Loading branch information
Kieron Browne and danail-branekov committed Jun 27, 2023
1 parent ffcd6af commit fae8e5d
Show file tree
Hide file tree
Showing 32 changed files with 441 additions and 432 deletions.
9 changes: 3 additions & 6 deletions api/handlers/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ var _ = Describe("Build", func() {
req, err = http.NewRequestWithContext(ctx, "GET", "/v3/builds/"+buildGUID, nil)
Expect(err).NotTo(HaveOccurred())

decoderValidator, err := NewDefaultDecoderValidator()
Expect(err).NotTo(HaveOccurred())
decoderValidator := NewDefaultDecoderValidator()

apiHandler := NewBuild(
*serverURL,
Expand Down Expand Up @@ -315,18 +314,16 @@ var _ = Describe("Build", func() {

Describe("the PATCH /v3/builds endpoint", func() {
BeforeEach(func() {
decoderValidator, err := NewDefaultDecoderValidator()
Expect(err).NotTo(HaveOccurred())

apiHandler := NewBuild(
*serverURL,
new(fake.CFBuildRepository),
new(fake.CFPackageRepository),
new(fake.CFAppRepository),
decoderValidator,
NewDefaultDecoderValidator(),
)
routerBuilder.LoadRoutes(apiHandler)

var err error
req, err = http.NewRequestWithContext(context.Background(), "PATCH", "/v3/builds/build-guid", strings.NewReader(`{}`))
Expect(err).NotTo(HaveOccurred())
})
Expand Down
3 changes: 1 addition & 2 deletions api/handlers/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ var _ = Describe("Deployment", func() {
runnerInfoRepo = new(fake.RunnerInfoRepository)
runnerName = "statefulset-runner"

decoderValidator, err := handlers.NewDefaultDecoderValidator()
Expect(err).NotTo(HaveOccurred())
decoderValidator := handlers.NewDefaultDecoderValidator()

apiHandler := handlers.NewDeployment(*serverURL, decoderValidator, deploymentsRepo, runnerInfoRepo, runnerName)
routerBuilder.LoadRoutes(apiHandler)
Expand Down
4 changes: 2 additions & 2 deletions api/handlers/service_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ var _ = Describe("ServiceInstance", func() {
Name: "service-instance-name",
Type: "user-provided",
Tags: []string{"foo", "bar"},
Relationships: payloads.ServiceInstanceRelationships{
Space: payloads.Relationship{
Relationships: &payloads.ServiceInstanceRelationships{
Space: &payloads.Relationship{
Data: &payloads.RelationshipData{
GUID: "space-guid",
},
Expand Down
4 changes: 2 additions & 2 deletions api/handlers/space.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ type SpaceRepository interface {
type Space struct {
spaceRepo SpaceRepository
apiBaseURL url.URL
decoderValidator *DecoderValidator
decoderValidator DecoderValidator
}

func NewSpace(apiBaseURL url.URL, spaceRepo SpaceRepository, decoderValidator *DecoderValidator) *Space {
func NewSpace(apiBaseURL url.URL, spaceRepo SpaceRepository, decoderValidator DecoderValidator) *Space {
return &Space{
apiBaseURL: apiBaseURL,
spaceRepo: spaceRepo,
Expand Down
4 changes: 2 additions & 2 deletions api/handlers/space_manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type SpaceManifest struct {
serverURL url.URL
manifestApplier ManifestApplier
spaceRepo CFSpaceRepository
decoderValidator *DecoderValidator
decoderValidator DecoderValidator
}

//counterfeiter:generate -o fake -fake-name ManifestApplier . ManifestApplier
Expand All @@ -45,7 +45,7 @@ func NewSpaceManifest(
serverURL url.URL,
manifestApplier ManifestApplier,
spaceRepo CFSpaceRepository,
decoderValidator *DecoderValidator,
decoderValidator DecoderValidator,
) *SpaceManifest {
return &SpaceManifest{
serverURL: serverURL,
Expand Down
3 changes: 1 addition & 2 deletions api/handlers/space_manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ var _ = Describe("SpaceManifest", func() {
manifestApplier = new(fake.ManifestApplier)
spaceRepo = new(fake.CFSpaceRepository)

decoderValidator, err := NewDefaultDecoderValidator()
Expect(err).NotTo(HaveOccurred())
decoderValidator := NewDefaultDecoderValidator()

apiHandler := NewSpaceManifest(
*serverURL,
Expand Down
13 changes: 6 additions & 7 deletions api/handlers/space_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ var _ = Describe("Space", func() {
OrganizationGUID: "the-org-guid",
}, nil)

decoderValidator, err := handlers.NewDefaultDecoderValidator()
Expect(err).NotTo(HaveOccurred())
decoderValidator := handlers.NewDefaultDecoderValidator()

apiHandler = handlers.NewSpace(
*serverURL,
Expand Down Expand Up @@ -149,7 +148,7 @@ var _ = Describe("Space", func() {
})

It("returns a bad request error", func() {
expectUnprocessableEntityError("Data is a required field")
expectUnprocessableEntityError("organization is required")
})
})
})
Expand Down Expand Up @@ -362,7 +361,7 @@ var _ = Describe("Space", func() {
})

It("returns an unprocessable entity error", func() {
expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`)
expectUnprocessableEntityError("cannot use the cloudfoundry.org domain")
})
})

Expand All @@ -378,7 +377,7 @@ var _ = Describe("Space", func() {
})

It("returns an unprocessable entity error", func() {
expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`)
expectUnprocessableEntityError("cannot use the cloudfoundry.org domain")
})
})
})
Expand All @@ -396,7 +395,7 @@ var _ = Describe("Space", func() {
})

It("returns an unprocessable entity error", func() {
expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`)
expectUnprocessableEntityError("cannot use the cloudfoundry.org domain")
})
})

Expand All @@ -412,7 +411,7 @@ var _ = Describe("Space", func() {
})

It("returns an unprocessable entity error", func() {
expectUnprocessableEntityError(`Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`)
expectUnprocessableEntityError("cannot use the cloudfoundry.org domain")
})
})
})
Expand Down
165 changes: 9 additions & 156 deletions api/handlers/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@ import (

apierrors "code.cloudfoundry.org/korifi/api/errors"

"github.com/go-playground/locales/en"
ut "github.com/go-playground/universal-translator"
"github.com/go-playground/validator/v10"
en_translations "github.com/go-playground/validator/v10/translations/en"
"github.com/jellydator/validation"
"golang.org/x/exp/maps"
"golang.org/x/text/cases"
"golang.org/x/text/language"
"gopkg.in/yaml.v3"
Expand All @@ -35,24 +30,13 @@ type KeyedPayload interface {
DecodeFromURLValues(url.Values) error
}

type DecoderValidator struct {
validator *validator.Validate
translator ut.Translator
}

func NewDefaultDecoderValidator() (*DecoderValidator, error) {
validator, translator, err := wireValidator()
if err != nil {
return nil, err
}
type DecoderValidator struct{}

return &DecoderValidator{
validator: validator,
translator: translator,
}, nil
func NewDefaultDecoderValidator() DecoderValidator {
return DecoderValidator{}
}

func (dv *DecoderValidator) DecodeAndValidateJSONPayload(r *http.Request, object interface{}) error {
func (dv DecoderValidator) DecodeAndValidateJSONPayload(r *http.Request, object interface{}) error {
decoder := json.NewDecoder(r.Body)
defer r.Body.Close()
decoder.DisallowUnknownFields()
Expand All @@ -74,7 +58,7 @@ func (dv *DecoderValidator) DecodeAndValidateJSONPayload(r *http.Request, object
return dv.validatePayload(object)
}

func (dv *DecoderValidator) DecodeAndValidateYAMLPayload(r *http.Request, object interface{}) error {
func (dv DecoderValidator) DecodeAndValidateYAMLPayload(r *http.Request, object interface{}) error {
decoder := yaml.NewDecoder(r.Body)
defer r.Body.Close()
decoder.KnownFields(false) // TODO: change this to true once we've added all manifest fields to payloads.Manifest
Expand All @@ -86,7 +70,7 @@ func (dv *DecoderValidator) DecodeAndValidateYAMLPayload(r *http.Request, object
return dv.validatePayload(object)
}

func (dv *DecoderValidator) DecodeAndValidateURLValues(r *http.Request, object KeyedPayload) error {
func (dv DecoderValidator) DecodeAndValidateURLValues(r *http.Request, object KeyedPayload) error {
if err := r.ParseForm(); err != nil {
return err
}
Expand Down Expand Up @@ -114,34 +98,13 @@ func checkKeysAreSupported(payloadObject KeyedPayload, values url.Values) error
}

func (dv *DecoderValidator) validatePayload(object interface{}) error {
// New validation library for which we have implemented manifest payload validation
t, ok := object.(validation.Validatable)
if ok {
err := t.Validate()
if err != nil {
return apierrors.NewUnprocessableEntityError(err, strings.Join(errorMessages(err), ", "))
}
if !ok {
return nil
}

// Existing validation library for payloads that have not yet implemented validation.Validatable
err := dv.validator.Struct(object)
if err != nil {
errorMessage := err.Error()

var typedErr validator.ValidationErrors
if errors.As(err, &typedErr) {
errorMap := typedErr.Translate(dv.translator)
var errorMessages []string
for _, msg := range errorMap {
errorMessages = append(errorMessages, msg)
}

if len(errorMessages) > 0 {
errorMessage = strings.Join(errorMessages, ",")
}
}
return apierrors.NewUnprocessableEntityError(err, errorMessage)
if err := t.Validate(); err != nil {
return apierrors.NewUnprocessableEntityError(err, strings.Join(errorMessages(err), ", "))
}

return nil
Expand Down Expand Up @@ -184,113 +147,3 @@ func prefixedErrorMessages(field string, err error) []string {

return messages
}

func wireValidator() (*validator.Validate, ut.Translator, error) {
v := validator.New()

trans, err := registerDefaultTranslator(v)
if err != nil {
return nil, nil, err
}
// Register custom validators
err = v.RegisterValidation("serviceinstancetaglength", serviceInstanceTagLength)
if err != nil {
return nil, nil, err
}

err = v.RegisterValidation("metadatavalidator", metadataValidator)
if err != nil {
return nil, nil, err
}
err = v.RegisterTranslation("metadatavalidator", trans, func(ut ut.Translator) error {
return ut.Add("metadatavalidator", `Labels and annotations cannot begin with "cloudfoundry.org" or its subdomains`, false)
}, func(ut ut.Translator, fe validator.FieldError) string {
t, _ := ut.T("metadatavalidator", fe.Field())
return t
})
if err != nil {
return nil, nil, err
}

err = v.RegisterValidation("buildmetadatavalidator", buildMetadataValidator)
if err != nil {
return nil, nil, err
}
err = v.RegisterTranslation("buildmetadatavalidator", trans, func(ut ut.Translator) error {
return ut.Add("buildmetadatavalidator", `Labels and annotations are not supported for builds`, false)
}, func(ut ut.Translator, fe validator.FieldError) string {
t, _ := ut.T("buildmetadatavalidator", fe.Field())
return t
})
if err != nil {
return nil, nil, err
}

return v, trans, nil
}

func registerDefaultTranslator(v *validator.Validate) (ut.Translator, error) {
en := en.New()
uni := ut.New(en, en)
trans, _ := uni.GetTranslator("en")

err := en_translations.RegisterDefaultTranslations(v, trans)
if err != nil {
return nil, err
}

return trans, nil
}

func serviceInstanceTagLength(fl validator.FieldLevel) bool {
tags, ok := fl.Field().Interface().([]string)
if !ok {
return true // the value is optional, and is set to nil
}

tagLen := 0
for _, tag := range tags {
tagLen += len(tag)
}

return tagLen < 2048
}

func metadataValidator(fl validator.FieldLevel) bool {
metadata, isMeta := fl.Field().Interface().(map[string]string)
if isMeta {
return validateMetadataKeys(maps.Keys(metadata))
}

metadataPatch, isMetaPatch := fl.Field().Interface().(map[string]*string)
if isMetaPatch {
return validateMetadataKeys(maps.Keys(metadataPatch))
}

return true
}

func buildMetadataValidator(fl validator.FieldLevel) bool {
metadata, isMeta := fl.Field().Interface().(map[string]string)
if isMeta {
if len(metadata) > 0 {
return false
}
}
return true
}

func validateMetadataKeys(metaKeys []string) bool {
for _, key := range metaKeys {
u, err := url.ParseRequestURI("https://" + key) // without the scheme, the hostname will be parsed as a path
if err != nil {
continue
}

if strings.HasSuffix(u.Hostname(), "cloudfoundry.org") {
return false
}
}

return true
}
8 changes: 2 additions & 6 deletions api/handlers/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,15 @@ import (
var _ = Describe("Validator", func() {
Describe("DecodeAndValidateURLValues", func() {
var (
requestValidator *handlers.DecoderValidator
requestValidator handlers.DecoderValidator
decoded DecodeTestPayload
decodeErr error
requestUrl string
)

BeforeEach(func() {
var err error
requestValidator, err = handlers.NewDefaultDecoderValidator()
Expect(err).NotTo(HaveOccurred())

requestValidator = handlers.NewDefaultDecoderValidator()
requestUrl = "http://foo.com?key=3"

decoded = DecodeTestPayload{}
})

Expand Down
5 changes: 1 addition & 4 deletions api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,7 @@ func main() {
)
appLogs := actions.NewAppLogs(appRepo, buildRepo, podRepo)

decoderValidator, err := handlers.NewDefaultDecoderValidator()
if err != nil {
panic(fmt.Sprintf("could not wire validator: %v", err))
}
decoderValidator := handlers.NewDefaultDecoderValidator()

routerBuilder := routing.NewRouterBuilder()
routerBuilder.UseMiddleware(
Expand Down
Loading

0 comments on commit fae8e5d

Please sign in to comment.