The boolean "TrueField" introduced for CORS can cause generator issues

[shaneutt]

When updating gateway-api-rs to Gateway API v1.3.0, the generators failed for the experimental version of HTTPRoute due to a boolean enum in the spec that was introduced for CORS.

properties:
  cors:
    description: |-
      CORS defines a schema for a filter that responds to the
      cross-origin request based on HTTP response header.

      Support: Extended
    properties:
      allowCredentials:
        description: |-
          AllowCredentials indicates whether the actual cross-origin request allows
          to include credentials.
          The only valid value for the `Access-Control-Allow-Credentials` response
          header is true (case-sensitive).
          If the credentials are not allowed in cross-origin requests, the gateway
          will omit the header `Access-Control-Allow-Credentials` entirely rather
          than setting its value to false.

          Support: Extended
        enum:
        - true
        type: boolean

More details are available in kube-rs/gateway-api-rs#144.

I suggest we change the type away from boolean as part of #3756.

cc @lianglli @EyalPazz @robscott @youngnick @mlavacca

[EyalPazz]

/assign

[howardjohn]

A string would require a user to write "true" which seems like a regression in user experience and likely source of errors/confusion?

[shaneutt]

A string would require a user to write "true" which seems like a regression in user experience and likely source of errors/confusion?

If we decided that we wanted to do a string (which I explicitly avoided recommending above in anticipation of some problems with that), yes we would need to make sure to clearly highlight the change as backwards incompatible in the v1.4.0 release 👍

[howardjohn]

A string would require a user to write "true" which seems like a regression in user experience and likely source of errors/confusion?

Yes, we would need to make sure to clearly highlight the change as backwards incompatible in the v1.4.0 release 👍

I am not talking about compatibility, I am talking about long term. Representing booleans as a string is quite confusing for a user -- picking a bool to represent a bool was not an accident but an intentional shift from the original GEP because of the confusion this caused; it was originally merged as string!

[JoelSpeed]

If you are changing the serialization of the field then you really should be picking a new name for the field. If you change this to a string then any generated client that is on the older version of the API will not be able to decode data from newer clients and will panic.

Changing to a string doesn't really help the issue of the API design here.

Have you considered making an enum string field that has meaningful behavioural descriptions for the values.

Would the following make more sense? (and be able to be expanded in the future with a Disallowed value if that made sense?)

cors:
  credentials: Allowed | Omitted (|Disallowed)

[howardjohn]

@JoelSpeed in 99% of cases I would agree to use an enum like this but in this case, given the API we are configuring is Access-Control-Allow-Credentials: true (and there is no other possible value), IMO anything other than true is actually making things more confusing.

As a user, I know I want the header Access-Control-Allow-Credentials: true to be set. Nothing will be more clear than setting allowCredentials: true in the HTTPRoute IMO.

[aojea]

/api-review

Please include API reviewers on the conversation so we can standardize this situation and be consistent across apis

/assign @liggitt @deads2k @thockin

[thockin]

Booleans in API are not forbidden, but we often end up with multiple booleans which form a sort of non-orthogonal chain, which would have been better represented as an enum.

I don't mean "true", "false", "idunno" (booleans-in-strings are gross), I mean changing the question so that "true" is implied by the answer. E.g. CORSMode: Strict. I honestly don't know if that makes sense here, I'm just clarifying the convention.