Merge branch 'sh-remove-port-from-logging' into 'master'

Strip port and include remote IP in access logs

Closes #201

See merge request gitlab-org/gitlab-workhorse!337

authorization_test.go

@@ -7,7 +7,7 @@ import (
 	"regexp"
 	"testing"
 
-	"github.com/dgrijalva/jwt-go"
+	jwt "github.com/dgrijalva/jwt-go"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"

internal/gitaly/gitaly.go

@@ -3,10 +3,10 @@ package gitaly
 import (
 	"sync"
 
-	"github.com/grpc-ecosystem/go-grpc-middleware"
+	grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
 	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/auth"
+	gitalyauth "gitlab.com/gitlab-org/gitaly/auth"
 	gitalyclient "gitlab.com/gitlab-org/gitaly/client"
 	"google.golang.org/grpc"
 

internal/helper/logging.go

@@ -94,8 +94,11 @@ func (l *statsCollectingResponseWriter) writeAccessLog(r *http.Request) {
 func (l *statsCollectingResponseWriter) accessLogFields(r *http.Request) log.Fields {
 	duration := time.Since(l.started)
 
+	ip, _, _ := net.SplitHostPort(r.RemoteAddr)
+
 	return log.Fields{
 		"host":       r.Host,
+		"remoteIp":   ip,
 		"remoteAddr": r.RemoteAddr,
 		"method":     r.Method,
 		"uri":        ScrubURLParams(r.RequestURI),

internal/helper/logging_test.go

@@ -10,6 +10,40 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func Test_statsCollectingResponseWriter_remoteIp_accessLogFields(t *testing.T) {
+	testCases := []struct {
+		remoteAddr string
+		expected   string
+	}{
+		{remoteAddr: "", expected: ""},
+		{remoteAddr: "bogus", expected: ""},
+		{remoteAddr: "8.8.8.8:1234", expected: "8.8.8.8"},
+		{remoteAddr: "8.8.8.8", expected: ""},
+		{remoteAddr: "[2001:db8:85a3:8d3:1319:8a2e:370:7348]", expected: ""},
+		{remoteAddr: "[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443", expected: "2001:db8:85a3:8d3:1319:8a2e:370:7348"},
+	}
+
+	for _, tc := range testCases {
+		req, err := http.NewRequest("GET", "/blah", nil)
+		req.RemoteAddr = tc.remoteAddr
+		assert.Nil(t, err)
+
+		l := &statsCollectingResponseWriter{
+			rw:          nil,
+			status:      200,
+			wroteHeader: true,
+			written:     50,
+			started:     time.Now(),
+		}
+
+		fields := l.accessLogFields(req)
+
+		ip := fields["remoteIp"].(string)
+
+		assert.Equal(t, tc.expected, ip)
+	}
+}
+
 func Test_statsCollectingResponseWriter_accessLogFields(t *testing.T) {
 	passwords := []string{
 		"should_be_filtered",        // Basic case

internal/helper/raven.go

@@ -4,7 +4,7 @@ import (
 	"net/http"
 	"reflect"
 
-	"github.com/getsentry/raven-go"
+	raven "github.com/getsentry/raven-go"
 
 	correlation "gitlab.com/gitlab-org/labkit/correlation/raven"
 )

internal/secret/jwt.go

@@ -3,7 +3,7 @@ package secret
 import (
 	"fmt"
 
-	"github.com/dgrijalva/jwt-go"
+	jwt "github.com/dgrijalva/jwt-go"
 )
 
 var (

raven.go

@@ -4,7 +4,7 @@ import (
 	"net/http"
 	"os"
 
-	"github.com/getsentry/raven-go"
+	raven "github.com/getsentry/raven-go"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
 )

upload_test.go

@@ -11,7 +11,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/dgrijalva/jwt-go"
+	jwt "github.com/dgrijalva/jwt-go"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"