Merge branch 'remove/generic-args' into 'main'

refactor: remove commandargs.GenericArgs

Closes #212

See merge request gitlab-org/gitlab-shell!506

11 files changed, 41 insertions, 39 deletions

diff --git a/cmd/check/main.go b/cmd/check/main.go
index a6a8fa3..44d8175 100644
--- a/cmd/check/main.go
+++ b/cmd/check/main.go
@@ -19,7 +19,7 @@ func main() {
 		ErrOut: os.Stderr,
 	}
 
-	executable, err := executable.New(executable.Healthcheck)
+	executable, err := executable.New(executable.Healthcheck, false)
 	if err != nil {
 		fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting")
 		os.Exit(1)
diff --git a/cmd/gitlab-shell-authorized-keys-check/main.go b/cmd/gitlab-shell-authorized-keys-check/main.go
index 81937a7..cda3e0b 100644
--- a/cmd/gitlab-shell-authorized-keys-check/main.go
+++ b/cmd/gitlab-shell-authorized-keys-check/main.go
@@ -20,7 +20,7 @@ func main() {
 		ErrOut: os.Stderr,
 	}
 
-	executable, err := executable.New(executable.AuthorizedKeysCheck)
+	executable, err := executable.New(executable.AuthorizedKeysCheck, true)
 	if err != nil {
 		fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting")
 		os.Exit(1)
diff --git a/cmd/gitlab-shell-authorized-principals-check/main.go b/cmd/gitlab-shell-authorized-principals-check/main.go
index 8d0aba8..87f7fa3 100644
--- a/cmd/gitlab-shell-authorized-principals-check/main.go
+++ b/cmd/gitlab-shell-authorized-principals-check/main.go
@@ -20,7 +20,7 @@ func main() {
 		ErrOut: os.Stderr,
 	}
 
-	executable, err := executable.New(executable.AuthorizedPrincipalsCheck)
+	executable, err := executable.New(executable.AuthorizedPrincipalsCheck, true)
 	if err != nil {
 		fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting")
 		os.Exit(1)
diff --git a/cmd/gitlab-shell/main.go b/cmd/gitlab-shell/main.go
index e139ab7..fe52bfc 100644
--- a/cmd/gitlab-shell/main.go
+++ b/cmd/gitlab-shell/main.go
@@ -34,7 +34,7 @@ func main() {
 		ErrOut: os.Stderr,
 	}
 
-	executable, err := executable.New(executable.GitlabShell)
+	executable, err := executable.New(executable.GitlabShell, true)
 	if err != nil {
 		fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting")
 		os.Exit(1)
diff --git a/internal/command/command.go b/internal/command/command.go
index 6696f0f..dadf41a 100644
--- a/internal/command/command.go
+++ b/internal/command/command.go
@@ -29,9 +29,13 @@ type Command interface {
 }
 
 func New(e *executable.Executable, arguments []string, env sshenv.Env, config *config.Config, readWriter *readwriter.ReadWriter) (Command, error) {
-	args, err := commandargs.Parse(e, arguments, env)
-	if err != nil {
-		return nil, err
+	var args commandargs.CommandArgs
+	if e.AcceptArgs {
+		var err error
+		args, err = commandargs.Parse(e, arguments, env)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	if cmd := buildCommand(e, args, config, readWriter); cmd != nil {
diff --git a/internal/command/command_test.go b/internal/command/command_test.go
index 3617d39..a538745 100644
--- a/internal/command/command_test.go
+++ b/internal/command/command_test.go
@@ -26,10 +26,10 @@ import (
 )
 
 var (
-	authorizedKeysExec       = &executable.Executable{Name: executable.AuthorizedKeysCheck}
-	authorizedPrincipalsExec = &executable.Executable{Name: executable.AuthorizedPrincipalsCheck}
-	checkExec                = &executable.Executable{Name: executable.Healthcheck}
-	gitlabShellExec          = &executable.Executable{Name: executable.GitlabShell}
+	authorizedKeysExec       = &executable.Executable{Name: executable.AuthorizedKeysCheck, AcceptArgs: true}
+	authorizedPrincipalsExec = &executable.Executable{Name: executable.AuthorizedPrincipalsCheck, AcceptArgs: true}
+	checkExec                = &executable.Executable{Name: executable.Healthcheck, AcceptArgs: false}
+	gitlabShellExec          = &executable.Executable{Name: executable.GitlabShell, AcceptArgs: true}
 
 	basicConfig    = &config.Config{GitlabUrl: "http+unix://gitlab.socket"}
 	advancedConfig = &config.Config{GitlabUrl: "http+unix://gitlab.socket", SslCertDir: "/tmp/certs"}
diff --git a/internal/command/commandargs/command_args.go b/internal/command/commandargs/command_args.go
index b7d04a8..a01b8b2 100644
--- a/internal/command/commandargs/command_args.go
+++ b/internal/command/commandargs/command_args.go
@@ -1,6 +1,9 @@
 package commandargs
 
 import (
+	"errors"
+	"fmt"
+
 	"gitlab.com/gitlab-org/gitlab-shell/internal/executable"
 	"gitlab.com/gitlab-org/gitlab-shell/internal/sshenv"
 )
@@ -13,7 +16,7 @@ type CommandArgs interface {
 }
 
 func Parse(e *executable.Executable, arguments []string, env sshenv.Env) (CommandArgs, error) {
-	var args CommandArgs = &GenericArgs{Arguments: arguments}
+	var args CommandArgs
 
 	switch e.Name {
 	case executable.GitlabShell:
@@ -22,6 +25,8 @@ func Parse(e *executable.Executable, arguments []string, env sshenv.Env) (Comman
 		args = &AuthorizedKeys{Arguments: arguments}
 	case executable.AuthorizedPrincipalsCheck:
 		args = &AuthorizedPrincipals{Arguments: arguments}
+	default:
+		return nil, errors.New(fmt.Sprintf("unknown executable: %s", e.Name))
 	}
 
 	if err := args.Parse(); err != nil {
diff --git a/internal/command/commandargs/command_args_test.go b/internal/command/commandargs/command_args_test.go
index 7b9f0ad..119ecd4 100644
--- a/internal/command/commandargs/command_args_test.go
+++ b/internal/command/commandargs/command_args_test.go
@@ -16,6 +16,7 @@ func TestParseSuccess(t *testing.T) {
 		env          sshenv.Env
 		arguments    []string
 		expectedArgs CommandArgs
+		expectError  bool
 	}{
 		{
 			desc:         "It sets discover as the command when the command string was empty",
@@ -100,10 +101,10 @@ func TestParseSuccess(t *testing.T) {
 			arguments:    []string{"key", "principal-1", "principal-2"},
 			expectedArgs: &AuthorizedPrincipals{Arguments: []string{"key", "principal-1", "principal-2"}, KeyId: "key", Principals: []string{"principal-1", "principal-2"}},
 		}, {
-			desc:         "Unknown executable",
-			executable:   &executable.Executable{Name: "unknown"},
-			arguments:    []string{},
-			expectedArgs: &GenericArgs{Arguments: []string{}},
+			desc:        "Unknown executable",
+			executable:  &executable.Executable{Name: "unknown"},
+			arguments:   []string{},
+			expectError: true,
 		},
 	}
 
@@ -111,8 +112,12 @@ func TestParseSuccess(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			result, err := Parse(tc.executable, tc.arguments, tc.env)
 
-			require.NoError(t, err)
-			require.Equal(t, tc.expectedArgs, result)
+			if !tc.expectError {
+				require.NoError(t, err)
+				require.Equal(t, tc.expectedArgs, result)
+			} else {
+				require.Error(t, err)
+			}
 		})
 	}
 }
diff --git a/internal/command/commandargs/generic_args.go b/internal/command/commandargs/generic_args.go
deleted file mode 100644
index 96bed99..0000000
--- a/internal/command/commandargs/generic_args.go
+++ /dev/null
@@ -1,14 +0,0 @@
-package commandargs
-
-type GenericArgs struct {
-	Arguments []string
-}
-
-func (b *GenericArgs) Parse() error {
-	// Do nothing
-	return nil
-}
-
-func (b *GenericArgs) GetArguments() []string {
-	return b.Arguments
-}
diff --git a/internal/executable/executable.go b/internal/executable/executable.go
index c6355b9..8b6b586 100644
--- a/internal/executable/executable.go
+++ b/internal/executable/executable.go
@@ -14,8 +14,9 @@ const (
 )
 
 type Executable struct {
-	Name    string
-	RootDir string
+	Name       string
+	RootDir    string
+	AcceptArgs bool
 }
 
 var (
@@ -23,7 +24,7 @@ var (
 	osExecutable = os.Executable
 )
 
-func New(name string) (*Executable, error) {
+func New(name string, acceptArgs bool) (*Executable, error) {
 	path, err := osExecutable()
 	if err != nil {
 		return nil, err
@@ -35,8 +36,9 @@ func New(name string) (*Executable, error) {
 	}
 
 	executable := &Executable{
-		Name:    name,
-		RootDir: rootDir,
+		Name:       name,
+		RootDir:    rootDir,
+		AcceptArgs: acceptArgs,
 	}
 
 	return executable, nil
diff --git a/internal/executable/executable_test.go b/internal/executable/executable_test.go
index 3915f1a..71984c3 100644
--- a/internal/executable/executable_test.go
+++ b/internal/executable/executable_test.go
@@ -59,7 +59,7 @@ func TestNewSuccess(t *testing.T) {
 			fake.Setup()
 			defer fake.Cleanup()
 
-			result, err := New("gitlab-shell")
+			result, err := New("gitlab-shell", true)
 
 			require.NoError(t, err)
 			require.Equal(t, result.Name, "gitlab-shell")
@@ -96,7 +96,7 @@ func TestNewFailure(t *testing.T) {
 			fake.Setup()
 			defer fake.Cleanup()
 
-			_, err := New("gitlab-shell")
+			_, err := New("gitlab-shell", true)
 
 			require.Error(t, err)
 		})