[Feat] Resource Deletion Protection #4074

Vacant2333 · 2023-09-18T10:28:24Z

What type of PR is this?
/kind feature
Add the resource deletion protection

What this PR does / why we need it:
Proposal: #3802

Which issue(s) this PR fixes:
Part of ##3728

Special notes for your reviewer:
install by helm:

install by local-up-karmada.sh:

install by karmada-operator:

Does this PR introduce a user-facing change?:

`karmada-webhook`: Introduced `validate-resourcedeletionprotection` webhook which enforces resource protection rules for those resources labeled with `resourcetemplate.karmada.io/deletion-protected`.

codecov-commenter · 2023-09-18T10:47:19Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (5135e8f) 54.71% compared to head (1df2420) 53.50%.
Report is 98 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    #4074      +/-   ##
==========================================
- Coverage   54.71%   53.50%   -1.21%     
==========================================
  Files         232      234       +2     
  Lines       22828    23296     +468     
==========================================
- Hits        12490    12465      -25     
- Misses       9659    10149     +490     
- Partials      679      682       +3

Flag	Coverage Δ
unittests	`53.50% <100.00%> (-1.21%)`	⬇️

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

Files	Coverage Δ
...armadactl/cmdinit/karmada/webhook_configuration.go	`88.81% <100.00%> (+0.66%)`	⬆️
pkg/webhook/cronfederatedhpa/validating.go	`80.68% <ø> (ø)`

... and 32 files with indirect coverage changes

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

XiShanYongYe-Chang · 2023-09-18T11:22:51Z

Thanks, @Vacant2333, I will review it later.
/assign

artifacts/deploy/webhook-configuration.yaml

chaunceyjiang

LGTM

XiShanYongYe-Chang

Good Job!

pkg/webhook/resourcedeletionprotection/validating.go

XiShanYongYe-Chang

Thanks~
/lgtm
/cc @RainbowMango @chaunceyjiang

XiShanYongYe-Chang · 2023-10-11T07:06:13Z

/cc @RainbowMango

chaunceyjiang

/lgtm

RainbowMango

/assign

RainbowMango · 2023-10-13T06:44:23Z

pkg/webhook/cronfederatedhpa/validating.go

nice finding! for the renaming: validation.go --> validating.go.
cc @jwcesign
It's no big deal, but it'd be great if we kept the consistent naming rule.

RainbowMango

Generally looks good, just proposed another label key.
Please let me know what you think.

By the way, @Vacant2333 sorry for the day.

pkg/webhook/resourcedeletionprotection/validating.go

artifacts/deploy/webhook-configuration.yaml

charts/karmada/templates/_karmada_webhook_configuration.tpl

operator/pkg/karmadaresource/webhookconfiguration/manifests.go

RainbowMango · 2023-10-16T08:33:59Z

pkg/karmadactl/cmdinit/karmada/webhook_configuration.go

@@ -256,6 +256,23 @@ webhooks:
    sideEffects: None
    admissionReviewVersions: [ "v1" ]
    timeoutSeconds: 3
+  - name: resourcedeletionprotection.karmada.io


Suggested change

- name: resourcedeletionprotection.karmada.io

- name: resource deletion protection

we may use the consist name like the other webohooks?
or resource-deletion-protection

Yeah, you are right, I didn't notice that the name should be fully qualified.
Then, please ignore my suggestions. Let's keep the resourcedeletionprotection.karmada.io.

pkg/webhook/resourcedeletionprotection/validating.go

Signed-off-by: Vacant2333 <vacant2333@gmail.com>

RainbowMango

/approve

Leave final decisions to @chaunceyjiang @XiShanYongYe-Chang

karmada-bot · 2023-10-17T08:48:52Z

