Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: set default mount permissions for SMB to 0775 #2310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ The following table lists the configurable parameters of the latest Azure File C
| `linux.kubelet` | configure kubelet directory path on Linux agent node node | `/var/lib/kubelet` |
| `linux.kubeconfig` | configure kubeconfig path on Linux agent node | '' (empty, use InClusterConfig by default) |
| `linux.distro` | configure ssl certificates for different Linux distribution(available values: `debian`, `fedora`) |
| `linux.mountPermissions` | mounted folder permissions | `0777`
| `linux.mountPermissions` | mounted folder permissions | `0775`
| `linux.enableRegistrationProbe` | enable [kubelet-registration-probe](https://github.com/kubernetes-csi/node-driver-registrar#health-check-with-an-exec-probe) on Linux driver config | `true`
| `linux.tolerations` | linux node driver tolerations |
| `linux.affinity` | linux node pod affinity | `{}` |
Expand Down
4 changes: 2 additions & 2 deletions deploy/example/kata-cc/storageclass-azurefile-kata-cc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ parameters:
reclaimPolicy: Delete
volumeBindingMode: Immediate
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- mfsymlinks
- cache=strict # https://linux.die.net/man/8/mount.cifs
- nosharesock # reduce probability of reconnect race
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ parameters:
reclaimPolicy: Delete
volumeBindingMode: Immediate
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- uid=0
- gid=0
- mfsymlinks
Expand Down
2 changes: 1 addition & 1 deletion deploy/example/nginx-pod-azurefile-inline-volume.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ spec:
volumeAttributes:
shareName: EXISTING_SHARE_NAME # required
secretName: azure-secret # required
mountOptions: "dir_mode=0777,file_mode=0777,cache=strict,actimeo=30,nosharesock" # optional
mountOptions: "dir_mode=0775,file_mode=0775,cache=strict,actimeo=30,nosharesock" # optional
4 changes: 2 additions & 2 deletions deploy/example/pv-azurefile-csi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ spec:
persistentVolumeReclaimPolicy: Retain # if set as "Delete" file share would be removed in pvc deletion
storageClassName: azurefile-csi
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- uid=0
- gid=0
- mfsymlinks
Expand Down
4 changes: 2 additions & 2 deletions deploy/example/storageclass-azurefile-csi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ parameters:
reclaimPolicy: Delete
volumeBindingMode: Immediate
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- mfsymlinks
- cache=strict # https://linux.die.net/man/8/mount.cifs
- nosharesock # reduce probability of reconnect race
Expand Down
4 changes: 2 additions & 2 deletions deploy/example/storageclass-azurefile-existing-share.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ parameters:
reclaimPolicy: Delete
volumeBindingMode: Immediate
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- uid=0
- gid=0
- mfsymlinks
Expand Down
4 changes: 2 additions & 2 deletions deploy/example/storageclass-azurefile-large-scale.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ parameters:
reclaimPolicy: Delete
volumeBindingMode: Immediate
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- uid=0
- gid=0
- mfsymlinks
Expand Down
4 changes: 2 additions & 2 deletions deploy/example/storageclass-azurefile-secret.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ parameters:
csi.storage.k8s.io/controller-expand-secret-name: azure-secret
csi.storage.k8s.io/controller-expand-secret-namespace: default
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- uid=0
- gid=0
- mfsymlinks
Expand Down
4 changes: 2 additions & 2 deletions docs/driver-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ enableMultichannel | specify whether enable [SMB multi-channel](https://learn.mi
--- | **Following parameters are only for NFS protocol** | --- | --- |
allowSharedKeyAccess | Allow or disallow shared key access for storage account created by driver | `true`,`false` | No | `true`
rootSquashType | specify root squashing behavior on the share. The default is `NoRootSquash` | `AllSquash`, `NoRootSquash`, `RootSquash` | No |
mountPermissions | mounted folder permissions. The default is `0777`, if set as `0`, driver will not perform `chmod` after mount | `0777` | No |
mountPermissions | mounted folder permissions. The default is `0775`, if set as `0`, driver will not perform `chmod` after mount | `0775` | No |
--- | **Following parameters are only for vnet setting, e.g. NFS, private endpoint** | --- | --- |
vnetResourceGroup | specify vnet resource group where virtual network is | existing resource group name | No | if empty, driver will use the `vnetResourceGroup` value in azure cloud config file
vnetName | virtual network name | existing virtual network name | No | if empty, driver will use the `vnetName` value in azure cloud config file
Expand Down Expand Up @@ -108,7 +108,7 @@ nodeStageSecretRef.name | secret name that stores storage account name and key |
nodeStageSecretRef.namespace | secret namespace | k8s namespace | Yes |
--- | **Following parameters are only for NFS protocol** | --- | --- |
volumeAttributes.fsGroupChangePolicy | indicates how volume's ownership will be changed by the driver, pod `securityContext.fsGroupChangePolicy` is ignored | `OnRootMismatch`(by default), `Always`, `None` | No | `OnRootMismatch`
volumeAttributes.mountPermissions | mounted folder permissions. The default is `0777` | | No |
volumeAttributes.mountPermissions | mounted folder permissions. The default is `0775` | | No |

- create a Kubernetes secret for `nodeStageSecretRef.name`
```console
Expand Down
2 changes: 1 addition & 1 deletion docs/install-csi-driver-master.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ curl -skSL https://raw.githubusercontent.com/kubernetes-sigs/azurefile-csi-drive
git clone https://github.com/kubernetes-sigs/azurefile-csi-driver.git
cd azurefile-csi-driver
git checkout master
./deploy/install-driver.sh master local
./deploy/uninstall-driver.sh master local
```
4 changes: 2 additions & 2 deletions docs/workload-identity-static-pv-mount.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ spec:
persistentVolumeReclaimPolicy: Retain
storageClassName: azurefile-csi
mountOptions:
- dir_mode=0777
- file_mode=0777
- dir_mode=0775
- file_mode=0775
- uid=0
- gid=0
- mfsymlinks
Expand Down
4 changes: 2 additions & 2 deletions pkg/azurefile/azurefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ const (
actimeo = "actimeo"
noResvPort = "noresvport"
mfsymlinks = "mfsymlinks"
defaultFileMode = "0777"
defaultDirMode = "0777"
defaultFileMode = "0775"
defaultDirMode = "0775"
defaultActimeo = "30"

// See https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-shares--directories--files--and-metadata#share-names
Expand Down
2 changes: 1 addition & 1 deletion pkg/azurefile/azurefile_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (o *DriverOptions) AddFlags() *flag.FlagSet {
fs.BoolVar(&o.EnableGetVolumeStats, "enable-get-volume-stats", true, "allow GET_VOLUME_STATS on agent node")
fs.BoolVar(&o.EnableKataCCMount, "enable-kata-cc-mount", false, "enable Kata Confidential Containers mount")
fs.BoolVar(&o.AppendMountErrorHelpLink, "append-mount-error-help-link", true, "Whether to include a link for help with mount errors when a mount error occurs.")
fs.Uint64Var(&o.MountPermissions, "mount-permissions", 0777, "mounted folder permissions")
fs.Uint64Var(&o.MountPermissions, "mount-permissions", 0775, "mounted folder permissions")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a breaking change. If I have an app that counts with 777 as the default and I update to this code, my application breaks. This needs to be announced properly, at least with a "breaking change" warning in the release notes.

I would personally also bump the major version, but I am not sure what are rules in this repo.

fs.StringVar(&o.FSGroupChangePolicy, "fsgroup-change-policy", "", "indicates how the volume's ownership will be changed by the driver, OnRootMismatch is the default value")
fs.Float64Var(&o.KubeAPIQPS, "kube-api-qps", 25.0, "QPS to use while communicating with the kubernetes apiserver.")
fs.IntVar(&o.KubeAPIBurst, "kube-api-burst", 50, "Burst to use while communicating with the kubernetes apiserver.")
Expand Down
2 changes: 1 addition & 1 deletion pkg/azurefile/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
if !strings.HasSuffix(diskName, vhdSuffix) {
return nil, status.Errorf(codes.Internal, "diskname could not be empty, targetPath: %s", targetPath)
}
cifsMountFlags = []string{"dir_mode=0777,file_mode=0777,cache=strict,actimeo=30", "nostrictsync"}
cifsMountFlags = []string{"dir_mode=0775,file_mode=0775,cache=strict,actimeo=30", "nostrictsync"}
cifsMountPath = filepath.Join(filepath.Dir(targetPath), proxyMount)
}

Expand Down