summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2020-08-17 22:19:56 -0700
committerStan Hu <stanhu@gmail.com>2020-08-20 16:54:36 -0700
commiteb3b35b9b0cc55fb8464d9b0662e6b94aafc54cc (patch)
treef25886b6a225f108c67c423dcbe13f027d4a18c1
parentfa730d2f859671f54c6f88bf2551fc771a1a5e6a (diff)
downloadgitlab-shell-sh-fix-unix-relative-url-access.tar.gz
Fix gitlab-shell not handling relative URLs over UNIX socketssh-fix-unix-relative-url-access
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
-rw-r--r--client/client_test.go127
-rw-r--r--client/httpclient.go16
-rw-r--r--client/httpclient_test.go4
-rw-r--r--client/httpsclient_test.go2
-rw-r--r--config.yml.example4
-rw-r--r--internal/config/config.go22
6 files changed, 105 insertions, 70 deletions
diff --git a/client/client_test.go b/client/client_test.go
index d520bbb..e92093a 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -7,6 +7,7 @@ import (
"io/ioutil"
"net/http"
"path"
+ "strings"
"testing"
"github.com/sirupsen/logrus"
@@ -21,63 +22,27 @@ func TestClients(t *testing.T) {
require.NoError(t, err)
defer testDirCleanup()
- requests := []testserver.TestRequestHandler{
- {
- Path: "/api/v4/internal/hello",
- Handler: func(w http.ResponseWriter, r *http.Request) {
- require.Equal(t, http.MethodGet, r.Method)
-
- fmt.Fprint(w, "Hello")
- },
- },
- {
- Path: "/api/v4/internal/post_endpoint",
- Handler: func(w http.ResponseWriter, r *http.Request) {
- require.Equal(t, http.MethodPost, r.Method)
-
- b, err := ioutil.ReadAll(r.Body)
- defer r.Body.Close()
-
- require.NoError(t, err)
-
- fmt.Fprint(w, "Echo: "+string(b))
- },
- },
- {
- Path: "/api/v4/internal/auth",
- Handler: func(w http.ResponseWriter, r *http.Request) {
- fmt.Fprint(w, r.Header.Get(secretHeaderName))
- },
- },
- {
- Path: "/api/v4/internal/error",
- Handler: func(w http.ResponseWriter, r *http.Request) {
- w.Header().Set("Content-Type", "application/json")
- w.WriteHeader(http.StatusBadRequest)
- body := map[string]string{
- "message": "Don't do that",
- }
- json.NewEncoder(w).Encode(body)
- },
- },
- {
- Path: "/api/v4/internal/broken",
- Handler: func(w http.ResponseWriter, r *http.Request) {
- panic("Broken")
- },
- },
- }
-
testCases := []struct {
- desc string
- caFile string
- server func(*testing.T, []testserver.TestRequestHandler) (string, func())
+ desc string
+ relativeURLRoot string
+ caFile string
+ server func(*testing.T, []testserver.TestRequestHandler) (string, func())
}{
{
desc: "Socket client",
server: testserver.StartSocketHttpServer,
},
{
+ desc: "Socket client with a relative URL at /",
+ relativeURLRoot: "/",
+ server: testserver.StartSocketHttpServer,
+ },
+ {
+ desc: "Socket client with relative URL at /gitlab",
+ relativeURLRoot: "/gitlab",
+ server: testserver.StartSocketHttpServer,
+ },
+ {
desc: "Http client",
server: testserver.StartHttpServer,
},
@@ -90,12 +55,12 @@ func TestClients(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
- url, cleanup := tc.server(t, requests)
+ url, cleanup := tc.server(t, buildRequests(t, tc.relativeURLRoot))
defer cleanup()
secret := "sssh, it's a secret"
- httpClient := NewHTTPClient(url, tc.caFile, "", false, 1)
+ httpClient := NewHTTPClient(url, tc.relativeURLRoot, tc.caFile, "", false, 1)
client, err := NewGitlabNetClient("", "", secret, httpClient)
require.NoError(t, err)
@@ -275,3 +240,61 @@ func testAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
assert.Equal(t, "sssh, it's a secret", string(header))
})
}
+
+func buildRequests(t *testing.T, relativeURLRoot string) []testserver.TestRequestHandler {
+ requests := []testserver.TestRequestHandler{
+ {
+ Path: "/api/v4/internal/hello",
+ Handler: func(w http.ResponseWriter, r *http.Request) {
+ require.Equal(t, http.MethodGet, r.Method)
+
+ fmt.Fprint(w, "Hello")
+ },
+ },
+ {
+ Path: "/api/v4/internal/post_endpoint",
+ Handler: func(w http.ResponseWriter, r *http.Request) {
+ require.Equal(t, http.MethodPost, r.Method)
+
+ b, err := ioutil.ReadAll(r.Body)
+ defer r.Body.Close()
+
+ require.NoError(t, err)
+
+ fmt.Fprint(w, "Echo: "+string(b))
+ },
+ },
+ {
+ Path: "/api/v4/internal/auth",
+ Handler: func(w http.ResponseWriter, r *http.Request) {
+ fmt.Fprint(w, r.Header.Get(secretHeaderName))
+ },
+ },
+ {
+ Path: "/api/v4/internal/error",
+ Handler: func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Type", "application/json")
+ w.WriteHeader(http.StatusBadRequest)
+ body := map[string]string{
+ "message": "Don't do that",
+ }
+ json.NewEncoder(w).Encode(body)
+ },
+ },
+ {
+ Path: "/api/v4/internal/broken",
+ Handler: func(w http.ResponseWriter, r *http.Request) {
+ panic("Broken")
+ },
+ },
+ }
+
+ relativeURLRoot = strings.Trim(relativeURLRoot, "/")
+ if relativeURLRoot != "" {
+ for i, r := range requests {
+ requests[i].Path = fmt.Sprintf("/%s%s", relativeURLRoot, r.Path)
+ }
+ }
+
+ return requests
+}
diff --git a/client/httpclient.go b/client/httpclient.go
index 63386f7..6635f1b 100644
--- a/client/httpclient.go
+++ b/client/httpclient.go
@@ -27,12 +27,12 @@ type HttpClient struct {
Host string
}
-func NewHTTPClient(gitlabURL, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64) *HttpClient {
+func NewHTTPClient(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64) *HttpClient {
var transport *http.Transport
var host string
if strings.HasPrefix(gitlabURL, unixSocketProtocol) {
- transport, host = buildSocketTransport(gitlabURL)
+ transport, host = buildSocketTransport(gitlabURL, gitlabRelativeURLRoot)
} else if strings.HasPrefix(gitlabURL, httpProtocol) {
transport, host = buildHttpTransport(gitlabURL)
} else if strings.HasPrefix(gitlabURL, httpsProtocol) {
@@ -41,7 +41,6 @@ func NewHTTPClient(gitlabURL, caFile, caPath string, selfSignedCert bool, readTi
return nil
}
-
c := &http.Client{
Transport: correlation.NewInstrumentedRoundTripper(transport),
Timeout: readTimeout(readTimeoutSeconds),
@@ -52,8 +51,9 @@ func NewHTTPClient(gitlabURL, caFile, caPath string, selfSignedCert bool, readTi
return client
}
-func buildSocketTransport(gitlabURL string) (*http.Transport, string) {
+func buildSocketTransport(gitlabURL, gitlabRelativeURLRoot string) (*http.Transport, string) {
socketPath := strings.TrimPrefix(gitlabURL, unixSocketProtocol)
+
transport := &http.Transport{
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
dialer := net.Dialer{}
@@ -61,7 +61,13 @@ func buildSocketTransport(gitlabURL string) (*http.Transport, string) {
},
}
- return transport, socketBaseUrl
+ host := socketBaseUrl
+ gitlabRelativeURLRoot = strings.Trim(gitlabRelativeURLRoot, "/")
+ if gitlabRelativeURLRoot != "" {
+ host = host + "/" + gitlabRelativeURLRoot
+ }
+
+ return transport, host
}
func buildHttpsTransport(caFile, caPath string, selfSignedCert bool, gitlabURL string) (*http.Transport, string) {
diff --git a/client/httpclient_test.go b/client/httpclient_test.go
index 1f0a4ed..fce0cd5 100644
--- a/client/httpclient_test.go
+++ b/client/httpclient_test.go
@@ -17,7 +17,7 @@ import (
func TestReadTimeout(t *testing.T) {
expectedSeconds := uint64(300)
- client := NewHTTPClient("http://localhost:3000", "", "", false, expectedSeconds)
+ client := NewHTTPClient("http://localhost:3000", "", "", "", false, expectedSeconds)
require.NotNil(t, client)
assert.Equal(t, time.Duration(expectedSeconds)*time.Second, client.Client.Timeout)
@@ -96,7 +96,7 @@ func TestEmptyBasicAuthSettings(t *testing.T) {
func setup(t *testing.T, username, password string, requests []testserver.TestRequestHandler) (*GitlabNetClient, func()) {
url, cleanup := testserver.StartHttpServer(t, requests)
- httpClient := NewHTTPClient(url, "", "", false, 1)
+ httpClient := NewHTTPClient(url, "", "", "", false, 1)
client, err := NewGitlabNetClient(username, password, "", httpClient)
require.NoError(t, err)
diff --git a/client/httpsclient_test.go b/client/httpsclient_test.go
index 6c3ae08..1c7435f 100644
--- a/client/httpsclient_test.go
+++ b/client/httpsclient_test.go
@@ -106,7 +106,7 @@ func setupWithRequests(t *testing.T, caFile, caPath string, selfSigned bool) (*G
url, cleanup := testserver.StartHttpsServer(t, requests)
- httpClient := NewHTTPClient(url, caFile, caPath, selfSigned, 1)
+ httpClient := NewHTTPClient(url, "", caFile, caPath, selfSigned, 1)
client, err := NewGitlabNetClient("", "", "", httpClient)
require.NoError(t, err)
diff --git a/config.yml.example b/config.yml.example
index 60435a3..6689ca8 100644
--- a/config.yml.example
+++ b/config.yml.example
@@ -15,6 +15,10 @@ user: git
# "http+unix://%2Fpath%2Fto%2Fsocket"
gitlab_url: "http+unix://%2Fhome%2Fgit%2Fgitlab%2Ftmp%2Fsockets%2Fgitlab-workhorse.socket"
+# When a http+unix:// is used in gitlab_url, this is the relative URL root to GitLab.
+# Not used if gitlab_url is http:// or https://.
+# gitlab_relative_url_root: "/"
+
# See installation.md#using-https for additional HTTPS configuration details.
http_settings:
# read_timeout: 300
diff --git a/internal/config/config.go b/internal/config/config.go
index be9efa3..e7abd59 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -27,16 +27,17 @@ type HttpSettingsConfig struct {
}
type Config struct {
- RootDir string
- LogFile string `yaml:"log_file"`
- LogFormat string `yaml:"log_format"`
- GitlabUrl string `yaml:"gitlab_url"`
- GitlabTracing string `yaml:"gitlab_tracing"`
- SecretFilePath string `yaml:"secret_file"`
- Secret string `yaml:"secret"`
- SslCertDir string `yaml:"ssl_cert_dir"`
- HttpSettings HttpSettingsConfig `yaml:"http_settings"`
- HttpClient *client.HttpClient
+ RootDir string
+ LogFile string `yaml:"log_file"`
+ LogFormat string `yaml:"log_format"`
+ GitlabUrl string `yaml:"gitlab_url"`
+ GitlabRelativeURLRoot string `yaml:"gitlab_relative_url_root"`
+ GitlabTracing string `yaml:"gitlab_tracing"`
+ SecretFilePath string `yaml:"secret_file"`
+ Secret string `yaml:"secret"`
+ SslCertDir string `yaml:"ssl_cert_dir"`
+ HttpSettings HttpSettingsConfig `yaml:"http_settings"`
+ HttpClient *client.HttpClient
}
func (c *Config) GetHttpClient() *client.HttpClient {
@@ -46,6 +47,7 @@ func (c *Config) GetHttpClient() *client.HttpClient {
client := client.NewHTTPClient(
c.GitlabUrl,
+ c.GitlabRelativeURLRoot,
c.HttpSettings.CaFile,
c.HttpSettings.CaPath,
c.HttpSettings.SelfSignedCert,