Skip to content

Return BadRequest for node proxy when node has no usable address#139603

Open
sanidhyasin wants to merge 1 commit into
kubernetes:masterfrom
sanidhyasin:node-proxy-no-address-badrequest
Open

Return BadRequest for node proxy when node has no usable address#139603
sanidhyasin wants to merge 1 commit into
kubernetes:masterfrom
sanidhyasin:node-proxy-no-address-badrequest

Conversation

@sanidhyasin

Copy link
Copy Markdown

What type of PR is this?

/kind bug

What this PR does / why we need it:

A node proxy request (e.g. POST /api/v1/nodes/<name>/proxy/) to a Node that has no usable address in status.addresses currently returns an HTTP 500 Internal Server Error:

{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {},
  "status": "Failure",
  "message": "no preferred addresses found; known addresses: []",
  "code": 500
}

The error originates from GetPreferredNodeAddress, which returns a typed *node.NoMatchError when none of the preferred address types are present. That error was passed through ResourceLocation unchanged and, since it is not a recognized API status error, the endpoint reported it as an internal server error.

A Node without a routable address is not a server fault — there is simply nowhere to proxy to, which the client can correct. This PR makes ResourceLocation detect the NoMatchError and return a 400 Bad Request instead, consistent with how an unresolvable proxy hostname is already handled a few lines below in the same function (isProxyableHostname -> NewBadRequest). Other errors from GetConnectionInfo (such as a NotFound when the node does not exist) are left untouched.

Which issue(s) this PR is related to:

Fixes #139486

Special notes for your reviewer:

The chosen status code (400) mirrors the existing behavior of the sibling pod proxy ResourceLocation, which returns NewBadRequest("address not allowed") when a pod has no usable address, and the adjacent non-proxyable-hostname branch in this same function. TestResourceLocation is extended with a "node without any address" case and now asserts that every error case is a BadRequest rather than a generic error.

Does this PR introduce a user-facing change?

kube-apiserver: a node proxy request to a Node that has no usable address in `status.addresses` now returns an HTTP 400 (Bad Request) instead of an HTTP 500 (Internal Server Error).

A node proxy request to a Node that has no usable address in
status.addresses (for example a Node object created without any
addresses) caused the apiserver to return an HTTP 500. The error
originates from GetPreferredNodeAddress, which returns a typed
NoMatchError that was passed through unchanged and therefore treated
as an internal server error.

This is a client-correctable condition rather than a server fault, so
ResourceLocation now detects the NoMatchError and returns a 400 Bad
Request, consistent with how an unresolvable proxy hostname is already
handled in the same function.
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. kind/bug Categorizes issue or PR as related to a bug. labels Jun 9, 2026
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 9, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: sanidhyasin / name: Sanidhya Singh (387a21f)

@k8s-ci-robot k8s-ci-robot added do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 9, 2026
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

Welcome @sanidhyasin!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jun 9, 2026
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

Hi @sanidhyasin. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 9, 2026
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sanidhyasin
Once this PR has been reviewed and has the lgtm label, please assign sjenning for approval. For more information see the Code Review Process.

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

Details 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 sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 9, 2026
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Development

Successfully merging this pull request may close these issues.

apiserver: return non-500 response for node proxy request when node has no preferred addresses

2 participants