summaryrefslogtreecommitdiff
path: root/client
Commit message (Collapse)AuthorAgeFilesLines
* Add DNS discovery support for Gitaly/PraefectQuang-Minh Nguyen2023-02-141-11/+32
| | | | | | | | | | All the implementations of DNS discovery were done in this epic: https://gitlab.com/groups/gitlab-org/-/epics/8971. Gitaly allows clients to configure DNS discovery via dial option. This MR adds the exposed dial options to client connection creation in Gitlab-shell. Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/4722 Changelog: added
* Define Do function for Gitlab net clientIgor Drozdov2023-02-073-37/+77
| | | | | | | | | | | | | | | | | In future, we'll need to perform http requests for Geo related code area. We cannot use retryable requests because: - It's not necessary for the to be retryable - In order to retry, the whole request body is stored in RAM, while we need to stream large blobs of data This commit: - Extracts logging into a separate round tripper in order to reuse it for other http requests by default - Defines Do function that accepts raw request as an argument
* feat: make retryable http default clientSteve Azzopardi2023-01-305-116/+30
| | | | | | | | | | | | | | | | | | | | | | 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>
* Stub retryable http values in testsIgor Drozdov2023-01-253-18/+38
| | | | | | | | 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
* feat: put retryablehttp.Client behind feature flagSteve Azzopardi2023-01-124-17/+75
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* feat: retry on errorSteve Azzopardi2023-01-125-14/+92
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* Remove secret from request headersIgor Drozdov2022-10-172-37/+0
| | | | Now the requests are verified via JWT
* Trim secret before signing JWT tokensIgor Drozdov2022-09-272-10/+23
| | | | | With this change we don't rely on the secret to either contain a newline or not contain it.
* Merge branch 'id-update-gitaly-to-v15' into 'main'Patrick Bajao2022-08-101-2/+2
|\ | | | | | | | | Update Gitaly to v15 See merge request gitlab-org/gitlab-shell!676
| * Update Gitaly to v15Igor Drozdov2022-08-051-2/+2
| | | | | | | | | | | | | | | | | | | | | | This commit also excludes gitlab-shell from dependencies: Gitaly specifies Gitlab Shell as a dependency as well in order to use gitlabnet client to perform API endpoints to Gitlab Rails. As a result, Gitlab Shell requires Gitaly -> Gitaly requires an older version of Gitlab Shell -> that version requires an older version of Gitlab Shell, etc. Let's use exclude to break the chain earlier
* | Fixed extra slashes in API request paths generated for geoCarlos Yu2022-08-052-1/+16
|/
* go: Bump major version to v14Patrick Steinhardt2022-07-054-6/+6
| | | | | | | | | | | | | 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
* Pass original IP from PROXY requests to internal API callsAlejandro Rodríguez2022-06-302-0/+30
|
* Exclude API errors from error rateIgor Drozdov2022-05-181-3/+11
| | | | | | When API isn't responsible or the resource is not accessible (returns 404 or 403), then we shouldn't consider it as an error on gitlab-sshd side
* Use require.WithinDuration to fix flacky testIgor Drozdov2022-05-091-2/+2
|
* Merge branch '484_remove_outdated_func' into 'main'Ash McKenzie2022-04-271-10/+0
|\ | | | | | | | | Remove deprecated function NewHTTPClient See merge request gitlab-org/gitlab-shell!603
| * Remove deprecated function NewHTTPClientVasilii Iakliushin2022-04-261-10/+0
| | | | | | | | | | | | Contributes to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/484 Changelog: removed
* | Add JWT token to GitLab Rails requestIgor Drozdov2022-04-262-9/+74
|/ | | | | It is passed as a Gitlab-Shell-Api-Request header and uses the same shared secret in order to encrypt the token
* Remove `self_signed_cert` optionVasilii Iakliushin2022-04-224-22/+16
| | | | | | Contributes to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/541 Changelog: removed
* Optionally use SSHUploadPackWithSidechannelJacob Vosmaer2022-01-251-1/+28
| | | | | | | If the GitLab API returns an allowed response with use_sidechannel set to true, gitlab-shell will establish a sidechannel connection and use SSHUploadPackWithSidechannel instead of SSHUploadPack. This is an efficiency improvement.
* Update gitaly/v14/client to 2e398afa0490ccdf5a82e1a7c7d824ae491eba16Jacob Vosmaer2022-01-251-1/+4
| | | | | | This updates the Gitaly client go.mod dependency to Gitaly commit 2e398afa0490ccdf5a82e1a7c7d824ae491eba16. This causes a grpc-go version bump, and hence a minor change in some of our test code.
* Deprecate self_signed_cert config settingid-deprecate-self-signed-certIgor Drozdov2022-01-121-1/+4
| | | | | | | | | The option isn't required to accept self-signed certs On the other hand, if the option set to true it makes machine-in-the-middle attack possible Let's clarify it in the code that the option is deprecated
* Merge branch 'fix/name-certificate' into 'main'Igor Drozdov2021-10-122-2/+0
|\ | | | | | | | | refactor: remove call to BuildNameToCertificate (deprecated) See merge request gitlab-org/gitlab-shell!515
| * refactor: remove call to BuildNameToCertificate (deprecated)feistel2021-09-082-2/+0
| |
* | Only validate SSL cert file exists if a value is suppliedStan Hu2021-09-232-12/+34
|/ | | | | | | | | This fixes a regression in https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/508. If an HTTPS internal API URL were used, gitlab-shell would not work at all. We now handle blank `caFile` properly. Relates to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/529
* refactor: move away from ioutil (deprecated)feistel2021-08-196-19/+17
|
* Merge branch 'verify/cafile' into 'main'Ash McKenzie2021-08-182-29/+47
|\ | | | | | | | | | | | | fix: validate client cert paths exist on disk before proceeding Closes #486 See merge request gitlab-org/gitlab-shell!508
| * test: move os.stat check before the hcc creationAsh McKenzie2021-08-171-7/+7
| |
| * fix: make sure ErrCafileNotFound is returned only when the file doesn't existfeistel2021-08-111-1/+4
| |
| * fix: validate client cert paths exist on disk before proceedingfeistel2021-08-112-29/+44
| |
* | refactor: update usage of NewHTTPClient to NewHTTPClientWithOptsfeistel2021-08-112-3/+6
|/
* Remove some unreliable testsNick Thomas2021-07-301-65/+1
| | | | | | | | | | | | | 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.
* Fix formatting via make fmtStan Hu2021-07-262-4/+4
|
* Switch to labkit/log for logging functionalityIgor Drozdov2021-07-222-13/+8
|
* Add tracing instrumentation to http clientIgor2021-07-201-1/+2
|
* Refactor testhelper.PrepareTestRootDir using t.CleanupIgor Drozdov2021-07-142-6/+2
|
* fix: upgrade of the gitaly dependencyPavlo Strokov2021-06-021-1/+1
| | | | | | | | | | | Gitaly project now properly respects module release flow and includes a module suffix in the package name. It requires to re-write all non-suffixed imports with suffixed of a specific version of tha module. With proper module versioning we don't need to use a 'replace' directive to point to specific commit and can use semantic versioning for the gitaly dependency. Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/3177
* Replace cleanup functions with t.CleanupIgor Drozdov2021-03-175-36/+36
| | | | | In this case we don't need to propagate cleanup function. It simplifies the code.
* Update testserver.go with tls 1.2 min versionUpdating-min-TLS-version-to-12Vitor Meireles De Sousa2020-11-171-0/+1
|
* Update httpclient.go with TLS 1.2 as minimum versionVitor Meireles De Sousa2020-11-171-0/+1
|
* GitLab API Client support for client certificatesPaul Okstad2020-11-174-25/+113
|
* Log Content-Length bytes in API resposneStan Hu2020-11-022-0/+6
| | | | | This will be useful to measure bandwidth sent in response to an API request, particularly with measuring the /api/v4/internal/lfs endpoint.
* tests: Replace assert with requirezj-remove-testify-assertZeger-Jan van de Weg2020-10-153-67/+64
| | | | | | | | | 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`.
* client: Allow User-Agent header to be overriddenzj-override-user-agentZeger-Jan van de Weg2020-10-142-2/+43
| | | | | | | | | | The user agent for requests to the internal API endpoints used the default Go provided user agent. This change updates that to always set something else, by default `GitLab-Shell`. Than for others importing the package, there's a new API to set it to something else. This has been done with new method, a setter, to maintain backwards compatibility in the API.
* Make it possible to propagate correlation ID across processesStan Hu2020-09-204-32/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix gitlab-shell not handling relative URLs over UNIX socketssh-fix-unix-relative-url-accessStan Hu2020-08-204-60/+89
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Generate and log correlation IDsStan Hu2020-07-313-10/+31
| | | | | This will make it easier to tie an SSH access request to Rails API and Gitaly requests.
* Fix race conditions with logrus testingsh-fix-logrus-raceStan Hu2020-05-111-0/+6
| | | | | | | | | 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
* Fix race conditions in testssh-add-http-status-codeStan Hu2020-05-081-30/+36
| | | | | | | | 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
* Add HTTP status code to internal API logsStan Hu2020-05-082-12/+35
| | | | This would make it easier to filter the logs by status code.
View Lorry Controller Status