diff options
author | Patrick Bajao <ebajao@gitlab.com> | 2019-08-15 18:06:04 +0800 |
---|---|---|
committer | Patrick Bajao <ebajao@gitlab.com> | 2019-08-15 18:15:49 +0800 |
commit | 41f919eb86a3b1f69876f8b97572615b06521538 (patch) | |
tree | c840113cc7dc11643f5c8d7f55601322b2ee698e | |
parent | f34e2cd5c4194aa8bb049e1ac8aa1b2f002395b5 (diff) | |
download | gitlab-shell-41f919eb86a3b1f69876f8b97572615b06521538.tar.gz |
Replace symlinks with actual binaries
We had `gitlab-shell-authorized-keys-check` and
`gitlab-shell-authorized-principals-check` as symlinks to
`gitlab-shell` before.
We determine the `Command` and `CommandArgs` that we build based
on the `Name` of the `Executable`. We also use that to know which
fallback ruby executable should we fallback to. We use
`os.Executable()` to do that.
`os.Executable()` behaves differently depending on OS. It may
return the symlink or the target's name. That can result to a
buggy behavior.
The fix is to create binaries for each instead of using a symlink.
That way we don't need to rely on `os.Executable()` to get the name.
We pass the `Name` of the executable instead.
-rw-r--r-- | .gitignore | 2 | ||||
-rw-r--r-- | Makefile | 17 | ||||
l--------- | bin/gitlab-shell-authorized-keys-check | 1 | ||||
l--------- | bin/gitlab-shell-authorized-principals-check | 1 | ||||
-rw-r--r-- | go/cmd/gitlab-shell-authorized-keys-check/main.go | 46 | ||||
-rw-r--r-- | go/cmd/gitlab-shell-authorized-principals-check/main.go | 46 | ||||
-rw-r--r-- | go/cmd/gitlab-shell/main.go | 2 | ||||
-rw-r--r-- | go/internal/executable/executable.go | 4 | ||||
-rw-r--r-- | go/internal/executable/executable_test.go | 4 |
9 files changed, 106 insertions, 17 deletions
@@ -13,6 +13,8 @@ custom_hooks hooks/*.d /go_build /bin/gitlab-shell +/bin/gitlab-shell-authorized-keys-check +/bin/gitlab-shell-authorized-principals-check /bin/gitaly-upload-pack /bin/gitaly-receive-pack /bin/gitaly-upload-archive @@ -13,17 +13,14 @@ verify_golang: test: test_ruby test_golang test_ruby: - # bin/gitlab-shell must exist and needs to be the Ruby version for - # rspec to be able to test. + # bin/gitlab-shell, bin/gitlab-shell-authorized-keys-check and + # bin/gitlab-shell-authorized-principals-check must exist and need to be + # the Ruby version for rspec to be able to test. cp bin/gitlab-shell-ruby bin/gitlab-shell - # bin/gitlab-shell-authorized-keys-check and bin/gitlab-shell-authorized-principals-check - # should link to ruby scripts for rspec to be able to test. - ln -sf ./gitlab-shell-authorized-keys-check-ruby bin/gitlab-shell-authorized-keys-check - ln -sf ./gitlab-shell-authorized-principals-check-ruby bin/gitlab-shell-authorized-principals-check + cp bin/gitlab-shell-authorized-keys-check-ruby bin/gitlab-shell-authorized-keys-check + cp bin/gitlab-shell-authorized-principals-check-ruby bin/gitlab-shell-authorized-principals-check bundle exec rspec --color --tag '~go' --format d spec - rm -f bin/gitlab-shell - ln -sf ./gitlab-shell bin/gitlab-shell-authorized-keys-check - ln -sf ./gitlab-shell bin/gitlab-shell-authorized-principals-check + rm -f bin/gitlab-shell bin/gitlab-shell-authorized-keys-check bin/gitlab-shell-authorized-principals-check test_golang: support/go-test @@ -42,4 +39,4 @@ check: bin/check clean: - rm -f bin/gitlab-shell + rm -f bin/gitlab-shell bin/gitlab-shell-authorized-keys-check bin/gitlab-shell-authorized-principals-check diff --git a/bin/gitlab-shell-authorized-keys-check b/bin/gitlab-shell-authorized-keys-check deleted file mode 120000 index 3dc14d1..0000000 --- a/bin/gitlab-shell-authorized-keys-check +++ /dev/null @@ -1 +0,0 @@ -./gitlab-shell
\ No newline at end of file diff --git a/bin/gitlab-shell-authorized-principals-check b/bin/gitlab-shell-authorized-principals-check deleted file mode 120000 index 3dc14d1..0000000 --- a/bin/gitlab-shell-authorized-principals-check +++ /dev/null @@ -1 +0,0 @@ -./gitlab-shell
\ No newline at end of file diff --git a/go/cmd/gitlab-shell-authorized-keys-check/main.go b/go/cmd/gitlab-shell-authorized-keys-check/main.go new file mode 100644 index 0000000..2bde63d --- /dev/null +++ b/go/cmd/gitlab-shell-authorized-keys-check/main.go @@ -0,0 +1,46 @@ +package main + +import ( + "fmt" + "os" + + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/readwriter" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" +) + +func main() { + readWriter := &readwriter.ReadWriter{ + Out: os.Stdout, + In: os.Stdin, + ErrOut: os.Stderr, + } + + executable, err := executable.New(executable.AuthorizedKeysCheck) + if err != nil { + fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting") + os.Exit(1) + } + + config, err := config.NewFromDir(executable.RootDir) + if err != nil { + fmt.Fprintln(readWriter.ErrOut, "Failed to read config, exiting") + os.Exit(1) + } + + cmd, err := command.New(executable, os.Args[1:], config, readWriter) + if err != nil { + // For now this could happen if `SSH_CONNECTION` is not set on + // the environment + fmt.Fprintf(readWriter.ErrOut, "%v\n", err) + os.Exit(1) + } + + // The command will write to STDOUT on execution or replace the current + // process in case of the `fallback.Command` + if err = cmd.Execute(); err != nil { + fmt.Fprintf(readWriter.ErrOut, "%v\n", err) + os.Exit(1) + } +} diff --git a/go/cmd/gitlab-shell-authorized-principals-check/main.go b/go/cmd/gitlab-shell-authorized-principals-check/main.go new file mode 100644 index 0000000..121cc17 --- /dev/null +++ b/go/cmd/gitlab-shell-authorized-principals-check/main.go @@ -0,0 +1,46 @@ +package main + +import ( + "fmt" + "os" + + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/readwriter" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" +) + +func main() { + readWriter := &readwriter.ReadWriter{ + Out: os.Stdout, + In: os.Stdin, + ErrOut: os.Stderr, + } + + executable, err := executable.New(executable.AuthorizedPrincipalsCheck) + if err != nil { + fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting") + os.Exit(1) + } + + config, err := config.NewFromDir(executable.RootDir) + if err != nil { + fmt.Fprintln(readWriter.ErrOut, "Failed to read config, exiting") + os.Exit(1) + } + + cmd, err := command.New(executable, os.Args[1:], config, readWriter) + if err != nil { + // For now this could happen if `SSH_CONNECTION` is not set on + // the environment + fmt.Fprintf(readWriter.ErrOut, "%v\n", err) + os.Exit(1) + } + + // The command will write to STDOUT on execution or replace the current + // process in case of the `fallback.Command` + if err = cmd.Execute(); err != nil { + fmt.Fprintf(readWriter.ErrOut, "%v\n", err) + os.Exit(1) + } +} diff --git a/go/cmd/gitlab-shell/main.go b/go/cmd/gitlab-shell/main.go index b716820..9b0b6c5 100644 --- a/go/cmd/gitlab-shell/main.go +++ b/go/cmd/gitlab-shell/main.go @@ -17,7 +17,7 @@ func main() { ErrOut: os.Stderr, } - executable, err := executable.New() + executable, err := executable.New(executable.GitlabShell) if err != nil { fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting") os.Exit(1) diff --git a/go/internal/executable/executable.go b/go/internal/executable/executable.go index ef07115..2e7d26e 100644 --- a/go/internal/executable/executable.go +++ b/go/internal/executable/executable.go @@ -22,7 +22,7 @@ var ( osExecutable = os.Executable ) -func New() (*Executable, error) { +func New(name string) (*Executable, error) { path, err := osExecutable() if err != nil { return nil, err @@ -34,7 +34,7 @@ func New() (*Executable, error) { } executable := &Executable{ - Name: filepath.Base(path), + Name: name, RootDir: rootDir, } diff --git a/go/internal/executable/executable_test.go b/go/internal/executable/executable_test.go index 4d2174b..581821d 100644 --- a/go/internal/executable/executable_test.go +++ b/go/internal/executable/executable_test.go @@ -59,7 +59,7 @@ func TestNewSuccess(t *testing.T) { fake.Setup() defer fake.Cleanup() - result, err := New() + result, err := New("gitlab-shell") 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() + _, err := New("gitlab-shell") require.Error(t, err) }) |