CRD validation doesn't accept empty values for type "object" fields

[pwittrock]

Create example CRD, but make spec an object

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: crontabs.stable.example.com
spec:
  group: stable.example.com
  version: v1
  scope: Namespaced
  names:
    plural: crontabs
    singular: crontab
    kind: CronTab
    shortNames:
    - ct
  validation:
   # openAPIV3Schema is the schema for validating custom objects.
    openAPIV3Schema:
      properties:
        spec:
          type: object
          properties:
            cronSpec:
              type: string
              pattern: '^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$'
            replicas:
              type: integer
              minimum: 1
              maximum: 10

Create an instance with an empty spec

apiVersion: stable.example.com/v1
kind: CronTab
metadata:
  name: c
spec:

This throws the following error, but shouldn't since spec doesn't have any required fields:

The CronTab "c" is invalid: []: Invalid value: map[string]interface {}{"apiVersion":"stable.example.com/v1", "kind":"CronTab", "metadata":map[string]interface {}{"name":"c", "namespace":"default", "creationTimestamp":"2018-02-16T15:53:27Z", "uid":"86df4c72-1331-11e8-8944-42010a800029", "selfLink":"", "clusterName":""}, "spec":interface {}(nil)}: validation failure list:

[nikhita]

@pwittrock If spec is defined as an object in the schema, for it to be empty, it can have an empty object value but not a null value i.e. the following would work:

apiVersion: stable.example.com/v1
kind: CronTab
metadata:
  name: c
spec: {}

/cc @sttts

[sttts]

I agree with @pwittrock that the error is unexpected from a user point of view. Actually I am surprised that the given yaml translates to the null in JSON. Is this like that by definition of yaml?

[enisoc]

Actually I am surprised that the given yaml translates to the null in JSON. Is this like that by definition of yaml?

Yes, I think it is:

YAML allows the node content to be omitted in many cases. Nodes with empty content are interpreted as if they were plain scalars with an empty value. Such nodes are commonly resolved to a “null” value.

http://yaml.org/spec/1.2/spec.html#id2786563

This seems more like a YAML gotcha than a CRD validation gotcha. Although spec has no required fields, the provided schema says that spec itself is required, so null is invalid. If spec were completely optional, the schema should say it must be either null or an object.

[sttts]

Yes, the OpenAPI spec does not accept null for type: object. So the validator is correct.

One can add null to the spec though:

openAPIV3Schema:
      properties:
        spec:
          anyOf:
          - type: "null"
          - type: object
            properties:
            ...

[nikhita]

This is not ideal but as @enisoc mentioned

This seems more like a YAML gotcha than a CRD validation gotcha.

Closing. Please reopen if you think this is still an issue.
/close

[mbohlool]

the provided schema says that spec itself is required,

where it said spec is required?

the OpenAPI spec does not accept null for type: object.

I don't think spec is said something about this. It look like this is a validation bug. If the field is not in Required list, it is optional and both null or empty object should be accepted.

Analogous to that is string, if a string field is optional, null or empty string both should be accepted as not provided.

[enisoc]

where it said spec is required?

You're right; I was conflating "required" and "can't be null". The schema doesn't say spec is required, but it does say that if spec is present in the object, it can't be null.

If the field is not in Required list, it is optional and both null or empty object should be accepted.

That's how "optional" works in k8s APIs, but unfortunately OpenAPI Schema (via JSON Schema) works differently.

In JSON Schema, required is defined as:

An object instance is valid against this keyword if its property set contains all elements in this keyword's array value.

It is only concerned with whether or not the property set of the object being validated contains the given fields. If you pass spec: null as in this case, the field is present, so whether it's required or not doesn't enter into it.

if a string field is optional, null or empty string both should be accepted

I agree that makes sense intuitively, but it's not how JSON Schema is defined. Notice in the data model, a null value is an instance with type: null, and no other type lists null among the range of possible values:

JSON Schema interperts documents according to a data model.  A JSON
   value interperted according to this data model is called an
   "instance".

   An instance has one of six primitive types, and a range of possible
   values depending on the type:

   null  A JSON "null" production

   boolean  A "true" or "false" value, from the JSON "true" or "false"
      productions

   object  An unordered set of properties mapping a string to an
      instance, from the JSON "object" production

   array  An ordered list of instances, from the JSON "array" production

   number  An arbitrary-precision, base-10 decimal number value, from
      the JSON "number" production

   string  A string of Unicode code points, from the JSON "string"
      production

That means, according to JSON Schema, null should fail to validate against type: string as well.

[mbohlool]

type: null is not supported in openapi schema, maybe that's why there is a new flag nullable in openapi v3.

in any sense, I think we should support this by patching the validation library or having a layer on top of validation to set all optional null object field to empty object before sending it to validation. This is the way our optional works and we should do the same here.

[enisoc]

I agree that matching the meaning of "optional" used by core APIs would be a nice UX, but it would mean we no longer match the OpenAPIV3 and JSON Schema specs, and that's a break that can't be taken lightly.

@lavalamp recently proposed a policy within SIG API Machinery that, if we say we use a standard (e.g. JSON Patch), any failure to match it exactly is a bug. So at best, we would need to stop saying we use OpenAPIV3, which means changing the field name or adding a new field for kubernetesAPISchema which is "mostly like OpenAPIV3 but see this doc for a list of differences".

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[aermakov-zalando]

/remove-lifecycle rotten

[aermakov-zalando]

Are there any updates on this? We have a CRD that (for historical reasons) has one of the elements defined as either null or a list of strings. Right now I don't see any way of writing a schema for it, because the obvious solution of "oneOf: /<type: null>" isn't valid.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[sttts]

This is fixed with nullable support in 1.14. The example

...
spec:

is equivalent to spec: null and nullable: true can allow that in the schema.

[objcoding]

Yes, the OpenAPI spec does not accept null for type: object. So the validator is correct.

One can add null to the spec though:

openAPIV3Schema:
      properties:
        spec:
          anyOf:
          - type: "null"
          - type: object
            properties:
            ...

spec.validation.openAPIV3Schema.properties[spec].properties[sentinel].anyOf[0].type: Unsupported value: "null": supported values: "array", "boolean", "integer", "number", "object", "string"

[sttts]

nullable: true