openapi: ExpandReferences causes malformed "oneOf" result

[cueckoo]

Originally opened by @samalba in cuelang/cue#504

What version of CUE are you using (cue version)?

built from source
git tag v0.2.2

Does this issue reproduce with the latest release?

I was able to reproduce on the latest commit on master: commit id c6ade67

What did you do?

package main

import (
	"fmt"
	"os"

	"cuelang.org/go/cue"
	"cuelang.org/go/encoding/openapi"
)

func main() {
	source := `
	package main

	#Foo: {a: number} | {b: number}
`

	r := new(cue.Runtime)

	inst, err := r.Compile("tmp", source)
	if err != nil {
		fmt.Println("r.Compile", err)
		return
	}

	cfg := openapi.Config{}
	if os.Getenv("EXPAND_REFS") != "" {
		cfg.ExpandReferences = true
	}
	oapi, err := openapi.Gen(inst, &cfg)
	if err != nil {
		fmt.Println("openapi.Gen", err)
		return
	}

	fmt.Println(string(oapi))
}

$ go build -o main
$ ./main | jq
$ EXPAND_REFS=true ./main | jq

What did you expect to see?

Same output in both cases.

What did you see instead?

When ExpandReferences is true, the oneOf does not include the properties, they are separated instead.

[cueckoo]

Original reply by @shykes in cuelang/cue#504 (comment)

After investigating this, it appears that this is a correct implementation of the OpenAPI spec. The confusion is caused by slight incompatibilities between the json-schema spec and the "extended superset of json-schema" which OpenAPI uses. One of the incompatibilities is the oneOf keyword.

From the OpenAPI spec:

The following properties are taken from the JSON Schema definition
but their definitions were adjusted to the OpenAPI Specification.

   [...]
    oneOf - Inline or referenced schema
         MUST be of a Schema Object and not a standard JSON Schema.

Source: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#schema-object

So this is not a bug after all. However it's a shame that OpenAPI is not 100%!c(MISSING)ompatible with json-schema, since there is no Cue json-schema encoder yet...

[cueckoo]

Original reply by @samalba in cuelang/cue#504 (comment)

Closing this as it appears it's not a bug. Will update a workaround if we find one.

[cueckoo]

Original reply by @shykes in cuelang/cue#504 (comment)

Actually I may have spoken too soon. Even though the OpenAPI spec does alter the meaning of OneOf, the current behavior in cue/openapi.Generate seems to be incorrect in both implementations. So this might be a bug after all...

cc @samalba @mpvl

[cueckoo]

Original reply by @mpvl in cuelang/cue#504 (comment)

@shykes : I'm curious why you think that both renderings are incorrect. AFAICT they are both correct.

The main issue is that for historical reasons ExpandReferences is overloaded to do two things: expand references and rearrange fields in structural form. CRDs (in K8s) puts additional restrictions on OpenAPI, namely that they need to be in structural normal form. This seems to be a good idea in general. The converter uses this form when possible, which happened to coincide with ExternalRefs. But these should probably be two separate options.

It is not always possible to generate the structural form, but when it is, it should be equivalent to the more straightforward translation.

[cueckoo]

Original reply by @proppy in cuelang/cue#504 (comment)

So this is not a bug after all. However it's a shame that OpenAPI is not 100%!c(MISSING)ompatible with json-schema

Note that OpenAPI 3.1 will fix those discrepancies and align OpenAPI schema object with the JSON Schema 2019-09 spec:

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md#properties

The OpenAPI Schema Object is a JSON Schema vocabulary which extends JSON Schema Core and Validation vocabularies. As such any keyword available for those vocabularies is by definition available in OpenAPI, and will work the exact same way.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md#schema-object

The Schema Object allows the definition of input and output data types. These types can be objects, but also primitives and arrays. This object is a superset of the JSON Schema Specification Draft 2019-09.

This is all thanks to the hard work of @philsturgeon :) You can read more about it on his blog: https://apisyouwonthate.com/blog/openapi-v31-and-json-schema-2019-09

[cueckoo]

Original reply by @shykes in cuelang/cue#504 (comment)

Johan, first of all I am very happy to see you involved in Cue!

I understand that OpenAPI object schema and json-schema are not exactly the same, and that includes the definition of oneOf... But are you sure that explains this particular output?

In both json-schema and openapi, oneOf means “one of the schemas in this array must match”. So for example this cue input:

{a: string} | {b: string}

should produce this output:

