Added checks to make sure delete-access-point should not delete entir…#1201
Added checks to make sure delete-access-point should not delete entir…#1201mskanth972 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mskanth972 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| } | ||
|
|
||
| klog.V(2).Infof("Access point root directory : %s", accessPoint.AccessPointRootDir) | ||
| if accessPoint.AccessPointRootDir == "/" { |
There was a problem hiding this comment.
What happens when accessPoint.AccessPointRootDir is an empty string? (The field seems not mandatory when creating an AP, so could in rare cases be empty).
According to my understanding the efs-plugin mounts the entire filesystem in /var/lib/csi/pv/<fsap-id> and later calls os.RemoveAll(target, accessPoint.AccessPointRootDir). Now if accessPoint.AccessPointRootDir is empty, the left-over path for os.RemoveAll is /var/lib/csi/pv/<fsap-id> which represents the entire filesystem. I'm I right with that?
There was a problem hiding this comment.
We pull AccessPointRootDir from an API call to the AWS EFS console API:
aws-efs-csi-driver/pkg/driver/controller.go
Line 374 in 8b78393
The API layer has empty access point root dirs default to the root path, so an empty access point root dir shouldn't be possible.
But, I agree, we should validate our assumptions and add a check, just in case.
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
PR needs rebase. DetailsInstructions 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. |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
|
/remove-lifecycle rotten |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
|
/remove-lifecycle rotten Xref: |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
|
/remove-lifecycle rotten @dobsonj @mskanth972 is anything missing here or why did you not merge it? |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
|
/remove-lifecycle rotten |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
…e EFS
Is this a bug fix or adding new feature?
This is a bug fix
What is this PR about? / Why do we need it?
Added checks to make sure delete-access-point should not delete entire EFS
What testing is done?