[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

XiShanYongYe-Chang · 2023-10-18T03:34:43Z

Thanks~
/lgtm

karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 18, 2023

karmada-bot requested review from calvin0327, lonelyCZ, rv64m, prodanlabs and XiShanYongYe-Chang September 18, 2023 10:28

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 18, 2023

karmada-bot assigned XiShanYongYe-Chang Sep 18, 2023

chaunceyjiang reviewed Sep 22, 2023

View reviewed changes

artifacts/deploy/webhook-configuration.yaml Show resolved Hide resolved

chaunceyjiang reviewed Sep 22, 2023

View reviewed changes

XiShanYongYe-Chang reviewed Sep 22, 2023

View reviewed changes

pkg/webhook/resourcedeletionprotection/validating.go Outdated Show resolved Hide resolved

pkg/webhook/resourcedeletionprotection/validating.go Outdated Show resolved Hide resolved

pkg/webhook/resourcedeletionprotection/validating.go Outdated Show resolved Hide resolved

Vacant2333 force-pushed the add-resource-deletion-protection-by-label branch from 5723823 to 7902764 Compare September 22, 2023 11:34

XiShanYongYe-Chang reviewed Oct 8, 2023

View reviewed changes

pkg/webhook/resourcedeletionprotection/validating.go Outdated Show resolved Hide resolved

Vacant2333 force-pushed the add-resource-deletion-protection-by-label branch from 7902764 to 7bafc02 Compare October 8, 2023 08:28

XiShanYongYe-Chang reviewed Oct 8, 2023

View reviewed changes

karmada-bot requested review from chaunceyjiang and RainbowMango October 8, 2023 09:07

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2023

chaunceyjiang reviewed Oct 11, 2023

View reviewed changes

karmada-bot assigned chaunceyjiang Oct 11, 2023

RainbowMango reviewed Oct 13, 2023

View reviewed changes

karmada-bot assigned RainbowMango Oct 13, 2023

RainbowMango reviewed Oct 13, 2023

View reviewed changes

pkg/webhook/resourcedeletionprotection/validating.go Outdated Show resolved Hide resolved

pkg/webhook/resourcedeletionprotection/validating.go Outdated Show resolved Hide resolved

RainbowMango reviewed Oct 13, 2023

View reviewed changes

pkg/webhook/resourcedeletionprotection/validating.go Outdated Show resolved Hide resolved

Vacant2333 force-pushed the add-resource-deletion-protection-by-label branch from 7bafc02 to 76c6a08 Compare October 15, 2023 15:41

karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2023

RainbowMango reviewed Oct 16, 2023

View reviewed changes

Vacant2333 force-pushed the add-resource-deletion-protection-by-label branch from 76c6a08 to f915995 Compare October 17, 2023 05:46

[feat] namespace deletion protection

1df2420

Signed-off-by: Vacant2333 <vacant2333@gmail.com>

Vacant2333 force-pushed the add-resource-deletion-protection-by-label branch from f915995 to 1df2420 Compare October 17, 2023 06:55

RainbowMango reviewed Oct 17, 2023

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 17, 2023

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 18, 2023

karmada-bot merged commit 557348f into karmada-io:master Oct 18, 2023

Vacant2333 mentioned this pull request Oct 20, 2023

[e2e] add resource deletion protection e2e test #4156

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat] Resource Deletion Protection #4074

[Feat] Resource Deletion Protection #4074

Vacant2333 commented Sep 18, 2023 •

edited by RainbowMango

Loading

codecov-commenter commented Sep 18, 2023 •

edited

Loading

XiShanYongYe-Chang commented Sep 18, 2023

chaunceyjiang left a comment

XiShanYongYe-Chang left a comment

XiShanYongYe-Chang left a comment

XiShanYongYe-Chang commented Oct 11, 2023

chaunceyjiang left a comment

RainbowMango left a comment

RainbowMango Oct 13, 2023

RainbowMango left a comment

RainbowMango Oct 16, 2023

Vacant2333 Oct 16, 2023

RainbowMango Oct 17, 2023 •

edited

Loading

RainbowMango left a comment

karmada-bot commented Oct 17, 2023

XiShanYongYe-Chang commented Oct 18, 2023

	- name: resourcedeletionprotection.karmada.io
	- name: resource deletion protection

[Feat] Resource Deletion Protection #4074

[Feat] Resource Deletion Protection #4074

Conversation

Vacant2333 commented Sep 18, 2023 • edited by RainbowMango Loading

codecov-commenter commented Sep 18, 2023 • edited Loading

Codecov Report

XiShanYongYe-Chang commented Sep 18, 2023

chaunceyjiang left a comment

Choose a reason for hiding this comment

XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

XiShanYongYe-Chang commented Oct 11, 2023

chaunceyjiang left a comment

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

RainbowMango Oct 13, 2023

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

RainbowMango Oct 16, 2023

Choose a reason for hiding this comment

Vacant2333 Oct 16, 2023

Choose a reason for hiding this comment

RainbowMango Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Oct 17, 2023

XiShanYongYe-Chang commented Oct 18, 2023

Vacant2333 commented Sep 18, 2023 •

edited by RainbowMango

Loading

codecov-commenter commented Sep 18, 2023 •

edited

Loading

RainbowMango Oct 17, 2023 •

edited

Loading