Test(webhook): add verify empty placement does not panic in propagation policies by Priyanshu8023 · Pull Request #7640 · karmada-io/karmada

Priyanshu8023 · 2026-06-19T07:30:47Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
These PRs provide regression test cases for the PropagationPolicy and ClusterPropagationPolicy mutating webhooks in case spec.placement is missing.

spec.placement is an optional field with value type, and therefore, if it is not provided, it is decoded into an empty Placement value. These new test cases illustrate such behavior.

Which issue(s) this PR fixes:

Fixes #7638

Special notes for your reviewer:
The reported panic scenario assumes spec.placement is a pointer. In the current API, PropagationSpec.Placement is a value field:
Because of that, omitting spec.placement decodes to an empty Placement value, not nil. This PR keeps the production webhook logic unchanged and adds focused tests to document and protect the expected behavior for both namespaced and cluster-scoped propagation policies.

Tested with:

go test ./pkg/webhook/propagationpolicy
go test ./pkg/webhook/clusterpropagationpolicy

Does this PR introduce a user-facing change?:
NONE

gemini-code-assist · 2026-06-19T07:30:55Z

Summary of Changes

Hello, 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 introduces regression tests for the mutating webhooks associated with propagation policies. The goal is to ensure that the system remains stable when the optional spec.placement field is omitted, preventing potential panics during the admission process. These changes document and protect the expected behavior for both namespaced and cluster-scoped policies.

Highlights

Regression Testing: Added new test cases to verify that mutating webhooks for PropagationPolicy and ClusterPropagationPolicy handle empty placement fields correctly without panicking.
Webhook Robustness: Ensured that the optional spec.placement field, which decodes to an empty value rather than nil, is properly supported by the admission handlers.

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 the 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 counterproductive. 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.

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. ↩

karmada-bot · 2026-06-19T07:30:58Z

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

karmada-bot · 2026-06-19T07:30:59Z

[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 whitewindmills 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

gemini-code-assist

Code Review

This pull request introduces new unit tests to verify the behavior of mutating admission webhooks when handling empty placement specifications. Specifically, it adds TestMutatingAdmission_Handle_EmptyPlacement to both pkg/webhook/clusterpropagationpolicy/mutating_test.go and pkg/webhook/propagationpolicy/mutating_test.go to ensure that both cluster-scoped and namespaced propagation policies with an empty Spec.Placement are correctly allowed and processed by the mutating handlers. I have no feedback to provide as there are no review comments and the changes are well-implemented.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Copilot

Pull request overview

Adds regression coverage for the PropagationPolicy and ClusterPropagationPolicy mutating webhooks to ensure they don’t panic when spec.placement is omitted (decoded as an empty Placement value), documenting the expected behavior without changing production webhook logic.

Changes:

Add a new mutating-webhook unit test for namespaced PropagationPolicy when spec.placement is missing.
Add a new mutating-webhook unit test for cluster-scoped ClusterPropagationPolicy when spec.placement is missing.

Reviewed changes

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

File	Description
pkg/webhook/propagationpolicy/mutating_test.go	Adds a regression test covering omitted `spec.placement` for namespaced propagation policies.
pkg/webhook/clusterpropagationpolicy/mutating_test.go	Adds a regression test covering omitted `spec.placement` for cluster-scoped propagation policies.

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

Priyanshu8023 · 2026-06-19T09:12:28Z

+		Spec: policyv1alpha1.PropagationSpec{
+			// Keep the selector namespace empty so the webhook can default it.
+			ResourceSelectors: []policyv1alpha1.ResourceSelector{
+				{Namespace: ""},
+			},
+		},


fixed the issue in the commit

Priyanshu8023 · 2026-06-19T09:11:31Z

+		Spec: policyv1alpha1.PropagationSpec{
+			// Keep a minimal selector so the test focuses on placement handling.
+			ResourceSelectors: []policyv1alpha1.ResourceSelector{
+				{},
+			},
+		},


fixed the issue in the commit

codecov-commenter · 2026-06-19T07:49:06Z

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.06%. Comparing base (7edc6e6) to head (19f4dc0).
⚠️ Report is 8 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7640      +/-   ##
==========================================
+ Coverage   42.04%   42.06%   +0.01%     
==========================================
  Files         879      879              
  Lines       54827    54827              
==========================================
+ Hits        23053    23061       +8     
+ Misses      30027    30021       -6     
+ Partials     1747     1745       -2

Flag	Coverage Δ
unittests	`42.06% <ø> (+0.01%)`	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

…utating webhooks Signed-off-by: Priyanshu8023 <priyanshukumar8023@gmail.com>

RainbowMango · 2026-06-24T03:54:12Z

+	// Empty placement is valid and should not block admission.
+	got := mutatingHandler.Handle(context.Background(), req)
+	if !got.Allowed {
+		t.Errorf("Handle() got.Allowed = false, want true")
+	}


What is expected is that the request could be allowed. Why do you think this is a bug? @Priyanshu8023

Copilot AI review requested due to automatic review settings June 19, 2026 07:30

karmada-bot requested review from XiShanYongYe-Chang and mszacillo June 19, 2026 07:30

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

Copilot started reviewing on behalf of Priyanshu8023 June 19, 2026 07:31 View session

gemini-code-assist Bot reviewed Jun 19, 2026

View reviewed changes

Copilot AI reviewed Jun 19, 2026

View reviewed changes

test: add unit tests for propagation and cluster propagation policy m…

19f4dc0

…utating webhooks Signed-off-by: Priyanshu8023 <priyanshukumar8023@gmail.com>

Priyanshu8023 force-pushed the fix/handle-nil-placement branch from 7d7eb87 to 19f4dc0 Compare June 19, 2026 08:08

Priyanshu8023 changed the title ~~test(webhook): verify empty placement does not panic in propagation policies~~ Test(webhook): add verify empty placement does not panic in propagation policies Jun 19, 2026

RainbowMango reviewed Jun 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Test(webhook): add verify empty placement does not panic in propagation policies#7640

Test(webhook): add verify empty placement does not panic in propagation policies#7640
Priyanshu8023 wants to merge 1 commit into
karmada-io:masterfrom
Priyanshu8023:fix/handle-nil-placement

Priyanshu8023 commented Jun 19, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot commented Jun 19, 2026

Uh oh!

karmada-bot commented Jun 19, 2026

Uh oh!

karmada-bot commented Jun 19, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Priyanshu8023 Jun 19, 2026

Uh oh!

Priyanshu8023 Jun 19, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Jun 19, 2026 •

edited

Loading

Uh oh!

RainbowMango Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

Priyanshu8023 commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot commented Jun 19, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

karmada-bot commented Jun 19, 2026

Uh oh!

karmada-bot commented Jun 19, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Priyanshu8023 Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Priyanshu8023 Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

RainbowMango Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Priyanshu8023 commented Jun 19, 2026 •

edited

Loading

Priyanshu8023 Jun 19, 2026 •

edited

Loading

codecov-commenter commented Jun 19, 2026 •

edited

Loading