[TT-12957] OAS Uptime Tests migrations

[titpetric]

PR Type

Enhancement, Tests

---

Description

• Renamed Test struct to UptimeTests for clarity.

• Introduced UptimeTestsServiceDiscovery with RecheckWait field.

• Updated methods and logic to support the new struct names.

• Adjusted and added corresponding unit tests for the changes.

---

Changes walkthrough 📝

	Relevant files
Enhancement	upstream.go Refactor and enhance uptime test configuration                      apidef/oas/upstream.go Renamed Test struct to UptimeTests. Added UptimeTestsServiceDiscovery with RecheckWait field. Updated methods Fill and ExtractTo to handle new struct. Introduced NewUptimeTestsServiceDiscovery constructor. +33/-19
Tests	upstream_test.go Update unit tests for UptimeTests struct                                  apidef/oas/upstream_test.go Renamed test function TestTest to TestUptimeTests. Updated test logic to align with UptimeTests struct. +3/-3

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[buger]

💔 The detected issue is not in one of the allowed statuses 💔

Detected Status	Blocked	❌
Allowed Statuses	In Dev,In Code Review,Ready for Testing,In Test,In Progress,In Review	✔️

Please ensure your jira story is in one of the allowed statuses

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The UptimeTests struct is initialized with a ServiceDiscovery field in multiple places, but there is no validation or error handling to ensure the field is correctly populated. This could lead to runtime issues if the field is unexpectedly nil. if u.UptimeTests == nil { u.UptimeTests = &UptimeTests{} } u.UptimeTests.Fill(api.UptimeTests) if ShouldOmit(u.UptimeTests) { u.UptimeTests = nil Code Duplication The logic for initializing and handling UptimeTests and its ServiceDiscovery field appears to be duplicated across multiple methods. Consider refactoring to reduce redundancy and improve maintainability. if u.UptimeTests == nil { u.UptimeTests = &UptimeTests{} defer func() { u.UptimeTests = nil }() } u.UptimeTests.ExtractTo(&api.UptimeTests)

[github-actions]

API Changes

--- prev.txt	2025-02-12 07:59:16.677952131 +0000
+++ current.txt	2025-02-12 07:59:11.796906085 +0000
@@ -412,6 +412,7 @@
 	Name    string `bson:"name" json:"name"`
 	Message string `bson:"message" json:"message"`
 }
+    UptimeTestCommand handles additional checks for tcp connections.
 
 type CircuitBreakerMeta struct {
 	Disabled             bool    `bson:"disabled" json:"disabled"`
@@ -773,6 +774,10 @@
 	Headers             map[string]string `bson:"headers" json:"headers"`
 	Body                string            `bson:"body" json:"body"`
 }
+    HostCheckObject represents a single uptime test check.
+
+func (h *HostCheckObject) AddCommand(name, message string)
+    AddCommand will append a new command to the test.
 
 type HostDetails struct {
 	Hostname string
@@ -1349,6 +1354,7 @@
 	CheckList []HostCheckObject `bson:"check_list" json:"check_list"`
 	Config    UptimeTestsConfig `bson:"config" json:"config"`
 }
+    UptimeTests holds the test configuration for uptime tests.
 
 type UptimeTestsConfig struct {
 	ExpireUptimeAnalyticsAfter int64                         `bson:"expire_utime_after" json:"expire_utime_after"` // must have an expireAt TTL index set (http://docs.mongodb.org/manual/tutorial/expire-data/)
@@ -4106,19 +4112,6 @@
 func (t *TLSTransport) Fill(api apidef.APIDefinition)
     Fill fills *TLSTransport from apidef.ServiceDiscoveryConfiguration.
 
-type Test struct {
-	// ServiceDiscovery contains the configuration related to test Service Discovery.
-	// Tyk classic API definition: `proxy.service_discovery`
-	ServiceDiscovery *ServiceDiscovery `bson:"serviceDiscovery,omitempty" json:"serviceDiscovery,omitempty"`
-}
-    Test holds the test configuration for service discovery.
-
-func (t *Test) ExtractTo(uptimeTests *apidef.UptimeTests)
-    ExtractTo extracts *Test into *apidef.UptimeTests.
-
-func (t *Test) Fill(uptimeTests apidef.UptimeTests)
-    Fill fills *Test from apidef.UptimeTests.
-
 type Token struct {
 	// Enabled activates the token based authentication mode.
 	//
@@ -4362,8 +4355,8 @@
 	// Tyk classic API definition: `proxy.service_discovery`
 	ServiceDiscovery *ServiceDiscovery `bson:"serviceDiscovery,omitempty" json:"serviceDiscovery,omitempty"`
 
-	// Test contains the configuration related to uptime tests.
-	Test *Test `bson:"test,omitempty" json:"test,omitempty"`
+	// UptimeTests contains the configuration related to uptime tests.
+	UptimeTests *UptimeTests `bson:"uptimeTests,omitempty" json:"uptimeTests,omitempty"`
 
 	// MutualTLS contains the configuration for establishing a mutual TLS connection between Tyk and the upstream server.
 	MutualTLS *MutualTLS `bson:"mutualTLS,omitempty" json:"mutualTLS,omitempty"`
@@ -4479,6 +4472,81 @@
     Fill populates the UpstreamRequestSigning fields from the given
     apidef.APIDefinition configuration.
 
+type UptimeTest struct {
+	// CheckURL is the URL for a request. If service discovery is in use,
+	// the hostname will be resolved to a service host.
+	//
+	// Examples:
+	//
+	// - `http://database1.company.local`
+	// - `https://webcluster.service/health`
+	// - `127.0.0.1:6379` (for TCP checks).
+	CheckURL string `bson:"url" json:"url"`
+
+	// Protocol is the protocol for the request. Supported values are
+	// `http` and `tcp`, depending on what kind of check is performed.
+	Protocol string `bson:"protocol" json:"protocol"`
+
+	// Timeout declares a timeout for the request. If the test exceeds
+	// this timeout, the check fails.
+	Timeout time.ReadableDuration `bson:"timeout" json:"timeout"`
+
+	// Method allows you to customize the HTTP method for the test (`GET`, `POST`,...).
+	Method string `bson:"method" json:"method"`
+
+	// Headers contain any custom headers for the back end service.
+	Headers map[string]string `bson:"headers" json:"headers,omitempty"`
+
+	// Body is the body of the test request.
+	Body string `bson:"body" json:"body"`
+
+	// Commands are used for TCP checks.
+	Commands []UptimeTestCommand `bson:"commands" json:"commands,omitempty"`
+
+	// EnableProxyProtocol enables proxy protocol support when making request.
+	// The back end service needs to support this.
+	EnableProxyProtocol bool `bson:"enableProxyProtocol" json:"enableProxyProtocol"`
+}
+    UptimeTest configures an uptime test check.
+
+func (t *UptimeTest) AddCommand(name, message string)
+    AddCommand will append a new command to the test.
+
+type UptimeTestCommand struct {
+	// Name can be either `send` or `expect`, designating if the
+	// message should be sent, or read from the connection.
+	Name string `bson:"name" json:"name"`
+
+	// Message contains the payload to send or expect.
+	Message string `bson:"message" json:"message"`
+}
+    UptimeTestCommand handles additional checks for tcp connections.
+
+type UptimeTests struct {
+	// ServiceDiscovery contains the configuration related to test Service Discovery.
+	// Tyk classic API definition: `proxy.service_discovery`
+	ServiceDiscovery *ServiceDiscovery `bson:"serviceDiscovery,omitempty" json:"serviceDiscovery,omitempty"`
+
+	// Tests contains individual connectivity tests defined for checking if a service is online.
+	Tests []UptimeTest `bson:"tests,omitempty" json:"tests,omitempty"`
+
+	// HostDownRetestPeriod is the time to wait until rechecking a failed test.
+	// If undefined, the default testing interval (10s) is in use.
+	// Setting this to a lower value would result in quicker recovery on failed checks.
+	HostDownRetestPeriod time.ReadableDuration `bson:"hostDownRetestPeriod" json:"hostDownRetestPeriod"`
+
+	// LogRetentionPeriod holds a time to live for the uptime test results.
+	// If unset, a value of 100 years is the default.
+	LogRetentionPeriod time.ReadableDuration `bson:"logRetentionPeriod" json:"logRetentionPeriod"`
+}
+    UptimeTests configures uptime tests.
+
+func (t *UptimeTests) ExtractTo(uptimeTests *apidef.UptimeTests)
+    ExtractTo extracts *UptimeTests into *apidef.UptimeTests.
+
+func (t *UptimeTests) Fill(uptimeTests apidef.UptimeTests)
+    Fill fills *UptimeTests from apidef.UptimeTests.
+
 type ValidateRequest struct {
 	// Enabled is a boolean flag, if set to `true`, it enables request validation.
 	Enabled bool `bson:"enabled" json:"enabled"`

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Validate ServiceDiscovery before method call Ensure the ServiceDiscovery field in UptimeTests is properly validated or initialized before calling its Fill method to avoid runtime errors. apidef/oas/upstream.go [379] -t.ServiceDiscovery.Fill(uptimeTests.Config.ServiceDiscovery) +if t.ServiceDiscovery != nil { + t.ServiceDiscovery.Fill(uptimeTests.Config.ServiceDiscovery) +} Suggestion importance[1-10]: 8 Why: Adding a nil check for ServiceDiscovery before calling its Fill method is a good practice to avoid runtime errors. This suggestion enhances the robustness of the code.	8
	Validate or initialize RecheckWait field Ensure that the RecheckWait field in UptimeTestsServiceDiscovery is properly initialized or validated to avoid potential nil pointer dereferences or uninitialized values during runtime. apidef/oas/upstream.go [363] -RecheckWait time.ReadableDuration +RecheckWait time.ReadableDuration // Ensure this is initialized or validated before use Suggestion importance[1-10]: 7 Why: Ensuring the RecheckWait field is properly initialized or validated is important to prevent runtime errors. This suggestion is relevant and addresses a potential issue in the PR.	7
General	Add edge case tests for UptimeTests Include additional test cases in TestUptimeTests to cover scenarios where ServiceDiscovery or RecheckWait fields are nil or uninitialized to ensure robustness. apidef/oas/upstream_test.go [158] -assert.Equal(t, emptyTest, resultTest) +assert.Equal(t, emptyTest, resultTest) // Add cases for nil or uninitialized fields Suggestion importance[1-10]: 6 Why: Including additional test cases for edge scenarios improves test coverage and ensures robustness. However, the suggestion is general and lacks specific implementation details, slightly reducing its impact.	6

[sonarqubecloud]

Quality Gate failed

Failed conditions
11 Security Hotspots
E Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE