pkg/webhook: bind Multi-Cluster Ingress validation functions to ValidationAdmission struct

[mohamedawnallah]

Description

In this commit, we bind the validation functions of MultiClusterIngress webhook i.e validateMCIUpdate and validateMCI functions to ValidatingAdmission struct using pointer receivers.

Motivation and Context

While testing the multi-cluster ingress webhook (#5518), I identified an opportunity to improve consistency and maintainability by aligning the validation methods in Multi-Cluster Ingress with the pattern used in pkg/webhook/multiclusterservice, leading to this submission.

What type of PR is this?

/kind cleanup

Does this PR introduce a user-facing change?:

NONE

[karmada-bot]

[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 chaunceyjiang for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[codecov-commenter]

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

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 32.45%. Comparing base (32c2ef7) to head (48c034c).
Report is 16 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/webhook/multiclusteringress/validating.go	0.00%	5 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5520      +/-   ##
==========================================
+ Coverage   31.70%   32.45%   +0.74%     
==========================================
  Files         643      643              
  Lines       44445    44497      +52     
==========================================
+ Hits        14090    14440     +350     
+ Misses      29325    28961     -364     
- Partials     1030     1096      +66     

Flag	Coverage Δ
unittests	32.45% <0.00%> (+0.74%)	⬆️

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.

[mohamedawnallah]

After reflecting on this PR, I think it is subjective and does not necessarily enhance maintainability or consistency. As I review the repository, I notice a mix of both approaches.

What do you think?

cc: @XiShanYongYe-Chang, @zhzhuang-zju

[XiShanYongYe-Chang]

You mean this change is not necessary, right?

[mohamedawnallah]

You mean this change is not necessary, right?

Yes. I'd also like to hear your thoughts on it?

[XiShanYongYe-Chang]

Yes, but it's still a positive change.

Why do you say that? Can you give some reason?

[mohamedawnallah]

Yes, but it's still a positive change.

Why do you say that? Can you give some reason?

Sorry for removing it I thought it would cause confusion. the reason in the Motivation and Context:
"
While testing the multi-cluster ingress webhook (#5518), I identified an opportunity to improve consistency and maintainability by aligning the validation methods in Multi-Cluster Ingress with the pattern used in pkg/webhook/multiclusterservice, leading to this submission.
"

[XiShanYongYe-Chang]

After reading, the current modification is of little significance. At least for now, I have not seen any improvement in maintainability. If there are great benefits in the future, let's continue the improvement. Do you think it's okay? @mohamedawnallah

[mohamedawnallah]

Do you think it's okay?

Yes I'm okay with that. Let's close this PR and remove the dependency that this PR #5518 depends on 👍