What
---
Make the retryableHTTP client introduced in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/703 the
default HTTP client.

Why
---
In
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979#note_1254964426
we've seen a 99% error reduction on `git` commands from `gitlab-shell`
when the retryableHTTP client is used.

This has been running in production for over 2 weeks in `us-east1-b` and
5 days fleet-wide so we should be confident that this client works as
expected.

Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979


Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>

Currently, the default values are used for retryable http.
That's why a test waits 1 second minimun to retry a request.
Client test takes 25 seconds to execute as a result.
When we stub the value to 1 millisecond instead, we get 0.5s of
execution

What
---
- Update the `client.HttpClient` fields to have `http.Client` and
  `retryablehttp.Client`, one of them will be `nil` depending on the
  feature flag toggle.
- Create new method `newRetryableRequest` which will create a
  `retryablehttp.Request` and use that if the
  `FF_GITLAB_SHELL_RETRYABLE_HTTP` feature flag is turned on.
- Add checks for `FF_GITLAB_SHELL_RETRYABLE_HTTP` everywhere we use the
  http client to use the `retryablehttp.Client` or the default
  `http.Client`
- New job `tests-integration-retryableHttp` to run the integraiton tests
  with the new retryablehttp client. We didn't update go tests because
  some assertions are different and will break table driven tests.

Why
---
As discussed in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/703#note_1229645097
we want to put the client behind a feature flag, not just the retry
logic. This does bring extra risk for accessing a `nil` field but there
should be checks everytime we access `RetryableHTTP` and `HTTPClient`.

Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979


Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>

What
---
Change the default `HTTP.Client` to
`github.com/hashicorp/go-retryablehttp.Client` to get automatic retries
and exponential backoff.

We retry the request 2 times resulting in 3 attempts of sending the
request, the min retry wait is 1 second, and the maximum is 15
seconds.

Hide the retry logic behind a temporary feature flag
`FF_GITLAB_SHELL_RETRYABLE_HTTP` to easily roll this out in GitLab.com.
When we verify that this works as expected we will remove
`FF_GITLAB_SHELL_RETRYABLE_HTTP` and have the retry logic as the default
logic.

Why
---
In https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979 users
end up seeing the following errors when trying to `git-clone(1)` a
repository locally on in CI.

```shell
remote: ===============================
remote:
remote: ERROR: Internal API unreachable
remote:
remote: ================================
```

When we look at the application logs we see the following error:

```json
{ "err": "http://gitlab-webservice-git.gitlab.svc:8181/api/v4/internal/allowed":
dial tcp 10.69.184.120:8181: connect: connection refused", "msg":
"Internal API unreachable"}
```

In
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979#note_1222670120
we've correlated these `connection refused` errors with infrastructure
events that remove the git pods that are hosting
`gitlab-webservice-git` service. We could try to make the underlying
infrastructure more reactive to these changes as suggested in
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979#note_1225164944
but we can still end up serving bad requests.

Implementing retry logic for 5xx or other errors would allow users to
still be able to `git-clone(1)` reposirories, although it being slower.
This is espically important during CI runs so users don't have to retry
jobs themselves.

Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979
Closes: https://gitlab.com/gitlab-org/gitlab-shell/-/issues/604


Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>

Now the requests are verified via JWT

With this change we don't rely on the secret to either contain
a newline or not contain it.

While gitlab-shell currently has a major version of v14, the module path
it exposes is not using that major version like it is required by the Go
standard. This makes it impossible for dependents to import gitlab-shell
as a dependency without using a commit as version.

Fix this by changing the module path of gitlab-shell to instead be
`gitlab.com/gitlab-org/gitlab-shell/v14` and adjust all imports
accordingly.

Changelog: fixed

It is passed as a Gitlab-Shell-Api-Request header and uses
the same shared secret in order to encrypt the token

Contributes to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/541

Changelog: removed

Logrus buffers its output internally, which makes these tests fail
intermittently. They're also not a good example to follow generally.

We now have acceptance tests that exercise this functionality so I'm
pretty relaxed about losing the expectations. However, we can test
them by inspecting the server-received metadata too, so there's no loss
of coverage here.

The move from logrus to labkit for logging also makes these tests hard
to justify keeping.

In this case we don't need to propagate cleanup
function. It simplifies the code.

This will be useful to measure bandwidth sent in response to an API
request, particularly with measuring the /api/v4/internal/lfs endpoint.

Testify features sub packages `assert` and `require`. The difference is
subtle, and lost on novice Golang developers that don't read the docs.
To create a more consistent code base `assert` will no longer be used.

This change was generated by a running a sed command on all `_test.go`
files, followed by `goimports -w`.

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

From
https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/4498#note_397401883,
if you specify a relative path such as:

```
external_url 'http://gitlab.example.com/gitlab'
```

gitlab-shell doesn't have a way to pass the `/gitlab` to the host. For example, let's say we have:

```
gitlab_url: "http+unix://%2Fvar%2Fopt%2Fgitlab%2Fgitlab-workhorse%2Fsocket"
```

If we have `/gitlab` as the relative path, how do we specify what is the
UNIX socket path and what is the relative path? If we specify:

```
gitlab_url: "http+unix:///var/opt/gitlab/gitlab-workhorse.socket/gitlab
```

This is ambiguous. Is the socket in
`/var/opt/gitlab/gitlab-workhorse.socket/gitlab` or in
`/var/opt/gitlab/gitlab-workhorse.socket`?

To fix this, this merge request adds an optional
`gitlab_relative_url_root` config parameter:

```
gitlab_url: "http+unix://%2Fvar%2Fopt%2Fgitlab%2Fgitlab-workhorse%2Fsocket"
gitlab_relative_url_root: /gitlab
```

This is only used with UNIX domain sockets to disambiguate the socket
and base URL path. If `gitlab_url` uses `http://` or `https://`, then
`gitlab_relative_url_root` is ignored.

Relates to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/476

Stan Hu authored 4 years ago and Igor Drozdov committed 4 years ago

This will make it easier to tie an SSH access request to Rails API and
Gitaly requests.

logrus fires a Goroutine to write logs, so the tests could fail if they
checked the event queue before the logrus have fired.  Since there isn't
an easy way to flush all outstanding hooks, we just retry every 100 ms
for up to a second for log to arrive in the queue.

Closes https://gitlab.com/gitlab-org/gitlab-shell/-/issues/450

Calling logrus hook.LastEntry() can lead to race conditions.  Use
AllEntries instead:
https://github.com/sirupsen/logrus/blob/60c74ad9be0d874af0ab0daef6ab07c5c5911f0d/hooks/test/test.go#L77

Closes https://gitlab.com/gitlab-org/gitlab-shell/-/issues/450

This would make it easier to filter the logs by status code.

This restores the previous behavior of logging the success and failures
of internal HTTP requests.

Part of https://gitlab.com/gitlab-org/gitlab/issues/207916