Browse Source

ipn/localapi: log calls to localapi (#17880)

Updates tailscale/corp#34238

Signed-off-by: James Sanderson <[email protected]>
James 'zofrex' Sanderson 3 months ago
parent
commit
9048ea25db
2 changed files with 41 additions and 16 deletions
  1. 17 7
      ipn/localapi/localapi.go
  2. 24 9
      ipn/localapi/localapi_test.go

+ 17 - 7
ipn/localapi/localapi.go

@@ -264,7 +264,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	}
-	if fn, ok := handlerForPath(r.URL.Path); ok {
+	if fn, route, ok := handlerForPath(r.URL.Path); ok {
+		h.logRequest(r.Method, route)
 		fn(h, w, r)
 	} else {
 		http.NotFound(w, r)
@@ -300,9 +301,9 @@ func (h *Handler) validHost(hostname string) bool {
 
 // handlerForPath returns the LocalAPI handler for the provided Request.URI.Path.
 // (the path doesn't include any query parameters)
-func handlerForPath(urlPath string) (h LocalAPIHandler, ok bool) {
+func handlerForPath(urlPath string) (h LocalAPIHandler, route string, ok bool) {
 	if urlPath == "/" {
-		return (*Handler).serveLocalAPIRoot, true
+		return (*Handler).serveLocalAPIRoot, "/", true
 	}
 	suff, ok := strings.CutPrefix(urlPath, "/localapi/v0/")
 	if !ok {
@@ -310,22 +311,31 @@ func handlerForPath(urlPath string) (h LocalAPIHandler, ok bool) {
 		// to people that they're not necessarily stable APIs. In practice we'll
 		// probably need to keep them pretty stable anyway, but for now treat
 		// them as an internal implementation detail.
-		return nil, false
+		return nil, "", false
 	}
 	if fn, ok := handler[suff]; ok {
 		// Here we match exact handler suffixes like "status" or ones with a
 		// slash already in their name, like "tka/status".
-		return fn, true
+		return fn, "/localapi/v0/" + suff, true
 	}
 	// Otherwise, it might be a prefix match like "files/*" which we look up
 	// by the prefix including first trailing slash.
 	if i := strings.IndexByte(suff, '/'); i != -1 {
 		suff = suff[:i+1]
 		if fn, ok := handler[suff]; ok {
-			return fn, true
+			return fn, "/localapi/v0/" + suff, true
 		}
 	}
-	return nil, false
+	return nil, "", false
+}
+
+func (h *Handler) logRequest(method, route string) {
+	switch method {
+	case httpm.GET, httpm.HEAD, httpm.OPTIONS:
+		// don't log safe methods
+	default:
+		h.Logf("localapi: [%s] %s", method, route)
+	}
 }
 
 func (*Handler) serveLocalAPIRoot(w http.ResponseWriter, r *http.Request) {

+ 24 - 9
ipn/localapi/localapi_test.go

@@ -40,6 +40,19 @@ import (
 	"tailscale.com/wgengine"
 )
 
+func handlerForTest(t testing.TB, h *Handler) *Handler {
+	if h.Actor == nil {
+		h.Actor = &ipnauth.TestActor{}
+	}
+	if h.b == nil {
+		h.b = &ipnlocal.LocalBackend{}
+	}
+	if h.logf == nil {
+		h.logf = logger.TestLogger(t)
+	}
+	return h
+}
+
 func TestValidHost(t *testing.T) {
 	tests := []struct {
 		host  string
@@ -57,7 +70,7 @@ func TestValidHost(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.host, func(t *testing.T) {
-			h := &Handler{}
+			h := handlerForTest(t, &Handler{})
 			if got := h.validHost(test.host); got != test.valid {
 				t.Errorf("validHost(%q)=%v, want %v", test.host, got, test.valid)
 			}
@@ -68,10 +81,9 @@ func TestValidHost(t *testing.T) {
 func TestSetPushDeviceToken(t *testing.T) {
 	tstest.Replace(t, &validLocalHostForTesting, true)
 
-	h := &Handler{
+	h := handlerForTest(t, &Handler{
 		PermitWrite: true,
-		b:           &ipnlocal.LocalBackend{},
-	}
+	})
 	s := httptest.NewServer(h)
 	defer s.Close()
 	c := s.Client()
@@ -125,9 +137,9 @@ func (b whoIsBackend) PeerCaps(ip netip.Addr) tailcfg.PeerCapMap {
 //
 // And https://github.com/tailscale/tailscale/issues/12465
 func TestWhoIsArgTypes(t *testing.T) {
-	h := &Handler{
+	h := handlerForTest(t, &Handler{
 		PermitRead: true,
-	}
+	})
 
 	match := func() (n tailcfg.NodeView, u tailcfg.UserProfile, ok bool) {
 		return (&tailcfg.Node{
@@ -190,7 +202,10 @@ func TestWhoIsArgTypes(t *testing.T) {
 
 func TestShouldDenyServeConfigForGOOSAndUserContext(t *testing.T) {
 	newHandler := func(connIsLocalAdmin bool) *Handler {
-		return &Handler{Actor: &ipnauth.TestActor{LocalAdmin: connIsLocalAdmin}, b: newTestLocalBackend(t)}
+		return handlerForTest(t, &Handler{
+			Actor: &ipnauth.TestActor{LocalAdmin: connIsLocalAdmin},
+			b:     newTestLocalBackend(t),
+		})
 	}
 	tests := []struct {
 		name     string
@@ -298,11 +313,11 @@ func TestServeWatchIPNBus(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			h := &Handler{
+			h := handlerForTest(t, &Handler{
 				PermitRead:  tt.permitRead,
 				PermitWrite: tt.permitWrite,
 				b:           newTestLocalBackend(t),
-			}
+			})
 			s := httptest.NewServer(h)
 			defer s.Close()
 			c := s.Client()