Create mountpoint.Args for parsing and accessing Mountpoint args

cmd/aws-s3-csi-mounter/csimounter/csimounter.go

@@ -7,10 +7,10 @@ import (
 	"io/fs"
 	"os"
 	"os/exec"
-	"slices"
 
 	"k8s.io/klog/v2"
 
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/podmounter/mountoptions"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/podmounter/mppod"
 )
@@ -52,20 +52,17 @@ func Run(options Options) (int, error) {
 		return 0, fmt.Errorf("passed file descriptor %d is invalid", mountOptions.Fd)
 	}
 
-	args := mountOptions.Args
+	mountpointArgs := mountpoint.ParseArgs(mountOptions.Args)
 
 	// By default Mountpoint runs in a detached mode. Here we want to monitor it by relaying its output,
 	// and also we want to wait until it terminates. We're passing `--foreground` to achieve this.
-	const foreground, foregroundShort = "--foreground", "-f"
-	if !(slices.Contains(args, foreground) || slices.Contains(args, foregroundShort)) {
-		args = append(args, foreground)
-	}
+	mountpointArgs.Set(mountpoint.ArgForeground, mountpoint.ArgNoValue)
 
-	args = append([]string{
+	args := append([]string{
 		mountOptions.BucketName,
 		// We pass FUSE fd using `ExtraFiles`, and each entry becomes as file descriptor 3+i.
 		"/dev/fd/3",
-	}, args...)
+	}, mountpointArgs.SortedList()...)
 
 	cmd := exec.Command(options.MountpointPath, args...)
 	cmd.ExtraFiles = []*os.File{fuseDev}

pkg/driver/node/mounter/fake_mounter.go

@@ -1,9 +1,11 @@
 package mounter
 
+import "github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
+
 type FakeMounter struct{}
 
 func (m *FakeMounter) Mount(bucketName string, target string,
-	credentials *MountCredentials, options []string) error {
+	credentials *MountCredentials, args mountpoint.Args) error {
 	return nil
 }
 

pkg/driver/node/mounter/mount_credentials.go

@@ -20,7 +20,6 @@ const (
 	MountpointCacheKey          = "UNSTABLE_MOUNTPOINT_CACHE_KEY"
 	defaultMountS3Path          = "/usr/bin/mount-s3"
 	userAgentPrefix             = "--user-agent-prefix"
-	awsMaxAttemptsOption        = "--aws-max-attempts"
 )
 
 type MountCredentials struct {

pkg/driver/node/mounter/mounter.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"os"
 
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/system"
 )
 
