work suspension: webhook validation

[a7i]

What type of PR is this?
/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #5118

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

[codecov-commenter]

⚠️ 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 28.40%. Comparing base (b4b6d69) to head (d9e174b).
Report is 408 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    #5282      +/-   ##
==========================================
+ Coverage   28.38%   28.40%   +0.02%     
==========================================
  Files         632      632              
  Lines       43801    43810       +9     
==========================================
+ Hits        12432    12444      +12     
+ Misses      30464    30462       -2     
+ Partials      905      904       -1     

Flag	Coverage Δ
unittests	28.40% <100.00%> (+0.02%)	⬆️

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.

[a7i]

/hold

until #4838 is merged

[XiShanYongYe-Chang]

/assign

[XiShanYongYe-Chang]

Hi @a7i we can continue this task ^-^

[a7i]

/remove-hold

[RainbowMango]

/assign

[RainbowMango]

Generally looks good.

I'm surprised that this PR is not based on the latest commit, but all tests are passing. I fetched the PR on my side (by git fetch upstream pull/5282/head:5282) and it failed to compile as missing the API definition.

I'm unsure if this is due to GitHub Action changing how it fetches code.

Anyway, @a7i could you please help to rebase this and push again?

[RainbowMango]

I would prefer this straightforward style:

Suggested change

	if !ptr.Deref(suspension.Dispatching, false) {
	return nil
	}
	if suspension.DispatchingOnClusters == nil || len(suspension.DispatchingOnClusters.ClusterNames) == 0 {
	return nil
	}
	return field.ErrorList{
	field.Invalid(fldPath.Child("suspension"), suspension, "suspension dispatching cannot co-exist with dispatchingOnClusters.clusterNames"),
	}
	if (suspension.Dispatching != nil && *suspension.Dispatching == true) &&
	(suspension.DispatchingOnClusters != nil && len(suspension.DispatchingOnClusters.ClusterNames) > 0) {
	return field.ErrorList{
	field.Invalid(fldPath.Child("suspension"), suspension, "suspension dispatching cannot co-exist with dispatchingOnClusters.clusterNames"),
	}
	}
	return nil

[grosser]

the old version was easier to digest for me ... ideally 1 comment per if explaining what is going on
suggested feels a overwhelming

[RainbowMango]

I'm sorry for not explaining why I'm suggesting that in detail.

Firstly the old version is great, it looks simple. But given that we need a single check on whether the two fields co-exist, I tend to a more cohesive approach to make it easier to add more checks in the future.

[a7i]

thanks for the review @RainbowMango

rebased and applied the suggestion

[RainbowMango]

/lgtm
/approve

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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:

• pkg/util/OWNERS [RainbowMango]

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

[RainbowMango]

/retest
The failing case seems unrelated.