KEP-2896: OpenAPI v3 to Beta

[Jefftree]

• One-line PR description: OpenAPI V3 to beta

• Issue link: OpenAPI v3 #2896

• Other comments: Seeking discussion around OpenAPI V3 to beta path

[Jefftree]

/cc @apelisse @sttts

[sttts]

looking at this:

// UnmarshalJSON implements the json.Unmarshaller interface.
// TODO: Remove support for leading/trailing whitespace
func (q *Quantity) UnmarshalJSON(value []byte) error {
	l := len(value)
	if l == 4 && bytes.Equal(value, []byte("null")) {
		q.d.Dec = nil
		q.i = int64Amount{}
		return nil
	}
	if l >= 2 && value[0] == '"' && value[l-1] == '"' {
		value = value[1 : l-1]
	}

	parsed, err := ParseQuantity(strings.TrimSpace(string(value)))
	if err != nil {
		return err
	}

	// This copy is safe because parsed will not be referred to again.
	*q = parsed
	return nil
}

Do I read this correctly that both a numeral like 1024 is allowed and also "1k"? That makes it another candidate we should add here.

[sttts]

Some more to verify how they are represented today. All of them are polymorphic, and all from CRD types:

• func (s *JSONSchemaPropsOrBool) UnmarshalJSON(data []byte) error
• func (s *JSONSchemaPropsOrStringArray) UnmarshalJSON(data []byte) error
• func (s *JSONSchemaPropsOrArray) UnmarshalJSON(data []byte) error
• func (s *JSON) UnmarshalJSON(data []byte) error(for detault afaik)

[Jefftree]

If I understand correctly, the marshaling/unmarshaling process works properly for the polymorphic types and it's just exposing the information in the OpenAPI? I reworded this to cover more than just int-or-string types.

[Jefftree]

On a second look, it looks like we're generating the types in OpenAPI (currently marked as just "object") for these structs based on the go structs. I don't believe we're able to represent anyOf directly inside a go struct and we would need the help of annotations. I'm not sure if there's a better approach, but I think we may need something like

XPropsOrBool, XPropsOrArray, etc similar to the XIntOrString annotation that we currently have.

[sttts]

There are these

func (_ JSON) OpenAPISchemaType() []string {
	// TODO: return actual types when anyOf is supported
	return nil
}

// OpenAPISchemaFormat is used by the kube-openapi generator when constructing
// the OpenAPI spec of this type.
func (_ JSON) OpenAPISchemaFormat() string { return "" }

We need a more flexible variant that lets us return proper anyOf types.

[ehashman]

/assign @deads2k

[sttts]

To be precise this is about two cases:

1. intOrString is typed incorrectly today (as string? but definitely not polymorphic with anyOf)
2. other types that can benefit from anyOf to get a more complete type and which are just type: "" today

(1) is a blocker for beta. (2) is cosmetics, but good to have.

[Jefftree]

Thanks for the clarification. I'll keep just 1 as part of the beta requirements. It's most likely that 2 will also be done for beta but we can keep that outside the blocking tasks in the KEP.

[sttts]

Lgtm.

Some nits are left.

[sttts]

/lgtm
/assign @deads2k

Feel free to address the nits as follow-up or before if time permits.

[apelisse]

Thanks, LGTM for me too.

[sttts]

Looks like I caused some confusion with my comment about polymorphic type. We have to check all other mentioned polymorphic type.

I just did that for quantity and its OpenAPI v2 type is wrong: type: string. So fixing this is also a blocker.

    "io.k8s.apimachinery.pkg.api.resource.Quantity": {
      "description": "Quantity is a fixed-point representation of a number. It provides convenient marshaling/unmarshaling in JSON and YAML, in addition to String() and AsInt64() accessors.\n\nThe serialization format is:\n\n<quantity>        ::= <signedNumber><suffix>\n  (Note that <suffix> may be empty, from the \"\" case in <decimalSI>.)\n<digit>           ::= 0 | 1 | ... | 9 <digits>          ::= <digit> | <digit><digits> <number>          ::= <digits> | <digits>.<digits> | <digits>. | .<digits> <sign>            ::= \"+\" | \"-\" <signedNumber>    ::= <number> | <sign><number> <suffix>          ::= <binarySI> | <decimalExponent> | <decimalSI> <binarySI>        ::= Ki | Mi | Gi | Ti | Pi | Ei\n  (International System of units; See: http://physics.nist.gov/cuu/Units/binary.html)\n<decimalSI>       ::= m | \"\" | k | M | G | T | P | E\n  (Note that 1024 = 1Ki but 1000 = 1k; I didn't choose the capitalization.)\n<decimalExponent> ::= \"e\" <signedNumber> | \"E\" <signedNumber>\n\nNo matter which of the three exponent forms is used, no quantity may represent a number greater than 2^63-1 in magnitude, nor may it have more than 3 decimal places. Numbers larger or more precise will be capped or rounded up. (E.g.: 0.1m will rounded up to 1m.) This may be extended in the future if we require larger or smaller quantities.\n\nWhen a Quantity is parsed from a string, it will remember the type of suffix it had, and will use the same type again when it is serialized.\n\nBefore serializing, Quantity will be put in \"canonical form\". This means that Exponent/suffix will be adjusted up or down (with a corresponding increase or decrease in Mantissa) such that:\n  a. No precision is lost\n  b. No fractional digits will be emitted\n  c. The exponent (or suffix) is as large as possible.\nThe sign will be omitted unless the number is negative.\n\nExamples:\n  1.5 will be serialized as \"1500m\"\n  1.5Gi will be serialized as \"1536Mi\"\n\nNote that the quantity will NEVER be internally represented by a floating point number. That is the whole point of this exercise.\n\nNon-canonical values will still parse as long as they are well formed, but will be re-emitted in their canonical form. (So always use canonical form, or don't diff.)\n\nThis format is intended to make it difficult to use these numbers without writing some sort of special handling code in the hopes that that will cause implementors to also use a fixed point implementation.",
      "type": "string"
    },

[apelisse]

@sttts, are you asking that we somehow identify all of the things that are built like this, or do we know that Quantity and IntOrString is an exhaustive list, or do you have an exhaustive list?

[deads2k]

PRR lgtm. The design looks ok to me to too, but I'd like to leave lgtm to
/assign @sttts @apelisse

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, Jefftree

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• keps/prod-readiness/OWNERS [deads2k]
• keps/sig-api-machinery/OWNERS [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[apelisse]

mostly minor nits, thanks.

[apelisse]

How easy is it to build these keys if I know what I'm looking for? Should we make the keys as easy as possible to compute (especially given the gvk of an object)?

[Jefftree]

All k8s apis/crds follow the pattern of apis/<group>/<version> which already seems pretty intuitive?

[apelisse]

/lgtm

[sttts]

are you asking that we somehow identify all of the things that are built like this, or do we know that Quantity and IntOrString is an exhaustive list, or do you have an exhaustive list?

@apelisse I just looked through all UnmarshalJSON methods in the codebase. So I think my list is exhaustive now.

[apelisse]

Thanks for going through that Stefan!