-
Notifications
You must be signed in to change notification settings - Fork 351
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
Create validator
object to give more validation flexibility
#2484
Conversation
5296ff6
to
8ea1cee
Compare
return rl, err | ||
} | ||
|
||
type RouteGroupValidator struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought about making Validator
interface, something like
type Validator interface {
Validate(item interface{}) error
}
but I think it's to early for this. It was to make all these validate
functions to be used in other places and interface{}
wasn't the best thing to use in this place in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also use an interface function that doesn't pass a value as func argument.
Then you can use proper OO to figure out how to implement the Validate() error
interface for a given type.
8ea1cee
to
5b915c7
Compare
Real change is from L263 to EOF rest are just moved from |
validator
object to give more validation flexiabilityvalidator
object to give more validation flexibility
4a838db
to
c36ad46
Compare
0f98572
to
4c48a48
Compare
Still need to add some extra test cases to parsing filters improvement |
c85fd37
to
35b2d99
Compare
ed092f3
to
20fce69
Compare
Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
…eskip in case of all valid grammer. Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Mustafa Abdelrahman <[email protected]>
…ming Signed-off-by: Mustafa Abdelrahman <[email protected]>
e8ed695
to
ceefece
Compare
Signed-off-by: Mustafa Abdelrahman <[email protected]>
var err []error | ||
for _, r := range item.Spec.Routes { | ||
for _, p := range r.Predicates { | ||
_, e := eskip.ParsePredicates(p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we actually allow a list of predicates and I think dataclient does the same while parsing.
This is an undocumented feature that might be surprising for users.
Lets address this in a separate PR after we check existing cluster resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for the filters.
Signed-off-by: Mustafa Abdelrahman <[email protected]>
👍 |
1 similar comment
👍 |
This will help with #2478 to try improving validation and how to get around some
DataClients
testsAlso this run
eskip.ParseFilters
by default so that's a step forward.Related to #1618