`{“oneOf”: [{“properties”: {“a”: {“type”: “string”}}}, {“properties”: {“b”: {“type”: “string”}}}]}

And indeed that is the output with ExpandReferences:true.

But try the same input with ExpandReferences:false and you’ll see a very different output which doesn’t seem to match the input.

Something like this:

{
 “properties”: {
  “a”: {“type”: “string”},
  “b”: {“type”: “string”}
 },
 “oneOf”: [ {
  “required”: [“a”]
 }, {
  “required”: [“b”]
 }
}

Is this really a correct translation of the cue input above? If so, I don’t understand it.

Thank you for your help!

On Sep 6, 2020, at 9:21 PM, Johan Euphrosine notifications@github.com wrote:


So this is not a bug after all. However it's a shame that OpenAPI is not 100%!c(MISSING)ompatible with json-schema

Note that OpenAPI 3.1 will fix those discrepancies and align OpenAPI schema object with the JSON Schema 2019-09 spec:

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md#properties

The OpenAPI Schema Object is a JSON Schema vocabulary which extends JSON Schema Core and Validation vocabularies. As such any keyword available for those vocabularies is by definition available in OpenAPI, and will work the exact same way.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.1.0.md#schema-object

The Schema Object allows the definition of input and output data types. These types can be objects, but also primitives and arrays. This object is a superset of the JSON Schema Specification Draft 2019-09.

This is all thanks to the hard work of @philsturgeon :) You can read more about it on his blog: https://apisyouwonthate.com/blog/openapi-v31-and-json-schema-2019-09

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

[cueckoo]

Original reply by @proppy in cuelang/cue#504 (comment)

Johan, first of all I am very happy to see you involved in Cue!

So am I !

I understand that OpenAPI object schema and json-schema are not exactly the same, and that includes the definition of oneOf...
But are you sure that explains this particular output?

Sorry for the confusion, I was just trying to point out that "not exactly the same" will away with 3.1.0, as openapi is become a "strict superset" of jsonschema rather than being a weird "superset" and a "subset" at the same time (see OAI/OpenAPI-Specification#1977 for the gory details).

Is this really a correct translation of the cue input above? If so, I don’t understand it.

Yes, this confused me at first too, but according to the spec:
https://tools.ietf.org/html/draft-handrews-json-schema-02#section-9.2.1.3

An instance validates successfully against this keyword if it
validates successfully against exactly one schema defined by this
keyword's value.

those two schemas (a):

{
  "oneOf": [
    {
      "properties": {
        "a": {
          "type": "string"
        }
      },
      "required": [
        "a"
      ]
    },
    {
      "properties": {
        "b": {
          "type": "string"
        }
      },
      "required": [
        "b"
      ]
    }
  ]
}

and (b)

{
  "properties": {
    "a": {
      "type": "string"
    },
    "b": {
      "type": "string"
    }
  },
  "oneOf": [
    {
      "required": [
        "a"
      ]
    },
    {
      "required": [
        "b"
      ]
    }
  ]
}

would be equivalent.

Take the instance:

{
  "a": "foo",
}

For (a), it would validate only the first oneOf clause:

    {
      "properties": {
        "a": {
          "type": "string"
        }
      },
      "required": [
        "a"
      ]
    }

causing (a) to validate.

And for (b), it would validate the properties constraint:

  "properties": {
    "a": {
      "type": "string"
    },
    "b": {
      "type": "string"
    }
  }

as well as only the first oneOf clause:

    {
      "required": [
        "a"
      ]
    }

causing the (b) to also validate.

And now take this other instance:

{
  "a": "foo",
  "b": "bar"
}

For (a), it would validate both oneOf clause:

[
    {
      "properties": {
        "a": {
          "type": "string"
        }
      },
      "required": [
        "a"
      ]
    },
    {
      "properties": {
        "b": {
          "type": "string"
        }
      },
      "required": [
        "b"
      ]
    }
  ]

causing (a) to not validate (per oneOf spec definition).

And for (b) it would validate:

  "properties": {
    "a": {
      "type": "string"
    },
    "b": {
      "type": "string"
    }
  }

But also validate both oneOf clauses:

  [
    {
      "required": [
        "a"
      ]
    },
    {
      "required": [
        "b"
      ]
    }
  ]

causing the (b) to not validate (per oneOf spec definition).

So while this does sounds like an semantic abuse (saying either "a" and "b" are defined and required, and saying "a" and "b" are both defined but either one is required do sounds like different things) in practice the way oneOf and required compose cause them to be equivalent (because "required" only fails if the property is absent, "either one is required" as expressed with oneOf/required actually means that "only one can be present" at a time, even if both are defined).

Hope that helps!