fix: use QuotaExceeded reason when FederatedResourceQuota is exceeded #7086

kajal-jotwani · 2026-01-05T14:39:30Z

What type of PR is this?

What this PR does / why we need it:
Fixes the admission webhook to return the specific QuotaExceeded reason (and 403 status) when FRQ limits are hit, instead of a generic error. Added a buildDenyResponse helper to handle this case and updated tests to verify.

Which issue(s) this PR fixes:

Fixes #7069

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

None

karmada-bot · 2026-01-05T14:39:34Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign seanlaii 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:

pkg/webhook/OWNERS

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

karmada-bot · 2026-01-05T14:39:39Z

Welcome @kajal-jotwani! It looks like this is your first PR to karmada-io/karmada 🎉

gemini-code-assist · 2026-01-05T14:39:43Z

Summary of Changes

Hello @kajal-jotwani, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the behavior of the admission webhook responsible for FederatedResourceQuota enforcement. The primary goal is to provide more precise feedback to users when resource quotas are exceeded. Instead of a general denial, the system will now explicitly indicate a 'QuotaExceeded' reason with a 403 Forbidden status, which allows for better programmatic handling and clearer error messages for users.

Highlights

Specific Quota Exceeded Reason: The admission webhook now returns a specific 'QuotaExceeded' reason and an HTTP 403 status when a FederatedResourceQuota limit is reached, rather than a generic denial.
New Helper Function: A new helper function, buildDenyResponse, has been introduced to encapsulate the logic for constructing the appropriate admission response for quota exceeded scenarios.
Enhanced Test Coverage: Existing test cases have been updated to verify that the correct 'QuotaExceeded' reason and HTTP 403 status code are returned when FederatedResourceQuota limits are exceeded.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request correctly fixes an issue where a generic error was returned when a FederatedResourceQuota was exceeded. The new buildDenyResponse helper function properly constructs a specific QuotaExceeded response with a 403 status code. The tests have been effectively updated to verify this new behavior, and the addition of the quotaExceededResponse test helper improves the clarity of the test cases. Overall, the changes are well-implemented and address the problem effectively.

gemini-code-assist · 2026-01-05T14:41:18Z

pkg/webhook/resourcebinding/validating.go

 }

