nit: we could request application/json (just that, no parameters) and / or application/yaml, application/cbor and see what happens.

I have that below in the "unsupported application/json without params" testcase

wouldn't only allowing application/json when the gvk is set be better?

if mimeType == "application" && mimeSubType == "json" {
  return isStructured(gvk)
}

return false

If someone requests a zpage and their Accept: header indicates a preference for CBOR, and CBOR support is enabled, what should we serve?

@dgrisonnet yeah thats more readable, except that I added checks for both json and yaml to the conditional.

@lmktfy I am not entirely sure that we want to support CBOR for statusz/flagz. Both the endpoints serve as debugging endpoints and have relatively small payloads. I also dont foresee users polling these endpoints every few seconds (which if it was the case, would be a good reason to support CBOR for faster parsing of the response). Since statusz and flagz responses are small diagnostic payloads that never make it to storage, I'm not sure there's significant benefit to adding CBOR support beyond consistency with other Kubernetes APIs. But I cant also think of any obvious downsides to it, so I am open to changing my mind :)

Also cc @liggitt for this opinion on CBOR support for statusz/flagz

My (personal) opinion here, not representing any SIG.

Overall I would have it as either an exclusion list ("no Protobuf") or by calling a method to check "is this a text-like format".

So I don't especially support hard coding JSON and YAML as the formats. If we did add, let's say, XML or TOML or something, then the people doing that shouldn't need to look into the z-pages code specifically to find where to change the negotiation rules.

@benluddy is cbor negotiation wired in for things like serialization of discovery formats today?

I agree with supporting json / yaml / cbor consistently. I'm actually fine with specifically allowing those three so we can also pair that with tests of the behavior of those three. Making an exclusion list makes it hard to be confident we have exhaustively tested the outputs we allow.

is cbor negotiation wired in for things like discovery today?

Not today. I don't know why it should not be. Opened a PR to add it to the KEP (kubernetes/enhancements#5740).

sounds good, commented there

So I added CBOR handling (I check for the CBORServingAndStorage feature gate, and if enabled, include the CBOR serializer when creating the codecFactory. Also added a TODO to remove this explicit check from statusz/flagz once we implement this in the codec_factory.go

Please lmk if this is ok or if I should just remove the explicit feature-gate check and enablement of CBOR support for statusz/flag, and instead wait for CBOR serializer to be included in code_factory in future to use it for zpages.

I agree with supporting json / yaml / cbor consistently. I'm actually fine with specifically allowing those three so we can also pair that with tests of the behavior of those three. Making an exclusion list makes it hard to be confident we have exhaustively tested the outputs we allow.

is this called on every request to /flagz during negotiation? if so, let's calculate this once and cache/return to avoid reallocating on every request

yeah it was. Now caching it in flagzCodecFactory once and resuing that, thanks for the tip!

prefer both positive and negative matching so any future additions get detected / handled

• include text, json, yaml, cbor
• exclude proto
• consider panicing on another unknown type when running tests so we catch this and make a decision about whether to support a new format for this

Makes sense. Done and added a unit test to check for no errors when creating the flagzCodecFactory and statuszCodecFactory