@@ -15,7 +16,7 @@ type ServiceRunner interface {
 
 // Mounter is an interface for mount operations
 type Mounter interface {
-	Mount(bucketName string, target string, credentials *MountCredentials, options []string) error
+	Mount(bucketName string, target string, credentials *MountCredentials, args mountpoint.Args) error
 	Unmount(target string) error
 	IsMountPoint(target string) (bool, error)
 }

pkg/driver/node/mounter/systemd_mounter.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/awsprofile"
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/system"
 	"github.com/google/uuid"
 	"k8s.io/klog/v2"
@@ -79,7 +80,7 @@ func (m *SystemdMounter) IsMountPoint(target string) (bool, error) {
 //
 // This method will create the target path if it does not exist and if there is an existing corrupt
 // mount, it will attempt an unmount before attempting the mount.
-func (m *SystemdMounter) Mount(bucketName string, target string, credentials *MountCredentials, options []string) error {
+func (m *SystemdMounter) Mount(bucketName string, target string, credentials *MountCredentials, args mountpoint.Args) error {
 	if bucketName == "" {
 		return fmt.Errorf("bucket name is empty")
 	}
@@ -147,14 +148,14 @@ func (m *SystemdMounter) Mount(bucketName string, target string, credentials *Mo
 
 		env = credentials.Env(awsProfile)
 	}
-	options, env = moveOptionToEnvironmentVariables(awsMaxAttemptsOption, awsMaxAttemptsEnv, options, env)
-	options = addUserAgentToOptions(options, UserAgent(authenticationSource, m.kubernetesVersion))
+	args, env = moveArgumentsToEnv(args, env)
+	args = addUserAgentToArguments(args, UserAgent(authenticationSource, m.kubernetesVersion))
 
 	output, err := m.Runner.StartService(timeoutCtx, &system.ExecConfig{
 		Name:        "mount-s3-" + m.MpVersion + "-" + uuid.New().String() + ".service",
 		Description: "Mountpoint for Amazon S3 CSI driver FUSE daemon",
 		ExecPath:    m.MountS3Path,
-		Args:        append(options, bucketName, target),
+		Args:        append(args.SortedList(), bucketName, target),
 		Env:         env,
 	})
 
@@ -168,36 +169,18 @@ func (m *SystemdMounter) Mount(bucketName string, target string, credentials *Mo
 	return nil
 }
 
-// Moves a parameter optionName from the options list to MP's environment variable list. We need this as options are
-// passed to the driver in a single field, but MP sometimes only supports config from environment variables.
-// Returns an updated options and environment.
-func moveOptionToEnvironmentVariables(optionName string, envName string, options []string, env []string) ([]string, []string) {
-	optionIdx := -1
-	for i, o := range options {
-		if strings.HasPrefix(o, optionName) {
-			optionIdx = i
-			break
-		}
-	}
-	if optionIdx != -1 {
-		// We can do replace here as we've just verified it has the right prefix
-		env = append(env, strings.Replace(options[optionIdx], optionName, envName, 1))
-		options = append(options[:optionIdx], options[optionIdx+1:]...)
+func moveArgumentsToEnv(args mountpoint.Args, env []string) (mountpoint.Args, []string) {
+	if maxAttempts, ok := args.Remove(mountpoint.ArgAWSMaxAttempts); ok {
+		env = append(env, fmt.Sprintf("%s=%s", awsMaxAttemptsEnv, maxAttempts))
 	}
-	return options, env
+	return args, env
 }
 
-// method to add the user agent prefix to the Mountpoint headers
+// method to add the user agent prefix to the Mountpoint arguments.
 // https://github.com/awslabs/mountpoint-s3/pull/548
-func addUserAgentToOptions(options []string, userAgent string) []string {
-	// first remove it from the options in case it's in there
-	for i := len(options) - 1; i >= 0; i-- {
-		if strings.Contains(options[i], userAgentPrefix) {
-			options = append(options[:i], options[i+1:]...)
-		}
-	}
-	// add the hard coded S3 CSI driver user agent string
-	return append(options, userAgentPrefix+"="+userAgent)
+func addUserAgentToArguments(args mountpoint.Args, userAgent string) mountpoint.Args {
+	args.Set(mountpoint.ArgUserAgentPrefix, userAgent)
+	return args
 }
 
 func (m *SystemdMounter) Unmount(target string) error {

pkg/driver/node/mounter/systemd_mounter_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/awsprofile"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/mounter"
 	mock_driver "github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/mounter/mocks"
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/system"
 	"github.com/golang/mock/gomock"
 	"k8s.io/mount-utils"
@@ -156,7 +157,7 @@ func TestS3MounterMount(t *testing.T) {
 				testCase.before(t, env)
 			}
 			err := env.mounter.Mount(testCase.bucketName, testCase.targetPath,
-				testCase.credentials, testCase.options)
+				testCase.credentials, mountpoint.ParseArgs(testCase.options))
 			env.mockCtl.Finish()
 			if err != nil && !testCase.expectedErr {
 				t.Fatal(err)

pkg/driver/node/node.go

@@ -32,6 +32,7 @@ import (
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/mounter"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/targetpath"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/volumecontext"
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
 )
 
 const (
@@ -121,25 +122,16 @@ func (ns *S3NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePubl
 
 	mountpointArgs := []string{}
 	if req.GetReadonly() || volCap.GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY {
-		mountpointArgs = append(mountpointArgs, "--read-only")
+		mountpointArgs = append(mountpointArgs, mountpoint.ArgReadOnly)
 	}
 
-	// get the mount(point) options (yaml list)
 	if capMount := volCap.GetMount(); capMount != nil {
 		mountFlags := capMount.GetMountFlags()
-		for i := range mountFlags {
-			// trim left and right spaces
-			// trim spaces in between from multiple spaces to just one i.e. uid   1001 would turn into uid 1001
-			// if there is a space between, replace it with an = sign
-			mountFlags[i] = strings.Replace(strings.Join(strings.Fields(strings.Trim(mountFlags[i], " ")), " "), " ", "=", -1)
-			// prepend -- if it's not already there
-			if !strings.HasPrefix(mountFlags[i], "-") {
-				mountFlags[i] = "--" + mountFlags[i]
-			}
-		}
-		mountpointArgs = compileMountOptions(mountpointArgs, mountFlags)
+		mountpointArgs = append(mountpointArgs, mountFlags...)
 	}
 
+	args := mountpoint.ParseArgs(mountpointArgs)
+
 	credentials, err := ns.credentialProvider.Provide(ctx, req.VolumeId, req.VolumeContext, mountpointArgs)
 	if err != nil {
 		klog.Errorf("NodePublishVolume: failed to provide credentials: %v", err)
@@ -148,7 +140,7 @@ func (ns *S3NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePubl
 
 	klog.V(4).Infof("NodePublishVolume: mounting %s at %s with options %v", bucket, target, mountpointArgs)
 
-	if err := ns.Mounter.Mount(bucket, target, credentials, mountpointArgs); err != nil {
+	if err := ns.Mounter.Mount(bucket, target, credentials, args); err != nil {
 		os.Remove(target)
 		return nil, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", bucket, target, err)
 	}

pkg/driver/node/node_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/mounter"
 	mock_driver "github.com/awslabs/aws-s3-csi-driver/pkg/driver/node/mounter/mocks"
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/util/testutil/assert"
 	csi "github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/golang/mock/gomock"
@@ -100,7 +101,7 @@ func TestNodePublishVolume(t *testing.T) {
 					VolumeContext: map[string]string{"bucketName": bucketName},
 				}
 
-				nodeTestEnv.mockMounter.EXPECT().Mount(gomock.Eq(bucketName), gomock.Eq(targetPath), gomock.Any(), gomock.Eq([]string{"--read-only"}))
+				nodeTestEnv.mockMounter.EXPECT().Mount(gomock.Eq(bucketName), gomock.Eq(targetPath), gomock.Any(), gomock.Eq(mountpoint.ParseArgs([]string{"--read-only"})))
 				_, err := nodeTestEnv.server.NodePublishVolume(ctx, req)
 				if err != nil {
 					t.Fatalf("NodePublishVolume is failed: %v", err)
@@ -131,7 +132,7 @@ func TestNodePublishVolume(t *testing.T) {
 					Readonly:      true,
 				}
 
-				nodeTestEnv.mockMounter.EXPECT().Mount(gomock.Eq(bucketName), gomock.Eq(targetPath), gomock.Any(), gomock.Eq([]string{"--bar", "--foo", "--read-only", "--test=123"}))
+				nodeTestEnv.mockMounter.EXPECT().Mount(gomock.Eq(bucketName), gomock.Eq(targetPath), gomock.Any(), gomock.Eq(mountpoint.ParseArgs([]string{"--bar", "--foo", "--read-only", "--test=123"})))
 				_, err := nodeTestEnv.server.NodePublishVolume(ctx, req)
 				if err != nil {
 					t.Fatalf("NodePublishVolume is failed: %v", err)
@@ -164,7 +165,7 @@ func TestNodePublishVolume(t *testing.T) {
 
 				nodeTestEnv.mockMounter.EXPECT().Mount(
 					gomock.Eq(bucketName), gomock.Eq(targetPath), gomock.Any(),
-					gomock.Eq([]string{"--read-only", "--test=123"})).Return(nil)
+					gomock.Eq(mountpoint.ParseArgs([]string{"--read-only", "--test=123"}))).Return(nil)
 				_, err := nodeTestEnv.server.NodePublishVolume(ctx, req)
 				if err != nil {
 					t.Fatalf("NodePublishVolume is failed: %v", err)
@@ -341,7 +342,7 @@ var _ mounter.Mounter = &dummyMounter{}
 type dummyMounter struct {
 }
 
-func (d *dummyMounter) Mount(bucketName string, target string, credentials *mounter.MountCredentials, options []string) error {
+func (d *dummyMounter) Mount(bucketName string, target string, credentials *mounter.MountCredentials, args mountpoint.Args) error {
 	return nil
 }
 func (d *dummyMounter) Unmount(target string) error {