Skip to content

Conversation

serathius
Copy link
Contributor

@k8s-ci-robot k8s-ci-robot added this to the 1.34 milestone Jul 1, 2025
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 1, 2025
Copy link

netlify bot commented Jul 1, 2025

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit fc094d4
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-io-vnext-staging/deploys/68906b1b57699e00086df510

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 1, 2025
@k8s-ci-robot k8s-ci-robot added the language/en Issues or PRs related to English language label Jul 1, 2025
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 1, 2025
@lmktfy
Copy link
Contributor

lmktfy commented Jul 1, 2025

This KEP is byte for byte compatible IIRC. Is there much to document?

If we want to expand anything, it would be the documentation about API server performance.

@serathius
Copy link
Contributor Author

If we want to expand anything, it would be the documentation about API server performance.

Exactly

@lmktfy
Copy link
Contributor

lmktfy commented Jul 1, 2025

I suggest adding a brief (stub?) page under https://kubernetes.io/docs/concepts/cluster-administration/#managing-a-cluster, about API server performance, and making no other changes.

@serathius
Copy link
Contributor Author

Don't think https://kubernetes.io/docs/concepts/cluster-administration/#managing-a-cluster has anything related to performance.

@serathius serathius marked this pull request as ready for review July 28, 2025 20:13
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 28, 2025
@k8s-ci-robot k8s-ci-robot requested a review from liggitt July 28, 2025 20:13
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 28, 2025
Copy link

netlify bot commented Jul 28, 2025

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit fc094d4
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-io-main-staging/deploys/68906b1b4d1ff30008519104
😎 Deploy Preview https://deploy-preview-51457--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@serathius
Copy link
Contributor Author

Ready for review.

@yujen77300
Copy link
Contributor

Hello @serathius 👋! I'm reaching out from the Docs team.

Just checking in as we approach Docs Freeze on Wednesday August 6, 2025 18:00 PDT. This documentation appears to still be under review. To meet the Docs Freeze, this PR must have a technical review as well as lgtm and approve labels applied, without any unaddressed comments or concerns from SIG Docs. Thank you!

Copy link
Member

@liggitt liggitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one tweak, lgtm otherwise

Copy link
Contributor

@lmktfy lmktfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should emphasize that this is streaming but it's not about the kind of streaming you can do with a watch (nor, if we want to be clear, is it support for fetching collections over a WebSocket).

Over HTTP, Kubernetes supports JSON, YAML, CBOR and Protobuf wire encodings.

By default, Kubernetes returns objects in [JSON serialization](#json-encoding), using the
`application/json` media type. Although JSON is the default, clients may request a response in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we mention (alpha) CBOR here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should mention stage, I think CBOR should be going Beta in 1.34. All the media types are explained in more detail below, so I would depend on that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(CBOR is still alpha in 1.34, fwiw)

but allows API server to avoid loading whole responses into memory.
Using other types of encoding (including pretty representation of JSON)
should be avoided for large collections of resources (>100MB) as it can have negative performance impact.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a to impose rate limits (APF) or outright restrictions on the media type that Kubernetes responds with, right?
(if we did, I would be recommending adding an item to the security checklist, or recommending a custom expression APF rule, or something).

Can we somehow make it easy for someone reading this to spot that support for CBOR (optional?) and YAML (GA) makes it easier for a malicious or reckless client to DoS your control plane?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a to impose rate limits (APF) or outright restrictions on the media type that Kubernetes responds with, right?

I have been looking into it as part of kubernetes/kubernetes#132233 however I discovered that APF is not very good at punishing very expensive requests. It's focuses on priority and fairness and not resource protection. I plan to add the logic to detect whether media type is streamed to APF cost estimation, however at the current state APF it would not help much with preventing apiserver overload.

Can we somehow make it easy for someone reading this to spot that support for CBOR (optional?) and YAML (GA) makes it easier for a malicious or reckless client to DoS your control plane?

This is the biggest issue with K8s for any cloud provider. It's too easy for misguided client to take down control plane.

@serathius
Copy link
Contributor Author

serathius commented Aug 4, 2025

We should emphasize that this is streaming but it's not about the kind of streaming you can do with a watch (nor, if we want to be clear, is it support for fetching collections over a WebSocket).

Alternative name I was considering was chunked response, but has a special meaning for HTTP Chunked transfer encoding.

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 4, 2025
@michellengnx
Copy link
Contributor

/assign @divya-mohan0209

## HTTP media types {#alternate-representations-of-resources}

Over HTTP, Kubernetes supports JSON and Protobuf wire encodings.
Over HTTP, Kubernetes supports JSON, YAML, CBOR and Protobuf wire encodings.
Copy link
Member

@reylejano reylejano Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree about mentioning CBOR being alpha (non blocking)

Suggested change
Over HTTP, Kubernetes supports JSON, YAML, CBOR and Protobuf wire encodings.
Over HTTP, Kubernetes supports JSON, YAML, CBOR (alpha) and Protobuf wire encodings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 341 we have whole section for CBOR that doesn't mention alpha at all, only the feautre gate CBORServingAndStorage. Why mention Alpha here and not there?

@drewhagen
Copy link
Member

Hello @serathius 👋!

I'm reaching out on behalf of the Release Docs team.

A friendly reminder that Docs Freeze is approaching today, at 18:00 PT (less than 8 hours). This documentation PR appears to still be under review. Here's where it currently stands:

  • Needs a technical approval from SIG API Machinery (looks like @liggitt once said lgtm, but we could still use a label)
  • Comments and concerns from SIG Docs (at this moment) have been discussed and need resolution
  • Needs a lgtm label applied
  • Needs an approval label applied

If you wish to include this enhancement in 1.34 but are concerned about meeting the deadline, please consider proactively filing an exception request. Thanks!

cc: @kubernetes/sig-api-machinery-emeritus FYSA

@liggitt
Copy link
Member

liggitt commented Aug 6, 2025

/lgtm
for technical content

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 23aad1499e2ab74bb5e6163d2481c4445dcf228d

Copy link
Contributor

@lmktfy lmktfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to document any feature gates? I think the change in this PR is OK to merge but I also expect there needs to be more than just this part.

/lgtm
/approve

a `406 Not acceptable` error message.
All built-in resource types support the `application/json` media type.

#### Chunked encoding of collections
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is OK and I'll merge it, but AIUI we support HTTP/2 and you can get streaming response serialisation without chunked encoding.

I also don't know whether the HTTP/1 implementation means we actually emit chunks for this (I assume we do, but haven't checked).

Copy link
Member

@liggitt liggitt Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AIUI we support HTTP/2 and you can get streaming response serialisation without chunked encoding.

HTTP/2 is the transport / network transfer, not the serialization.

This improvement and documentation is describing how the server can do streaming serialization of the response to the writer / network, rather than fully serializing into a byte array in memory, then copying that byte array to the writer / network, which is what it did previously.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lmktfy

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 6, 2025
@k8s-ci-robot k8s-ci-robot merged commit 41b96ba into kubernetes:dev-1.34 Aug 6, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants