[TT-11470] Add human identifiable information in NodeData#6229
Conversation
|
PR Description updated to latest commit (904be81) |
PR Review
Code feedback:
✨ Review tool usage guide:Overview: The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.
See the review usage page for a comprehensive guide on using this tool. |
PR Code Suggestions
✨ Improve tool usage guide:Overview:
See the improve usage page for a comprehensive guide on using this tool. |
|
Hey @padiazg, nice to see a contribution from your side. Could you please consider moving any extractable parts into subpackages of /internal? The PR:
if there's no better location, create an |
💥 CI tests failed 🙈git-statediff --git a/gateway/server_test.go b/gateway/server_test.go
index 1d1aab4..3c32100 100644
--- a/gateway/server_test.go
+++ b/gateway/server_test.go
@@ -9,11 +9,12 @@ import (
"testing"
"time"
+ "github.com/sirupsen/logrus"
+ "github.com/stretchr/testify/assert"
+
"github.com/TykTechnologies/tyk/config"
"github.com/TykTechnologies/tyk/internal/otel"
"github.com/TykTechnologies/tyk/user"
- "github.com/sirupsen/logrus"
- "github.com/stretchr/testify/assert"
)
func TestGateway_afterConfSetup(t *testing.T) {
diff --git a/gateway/util_test.go b/gateway/util_test.go
index 6d18a1c..e50f08b 100644
--- a/gateway/util_test.go
+++ b/gateway/util_test.go
@@ -6,9 +6,10 @@ import (
"strings"
"testing"
+ "github.com/stretchr/testify/assert"
+
"github.com/TykTechnologies/tyk/apidef"
"github.com/TykTechnologies/tyk/config"
- "github.com/stretchr/testify/assert"
)
func TestAppendIfMissingUniqueness(t *testing.T) {Please look at the run or in the Checks tab. |
💥 CI tests failed 🙈git-statediff --git a/gateway/server_test.go b/gateway/server_test.go
index c7fd075..41e8cab 100644
--- a/gateway/server_test.go
+++ b/gateway/server_test.go
@@ -8,12 +8,13 @@ import (
"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/user"
- "github.com/sirupsen/logrus"
- "github.com/stretchr/testify/assert"
)
func TestGateway_afterConfSetup(t *testing.T) {Please look at the run or in the Checks tab. |
this is meant to be used only to catch the logs during tests, not to be used in any regular flow |
💥 CI tests failed 🙈git-stateall okPlease look at the run or in the Checks tab. |
|
API Changes --- prev.txt 2024-05-28 13:30:09.470612225 +0000
+++ current.txt 2024-05-28 13:30:06.346551925 +0000
@@ -1605,6 +1605,7 @@
Tags []string `json:"tags"`
Health map[string]HealthCheckItem `json:"health"`
Stats GWStats `json:"stats"`
+ HostDetails model.HostDetails `json:"host_details"`
}
type NotificationsManager struct {
@@ -11499,6 +11500,37 @@
TYPES
+type BufferedLog struct {
+ Message string `json:"message"`
+ Time time.Time `json:"time"`
+ Level logrus.Level `json:"level"`
+}
+ BufferedLog is the struct that holds the log entry.
+
+type BufferedLogger struct {
+ *logrus.Logger
+ // Has unexported fields.
+}
+ BufferedLogger is the struct that holds the logger and the buffer.
+
+func NewBufferingLogger() *BufferedLogger
+ NewBufferingLogger creates a new buffered logger.
+
+func (bl *BufferedLogger) ClearLogs()
+ ClearLogs clears the logs from the buffer. IMPORTANT: Must be called before
+ each test iteration.
+
+func (bl *BufferedLogger) GetLogs(Level logrus.Level) []*BufferedLog
+ GetLogs returns the logs that are stored in the buffer.
+
+type BufferingFormatter struct {
+ // Has unexported fields.
+}
+ BufferingFormatter is the struct that holds the buffer of the logs.
+
+func (f *BufferingFormatter) Format(entry *logrus.Entry) ([]byte, error)
+ Format is the method that stores the log entry in the buffer.
+
type ClientOption func(*http.Client)
Options for populating a http.Client
|
💥 CI tests failed 🙈git-stateall okPlease look at the run or in the Checks tab. |
💥 CI tests failed 🙈git-stateall okPlease look at the run or in the Checks tab. |
💥 CI tests failed 🙈git-stateall okPlease look at the run or in the Checks tab. |
| } | ||
|
|
||
| gw.hostDetails.Address = gw.GetConfig().ListenAddress | ||
| if gw.hostDetails.Address == "" { |
There was a problem hiding this comment.
Not sure this is correct. Wouldn't listenaddress be set to 0.0.0.0 to listen on all interfaces by default?
There was a problem hiding this comment.
probably, but that was on the original code: https://github.com/TykTechnologies/tyk/blob/master/gateway/server.go#L1923
I just changed gw.GetConfig().ListenAddress for gw.hostDetails.Address not to call the function twice.
titpetric
left a comment
titpetric
left a comment
There was a problem hiding this comment.
Mostly reviewed for the code structure conventions, models/ needs a rename, everything else LGTM
sredxny
left a comment
sredxny
left a comment
There was a problem hiding this comment.
looks good! Only address the comment above from Tit
💥 CI tests failed 🙈git-statediff --git a/gateway/server.go b/gateway/server.go
index 11e0676..0f7a00c 100644
--- a/gateway/server.go
+++ b/gateway/server.go
@@ -25,6 +25,7 @@ import (
textTemplate "text/template"
"time"
+ "github.com/TykTechnologies/storage/temporal/model"
"github.com/TykTechnologies/tyk/internal/crypto"
"github.com/TykTechnologies/tyk/internal/httputil"
"github.com/TykTechnologies/tyk/internal/otel"Please look at the run or in the Checks tab. |
GetIpAddres returns all addreses, IPv4 and IPv6 Updated tests to test IPv6
Updated tests that uses it
…t to use testify/assert
cbcc2ad to
7ecaa66
Compare
|
## **User description** <!-- Provide a general summary of your changes in the Title above --> ## Description <!-- Describe your changes in detail --> 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](TykTechnologies/tyk-sink#525) ## Related Issue https://tyktech.atlassian.net/browse/TT-11470 ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## 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 <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] 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 <!-- Go over all the following points, and put an `x` in all the boxes that apply --> <!-- If there are no documentation updates required, mark the item as checked. --> <!-- Raise up any additional concerns not covered by the 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 <[email protected]>
3 participants
User description
Description
Provide IP, PID and Hostname as identifiers for a node when querying MDCB in the
dataplanesendpoint. This should be tested with the MDCB version in this prRelated Issue
https://tyktech.atlassian.net/browse/TT-11470
Motivation and Context
How This Has Been Tested
/dataplanesendpoint, and now you get host details per nodeScreenshots (if appropriate)
Types of changes
Checklist
Type
enhancement, tests
Description
BufferedLoggerandBufferingFormatter) to facilitate better logging, especially for testing scenarios.Gatewaystruct to include node IP address handling, which fetches and stores the IP if not provided.getIpAddressto fetch the first non-loopback IPv4 address, enhancing node identification.Changes walkthrough
buffered-logger.go
Implement Buffered Logger for Enhanced Testinggateway/buffered-logger.go
BufferedLoggerandBufferingFormatterto handle logging ina buffered manner, primarily for use in tests.
BufferedLoggerincludes methods to retrieve logs of a specific level.server.go
Enhance Gateway with Node IP Address Handlinggateway/server.go
Addressfield tohostDetailsstruct to store node IP address.getHostDetailsto fetch and store the node's IP address ifnot already provided.
util.go
Add Utility Function to Fetch Non-Loopback IPv4 Addressgateway/util.go
getIpAddressfunction to retrieve the first non-loopback IPv4address.
server_test.go
Add Tests for Gateway Host Details and IP Address Retrievalgateway/server_test.go
getHostDetailsto verify correct logging and IPaddress retrieval.
BufferedLoggerin tests to capture log outputs.util_test.go
Implement Tests for IP Address Retrieval Utility Functiongateway/util_test.go
getIpAddressto ensure it correctly identifies andreturns non-loopback IPv4 addresses.