[TT-11470] Add human identifiable information in NodeData (TykTechnol…

…ogies#6229) ## **User description**  ## Description  Provide IP, PID and Hostname as identifiers for a node when querying MDCB in the `dataplanes` endpoint. This should be tested with the MDCB version in [this pr](https://github.com/TykTechnologies/tyk-sink/pull/525) ## Related Issue https://tyktech.atlassian.net/browse/TT-11470 ## Motivation and Context  ## How This Has Been Tested - Run Tyk in MDCB Env - Enable secure endpoints in MDCB - Consume the `/dataplanes` endpoint, and now you get host details per node ## Screenshots (if appropriate) ## Types of changes  - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist    - [ ] I ensured that the documentation is up to date - [ ] I explained why this PR updates go.mod in detail with reasoning why it's required - [ ] I would like a code coverage CI quality gate exception and have explained why ___ ## **Type** enhancement, tests ___ ## **Description** - Implemented a buffered logger (`BufferedLogger` and `BufferingFormatter`) to facilitate better logging, especially for testing scenarios. - Enhanced the `Gateway` struct to include node IP address handling, which fetches and stores the IP if not provided. - Added comprehensive tests for new features including the retrieval and logging of node IP addresses. - Introduced a utility function `getIpAddress` to fetch the first non-loopback IPv4 address, enhancing node identification. ___ ## **Changes walkthrough** <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement </strong></td><td><table> <tr> <td> <details> <summary><strong>buffered-logger.go</strong><dd><code>Implement Buffered Logger for Enhanced Testing</code>                      </dd></summary> <hr> gateway/buffered-logger.go <li>Added a new <code>BufferedLogger</code> and <code>BufferingFormatter</code> to handle logging in <br>a buffered manner, primarily for use in tests.<br> <li> <code>BufferedLogger</code> includes methods to retrieve logs of a specific level.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6229/files#diff-6f5a2cb4531e50ec9d0929cb7c1e5437c72dc829f5a1fe722048b0b117f391a2">+72/-0</a>    </td> </tr> <tr> <td> <details> <summary><strong>server.go</strong><dd><code>Enhance Gateway with Node IP Address Handling</code>                        </dd></summary> <hr> gateway/server.go <li>Added <code>Address</code> field to <code>hostDetails</code> struct to store node IP address.<br> <li> Enhanced <code>getHostDetails</code> to fetch and store the node's IP address if <br>not already provided.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6229/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+9/-1</a>      </td> </tr> <tr> <td> <details> <summary><strong>util.go</strong><dd><code>Add Utility Function to Fetch Non-Loopback IPv4 Address</code>    </dd></summary> <hr> gateway/util.go <li>Added <code>getIpAddress</code> function to retrieve the first non-loopback IPv4 <br>address.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6229/files#diff-1aa619f406837fb44ac6fba2c8922795eba0285dc4c8d2b0c15755b60b9ee1a5">+25/-0</a>    </td> </tr> </table></td></tr><tr><td><strong>Tests </strong></td><td><table> <tr> <td> <details> <summary><strong>server_test.go</strong><dd><code>Add Tests for Gateway Host Details and IP Address Retrieval</code></dd></summary> <hr> gateway/server_test.go <li>Added tests for <code>getHostDetails</code> to verify correct logging and IP <br>address retrieval.<br> <li> Utilized <code>BufferedLogger</code> in tests to capture log outputs.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6229/files#diff-d9f006370c9748c09affd99d0a4edeb8f3419057703a67fd70838a764a485696">+109/-2</a>  </td> </tr> <tr> <td> <details> <summary><strong>util_test.go</strong><dd><code>Implement Tests for IP Address Retrieval Utility Function</code></dd></summary> <hr> gateway/util_test.go <li>Added tests for <code>getIpAddress</code> to ensure it correctly identifies and <br>returns non-loopback IPv4 addresses.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6229/files#diff-3ca88235eab68f9eb691c139bb6e45f10a4038d4c02cf435f66ef7394734f925">+82/-1</a>    </td> </tr> </table></td></tr></tr></tbody></table> ___ > ✨ **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions --------- Co-authored-by: sredny buitrago <sredny.buitrago@gmail.com>
jonathanfoster · May 28, 2024 · 694483a · 694483a
1 parent 46ca642
commit 694483a
Show file tree

Hide file tree

Showing 10 changed files with 659 additions and 7 deletions.
diff --git a/apidef/rpc.go b/apidef/rpc.go
@@ -1,5 +1,7 @@
 package apidef
 
+import "github.com/TykTechnologies/tyk/internal/model"
+
 type InboundData struct {
 	KeyName      string
 	Value        string
@@ -32,6 +34,7 @@ type NodeData struct {
 	Tags            []string                   `json:"tags"`
 	Health          map[string]HealthCheckItem `json:"health"`
 	Stats           GWStats                    `json:"stats"`
+	HostDetails     model.HostDetails          `json:"host_details"`
 }
 
 type GWStats struct {

diff --git a/gateway/rpc_storage_handler.go b/gateway/rpc_storage_handler.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/TykTechnologies/storage/temporal/model"
 	"github.com/TykTechnologies/tyk/internal/cache"
+	im "github.com/TykTechnologies/tyk/internal/model"
 	"github.com/TykTechnologies/tyk/rpc"
 
 	"github.com/TykTechnologies/tyk/apidef"
@@ -158,6 +159,7 @@ func (r *RPCStorageHandler) buildNodeInfo() []byte {
 		intCheckDuration = int64(checkDuration / time.Second)
 	}
 
+	r.Gw.getHostDetails(r.Gw.GetConfig().PIDFileLocation)
 	node := apidef.NodeData{
 		NodeID:          r.Gw.GetNodeID(),
 		GroupID:         config.SlaveOptions.GroupID,
@@ -171,6 +173,11 @@ func (r *RPCStorageHandler) buildNodeInfo() []byte {
 			APIsCount:     r.Gw.apisByIDLen(),
 			PoliciesCount: r.Gw.policiesByIDLen(),
 		},
+		HostDetails: im.HostDetails{
+			Hostname: r.Gw.hostDetails.Hostname,
+			PID:      r.Gw.hostDetails.PID,
+			Address:  r.Gw.hostDetails.Address,
+		},
 	}
 
 	data, err := json.Marshal(node)

diff --git a/gateway/rpc_storage_handler_test.go b/gateway/rpc_storage_handler_test.go
@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/TykTechnologies/tyk/internal/model"
 	"github.com/TykTechnologies/tyk/rpc"
 
 	"github.com/TykTechnologies/tyk/apidef"
@@ -337,6 +338,11 @@ func TestGetGroupLoginCallback(t *testing.T) {
 					APIsCount:     0,
 					PoliciesCount: 0,
 				},
+				HostDetails: model.HostDetails{
+					Hostname: ts.Gw.hostDetails.Hostname,
+					PID:      ts.Gw.hostDetails.PID,
+					Address:  ts.Gw.hostDetails.Address,
+				},
 			}
 
 			nodeData, err := json.Marshal(expectedNodeInfo)
@@ -503,6 +509,14 @@ func TestRPCStorageHandler_BuildNodeInfo(t *testing.T) {
 				tc.expectedNodeInfo.NodeID = ts.Gw.GetNodeID()
 			}
 
+			if tc.expectedNodeInfo.HostDetails.Hostname == "" {
+				tc.expectedNodeInfo.HostDetails = model.HostDetails{
+					Hostname: ts.Gw.hostDetails.Hostname,
+					PID:      ts.Gw.hostDetails.PID,
+					Address:  ts.Gw.hostDetails.Address,
+				}
+			}
+
 			expected, err := json.Marshal(tc.expectedNodeInfo)
 			assert.Nil(t, err)
 

diff --git a/gateway/server.go b/gateway/server.go
@@ -65,6 +65,8 @@ import (
 	"github.com/TykTechnologies/tyk/user"
 
 	"github.com/TykTechnologies/tyk/internal/cache"
+	"github.com/TykTechnologies/tyk/internal/model"
+	"github.com/TykTechnologies/tyk/internal/netutil"
 )
 
 var (
@@ -193,18 +195,13 @@ type Gateway struct {
 
 	// RedisController keeps track of redis connection and singleton
 	StorageConnectionHandler *storage.ConnectionHandler
-	hostDetails              hostDetails
+	hostDetails              model.HostDetails
 
 	healthCheckInfo atomic.Value
 
 	dialCtxFn test.DialContext
 }
 
-type hostDetails struct {
-	Hostname string
-	PID      int
-}
-
 func NewGateway(config config.Config, ctx context.Context) *Gateway {
 	gw := &Gateway{
 		DefaultProxyMux: &proxyMux{
@@ -1349,7 +1346,7 @@ func writePIDFile(file string) error {
 	return ioutil.WriteFile(file, []byte(pid), 0600)
 }
 
-func readPIDFromFile(file string) (int, error) {
+var readPIDFromFile = func(file string) (int, error) {
 	b, err := ioutil.ReadFile(file)
 	if err != nil {
 		return 0, err
@@ -1532,6 +1529,8 @@ func (gw *Gateway) setUpConsul() error {
 	return err
 }
 
+var getIpAddress = netutil.GetIpAddress
+
 func (gw *Gateway) getHostDetails(file string) {
 	var err error
 	if gw.hostDetails.PID, err = readPIDFromFile(file); err != nil {
@@ -1540,6 +1539,17 @@ func (gw *Gateway) getHostDetails(file string) {
 	if gw.hostDetails.Hostname, err = os.Hostname(); err != nil {
 		mainLog.Error("Failed to get hostname: ", err)
 	}
+
+	gw.hostDetails.Address = gw.GetConfig().ListenAddress
+	if gw.hostDetails.Address == "" {
+		ips, err := getIpAddress()
+		if err != nil {
+			mainLog.Error("Failed to get node address: ", err)
+		}
+		if len(ips) > 0 {
+			gw.hostDetails.Address = ips[0]
+		}
+	}
 }
 
 func (gw *Gateway) getGlobalMDCBStorageHandler(keyPrefix string, hashKeys bool) storage.Handler {

diff --git a/gateway/server_test.go b/gateway/server_test.go
@@ -3,13 +3,18 @@ package gateway
 import (
 	"context"
 	"errors"
+	"fmt"
+	"net"
 	"testing"
 	"time"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 
 	"github.com/TykTechnologies/tyk/config"
+	"github.com/TykTechnologies/tyk/internal/netutil"
 	"github.com/TykTechnologies/tyk/internal/otel"
+	"github.com/TykTechnologies/tyk/test"
 	"github.com/TykTechnologies/tyk/user"
 )
 
@@ -216,3 +221,145 @@ func TestGateway_SyncResourcesWithReload(t *testing.T) {
 	})
 
 }
+
+type gatewayGetHostDetailsTestCheckFn func(*testing.T, *test.BufferedLogger, *Gateway)
+
+func gatewayGetHostDetailsTestHasErr(wantErr bool, errorText string) gatewayGetHostDetailsTestCheckFn {
+	return func(t *testing.T, bl *test.BufferedLogger, _ *Gateway) {
+		t.Helper()
+		logs := bl.GetLogs(logrus.ErrorLevel)
+		if wantErr {
+			assert.NotEmpty(t, logs, "Expected error logs but got none")
+			if errorText != "" {
+				for _, log := range logs {
+					assert.Contains(t, log.Message, errorText, "Expected log message to contain %q", errorText)
+				}
+			}
+		} else {
+			assert.Empty(t, logs, "Expected no error logs but got some")
+		}
+	}
+}
+
+func gatewayGetHostDetailsTestAddress() gatewayGetHostDetailsTestCheckFn {
+	return func(t *testing.T, _ *test.BufferedLogger, gw *Gateway) {
+		t.Helper()
+		assert.NotNil(t, net.ParseIP(gw.hostDetails.Address))
+	}
+}
+
+func defineGatewayGetHostDetailsTests() []struct {
+	name                string
+	before              func(*Gateway)
+	readPIDFromFile     func(string) (int, error)
+	netutilGetIpAddress func() ([]string, error)
+	checks              []gatewayGetHostDetailsTestCheckFn
+} {
+	var check = func(fns ...gatewayGetHostDetailsTestCheckFn) []gatewayGetHostDetailsTestCheckFn { return fns }
+
+	return []struct {
+		name                string
+		before              func(*Gateway)
+		readPIDFromFile     func(string) (int, error)
+		netutilGetIpAddress func() ([]string, error)
+		checks              []gatewayGetHostDetailsTestCheckFn
+	}{
+		{
+			name:            "fail-read-pid",
+			readPIDFromFile: func(_ string) (int, error) { return 0, fmt.Errorf("Error opening file") },
+			before: func(gw *Gateway) {
+				gw.SetConfig(config.Config{
+					ListenAddress: "127.0.0.1",
+				})
+			},
+			checks: check(
+				gatewayGetHostDetailsTestHasErr(true, "Error opening file"),
+			),
+		},
+		{
+			name:            "success-listen-address-set",
+			readPIDFromFile: func(string) (int, error) { return 1000, nil },
+			before: func(gw *Gateway) {
+				gw.SetConfig(config.Config{
+					ListenAddress: "127.0.0.1",
+				})
+			},
+			checks: check(
+				gatewayGetHostDetailsTestHasErr(false, ""),
+				gatewayGetHostDetailsTestAddress(),
+			),
+		},
+		{
+			name:            "success-listen-address-not-set",
+			readPIDFromFile: func(_ string) (int, error) { return 1000, nil },
+			before: func(gw *Gateway) {
+				gw.SetConfig(config.Config{
+					ListenAddress: "",
+				})
+			},
+			checks: check(
+				gatewayGetHostDetailsTestHasErr(false, ""),
+				gatewayGetHostDetailsTestAddress(),
+			),
+		},
+		{
+			name:            "fail-getting-network-address",
+			readPIDFromFile: func(_ string) (int, error) { return 1000, nil },
+			before: func(gw *Gateway) {
+				gw.SetConfig(config.Config{
+					ListenAddress: "",
+				})
+			},
+			netutilGetIpAddress: func() ([]string, error) { return nil, fmt.Errorf("Error getting network addresses") },
+			checks: check(
+				gatewayGetHostDetailsTestHasErr(true, "Error getting network addresses"),
+			),
+		},
+	}
+}
+
+func TestGatewayGetHostDetails(t *testing.T) {
+
+	var (
+		orig_readPIDFromFile = readPIDFromFile
+		orig_mainLog         = mainLog
+		orig_getIpAddress    = netutil.GetIpAddress
+		bl                   = test.NewBufferingLogger()
+	)
+
+	tests := defineGatewayGetHostDetailsTests()
+
+	// restore the original functions
+	defer func() {
+		readPIDFromFile = orig_readPIDFromFile
+		mainLog = orig_mainLog
+		getIpAddress = orig_getIpAddress
+	}()
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// clear logger mock buffer
+			bl.ClearLogs()
+			// replace functions with mocks
+			mainLog = bl.Logger.WithField("prefix", "test")
+			if tt.readPIDFromFile != nil {
+				readPIDFromFile = tt.readPIDFromFile
+			}
+
+			if tt.netutilGetIpAddress != nil {
+				getIpAddress = tt.netutilGetIpAddress
+			}
+
+			gw := &Gateway{}
+
+			if tt.before != nil {
+				tt.before(gw)
+			}
+
+			gw.getHostDetails(gw.GetConfig().PIDFileLocation)
+			for _, c := range tt.checks {
+				c(t, bl, gw)
+			}
+		})
+	}
+}
diff --git a/internal/model/host_details.go b/internal/model/host_details.go
@@ -0,0 +1,9 @@
+package model
+
+// HostDetails contains information about a host machine,
+// including its hostname, process ID (PID), and IP address.
+type HostDetails struct {
+	Hostname string
+	PID      int
+	Address  string
+}
diff --git a/internal/netutil/ip_address.go b/internal/netutil/ip_address.go
@@ -0,0 +1,30 @@
+package netutil
+
+import "net"
+
+var (
+	netInterfaceAddrs = net.InterfaceAddrs // used to allow mocking in tests
+)
+
+// GetIpAddress returns the list of non-loopback IP address (IPv4 and IPv6) found.
+// Returns error if it fails to get the list of addresses, empty if there's no valid IP addresses.
+func GetIpAddress() ([]string, error) {
+	var (
+		ips        []string
+		addrs, err = netInterfaceAddrs()
+	)
+
+	if err != nil {
+		return []string{}, err
+	}
+
+	for _, addr := range addrs {
+		if ipnet, ok := addr.(*net.IPNet); ok && !ipnet.IP.IsLoopback() {
+			if ipAdrr := ipnet.IP.To16(); ipAdrr != nil {
+				ips = append(ips, ipAdrr.String())
+			}
+		}
+	}
+
+	return ips, nil
+}