Skip to content

Commit 8cf2b83

Browse files
committed
fix: uwirtefile with lock invoid conficts from different reqs
1 parent 1f86359 commit 8cf2b83

File tree

6 files changed

+83
-36
lines changed

6 files changed

+83
-36
lines changed

pkg/mounter/proxy/server/ossfs/driver.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ func (h *Driver) Mount(ctx context.Context, req *proxy.MountRequest) error {
9090
klog.InfoS("ossfs exited", "mountpoint", target, "pid", pid)
9191
}
9292
ossfsExited <- err
93+
// Note: No need to clean up credential files since after rotation support,
94+
// files are stored in fixed paths and won't generate multiple copies that
95+
// could lead to files leakage.
9396
close(ossfsExited)
9497
}()
9598

@@ -98,16 +101,6 @@ func (h *Driver) Mount(ctx context.Context, req *proxy.MountRequest) error {
98101
case err := <-ossfsExited:
99102
// TODO: collect ossfs outputs to return in error message
100103
if err != nil {
101-
if passwdFile != "" {
102-
if errRemove := os.Remove(passwdFile); errRemove != nil {
103-
klog.ErrorS(err, "Remove passwd file", "mountpoint", target, "path", passwdFile)
104-
}
105-
}
106-
if tokenDir != "" {
107-
if errRemove := os.RemoveAll(tokenDir); errRemove != nil {
108-
klog.ErrorS(err, "Remove token directory", "mountpoint", target, "dir", tokenDir)
109-
}
110-
}
111104
return false, fmt.Errorf("ossfs exited: %w", err)
112105
}
113106
return false, fmt.Errorf("ossfs exited")

pkg/mounter/proxy/server/ossfs/utils.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ import (
1111
"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/mounter/utils"
1212
)
1313

14-
1514
// rotateTokenFiles rotates (or initializes) token files
1615
func rotateTokenFiles(dir string, secrets map[string]string) (rotated bool, err error) {
1716
if secrets == nil {
1817
return false, nil
1918
}
2019
// token
20+
var fileUpdate bool
2121
tokenKey := []string{oss.KeyAccessKeyId, oss.KeyAccessKeySecret, oss.KeySecurityToken, oss.KeyExpiration}
2222
for _, key := range tokenKey {
2323
val := secrets[filepath.Join(utils.GetPasswdFileName("ossfs"), key)]
@@ -26,12 +26,12 @@ func rotateTokenFiles(dir string, secrets map[string]string) (rotated bool, err
2626
klog.Error(err)
2727
return
2828
}
29-
err = os.WriteFile(filepath.Join(dir, key), []byte(val), 0o600)
29+
fileUpdate, err = utils.WriteFileWithLock(filepath.Join(dir, key), []byte(val), 0o600)
3030
if err != nil {
31-
klog.Errorf("writeFile %s failed %v", key, err)
31+
klog.Errorf("WriteFileWithLock %s failed %v", key, err)
3232
return
3333
}
34-
rotated = true
34+
rotated = fileUpdate || rotated
3535
}
3636
return
3737
}
@@ -51,7 +51,7 @@ func prepareCredentialFiles(target string, secrets map[string]string) (file, dir
5151
}
5252

5353
if passwd := secrets[utils.GetPasswdFileName("ossfs")]; passwd != "" {
54-
err = os.WriteFile(filepath.Join(hashDir, utils.GetPasswdFileName("ossfs")), []byte(passwd), 0o600)
54+
_, err = utils.WriteFileWithLock(filepath.Join(hashDir, utils.GetPasswdFileName("ossfs")), []byte(passwd), 0o600)
5555
if err != nil {
5656
return
5757
}

pkg/mounter/proxy/server/ossfs2/driver.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ func (h *Driver) Mount(ctx context.Context, req *proxy.MountRequest) error {
9797
klog.InfoS("ossfs2 exited", "mountpoint", target, "pid", pid)
9898
}
9999
ossfsExited <- err
100+
// Note: No need to clean up credential files since after rotation support,
101+
// files are stored in fixed paths and won't generate multiple copies that
102+
// could lead to files leakage.
100103
close(ossfsExited)
101104
}()
102105

@@ -105,9 +108,6 @@ func (h *Driver) Mount(ctx context.Context, req *proxy.MountRequest) error {
105108
case err := <-ossfsExited:
106109
// TODO: collect ossfs outputs to return in error message
107110
if err != nil {
108-
if errRemove := os.Remove(passwdFile); errRemove != nil {
109-
klog.ErrorS(err, "Remove configuration file", "mountpoint", target, "path", passwdFile)
110-
}
111111
return false, fmt.Errorf("ossfs2 exited: %w", err)
112112
}
113113
return false, fmt.Errorf("ossfs2 exited")
@@ -150,11 +150,11 @@ func (h *Driver) Mount(ctx context.Context, req *proxy.MountRequest) error {
150150
func (h *Driver) RotateToken(ctx context.Context, req *proxy.RotateTokenRequest) error {
151151
// prepare passwd file
152152
hashDir := utils.GetPasswdHashDir(req.Target)
153-
tokenFiles, err := rotateTokenFiles(hashDir, req.Secrets)
153+
rotated, err := rotateTokenFiles(hashDir, req.Secrets)
154154
if err != nil {
155155
return fmt.Errorf("rotate token files failed: %w", err)
156156
}
157-
if len(tokenFiles) != 0 {
157+
if rotated {
158158
klog.V(4).InfoS("rotate ossfs2 token files")
159159
}
160160
return nil

pkg/mounter/proxy/server/ossfs2/utils.go

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ import (
1212
)
1313

1414
// rotateTokenFiles rotates (or initializes) token files
15-
func rotateTokenFiles(dir string, secrets map[string]string) (rotatedFiles map[string]string, err error) {
15+
func rotateTokenFiles(dir string, secrets map[string]string) (rotated bool, err error) {
1616
if secrets == nil {
17-
return nil, nil
17+
return false, nil
1818
}
19-
// token
20-
rotatedFiles = make(map[string]string)
19+
// tokem
20+
var fileUpdate bool
2121
tokenKey := []string{oss.KeyAccessKeyId, oss.KeyAccessKeySecret, oss.KeySecurityToken}
2222
for _, key := range tokenKey {
2323
val := secrets[filepath.Join(utils.GetPasswdFileName("ossfs2"), key)]
@@ -26,12 +26,12 @@ func rotateTokenFiles(dir string, secrets map[string]string) (rotatedFiles map[s
2626
klog.Error(err)
2727
return
2828
}
29-
err = os.WriteFile(filepath.Join(dir, key), []byte(val), 0o600)
29+
fileUpdate, err = utils.WriteFileWithLock(filepath.Join(dir, key), []byte(val), 0o600)
3030
if err != nil {
31-
klog.Errorf("writeFile %s failed %v", key, err)
31+
klog.Errorf("WriteFileWithLock %s failed %v", key, err)
3232
return
3333
}
34-
rotatedFiles[key] = filepath.Join(dir, key)
34+
rotated = rotated || fileUpdate
3535
}
3636
return
3737
}
@@ -51,7 +51,7 @@ func prepareCredentialFiles(target string, secrets map[string]string) (file, dir
5151
}
5252

5353
if passwd := secrets[utils.GetPasswdFileName("ossfs2")]; passwd != "" {
54-
err = os.WriteFile(filepath.Join(hashDir, utils.GetPasswdFileName("ossfs2")), []byte(passwd), 0o600)
54+
_, err = utils.WriteFileWithLock(filepath.Join(hashDir, utils.GetPasswdFileName("ossfs2")), []byte(passwd), 0o600)
5555
if err != nil {
5656
return
5757
}
@@ -60,13 +60,16 @@ func prepareCredentialFiles(target string, secrets map[string]string) (file, dir
6060
}
6161

6262
// token
63-
tokenFiles, err := rotateTokenFiles(hashDir, secrets)
64-
if len(tokenFiles) != 0 {
63+
token, err := rotateTokenFiles(hashDir, secrets)
64+
if err != nil {
65+
return
66+
}
67+
if token {
6568
dir = hashDir
6669
options = append(options,
67-
fmt.Sprintf("oss_sts_multi_conf_ak_file=%s", tokenFiles[oss.KeyAccessKeyId]),
68-
fmt.Sprintf("oss_sts_multi_conf_sk_file=%s", tokenFiles[oss.KeyAccessKeySecret]),
69-
fmt.Sprintf("oss_sts_multi_conf_token_file=%s", tokenFiles[oss.KeySecurityToken]),
70+
fmt.Sprintf("oss_sts_multi_conf_ak_file=%s", filepath.Join(hashDir, oss.KeyAccessKeyId)),
71+
fmt.Sprintf("oss_sts_multi_conf_sk_file=%s", filepath.Join(hashDir, oss.KeyAccessKeySecret)),
72+
fmt.Sprintf("oss_sts_multi_conf_token_file=%s", filepath.Join(hashDir, oss.KeySecurityToken)),
7073
)
7174
return
7275
}

pkg/mounter/proxy/server/ossfs2/utils_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func TestRotateTokenFiles(t *testing.T) {
6868
// case 1: initialize fiexd AKSK
6969
secrets := map[string]string{OssfsPasswdFile: "testPasswd"}
7070
rotated, err := rotateTokenFiles("/tmp/token-files", secrets)
71-
assert.Len(t, rotated, 0)
71+
assert.False(t, rotated)
7272
assert.NoError(t, err)
7373
err = os.RemoveAll("/tmp/token-files")
7474
klog.ErrorS(err, "Remove token directory", "dir", "/tmp/token-files")
@@ -81,7 +81,7 @@ func TestRotateTokenFiles(t *testing.T) {
8181
filepath.Join(OssfsPasswdFile, oss.KeySecurityToken): "testSecurityToken",
8282
}
8383
rotated, err = rotateTokenFiles("/tmp/token-files", secrets)
84-
assert.Len(t, rotated, 3)
84+
assert.True(t, true)
8585
assert.NoError(t, err)
8686
ak, _ := os.ReadFile(filepath.Join("/tmp/token-files", oss.KeyAccessKeyId))
8787
assert.Equal(t, "testAKID", string(ak))
@@ -100,7 +100,7 @@ func TestRotateTokenFiles(t *testing.T) {
100100
filepath.Join(OssfsPasswdFile, oss.KeySecurityToken): "newSecurityToken",
101101
}
102102
rotated, err = rotateTokenFiles("/tmp/token-files", secrets)
103-
assert.Len(t, rotated, 3)
103+
assert.True(t, rotated)
104104
assert.NoError(t, err)
105105
ak, _ = os.ReadFile(filepath.Join("/tmp/token-files", oss.KeyAccessKeyId))
106106
assert.Equal(t, "newAKID", string(ak))

pkg/mounter/utils/filelock.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package utils
2+
3+
import (
4+
"os"
5+
"sync"
6+
)
7+
8+
var (
9+
// Store the read-write lock corresponding to each file path
10+
fileLocks = sync.Map{}
11+
)
12+
13+
func getFileLock(path string) *sync.RWMutex {
14+
lock, _ := fileLocks.LoadOrStore(path, &sync.RWMutex{})
15+
return lock.(*sync.RWMutex)
16+
}
17+
18+
// WriteFileWithLock safely writes data to file with locking
19+
func WriteFileWithLock(path string, data []byte, perm os.FileMode) (done bool, err error) {
20+
lock := getFileLock(path)
21+
22+
// First try to acquire read lock to check if content is consistent
23+
lock.RLock()
24+
if existingData, err := os.ReadFile(path); err == nil {
25+
// If file exists and content is the same, return directly to avoid redundant write
26+
if string(existingData) == string(data) {
27+
lock.RUnlock()
28+
return false, nil
29+
}
30+
}
31+
lock.RUnlock()
32+
33+
// Content is different or file does not exist, need to write new content
34+
// Acquire write lock
35+
lock.Lock()
36+
defer lock.Unlock()
37+
38+
// Check content again (double-checked locking pattern)
39+
if existingData, err := os.ReadFile(path); err == nil {
40+
if string(existingData) == string(data) {
41+
return false, nil
42+
}
43+
}
44+
45+
// Perform write operation
46+
err = os.WriteFile(path, data, perm)
47+
if err == nil {
48+
done = true
49+
}
50+
return
51+
}

0 commit comments

Comments
 (0)