Add Support for OpenAPIEnum in OpenAPI v2

[jiahuif]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Prune enum types from schema if OpenAPIEnum feature is not enabled. Additionally, this PR provides a pattern to revert schema changes from alpha features.

Special notes for your reviewer:

• Change to vendor/ is actually from Enum type generator kube-openapi#242 as a result of dependency upgrade
• In this PR, enum markers are added to known enum types of built-in v1 API to better demonstrate how this PR works.
• Bump k8s.io/kube-openapi will also bump go.mod files of all staging packages. That's why this PR touches so many SIGs.
• Runtime cost: effectively, the runtime cost of GetOpenAPIDefinitions is roughly doubled, because it is constructed then overwritten while being traversed.
• This PR affects specs of ONLY built-in types. CRDs and extensions are out of scope of this single PR.
• The descriptions in OpenAPI spec (and thus in swagger.json) are generated from comments in Go source files. This PR does not author or change any comments of such types.

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/issues/2887

[jiahuif]

/sig api-machinery
/area apiserver

[caesarxuchao]

/triage accepted

[cheftako]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse, cheftako, jiahuif, jpbetz, thockin

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:

• OWNERS [thockin]
• api/OWNERS [thockin]
• staging/src/k8s.io/api/OWNERS [thockin]
• staging/src/k8s.io/legacy-cloud-providers/OWNERS [cheftako]

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

[thockin]

/milestone v1.23

[jiahuif]

/retest

[yue9944882]

this PR is breaking the openapi spec for the type corev1.PodReadinessGate. the type has a casttype marker which points the declared openapi type to corev1.PodConditionType, as a result, in the 1.23 openapi spec, the field conditionType was forced to an enum of which the candidate values are completely irrelevant to readiness gates:

 "io.k8s.api.core.v1.PodReadinessGate": {
      "description": "PodReadinessGate contains the reference to a pod condition",
      "properties": {
        "conditionType": {
          "description": "ConditionType refers to a condition in the pod's condition list with matching type.\n\nPossible enum values:\n - `\"ContainersReady\"` indicates whether all containers in the pod are ready.\n - `\"Initialized\"` means that all init containers in the pod have started successfully.\n - `\"PodScheduled\"` represents status of the scheduling process for this pod.\n - `\"Ready\"` means the pod is able to service requests and should be added to the load balancing pools of all matching services.",
          "enum": [
            "ContainersReady",
            "Initialized",
            "PodScheduled",
            "Ready"
          ],
          "type": "string"
        }
      },
      "required": [
        "conditionType"
      ],
      "type": "object"
    },

oddly, the latest openapi spec on the master branch doesnt have this issue tho..

[jiahuif]

@yue9944882 Thanks for checking. This is the fix #108639
Here is the backport to 1.23 that fix this issue #108740

[brendandburns]

@yue9944882 since this is committed into the release branch for 1.23, I think we can regenerate to fix this, right?

[liggitt]

As an aside, it seems like there's a mismatch in the generation/use of the committed openapi spec... it's being generated with all alpha features enabled, including alpha features impacting the content of the openapi spec (like this enum feature). That is then being used to generate decidedly non-alpha clients.

The fact that an alpha, off-by-default feature in kubernetes/kubernetes broke generated clients is pretty concerning.

I'm wondering if we should be generating the committed openapi only enabling alpha APIs, but not all alpha feature gates.

[apelisse]

Should we generate both openapi-spec, one with alpha enabled and one without? First it'd give us an opportunity to compare the two, make sure that alpha features generate the right thing, but also provide a more stable openapi for offline clients generation?

[liggitt]

maybe?

[yue9944882]

@yue9944882 since this is committed into the release branch for 1.23, I think we can regenerate to fix this, right?

yes, after the fix is picked we also need to wait until the next patch release. i guess 1-2 weeks is expected.

i presume it's fine to preserve the alpha objects/fields in the openapi (as long as it doesnt break the backward-compatibility) b/c users may wanna opt-in to the alpha feature. but the enum change involved by this PR is a bit different. the crux is that there's no native enum type in golang, the so-called "enum" in golang doesnt have a hard requirement on the candidate values. e.g. for a type MyEnum string we can cast any string to the enum by MyEnum("..."). while in other languages, the enum type strictly forbids non-candidate values.

[liggitt]

the enum change involved by this PR is a bit different. the crux is that there's no native enum type in golang, the so-called "enum" in golang doesnt have a hard requirement on the candidate values. e.g. for a type MyEnum string we can cast any string to the enum by MyEnum("..."). while in other languages, the enum type strictly forbids non-candidate values

Agreed. The more I think about this, the more concerning it is.

First, some of the currently marked +enum fields are documented to allow expansion in the future, which would break clients generated against earlier openapi schemas:

• // CompletionMode specifies how Pod completions of a Job are tracked.
  // +enum
  type CompletionMode string
  
    // More completion modes can be added in the future.
  

• These are only well-known types, other types are allowed:

  // RequestConditionType is the type of a CertificateSigningRequestCondition
  // +enum
  type RequestConditionType string
  

• This makes explicit reference to a future value we want to add:

  // +enum
  type TaintEffect string
    // NOT YET IMPLEMENTED. TODO: Uncomment field once it is implemented.
    // Like TaintEffectNoSchedule, but additionally do not allow pods submitted to
    // Kubelet without going through the scheduler to start.
    // Enforced by Kubelet and the scheduler.
    // TaintEffectNoScheduleNoAdmit TaintEffect = "NoScheduleNoAdmit"
  

• We've regularly added new resourcequota scopes:

  // A ResourceQuotaScope defines a filter that must match each object tracked by a quota
  // +enum
  type ResourceQuotaScope string
  

• Docs reference "currently supported" values, which sounds like there could be expansion in the future:

  // AddressType represents the type of address referred to by an endpoint.
  // +enum
  type AddressType string
  
  The following address types are currently supported:
    // * IPv4: Represents an IPv4 Address.
    // * IPv6: Represents an IPv6 Address.
    // * FQDN: Represents a Fully Qualified Domain Name.
  

Second, and more concerning, this breaks deserialization of new values by old generated clients using enums. I didn't see this aspect discussed in the KEP at all, and it's fairly significant.

[liggitt]

I opened #109177 to track this issue

For 1.24, I opened #109178 to fix the CSR condition and omit enums from the static openapi snapshot to avoid propagating enums into generated clients until we understand the implications better.

For 1.23, I picked that change back in #109179 as well, to fix the CSR condition and avoid exposing an alpha-level feature.

[liggitt]

#109179 is now merged to the release-1.23 branch, so regenerating a client from the HEAD of release-1.23 would resolve enum-based issues