<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/gitlab/gitlab-shell.git/client, branch main</title>
<subtitle>gitlab.com: gitlab-org/gitlab-shell.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/'/>
<entry>
<title>Add DNS discovery support for Gitaly/Praefect</title>
<updated>2023-02-14T09:16:13+00:00</updated>
<author>
<name>Quang-Minh Nguyen</name>
<email>qmnguyen@gitlab.com</email>
</author>
<published>2023-02-14T09:16:13+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=11227dd8a136f8735fc2d3d434345f2c24112f87'/>
<id>11227dd8a136f8735fc2d3d434345f2c24112f87</id>
<content type='text'>
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
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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
</pre>
</div>
</content>
</entry>
<entry>
<title>Define Do function for Gitlab net client</title>
<updated>2023-02-07T19:45:48+00:00</updated>
<author>
<name>Igor Drozdov</name>
<email>idrozdov@gitlab.com</email>
</author>
<published>2023-02-06T04:34:18+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=3a733910307284f8e5e33d63eadf521c74e146cf'/>
<id>3a733910307284f8e5e33d63eadf521c74e146cf</id>
<content type='text'>
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
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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
</pre>
</div>
</content>
</entry>
<entry>
<title>feat: make retryable http default client</title>
<updated>2023-01-30T08:54:42+00:00</updated>
<author>
<name>Steve Azzopardi</name>
<email>sazzopardi@gitlab.com</email>
</author>
<published>2023-01-30T08:21:17+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=80f684e48eca2bf1ef2006d84f8c49bec7104344'/>
<id>80f684e48eca2bf1ef2006d84f8c49bec7104344</id>
<content type='text'>
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 &lt;sazzopardi@gitlab.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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 &lt;sazzopardi@gitlab.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Stub retryable http values in tests</title>
<updated>2023-01-25T18:07:08+00:00</updated>
<author>
<name>Igor Drozdov</name>
<email>idrozdov@gitlab.com</email>
</author>
<published>2023-01-25T18:03:45+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=952a18f639eeef9f9d85506c5dc23be9549244f2'/>
<id>952a18f639eeef9f9d85506c5dc23be9549244f2</id>
<content type='text'>
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
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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
</pre>
</div>
</content>
</entry>
<entry>
<title>feat: put retryablehttp.Client behind feature flag</title>
<updated>2023-01-12T02:56:43+00:00</updated>
<author>
<name>Steve Azzopardi</name>
<email>sazzopardi@gitlab.com</email>
</author>
<published>2023-01-06T18:35:21+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=16a5c84310f6bb1790f16b6b2e4df90af493b07c'/>
<id>16a5c84310f6bb1790f16b6b2e4df90af493b07c</id>
<content type='text'>
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 &lt;sazzopardi@gitlab.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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 &lt;sazzopardi@gitlab.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>feat: retry on error</title>
<updated>2023-01-12T02:56:43+00:00</updated>
<author>
<name>Steve Azzopardi</name>
<email>sazzopardi@gitlab.com</email>
</author>
<published>2023-01-02T14:50:27+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=a093c9d3cfc1ee18368ebbf828dc61c15b74540c'/>
<id>a093c9d3cfc1ee18368ebbf828dc61c15b74540c</id>
<content type='text'>
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 &lt;sazzopardi@gitlab.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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 &lt;sazzopardi@gitlab.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Remove secret from request headers</title>
<updated>2022-10-17T15:59:16+00:00</updated>
<author>
<name>Igor Drozdov</name>
<email>idrozdov@gitlab.com</email>
</author>
<published>2022-10-17T15:59:14+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=07604117a05142f649e4194f6b5c67fee861f0d9'/>
<id>07604117a05142f649e4194f6b5c67fee861f0d9</id>
<content type='text'>
Now the requests are verified via JWT
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Now the requests are verified via JWT
</pre>
</div>
</content>
</entry>
<entry>
<title>Trim secret before signing JWT tokens</title>
<updated>2022-09-27T10:09:46+00:00</updated>
<author>
<name>Igor Drozdov</name>
<email>idrozdov@gitlab.com</email>
</author>
<published>2022-09-27T09:18:57+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=6efd76947ccce682ade20bf82ed1852123c29a04'/>
<id>6efd76947ccce682ade20bf82ed1852123c29a04</id>
<content type='text'>
With this change we don't rely on the secret to either contain
a newline or not contain it.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
With this change we don't rely on the secret to either contain
a newline or not contain it.
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'id-update-gitaly-to-v15' into 'main'</title>
<updated>2022-08-10T01:04:09+00:00</updated>
<author>
<name>Patrick Bajao</name>
<email>ebajao@gitlab.com</email>
</author>
<published>2022-08-10T01:04:09+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=dbf5fce6346184fc46e3dfd2128fd2fe18e7e81c'/>
<id>dbf5fce6346184fc46e3dfd2128fd2fe18e7e81c</id>
<content type='text'>
Update Gitaly to v15

See merge request gitlab-org/gitlab-shell!676</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Update Gitaly to v15

See merge request gitlab-org/gitlab-shell!676</pre>
</div>
</content>
</entry>
<entry>
<title>Fixed extra slashes in API request paths generated for geo</title>
<updated>2022-08-05T15:55:42+00:00</updated>
<author>
<name>Carlos Yu</name>
<email>carlosy@indeed.com</email>
</author>
<published>2022-08-05T15:55:42+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-shell.git/commit/?id=15e7e01ef05f9ef72c72a82217267837ca1beaa7'/>
<id>15e7e01ef05f9ef72c72a82217267837ca1beaa7</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
</feed>
