diff options
author | Nick Thomas <nick@gitlab.com> | 2021-09-08 10:06:06 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2021-09-08 10:06:06 +0000 |
commit | 7884a4420ac8ffd3ee34589c0f8e0d25ca0fd076 (patch) | |
tree | 612c450010837d2dde0f11446c4cbe79bc20af49 | |
parent | 07bbfd279bc236229d95942372370b955db08b75 (diff) | |
parent | 8b4621aa6cba1674192ffb6e3c3e801a567f2516 (diff) | |
download | gitlab-shell-7884a4420ac8ffd3ee34589c0f8e0d25ca0fd076.tar.gz |
Merge branch 'remove/generic-args' into 'main'
refactor: remove commandargs.GenericArgs
Closes #212
See merge request gitlab-org/gitlab-shell!506
-rw-r--r-- | cmd/check/main.go | 2 | ||||
-rw-r--r-- | cmd/gitlab-shell-authorized-keys-check/main.go | 2 | ||||
-rw-r--r-- | cmd/gitlab-shell-authorized-principals-check/main.go | 2 | ||||
-rw-r--r-- | cmd/gitlab-shell/main.go | 2 | ||||
-rw-r--r-- | internal/command/command.go | 10 | ||||
-rw-r--r-- | internal/command/command_test.go | 8 | ||||
-rw-r--r-- | internal/command/commandargs/command_args.go | 7 | ||||
-rw-r--r-- | internal/command/commandargs/command_args_test.go | 17 | ||||
-rw-r--r-- | internal/command/commandargs/generic_args.go | 14 | ||||
-rw-r--r-- | internal/executable/executable.go | 12 | ||||
-rw-r--r-- | internal/executable/executable_test.go | 4 |
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) }) |