diff options
author | Stan Hu <stanhu@gmail.com> | 2020-09-19 03:34:49 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2020-09-19 14:00:45 -0700 |
commit | 0590d9198f653ff2170e0f26790056bef4f056fe (patch) | |
tree | dc0d68866ea16ba4f74d441c3aa2048b12fb9e95 /internal/command/shared | |
parent | f100e7e83943b3bb5db232f5bf79a616fdba88f1 (diff) | |
download | gitlab-shell-sh-extract-context-from-env.tar.gz |
Make it possible to propagate correlation ID across processessh-extract-context-from-env
Previously, gitlab-shell did not pass a context through the application.
Correlation IDs were generated down the call stack, since we don't pass
the context around from the start execution.
This has several potential downsides:
1. It's easier for programming mistakes to be made in future which 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/shared')
4 files changed, 16 insertions, 12 deletions
diff --git a/internal/command/shared/accessverifier/accessverifier.go b/internal/command/shared/accessverifier/accessverifier.go index 5d2d709..9fcdde4 100644 --- a/internal/command/shared/accessverifier/accessverifier.go +++ b/internal/command/shared/accessverifier/accessverifier.go @@ -1,6 +1,7 @@ package accessverifier import ( + "context" "errors" "gitlab.com/gitlab-org/gitlab-shell/internal/command/commandargs" @@ -18,13 +19,13 @@ type Command struct { ReadWriter *readwriter.ReadWriter } -func (c *Command) Verify(action commandargs.CommandType, repo string) (*Response, error) { +func (c *Command) Verify(ctx context.Context, action commandargs.CommandType, repo string) (*Response, error) { client, err := accessverifier.NewClient(c.Config) if err != nil { return nil, err } - response, err := client.Verify(c.Args, action, repo) + response, err := client.Verify(ctx, c.Args, action, repo) if err != nil { return nil, err } diff --git a/internal/command/shared/accessverifier/accessverifier_test.go b/internal/command/shared/accessverifier/accessverifier_test.go index 998e622..8ad87b8 100644 --- a/internal/command/shared/accessverifier/accessverifier_test.go +++ b/internal/command/shared/accessverifier/accessverifier_test.go @@ -2,6 +2,7 @@ package accessverifier import ( "bytes" + "context" "encoding/json" "io/ioutil" "net/http" @@ -65,7 +66,7 @@ func TestMissingUser(t *testing.T) { defer cleanup() cmd.Args = &commandargs.Shell{GitlabKeyId: "2"} - _, err := cmd.Verify(action, repo) + _, err := cmd.Verify(context.Background(), action, repo) require.Equal(t, "missing user", err.Error()) } @@ -75,7 +76,7 @@ func TestConsoleMessages(t *testing.T) { defer cleanup() cmd.Args = &commandargs.Shell{GitlabKeyId: "1"} - cmd.Verify(action, repo) + cmd.Verify(context.Background(), action, repo) require.Equal(t, "remote: \nremote: console\nremote: message\nremote: \n", errBuf.String()) require.Empty(t, outBuf.String()) diff --git a/internal/command/shared/customaction/customaction.go b/internal/command/shared/customaction/customaction.go index 2ba1091..0675d36 100644 --- a/internal/command/shared/customaction/customaction.go +++ b/internal/command/shared/customaction/customaction.go @@ -2,6 +2,7 @@ package customaction import ( "bytes" + "context" "errors" "gitlab.com/gitlab-org/gitlab-shell/client" @@ -34,7 +35,7 @@ type Command struct { EOFSent bool } -func (c *Command) Execute(response *accessverifier.Response) error { +func (c *Command) Execute(ctx context.Context, response *accessverifier.Response) error { data := response.Payload.Data apiEndpoints := data.ApiEndpoints @@ -42,10 +43,10 @@ func (c *Command) Execute(response *accessverifier.Response) error { return errors.New("Custom action error: Empty API endpoints") } - return c.processApiEndpoints(response) + return c.processApiEndpoints(ctx, response) } -func (c *Command) processApiEndpoints(response *accessverifier.Response) error { +func (c *Command) processApiEndpoints(ctx context.Context, response *accessverifier.Response) error { client, err := gitlabnet.GetClient(c.Config) if err != nil { @@ -64,7 +65,7 @@ func (c *Command) processApiEndpoints(response *accessverifier.Response) error { log.WithFields(fields).Info("Performing custom action") - response, err := c.performRequest(client, endpoint, request) + response, err := c.performRequest(ctx, client, endpoint, request) if err != nil { return err } @@ -95,8 +96,8 @@ func (c *Command) processApiEndpoints(response *accessverifier.Response) error { return nil } -func (c *Command) performRequest(client *client.GitlabNetClient, endpoint string, request *Request) (*Response, error) { - response, err := client.DoRequest(http.MethodPost, endpoint, request) +func (c *Command) performRequest(ctx context.Context, client *client.GitlabNetClient, endpoint string, request *Request) (*Response, error) { + response, err := client.DoRequest(ctx, http.MethodPost, endpoint, request) if err != nil { return nil, err } diff --git a/internal/command/shared/customaction/customaction_test.go b/internal/command/shared/customaction/customaction_test.go index 46c5f32..119da5b 100644 --- a/internal/command/shared/customaction/customaction_test.go +++ b/internal/command/shared/customaction/customaction_test.go @@ -2,6 +2,7 @@ package customaction import ( "bytes" + "context" "encoding/json" "io/ioutil" "net/http" @@ -78,7 +79,7 @@ func TestExecuteEOFSent(t *testing.T) { EOFSent: true, } - require.NoError(t, cmd.Execute(response)) + require.NoError(t, cmd.Execute(context.Background(), response)) // expect printing of info message, "custom" string from the first request // and "output" string from the second request @@ -148,7 +149,7 @@ func TestExecuteNoEOFSent(t *testing.T) { EOFSent: false, } - require.NoError(t, cmd.Execute(response)) + require.NoError(t, cmd.Execute(context.Background(), response)) // expect printing of info message, "custom" string from the first request // and "output" string from the second request |