summaryrefslogtreecommitdiff
path: root/internal/command/uploadpack
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2020-09-19 03:34:49 -0700
committerStan Hu <stanhu@gmail.com>2020-09-20 21:40:40 -0700
commita487572a904cc149840488eefdfe121173d8bcb5 (patch)
treed17cf7bff45492a587027851bb6e0bcb493cff58 /internal/command/uploadpack
parentf100e7e83943b3bb5db232f5bf79a616fdba88f1 (diff)
downloadgitlab-shell-a487572a904cc149840488eefdfe121173d8bcb5.tar.gz
Make it possible to propagate correlation ID across processes
Previously, gitlab-shell did not pass a context through the application. Correlation IDs were generated down the call stack instead of passed around from the start execution. This has several potential downsides: 1. It's easier for programming mistakes to be made in future that lead to multiple correlation IDs being generated for a single request. 2. Correlation IDs cannot be passed in from upstream requests 3. Other advantages of context passing, such as distributed tracing is not possible. This commit changes the behavior: 1. Extract the correlation ID from the environment at the start of the application. 2. If no correlation ID exists, generate a random one. 3. Pass the correlation ID to the GitLabNet API requests. This change also enables other clients of GitLabNet (e.g. Gitaly) to pass along the correlation ID in the internal API requests (https://gitlab.com/gitlab-org/gitaly/-/issues/2725). Fixes https://gitlab.com/gitlab-org/gitlab-shell/-/issues/474
Diffstat (limited to 'internal/command/uploadpack')
-rw-r--r--internal/command/uploadpack/gitalycall_test.go3
-rw-r--r--internal/command/uploadpack/uploadpack.go12
-rw-r--r--internal/command/uploadpack/uploadpack_test.go3
3 files changed, 11 insertions, 7 deletions
diff --git a/internal/command/uploadpack/gitalycall_test.go b/internal/command/uploadpack/gitalycall_test.go
index d6762a2..22189b8 100644
--- a/internal/command/uploadpack/gitalycall_test.go
+++ b/internal/command/uploadpack/gitalycall_test.go
@@ -2,6 +2,7 @@ package uploadpack
import (
"bytes"
+ "context"
"testing"
"github.com/stretchr/testify/assert"
@@ -37,7 +38,7 @@ func TestUploadPack(t *testing.T) {
hook := testhelper.SetupLogger()
- err := cmd.Execute()
+ err := cmd.Execute(context.Background())
require.NoError(t, err)
require.Equal(t, "UploadPack: "+repo, output.String())
diff --git a/internal/command/uploadpack/uploadpack.go b/internal/command/uploadpack/uploadpack.go
index 56814d7..fca3823 100644
--- a/internal/command/uploadpack/uploadpack.go
+++ b/internal/command/uploadpack/uploadpack.go
@@ -1,6 +1,8 @@
package uploadpack
import (
+ "context"
+
"gitlab.com/gitlab-org/gitlab-shell/internal/command/commandargs"
"gitlab.com/gitlab-org/gitlab-shell/internal/command/readwriter"
"gitlab.com/gitlab-org/gitlab-shell/internal/command/shared/accessverifier"
@@ -15,14 +17,14 @@ type Command struct {
ReadWriter *readwriter.ReadWriter
}
-func (c *Command) Execute() error {
+func (c *Command) Execute(ctx context.Context) error {
args := c.Args.SshArgs
if len(args) != 2 {
return disallowedcommand.Error
}
repo := args[1]
- response, err := c.verifyAccess(repo)
+ response, err := c.verifyAccess(ctx, repo)
if err != nil {
return err
}
@@ -33,14 +35,14 @@ func (c *Command) Execute() error {
ReadWriter: c.ReadWriter,
EOFSent: false,
}
- return customAction.Execute(response)
+ return customAction.Execute(ctx, response)
}
return c.performGitalyCall(response)
}
-func (c *Command) verifyAccess(repo string) (*accessverifier.Response, error) {
+func (c *Command) verifyAccess(ctx context.Context, repo string) (*accessverifier.Response, error) {
cmd := accessverifier.Command{c.Config, c.Args, c.ReadWriter}
- return cmd.Verify(c.Args.CommandType, repo)
+ return cmd.Verify(ctx, c.Args.CommandType, repo)
}
diff --git a/internal/command/uploadpack/uploadpack_test.go b/internal/command/uploadpack/uploadpack_test.go
index 7ea8e5d..20edb57 100644
--- a/internal/command/uploadpack/uploadpack_test.go
+++ b/internal/command/uploadpack/uploadpack_test.go
@@ -2,6 +2,7 @@ package uploadpack
import (
"bytes"
+ "context"
"testing"
"github.com/stretchr/testify/require"
@@ -26,6 +27,6 @@ func TestForbiddenAccess(t *testing.T) {
ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output},
}
- err := cmd.Execute()
+ err := cmd.Execute(context.Background())
require.Equal(t, "Disallowed by API call", err.Error())
}