From a1eba060eead371e0022d031c7c7eb27603ae57a Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Thu, 17 Nov 2022 12:07:00 +0000 Subject: [PATCH 1/2] fix: default mountPermissions issue --- charts/README.md | 2 +- charts/latest/csi-driver-nfs-v0.0.0.tgz | Bin 3728 -> 3727 bytes charts/latest/csi-driver-nfs/values.yaml | 2 +- cmd/nfsplugin/main.go | 2 +- pkg/nfs/controllerserver.go | 12 +++++++----- pkg/nfs/nodeserver.go | 11 ++--------- 6 files changed, 12 insertions(+), 17 deletions(-) diff --git a/charts/README.md b/charts/README.md index 2318974e..a156e8e9 100644 --- a/charts/README.md +++ b/charts/README.md @@ -38,7 +38,7 @@ The following table lists the configurable parameters of the latest NFS CSI Driv |---------------------------------------------------|------------------------------------------------------------|-------------------------------------------------------------------| | `customLabels` | optional extra labels to k8s resources deployed by chart | `{}` | | `driver.name` | alternative driver name | `nfs.csi.k8s.io` | -| `driver.mountPermissions` | mounted folder permissions name | `0777` +| `driver.mountPermissions` | default mounted folder permissions | `0` | `feature.enableFSGroupPolicy` | enable `fsGroupPolicy` on a k8s 1.20+ cluster | `true` | | `feature.enableInlineVolume` | enable inline volume | `false` | | `kubeletDir` | alternative kubelet directory | `/var/lib/kubelet` | diff --git a/charts/latest/csi-driver-nfs-v0.0.0.tgz b/charts/latest/csi-driver-nfs-v0.0.0.tgz index ffc385cf0220ee8edf110f12840b4183b9cfceb1..0c0d0bcb691c74842bda23fa14cc5dbfbfc21cdf 100644 GIT binary patch delta 3702 zcmV-+4vF!Q9giK5Jb&GD+qSaLGyjUcav##%kovNnUe5R-wVfVM8^>e0Idi7d@j&EC z!Z8T204Q5^eE;@407!|XC`b8UQF7F% zJWdh}p{i2ebWXk#bb^lewO9Po%B^PqQ^GP-kL&=}*#Azy^R90Hy?5Og`+trWWlD?W zH!?(~Lb$&1Xn#V+7@DmM2enBF{Q?Hpls$s;+QHe zXTil!%CJ*r!Kih*AW&sw+7Eg`XER0^ogzo2PNW#(-GBKghNotDjf-CJZc{F}h|#&+ zQ#Tp8r%Vi?!ro;;YKgC%pu3(gPobRB2;WDM$hbEAL=uU{5VXt?w&0%=NUPwbc_B=TxQGF>MfQxJ|}z3>B)(ZI)vW;?C$hTH!=PVr^rIs@c=G`yr6vi(Fn}0tJT&? z+VTKmt_sN<;D}^Q18f1A63UE#z{RQg%eS!4GQ;Z!DwX~x)sue+rH`@p!UG_q5#?0R ztbea_hBZKjkrpz4qyZ}HbexN*n(PL4-oLMzHELT;rJi?_SBKrNs8{`m=+)L1oZ<*2@(4`~6dX=FNuyOUt7%oZ`(@yKc65= zQK~?v4~2r(*nhXzuj9YF+ue6B_WvC1`nvrFrZfpbp@tD}q_Pf6JA;uBq2rY9iHr-m_FW%T(HE>l1Yoozuhd7#^ zpQb1!jS;?><1mk4w{JhvWRQ(U^a@)3f*A7Hes-)e3t(9kVS+4KWR1|sm+S6`xz8qn zTJ2jxXf=74#xV)uWQ?&toPX6u6$r$tfq8aweGP>bmW3Fs95)Bp?QBlA@8>vvKFmlZ zP?NUZtV-8>G5+F7G4hKA3ye2z{dWsm=dGNUktQ9@x2_B!f)zGs@h_6F<{n>w@))^> zo)y0%XLhr;v0vLVzK^!j|1X0__pS=u?*F@;o&HYM|MxmO-QJ7;e}9hV`@VNUc^txl zP4yDw+A9|YNt&wmwCg3PNlY|ZfD*=G6b8Of67Jkd-peL)u4_|{XM6?a5zFG5V-Cu0 z;^xLvDOy-|fnGMxUc7f46ltS79VaFc{B7>-O(SW#ejNoy>IP|xls1nkqp5CeVNW>+ za~W>1?p;UW4vYd6Vt+EN=B4Y0>IL+IC0a!&x}S}NoDg;J(b~S%rkG>`4Utsnn)qlAiwhp1ys*pw=mjr_%9sh;gKqLkAFHB%ZBKR?m>k))bNgx$(P3AJ3% zu0WZ0;AxJCmEb$Sg!ncUFmrRwGq|#K2352@Sd<0f<|bU%Ta$5~-OXK&9GCcVytVx4 zUaz(E&^O$YIDeL?)ZP!T-ya;D9u5XSHgsf2VwAq!Ln`?C9vc7!4MgxtOX52p;ul_b z&v>A6XXMlR0-f;l1ytUFL)AktnUVPyC=p%M_vNHiEnO$_MP;?@=>ev;?q`c)OB>*7 zi$qTxA2kjg_l7Uj{=#(vlwovhYxM-Z96|GrHFn_(@PAyJyS*~Qh0qFQ-mliWZ&BC^ zmcLG4tDQGf*Q#Q!+BnzKa_wuHpvd%IUoCZTWez_TQtvfKpel=tvPgA%9kuCR?@qc? zrF|dOxkWilTdi65P@M|5Yc+?D)F$d;@}xCNV!AA=hQcVbzF%;SSGp>gRMIJBIL3Hh zgkQ9Xq<>^c8PybP@&q7p9Ks*1!M_IQ?+-p69k>3hbRdk0%aelmp^6A2?hajTmi2pY zR<&x!o4vF@U8SOVF|XNen)piNy&qm5e>ylkKRRgg;K<`tP_FlWc)dS3IzK!MFUW1?K+%w@?pN2!d{tFYZ(<@1aeThg0(KH_S* z4|8%wG;%%uuDwg8&>|8ngtPsV(z_Mct*n3uWtqgYi9|IKEOzziMVn}y{(`zL+tj(V z6@P5lg*Lr2**qoe02!GLFcLggA@n(VYQ zZV186P1_J?9419d#Fp{@)!;sd?TFSSnw;Wa8I>l{DsBxr)BcT)Fq%aSKZ+RV zw}DO!OG>*dur(YazfBn|Uzg0ih7X771QV2mc~xBFR!WsinYQ~#Rjz4%pbRSnzkl}r zJ+!U0f0Kti@cRI3@c(YFzg?^UzU%G0-2Z=;R>J=YCV~&}9`L`_k*5Xzvot0e2NfvZ zyWZD3HLU!O)rxPRw(zL66l3GCuHfLJ-}g~!c&QHDE!lag3*1&0 zcvvy;iJkM;E=k(>V=|tWoS?t z(f|}2B`SFgqgX{sR{d)oj|?3@4aWsq`1rimzSUIO7t2CvH}ayKyMMZRDi~H)YI~DR zPlTj@TYp-r=iQ3rSC)!b+T_%%d=b@N>)m)+GGi{#`an`@D{0CcZ1VF=DWLz+O6T>y zJV582rYMKD!x(kzEwmU__NV<`r}^N-Vm`$t6rmk=j|n42XtMFFt{~AgSG9!NG;`h~ zVnnG8*SX=EaEe^tLw|DlYTiOCT{YeMWkre_>v+@!J&0f=gp4UKTR_ts)^49hXpOt? zN-0hA{pl_0aV6vanev$O@l)7%?Vh#ujgiicywSfdGQ7J)g3bJ^@kaTP zdD$i2-@n0p7jF^tf5f*HYx!R=L6*=l7ZM+Dj-1|9`K)`;z}XN85rEqKz5Y zZ(X^acYpO|m{De20i-0lAY)X4w*_Ytsz7C_MTkI6kTDoDF?7Xo%Exa(A|skk(Vi7c z&k2vcE#T8j|QDOYL{xmG>r4m#OzA z7?KO`O`wz1d-K2E7JMd>icG=L!J+bkR0`wtynx0?+OD4z|Mr5ZibRaUw3qhM UzNhwo0RRC1{}g=@698NQ0MbNExBvhE delta 3687 zcmV-t4w&(e9grQ6Jb&wR+qSaLXZ{s?<$g$WL+as2dO70{sqOT5+BhD|&6zWujt3%F z5{^ND1wh%VGPp#AB>6A!IUKI z)#J5Jr_`PPMHOx*6Ex;m66GI&<%CA~K8i%fwc#g{NHm6^WrnZ?{~RN?pWi~>7l`Yu z;EFN^1I&lSHj+$57C;|0XaPgQ6apQZhj2x>eV8dUze^BTJTUVKuBBiMrHeXzsQmde zUlG`X6MvE*h#~ZI4sv!4xzhAP8sf3%$pMM#OYs0)WHaH+bJ7dT4}(b2T;d);V(6cu zOsF#2r$Xqwd-u-shDdZKtr8(81BM^^zetg#g>>_&+`D7WD96u&Wo8a54bQvG2FOq! zQW-*fLS&oKK|6owc_nGgm|K*JXBCFgh^P?EHh&ux&^Vf&Pg4xx4Bg62w~T+{ABoaN zypDtX;en%e$_U5#s|HgUBVl@cDx?mf_dk2PJ=0B$e!~f}5OzI)DxbM)dRTOO3#vd=~#f(@0bzlM&{UYj)&}CRCS#OM;1Y zUpt-7N9q-B?XL_~xmT2CA#^**!mwU3s(;a2FPp6Uz{yJCaoH5T2Tb9aZCbQs^YkgL zBO%wn^BEoIBC004ft_#P*UTEVt)^1XyUEKV?^o2T?T6^q))t)M5GC>mO&Am$P5euQ zL{P^4Yn4Yw6B6OdL~)Y^P(A{i9V$aY;xL4<)~O2H?W!M%MWouAs7qyB%8-sSiGO}7 zVjG1BRo^OfysiYI4u}DXzZEtJKWud1TV|~X^2!H(PZGoQ~Ln|L@s5q8eFXzg8 zwc7blW0Kl)fBWRq`B6X6*ZSdetiyk{ciz>`f4jTedoSm|=V)*~#yYA)6^u}8WA8Dh zOiVF04#8&Lk&70A=ba0P#u#1JM}MaxR-iP|nR*K;L!!`RjiKiO7(Wqd2EK3XSfwPw z{hJ#I&d~M?PKu{+duw=b#C%`nfn&c#(zKSfP_8uL4TRwKwgtX_MRYX&+G75H+m_n@ z#TZ$NQUyAFC=|5D{=2>HI{ve_)9b$2|8umPoAw)+&?E$f8itgio~F2;7=N)uV+`R< z`}WrJY`3H9l<*kz7;P9fr)u*#vNZ$S@8-sb7>AT2w9L8KbZdb>e`6EXz*QNnjRv0` z;%It)nxK?4M)+dR!aRb#ZTpcX{cJd-*U<9k#E{4Kvtx}}0L!8XV`RxZWrRk)Tz5yz zeKy(Ca^Dg{tI4}Gj!6h7BY%wj!L&B2Kp>V4%u}128z`)>EW}{xxEa81XLGWBKg03! zVMZ!}8n^9cRl4Sj@#jy9kzXuQV7zhbzgy6{XyvqwH0f}*b!7+d~Iq^W98x?X~s#6*)hB4HdxVc>Hm;m)1py=*e)x;Eu_##c}tu`I4R z=Ai5*Zf`x6qJ?GW&}H-N#e2s=kv6)cabgm|-)7$4G?J$4*HHkZZjiPxY4eyel#I-g6Z0z<($}AtuvmUb=pwo>N~qYd3iCh0!xMf0`32Idb=4>@cP)Auscy z0Bn^&dmu~+;E1sW94u}Qd}x1hC^W>hs7{msuVy*SI?U&VE2_s$ zQx(wGe5M)LRhC^%NDJbrKw`74t_F9ZLSU3<*R%Ka=vz|$Ie%bZQk~V9bqK9ig*5WG zGi2(a=*W=7D1Ez!RPgmZHUJ75h~TxB#CJZ# zFTC!)@j&Iy$S3s$I^pLFsJsIQsfS=PBl9m%BD$>a%W0`vx=!Tt%4*ru159ku&sM}1 zHo(;ui5@#XY8*Q54PU7Jx$6WdW9Zh_>Ir%|g617-?0>=+;JG$;du4_Tp%t*aU#)eo zP}mBPzfNDPoi|h0s$wqNIM>s1?Q5E#$n^fUTIk?v9DXXK-fxaTRTk$pk?Qt3YLorm zoph&4`#!3(LOD!Zu37g`oeH;WHHQz?ChB7Hq%}%nx-6=O!YH%8UvQ1rx+<7d(g|fa z!gyAJpMSTAq+~!D)f8*;1R!x7!XK^vzxo&N4?iBCwEnDgAdHEt(}MVsiU=d_4qa`Q z^?PqtwQR?my|h1BrlNTzui0&y_)6pbA6}n)Iy|~KK5X*f$m3K{uJ?a2|Z`FVa|Qcfx^kEAxB;+B6IGvYvVQIojU%v^K$?9d7AJ06}T_IELz+jopf__@SuO} z>bt9fz-n-x!FEJz5{=LBuZ(|6lV}xJ2AygDMu!+pBZePEjI*mir-mh^-4)mx4v}A_ z3>NQ7=3c{xqjZc3O2WJEPFk)A#59__2{nB4XTcCRMY?dqN(&YmKd{*>^R-hlpV*aMv`j z8Ae-<7T7O1&eCM}>#}9IS%3Zg=c5aY8J@iVc=T+j;Rg6}gC&1Ft4Em+u59ya66LP< z^-c{dzhkxH8>lTjYAwasIIJr;xajwNlp0>D19wYyUg`p?>H-fd20pQK{@NvpySete z*Ap6rbH7?bQ+BYbxbR6H6kogEQ07lv<9NurFN*K-Z8R4wi~V048~YyGQv6pANaW}G z`=t19XM4BXS-^jPw_otzXK6LGuzn@h-NN;6Z@qeXYBk&;#a zTE`Uk@X{K``C zTAQ4jl`o>&YrPvUOJ>XkS|3PCZ6!^agH3*(DFyUDTIqkR-j@gHywe2bz;+m+ZoP#T zqssoY-|I9VoLJ1K*n}dqw8Ep zU(H)+rOT#UzpO}6V;zsWpa&6*gpe`iWeaGU!`kiB2(5AVT`8q$zCXQrJ+5TjKT{r4 zK6(lpuibw$_Xqh@8`tNu!M`!mxsf;e*Lj9_mq@Uge>L7HKQb@6#QXaULH|d5 zTd|h^1!H6h9dRM?@#a{2|EIUN@b~|AIxqR(bF?iuCEA#Q{nnM+dG{w@1{r0>6+lX& zOEN+gcw2BjrV3P+T7(GH7#V{R69ZQqr+oAlBr<=Z=>+XrvGknq*xLe*?wk*=Q;9=* zjj^jv{@0HIeBx|s{~Ar|Sb`KKFv>CTg2VnrUki!e795Bq5ga}r^dY8FdBKS4w*B9+ z?*)UuW!wH=JQ$DK=D*@sHR0_!p#h05v($d~U3qT;b(MN=f&sbo-UK>Hy*K~sZNX 0 { + // Reset directory permissions because of umask problems + if err = os.Chmod(internalVolumePath, os.FileMode(mountPermissions)); err != nil { + klog.Warningf("failed to chmod subdirectory: %v", err.Error()) + } } setKeyValueInMap(parameters, paramSubDir, nfsVol.subDir) diff --git a/pkg/nfs/nodeserver.go b/pkg/nfs/nodeserver.go index e7aae1c7..690c3f0a 100644 --- a/pkg/nfs/nodeserver.go +++ b/pkg/nfs/nodeserver.go @@ -60,7 +60,6 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis subDirReplaceMap := map[string]string{} mountPermissions := ns.Driver.mountPermissions - performChmodOp := (mountPermissions > 0) for k, v := range req.GetVolumeContext() { switch strings.ToLower(k) { case paramServer: @@ -82,15 +81,9 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis case mountPermissionsField: if v != "" { var err error - var perm uint64 - if perm, err = strconv.ParseUint(v, 8, 32); err != nil { + if mountPermissions, err = strconv.ParseUint(v, 8, 32); err != nil { return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid mountPermissions %s", v)) } - if perm == 0 { - performChmodOp = false - } else { - mountPermissions = perm - } } } } @@ -138,7 +131,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return nil, status.Error(codes.Internal, err.Error()) } - if performChmodOp { + if mountPermissions > 0 { if err := chmodIfPermissionMismatch(targetPath, os.FileMode(mountPermissions)); err != nil { return nil, status.Error(codes.Internal, err.Error()) } From fcb4ceb0b694246735239cba617168b24c7d7453 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Thu, 17 Nov 2022 12:09:30 +0000 Subject: [PATCH 2/2] doc: fix mountPermissions doc --- docs/driver-parameters.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/driver-parameters.md b/docs/driver-parameters.md index cef75b5f..e71282b7 100644 --- a/docs/driver-parameters.md +++ b/docs/driver-parameters.md @@ -9,7 +9,7 @@ Name | Meaning | Example Value | Mandatory | Default value server | NFS Server address | domain name `nfs-server.default.svc.cluster.local`
or IP address `127.0.0.1` | Yes | share | NFS share path | `/` | Yes | subDir | sub directory under nfs share | | No | if sub directory does not exist, this driver would create a new one -mountPermissions | mounted folder permissions. The default is `0777`, if set as `0`, driver will not perform `chmod` after mount | | No | +mountPermissions | mounted folder permissions. The default is `0`, if set as non-zero, driver will perform `chmod` after mount | | No | ### PV/PVC usage (static provisioning) > [`PersistentVolume` example](../deploy/example/pv-nfs-csi.yaml) @@ -18,7 +18,7 @@ Name | Meaning | Example Value | Mandatory | Default value --- | --- | --- | --- | --- volumeAttributes.server | NFS Server address | domain name `nfs-server.default.svc.cluster.local`
or IP address `127.0.0.1` | Yes | volumeAttributes.share | NFS share path | `/` | Yes | -volumeAttributes.mountPermissions | mounted folder permissions. The default is `0777` | | No | +volumeAttributes.mountPermissions | mounted folder permissions. The default is `0`, if set as non-zero, driver will perform `chmod` after mount | | No | ### Tips #### `subDir` parameter supports following pv/pvc metadata conversion