Bug 1472226 - Add additional field validations for JSON Schema. #615
Conversation
openshift-ci-robot
added
the
size/L
|
Changes Unknown when pulling 20d0822 on cfchase:param-validation into ** on openshift:master**. |
pkg/broker/util.go
Outdated
| if min, ok := pd.Minimum.(float64); ok { | ||
| prop.Minimum = schema.Number{Val: min, Initialized: true} | ||
| } | ||
|
|
There was a problem hiding this comment.
In the JSON schema validations documentation, exclusiveMinimum and exclusiveMaximum MUST be a number. However go-jsschema and the serialized values it sends uses boolean values for exclusiveMaximum and exclusiveMinimum are boolean and use the numbers in maximum and minimum.
I followed with the docs and used exclusive_maximum/exclusive_minimum as a number for the APB spec, but if there are strong opinions that I should use boolean exclusive_maximum/exclusive_minimum instead, I'd like to hear reasoning. I was divided myself.
There was a problem hiding this comment.
I agree with following the docs. exclusive_maximum representing the max number seems intuitive to me.
|
Changes Unknown when pulling a8c4117 on cfchase:param-validation into ** on openshift:master**. |
pkg/apb/types.go
Outdated
|
|
||
| // number validators | ||
| MultipleOf float64 `json:"multipleOf,omitempty" yaml:"multiple_of,omitempty"` | ||
| Maximum interface{} `json:"maximum,omitempty"` |
There was a problem hiding this comment.
I don't like the loss of type validation if someone is creating a ParameterDescription by hand. If we can make this not exported I would be much less worried about it.
This is another way around this. Using a type declaration for float64 and dereferencing the pointer when using that value. Example:
type nilableNumber float64
type ParameterDescription struct {
.....
Maximum *nilableNumber `json:"maximum,omitempty"`
.....
}
|
@shawn-hurley thanks for the tip. I switched to the nilableNumber in 7450587. Let me know if this addresses your concerns. |
| } | ||
|
|
||
| // max_length overrides maxlength | ||
| if pd.MaxLength > 0 { |
There was a problem hiding this comment.
I am pretty sure that you are going to need to dereference the pointer here.
pkg/apb/types.go
Outdated
| @@ -32,20 +32,34 @@ type Parameters map[string]interface{} | |||
| // SpecManifest - Spec ID to Spec manifest | |||
| type SpecManifest map[string]*Spec | |||
|
|
|||
| type nilableNumber float64 | |||
There was a problem hiding this comment.
I think you should make this exportable so that it can be used when creating a ParameterDescriptor outside of the APB package.
|
Changes Unknown when pulling 7450587 on cfchase:param-validation into ** on openshift:master**. |
|
Changes Unknown when pulling 29f6f46 on cfchase:param-validation into ** on openshift:master**. |
|
Changes Unknown when pulling eb04eb5 on cfchase:param-validation into ** on openshift:master**. |
|
Changes Unknown when pulling eb04eb5 on cfchase:param-validation into ** on openshift:master**. |
rthallisey
added
the
3.9 | release-1.1
…shift#615) * Bug 1472226 - Add additional field validations for JSON Schema. * Bug 1472226 - Use NilableNumber instead of interface{}
Updated apb spec to allow for more validations from JSON Schema.
https://datatracker.ietf.org/doc/draft-handrews-json-schema-validation/
Validations follow JSON schema keywords, and uses snake_case instead of camelCase for consistency when writing APBs
Added apb_field (JSON schema field):
Allow maxlength to be used for backwards compatibility as DeprecatedMaxlength.
Allowed
type: integersince it's used in JSON schema