diff --git a/.secrets.baseline b/.secrets.baseline index d1b6550a..523411e2 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "go.sum|^.secrets.baseline$", "lines": null }, - "generated_at": "2026-07-27T07:27:44Z", + "generated_at": "2026-11-05T08:45:07Z", "plugins_used": [ { "name": "AWSKeyDetector" @@ -80,6 +80,7 @@ "README.md": [ { "hashed_secret": "325778ab9d49a6df7bc13a83563bec2de2a84c95", + "is_secret": true, "is_verified": false, "line_number": 72, "type": "Secret Keyword", @@ -89,6 +90,7 @@ "examples/kubernetes/cos-s3-csi-rclone-pvc.yaml": [ { "hashed_secret": "ccae38d9a6159915fbbf9bf72dbf6760e0c63c35", + "is_secret": true, "is_verified": false, "line_number": 7, "type": "Secret Keyword", @@ -98,6 +100,7 @@ "examples/kubernetes/cos-s3-csi-rclone-secret.yaml": [ { "hashed_secret": "d08f88df745fa7950b104e4a707a31cfce7b5841", + "is_secret": true, "is_verified": false, "line_number": 9, "type": "Secret Keyword", @@ -107,6 +110,7 @@ "examples/kubernetes/cos-s3-csi-s3fs-pvc.yaml": [ { "hashed_secret": "f5691ce6d36f7c4434f9f927ddc09db95c0df745", + "is_secret": true, "is_verified": false, "line_number": 7, "type": "Secret Keyword", @@ -116,6 +120,7 @@ "examples/kubernetes/cos-s3-csi-s3fs-secret.yaml": [ { "hashed_secret": "d08f88df745fa7950b104e4a707a31cfce7b5841", + "is_secret": true, "is_verified": false, "line_number": 10, "type": "Secret Keyword", @@ -125,8 +130,16 @@ "pkg/constants/constants.go": [ { "hashed_secret": "84ad41837ea9d0e48a7186f627442db291621455", + "is_secret": true, "is_verified": false, - "line_number": 48, + "line_number": 54, + "type": "Secret Keyword", + "verified_result": null + }, + { + "hashed_secret": "d4c3d66fd0c38547a3c7a4c6bdc29c36911bc030", + "is_verified": false, + "line_number": 57, "type": "Secret Keyword", "verified_result": null } @@ -134,6 +147,7 @@ "pkg/driver/controllerserver.go": [ { "hashed_secret": "4e1e3828599786b2713ed5a843d6e0bcf854a2f6", + "is_secret": true, "is_verified": false, "line_number": 135, "type": "Secret Keyword", @@ -141,6 +155,7 @@ }, { "hashed_secret": "a1f0e99af8b76b514ef9e6a174e9c79970332082", + "is_secret": true, "is_verified": false, "line_number": 204, "type": "Secret Keyword", @@ -148,6 +163,7 @@ }, { "hashed_secret": "b732fb611fd46a38e8667f9972e0cde777fbe37f", + "is_secret": true, "is_verified": false, "line_number": 207, "type": "Secret Keyword", @@ -155,6 +171,7 @@ }, { "hashed_secret": "c7c6508b19455e3e8040e60e9833fbede92e5d8e", + "is_secret": false, "is_verified": false, "line_number": 382, "type": "Secret Keyword", @@ -162,6 +179,7 @@ }, { "hashed_secret": "62d6314bd936033553442d72fe8ce3b93ba8802d", + "is_secret": true, "is_verified": false, "line_number": 386, "type": "Secret Keyword", @@ -185,6 +203,7 @@ }, { "hashed_secret": "d4c3d66fd0c38547a3c7a4c6bdc29c36911bc030", + "is_secret": true, "is_verified": false, "line_number": 620, "type": "Secret Keyword", @@ -192,6 +211,7 @@ }, { "hashed_secret": "89fe47e5881e37bdb626d8354632f80baa3fe54f", + "is_secret": true, "is_verified": false, "line_number": 629, "type": "Secret Keyword", @@ -201,46 +221,76 @@ "pkg/driver/controllerserver_test.go": [ { "hashed_secret": "7e6a3680012346b94b54731e13d8a9ffa3790645", + "is_secret": true, "is_verified": false, "line_number": 248, "type": "Secret Keyword", "verified_result": null } ], + "pkg/driver/nodeserver_test.go": [ + { + "hashed_secret": "916705166579f315b6f82e66e68eeac6f3062d8b", + "is_verified": false, + "line_number": 742, + "type": "Secret Keyword", + "verified_result": null + } + ], + "pkg/mounter/mounter-mount-s3.go": [ + { + "hashed_secret": "a244263a4d970da788fefed5992903a119375616", + "is_verified": false, + "line_number": 197, + "type": "Secret Keyword", + "verified_result": null + }, + { + "hashed_secret": "2ace62c1befa19e3ea37dd52be9f6d508c5163e6", + "is_verified": false, + "line_number": 420, + "type": "Secret Keyword", + "verified_result": null + } + ], "pkg/mounter/mounter-rclone.go": [ { "hashed_secret": "39f69c278f46165447f30d10acf54277aaa3d5fc", "is_secret": false, "is_verified": false, - "line_number": 111, + "line_number": 199, "type": "Secret Keyword", "verified_result": null }, { "hashed_secret": "919673f52800c82484f1c9ab638faa1201c18477", + "is_secret": true, "is_verified": false, - "line_number": 303, + "line_number": 434, "type": "Secret Keyword", "verified_result": null }, { "hashed_secret": "769044d35edae0d0d6e0b8c24d68e456d61a7ac1", + "is_secret": true, "is_verified": false, - "line_number": 307, + "line_number": 438, "type": "Secret Keyword", "verified_result": null }, { "hashed_secret": "828564546c10e1da3568d3b502cd13ceada5fb93", + "is_secret": true, "is_verified": false, - "line_number": 311, + "line_number": 442, "type": "Secret Keyword", "verified_result": null }, { "hashed_secret": "6c9dcae2cd186e569853c911d0799b23f1bc9b95", + "is_secret": true, "is_verified": false, - "line_number": 312, + "line_number": 443, "type": "Secret Keyword", "verified_result": null } @@ -248,29 +298,33 @@ "pkg/mounter/mounter-rclone_test.go": [ { "hashed_secret": "2e7a7ee14caebf378fc32d6cf6f557f347c96773", + "is_secret": true, "is_verified": false, - "line_number": 115, + "line_number": 118, "type": "Secret Keyword", "verified_result": null }, { "hashed_secret": "8e2d2b7918839eef755ecb9181235cc83934befb", + "is_secret": true, "is_verified": false, - "line_number": 367, + "line_number": 378, "type": "Secret Keyword", "verified_result": null }, { "hashed_secret": "724ba0f0a4c4226b95478a7f304efd7951250ed3", + "is_secret": true, "is_verified": false, - "line_number": 368, + "line_number": 379, "type": "Secret Keyword", "verified_result": null }, { "hashed_secret": "a56dba7ed82ba76c3b271ce5302214cafc85d1c7", + "is_secret": true, "is_verified": false, - "line_number": 369, + "line_number": 380, "type": "Secret Keyword", "verified_result": null } @@ -288,8 +342,9 @@ "pkg/mounter/mounter-s3fs_test.go": [ { "hashed_secret": "2e7a7ee14caebf378fc32d6cf6f557f347c96773", + "is_secret": true, "is_verified": false, - "line_number": 22, + "line_number": 23, "type": "Secret Keyword", "verified_result": null } @@ -297,12 +352,29 @@ "pkg/mounter/mounter_test.go": [ { "hashed_secret": "2e7a7ee14caebf378fc32d6cf6f557f347c96773", + "is_secret": true, "is_verified": false, "line_number": 45, "type": "Secret Keyword", "verified_result": null } ], + "pkg/mounter/utils/mounter_utils_redact_test.go": [ + { + "hashed_secret": "cde95a6383d0da92748d43919da1ac80fe0ccd12", + "is_verified": false, + "line_number": 18, + "type": "Secret Keyword", + "verified_result": null + }, + { + "hashed_secret": "6673eee04215a90c2db3a0deae03420f178b4be7", + "is_verified": false, + "line_number": 24, + "type": "Secret Keyword", + "verified_result": null + } + ], "pkg/s3client/s3client.go": [ { "hashed_secret": "096aacba084fa9ee67efa56625495beaeba085a1", @@ -322,6 +394,7 @@ }, { "hashed_secret": "f75b33f87ffeacb3a4f793a09693e672e07449ff", + "is_secret": true, "is_verified": false, "line_number": 270, "type": "Secret Keyword", @@ -331,6 +404,7 @@ "pkg/s3client/s3client_test.go": [ { "hashed_secret": "d4c3d66fd0c38547a3c7a4c6bdc29c36911bc030", + "is_secret": true, "is_verified": false, "line_number": 57, "type": "Secret Keyword", @@ -338,6 +412,7 @@ }, { "hashed_secret": "2fa0ceed0ed702c459dd69734dfa1bc54e0bdd70", + "is_secret": true, "is_verified": false, "line_number": 123, "type": "Secret Keyword", @@ -347,8 +422,32 @@ "pkg/utils/driver_utils.go": [ { "hashed_secret": "c7c6508b19455e3e8040e60e9833fbede92e5d8e", + "is_secret": true, + "is_verified": false, + "line_number": 378, + "type": "Secret Keyword", + "verified_result": null + } + ], + "pkg/utils/driver_utils_masking_test.go": [ + { + "hashed_secret": "7acff81a051775d451003a7d79f82799a7b0ff33", + "is_verified": false, + "line_number": 37, + "type": "Secret Keyword", + "verified_result": null + }, + { + "hashed_secret": "b0b4ffd5dc5cbfccaad0436cac0a8a4d00749d7b", + "is_verified": false, + "line_number": 40, + "type": "Secret Keyword", + "verified_result": null + }, + { + "hashed_secret": "5c15ebc1739ce65c2d6eff352fb858af215b2bec", "is_verified": false, - "line_number": 376, + "line_number": 179, "type": "Secret Keyword", "verified_result": null } @@ -356,6 +455,7 @@ "tests/e2e/s3fs_test.go": [ { "hashed_secret": "d5cd5ec53cf11ad72f24451e6c077d94d0481046", + "is_secret": true, "is_verified": false, "line_number": 147, "type": "Secret Keyword", @@ -365,8 +465,9 @@ "tests/e2e/testsuites/basespecs.go": [ { "hashed_secret": "7519d22977923ba306715e7048cede9b5ee60d0b", + "is_secret": true, "is_verified": false, - "line_number": 117, + "line_number": 118, "type": "Secret Keyword", "verified_result": null } diff --git a/cos-csi-mounter/server/fake_server.go b/cos-csi-mounter/server/fake_server.go index fe11f584..3d36d74f 100644 --- a/cos-csi-mounter/server/fake_server.go +++ b/cos-csi-mounter/server/fake_server.go @@ -27,6 +27,11 @@ func (m *MockMounterUtils) FuseUnmount(path string) error { return argsCalled.Error(0) } +func (m *MockMounterUtils) FuseMountWithEnv(path string, comm string, args []string, envVars []string) error { + argsCalled := m.Called(path, comm, args, envVars) + return argsCalled.Error(0) +} + type fakeListener struct{} func (d *fakeListener) Accept() (net.Conn, error) { diff --git a/cos-csi-mounter/server/mount-s3.go b/cos-csi-mounter/server/mount-s3.go new file mode 100644 index 00000000..849009b3 --- /dev/null +++ b/cos-csi-mounter/server/mount-s3.go @@ -0,0 +1,220 @@ +package main + +import ( + "fmt" + "strconv" + + "go.uber.org/zap" +) + +var ( + // Safe base directory for mount-s3 log files. + s3MounterSafeLogDir = "/var/log/mount-s3" + // Safe base directory for mount-s3 cache files. + s3MounterSafeCacheDir = "/tmp" +) + +// s3MounterArgs holds the args sent from the node server for mount-s3. +// This struct must match the s3MounterArgs in pkg/mounter/mounter-mount-s3.go +type s3MounterArgs struct { + // Always set. + AllowOther string `json:"allow-other,omitempty"` + AwsCredentialsFile string `json:"aws-credentials-file,omitempty"` + AwsConfigFile string `json:"aws-config-file,omitempty"` + + // SecretMap-sourced. + EndpointURL string `json:"endpoint-url,omitempty"` + Region string `json:"region,omitempty"` + + // Identity. + UID string `json:"uid,omitempty"` + GID string `json:"gid,omitempty"` + + // Logging (translated from LogLevel). + LogLevel string `json:"log-level,omitempty"` + LogDirectory string `json:"log-directory,omitempty"` + + // Cache. + CacheDir string `json:"cache,omitempty"` + + // Write operations - conflict resolution applied when read-only is set. + ReadOnly string `json:"read-only,omitempty"` + AllowDelete string `json:"allow-delete,omitempty"` + AllowOverwrite string `json:"allow-overwrite,omitempty"` + IncrementalUpload string `json:"incremental-upload,omitempty"` + + // ForcePathStyle is always true for IBM COS compatibility. + ForcePathStyle string `json:"force-path-style,omitempty"` + + // Passthrough flags — user-supplied flags not handled by structured fields. + // Secret flags override SC flags when the same key appears in both. + // These are already deduplicated by the node server before being sent here. + Args []string `json:"args,omitempty"` +} + +// PopulateArgsSlice builds the CLI args slice for mount-s3. +func (args s3MounterArgs) PopulateArgsSlice(bucket, targetPath string) ([]string, error) { + result := []string{bucket, targetPath} + + // Block incremental-upload flag (causes data loss with IBM COS). + // This flag uses multiple PutObject calls that overwrite each other, + // resulting in ~75% data loss. + if args.IncrementalUpload == "true" { + logger.Warn("incremental-upload is not supported with IBM Cloud Object Storage, disabling") + args.IncrementalUpload = "" + } + + // Read-only priority resolution: suppress write-enabling flags. + if args.ReadOnly == "true" { + if args.AllowDelete == "true" { + logger.Warn("read-only is set, clearing allow-delete") + args.AllowDelete = "" + } + if args.AllowOverwrite == "true" { + logger.Warn("read-only is set, clearing allow-overwrite") + args.AllowOverwrite = "" + } + } + + if args.AllowOther == "true" { + result = append(result, "--allow-other") + } + if args.ReadOnly == "true" { + result = append(result, "--read-only") + } + if args.AllowDelete == "true" { + result = append(result, "--allow-delete") + } + if args.AllowOverwrite == "true" { + result = append(result, "--allow-overwrite") + } + if args.IncrementalUpload == "true" { + result = append(result, "--incremental-upload") + } + if args.UID != "" { + result = append(result, "--uid="+args.UID) + } + if args.GID != "" { + result = append(result, "--gid="+args.GID) + } + if args.EndpointURL != "" { + result = append(result, "--endpoint-url="+args.EndpointURL) + } + if args.Region != "" { + result = append(result, "--region="+args.Region) + } + + switch args.LogLevel { + case "debug": + result = append(result, "--debug") + case "debug-crt": + result = append(result, "--debug-crt") + case "no-log": + result = append(result, "--no-log") + default: + if args.LogLevel != "" { + logger.Warn("unsupported log level for mount-s3, ignoring", + zap.String("log-level", args.LogLevel), + zap.String("supported", "debug, debug-crt, no-log"), + ) + } + } + + if args.LogDirectory != "" { + result = append(result, "--log-directory="+args.LogDirectory) + } + if args.CacheDir != "" { + result = append(result, "--cache="+args.CacheDir) + } + if args.ForcePathStyle == "true" { + result = append(result, "--force-path-style") + } + + // Passthrough flags appended last — already deduplicated by node server. + result = append(result, args.Args...) + + return result, nil +} + +// Validate checks that required fields are present and values are valid. +func (args s3MounterArgs) Validate(targetPath string) error { + if err := pathValidator(targetPath); err != nil { + return err + } + + if args.AllowOther != "" { + if isBool := isBoolString(args.AllowOther); !isBool { + logger.Error("cannot convert value of allow-other into boolean", zap.Any("allow-other", args.AllowOther)) + return fmt.Errorf("cannot convert value of allow-other into boolean: %v", args.AllowOther) + } + } + + if args.AwsCredentialsFile == "" { + logger.Error("aws-credentials-file is required for mount-s3") + return fmt.Errorf("aws-credentials-file is required for mount-s3") + } + if args.AwsConfigFile == "" { + logger.Error("aws-config-file is required for mount-s3") + return fmt.Errorf("aws-config-file is required for mount-s3") + } + + if args.ReadOnly != "" { + if isBool := isBoolString(args.ReadOnly); !isBool { + logger.Error("cannot convert value of read-only into boolean", zap.Any("read-only", args.ReadOnly)) + return fmt.Errorf("cannot convert value of read-only into boolean: %v", args.ReadOnly) + } + } + if args.AllowDelete != "" { + if isBool := isBoolString(args.AllowDelete); !isBool { + logger.Error("cannot convert value of allow-delete into boolean", zap.Any("allow-delete", args.AllowDelete)) + return fmt.Errorf("cannot convert value of allow-delete into boolean: %v", args.AllowDelete) + } + } + if args.AllowOverwrite != "" { + if isBool := isBoolString(args.AllowOverwrite); !isBool { + logger.Error("cannot convert value of allow-overwrite into boolean", zap.Any("allow-overwrite", args.AllowOverwrite)) + return fmt.Errorf("cannot convert value of allow-overwrite into boolean: %v", args.AllowOverwrite) + } + } + if args.IncrementalUpload != "" { + if isBool := isBoolString(args.IncrementalUpload); !isBool { + logger.Error("cannot convert value of incremental-upload into boolean", zap.Any("incremental-upload", args.IncrementalUpload)) + return fmt.Errorf("cannot convert value of incremental-upload into boolean: %v", args.IncrementalUpload) + } + } + + if args.UID != "" { + if _, err := strconv.Atoi(args.UID); err != nil { + logger.Error("uid must be a valid integer", zap.String("uid", args.UID)) + return fmt.Errorf("uid must be a valid integer: %v", args.UID) + } + } + if args.GID != "" { + if _, err := strconv.Atoi(args.GID); err != nil { + logger.Error("gid must be a valid integer", zap.String("gid", args.GID)) + return fmt.Errorf("gid must be a valid integer: %v", args.GID) + } + } + if args.ForcePathStyle != "" { + if isBool := isBoolString(args.ForcePathStyle); !isBool { + logger.Error("cannot convert value of force-path-style into boolean", zap.Any("force-path-style", args.ForcePathStyle)) + return fmt.Errorf("cannot convert value of force-path-style into boolean: %v", args.ForcePathStyle) + } + } + + // ensureDir creates the directory if it does not exist, confined to its safe base. + if args.CacheDir != "" { + if err := ensureDir(args.CacheDir, s3MounterSafeCacheDir); err != nil { + logger.Error("failed to create cache directory", zap.String("cache-dir", args.CacheDir), zap.Error(err)) + return fmt.Errorf("failed to create cache directory '%s': %w", args.CacheDir, err) + } + } + if args.LogDirectory != "" { + if err := ensureDir(args.LogDirectory, s3MounterSafeLogDir); err != nil { + logger.Error("failed to create log directory", zap.String("log-dir", args.LogDirectory), zap.Error(err)) + return fmt.Errorf("failed to create log directory '%s': %w", args.LogDirectory, err) + } + } + + return nil +} diff --git a/cos-csi-mounter/server/mount-s3_test.go b/cos-csi-mounter/server/mount-s3_test.go new file mode 100644 index 00000000..4bd7086e --- /dev/null +++ b/cos-csi-mounter/server/mount-s3_test.go @@ -0,0 +1,344 @@ +package main + +import ( + "encoding/json" + "testing" + + "github.com/IBM/ibm-object-csi-driver/pkg/constants" + "github.com/stretchr/testify/assert" +) + +var ( + testCredFile = "/var/lib/coscsi-config/test/credentials" + testCfgFile = "/var/lib/coscsi-config/test/config" + testEndpoint = "https://s3.us-south.cloud-object-storage.appdomain.cloud" + testRegion = "us-south-standard" +) + +// --- PopulateArgsSlice tests --- + +func TestS3MounterPopulateArgsSlice_BasicArgs(t *testing.T) { + args := s3MounterArgs{ + AllowOther: "true", + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + EndpointURL: testEndpoint, + Region: testRegion, + AllowDelete: "true", + AllowOverwrite: "true", + ForcePathStyle: "true", + } + + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + assert.Equal(t, testBucket, result[0]) + assert.Equal(t, testTargetPath, result[1]) + assert.Contains(t, result, "--allow-other") + assert.Contains(t, result, "--allow-delete") + assert.Contains(t, result, "--allow-overwrite") + assert.Contains(t, result, "--endpoint-url="+testEndpoint) + assert.Contains(t, result, "--region="+testRegion) + assert.Contains(t, result, "--force-path-style") +} + +func TestS3MounterPopulateArgsSlice_ReadOnlyClearsWriteFlags(t *testing.T) { + args := s3MounterArgs{ + AllowOther: "true", + ReadOnly: "true", + AllowDelete: "true", + AllowOverwrite: "true", + } + + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + assert.Contains(t, result, "--read-only") + assert.NotContains(t, result, "--allow-delete") + assert.NotContains(t, result, "--allow-overwrite") +} + +func TestS3MounterPopulateArgsSlice_IncrementalUploadBlocked(t *testing.T) { + args := s3MounterArgs{ + IncrementalUpload: "true", + } + + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + assert.NotContains(t, result, "--incremental-upload") +} + +func TestS3MounterPopulateArgsSlice_UIDAndGID(t *testing.T) { + args := s3MounterArgs{ + UID: "1000", + GID: "2000", + } + + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + assert.Contains(t, result, "--uid=1000") + assert.Contains(t, result, "--gid=2000") +} + +func TestS3MounterPopulateArgsSlice_LogLevels(t *testing.T) { + for _, level := range []string{"debug", "debug-crt", "no-log"} { + args := s3MounterArgs{LogLevel: level} + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + assert.Contains(t, result, "--"+level) + } +} + +func TestS3MounterPopulateArgsSlice_UnsupportedLogLevel(t *testing.T) { + args := s3MounterArgs{LogLevel: "verbose"} + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + // unsupported log level is silently ignored + assert.NotContains(t, result, "--verbose") +} + +func TestS3MounterPopulateArgsSlice_PassthroughArgs(t *testing.T) { + args := s3MounterArgs{ + Args: []string{"--max-threads=32", "--read-part-size=16777216"}, + } + + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + assert.Contains(t, result, "--max-threads=32") + assert.Contains(t, result, "--read-part-size=16777216") +} + +func TestS3MounterPopulateArgsSlice_CacheAndLogDir(t *testing.T) { + args := s3MounterArgs{ + CacheDir: "/tmp/cache", + LogDirectory: "/var/log/mount-s3", + } + + result, err := args.PopulateArgsSlice(testBucket, testTargetPath) + assert.NoError(t, err) + assert.Contains(t, result, "--cache=/tmp/cache") + assert.Contains(t, result, "--log-directory=/var/log/mount-s3") +} + +// --- Validate tests --- + +func TestS3MounterValidate_Success(t *testing.T) { + args := s3MounterArgs{ + AllowOther: "true", + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + } + + err := args.Validate(testTargetPath) + assert.NoError(t, err) +} + +func TestS3MounterValidate_InvalidPath(t *testing.T) { + args := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + } + + err := args.Validate("invalid-path") + assert.Error(t, err) + assert.Contains(t, err.Error(), "bad value for target path") +} + +func TestS3MounterValidate_MissingCredentialsFile(t *testing.T) { + args := s3MounterArgs{ + AwsConfigFile: testCfgFile, + } + + err := args.Validate(testTargetPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "aws-credentials-file is required") +} + +func TestS3MounterValidate_MissingConfigFile(t *testing.T) { + args := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + } + + err := args.Validate(testTargetPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "aws-config-file is required") +} + +func TestS3MounterValidate_InvalidAllowOther(t *testing.T) { + args := s3MounterArgs{ + AllowOther: "yes", + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + } + + err := args.Validate(testTargetPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot convert value of allow-other into boolean") +} + +func TestS3MounterValidate_InvalidBoolFields(t *testing.T) { + base := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + } + + tests := []struct { + name string + apply func(*s3MounterArgs) + errMsg string + }{ + {"invalid read-only", func(a *s3MounterArgs) { a.ReadOnly = "yes" }, "cannot convert value of read-only"}, + {"invalid allow-delete", func(a *s3MounterArgs) { a.AllowDelete = "yes" }, "cannot convert value of allow-delete"}, + {"invalid allow-overwrite", func(a *s3MounterArgs) { a.AllowOverwrite = "yes" }, "cannot convert value of allow-overwrite"}, + {"invalid incremental-upload", func(a *s3MounterArgs) { a.IncrementalUpload = "yes" }, "cannot convert value of incremental-upload"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := base + tt.apply(&args) + err := args.Validate(testTargetPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + }) + } +} + +func TestS3MounterValidate_InvalidUID(t *testing.T) { + args := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + UID: "notanint", + } + err := args.Validate(testTargetPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "uid must be a valid integer") +} + +func TestS3MounterValidate_InvalidGID(t *testing.T) { + args := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + GID: "notanint", + } + err := args.Validate(testTargetPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "gid must be a valid integer") +} + +func TestS3MounterValidate_InvalidForcePathStyle(t *testing.T) { + args := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + ForcePathStyle: "yes", + } + err := args.Validate(testTargetPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot convert value of force-path-style into boolean") +} + +func TestS3MounterValidate_ValidUIDAndGID(t *testing.T) { + args := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + UID: "1000", + GID: "2000", + } + err := args.Validate(testTargetPath) + assert.NoError(t, err) +} + +// --- ParseMounterArgs with AMAZONS3MOUNTER tests (N2) --- + +func TestParseMounterArgs_AmazonS3_Valid(t *testing.T) { + argsStruct := s3MounterArgs{ + AllowOther: "true", + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + EndpointURL: testEndpoint, + Region: testRegion, + AllowDelete: "true", + AllowOverwrite: "true", + ForcePathStyle: "true", + } + b, _ := json.Marshal(argsStruct) + + req := MountRequest{ + Path: testTargetPath, + Bucket: testBucket, + Mounter: constants.AMAZONS3MOUNTER, + Args: b, + } + + args, err := req.ParseMounterArgs() + assert.NoError(t, err) + assert.NotNil(t, args) + assert.Contains(t, args, testBucket) + assert.Contains(t, args, "--allow-other") + assert.Contains(t, args, "--endpoint-url="+testEndpoint) +} + +func TestParseMounterArgs_AmazonS3_InvalidJSON(t *testing.T) { + req := MountRequest{ + Path: testTargetPath, + Bucket: testBucket, + Mounter: constants.AMAZONS3MOUNTER, + Args: json.RawMessage(`{"invalid-json"}`), + } + + args, err := req.ParseMounterArgs() + assert.Error(t, err) + assert.Nil(t, args) + assert.Contains(t, err.Error(), "invalid mount-s3 args decode error") +} + +func TestParseMounterArgs_AmazonS3_UnknownField(t *testing.T) { + req := MountRequest{ + Path: testTargetPath, + Bucket: testBucket, + Mounter: constants.AMAZONS3MOUNTER, + Args: json.RawMessage(`{"unknown-field": "value"}`), + } + + args, err := req.ParseMounterArgs() + assert.Error(t, err) + assert.Nil(t, args) + assert.Contains(t, err.Error(), "invalid mount-s3 args decode error") +} + +func TestParseMounterArgs_AmazonS3_ValidationFails(t *testing.T) { + argsStruct := s3MounterArgs{ + AllowOther: "true", + } + b, _ := json.Marshal(argsStruct) + + req := MountRequest{ + Path: testTargetPath, + Bucket: testBucket, + Mounter: constants.AMAZONS3MOUNTER, + Args: b, + } + + args, err := req.ParseMounterArgs() + assert.Error(t, err) + assert.Nil(t, args) + assert.Contains(t, err.Error(), "s3Mounter args validation failed") +} + +func TestParseMounterArgs_AmazonS3_InvalidTargetPath(t *testing.T) { + argsStruct := s3MounterArgs{ + AwsCredentialsFile: testCredFile, + AwsConfigFile: testCfgFile, + } + b, _ := json.Marshal(argsStruct) + + req := MountRequest{ + Path: "invalid-path", + Bucket: testBucket, + Mounter: constants.AMAZONS3MOUNTER, + Args: b, + } + + args, err := req.ParseMounterArgs() + assert.Error(t, err) + assert.Nil(t, args) + assert.Contains(t, err.Error(), "s3Mounter args validation failed") +} diff --git a/cos-csi-mounter/server/server.go b/cos-csi-mounter/server/server.go index ab347f86..ebdd33fe 100644 --- a/cos-csi-mounter/server/server.go +++ b/cos-csi-mounter/server/server.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "flag" "fmt" "net" @@ -169,7 +170,7 @@ func handleCosMount(mounter mounterUtils.MounterUtils, parser MounterArgsParser) logger.Info("New mount request with values:", zap.String("Bucket", request.Bucket), zap.String("Path", request.Path), zap.String("Mounter", request.Mounter), zap.Any("Args", redactArgsForLog(request.Args))) - if request.Mounter != constants.S3FS && request.Mounter != constants.RClone { + if request.Mounter != constants.S3FS && request.Mounter != constants.RClone && request.Mounter != constants.AMAZONS3MOUNTER { logger.Error("invalid mounter", zap.Any("mounter", request.Mounter)) c.JSON(http.StatusBadRequest, gin.H{"error": "invalid mounter"}) return @@ -181,16 +182,20 @@ func handleCosMount(mounter mounterUtils.MounterUtils, parser MounterArgsParser) return } - // validate mounter args + // validate and parse mounter args args, err := parser.Parse(request) if err != nil { logger.Error("failed to parse mounter args", zap.Any("mounter", request.Mounter), zap.Error(err)) - c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("invalid args for mounter: %v", err)}) return } - err = mounter.FuseMount(request.Path, request.Mounter, args) + // For mount-s3: pass AWS credential paths as env vars on the subprocess. + if request.Mounter == constants.AMAZONS3MOUNTER { + err = mounter.FuseMountWithEnv(request.Path, request.Mounter, args, buildS3EnvVars(request.Args)) + } else { + err = mounter.FuseMount(request.Path, request.Mounter, args) + } if err != nil { logger.Error("mount failed: ", zap.Error(err)) c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("mount failed: %v", err)}) @@ -202,6 +207,37 @@ func handleCosMount(mounter mounterUtils.MounterUtils, parser MounterArgsParser) } } +// buildS3EnvVars extracts AWS credential file paths from the request args map +// and returns them as env var strings for the mount-s3 subprocess. +// mount-s3 uses the AWS SDK which reads these standard env vars to locate credentials. +// +// IMPORTANT: must use map[string]interface{} — NOT map[string]string. +// The args payload contains mixed types (e.g. "log-metrics": true is a bool, +// not a string). Unmarshalling into map[string]string silently fails on any +// bool or numeric field, returning nil and dropping both credential env vars, +// which causes mount-s3 to fail with NoSigningCredentials. +func buildS3EnvVars(args json.RawMessage) []string { + var argsMap map[string]interface{} + if err := json.Unmarshal(args, &argsMap); err != nil { + logger.Error("buildS3EnvVars: failed to unmarshal args, credential env vars will be missing", + zap.Error(err)) + return nil + } + + var envVars []string + if v, ok := argsMap["aws-credentials-file"]; ok { + if s, ok := v.(string); ok && s != "" { + envVars = append(envVars, "AWS_SHARED_CREDENTIALS_FILE="+s) + } + } + if v, ok := argsMap["aws-config-file"]; ok { + if s, ok := v.(string); ok && s != "" { + envVars = append(envVars, "AWS_CONFIG_FILE="+s) + } + } + return envVars +} + func handleCosUnmount(mounter mounterUtils.MounterUtils) gin.HandlerFunc { return func(c *gin.Context) { var request struct { diff --git a/cos-csi-mounter/server/utils.go b/cos-csi-mounter/server/utils.go index 21d9a61f..f433aa18 100644 --- a/cos-csi-mounter/server/utils.go +++ b/cos-csi-mounter/server/utils.go @@ -220,6 +220,16 @@ func (req *MountRequest) ParseMounterArgs() ([]string, error) { } return args.PopulateArgsSlice(req.Bucket, req.Path) + case constants.AMAZONS3MOUNTER: + var args s3MounterArgs + if err := strictDecodeForUnknownFields(req.Args, &args); err != nil { + return nil, fmt.Errorf("invalid mount-s3 args decode error: %w", err) + } + if err := args.Validate(req.Path); err != nil { + return nil, fmt.Errorf("s3Mounter args validation failed: %w", err) + } + return args.PopulateArgsSlice(req.Bucket, req.Path) + default: return nil, fmt.Errorf("unknown mounter: %s", req.Mounter) } @@ -250,6 +260,41 @@ func fileExists(path string) (bool, error) { return !info.IsDir(), nil } +// ensureDir creates the directory at path if it doesn't exist. +// It first resolves the absolute path and verifies it is within safeBase +// to prevent path-traversal attacks before creating the directory. +func ensureDir(path, safeBase string) error { + absPath, err := absPathResolver(path) // #nosec G304 -- path is validated against safeBase below to prevent traversal + if err != nil { + return fmt.Errorf("failed to resolve absolute path: %v", err) + } + if !strings.HasPrefix(absPath, safeBase) { + return fmt.Errorf("path %v is outside the safe directory", absPath) + } + + // Check if directory already exists + info, err := os.Stat(absPath) // #nosec G304 -- absPath is validated against safeBase above + if err == nil { + // Path exists, verify it's a directory + if !info.IsDir() { + return fmt.Errorf("path exists but is not a directory: %s", absPath) + } + return nil + } + + // If error is not "not exist", return it + if !os.IsNotExist(err) { + return fmt.Errorf("failed to stat directory: %w", err) + } + + // Directory doesn't exist, create it with parent directories + if err := os.MkdirAll(absPath, 0750); err != nil { // #nosec G301 -- directory permissions set to 0750 for security + return fmt.Errorf("failed to create directory: %w", err) + } + + return nil +} + // redactArgsForLog returns a copy of the raw Args JSON with the value of // "passwd_file" replaced by "xxxxx", leaving all other fields unchanged. // If the JSON cannot be parsed, the original raw bytes are returned as-is. diff --git a/cos-csi-mounter/server/utils_test.go b/cos-csi-mounter/server/utils_test.go index 0d45ec2e..2b209306 100644 --- a/cos-csi-mounter/server/utils_test.go +++ b/cos-csi-mounter/server/utils_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" "testing" @@ -685,3 +686,53 @@ func TestPathValidator_AbsResolverFails_ReturnsError(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "failed to resolve absolute mount path") } + +func TestEnsureDir_OutsideSafeDirectory(t *testing.T) { + err := ensureDir("/etc/passwd", "/tmp") + assert.Error(t, err) + assert.Contains(t, err.Error(), "outside the safe directory") +} + +func TestEnsureDir_PathTraversal(t *testing.T) { + err := ensureDir("/tmp/../../etc", "/tmp") + assert.Error(t, err) + assert.Contains(t, err.Error(), "outside the safe directory") +} + +func TestEnsureDir_CreatesDirectory(t *testing.T) { + dir := filepath.Join(t.TempDir(), "subdir") + safeBase := filepath.Dir(dir) + err := ensureDir(dir, safeBase) + assert.NoError(t, err) + info, statErr := os.Stat(dir) + assert.NoError(t, statErr) + assert.True(t, info.IsDir()) +} + +func TestEnsureDir_ExistingDirectory(t *testing.T) { + dir := t.TempDir() + err := ensureDir(dir, filepath.Dir(dir)) + assert.NoError(t, err) +} + +func TestEnsureDir_PathIsFile(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "file.txt") + assert.NoError(t, os.WriteFile(file, []byte("x"), 0600)) + err := ensureDir(file, dir) + assert.Error(t, err) + assert.Contains(t, err.Error(), "not a directory") +} + +func TestEnsureDir_AbsFails(t *testing.T) { + originalAbs := absPathResolver + defer func() { absPathResolver = originalAbs }() + + absPathResolver = func(path string) (string, error) { + return "", errors.New("abs error") + } + + err := ensureDir("some-path", "/tmp") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to resolve absolute path") +} diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 9b35a589..fee30f2a 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -14,8 +14,10 @@ const ( KPEncryptionAlgorithm = "AES256" // https://github.com/IBM/ibm-cos-sdk-go/blob/master/service/s3/api.go#L9130-L9136 - S3FS = "s3fs" - RClone = "rclone" + S3FS = "s3fs" + RClone = "rclone" + AMAZONS3MOUNTER = "mount-s3" + DefaultNamespace = "default" IAMEP = "https://private.iam.cloud.ibm.com/identity/token" @@ -29,12 +31,16 @@ const ( NodeRegionLabel = "topology.kubernetes.io/region" // Timeout specifies a time limit for requests made by HTTP Client - Timeout = 3 * time.Minute - COSCSIMounterSocketPath = "/var/lib/coscsi-sock/coscsi.sock" - COSCSIMounterSocketPathEnv = "COS_CSI_MOUNTER_SOCKET" - MounterConfigPathOnHost = "/var/lib/coscsi-config" - MounterConfigPathOnPodS3fs = "/var/lib/ibmc-s3fs" - MounterConfigPathOnPodRclone = "/root/.config/rclone" + Timeout = 3 * time.Minute + COSCSIMounterSocketPath = "/var/lib/coscsi-sock/coscsi.sock" + COSCSIMounterSocketPathEnv = "COS_CSI_MOUNTER_SOCKET" + MounterConfigPathOnHost = "/var/lib/coscsi-config" + MounterConfigPathOnPodS3fs = "/var/lib/ibmc-s3fs" + MountpointS3BinaryPath = "/usr/bin/mount-s3" + + // Config paths for mountpoint-s3 (same pattern as rclone) + MounterConfigPathOnPodS3Mount = "/var/lib/coscsi-config/s3mount" + MounterConfigPathOnPodRclone = "/root/.config/rclone" // Interval to wait till next loop Interval = 500 * time.Millisecond @@ -56,7 +62,7 @@ const ( KubeNodeName = "KUBE_NODE_NAME" MaxVolumesPerNodeEnv = "MAX_VOLUMES_PER_NODE" - IAMEndpointEnv = "IAM_ENDPOINT" + IAMEndpointEnv = "IAM_ENDPOINT" COSResourceConfigEndpointEnv = "COS_RESOURCE_CONFIG_ENDPOINT" CipherSuitesKey = "cipher_suites" diff --git a/pkg/mounter/mounter-mount-s3.go b/pkg/mounter/mounter-mount-s3.go new file mode 100644 index 00000000..c3c692ce --- /dev/null +++ b/pkg/mounter/mounter-mount-s3.go @@ -0,0 +1,583 @@ +// Package mounter +package mounter + +import ( + "bufio" + "crypto/sha256" + "encoding/json" + "fmt" + "os" + "path" + "strings" + "time" + + "github.com/IBM/ibm-object-csi-driver/pkg/constants" + "github.com/IBM/ibm-object-csi-driver/pkg/mounter/utils" + "k8s.io/klog/v2" +) + +// MountpointS3Mounter implements Mounter for AWS Mountpoint S3. +type MountpointS3Mounter struct { + BucketName string + ObjectPath string + EndPoint string + LocConstraint string + AuthType string + AccessKey string + SecretKey string + + UID string + GID string + + LogLevel string + LogDirectory string + CacheDir string + + ReadOnly bool + AllowDelete bool + AllowOverwrite bool + IncrementalUpload bool + + // MountOptions holds deduplicated passthrough flags. + // SC flags are overridden by secret flags when the same key appears in both. + MountOptions []string + MounterUtils utils.MounterUtils +} + +const ( + // s3MountLogDirectory is the hardcoded log directory for mount-s3, matching + // the same convention as rclone's hardcoded --log-file=/var/log/rclone.log. + s3MountLogDirectory = "/var/log/mount-s3" +) + +// s3MounterArgs holds the args sent to the worker node daemon for mount-s3. +// This struct must stay in sync with s3MounterArgs in the worker (cos-csi-mounter). +type s3MounterArgs struct { + AllowOther string `json:"allow-other,omitempty"` + AwsCredentialsFile string `json:"aws-credentials-file,omitempty"` + AwsConfigFile string `json:"aws-config-file,omitempty"` + + EndpointURL string `json:"endpoint-url,omitempty"` + Region string `json:"region,omitempty"` + + UID string `json:"uid,omitempty"` + GID string `json:"gid,omitempty"` + + LogLevel string `json:"log-level,omitempty"` + LogDirectory string `json:"log-directory,omitempty"` + + CacheDir string `json:"cache,omitempty"` + + ReadOnly string `json:"read-only,omitempty"` + AllowDelete string `json:"allow-delete,omitempty"` + AllowOverwrite string `json:"allow-overwrite,omitempty"` + IncrementalUpload string `json:"incremental-upload,omitempty"` + + // ForcePathStyle is always true for IBM COS compatibility. + ForcePathStyle string `json:"force-path-style,omitempty"` + + // Passthrough flags — user-supplied flags not handled by structured fields. + // Secret flags override SC flags when the same key appears in both. + Args []string `json:"args,omitempty"` +} + +const ( + s3MountCredentialsFile = "credentials" + s3MountConfigFile = "config" + s3MountAwsProfile = "default" +) + +var ( + removeS3ConfigFile = removeS3MountConfigFile +) + +type MountpointS3MounterParams struct { + SecretMap map[string]string + MountOptions []string + MounterUtils utils.MounterUtils + Gid string + ReadOnly bool +} + +// NewMountpointS3Mounter creates a new MountpointS3Mounter from the unified +// secret format. +// +// Precedence (lowest → highest): +// 1. secretMap identity keys +// 2. StorageClass mountOptions — parsed first +// 3. Secret mountOptions — parsed second, wins over SC on any overlapping flag +// +// For passthrough flags, deduplication is applied: if the same flag key appears +// in both SC and secret, the SC entry is dropped and the secret entry is kept. +func NewMountpointS3Mounter(params MountpointS3MounterParams) Mounter { + secretMap := params.SecretMap + mountOptions := params.MountOptions + mounterUtils := params.MounterUtils + gid := params.Gid + readOnly := params.ReadOnly + + klog.Info("-newMountpointS3Mounter-") + + mounter := &MountpointS3Mounter{} + + if val, ok := secretMap["accessKey"]; ok { + mounter.AccessKey = val + } + if val, ok := secretMap["secretKey"]; ok { + mounter.SecretKey = val + } + if val, ok := secretMap["bucketName"]; ok { + mounter.BucketName = val + } + if val, ok := secretMap["objectPath"]; ok { + mounter.ObjectPath = val + } + if val, ok := secretMap["cosEndpoint"]; ok { + mounter.EndPoint = val + } + if val, ok := secretMap["locationConstraint"]; ok { + mounter.LocConstraint = val + } + + if gid != "" && secretMap["uid"] == "" { + mounter.UID = gid + } else if val := secretMap["uid"]; val != "" { + mounter.UID = val + } + if val := secretMap["gid"]; val != "" { + mounter.GID = val + } else { + mounter.GID = gid + } + + mounter.ReadOnly = readOnly + + mounter.AuthType = "hmac" + + // Step 1: parse SC options. + scRemainder := parseMountpointS3Options(mounter, mountOptions) + + // Step 2: parse secret options (structured fields set by secret override SC). + secretOpts := splitSecretMountOptions(secretMap["mountOptions"]) + secretRemainder := parseMountpointS3Options(mounter, secretOpts) + + // Step 3: deduplicate passthroughs — secret wins over SC on same key. + mounter.MountOptions = deduplicatePassthrough(scRemainder, secretRemainder) + + // Step 4: Block incremental-upload flag (causes data loss with IBM COS). + // This flag uses multiple PutObject calls that overwrite each other instead of + // using proper multipart upload, resulting in ~75% data loss. + if mounter.IncrementalUpload { + klog.Info("mount-s3: --incremental-upload is not supported ") + mounter.IncrementalUpload = false + } + + // Step 5: allow-delete and allow-overwrite are hardcoded defaults. + // They are suppressed later in Mount() when read-only is set. + mounter.AllowDelete = true + mounter.AllowOverwrite = true + + mounter.MounterUtils = mounterUtils + + klog.Infof("newMountpointS3Mounter args:\n\tbucketName: [%s]\n\tobjectPath: [%s]\n\tendPoint: [%s]\n\tlocationConstraint: [%s]\n\tauthType: [%s]", + mounter.BucketName, mounter.ObjectPath, mounter.EndPoint, mounter.LocConstraint, mounter.AuthType) + klog.Infof("newMountpointS3Mounter: SC mountOptions=%v secretMountOptions=%v deduplicated passthrough=%v", + mountOptions, secretOpts, mounter.MountOptions) + + return mounter +} + +// deduplicatePassthrough merges SC and secret passthrough flags. +// If the same flag key appears in both, the SC entry is dropped and the +// secret entry is kept (secret wins). +// +// Examples: +// +// SC: [--max-threads=32, --maximum-throughput-gbps=10, --allow-delete] +// Secret: [--max-threads=128, --maximum-throughput-gbps=25] +// Result: [--allow-delete, --max-threads=128, --maximum-throughput-gbps=25] +func deduplicatePassthrough(scFlags, secretFlags []string) []string { + // Build a set of keys present in secret flags. + secretKeys := make(map[string]struct{}, len(secretFlags)) + for _, flag := range secretFlags { + key := flagKey(flag) + secretKeys[key] = struct{}{} + } + + // Keep SC flags whose key does NOT appear in secret flags. + var result []string + for _, flag := range scFlags { + key := flagKey(flag) + if _, overridden := secretKeys[key]; overridden { + klog.Infof("deduplicatePassthrough: SC flag %q overridden by secret", flag) + } else { + result = append(result, flag) + } + } + + // Append all secret flags last. + result = append(result, secretFlags...) + return result +} + +// flagKey extracts the key portion from a passthrough flag. +// "--max-threads=128" → "max-threads" +// "--allow-delete" → "allow-delete" +func flagKey(flag string) string { + trimmed := strings.TrimPrefix(flag, "--") + key, _, _ := strings.Cut(trimmed, "=") + return key +} + +// splitSecretMountOptions splits the multiline mountOptions string from the secret. +func splitSecretMountOptions(raw string) []string { + if raw == "" { + return nil + } + var opts []string + for _, line := range strings.Split(raw, "\n") { + line = strings.TrimSpace(line) + line = strings.TrimRight(line, ",") + if line == "" || strings.HasPrefix(line, "#") { + continue + } + opts = append(opts, line) + } + return opts +} + +// parseMountpointS3Options parses mount-s3 options from the provided slice. +func parseMountpointS3Options(mounter *MountpointS3Mounter, opts []string) []string { + var remaining []string + + for _, opt := range opts { + trimmed := strings.TrimPrefix(opt, "--") + key, value, hasValue := strings.Cut(trimmed, "=") + + switch key { + case "allow-other": + klog.Infof("parseMountpointS3Options: 'allow-other' is always enabled and cannot be overridden.") + case "umask": + klog.Warningf("parseMountpointS3Options: 'umask' is not supported by mount-s3. " + + "Use --dir-mode / --file-mode instead. Ignoring.") + case "uid": + if hasValue { + mounter.UID = value + } + case "gid": + if hasValue { + mounter.GID = value + } + case "log-level": + if hasValue { + mounter.LogLevel = value + } + case "log-directory": + // Hardcoded — user-supplied value is ignored; the struct field is set + // to the fixed path so callers can read it back consistently. + klog.Warningf("parseMountpointS3Options: 'log-directory' is hardcoded to %q and cannot be overridden. Ignoring.", s3MountLogDirectory) + mounter.LogDirectory = s3MountLogDirectory + case "force-path-style": + // Hardcoded — always enabled for IBM COS compatibility. + klog.Warningf("parseMountpointS3Options: 'force-path-style' is hardcoded and cannot be overridden. Ignoring.") + case "cache": + if hasValue { + mounter.CacheDir = value + } + case "read-only": + // read-only is now set directly from MounterParams.ReadOnly — ignore if passed via flags. + klog.Warningf("parseMountpointS3Options: 'read-only' is set via MounterParams and cannot be overridden via mount options. Ignoring.") + case "allow-delete": + // Hardcoded — always enabled unless read-only. User override ignored. + klog.Warningf("parseMountpointS3Options: 'allow-delete' is hardcoded and cannot be overridden. Ignoring.") + case "allow-overwrite": + // Hardcoded — always enabled unless read-only. User override ignored. + klog.Warningf("parseMountpointS3Options: 'allow-overwrite' is hardcoded and cannot be overridden. Ignoring.") + case "incremental-upload": + mounter.IncrementalUpload = true + default: + if strings.HasPrefix(opt, "--") { + remaining = append(remaining, opt) + } else { + remaining = append(remaining, "--"+opt) + } + } + } + + return remaining +} + +// Mount mounts the S3 bucket using mountpoint-s3. +func (s3 *MountpointS3Mounter) Mount(source string, target string) error { + klog.Info("-MountpointS3Mounter Mount-") + klog.Infof("Mount args:\n\tsource: <%s>\n\ttarget: <%s>", source, target) + + // Read-only priority resolution: suppress write-enabling flags. + if s3.ReadOnly { + if s3.AllowDelete { + klog.Infof("MountpointS3Mounter: read-only set — clearing allow-delete") + s3.AllowDelete = false + } + if s3.AllowOverwrite { + klog.Infof("MountpointS3Mounter: read-only set — clearing allow-overwrite") + s3.AllowOverwrite = false + } + } + + // NOTE: No directory validation or creation here. + // LogDirectory and CacheDir are forwarded to the worker via workerNodeOp. + // The worker's Validate() calls ensureDir() which creates them if missing. + + var configPath string + if mountWorker { + configPath = constants.MounterConfigPathOnHost + } else { + configPath = constants.MounterConfigPathOnPodS3Mount + } + + configPathWithVolID := path.Join(configPath, fmt.Sprintf("%x", sha256.Sum256([]byte(target)))) + + if err := createS3MountConfig(configPathWithVolID, s3); err != nil { + klog.Errorf("MountpointS3Mounter Mount: Cannot create config file: %v", err) + return err + } + + bucketName := s3.BucketName + if s3.ObjectPath != "" { + trimmedPath := strings.TrimPrefix(s3.ObjectPath, "/") + bucketName = fmt.Sprintf("%s/%s", s3.BucketName, trimmedPath) + } + + args, envVars, wnOp := s3.formulateMountOptions(bucketName, target, configPathWithVolID) + + if mountWorker { + klog.Info("Mount on Worker started...") + + jsonData, err := json.Marshal(wnOp) + if err != nil { + klog.Errorf("Error marshalling mount args: %v", err) + return err + } + + payload := fmt.Sprintf(`{"path":"%s","bucket":"%s","mounter":"%s","args":%s}`, + target, bucketName, constants.AMAZONS3MOUNTER, jsonData) + + klog.Infof("MountpointS3Mounter: worker payload: %s", payload) + + err = mounterRequest(payload, "http://unix/api/cos/mount") + if err != nil { + klog.Errorf("failed to mount on worker: %v", err) + return err + } + return nil + } + + klog.Info("NodeServer Mounting...") + + return s3.MounterUtils.FuseMountWithEnv(target, constants.MountpointS3BinaryPath, args, envVars) +} + +// Unmount unmounts the S3 bucket. +func (s3 *MountpointS3Mounter) Unmount(target string) error { + klog.Info("-MountpointS3Mounter Unmount-") + + if mountWorker { + klog.Info("Unmount on Worker started...") + + payload := fmt.Sprintf(`{"path":"%s"}`, target) + + err := mounterRequest(payload, "http://unix/api/cos/unmount") + if err != nil { + klog.Errorf("failed to unmount on worker: %v", err) + return err + } + + removeS3ConfigFile(constants.MounterConfigPathOnHost, target) + return nil + } + + klog.Info("NodeServer Unmounting...") + err := s3.MounterUtils.FuseUnmount(target) + if err != nil { + return err + } + + removeS3ConfigFile(constants.MounterConfigPathOnPodS3Mount, target) + return nil +} + +// createS3MountConfig creates the AWS credentials and config files. +func createS3MountConfig(configPathWithVolID string, s3 *MountpointS3Mounter) error { + if err := MakeDir(configPathWithVolID, 0755); err != nil { // #nosec G301 -- config directory needs to be readable by mount-s3 process + klog.Errorf("MountpointS3Mounter: Cannot create config dir %s: %v", configPathWithVolID, err) + return err + } + + credFile := path.Join(configPathWithVolID, s3MountCredentialsFile) + credParams := []string{ + "[" + s3MountAwsProfile + "]", + "aws_access_key_id = " + s3.AccessKey, + "aws_secret_access_key = " + s3.SecretKey, + } + if err := writeConfigFile(credFile, credParams); err != nil { + klog.Errorf("MountpointS3Mounter: Cannot write credentials file: %v", err) + return err + } + + region := s3.LocConstraint + configParams := []string{ + "[" + s3MountAwsProfile + "]", + "region = " + region, + } + if s3.EndPoint != "" { + configParams = append(configParams, "endpoint_url = "+s3.EndPoint) + } + cfgFile := path.Join(configPathWithVolID, s3MountConfigFile) + if err := writeConfigFile(cfgFile, configParams); err != nil { + klog.Errorf("MountpointS3Mounter: Cannot write config file: %v", err) + return err + } + + klog.Infof("MountpointS3Mounter: Created config files at %s", configPathWithVolID) + return nil +} + +// writeConfigFile writes lines to a file with 0600 permissions. +func writeConfigFile(filePath string, lines []string) error { + f, err := OpenFile(filePath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) // #nosec G304 -- file path is constructed from controlled configPathWithVolID and constant filename + if err != nil { + return fmt.Errorf("cannot create file %s: %w", filePath, err) + } + defer func() { + if cerr := f.Close(); cerr != nil { + klog.Errorf("writeConfigFile: cannot close file %s: %v", filePath, cerr) + } + }() + + w := bufio.NewWriter(f) + for _, line := range lines { + if _, err := w.WriteString(line + "\n"); err != nil { + return fmt.Errorf("cannot write to file %s: %w", filePath, err) + } + } + return w.Flush() +} + +// formulateMountOptions builds the CLI args, env vars, and worker payload for mount-s3. +func (s3 *MountpointS3Mounter) formulateMountOptions(bucket, target, configPathWithVolID string) (nodeServerOp []string, envVars []string, workerNodeOp s3MounterArgs) { + nodeServerOp = []string{bucket, target, "--allow-other"} + + credFilePath := path.Join(configPathWithVolID, s3MountCredentialsFile) + cfgFilePath := path.Join(configPathWithVolID, s3MountConfigFile) + envVars = []string{ + "AWS_SHARED_CREDENTIALS_FILE=" + credFilePath, + "AWS_CONFIG_FILE=" + cfgFilePath, + } + + workerNodeOp = s3MounterArgs{ + AllowOther: "true", + AwsCredentialsFile: credFilePath, + AwsConfigFile: cfgFilePath, + } + + if s3.EndPoint != "" { + nodeServerOp = append(nodeServerOp, "--endpoint-url="+s3.EndPoint) + workerNodeOp.EndpointURL = s3.EndPoint + } + if s3.LocConstraint != "" { + nodeServerOp = append(nodeServerOp, "--region="+s3.LocConstraint) + workerNodeOp.Region = s3.LocConstraint + } + if s3.UID != "" { + nodeServerOp = append(nodeServerOp, "--uid="+s3.UID) + workerNodeOp.UID = s3.UID + } + if s3.GID != "" { + nodeServerOp = append(nodeServerOp, "--gid="+s3.GID) + workerNodeOp.GID = s3.GID + } + + switch s3.LogLevel { + case "debug": + nodeServerOp = append(nodeServerOp, "--debug") + workerNodeOp.LogLevel = "debug" + case "debug-crt": + nodeServerOp = append(nodeServerOp, "--debug-crt") + workerNodeOp.LogLevel = "debug-crt" + case "no-log": + nodeServerOp = append(nodeServerOp, "--no-log") + workerNodeOp.LogLevel = "no-log" + default: + if s3.LogLevel != "" { + klog.Warningf("MountpointS3Mounter: unsupported log-level %q. "+ + "Supported: 'debug', 'debug-crt', 'no-log'. Ignoring.", s3.LogLevel) + } + } + + // log-directory is hardcoded — always passed, never user-configurable. + nodeServerOp = append(nodeServerOp, "--log-directory="+s3MountLogDirectory) + workerNodeOp.LogDirectory = s3MountLogDirectory + + if s3.CacheDir != "" { + nodeServerOp = append(nodeServerOp, "--cache="+s3.CacheDir) + workerNodeOp.CacheDir = s3.CacheDir + } + + // force-path-style is hardcoded — required for IBM COS compatibility. + nodeServerOp = append(nodeServerOp, "--force-path-style") + workerNodeOp.ForcePathStyle = "true" + + // allow-delete and allow-overwrite are hardcoded defaults, suppressed when read-only. + if s3.AllowDelete { + nodeServerOp = append(nodeServerOp, "--allow-delete") + workerNodeOp.AllowDelete = "true" + } + if s3.AllowOverwrite { + nodeServerOp = append(nodeServerOp, "--allow-overwrite") + workerNodeOp.AllowOverwrite = "true" + } + if s3.IncrementalUpload { + nodeServerOp = append(nodeServerOp, "--incremental-upload") + workerNodeOp.IncrementalUpload = "true" + } + if s3.ReadOnly { + nodeServerOp = append(nodeServerOp, "--read-only") + workerNodeOp.ReadOnly = "true" + } + + // Passthrough — deduplicated so secret wins over SC. + // Set on BOTH paths so the worker daemon receives them too. + nodeServerOp = append(nodeServerOp, s3.MountOptions...) + workerNodeOp.Args = s3.MountOptions + + klog.Infof("MountpointS3 nodeServerOp: %v", nodeServerOp) + klog.Infof("MountpointS3 workerNodeOp: %+v", workerNodeOp) + + return nodeServerOp, envVars, workerNodeOp +} + +// removeS3MountConfigFile removes the config directory for the given volume target. +func removeS3MountConfigFile(configPath, target string) { + configPathWithVolID := path.Join(configPath, fmt.Sprintf("%x", sha256.Sum256([]byte(target)))) + + for retry := 1; retry <= maxRetries; retry++ { + _, err := Stat(configPathWithVolID) + if err != nil { + if os.IsNotExist(err) { + klog.Infof("removeS3MountConfigFile: Config dir does not exist: %s", configPathWithVolID) + return + } + klog.Errorf("removeS3MountConfigFile: Attempt %d - Failed to stat path %s: %v", retry, configPathWithVolID, err) + time.Sleep(constants.Interval) + continue + } + if err = RemoveAll(configPathWithVolID); err != nil { + klog.Errorf("removeS3MountConfigFile: Attempt %d - Failed to remove config dir %s: %v", retry, configPathWithVolID, err) + time.Sleep(constants.Interval) + continue + } + klog.Infof("removeS3MountConfigFile: Successfully removed config dir: %s", configPathWithVolID) + return + } + klog.Errorf("removeS3MountConfigFile: Failed to remove config dir after %d attempts", maxRetries) +} diff --git a/pkg/mounter/mounter-mount-s3_test.go b/pkg/mounter/mounter-mount-s3_test.go new file mode 100644 index 00000000..4ca9bd96 --- /dev/null +++ b/pkg/mounter/mounter-mount-s3_test.go @@ -0,0 +1,866 @@ +package mounter + +import ( + "errors" + mounterUtils "github.com/IBM/ibm-object-csi-driver/pkg/mounter/utils" + "github.com/stretchr/testify/assert" + "os" + "testing" +) + +var ( + s3MounterSecretMap = map[string]string{ + "cosEndpoint": "https://s3.us-east.cloud-object-storage.appdomain.cloud", + "locationConstraint": "us-east", + "bucketName": "test-bucket", + "objectPath": "test-path", + "accessKey": "test-access-key", + "secretKey": "test-secret-key", + "uid": "1000", + "mountOptions": `log-level=debug +cache=/tmp/cache`, + } + + s3MounterMountOptions = []string{"--dir-mode=0755", "--file-mode=0644"} +) + +func TestNewMountpointS3Mounter_Success(t *testing.T) { + mounter := NewMountpointS3Mounter(MountpointS3MounterParams{ + SecretMap: s3MounterSecretMap, + MountOptions: s3MounterMountOptions, + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{}), + Gid: "1000", + ReadOnly: true, + }) + + s3Mounter, ok := mounter.(*MountpointS3Mounter) + assert.True(t, ok) + + assert.Equal(t, s3MounterSecretMap["bucketName"], s3Mounter.BucketName) + assert.Equal(t, s3MounterSecretMap["objectPath"], s3Mounter.ObjectPath) + assert.Equal(t, s3MounterSecretMap["cosEndpoint"], s3Mounter.EndPoint) + assert.Equal(t, s3MounterSecretMap["locationConstraint"], s3Mounter.LocConstraint) + assert.Equal(t, s3MounterSecretMap["accessKey"], s3Mounter.AccessKey) + assert.Equal(t, s3MounterSecretMap["secretKey"], s3Mounter.SecretKey) + assert.Equal(t, s3MounterSecretMap["uid"], s3Mounter.UID) + assert.Equal(t, "1000", s3Mounter.GID) + assert.Equal(t, "debug", s3Mounter.LogLevel) + assert.True(t, s3Mounter.ReadOnly) + assert.Equal(t, "/tmp/cache", s3Mounter.CacheDir) + assert.Equal(t, "hmac", s3Mounter.AuthType) + // MountOptions should contain SC options (dir-mode, file-mode) + assert.Contains(t, s3Mounter.MountOptions, "--dir-mode=0755") + assert.Contains(t, s3Mounter.MountOptions, "--file-mode=0644") +} + +func TestNewMountpointS3Mounter_GidWithoutUid(t *testing.T) { + secretMap := map[string]string{} + + mounter := NewMountpointS3Mounter(MountpointS3MounterParams{ + SecretMap: secretMap, + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{}), + Gid: "2000", + }) + + s3Mounter, ok := mounter.(*MountpointS3Mounter) + assert.True(t, ok) + // When gid is set and no uid in secretMap, UID is also set to gid value. + assert.Equal(t, "2000", s3Mounter.UID) + assert.Equal(t, "2000", s3Mounter.GID) +} + +func TestNewMountpointS3Mounter_UidOverridesGid(t *testing.T) { + secretMap := map[string]string{ + "uid": "1000", + } + + mounter := NewMountpointS3Mounter(MountpointS3MounterParams{ + SecretMap: secretMap, + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{}), + Gid: "2000", + }) + + s3Mounter, ok := mounter.(*MountpointS3Mounter) + assert.True(t, ok) + // uid from secretMap takes precedence over gid param for the UID field. + assert.Equal(t, "1000", s3Mounter.UID) + assert.Equal(t, "2000", s3Mounter.GID) +} + +func TestMountpointS3Mount_NodeServer_Positive(t *testing.T) { + mountWorker = false + + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return os.CreateTemp("", "test") + } + + mockUtils := mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{ + FuseMountWithEnvFn: func(path, comm string, args, envVars []string) error { + return nil + }, + }) + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mockUtils, + BucketName: "test-bucket", + ObjectPath: "test-path", + EndPoint: "https://s3.us-east.cloud-object-storage.appdomain.cloud", + LocConstraint: "us-east", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + UID: "1000", + GID: "1000", + LogLevel: "debug", + ReadOnly: true, + MountOptions: s3MounterMountOptions, + } + + err := s3Mounter.Mount(source, target) + assert.NoError(t, err) +} + +func TestMountpointS3Mount_NodeServer_WithEnvMounter(t *testing.T) { + mountWorker = false + + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return os.CreateTemp("", "test") + } + + mockUtils := mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{ + FuseMountWithEnvFn: func(path, comm string, args, envVars []string) error { + return nil + }, + }) + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mockUtils, + BucketName: "test-bucket", + EndPoint: "https://s3.us-east.cloud-object-storage.appdomain.cloud", + LocConstraint: "us-east", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + MountOptions: s3MounterMountOptions, + } + + err := s3Mounter.Mount(source, target) + assert.NoError(t, err) +} + +func TestMountpointS3Mount_WorkerNode_Positive(t *testing.T) { + mountWorker = true + + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return os.CreateTemp("", "test") + } + mounterRequest = func(payload, url string) error { + return nil + } + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{}), + BucketName: "test-bucket", + EndPoint: "https://s3.us-east.cloud-object-storage.appdomain.cloud", + LocConstraint: "us-east", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + } + + err := s3Mounter.Mount(source, target) + assert.NoError(t, err) +} + +func TestMountpointS3Mount_CreateDirFails_Negative(t *testing.T) { + mountWorker = false + + MakeDir = func(path string, perm os.FileMode) error { + return errors.New("failed to create directory") + } + + s3Mounter := &MountpointS3Mounter{ + BucketName: "test-bucket", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + } + + err := s3Mounter.Mount(source, target) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create directory") +} + +func TestMountpointS3Mount_CreateCredFileFails_Negative(t *testing.T) { + mountWorker = false + + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return nil, errors.New("failed to create file") + } + + s3Mounter := &MountpointS3Mounter{ + BucketName: "test-bucket", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + } + + err := s3Mounter.Mount(source, target) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot create file") +} + +func TestMountpointS3Mount_WorkerNode_Negative(t *testing.T) { + mountWorker = true + + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return os.CreateTemp("", "test") + } + mounterRequest = func(payload, url string) error { + return errors.New("failed to perform http request") + } + + s3Mounter := &MountpointS3Mounter{ + BucketName: "test-bucket", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + } + + err := s3Mounter.Mount(source, target) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to perform http request") +} + +func TestMountpointS3Unmount_NodeServer(t *testing.T) { + mountWorker = false + + removeS3ConfigFile = func(configPath, target string) {} + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{ + FuseUnmountFn: func(path string) error { + return nil + }, + }), + } + + err := s3Mounter.Unmount(target) + assert.NoError(t, err) +} + +func TestMountpointS3Unmount_WorkerNode(t *testing.T) { + mountWorker = true + + removeS3ConfigFile = func(configPath, target string) {} + + mounterRequest = func(payload, url string) error { + return nil + } + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{ + FuseUnmountFn: func(path string) error { + return nil + }, + }), + } + + err := s3Mounter.Unmount(target) + assert.NoError(t, err) +} + +func TestMountpointS3Unmount_WorkerNode_Negative(t *testing.T) { + mountWorker = true + + mounterRequest = func(payload, url string) error { + return errors.New("failed to create http request") + } + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{ + FuseUnmountFn: func(path string) error { + return nil + }, + }), + } + + err := s3Mounter.Unmount(target) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create http request") +} + +func TestMountpointS3Unmount_NodeServer_Negative(t *testing.T) { + mountWorker = false + + removeS3ConfigFile = func(configPath, target string) {} + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{ + FuseUnmountFn: func(path string) error { + return errors.New("failed to unmount") + }, + }), + } + + err := s3Mounter.Unmount(target) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to unmount") +} + +func TestRemoveS3MountConfigFile_PathNotExists(t *testing.T) { + Stat = func(path string) (os.FileInfo, error) { + return nil, os.ErrNotExist + } + removeS3MountConfigFile("/test", target) +} + +func TestRemoveS3MountConfigFile_StatRetryThenSuccess(t *testing.T) { + attempt := 0 + Stat = func(_ string) (os.FileInfo, error) { + if attempt == 0 { + attempt++ + return nil, errors.New("stat error") + } + return nil, nil + } + RemoveAll = func(_ string) error { + return nil + } + + removeS3MountConfigFile("/test", target) +} + +func TestRemoveS3MountConfigFile_RemoveRetryThenSuccess(t *testing.T) { + Stat = func(_ string) (os.FileInfo, error) { + return nil, nil + } + attempt := 0 + RemoveAll = func(_ string) error { + if attempt == 0 { + attempt++ + return errors.New("remove error") + } + return nil + } + + removeS3MountConfigFile("/test", target) +} + +func TestRemoveS3MountConfigFile_Negative(t *testing.T) { + called := 0 + Stat = func(_ string) (os.FileInfo, error) { + return nil, nil + } + RemoveAll = func(_ string) error { + called++ + return errors.New("remove failed") + } + + removeS3MountConfigFile("/test", target) + assert.Equal(t, maxRetries, called) +} + +func TestFormulateMountOptions_AllOptions(t *testing.T) { + s3Mounter := &MountpointS3Mounter{ + BucketName: "test-bucket", + ObjectPath: "test-path", + EndPoint: "https://s3.us-east.cloud-object-storage.appdomain.cloud", + LocConstraint: "us-east", + UID: "1000", + GID: "2000", + LogLevel: "debug", + LogDirectory: "/var/log/mount-s3", + CacheDir: "/tmp/cache", + ReadOnly: true, + MountOptions: []string{"--dir-mode=0755", "--max-threads=32"}, + } + + nodeServerOp, envVars, workerNodeOp := s3Mounter.formulateMountOptions("test-bucket/test-path", target, "/config/path") + + // Check node server options + assert.Contains(t, nodeServerOp, "test-bucket/test-path") + assert.Contains(t, nodeServerOp, target) + assert.Contains(t, nodeServerOp, "--allow-other") + assert.Contains(t, nodeServerOp, "--endpoint-url=https://s3.us-east.cloud-object-storage.appdomain.cloud") + assert.Contains(t, nodeServerOp, "--region=us-east") + assert.Contains(t, nodeServerOp, "--uid=1000") + assert.Contains(t, nodeServerOp, "--gid=2000") + assert.Contains(t, nodeServerOp, "--debug") + assert.Contains(t, nodeServerOp, "--log-directory=/var/log/mount-s3") + assert.Contains(t, nodeServerOp, "--cache=/tmp/cache") + assert.Contains(t, nodeServerOp, "--read-only") + assert.Contains(t, nodeServerOp, "--dir-mode=0755") + assert.Contains(t, nodeServerOp, "--max-threads=32") + + // Check env vars + assert.Len(t, envVars, 2) + assert.Contains(t, envVars[0], "AWS_SHARED_CREDENTIALS_FILE=") + assert.Contains(t, envVars[1], "AWS_CONFIG_FILE=") + + // Check worker node options + assert.Equal(t, "true", workerNodeOp.AllowOther) + assert.Equal(t, "https://s3.us-east.cloud-object-storage.appdomain.cloud", workerNodeOp.EndpointURL) + assert.Equal(t, "us-east", workerNodeOp.Region) + assert.Equal(t, "1000", workerNodeOp.UID) + assert.Equal(t, "2000", workerNodeOp.GID) + assert.Equal(t, "debug", workerNodeOp.LogLevel) + assert.Equal(t, "/var/log/mount-s3", workerNodeOp.LogDirectory) + assert.Equal(t, "/tmp/cache", workerNodeOp.CacheDir) + assert.Equal(t, "true", workerNodeOp.ReadOnly) + assert.Contains(t, workerNodeOp.Args, "--dir-mode=0755") + assert.Contains(t, workerNodeOp.Args, "--max-threads=32") +} + +func TestFormulateMountOptions_LogLevelDebugCrt(t *testing.T) { + s3Mounter := &MountpointS3Mounter{ + LogLevel: "debug-crt", + } + + nodeServerOp, _, workerNodeOp := s3Mounter.formulateMountOptions("test-bucket", target, "/config/path") + + assert.Contains(t, nodeServerOp, "--debug-crt") + assert.Equal(t, "debug-crt", workerNodeOp.LogLevel) +} + +func TestFormulateMountOptions_LogLevelNoLog(t *testing.T) { + s3Mounter := &MountpointS3Mounter{ + LogLevel: "no-log", + } + + nodeServerOp, _, workerNodeOp := s3Mounter.formulateMountOptions("test-bucket", target, "/config/path") + + assert.Contains(t, nodeServerOp, "--no-log") + assert.Equal(t, "no-log", workerNodeOp.LogLevel) +} + +func TestFormulateMountOptions_InvalidLogLevel(t *testing.T) { + s3Mounter := &MountpointS3Mounter{ + LogLevel: "invalid-level", + } + + nodeServerOp, _, workerNodeOp := s3Mounter.formulateMountOptions("test-bucket", target, "/config/path") + + // Should not contain any log level flag + assert.NotContains(t, nodeServerOp, "--debug") + assert.NotContains(t, nodeServerOp, "--debug-crt") + assert.NotContains(t, nodeServerOp, "--no-log") + assert.Empty(t, workerNodeOp.LogLevel) +} + +func TestFormulateMountOptions_MinimalConfig(t *testing.T) { + s3Mounter := &MountpointS3Mounter{ + MountOptions: []string{}, + } + + nodeServerOp, envVars, workerNodeOp := s3Mounter.formulateMountOptions("test-bucket", target, "/config/path") + + // Check basic options are present + assert.Contains(t, nodeServerOp, "test-bucket") + assert.Contains(t, nodeServerOp, target) + assert.Contains(t, nodeServerOp, "--allow-other") + + // Check env vars + assert.Len(t, envVars, 2) + + // Check worker node options + assert.Equal(t, "true", workerNodeOp.AllowOther) +} + +func TestCreateS3MountConfig_Success(t *testing.T) { + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return os.CreateTemp("", "test") + } + + s3Mounter := &MountpointS3Mounter{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + LocConstraint: "us-east", + EndPoint: "https://s3.us-east.cloud-object-storage.appdomain.cloud", + } + + err := createS3MountConfig("/config/path", s3Mounter) + assert.NoError(t, err) +} + +func TestCreateS3MountConfig_MakeDirFails(t *testing.T) { + MakeDir = func(path string, perm os.FileMode) error { + return errors.New("failed to create directory") + } + + s3Mounter := &MountpointS3Mounter{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + } + + err := createS3MountConfig("/config/path", s3Mounter) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create directory") +} + +func TestCreateS3MountConfig_CreateFileFails(t *testing.T) { + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return nil, errors.New("failed to create file") + } + + s3Mounter := &MountpointS3Mounter{ + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + } + + err := createS3MountConfig("/config/path", s3Mounter) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create file") +} + +// Test deduplicatePassthrough function +func TestDeduplicatePassthrough(t *testing.T) { + tests := []struct { + name string + scFlags []string + secretFlags []string + expected []string + }{ + { + name: "No overlap - all flags kept", + scFlags: []string{"--allow-delete", "--dir-mode=0755"}, + secretFlags: []string{"--max-threads=128"}, + expected: []string{"--allow-delete", "--dir-mode=0755", "--max-threads=128"}, + }, + { + name: "Complete overlap - secret wins", + scFlags: []string{"--max-threads=32", "--maximum-throughput-gbps=10"}, + secretFlags: []string{"--max-threads=128", "--maximum-throughput-gbps=25"}, + expected: []string{"--max-threads=128", "--maximum-throughput-gbps=25"}, + }, + { + name: "Partial overlap - secret wins on duplicates", + scFlags: []string{"--max-threads=32", "--maximum-throughput-gbps=10", "--allow-delete"}, + secretFlags: []string{"--max-threads=128", "--maximum-throughput-gbps=25"}, + expected: []string{"--allow-delete", "--max-threads=128", "--maximum-throughput-gbps=25"}, + }, + { + name: "Empty SC flags", + scFlags: []string{}, + secretFlags: []string{"--max-threads=128"}, + expected: []string{"--max-threads=128"}, + }, + { + name: "Empty secret flags", + scFlags: []string{"--max-threads=32"}, + secretFlags: []string{}, + expected: []string{"--max-threads=32"}, + }, + { + name: "Both empty", + scFlags: []string{}, + secretFlags: []string{}, + expected: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := deduplicatePassthrough(tt.scFlags, tt.secretFlags) + assert.ElementsMatch(t, tt.expected, result) + }) + } +} + +// Test flagKey function +func TestFlagKey(t *testing.T) { + tests := []struct { + name string + flag string + expected string + }{ + { + name: "Flag with value", + flag: "--max-threads=128", + expected: "max-threads", + }, + { + name: "Flag without value", + flag: "--allow-delete", + expected: "allow-delete", + }, + { + name: "Flag without double dash", + flag: "max-threads=128", + expected: "max-threads", + }, + { + name: "Complex flag with multiple equals", + flag: "--endpoint-url=https://s3.example.com", + expected: "endpoint-url", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := flagKey(tt.flag) + assert.Equal(t, tt.expected, result) + }) + } +} + +// Test splitSecretMountOptions function +func TestSplitSecretMountOptions(t *testing.T) { + tests := []struct { + name string + raw string + expected []string + }{ + { + name: "Basic multiline options", + raw: `log-level=debug +read-only +cache=/tmp/cache`, + expected: []string{"log-level=debug", "read-only", "cache=/tmp/cache"}, + }, + { + name: "Options with trailing commas", + raw: `log-level=debug, +read-only, +cache=/tmp/cache,`, + expected: []string{"log-level=debug", "read-only", "cache=/tmp/cache"}, + }, + { + name: "Options with comments", + raw: `log-level=debug +# This is a comment +read-only +cache=/tmp/cache`, + expected: []string{"log-level=debug", "read-only", "cache=/tmp/cache"}, + }, + { + name: "Options with empty lines", + raw: `log-level=debug + +read-only + +cache=/tmp/cache`, + expected: []string{"log-level=debug", "read-only", "cache=/tmp/cache"}, + }, + { + name: "Options with whitespace", + raw: ` log-level=debug + read-only + cache=/tmp/cache `, + expected: []string{"log-level=debug", "read-only", "cache=/tmp/cache"}, + }, + { + name: "Empty string", + raw: "", + expected: nil, + }, + { + name: "Only comments and empty lines", + raw: `# Comment 1 + +# Comment 2`, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := splitSecretMountOptions(tt.raw) + assert.Equal(t, tt.expected, result) + }) + } +} + +// Test parseMountpointS3Options function +func TestParseMountpointS3Options(t *testing.T) { + tests := []struct { + name string + opts []string + expectedMounter *MountpointS3Mounter + expectedRemaining []string + }{ + { + name: "Parse structured options", + opts: []string{"--uid=1000", "--gid=2000", "--log-level=debug", "--cache=/tmp/cache", "--read-only"}, + expectedMounter: &MountpointS3Mounter{ + UID: "1000", + GID: "2000", + LogLevel: "debug", + CacheDir: "/tmp/cache", + // read-only is now set via MounterParams.ReadOnly, not parsed from flags. + ReadOnly: false, + }, + expectedRemaining: []string{}, + }, + { + name: "Parse with passthrough options", + opts: []string{"--uid=1000", "--max-threads=32", "--dir-mode=0755"}, + expectedMounter: &MountpointS3Mounter{ + UID: "1000", + }, + expectedRemaining: []string{"--max-threads=32", "--dir-mode=0755"}, + }, + { + name: "Parse allow-overwrite and incremental-upload", + opts: []string{"--allow-overwrite", "--incremental-upload"}, + expectedMounter: &MountpointS3Mounter{ + // allow-overwrite is now hardcoded — parsed value is ignored. + AllowOverwrite: false, + IncrementalUpload: true, + }, + expectedRemaining: []string{}, + }, + { + name: "Parse log-directory", + // log-directory is now hardcoded — any user-supplied value is ignored and + // the struct is always set to the fixed path s3MountLogDirectory. + opts: []string{"--log-directory=/some/custom/path"}, + expectedMounter: &MountpointS3Mounter{ + LogDirectory: s3MountLogDirectory, + }, + expectedRemaining: []string{}, + }, + { + name: "Ignore allow-other", + opts: []string{"--allow-other", "--uid=1000"}, + expectedMounter: &MountpointS3Mounter{ + UID: "1000", + }, + expectedRemaining: []string{}, + }, + { + name: "Add double dash to options without it", + opts: []string{"max-threads=32", "dir-mode=0755"}, + expectedMounter: &MountpointS3Mounter{}, + expectedRemaining: []string{"--max-threads=32", "--dir-mode=0755"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mounter := &MountpointS3Mounter{} + remaining := parseMountpointS3Options(mounter, tt.opts) + + assert.Equal(t, tt.expectedMounter.UID, mounter.UID) + assert.Equal(t, tt.expectedMounter.GID, mounter.GID) + assert.Equal(t, tt.expectedMounter.LogLevel, mounter.LogLevel) + assert.Equal(t, tt.expectedMounter.LogDirectory, mounter.LogDirectory) + assert.Equal(t, tt.expectedMounter.CacheDir, mounter.CacheDir) + assert.Equal(t, tt.expectedMounter.ReadOnly, mounter.ReadOnly) + assert.Equal(t, tt.expectedMounter.AllowOverwrite, mounter.AllowOverwrite) + assert.Equal(t, tt.expectedMounter.IncrementalUpload, mounter.IncrementalUpload) + assert.ElementsMatch(t, tt.expectedRemaining, remaining) + }) + } +} + +// Test incremental-upload blocking +func TestNewMountpointS3Mounter_IncrementalUploadBlocked(t *testing.T) { + secretMap := map[string]string{ + "bucketName": "test-bucket", + "accessKey": "test-access-key", + "secretKey": "test-secret-key", + "mountOptions": `incremental-upload +allow-overwrite`, + } + + mounter := NewMountpointS3Mounter(MountpointS3MounterParams{ + SecretMap: secretMap, + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{}), + }) + + s3Mounter, ok := mounter.(*MountpointS3Mounter) + assert.True(t, ok) + + // incremental-upload should be blocked (set to false) + assert.False(t, s3Mounter.IncrementalUpload, "incremental-upload should be blocked due to data loss issues") + // allow-overwrite should still be set + assert.True(t, s3Mounter.AllowOverwrite) +} + +// Test SC and secret mount options precedence +func TestNewMountpointS3Mounter_MountOptionsPrecedence(t *testing.T) { + secretMap := map[string]string{ + "bucketName": "test-bucket", + "accessKey": "test-access-key", + "secretKey": "test-secret-key", + "mountOptions": `max-threads=128 +maximum-throughput-gbps=25`, + } + + // --allow-delete is now hardcoded — it must not appear in passthrough MountOptions. + scMountOptions := []string{"--max-threads=32", "--maximum-throughput-gbps=10", "--allow-delete"} + + mounter := NewMountpointS3Mounter(MountpointS3MounterParams{ + SecretMap: secretMap, + MountOptions: scMountOptions, + MounterUtils: mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{}), + }) + + s3Mounter, ok := mounter.(*MountpointS3Mounter) + assert.True(t, ok) + + // Secret options should override SC options for same keys. + assert.Contains(t, s3Mounter.MountOptions, "--max-threads=128") + assert.Contains(t, s3Mounter.MountOptions, "--maximum-throughput-gbps=25") + assert.NotContains(t, s3Mounter.MountOptions, "--max-threads=32") + assert.NotContains(t, s3Mounter.MountOptions, "--maximum-throughput-gbps=10") + + // allow-delete is now hardcoded — it must NOT appear in the passthrough slice. + assert.NotContains(t, s3Mounter.MountOptions, "--allow-delete") + // But it is always enabled via the struct field. + assert.True(t, s3Mounter.AllowDelete) +} + +// Test read-only priority resolution in Mount +func TestMountpointS3Mount_ReadOnlyPriority(t *testing.T) { + mountWorker = false + + MakeDir = func(path string, perm os.FileMode) error { + return nil + } + OpenFile = func(path string, flag int, perm os.FileMode) (*os.File, error) { + return os.CreateTemp("", "test") + } + + mockUtils := mounterUtils.NewFakeMounterUtilsImpl(mounterUtils.FakeMounterUtilsFuncStruct{ + FuseMountWithEnvFn: func(path, comm string, args, envVars []string) error { + return nil + }, + }) + + s3Mounter := &MountpointS3Mounter{ + MounterUtils: mockUtils, + BucketName: "test-bucket", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + ReadOnly: true, + AllowDelete: true, // Should be cleared by read-only + AllowOverwrite: true, // Should be cleared by read-only + MountOptions: []string{"--max-threads=32"}, + } + + err := s3Mounter.Mount(source, target) + assert.NoError(t, err) + + // Verify read-only priority cleared all write-enabling flags. + assert.False(t, s3Mounter.AllowDelete) + assert.False(t, s3Mounter.AllowOverwrite) + // Passthrough options unrelated to write-access are preserved. + assert.Contains(t, s3Mounter.MountOptions, "--max-threads=32") +} diff --git a/pkg/mounter/mounter-s3fs_test.go b/pkg/mounter/mounter-s3fs_test.go index 96247fbe..11ad88b5 100644 --- a/pkg/mounter/mounter-s3fs_test.go +++ b/pkg/mounter/mounter-s3fs_test.go @@ -14,9 +14,9 @@ import ( var ( secretMap = map[string]string{ - "cosEndpoint": "test-endpoint", - "locationConstraint": "test-loc-constraint", - "bucketName": "test-bucket-name", + "cosEndpoint": "test-endpoint", + "locationConstraint": "test-loc-constraint", + "bucketName": "test-bucket-name", "objectPath": "test-obj-path", constants.AccessKey: "test-access-key", constants.SecretKey: "test-secret-key", @@ -48,18 +48,18 @@ func TestNewS3fsMounter_Success(t *testing.T) { func TestNewS3fsMounter_Success_Hmac(t *testing.T) { secretMap := map[string]string{ - "cosEndpoint": "test-endpoint", - "locationConstraint": "test-loc-constraint", - "bucketName": "test-bucket-name", + "cosEndpoint": "test-endpoint", + "locationConstraint": "test-loc-constraint", + "bucketName": "test-bucket-name", "objectPath": "test-obj-path", constants.AccessKey: "test-access-key", constants.SecretKey: "test-secret-key", constants.KpRootKeyCRN: "test-kp-root-key-crn", "mountOptions": "\nkey1\nkey2=value2\nkey=val1=val2", - "tmpdir": "test-tmpdir", - "use_cache": "true", - "gid": "test-gid", - "iamEndpoint": "test-iamEndpoint", + "tmpdir": "test-tmpdir", + "use_cache": "true", + "gid": "test-gid", + "iamEndpoint": "test-iamEndpoint", } mountOptions := []string{"opt1=val1", "opt2=val2", " ", "opt3"} @@ -301,11 +301,11 @@ func TestRemoveS3FSCredFile_Negative(t *testing.T) { func TestAddMountParam_Integration(t *testing.T) { tests := []struct { - name string - defaultOpts []string - secretOpts string - wantUnknown []string - wantEmpty bool + name string + defaultOpts []string + secretOpts string + wantUnknown []string + wantEmpty bool }{ {"OnlyKnown", []string{"allow_other"}, "kernel_cache", nil, true}, {"OnlyUnknown", nil, "enable_content_md5\ncomplement_stat", []string{"enable_content_md5", "complement_stat"}, false}, @@ -390,7 +390,7 @@ func TestUpdateS3FSMountOptions_SpecialSecretFields(t *testing.T) { t.Run(tt.name, func(t *testing.T) { opts, _ := updateS3FSMountOptions(tt.defaultOpts, tt.secret, GetKnownS3FSOptions(), nil, "", false) assert.NotNil(t, opts) - + optsStr := strings.Join(opts, " ") if tt.wantUID { assert.Contains(t, optsStr, "uid=") @@ -407,9 +407,9 @@ func TestUpdateS3FSMountOptions_DefaultParams(t *testing.T) { "cipher_suites": "AESGCM", "empty_param": "", } - + opts, _ := updateS3FSMountOptions(nil, map[string]string{}, GetKnownS3FSOptions(), defaultParams, "", false) - + optsStr := strings.Join(opts, " ") assert.Contains(t, optsStr, "cipher_suites=AESGCM") assert.NotContains(t, optsStr, "empty_param") @@ -448,11 +448,11 @@ func TestUpdateS3FSMountOptions_ReadOnly(t *testing.T) { func TestUpdateS3FSMountOptions_GidParam(t *testing.T) { tests := []struct { - name string - secret map[string]string - gid string - wantGID string - wantUID string + name string + secret map[string]string + gid string + wantGID string + wantUID string }{ { name: "gid param overrides secretMap gid", @@ -511,44 +511,44 @@ func TestUpdateS3FSMountOptionsWithUnknownOptions(t *testing.T) { expectAddMountParamContains string }{ { - name: "Known and unknown options", - defaultMountOp: []string{"allow_other", "enable_content_md5=true"}, - secretMap: map[string]string{"mountOptions": "cipher_suites=default"}, + name: "Known and unknown options", + defaultMountOp: []string{"allow_other", "enable_content_md5=true"}, + secretMap: map[string]string{"mountOptions": "cipher_suites=default"}, expectAddMountParamPresent: true, expectAddMountParamContains: "enable_content_md5=true", }, { - name: "Only known options", - defaultMountOp: []string{"allow_other"}, - secretMap: map[string]string{}, + name: "Only known options", + defaultMountOp: []string{"allow_other"}, + secretMap: map[string]string{}, expectAddMountParamPresent: false, expectAddMountParamContains: "", }, { - name: "Unknown option without value", - defaultMountOp: []string{"custom_flag"}, - secretMap: map[string]string{}, + name: "Unknown option without value", + defaultMountOp: []string{"custom_flag"}, + secretMap: map[string]string{}, expectAddMountParamPresent: true, expectAddMountParamContains: "custom_flag", }, { - name: "Multiple unknown options from secretMap", - defaultMountOp: []string{}, - secretMap: map[string]string{"mountOptions": "unknown1=val1\nunknown2=val2"}, + name: "Multiple unknown options from secretMap", + defaultMountOp: []string{}, + secretMap: map[string]string{"mountOptions": "unknown1=val1\nunknown2=val2"}, expectAddMountParamPresent: true, expectAddMountParamContains: "unknown1=val1", }, { - name: "Empty line in mountOptions", - defaultMountOp: []string{}, - secretMap: map[string]string{"mountOptions": "allow_other\n\nunknown_opt=test"}, + name: "Empty line in mountOptions", + defaultMountOp: []string{}, + secretMap: map[string]string{"mountOptions": "allow_other\n\nunknown_opt=test"}, expectAddMountParamPresent: true, expectAddMountParamContains: "unknown_opt=test", }, { - name: "Invalid option in defaultMountOp", - defaultMountOp: []string{"", "allow_other"}, - secretMap: map[string]string{}, + name: "Invalid option in defaultMountOp", + defaultMountOp: []string{"", "allow_other"}, + secretMap: map[string]string{}, expectAddMountParamPresent: false, expectAddMountParamContains: "", }, diff --git a/pkg/mounter/mounter.go b/pkg/mounter/mounter.go index 47337fdc..e0b8cc7e 100644 --- a/pkg/mounter/mounter.go +++ b/pkg/mounter/mounter.go @@ -27,6 +27,7 @@ var ( MakeDir = os.MkdirAll CreateFile = os.Create Chmod = os.Chmod + OpenFile = os.OpenFile Stat = os.Stat RemoveAll = os.RemoveAll ) @@ -39,13 +40,13 @@ type Mounter interface { type CSIMounterFactory struct{} type MounterParams struct { - Attrib map[string]string - SecretMap map[string]string - MountFlags []string + Attrib map[string]string + SecretMap map[string]string + MountFlags []string KnownS3FSOptions *pkgutils.Set - DefaultMOMap map[string]string - Gid string - ReadOnly bool + DefaultMOMap map[string]string + Gid string + ReadOnly bool } type NewMounterFactory interface { @@ -104,6 +105,14 @@ func (s *CSIMounterFactory) NewMounter(params MounterParams) Mounter { Gid: params.Gid, ReadOnly: params.ReadOnly, }) + case constants.AMAZONS3MOUNTER: + return NewMountpointS3Mounter(MountpointS3MounterParams{ + SecretMap: secretMap, + MountOptions: mountFlags, + MounterUtils: mounterUtils, + Gid: params.Gid, + ReadOnly: params.ReadOnly, + }) default: // default to s3fs return NewS3fsMounter(S3fsMounterParams{ diff --git a/pkg/mounter/mounter_test.go b/pkg/mounter/mounter_test.go index 1ce5837d..a098137d 100644 --- a/pkg/mounter/mounter_test.go +++ b/pkg/mounter/mounter_test.go @@ -90,6 +90,37 @@ func TestNewMounter(t *testing.T) { }, expectedErr: nil, }, + { + name: "MountpointS3 Mounter", + attrib: map[string]string{"mounter": constants.AMAZONS3MOUNTER}, + secretMap: map[string]string{ + "cosEndpoint": "test-endpoint", + "locationConstraint": "test-loc-constraint", + "bucketName": "test-bucket-name", + "objectPath": "test-obj-path", + "accessKey": "test-access-key", + "secretKey": "test-secret-key", + "uid": "1000", + "gid": "2000", + }, + mountOptions: []string{"--dir-mode=0755", "--file-mode=0644"}, + expected: &MountpointS3Mounter{ + BucketName: "test-bucket-name", + ObjectPath: "test-obj-path", + EndPoint: "test-endpoint", + LocConstraint: "test-loc-constraint", + AccessKey: "test-access-key", + SecretKey: "test-secret-key", + AuthType: "hmac", + UID: "1000", + GID: "2000", + AllowDelete: true, + AllowOverwrite: true, + MountOptions: []string{"--dir-mode=0755", "--file-mode=0644"}, + MounterUtils: &mounterUtils.MounterOptsUtils{}, + }, + expectedErr: nil, + }, { name: "Default Mounter", attrib: map[string]string{}, @@ -145,6 +176,14 @@ func TestNewMounter(t *testing.T) { rclone.MountOptions = nil expected.MountOptions = nil } + if s3mounter, ok := result.(*MountpointS3Mounter); ok { + expected := test.expected.(*MountpointS3Mounter) + if !stringSlicesEqualIgnoreOrder(s3mounter.MountOptions, expected.MountOptions) { + t.Errorf("MountOptions mismatch.\nGot: %v\nWant: %v", s3mounter.MountOptions, expected.MountOptions) + } + s3mounter.MountOptions = nil + expected.MountOptions = nil + } assert.Equal(t, result, test.expected) diff --git a/pkg/mounter/utils/fake_mounter_utils.go b/pkg/mounter/utils/fake_mounter_utils.go index 72a843cd..1a4c8edb 100644 --- a/pkg/mounter/utils/fake_mounter_utils.go +++ b/pkg/mounter/utils/fake_mounter_utils.go @@ -1,8 +1,9 @@ package utils type FakeMounterUtilsFuncStruct struct { - FuseMountFn func(path string, comm string, args []string) error - FuseUnmountFn func(path string) error + FuseMountFn func(path string, comm string, args []string) error + FuseUnmountFn func(path string) error + FuseMountWithEnvFn func(path string, comm string, args []string, envVars []string) error } type FakeMounterUtilsFuncStructImpl struct { @@ -28,3 +29,10 @@ func (m *FakeMounterUtilsFuncStructImpl) FuseUnmount(path string) error { } panic("requested method should not be nil") } + +func (m *FakeMounterUtilsFuncStructImpl) FuseMountWithEnv(path string, comm string, args []string, envVars []string) error { + if m.FuncStruct.FuseMountWithEnvFn != nil { + return m.FuncStruct.FuseMountWithEnvFn(path, comm, args, envVars) + } + panic("requested method should not be nil") +} diff --git a/pkg/mounter/utils/mounter_utils.go b/pkg/mounter/utils/mounter_utils.go index b3bbcb62..58e8944a 100644 --- a/pkg/mounter/utils/mounter_utils.go +++ b/pkg/mounter/utils/mounter_utils.go @@ -27,6 +27,7 @@ var ErrTimeoutWaitProcess = errors.New("timeout waiting for process to end") type MounterUtils interface { FuseUnmount(path string) error FuseMount(path string, comm string, args []string) error + FuseMountWithEnv(path string, comm string, args []string, envVars []string) error } type MounterOptsUtils struct { @@ -53,7 +54,17 @@ func redactMountArgs(args []string) []string { return redacted } +// FuseMount — signature unchanged, delegates to fuseMountInternal with no extra env vars. func (su *MounterOptsUtils) FuseMount(path string, comm string, args []string) error { + return su.fuseMountInternal(path, comm, args, nil) +} + +// FuseMountWithEnv — envVars are per-subprocess env vars (e.g. AWS_SHARED_CREDENTIALS_FILE) +func (su *MounterOptsUtils) FuseMountWithEnv(path string, comm string, args []string, envVars []string) error { + return su.fuseMountInternal(path, comm, args, envVars) +} + +func (su *MounterOptsUtils) fuseMountInternal(path string, comm string, args []string, envVars []string) error { klog.Info("-FuseMount-") klog.Infof("FuseMount: params:\n\tpath: <%s>\n\tcommand: <%s>\n\targs: <%v>", path, comm, redactMountArgs(args)) @@ -66,6 +77,26 @@ func (su *MounterOptsUtils) FuseMount(path string, comm string, args []string) e }() cmd := commandWithCtx(ctx, comm, args...) + + // If env vars provided, merge them with the inherited process env. + if len(envVars) > 0 { + overrides := make(map[string]bool, len(envVars)) + for _, e := range envVars { + key := strings.SplitN(e, "=", 2)[0] + overrides[key] = true + } + // Inherit all system env except keys we are overriding + baseEnv := os.Environ() + filteredEnv := make([]string, 0, len(baseEnv)+len(envVars)) + for _, e := range baseEnv { + key := strings.SplitN(e, "=", 2)[0] + if !overrides[key] { + filteredEnv = append(filteredEnv, e) + } + } + cmd.Env = append(filteredEnv, envVars...) + } + err := cmd.Start() if err != nil { klog.Errorf("FuseMount: command start failed: mounter=%s, args=%v, error=%v", comm, redactMountArgs(args), err) diff --git a/pkg/mounter/utils/mounter_utils_stub.go b/pkg/mounter/utils/mounter_utils_stub.go index 6e78291a..ede666f8 100644 --- a/pkg/mounter/utils/mounter_utils_stub.go +++ b/pkg/mounter/utils/mounter_utils_stub.go @@ -8,6 +8,7 @@ package utils type MounterUtils interface { FuseUnmount(path string) error FuseMount(path string, comm string, args []string) error + FuseMountWithEnv(path string, comm string, args []string, envVars []string) error } // MounterOptsUtils is an empty struct whose methods are defined in mounter_utils.go (Linux only). @@ -15,4 +16,7 @@ type MounterUtils interface { type MounterOptsUtils struct{} func (su *MounterOptsUtils) FuseMount(_ string, _ string, _ []string) error { return nil } -func (su *MounterOptsUtils) FuseUnmount(_ string) error { return nil } +func (su *MounterOptsUtils) FuseUnmount(_ string) error { return nil } +func (su *MounterOptsUtils) FuseMountWithEnv(_ string, _ string, _ []string, _ []string) error { + return nil +} diff --git a/pkg/utils/set.go b/pkg/utils/set.go index 980c6490..3656e330 100644 --- a/pkg/utils/set.go +++ b/pkg/utils/set.go @@ -61,4 +61,3 @@ func (s *Set) Contains(key string) bool { func (s *Set) Size() int { return len(s.m) } - diff --git a/tests/sanity/sanity_test.go b/tests/sanity/sanity_test.go index d20c5139..ec16aff0 100644 --- a/tests/sanity/sanity_test.go +++ b/tests/sanity/sanity_test.go @@ -231,6 +231,10 @@ func (m *FakeNewMounterOptsUtils) FuseMount(path string, comm string, args []str return nil } +func (m *FakeNewMounterOptsUtils) FuseMountWithEnv(path string, comm string, args []string, envVars []string) error { + return nil +} + // Fake DriverStatsUtils type FakeNewDriverStatsUtils struct { }