KEP: Resource deletion protection proposal

[Vacant2333]

What type of PR is this?
Add resource deletion protection proposal
Issue: #3728
/kind feature
/kind documentation

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Add resource deletion protection proposal

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: -1.87% ⚠️

Comparison is base (c921acd) 55.68% compared to head (810a674) 53.82%.
Report is 271 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    #3802      +/-   ##
==========================================
- Coverage   55.68%   53.82%   -1.87%     
==========================================
  Files         222      231       +9     
  Lines       21195    23013    +1818     
==========================================
+ Hits        11803    12386     +583     
- Misses       8765     9954    +1189     
- Partials      627      673      +46     

Flag	Coverage Δ
unittests	53.82% <ø> (-1.87%)	⬇️

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

see 84 files with indirect coverage changes

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

[XiShanYongYe-Chang]

Thanks~

[Vacant2333]

Regarding the issue of users force deleting resources, I believe we shouldn't block this operation. Our goal is to prevent users from accidentally deleting resources, but if a user uses the --force parameter, we should assume that they know what they are doing. If anyone has any thoughts on this, please feel free to share them below.
/cc @XiShanYongYe-Chang

[Vacant2333]

Regarding the issue of users force deleting resources, I believe we shouldn't block this operation. Our goal is to prevent users from accidentally deleting resources, but if a user uses the --force parameter, we should assume that they know what they are doing. If anyone has any thoughts on this, please feel free to share them below. /cc @XiShanYongYe-Chang

/cc @zishen @RainbowMango

[karmada-bot]

@Vacant2333: GitHub didn't allow me to request PR reviews from the following users: zishen.

Note that only karmada-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Regarding the issue of users force deleting resources, I believe we shouldn't block this operation. Our goal is to prevent users from accidentally deleting resources, but if a user uses the --force parameter, we should assume that they know what they are doing. If anyone has any thoughts on this, please feel free to share them below. /cc @XiShanYongYe-Chang

/cc @zishen @RainbowMango

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/test-infra repository.

[XiShanYongYe-Chang]

Regarding the issue of users force deleting resources, I believe we shouldn't block this operation. Our goal is to prevent users from accidentally deleting resources, but if a user uses the --force parameter, we should assume that they know what they are doing. If anyone has any thoughts on this, please feel free to share them below. /cc @XiShanYongYe-Chang

We can add this QA to the proposal.

[XiShanYongYe-Chang]

Please sign off refer to https://github.com/karmada-io/karmada/pull/3802/checks?check_run_id=15416905578

Ask an review from @zishen @RainbowMango

[Vacant2333]

Our label name may need to be changed. Does anyone have any suggestions?

[RainbowMango]

After a quick glance at this proposal, a question occurred to me: Will it be convenient enough for users to have only one label? What if users have a large number of namespaces to protect? They might need to add the labels to each of them.
@XiShanYongYe-Chang

[XiShanYongYe-Chang]

If there are numerous resources that need to be protected, it would be tedious for users to add labels one by one. It would be more convenient if a configurable resource could be provided to inform Karmada which resources need protection.
@RainbowMango

[Vacant2333]

If there are numerous resources that need to be protected, it would be tedious for users to add labels one by one. It would be more convenient if a configurable resource could be provided to inform Karmada which resources need protection. @RainbowMango

Wondering whether such implementation would be overly complex, we only need to protect the resources of the control plane, and in most cases, there may not be too many resources that need protection.

[XiShanYongYe-Chang]

Wondering whether such implementation would be overly complex, we only need to protect the resources of the control plane, and in most cases, there may not be too many resources that need protection.

The deletion protection for individual resources is an atomic capability that we provide. If there are a large number of resources that need to be protected, we may need to consider how to optimize the user experience.

[Vacant2333]

Wondering whether such implementation would be overly complex, we only need to protect the resources of the control plane, and in most cases, there may not be too many resources that need protection.

The deletion protection for individual resources is an atomic capability that we provide. If there are a large number of resources that need to be protected, we may need to consider how to optimize the user experience.

Perhaps we can consider this issue after implementing the proposal?
At the moment, I don't have a good idea to solve this problem.

[XiShanYongYe-Chang]

Perhaps we can consider this issue after implementing the proposal?
At the moment, I don't have a good idea to solve this problem.

Yes, I agree with your point.
In addition, we need to consider whether the default protection strategy for resources should require users to manually add labels to enable deletion protection, or if it should be automatically enabled once the resource template is created.

[Vacant2333]

In addition, we need to consider whether the default protection strategy for resources should require users to manually add labels to enable deletion protection, or if it should be automatically enabled once the resource template is created.

In Kruise, they did not adopt an automatic protection strategy but instead used two levels of protection. However, I believe their approach is not only complex but also has some limitations, and it also involves manually adding labels. I think our practice of manually adding them is sufficient, and adding a default policy may cause inconvenience to the users.

[XiShanYongYe-Chang]

I think we can add a new featureGate that, when turned on, enables deletion protection for all namespace resources (and extends to other resources as well).
@RainbowMango @Vacant2333

[Vacant2333]

This is a good idea, but perhaps users don't want to protect all namespaces by default? Maybe this featureGate could be solely responsible for enabling or disabling deletion protection.

[XiShanYongYe-Chang]

/assign

[XiShanYongYe-Chang]

Good job! Let's continue to improve this proposal.

[RainbowMango]

Hi, Please tidy the commits, let's get it ready to move forward.

[Vacant2333]

@kevin-wangzefeng

Hi, Please tidy the commits, let's get it ready to move forward.

~~ok

[XiShanYongYe-Chang]

/lgtm
I think we can merge it first and then iteratively optimize it.

[RainbowMango]

/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:

• OWNERS [RainbowMango]

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