Merge branch '74-go-go-go-go-go' into 'master'

Feature flag for go/ruby gitlab-shell implementations

Closes #74

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

7 files changed, 130 insertions, 19 deletions

diff --git a/.gitignore b/.gitignore
index bee5f85..4ea71fb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,6 +12,7 @@ tags
 custom_hooks
 hooks/*.d
 /go_build
+/bin/gitlab-shell
 /bin/gitaly-upload-pack
 /bin/gitaly-receive-pack
 /bin/gitaly-upload-archive
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 47514bd..9e6e1a1 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -6,9 +6,17 @@ before_script:
   - cp config.yml.example config.yml
   - bundle install
 
-rspec:
+.rspec_definition: &rspec_definition
   script:
+    # Skip the experimental Golang wrapper in the Ruby specs. These are now
+    # primarily regression tests for particular versions of Ruby.
+    #
+    # The full rspec suite is also run against each suppported golang version
+    - cp bin/gitlab-shell-ruby bin/gitlab-shell
     - bundle exec rspec --color --format d spec
+
+rspec:
+  <<: *rspec_definition
   tags:
     - ruby
   except:
@@ -25,8 +33,7 @@ rubocop:
 #ruby 2.2
 rspec:ruby2.2:
   image: ruby:2.2
-  script:
-    - bundle exec rspec --color --format d spec
+  <<: *rspec_definition
   tags:
     - ruby
   except:
@@ -35,8 +42,7 @@ rspec:ruby2.2:
 #ruby 2.1
 rspec:ruby2.1:
   image: ruby:2.1
-  script:
-    - bundle exec rspec --color --format d spec
+  <<: *rspec_definition
   tags:
     - ruby
   except:
@@ -44,14 +50,22 @@ rspec:ruby2.1:
 
 .go: &go_definition
   before_script:
-  - apt-get update -qq && apt-get install -y ruby
+  - apt-get update -qq && apt-get install -y ruby ruby-dev
   - ruby -v
+  - export PATH=~/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin
+  - gem install --bindir /usr/local/bin bundler
+  - cp config.yml.example config.yml
+  - bundle install
   script:
     - go version
     - which go
     - bin/compile
     - support/go-test
     - support/go-format check
+    # Run the full Ruby test suite in the "go" tests. As more functionality is
+    # migrated into these tests and out of Ruby, the amount of work here will
+    # reduce
+    - bundle exec rspec --color --format d spec
 
 go:1.9:
   <<: *go_definition
@@ -61,6 +75,10 @@ go:1.10:
   <<: *go_definition
   image: golang:1.10
 
+go:1.11:
+  <<: *go_definition
+  image: golang:1.10
+
 codequality:
   image: docker:stable
   variables:
@@ -114,4 +132,4 @@ dependency_scanning:
         --volume /var/run/docker.sock:/var/run/docker.sock
         "registry.gitlab.com/gitlab-org/security-products/dependency-scanning:$SP_VERSION" /code
   artifacts:
-    paths: [gl-dependency-scanning-report.json]
\ No newline at end of file
+    paths: [gl-dependency-scanning-report.json]
diff --git a/bin/gitlab-shell b/bin/gitlab-shell-ruby
index 93a79f6..93a79f6 100755
--- a/bin/gitlab-shell
+++ b/bin/gitlab-shell-ruby
diff --git a/config.yml.example b/config.yml.example
index 23743fd..3d3e580 100644
--- a/config.yml.example
+++ b/config.yml.example
@@ -49,3 +49,10 @@ log_level: INFO
 # Set to true to see real usernames in the logs instead of key ids, which is easier to follow, but
 # incurs an extra API call on every gitlab-shell command.
 audit_usernames: false
+
+# Migration to Go: anything listed here has two implementations. Use these flags
+# to try the new implementations out, or to revert to the old behaviour if there
+# problems arise.
+migration:
+  enabled: false
+  features: []
diff --git a/go/cmd/gitlab-shell/main.go b/go/cmd/gitlab-shell/main.go
new file mode 100644
index 0000000..ae54151
--- /dev/null
+++ b/go/cmd/gitlab-shell/main.go
@@ -0,0 +1,56 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+)
+
+var (
+	binDir  string
+	rootDir string
+)
+
+func init() {
+	binDir = filepath.Dir(os.Args[0])
+	rootDir = filepath.Dir(binDir)
+}
+
+func migrate(*config.Config) (int, bool) {
+	// TODO: Dispatch appropriate requests to Go handlers and return
+	// <exitstatus, true> depending on how they fare
+	return 0, false
+}
+
+// rubyExec will never return. It either replaces the current process with a
+// Ruby interpreter, or outputs an error and kills the process.
+func execRuby() {
+	rubyCmd := filepath.Join(binDir, "gitlab-shell-ruby")
+
+	execErr := syscall.Exec(rubyCmd, os.Args, os.Environ())
+	if execErr != nil {
+		fmt.Fprintf(os.Stderr, "Failed to exec(%q): %v\n", rubyCmd, execErr)
+		os.Exit(1)
+	}
+}
+
+func main() {
+	// Fall back to Ruby in case of problems reading the config, but issue a
+	// warning as this isn't something we can sustain indefinitely
+	config, err := config.NewFromDir(rootDir)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "Failed to read config, falling back to gitlab-shell-ruby")
+		execRuby()
+	}
+
+	// Try to handle the command with the Go implementation
+	if exitCode, done := migrate(config); done {
+		os.Exit(exitCode)
+	}
+
+	// Since a migration has not handled the command, fall back to Ruby to do so
+	execRuby()
+}
diff --git a/go/internal/config/config.go b/go/internal/config/config.go
index 7d521f5..435cb29 100644
--- a/go/internal/config/config.go
+++ b/go/internal/config/config.go
@@ -13,31 +13,44 @@ const (
 	logFile    = "gitlab-shell.log"
 )
 
+type MigrationConfig struct {
+	Enabled  bool     `yaml:"enabled"`
+	Features []string `yaml:"features"`
+}
+
 type Config struct {
 	RootDir   string
-	LogFile   string `yaml:"log_file"`
-	LogFormat string `yaml:"log_format"`
+	LogFile   string          `yaml:"log_file"`
+	LogFormat string          `yaml:"log_format"`
+	Migration MigrationConfig `yaml:"migration"`
 }
 
 func New() (*Config, error) {
-	cfg := Config{}
-
 	dir, err := os.Getwd()
 	if err != nil {
 		return nil, err
 	}
-	cfg.RootDir = dir
 
-	configBytes, err := ioutil.ReadFile(path.Join(cfg.RootDir, configFile))
+	return NewFromDir(dir)
+}
+
+func NewFromDir(dir string) (*Config, error) {
+	return newFromFile(path.Join(dir, configFile))
+}
+
+func newFromFile(filename string) (*Config, error) {
+	cfg := &Config{RootDir: path.Dir(filename)}
+
+	configBytes, err := ioutil.ReadFile(filename)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := parseConfig(configBytes, &cfg); err != nil {
+	if err := parseConfig(configBytes, cfg); err != nil {
 		return nil, err
 	}
 
-	return &cfg, nil
+	return cfg, nil
 }
 
 // parseConfig expects YAML data in configBytes and a Config instance with RootDir set.
diff --git a/go/internal/config/config_test.go b/go/internal/config/config_test.go
index 0a5c842..87a582f 100644
--- a/go/internal/config/config_test.go
+++ b/go/internal/config/config_test.go
@@ -2,20 +2,28 @@ package config
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 )
 
-func TestConfigLogFile(t *testing.T) {
+func TestParseConfig(t *testing.T) {
 	testRoot := "/foo/bar"
 	testCases := []struct {
-		yaml   string
-		path   string
-		format string
+		yaml      string
+		path      string
+		format    string
+		migration MigrationConfig
 	}{
 		{path: "/foo/bar/gitlab-shell.log", format: "text"},
 		{yaml: "log_file: my-log.log", path: "/foo/bar/my-log.log", format: "text"},
 		{yaml: "log_file: /qux/my-log.log", path: "/qux/my-log.log", format: "text"},
 		{yaml: "log_format: json", path: "/foo/bar/gitlab-shell.log", format: "json"},
+		{
+			yaml:      "migration:\n  enabled: true\n  features:\n    - foo\n    - bar",
+			path:      "/foo/bar/gitlab-shell.log",
+			format:    "text",
+			migration: MigrationConfig{Enabled: true, Features: []string{"foo", "bar"}},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -25,6 +33,14 @@ func TestConfigLogFile(t *testing.T) {
 				t.Fatal(err)
 			}
 
+			if cfg.Migration.Enabled != tc.migration.Enabled {
+				t.Fatalf("migration.enabled: expected %v, got %v", tc.migration.Enabled, cfg.Migration.Enabled)
+			}
+
+			if strings.Join(cfg.Migration.Features, ":") != strings.Join(tc.migration.Features, ":") {
+				t.Fatalf("migration.features: expected %#v, got %#v", tc.migration.Features, cfg.Migration.Features)
+			}
+
 			if cfg.LogFile != tc.path {
 				t.Fatalf("expected %q, got %q", tc.path, cfg.LogFile)
 			}