summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Makefile3
-rw-r--r--cmd/check/main.go2
-rw-r--r--cmd/gitlab-shell-authorized-keys-check/main.go2
-rw-r--r--cmd/gitlab-shell-authorized-principals-check/main.go2
-rw-r--r--cmd/gitlab-shell/main.go2
-rw-r--r--cmd/gitlab-sshd/main.go6
-rw-r--r--go.mod2
-rw-r--r--internal/command/command.go41
-rw-r--r--internal/command/command_test.go12
-rw-r--r--internal/command/receivepack/gitalycall_test.go1
-rw-r--r--internal/command/uploadarchive/gitalycall_test.go1
-rw-r--r--internal/command/uploadpack/gitalycall_test.go1
-rw-r--r--internal/config/config.go11
-rw-r--r--internal/handler/exec.go89
-rw-r--r--internal/handler/exec_test.go26
-rw-r--r--internal/sshd/sshd.go10
16 files changed, 118 insertions, 93 deletions
diff --git a/Makefile b/Makefile
index 4bd94e6..9e4abba 100644
--- a/Makefile
+++ b/Makefile
@@ -3,7 +3,8 @@
GO_SOURCES := $(shell find . -name '*.go')
VERSION_STRING := $(shell git describe --match v* 2>/dev/null || awk '$0="v"$0' VERSION 2>/dev/null || echo unknown)
BUILD_TIME := $(shell date -u +%Y%m%d.%H%M%S)
-GOBUILD_FLAGS := -ldflags "-X main.Version=$(VERSION_STRING) -X main.BuildTime=$(BUILD_TIME)"
+BUILD_TAGS := tracer_static tracer_static_jaeger continuous_profiler_stackdriver
+GOBUILD_FLAGS := -ldflags "-X main.Version=$(VERSION_STRING) -X main.BuildTime=$(BUILD_TIME)" -tags "$(BUILD_TAGS)"
validate: verify test
diff --git a/cmd/check/main.go b/cmd/check/main.go
index fbb520b..5a00781 100644
--- a/cmd/check/main.go
+++ b/cmd/check/main.go
@@ -39,7 +39,7 @@ func main() {
os.Exit(1)
}
- ctx, finished := command.ContextWithCorrelationID()
+ ctx, finished := command.Setup(executable.Name, config)
defer finished()
if err = cmd.Execute(ctx); err != nil {
diff --git a/cmd/gitlab-shell-authorized-keys-check/main.go b/cmd/gitlab-shell-authorized-keys-check/main.go
index 8746353..0fc7ecc 100644
--- a/cmd/gitlab-shell-authorized-keys-check/main.go
+++ b/cmd/gitlab-shell-authorized-keys-check/main.go
@@ -42,7 +42,7 @@ func main() {
os.Exit(1)
}
- ctx, finished := command.ContextWithCorrelationID()
+ ctx, finished := command.Setup(executable.Name, config)
defer finished()
if err = cmd.Execute(ctx); err != nil {
diff --git a/cmd/gitlab-shell-authorized-principals-check/main.go b/cmd/gitlab-shell-authorized-principals-check/main.go
index f14fbde..bdff1d3 100644
--- a/cmd/gitlab-shell-authorized-principals-check/main.go
+++ b/cmd/gitlab-shell-authorized-principals-check/main.go
@@ -42,7 +42,7 @@ func main() {
os.Exit(1)
}
- ctx, finished := command.ContextWithCorrelationID()
+ ctx, finished := command.Setup(executable.Name, config)
defer finished()
if err = cmd.Execute(ctx); err != nil {
diff --git a/cmd/gitlab-shell/main.go b/cmd/gitlab-shell/main.go
index 8286e66..16705eb 100644
--- a/cmd/gitlab-shell/main.go
+++ b/cmd/gitlab-shell/main.go
@@ -57,7 +57,7 @@ func main() {
os.Exit(1)
}
- ctx, finished := command.ContextWithCorrelationID()
+ ctx, finished := command.Setup(executable.Name, config)
defer finished()
if err = cmd.Execute(ctx); err != nil {
diff --git a/cmd/gitlab-sshd/main.go b/cmd/gitlab-sshd/main.go
index f777b9f..866bc8d 100644
--- a/cmd/gitlab-sshd/main.go
+++ b/cmd/gitlab-sshd/main.go
@@ -6,6 +6,7 @@ import (
log "github.com/sirupsen/logrus"
+ "gitlab.com/gitlab-org/gitlab-shell/internal/command"
"gitlab.com/gitlab-org/gitlab-shell/internal/config"
"gitlab.com/gitlab-org/gitlab-shell/internal/logger"
"gitlab.com/gitlab-org/gitlab-shell/internal/sshd"
@@ -59,6 +60,9 @@ func main() {
logger.ConfigureStandalone(cfg)
+ ctx, finished := command.Setup("gitlab-sshd", cfg)
+ defer finished()
+
// Startup monitoring endpoint.
if cfg.Server.WebListen != "" {
go func() {
@@ -71,7 +75,7 @@ func main() {
}()
}
- if err := sshd.Run(cfg); err != nil {
+ if err := sshd.Run(ctx, cfg); err != nil {
log.Fatalf("Failed to start GitLab built-in sshd: %v", err)
}
}
diff --git a/go.mod b/go.mod
index cd942c2..100c117 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,8 @@ module gitlab.com/gitlab-org/gitlab-shell
go 1.13
require (
+ github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
+ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/mattn/go-shellwords v1.0.11
github.com/mikesmitty/edkey v0.0.0-20170222072505-3356ea4e686a
github.com/otiai10/copy v1.4.2
diff --git a/internal/command/command.go b/internal/command/command.go
index c2709a1..6696f0f 100644
--- a/internal/command/command.go
+++ b/internal/command/command.go
@@ -21,7 +21,6 @@ import (
"gitlab.com/gitlab-org/gitlab-shell/internal/executable"
"gitlab.com/gitlab-org/gitlab-shell/internal/sshenv"
"gitlab.com/gitlab-org/labkit/correlation"
- "gitlab.com/gitlab-org/labkit/log"
"gitlab.com/gitlab-org/labkit/tracing"
)
@@ -42,24 +41,40 @@ func New(e *executable.Executable, arguments []string, env sshenv.Env, config *c
return nil, disallowedcommand.Error
}
-// ContextWithCorrelationID() will always return a background Context
-// with a correlation ID. It will first attempt to extract the ID from
-// an environment variable. If is not available, a random one will be
-// generated.
-func ContextWithCorrelationID() (context.Context, func()) {
+// Setup() initializes tracing from the configuration file and generates a
+// background context from which all other contexts in the process should derive
+// from, as it has a service name and initial correlation ID set.
+func Setup(serviceName string, config *config.Config) (context.Context, func()) {
+ closer := tracing.Initialize(
+ tracing.WithServiceName(serviceName),
+
+ // For GitLab-Shell, we explicitly initialize tracing from a config file
+ // instead of the default environment variable (using GITLAB_TRACING)
+ // This decision was made owing to the difficulty in passing environment
+ // variables into GitLab-Shell processes.
+ //
+ // Processes are spawned as children of the SSH daemon, which tightly
+ // controls environment variables; doing this means we don't have to
+ // enable PermitUserEnvironment
+ //
+ // gitlab-sshd could use the standard GITLAB_TRACING envvar, but that
+ // would lead to inconsistencies between the two forms of operation
+ tracing.WithConnectionString(config.GitlabTracing),
+ )
+
ctx, finished := tracing.ExtractFromEnv(context.Background())
+ ctx = correlation.ContextWithClientName(ctx, serviceName)
correlationID := correlation.ExtractFromContext(ctx)
if correlationID == "" {
- correlationID, err := correlation.RandomID()
- if err != nil {
- log.WithError(err).Warn("unable to generate correlation ID")
- } else {
- ctx = correlation.ContextWithCorrelation(ctx, correlationID)
- }
+ correlationID := correlation.SafeRandomID()
+ ctx = correlation.ContextWithCorrelation(ctx, correlationID)
}
- return ctx, finished
+ return ctx, func() {
+ finished()
+ closer.Close()
+ }
}
func buildCommand(e *executable.Executable, args commandargs.CommandArgs, config *config.Config, readWriter *readwriter.ReadWriter) Command {
diff --git a/internal/command/command_test.go b/internal/command/command_test.go
index a70ea84..3617d39 100644
--- a/internal/command/command_test.go
+++ b/internal/command/command_test.go
@@ -168,7 +168,7 @@ func TestFailingNew(t *testing.T) {
}
}
-func TestContextWithCorrelationID(t *testing.T) {
+func TestSetup(t *testing.T) {
testCases := []struct {
name string
additionalEnv map[string]string
@@ -190,16 +190,20 @@ func TestContextWithCorrelationID(t *testing.T) {
resetEnvironment := addAdditionalEnv(tc.additionalEnv)
defer resetEnvironment()
- ctx, finished := ContextWithCorrelationID()
+ ctx, finished := Setup("foo", &config.Config{})
+ defer finished()
+
require.NotNil(t, ctx, "ctx is nil")
require.NotNil(t, finished, "finished is nil")
+
correlationID := correlation.ExtractFromContext(ctx)
require.NotEmpty(t, correlationID)
-
if tc.expectedCorrelationID != "" {
require.Equal(t, tc.expectedCorrelationID, correlationID)
}
- defer finished()
+
+ clientName := correlation.ExtractClientNameFromContext(ctx)
+ require.Equal(t, "foo", clientName)
})
}
}
diff --git a/internal/command/receivepack/gitalycall_test.go b/internal/command/receivepack/gitalycall_test.go
index 70f2b2e..12c603d 100644
--- a/internal/command/receivepack/gitalycall_test.go
+++ b/internal/command/receivepack/gitalycall_test.go
@@ -62,6 +62,7 @@ func TestReceivePack(t *testing.T) {
hook := testhelper.SetupLogger()
ctx := correlation.ContextWithCorrelation(context.Background(), "a-correlation-id")
+ ctx = correlation.ContextWithClientName(ctx, "gitlab-shell-tests")
err := cmd.Execute(ctx)
require.NoError(t, err)
diff --git a/internal/command/uploadarchive/gitalycall_test.go b/internal/command/uploadarchive/gitalycall_test.go
index b89b749..3ec1449 100644
--- a/internal/command/uploadarchive/gitalycall_test.go
+++ b/internal/command/uploadarchive/gitalycall_test.go
@@ -37,6 +37,7 @@ func TestUploadPack(t *testing.T) {
hook := testhelper.SetupLogger()
ctx := correlation.ContextWithCorrelation(context.Background(), "a-correlation-id")
+ ctx = correlation.ContextWithClientName(ctx, "gitlab-shell-tests")
err := cmd.Execute(ctx)
require.NoError(t, err)
diff --git a/internal/command/uploadpack/gitalycall_test.go b/internal/command/uploadpack/gitalycall_test.go
index bedc6e6..6b4c6ba 100644
--- a/internal/command/uploadpack/gitalycall_test.go
+++ b/internal/command/uploadpack/gitalycall_test.go
@@ -37,6 +37,7 @@ func TestUploadPack(t *testing.T) {
hook := testhelper.SetupLogger()
ctx := correlation.ContextWithCorrelation(context.Background(), "a-correlation-id")
+ ctx = correlation.ContextWithClientName(ctx, "gitlab-shell-tests")
err := cmd.Execute(ctx)
require.NoError(t, err)
diff --git a/internal/config/config.go b/internal/config/config.go
index e3dd7c2..c112038 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -9,8 +9,10 @@ import (
"path/filepath"
"sync"
- "gitlab.com/gitlab-org/gitlab-shell/client"
+ "gitlab.com/gitlab-org/labkit/tracing"
yaml "gopkg.in/yaml.v2"
+
+ "gitlab.com/gitlab-org/gitlab-shell/client"
)
const (
@@ -83,7 +85,7 @@ func (c *Config) ApplyGlobalState() {
func (c *Config) HttpClient() *client.HttpClient {
c.httpClientOnce.Do(func() {
- c.httpClient = client.NewHTTPClient(
+ client := client.NewHTTPClient(
c.GitlabUrl,
c.GitlabRelativeURLRoot,
c.HttpSettings.CaFile,
@@ -91,6 +93,11 @@ func (c *Config) HttpClient() *client.HttpClient {
c.HttpSettings.SelfSignedCert,
c.HttpSettings.ReadTimeoutSeconds,
)
+
+ tr := client.Transport
+ client.Transport = tracing.NewRoundTripper(tr)
+
+ c.httpClient = client
})
return c.httpClient
diff --git a/internal/handler/exec.go b/internal/handler/exec.go
index 723d655..1a7716e 100644
--- a/internal/handler/exec.go
+++ b/internal/handler/exec.go
@@ -6,20 +6,21 @@ import (
"strconv"
"strings"
+ grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
+ grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
log "github.com/sirupsen/logrus"
+ "google.golang.org/grpc"
+ "google.golang.org/grpc/metadata"
gitalyauth "gitlab.com/gitlab-org/gitaly/auth"
"gitlab.com/gitlab-org/gitaly/client"
pb "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitlab-shell/internal/config"
- "gitlab.com/gitlab-org/gitlab-shell/internal/executable"
"gitlab.com/gitlab-org/gitlab-shell/internal/gitlabnet/accessverifier"
"gitlab.com/gitlab-org/gitlab-shell/internal/sshenv"
"gitlab.com/gitlab-org/labkit/correlation"
grpccorrelation "gitlab.com/gitlab-org/labkit/correlation/grpc"
- "gitlab.com/gitlab-org/labkit/tracing"
- "google.golang.org/grpc"
- "google.golang.org/grpc/metadata"
+ grpctracing "gitlab.com/gitlab-org/labkit/tracing/grpc"
)
// GitalyHandlerFunc implementations are responsible for making
@@ -27,12 +28,6 @@ import (
// and returning an error from the Gitaly call.
type GitalyHandlerFunc func(ctx context.Context, client *grpc.ClientConn) (int32, error)
-type GitalyConn struct {
- ctx context.Context
- conn *grpc.ClientConn
- close func()
-}
-
type GitalyCommand struct {
Config *config.Config
ServiceName string
@@ -45,14 +40,14 @@ type GitalyCommand struct {
// through GitLab-Shell. It ensures that logging, tracing and other
// common concerns are configured before executing the `handler`.
func (gc *GitalyCommand) RunGitalyCommand(ctx context.Context, handler GitalyHandlerFunc) error {
- gitalyConn, err := getConn(ctx, gc)
+ conn, err := getConn(ctx, gc)
if err != nil {
return err
}
+ defer conn.Close()
- defer gitalyConn.close()
-
- _, err = handler(gitalyConn.ctx, gitalyConn.conn)
+ childCtx := withOutgoingMetadata(ctx, gc.Features)
+ _, err = handler(childCtx, conn)
return err
}
@@ -106,23 +101,43 @@ func withOutgoingMetadata(ctx context.Context, features map[string]string) conte
return metadata.NewOutgoingContext(ctx, md)
}
-func getConn(ctx context.Context, gc *GitalyCommand) (*GitalyConn, error) {
+func getConn(ctx context.Context, gc *GitalyCommand) (*grpc.ClientConn, error) {
if gc.Address == "" {
return nil, fmt.Errorf("no gitaly_address given")
}
+ serviceName := correlation.ExtractClientNameFromContext(ctx)
+ if serviceName == "" {
+ log.Warn("No gRPC service name specified, defaulting to gitlab-shell-unknown")
+
+ serviceName = "gitlab-shell-unknown"
+ }
+
+ serviceName = fmt.Sprintf("%s-%s", serviceName, gc.ServiceName)
+
connOpts := client.DefaultDialOpts
- connOpts = append(connOpts,
+ connOpts = append(
+ connOpts,
grpc.WithStreamInterceptor(
- grpccorrelation.StreamClientCorrelationInterceptor(
- grpccorrelation.WithClientName(executable.GitlabShell),
+ grpc_middleware.ChainStreamClient(
+ grpctracing.StreamClientTracingInterceptor(),
+ grpc_prometheus.StreamClientInterceptor,
+ grpccorrelation.StreamClientCorrelationInterceptor(
+ grpccorrelation.WithClientName(serviceName),
+ ),
),
),
+
grpc.WithUnaryInterceptor(
- grpccorrelation.UnaryClientCorrelationInterceptor(
- grpccorrelation.WithClientName(executable.GitlabShell),
+ grpc_middleware.ChainUnaryClient(
+ grpctracing.UnaryClientTracingInterceptor(),
+ grpc_prometheus.UnaryClientInterceptor,
+ grpccorrelation.UnaryClientCorrelationInterceptor(
+ grpccorrelation.WithClientName(serviceName),
+ ),
),
- ))
+ ),
+ )
if gc.Token != "" {
connOpts = append(connOpts,
@@ -130,35 +145,5 @@ func getConn(ctx context.Context, gc *GitalyCommand) (*GitalyConn, error) {
)
}
- // Configure distributed tracing
- serviceName := fmt.Sprintf("gitlab-shell-%v", gc.ServiceName)
- closer := tracing.Initialize(
- tracing.WithServiceName(serviceName),
-
- // For GitLab-Shell, we explicitly initialize tracing from a config file
- // instead of the default environment variable (using GITLAB_TRACING)
- // This decision was made owing to the difficulty in passing environment
- // variables into GitLab-Shell processes.
- //
- // Processes are spawned as children of the SSH daemon, which tightly
- // controls environment variables; doing this means we don't have to
- // enable PermitUserEnvironment
- tracing.WithConnectionString(gc.Config.GitlabTracing),
- )
-
- childCtx, finished := tracing.ExtractFromEnv(ctx)
- childCtx = withOutgoingMetadata(childCtx, gc.Features)
-
- conn, err := client.DialContext(childCtx, gc.Address, connOpts)
- if err != nil {
- return nil, err
- }
-
- finish := func() {
- finished()
- closer.Close()
- conn.Close()
- }
-
- return &GitalyConn{ctx: childCtx, conn: conn, close: finish}, nil
+ return client.DialContext(ctx, gc.Address, connOpts)
}
diff --git a/internal/handler/exec_test.go b/internal/handler/exec_test.go
index 6f84709..9b8fee8 100644
--- a/internal/handler/exec_test.go
+++ b/internal/handler/exec_test.go
@@ -45,7 +45,7 @@ func TestMissingGitalyAddress(t *testing.T) {
require.EqualError(t, err, "no gitaly_address given")
}
-func TestGetConnMetadata(t *testing.T) {
+func TestRunGitalyCommandMetadata(t *testing.T) {
tests := []struct {
name string
gc *GitalyCommand
@@ -70,19 +70,23 @@ func TestGetConnMetadata(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- conn, err := getConn(context.Background(), tt.gc)
- require.NoError(t, err)
+ cmd := tt.gc
- md, exists := metadata.FromOutgoingContext(conn.ctx)
- require.True(t, exists)
- require.Equal(t, len(tt.want), md.Len())
+ err := cmd.RunGitalyCommand(context.Background(), func(ctx context.Context, _ *grpc.ClientConn) (int32, error) {
+ md, exists := metadata.FromOutgoingContext(ctx)
+ require.True(t, exists)
+ require.Equal(t, len(tt.want), md.Len())
- for k, v := range tt.want {
- values := md.Get(k)
- require.Equal(t, 1, len(values))
- require.Equal(t, v, values[0])
- }
+ for k, v := range tt.want {
+ values := md.Get(k)
+ require.Equal(t, 1, len(values))
+ require.Equal(t, v, values[0])
+ }
+ return 0, nil
+ })
+
+ require.NoError(t, err)
})
}
}
diff --git a/internal/sshd/sshd.go b/internal/sshd/sshd.go
index 470c069..b04366e 100644
--- a/internal/sshd/sshd.go
+++ b/internal/sshd/sshd.go
@@ -20,7 +20,7 @@ import (
"gitlab.com/gitlab-org/labkit/correlation"
)
-func Run(cfg *config.Config) error {
+func Run(ctx context.Context, cfg *config.Config) error {
authorizedKeysClient, err := authorizedkeys.NewClient(cfg)
if err != nil {
return fmt.Errorf("failed to initialize GitLab client: %w", err)
@@ -47,7 +47,7 @@ func Run(cfg *config.Config) error {
if key.Type() == ssh.KeyAlgoDSA {
return nil, errors.New("DSA is prohibited")
}
- ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+ ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
defer cancel()
res, err := authorizedKeysClient.GetByKey(ctx, base64.RawStdEncoding.EncodeToString(key.Marshal()))
if err != nil {
@@ -89,11 +89,11 @@ func Run(cfg *config.Config) error {
continue
}
- go handleConn(cfg, sshCfg, nconn)
+ go handleConn(ctx, cfg, sshCfg, nconn)
}
}
-func handleConn(cfg *config.Config, sshCfg *ssh.ServerConfig, nconn net.Conn) {
+func handleConn(ctx context.Context, cfg *config.Config, sshCfg *ssh.ServerConfig, nconn net.Conn) {
remoteAddr := nconn.RemoteAddr().String()
defer nconn.Close()
@@ -105,7 +105,7 @@ func handleConn(cfg *config.Config, sshCfg *ssh.ServerConfig, nconn net.Conn) {
}
}()
- ctx, cancel := context.WithCancel(correlation.ContextWithCorrelation(context.Background(), correlation.SafeRandomID()))
+ ctx, cancel := context.WithCancel(correlation.ContextWithCorrelation(ctx, correlation.SafeRandomID()))
defer cancel()
sconn, chans, reqs, err := ssh.NewServerConn(nconn, sshCfg)
View Lorry Controller Status