Skip to content

Commit 1f6b19d

Browse files
author
Nick Thomas
committed
Merge branch 'id-api-https' into 'master'
Support calling internal API using HTTPS Closes #179 See merge request gitlab-org/gitlab-shell!297
2 parents 805bdd0 + cffbe0e commit 1f6b19d

File tree

10 files changed

+263
-16
lines changed

10 files changed

+263
-16
lines changed

‎go/internal/config/config.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"os"
77
"path"
88
"path/filepath"
9-
"strings"
109

1110
yaml "gopkg.in/yaml.v2"
1211
)
@@ -26,6 +25,9 @@ type HttpSettingsConfig struct {
2625
User string `yaml:"user"`
2726
Password string `yaml:"password"`
2827
ReadTimeoutSeconds uint64 `yaml:"read_timeout"`
28+
CaFile string `yaml:"ca_file"`
29+
CaPath string `yaml:"ca_path"`
30+
SelfSignedCert bool `yaml:"self_signed_cert"`
2931
}
3032

3133
type Config struct {
@@ -59,10 +61,6 @@ func (c *Config) FeatureEnabled(featureName string) bool {
5961
return false
6062
}
6163

62-
if !strings.HasPrefix(c.GitlabUrl, "http+unix://") && !strings.HasPrefix(c.GitlabUrl, "http://") {
63-
return false
64-
}
65-
6664
for _, enabledFeature := range c.Migration.Features {
6765
if enabledFeature == featureName {
6866
return true

‎go/internal/config/config_test.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ func TestParseConfig(t *testing.T) {
9494
secret: "default-secret-content",
9595
httpSettings: HttpSettingsConfig{User: "user_basic_auth", Password: "password_basic_auth", ReadTimeoutSeconds: 500},
9696
},
97+
{
98+
yaml: "http_settings:\n ca_file: /etc/ssl/cert.pem\n ca_path: /etc/pki/tls/certs\n self_signed_cert: true",
99+
path: path.Join(testRoot, "gitlab-shell.log"),
100+
format: "text",
101+
secret: "default-secret-content",
102+
httpSettings: HttpSettingsConfig{CaFile: "/etc/ssl/cert.pem", CaPath: "/etc/pki/tls/certs", SelfSignedCert: true},
103+
},
97104
}
98105

99106
for _, tc := range testCases {
@@ -158,13 +165,13 @@ func TestFeatureEnabled(t *testing.T) {
158165
expectEnabled: true,
159166
},
160167
{
161-
desc: "When the protocol is not supported",
168+
desc: "When the protocol is https and the feature enabled",
162169
config: &Config{
163170
GitlabUrl: "https://localhost:3000",
164171
Migration: MigrationConfig{Enabled: true, Features: []string{"discover"}},
165172
},
166173
feature: "discover",
167-
expectEnabled: false,
174+
expectEnabled: true,
168175
},
169176
}
170177

‎go/internal/config/httpclient.go

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,21 @@ package config
22

33
import (
44
"context"
5+
"crypto/tls"
6+
"crypto/x509"
7+
"io/ioutil"
58
"net"
69
"net/http"
10+
"path/filepath"
711
"strings"
812
"time"
913
)
1014

1115
const (
1216
socketBaseUrl = "http://unix"
13-
UnixSocketProtocol = "http+unix://"
14-
HttpProtocol = "http://"
17+
unixSocketProtocol = "http+unix://"
18+
httpProtocol = "http://"
19+
httpsProtocol = "https://"
1520
defaultReadTimeoutSeconds = 300
1621
)
1722

@@ -27,10 +32,12 @@ func (c *Config) GetHttpClient() *HttpClient {
2732

2833
var transport *http.Transport
2934
var host string
30-
if strings.HasPrefix(c.GitlabUrl, UnixSocketProtocol) {
35+
if strings.HasPrefix(c.GitlabUrl, unixSocketProtocol) {
3136
transport, host = c.buildSocketTransport()
32-
} else if strings.HasPrefix(c.GitlabUrl, HttpProtocol) {
37+
} else if strings.HasPrefix(c.GitlabUrl, httpProtocol) {
3338
transport, host = c.buildHttpTransport()
39+
} else if strings.HasPrefix(c.GitlabUrl, httpsProtocol) {
40+
transport, host = c.buildHttpsTransport()
3441
} else {
3542
return nil
3643
}
@@ -48,7 +55,7 @@ func (c *Config) GetHttpClient() *HttpClient {
4855
}
4956

5057
func (c *Config) buildSocketTransport() (*http.Transport, string) {
51-
socketPath := strings.TrimPrefix(c.GitlabUrl, UnixSocketProtocol)
58+
socketPath := strings.TrimPrefix(c.GitlabUrl, unixSocketProtocol)
5259
transport := &http.Transport{
5360
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
5461
dialer := net.Dialer{}
@@ -59,6 +66,47 @@ func (c *Config) buildSocketTransport() (*http.Transport, string) {
5966
return transport, socketBaseUrl
6067
}
6168

69+
func (c *Config) buildHttpsTransport() (*http.Transport, string) {
70+
certPool, err := x509.SystemCertPool()
71+
72+
if err != nil {
73+
certPool = x509.NewCertPool()
74+
}
75+
76+
caFile := c.HttpSettings.CaFile
77+
if caFile != "" {
78+
addCertToPool(certPool, caFile)
79+
}
80+
81+
caPath := c.HttpSettings.CaPath
82+
if caPath != "" {
83+
fis, _ := ioutil.ReadDir(caPath)
84+
for _, fi := range fis {
85+
if fi.IsDir() {
86+
continue
87+
}
88+
89+
addCertToPool(certPool, filepath.Join(caPath, fi.Name()))
90+
}
91+
}
92+
93+
transport := &http.Transport{
94+
TLSClientConfig: &tls.Config{
95+
RootCAs: certPool,
96+
InsecureSkipVerify: c.HttpSettings.SelfSignedCert,
97+
},
98+
}
99+
100+
return transport, c.GitlabUrl
101+
}
102+
103+
func addCertToPool(certPool *x509.CertPool, fileName string) {
104+
cert, err := ioutil.ReadFile(fileName)
105+
if err == nil {
106+
certPool.AppendCertsFromPEM(cert)
107+
}
108+
}
109+
62110
func (c *Config) buildHttpTransport() (*http.Transport, string) {
63111
return &http.Transport{}, c.GitlabUrl
64112
}

‎go/internal/gitlabnet/client_test.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,21 @@ import (
66
"fmt"
77
"io/ioutil"
88
"net/http"
9+
"path"
910
"testing"
1011

1112
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
1314
"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
1415
"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet/testserver"
16+
"gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper"
1517
)
1618

1719
func TestClients(t *testing.T) {
20+
testDirCleanup, err := testhelper.PrepareTestRootDir()
21+
require.NoError(t, err)
22+
defer testDirCleanup()
23+
1824
requests := []testserver.TestRequestHandler{
1925
{
2026
Path: "/api/v4/internal/hello",
@@ -64,19 +70,26 @@ func TestClients(t *testing.T) {
6470

6571
testCases := []struct {
6672
desc string
67-
secret string
73+
config *config.Config
6874
server func([]testserver.TestRequestHandler) (func(), string, error)
6975
}{
7076
{
7177
desc: "Socket client",
72-
secret: "sssh, it's a secret",
78+
config: &config.Config{},
7379
server: testserver.StartSocketHttpServer,
7480
},
7581
{
7682
desc: "Http client",
77-
secret: "sssh, it's a secret",
83+
config: &config.Config{},
7884
server: testserver.StartHttpServer,
7985
},
86+
{
87+
desc: "Https client",
88+
config: &config.Config{
89+
HttpSettings: config.HttpSettingsConfig{CaFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt")},
90+
},
91+
server: testserver.StartHttpsServer,
92+
},
8093
}
8194

8295
for _, tc := range testCases {
@@ -85,7 +98,10 @@ func TestClients(t *testing.T) {
8598
defer cleanup()
8699
require.NoError(t, err)
87100

88-
client, err := GetClient(&config.Config{GitlabUrl: url, Secret: tc.secret})
101+
tc.config.GitlabUrl = url
102+
tc.config.Secret = "sssh, it's a secret"
103+
104+
client, err := GetClient(tc.config)
89105
require.NoError(t, err)
90106

91107
testBrokenRequest(t, client)

‎go/internal/gitlabnet/httpsclient_test.go

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
package gitlabnet
2+
3+
import (
4+
"fmt"
5+
"io/ioutil"
6+
"net/http"
7+
"path"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
13+
"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet/testserver"
14+
"gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper"
15+
)
16+
17+
func TestSuccessfulRequests(t *testing.T) {
18+
testCases := []struct {
19+
desc string
20+
config *config.Config
21+
}{
22+
{
23+
desc: "Valid CaFile",
24+
config: &config.Config{
25+
HttpSettings: config.HttpSettingsConfig{CaFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt")},
26+
},
27+
},
28+
{
29+
desc: "Valid CaPath",
30+
config: &config.Config{
31+
HttpSettings: config.HttpSettingsConfig{CaPath: path.Join(testhelper.TestRoot, "certs/valid")},
32+
},
33+
},
34+
{
35+
desc: "Self signed cert option enabled",
36+
config: &config.Config{
37+
HttpSettings: config.HttpSettingsConfig{SelfSignedCert: true},
38+
},
39+
},
40+
{
41+
desc: "Invalid cert with self signed cert option enabled",
42+
config: &config.Config{
43+
HttpSettings: config.HttpSettingsConfig{SelfSignedCert: true, CaFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt")},
44+
},
45+
},
46+
}
47+
48+
for _, tc := range testCases {
49+
t.Run(tc.desc, func(t *testing.T) {
50+
client, cleanup := setupWithRequests(t, tc.config)
51+
defer cleanup()
52+
53+
response, err := client.Get("/hello")
54+
require.NoError(t, err)
55+
require.NotNil(t, response)
56+
57+
defer response.Body.Close()
58+
59+
responseBody, err := ioutil.ReadAll(response.Body)
60+
assert.NoError(t, err)
61+
assert.Equal(t, string(responseBody), "Hello")
62+
})
63+
}
64+
}
65+
66+
func TestFailedRequests(t *testing.T) {
67+
testCases := []struct {
68+
desc string
69+
config *config.Config
70+
}{
71+
{
72+
desc: "Invalid CaFile",
73+
config: &config.Config{
74+
HttpSettings: config.HttpSettingsConfig{CaFile: path.Join(testhelper.TestRoot, "certs/invalid/server.crt")},
75+
},
76+
},
77+
{
78+
desc: "Invalid CaPath",
79+
config: &config.Config{
80+
HttpSettings: config.HttpSettingsConfig{CaPath: path.Join(testhelper.TestRoot, "certs/invalid")},
81+
},
82+
},
83+
{
84+
desc: "Empty config",
85+
config: &config.Config{},
86+
},
87+
}
88+
89+
for _, tc := range testCases {
90+
t.Run(tc.desc, func(t *testing.T) {
91+
client, cleanup := setupWithRequests(t, tc.config)
92+
defer cleanup()
93+
94+
_, err := client.Get("/hello")
95+
require.Error(t, err)
96+
97+
assert.Equal(t, err.Error(), "Internal API unreachable")
98+
})
99+
}
100+
}
101+
102+
func setupWithRequests(t *testing.T, config *config.Config) (*GitlabClient, func()) {
103+
testDirCleanup, err := testhelper.PrepareTestRootDir()
104+
require.NoError(t, err)
105+
defer testDirCleanup()
106+
107+
requests := []testserver.TestRequestHandler{
108+
{
109+
Path: "/api/v4/internal/hello",
110+
Handler: func(w http.ResponseWriter, r *http.Request) {
111+
require.Equal(t, http.MethodGet, r.Method)
112+
113+
fmt.Fprint(w, "Hello")
114+
},
115+
},
116+
}
117+
118+
cleanup, url, err := testserver.StartHttpsServer(requests)
119+
require.NoError(t, err)
120+
121+
config.GitlabUrl = url
122+
client, err := GetClient(config)
123+
require.NoError(t, err)
124+
125+
return client, cleanup
126+
}

‎go/internal/gitlabnet/testserver/testserver.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package testserver
22

33
import (
4+
"crypto/tls"
45
"io/ioutil"
56
"log"
67
"net"
@@ -9,6 +10,8 @@ import (
910
"os"
1011
"path"
1112
"path/filepath"
13+
14+
"gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper"
1215
)
1316

1417
var (
@@ -50,6 +53,23 @@ func StartHttpServer(handlers []TestRequestHandler) (func(), string, error) {
5053
return server.Close, server.URL, nil
5154
}
5255

56+
func StartHttpsServer(handlers []TestRequestHandler) (func(), string, error) {
57+
crt := path.Join(testhelper.TestRoot, "certs/valid/server.crt")
58+
key := path.Join(testhelper.TestRoot, "certs/valid/server.key")
59+
60+
server := httptest.NewUnstartedServer(buildHandler(handlers))
61+
cer, err := tls.LoadX509KeyPair(crt, key)
62+
63+
if err != nil {
64+
return nil, "", err
65+
}
66+
67+
server.TLS = &tls.Config{Certificates: []tls.Certificate{cer}}
68+
server.StartTLS()
69+
70+
return server.Close, server.URL, nil
71+
}
72+
5373
func cleanupSocket() {
5474
os.RemoveAll(tempDir)
5575
}

‎go/internal/testhelper/testdata/testroot/certs/invalid/server.crt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-----BEGIN CERTIFICATE-----
2+
MinvalidcertAOvHjs6cs1R9MAoGCCqGSM49BAMCMBQxEjAQBgNVBAMMCWxvY2Fs
3+
ainvalidcertOTA0MjQxNjM4NTBaFw0yOTA0MjExNjM4NTBaMBQxEjAQBgNVBAMM
4+
CinvalidcertdDB2MBAGByqGSM49AgEGBSuBBAAiA2IABJ5m7oW9OuL7aTAC04sL
5+
3invalidcertdB2L0GsVCImav4PEpx6UAjkoiNGW9j0zPdNgxTYDjiCaGmr1aY2X
6+
kinvalidcert7MNq7H8v7Ce/vrKkcDMOX8Gd/ddT3dEVqzAKBggqhkjOPQQDAgNp
7+
AinvalidcertswcyjiB+A+ZjMSfaOsA2hAP0I3fkTcry386DePViMfnaIjm7rcuu
8+
Jinvalidcert5V5CHypOxio1tOtGjaDkSH2FCdoatMyIe02+F6TIo44i4J/zjN52
9+
Jinvalidcert
10+
-----END CERTIFICATE-----

‎go/internal/testhelper/testdata/testroot/certs/valid/dir/.gitkeep

Whitespace-only changes.

0 commit comments

Comments
 (0)