schemars icon indicating copy to clipboard operation
schemars copied to clipboard

Flattening maps into structs is problematic with Kubernetes

Open jcaesar opened this issue 3 years ago • 5 comments
trafficstars

Preface

My original problem was that the schema derived for the following

struct ValueMap {
    foo: String,
    #[serde(flatten)]
    bar: HashMap<String, serde_json::Value>,
}

contains both properties: {…} and additionalProperties: true, which Kubernetes disallows. At this point, I think I have found an acceptable solution for this problem. (But if there's some case where using a Visitor to rewrite additionalProperties to x-kubernetes-preserve-unknown-fields would cause problems, or if schemas with both properties and additionalProperties are generally meaningless and this deserves some broader fix, that would be nice to know.)

The actual problem

The following three structs

struct StringMap {
    foo: String,
    #[serde(flatten)]
    bar: HashMap<String, String>,
}

struct Value {
    foo: String,
    #[serde(flatten)]
    bar: serde_json::Value,
}

struct No {
    foo: String,
}

generate the exact same schema. I'm not sure about the general case, but for Kubernetes, this is problematic: If x-kubernetes-preserve-unknown-fields is not declared, objects containing other properties than the ones declared in properties will either be rejected by validation, or the non-declared properties will be deleted on forced submission. This cannot be fixed by a Visitor, since the information whether a flattened field was present is not available to the visitor. Do you have any suggestion on how this could be handled or fixed?

Example code

jcaesar avatar Mar 06 '22 07:03 jcaesar

FWIW if you do this:

#[derive(JsonSchema)]
#[schemars(deny_unknown_fields)]
struct StringMap {
    foo: String,
    #[serde(flatten)]
    bar: std::collections::HashMap<String, String>,
}

#[derive(JsonSchema)]
#[schemars(deny_unknown_fields)]
struct Value {
    foo: String,
    #[serde(flatten)]
    bar: std::collections::HashMap<String, serde_json::Value>,
}

#[derive(JsonSchema)]
#[schemars(deny_unknown_fields)]
struct No {
    foo: String,
}

You get different schemas:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "StringMap",
  "type": "object",
  "required": [
    "foo"
  ],
  "properties": {
    "foo": {
      "type": "string"
    }
  },
  "additionalProperties": {
    "type": "string"
  }
}
{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "Value",
  "type": "object",
  "required": [
    "foo"
  ],
  "properties": {
    "foo": {
      "type": "string"
    }
  },
  "additionalProperties": true
}
{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "No",
  "type": "object",
  "required": [
    "foo"
  ],
  "properties": {
    "foo": {
      "type": "string"
    }
  },
  "additionalProperties": false
}

Does that suffice to let your visitor identify the cases where it would add the extension?

ahl avatar May 20 '22 22:05 ahl

Hmm. Not sure what to make of this.

I guess kube-rs would have to rewrite additionalProperties: { type: string } as well (not sure to what).

It is a bit weird that it generates additionalProperties: false for #[serde(flatten)] serde_json::Value (but imho, using serde_json::Value is a bit weird anyway. That'll fail serialization if the value is not a Map). additionalProperties: false is also forbidden in K8s CRDs, but I guess kube-rs could just delete it.

jcaesar avatar May 26 '22 08:05 jcaesar

It is a bit weird that it generates additionalProperties: false for #[serde(flatten)] serde_json::Value (but imho, using serde_json::Value is a bit weird anyway. That'll fail serialization if the value is not a Map).

additionalProperties: true? Agreed that it's weird and kind of nonsensical...

additionalProperties: false is also forbidden in K8s CRDs, but I guess kube-rs could just delete it.

That's surprising:

If "additionalProperties" is absent, it may be considered present with an empty schema as a value. https://datatracker.ietf.org/doc/html/draft-wright-json-schema-validation-00#section-5.18

The empty schema is always valid so it's equivalent to having a default of true.

ahl avatar May 26 '22 16:05 ahl