+func buildDenyResponse(errMsg string) admission.Response {
+	if strings.Contains(errMsg, "FederatedResourceQuota") && strings.Contains(errMsg, "exceeded") {


The current implementation for detecting a quota exceeded error relies on strings.Contains, which could be a bit brittle. Since the error message for a quota violation has a specific format starting with FederatedResourceQuota(, using strings.HasPrefix would make this check more robust and less prone to accidentally matching other error messages that might contain the substring "FederatedResourceQuota" in a different context.

Suggested change

if strings.Contains(errMsg, "FederatedResourceQuota") && strings.Contains(errMsg, "exceeded") {

if strings.HasPrefix(errMsg, "FederatedResourceQuota(") && strings.Contains(errMsg, "exceeded") {

Copilot

Pull request overview

This PR fixes the admission webhook to return the specific QuotaExceeded reason (with HTTP 403 status) when FederatedResourceQuota limits are exceeded, instead of a generic admission denied error. This ensures proper error classification for downstream components like the scheduler to handle quota exceeded scenarios appropriately.

Key Changes:

Added buildDenyResponse helper function to detect quota exceeded errors and set the appropriate reason and status code
Updated test helper quotaExceededResponse and test assertions to verify the correct reason is returned
Modified all quota exceeded test cases to use the new response helper

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/webhook/resourcebinding/validating.go	Added `buildDenyResponse` function to detect quota exceeded errors via string matching and return proper HTTP 403 status with QuotaExceeded reason; updated error handling to use this new function
pkg/webhook/resourcebinding/validating_test.go	Added `quotaExceededResponse` test helper function; updated all quota exceeded test cases to verify the correct status code and reason; added assertion to check the Reason field in test validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-05T14:49:33Z

pkg/webhook/resourcebinding/validating.go

 }

+func buildDenyResponse(errMsg string) admission.Response {
+	if strings.Contains(errMsg, "FederatedResourceQuota") && strings.Contains(errMsg, "exceeded") {


The string matching approach using Contains for both "FederatedResourceQuota" and "exceeded" is fragile and could produce false positives. For example, if a future error message contains both strings in an unrelated context, it would incorrectly be treated as a quota exceeded error. Consider using a more robust approach such as: 1) Creating a custom error type for quota exceeded errors and using errors.As() to check for it, or 2) Using a specific prefix or pattern matching instead of two independent Contains checks.

codecov-commenter · 2026-01-05T14:56:37Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 46.55%. Comparing base (b175217) to head (c03efdb).

Files with missing lines	Patch %	Lines
pkg/webhook/resourcebinding/validating.go	92.85%	1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7086      +/-   ##
==========================================
- Coverage   46.55%   46.55%   -0.01%     
==========================================
  Files         700      700              
  Lines       48091    48104      +13     
==========================================
+ Hits        22389    22393       +4     
- Misses      24020    24027       +7     
- Partials     1682     1684       +2

Flag	Coverage Δ
unittests	`46.55% <92.85%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: kajal-jotwani <[email protected]>

RainbowMango

/assign

RainbowMango · 2026-01-06T12:33:30Z

/retest
It is unrelated.
https://github.com/karmada-io/karmada/actions/runs/20720286612/job/59482638502?pr=7086

[FAILED] in [It] - /home/runner/work/karmada/karmada/test/e2e/suites/base/karmadactl_test.go:596 @ 01/05/26 15:53:31.367
  STEP: Removing PropagationPolicy(karmadatest-zrvdm/pod-6n7h6) @ 01/05/26 15:53:31.812
  STEP: Removing Pod(karmadatest-zrvdm/pod-bgfqq) @ 01/05/26 15:53:31.826
  << Timeline

  [FAILED] Unexpected error:
      <exec.CodeExitError>: 
      error running /home/runner/go/bin/karmadactl --kubeconfig=/home/runner/.kube/karmada.config --karmada-context=karmada-apiserver --namespace=karmadatest-zrvdm top pod pod-bgfqq -n karmadatest-zrvdm -C member1:
      Command stdout:
      
      stderr:
      error: cluster(member1): podmetrics.metrics.k8s.io "karmadatest-zrvdm/pod-bgfqq" not found
      
      error:
      exit status 1
      {
          Err: <*errors.errorString | 0xc000389030>{
              s: "error running /home/runner/go/bin/karmadactl --kubeconfig=/home/runner/.kube/karmada.config --karmada-context=karmada-apiserver --namespace=karmadatest-zrvdm top pod pod-bgfqq -n karmadatest-zrvdm -C member1:\nCommand stdout:\n\nstderr:\nerror: cluster(member1): podmetrics.metrics.k8s.io \"karmadatest-zrvdm/pod-bgfqq\" not found\n\nerror:\nexit status 1",
          },
          Code: 1,
      }
  occurred
  In [It] at: /home/runner/work/karmada/karmada/test/e2e/suites/base/karmadactl_test.go:596 @ 01/05/26 15:53:31.367

  Full Stack Trace
    github.com/karmada-io/karmada/test/e2e/suites/base.init.func32.2.2.1()
    	/home/runner/work/karmada/karmada/test/e2e/suites/base/karmadactl_test.go:596 +0x98
    github.com/karmada-io/karmada/test/e2e/suites/base.init.func32.2.2()
    	/home/runner/work/karmada/karmada/test/e2e/suites/base/karmadactl_test.go:592 +0x90

RainbowMango · 2026-01-06T12:52:46Z

pkg/webhook/resourcebinding/validating.go

 }

+func buildDenyResponse(errMsg string) admission.Response {
+	if strings.Contains(errMsg, "FederatedResourceQuota") && strings.Contains(errMsg, "exceeded") {


@zhzhuang-zju could you please help confirm if we need to match the error message here?

I'm afraid so. The scheduler distinguishes specific scenarios by the return code and predefined reason from the webhook. The corresponding return code for quota exceeded is 403, with the reason being QuotaExceeded. For this purpose, the corresponding return code and reason need to be set when the webhook denies a request.

karmada/pkg/scheduler/helper.go

Lines 140 to 142 in 050353e

if errors.As(ae, &statusErr) && statusErr.Status().Code == http.StatusForbidden && statusErr.Status().Reason == util.QuotaExceededReason {

return util.NewCondition(workv1alpha2.Scheduled, workv1alpha2.BindingReasonQuotaExceeded, ae.Error(), metav1.ConditionFalse), false

}

I can confirm this behavior is necessary, but I need to look over the implementation method again.

In addition, this also exposes an issue with the E2E test. At present, it only checks the status and type of the condition; other fields such as condition.reason also require attention.

karmada/test/e2e/suites/base/federatedresourcequota_test.go

Line 446 in 050353e

return rb != nil && meta.IsStatusConditionPresentAndEqual(rb.Status.Conditions, workv1alpha2.Scheduled, metav1.ConditionFalse)

In addition, this also exposes an issue with the E2E test. At present, it only checks the status and type of the condition; other fields such as condition.reason also require attention.

+1

zhzhuang-zju · 2026-01-07T03:05:06Z

/assign

Copilot AI review requested due to automatic review settings January 5, 2026 14:39

karmada-bot requested review from XiShanYongYe-Chang and mszacillo January 5, 2026 14:39

karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 5, 2026

Copilot started reviewing on behalf of kajal-jotwani January 5, 2026 14:39 View session

gemini-code-assist bot reviewed Jan 5, 2026

View reviewed changes

Copilot AI reviewed Jan 5, 2026

View reviewed changes

fix: use QuotaExceeded reason when FederatedResourceQuota is exceeded

c03efdb

Signed-off-by: kajal-jotwani <[email protected]>

kajal-jotwani force-pushed the fix/frq-quota-exceeded-reason branch from aa23390 to c03efdb Compare January 5, 2026 15:30

RainbowMango mentioned this pull request Jan 6, 2026

QuotaExceeded reason is not being used when FRQ exceeded limits #7069

Open

RainbowMango reviewed Jan 6, 2026

View reviewed changes

karmada-bot assigned RainbowMango Jan 6, 2026

RainbowMango reviewed Jan 6, 2026

View reviewed changes

karmada-bot assigned zhzhuang-zju Jan 7, 2026

	if strings.Contains(errMsg, "FederatedResourceQuota") && strings.Contains(errMsg, "exceeded") {
	if strings.HasPrefix(errMsg, "FederatedResourceQuota(") && strings.Contains(errMsg, "exceeded") {

	if errors.As(ae, &statusErr) && statusErr.Status().Code == http.StatusForbidden && statusErr.Status().Reason == util.QuotaExceededReason {
	return util.NewCondition(workv1alpha2.Scheduled, workv1alpha2.BindingReasonQuotaExceeded, ae.Error(), metav1.ConditionFalse), false
	}

fix: use QuotaExceeded reason when FederatedResourceQuota is exceeded #7086

Are you sure you want to change the base?

fix: use QuotaExceeded reason when FederatedResourceQuota is exceeded #7086

Conversation

kajal-jotwani commented Jan 5, 2026

Uh oh!

karmada-bot commented Jan 5, 2026

Uh oh!

karmada-bot commented Jan 5, 2026

Uh oh!

gemini-code-assist bot commented Jan 5, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

RainbowMango left a comment

Choose a reason for hiding this comment

Uh oh!

RainbowMango commented Jan 6, 2026

Uh oh!

RainbowMango Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

zhzhuang-zju Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

RainbowMango Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

zhzhuang-zju commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Jan 5, 2026 •

edited

Loading