feat: [CODE-2212]: Improve SSH Configuration (#2446)

.local.env

@@ -4,7 +4,10 @@ GITNESS_PRINCIPAL_ADMIN_EMAIL=admin@gitness.io
 GITNESS_PRINCIPAL_ADMIN_PASSWORD=changeit
 GITNESS_WEBHOOK_ALLOW_LOOPBACK=true
 GITNESS_METRIC_ENABLED=false
+GITNESS_HTTP_HOST=localhost
 GITNESS_GITSPACE_ENABLE=true
 GITNESS_DEBUG=true
 GITNESS_DOCKER_API_VERSION=1.40
 GITNESS_SSH_ENABLE=true
+GITNESS_SSH_HOST=localhost
+GITNESS_SSH_PORT=2222

app/server/wire.go

@@ -30,9 +30,10 @@ func ProvideServer(config *types.Config, router *router.Router) *Server {
 	return &Server{
 		http.NewServer(
 			http.Config{
-				Port:     config.Server.HTTP.Port,
+				Host:     config.HTTP.Host,
-				Acme:     config.Server.Acme.Enabled,
+				Port:     config.HTTP.Port,
-				AcmeHost: config.Server.Acme.Host,
+				Acme:     config.Acme.Enabled,
+				AcmeHost: config.Acme.Host,
 			},
 			router,
 		),

app/url/provider.go

@@ -17,6 +17,7 @@ package url
 import (
 	"context"
 	"fmt"
+	"net"
 	"net/url"
 	"path"
 	"strconv"
@@ -186,12 +187,7 @@ func (p *provider) GenerateGITCloneSSHURL(_ context.Context, repoPath string) st
 	if !p.SSHEnabled {
 		return ""
 	}
-	repoPath = path.Clean(repoPath)
+	return BuildGITCloneSSHURL(p.SSHDefaultUser, p.gitSSHURL, repoPath)
-	if !strings.HasSuffix(repoPath, GITSuffix) {
-		repoPath += GITSuffix
-	}
-
-	return fmt.Sprintf("%s@%s:%s", p.SSHDefaultUser, p.gitSSHURL.String(), repoPath)
 }
 
 func (p *provider) GenerateUIBuildURL(_ context.Context, repoPath, pipelineIdentifier string, seqNumber int64) string {
@@ -222,3 +218,26 @@ func (p *provider) GetGITHostname(context.Context) string {
 func (p *provider) GetAPIProto(context.Context) string {
 	return p.apiURL.Scheme
 }
+
+func BuildGITCloneSSHURL(user string, sshURL *url.URL, repoPath string) string {
+	repoPath = path.Clean(repoPath)
+	if !strings.HasSuffix(repoPath, GITSuffix) {
+		repoPath += GITSuffix
+	}
+
+	// SSH clone url requires custom format depending on port to satisfy git
+	combinedPath := strings.Trim(path.Join(sshURL.Path, repoPath), "/")
+
+	// handle custom ports differently as otherwise git clone fails
+	if sshURL.Port() != "" && sshURL.Port() != "0" && sshURL.Port() != "22" {
+		return fmt.Sprintf(
+			"ssh://%s@%s/%s",
+			user, net.JoinHostPort(sshURL.Hostname(), sshURL.Port()), combinedPath,
+		)
+	}
+
+	return fmt.Sprintf(
+		"%s@%s:%s",
+		user, sshURL.Hostname(), combinedPath,
+	)
+}

cli/operations/server/config.go

@@ -51,6 +51,7 @@ import (
 const (
 	schemeHTTP     = "http"
 	schemeHTTPS    = "https"
+	schemeSSH      = "ssh"
 	gitnessHomeDir = ".gitness"
 	blobDir        = "blob"
 )
@@ -105,14 +106,27 @@ func LoadConfig() (*types.Config, error) {
 
 //nolint:gocognit // refactor if required
 func backfillURLs(config *types.Config) error {
-	// default base url
+	// default values for HTTP
-	// TODO: once we actually use the config.Server.HTTP.Proto, we have to update that here.
+	// TODO: once we actually use the config.HTTP.Proto, we have to update that here.
 	scheme, host, port, path := schemeHTTP, "localhost", "", ""
-
+	if config.HTTP.Host != "" {
+		host = config.HTTP.Host
+	}
 	// by default drop scheme's default port
-	if (scheme != schemeHTTP || config.Server.HTTP.Port != 80) &&
+	if config.HTTP.Port > 0 &&
-		(scheme != schemeHTTPS || config.Server.HTTP.Port != 443) {
+		(scheme != schemeHTTP || config.HTTP.Port != 80) &&
-		port = fmt.Sprint(config.Server.HTTP.Port)
+		(scheme != schemeHTTPS || config.HTTP.Port != 443) {
+		port = fmt.Sprint(config.HTTP.Port)
+	}
+
+	// default values for SSH
+	sshHost, sshPort := "localhost", ""
+	if config.SSH.Host != "" {
+		sshHost = config.SSH.Host
+	}
+	// by default drop scheme's default port
+	if config.SSH.Port > 0 && config.SSH.Port != 22 {
+		sshPort = fmt.Sprint(config.SSH.Port)
 	}
 
 	// backfil internal URLS before continuing override with user provided base (which is external facing)
@@ -150,6 +164,14 @@ func backfillURLs(config *types.Config) error {
 		host = u.Hostname()
 		port = u.Port()
 		path = u.Path
+
+		// overwrite sshhost with base url host, but keep port as is
+		sshHost = u.Hostname()
+	}
+
+	// backfill external facing URLs
+	if config.URL.GitSSH == "" {
+		config.URL.GitSSH = combineToRawURL(schemeSSH, sshHost, sshPort, "")
 	}
 
 	// create base URL object

cli/operations/server/config_test.go

@@ -22,9 +22,33 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestBackfillURLsHTTPPort(t *testing.T) {
+func TestBackfillURLsHTTPEmptyPort(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+
+	err := backfillURLs(config)
+	require.NoError(t, err)
+
+	require.Equal(t, "http://localhost", config.URL.Internal)
+	require.Equal(t, "http://host.docker.internal", config.URL.Container)
+
+	require.Equal(t, "http://localhost/api", config.URL.API)
+	require.Equal(t, "http://localhost/git", config.URL.Git)
+	require.Equal(t, "http://localhost", config.URL.UI)
+}
+
+func TestBackfillURLsSSHEmptyPort(t *testing.T) {
+	config := &types.Config{}
+
+	err := backfillURLs(config)
+	require.NoError(t, err)
+
+	require.Equal(t, "ssh://localhost", config.URL.GitSSH)
+}
+
+func TestBackfillURLsHTTPHostPort(t *testing.T) {
+	config := &types.Config{}
+	config.HTTP.Host = "myhost"
+	config.HTTP.Port = 1234
 
 	err := backfillURLs(config)
 	require.NoError(t, err)
@@ -32,14 +56,25 @@ func TestBackfillURLsHTTPPort(t *testing.T) {
 	require.Equal(t, "http://localhost:1234", config.URL.Internal)
 	require.Equal(t, "http://host.docker.internal:1234", config.URL.Container)
 
-	require.Equal(t, "http://localhost:1234/api", config.URL.API)
+	require.Equal(t, "http://myhost:1234/api", config.URL.API)
-	require.Equal(t, "http://localhost:1234/git", config.URL.Git)
+	require.Equal(t, "http://myhost:1234/git", config.URL.Git)
-	require.Equal(t, "http://localhost:1234", config.URL.UI)
+	require.Equal(t, "http://myhost:1234", config.URL.UI)
+}
+
+func TestBackfillURLsSSHHostPort(t *testing.T) {
+	config := &types.Config{}
+	config.SSH.Host = "myhost"
+	config.SSH.Port = 1234
+
+	err := backfillURLs(config)
+	require.NoError(t, err)
+
+	require.Equal(t, "ssh://myhost:1234", config.URL.GitSSH)
 }
 
 func TestBackfillURLsHTTPPortStripsDefaultHTTP(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 80
+	config.HTTP.Port = 80
 
 	err := backfillURLs(config)
 	require.NoError(t, err)
@@ -55,7 +90,7 @@ func TestBackfillURLsHTTPPortStripsDefaultHTTP(t *testing.T) {
 // TODO: Update once we add proper https support - as of now nothing is stripped!
 func TestBackfillURLsHTTPPortStripsDefaultHTTPS(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 443
+	config.HTTP.Port = 443
 
 	err := backfillURLs(config)
 	require.NoError(t, err)
@@ -68,6 +103,16 @@ func TestBackfillURLsHTTPPortStripsDefaultHTTPS(t *testing.T) {
 	require.Equal(t, "http://localhost:443", config.URL.UI)
 }
 
+func TestBackfillURLsSSHPortStripsDefault(t *testing.T) {
+	config := &types.Config{}
+	config.SSH.Port = 22
+
+	err := backfillURLs(config)
+	require.NoError(t, err)
+
+	require.Equal(t, "ssh://localhost", config.URL.GitSSH)
+}
+
 func TestBackfillURLsBaseInvalidProtocol(t *testing.T) {
 	config := &types.Config{}
 	config.URL.Base = "abc://xyz:4321/test"
@@ -102,7 +147,10 @@ func TestBackfillURLsBaseInvalidPort(t *testing.T) {
 
 func TestBackfillURLsBase(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+	config.HTTP.Host = "abc"
+	config.HTTP.Port = 1234
+	config.SSH.Host = "abc"
+	config.SSH.Port = 421
 	config.URL.Base = "https://xyz:4321/test"
 
 	err := backfillURLs(config)
@@ -114,11 +162,13 @@ func TestBackfillURLsBase(t *testing.T) {
 	require.Equal(t, "https://xyz:4321/test/api", config.URL.API)
 	require.Equal(t, "https://xyz:4321/test/git", config.URL.Git)
 	require.Equal(t, "https://xyz:4321/test", config.URL.UI)
+
+	require.Equal(t, "ssh://xyz:421", config.URL.GitSSH)
 }
 
 func TestBackfillURLsBaseDefaultPortHTTP(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+	config.HTTP.Port = 1234
 	config.URL.Base = "http://xyz/test"
 
 	err := backfillURLs(config)
@@ -134,7 +184,7 @@ func TestBackfillURLsBaseDefaultPortHTTP(t *testing.T) {
 
 func TestBackfillURLsBaseDefaultPortHTTPExplicit(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+	config.HTTP.Port = 1234
 	config.URL.Base = "http://xyz:80/test"
 
 	err := backfillURLs(config)
@@ -150,7 +200,7 @@ func TestBackfillURLsBaseDefaultPortHTTPExplicit(t *testing.T) {
 
 func TestBackfillURLsBaseDefaultPortHTTPS(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+	config.HTTP.Port = 1234
 	config.URL.Base = "https://xyz/test"
 
 	err := backfillURLs(config)
@@ -166,7 +216,7 @@ func TestBackfillURLsBaseDefaultPortHTTPS(t *testing.T) {
 
 func TestBackfillURLsBaseDefaultPortHTTPSExplicit(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+	config.HTTP.Port = 1234
 	config.URL.Base = "https://xyz:443/test"
 
 	err := backfillURLs(config)
@@ -182,7 +232,7 @@ func TestBackfillURLsBaseDefaultPortHTTPSExplicit(t *testing.T) {
 
 func TestBackfillURLsBaseRootPathStripped(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+	config.HTTP.Port = 1234
 	config.URL.Base = "https://xyz:4321/"
 
 	err := backfillURLs(config)
@@ -196,15 +246,30 @@ func TestBackfillURLsBaseRootPathStripped(t *testing.T) {
 	require.Equal(t, "https://xyz:4321", config.URL.UI)
 }
 
+func TestBackfillURLsSSHBasePathIgnored(t *testing.T) {
+	config := &types.Config{}
+	config.SSH.Port = 1234
+	config.URL.Base = "https://xyz:4321/abc"
+
+	err := backfillURLs(config)
+	require.NoError(t, err)
+
+	require.Equal(t, "ssh://xyz:1234", config.URL.GitSSH)
+}
+
 func TestBackfillURLsCustom(t *testing.T) {
 	config := &types.Config{}
-	config.Server.HTTP.Port = 1234
+	config.HTTP.Host = "abc"
+	config.HTTP.Port = 1234
+	config.SSH.Host = "abc"
+	config.SSH.Port = 1234
 	config.URL.Internal = "http://APIInternal/APIInternal/p"
 	config.URL.Container = "https://GitContainer/GitContainer/p"
 	config.URL.Base = "https://xyz:4321/test"
 	config.URL.API = "http://API:1111/API/p"
-	config.URL.Git = "https://Git:443/Git/p"
+	config.URL.Git = "https://GIT:443/GIT/p"
 	config.URL.UI = "http://UI:80/UI/p"
+	config.URL.GitSSH = "ssh://GITSSH:21/GITSSH/p"
 
 	err := backfillURLs(config)
 	require.NoError(t, err)
@@ -213,6 +278,8 @@ func TestBackfillURLsCustom(t *testing.T) {
 	require.Equal(t, "https://GitContainer/GitContainer/p", config.URL.Container)
 
 	require.Equal(t, "http://API:1111/API/p", config.URL.API)
-	require.Equal(t, "https://Git:443/Git/p", config.URL.Git)
+	require.Equal(t, "https://GIT:443/GIT/p", config.URL.Git)
 	require.Equal(t, "http://UI:80/UI/p", config.URL.UI)
+
+	require.Equal(t, "ssh://GITSSH:21/GITSSH/p", config.URL.GitSSH)
 }

cli/operations/server/server.go

@@ -138,7 +138,8 @@ func (c *command) run(*kingpin.ParseContext) error {
 	}
 
 	log.Info().
-		Int("port", config.Server.HTTP.Port).
+		Str("host", config.HTTP.Host).
+		Int("port", config.HTTP.Port).
 		Str("revision", version.GitCommit).
 		Str("repository", version.GitRepository).
 		Stringer("version", version.Version).

http/server.go

@@ -37,6 +37,7 @@ const (
 // TODO: expose via options?
 type Config struct {
 	Acme              bool
+	Host              string
 	Port              int
 	Cert              string
 	Key               string
@@ -78,7 +79,7 @@ func (s *Server) ListenAndServe() (*errgroup.Group, ShutdownFunction) {
 func (s *Server) listenAndServe() (*errgroup.Group, ShutdownFunction) {
 	var g errgroup.Group
 	s1 := &http.Server{
-		Addr:              fmt.Sprintf(":%d", s.config.Port),
+		Addr:              fmt.Sprintf("%s:%d", s.config.Host, s.config.Port),
 		ReadHeaderTimeout: s.config.ReadHeaderTimeout,
 		Handler:           s.handler,
 	}

types/config.go

@@ -53,7 +53,7 @@ type Config struct {
 	// URL defines the URLs via which the different parts of the service are reachable by.
 	URL struct {
 		// Base is used to generate external facing URLs in case they aren't provided explicitly.
-		// Value is derived from HTTP.Server unless explicitly specified (e.g. http://localhost:3000).
+		// Value is derived from Server.HTTP Config unless explicitly specified (e.g. http://localhost:3000).
 		Base string `envconfig:"GITNESS_URL_BASE"`
 
 		// Git defines the external URL via which the GIT API is reachable.
@@ -66,7 +66,8 @@ type Config struct {
 		Git string `envconfig:"GITNESS_URL_GIT"`
 
 		// GitSSH defines the external URL via which the GIT SSH server is reachable.
-		GitSSH string `envconfig:"GITNESS_URL_GIT_SSH" default:"localhost"`
+		// Value is derived from Base or SSH Config unless explicitly specified (e.g. ssh://localhost).
+		GitSSH string `envconfig:"GITNESS_URL_GIT_SSH"`
 
 		// API defines the external URL via which the rest API is reachable.
 		// NOTE: for routing to work properly, the request path reaching gitness has to end with `/api`
@@ -120,21 +121,19 @@ type Config struct {
 		MixedContent bool   `envconfig:"GITNESS_ENCRYPTER_MIXED_CONTENT"`
 	}
 
-	// Server defines the server configuration parameters.
+	// HTTP defines the http server configuration parameters
-	Server struct {
+	HTTP struct {
-		// HTTP defines the http configuration parameters
+		Port  int    `envconfig:"GITNESS_HTTP_PORT" default:"3000"`
-		HTTP struct {
+		Host  string `envconfig:"GITNESS_HTTP_HOST"`
-			Port  int    `envconfig:"GITNESS_HTTP_PORT" default:"3000"`
+		Proto string `envconfig:"GITNESS_HTTP_PROTO" default:"http"`
-			Proto string `envconfig:"GITNESS_HTTP_PROTO" default:"http"`
+	}
-		}
 
-		// Acme defines Acme configuration parameters.
+	// Acme defines Acme configuration parameters.
-		Acme struct {
+	Acme struct {
-			Enabled bool   `envconfig:"GITNESS_ACME_ENABLED"`
+		Enabled bool   `envconfig:"GITNESS_ACME_ENABLED"`
-			Endpont string `envconfig:"GITNESS_ACME_ENDPOINT"`
+		Endpont string `envconfig:"GITNESS_ACME_ENDPOINT"`
-			Email   bool   `envconfig:"GITNESS_ACME_EMAIL"`
+		Email   bool   `envconfig:"GITNESS_ACME_EMAIL"`
-			Host    string `envconfig:"GITNESS_ACME_HOST"`
+		Host    string `envconfig:"GITNESS_ACME_HOST"`
-		}
 	}
 
 	SSH struct {