feat(webhook): add validation for namspace delete requests #1754
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR adds validation on deletion of openebs namespace
to avoid
- loss of data in case of accidental deletion of
namespace
- stale resources with finalizers in openebs namespace which
may be stuck on namespace deletion.
Signed-off-by: shubham <[email protected]>
shubham14bajpai
requested review from
prateekpandey14,
kmova and
mittachaitu
kmova
reviewed
Sep 14, 2020
pkg/webhook/webhook.go
Outdated
| if req.Operation == v1beta1.Delete { | ||
| return wh.validateNamespaceDeleteRequest(req) | ||
| } | ||
| klog.V(2).Info("Admission wehbook for PVC module not " + |
There was a problem hiding this comment.
Can you also check that this rule only applies to openebs namespace? Other namespace requests shouldn't get impacted by this.
Add a test to verify that when BDCs are present, a test namespace can be created and deleted without any issues.
There was a problem hiding this comment.
Fixed the log message and added the manual test result to the PR description.
Signed-off-by: shubham <[email protected]>
mittachaitu
reviewed
Sep 14, 2020
Signed-off-by: shubham <[email protected]>
kmova
approved these changes
Sep 14, 2020
shubham14bajpai
added a commit
to shubham14bajpai/maya
that referenced
this pull request
Sep 14, 2020
…rchive#1754) This PR adds validation on deletion of openebs namespace to avoid - loss of data in case of accidental deletion of namespace - stale resources with finalizers in openebs namespace which may be stuck on namespace deletion. Signed-off-by: shubham <[email protected]>
shubham14bajpai
added a commit
to shubham14bajpai/maya
that referenced
this pull request
Sep 14, 2020
…rchive#1754) This PR adds validation on deletion of openebs namespace to avoid - loss of data in case of accidental deletion of namespace - stale resources with finalizers in openebs namespace which may be stuck on namespace deletion. Signed-off-by: shubham <[email protected]>
shubham14bajpai
added a commit
to shubham14bajpai/maya
that referenced
this pull request
Sep 14, 2020
…rchive#1754) This PR adds validation on deletion of openebs namespace to avoid - loss of data in case of accidental deletion of namespace - stale resources with finalizers in openebs namespace which may be stuck on namespace deletion. Signed-off-by: shubham <[email protected]>
This was referenced Sep 14, 2020
kmova
pushed a commit
that referenced
this pull request
Sep 14, 2020
This PR adds validation on deletion of openebs namespace
to avoid
- loss of data in case of accidental deletion of
namespace
- stale resources with finalizers in openebs namespace which
may be stuck on namespace deletion.
Signed-off-by: shubham <[email protected]>
kmova
pushed a commit
that referenced
this pull request
Sep 14, 2020
This PR adds validation on deletion of openebs namespace
to avoid
- loss of data in case of accidental deletion of
namespace
- stale resources with finalizers in openebs namespace which
may be stuck on namespace deletion.
Signed-off-by: shubham <[email protected]>
kmova
pushed a commit
that referenced
this pull request
Sep 14, 2020
This PR adds validation on deletion of openebs namespace
to avoid
- loss of data in case of accidental deletion of
namespace
- stale resources with finalizers in openebs namespace which
may be stuck on namespace deletion.
Signed-off-by: shubham <[email protected]>
| if err != nil { | ||
| response.Allowed = false | ||
| response.Result = &metav1.Status{ | ||
| Message: fmt.Sprintf("error listing BDC in namespace %s: %v", req.Name, err.Error()), |
There was a problem hiding this comment.
Failure is here in case of helm chart deletion
| @@ -421,6 +426,9 @@ func (wh *webhook) validate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionRespo | |||
| response.Allowed = true | |||
| klog.Info("Admission webhook request received") | |||
| switch req.Kind.Kind { | |||
| case "Namespace": | |||
| klog.V(2).Infof("Admission webhook request for type %s", req.Kind.Kind) | |||
| return wh.validateNamespace(ar) | |||
There was a problem hiding this comment.
this validate kicks in for any namespace deletion , can we use the webhook namespace itself to get the openebs namespace and skip for other namespace validations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does?:
This PR adds validation on the deletion of openebs namespace
to avoid
namespace
maybe stuck on namespace deletion.
Does this PR require any upgrade changes?:
Yes already handled in the PR itself. Tested 1.10 to latest upgrade: namespace gets added to validatingwebhookconfigurations automatically.
If the changes in this PR are manually verified, list down the scenarios covered::
Local test performed:
Both jiva and cstor volume exist
Only cstor volume exists
Only jiva volume exists
After deleting all volumes and pools
mayadata:setup$ kubectl delete ns openebs namespace "openebs" deletedCreated a cstor & jiva volume in test namespace and deleted the namespace. The webhook did not impact the deletion and cleanup was successful.
TODO
Add unit test for the added webhook validation.
Checklist:
<type>(<scope>): <subject>