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

Default value for attribute 'type' not specified #1388

Closed
loewenstein opened this issue Oct 24, 2017 · 12 comments
Closed

Default value for attribute 'type' not specified #1388

loewenstein opened this issue Oct 24, 2017 · 12 comments

Comments

@loewenstein
Copy link

loewenstein commented Oct 24, 2017

Apparently there exists defaulting logic for the attribute 'type' of 'definitions' to 'object' (at least in case there are properties specified)

Unfortunately this is not part of the spec (or I couldn't find it) so that there are tools that are not aware of this. E.g. https://hackage.haskell.org/package/swagger2

The Kubernetes OpenAPI spec at https://raw.githubusercontent.com/kubernetes/kubernetes/master/api/openapi-spec/swagger.json does not specify types explicitly, swagger2 cannot parse it because it expects explicit types to be specified. Swagger-cli validates the same spec successfully.

I will open a bug in the related repo as well and will provide a link here as soon as it exists. GetShopTV/swagger2#131

@MikeRalphson
Copy link
Member

MikeRalphson commented Oct 24, 2017

It does not appear to be defined by the JSON Schema standard, but I believe these rules for type inference are valid and used by other tools:

https://github.com/json-schema-faker/json-schema-faker#inferred-types

@Doqnach
Copy link

Doqnach commented Oct 24, 2017

It would be good if this was specified in the spec since this behaviour is already used in the wild.

@handrews
Copy link
Contributor

JSON Schema does not specify this because JSON Schema keywords do not have type implications. They work the other way around. If you have a schema like this:

{
    "required": ["foo"],
    "minItems": 4
}

Then {"foo": 1}, [1, 2, 3, 4] and "this is a string" are all valid, while {"bar": 2} and [1, 2, 3] are not. If the instance is an object, then it is required to have a field named "foo". If the instance is an array, then it must have at least 4 items. If the instance is any other type, then there are no constraints on it.

This may seem counter-intuitive, and this specific example would be strange to encounter, but it makes a lot more sense when you think about "type": ["object", "null"] where you want the object restrictions to apply when it is an object, but not to cause validation to fail on a null.

@MikeRalphson
Copy link
Member

@handrews granted, but as OpenAPI doesn't allow for type taking an array, I'd suggest the most common case is still type being accidentally omitted, rather than a deliberate attempt to allow validation of multiple types.

@loewenstein
Copy link
Author

Does that mean it is likely a mistake of the open api spec of Kubernetes that it omits so many type attributes?

Should the rest of the open api tooling world actually reject specs not having type attributes?

@webron
Copy link
Member

webron commented Oct 24, 2017

type is not required by the OpenAPI spec, and much like JSON Schema, if it's omitted, it means any type can be used. As @handrews kindly explained, any JSON Schema validation attributes that are being used, are logically applied only in the case their respective type is being used. JSON Schema Validation defines which type each validation keyword applies to.

@MikeRalphson
Copy link
Member

Does that mean it is likely a mistake of the open api spec of Kubernetes that it omits so many type attributes?

It means it's possible that the schema objects in the Kubernetes OpenAPI definition validate too loosely. It doesn't make it an invalid definition.

The type inference rules I linked above are used in at least two projects I know of where example data is generated from a schema object, but they should probably not be used to tighten validation. If you are trying to generate types for a statically-typed language from the schema objects, you may have no choice though.

Should the rest of the open api tooling world actually reject specs not having type attributes?

No, not unless the specification is changed to make type mandatory. That would create further gaps between OpenAPI tooling and JSON Schema tooling, when the general hope is that they will continue to converge.

@loewenstein
Copy link
Author

Ok, so the real bug is with swagger2 which apparently requires a type attribute for definition objects.

Still it might be an unfortunate choice not to provide a type in the Kubernetes Spec.

@webron
Copy link
Member

webron commented Oct 24, 2017

It looks like this issue can be closed. As for the Kubernetes definition - it really depends on what their API accepts, and I know they had to work within some of the limitations of 2.0.

@webron webron closed this as completed Oct 24, 2017
@handrews
Copy link
Contributor

handrews commented Oct 24, 2017

@MikeRalphson

but as OpenAPI doesn't allow for type taking an array

I still hold out hope that OpenAPI will support something like

""type" MUST be either a string or a two-element array, and if an array, the second element MUST be "null""

which would have exactly the same implications as the current nullable approach but actually be compatible (I think it would eliminate the last outright conflict?)

@MikeRalphson
Copy link
Member

@handrews But then you'd get people doing

"type:" ["null", "null"]

😃

@handrews
Copy link
Contributor

handrews commented Oct 24, 2017

@MikeRalphson

Oh FFS... ;-)

""type" MUST be either a string or a two-element array, and if an array, the elements MUST be unique and the second element MUST be "null""

or even just

""type" MUST be either a string or a two-element array, and if an array, the elements MUST be unique and one of the elements MUST be "null""

Satisfied? :-P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants