Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace go-playground/validator with jellidator #2640

Merged
merged 1 commit into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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