Skip to content

Merging to release-5.8: Disabled/empty uptime test fields migrated to Tyk OAS (#7069) #7085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 28, 2025
Merged

Merging to release-5.8: Disabled/empty uptime test fields migrated to Tyk OAS (#7069) #7085

merged 2 commits into from
May 28, 2025

Conversation

buger
Copy link
Member

@buger buger commented May 27, 2025 •
edited by github-actions bot
Loading

User description

Disabled/empty uptime test fields migrated to Tyk OAS (#7069)

User description

TT-14183
Summary Disabled/empty uptime test fields migrated to Tyk OAS
Type Bug Bug
Status In Code Review
Points N/A
Labels 5.8.0Regression, 5.8.1Refinement, QA_Fail, codilime_refined

Chnage migration rules

Related Issue

Motivation and Context

How This Has Been Tested

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

PR Type

Bug fix, Tests

Description

  • Fixes migration of disabled/empty uptime test fields to Tyk OAS.

  • Ensures empty uptime test fields are omitted in JSON serialization.

  • Updates service discovery cache logic for disabled state.

  • Adds and enhances tests for uptime test migration and serialization.

Changes walkthrough 📝

Relevant files
Bug fix
upstream.go
Refactor uptime test and cache migration logic                     

apidef/oas/upstream.go

  • Refactored uptime test migration to omit empty fields in JSON.
  • Changed cache logic to set cache to nil if not enabled.
  • Updated uptime test enabling logic to depend on test presence and
    disabled flag.
  • Ensured empty body is omitted from JSON output.
    		• +6/-11   
    Tests
    upstream_test.go
    Add tests for uptime test migration and serialization       

    apidef/oas/upstream_test.go

  • Added tests for empty uptime test migration and JSON serialization.
  • Verified omission of empty body and protocol fields in JSON.
  • Checked that Fill produces empty structure when no tests are present.
    		• +44/-0   
    fixtures_test.go
    Assert migration errors in test fixtures                                 

    apidef/oas/fixtures_test.go

    • Updated migration call to handle error and assert no error occurs.
    		 +2/-1     
    service_discovery.yml
    Update service discovery fixture for disabled state           

    apidef/oas/testdata/fixtures/service_discovery.yml

    • Updated expected output for disabled service discovery to be nil.
    		 +1/-5     
    Need help?
  • Type /help how to

    • ... in the comments thread for any questions about PR-Agent
    usage.

  • Check out the documentation
    for more information.

    • PR Type

    Bug fix, Tests

    Description

    • Fix migration to omit disabled/empty uptime test fields in Tyk OAS.

    • Ensure empty uptime test fields are omitted in JSON serialization.

    • Update service discovery cache logic for disabled state.

    • Add and enhance tests for uptime test migration and serialization.

    Changes walkthrough 📝

    Relevant files
    Bug fix
    upstream.go
    Refactor uptime test and service discovery migration logic

    apidef/oas/upstream.go

  • Omit empty uptime test fields in JSON serialization.
  • Update service discovery cache logic to set cache to nil if not
    enabled.
  • Refactor uptime test enabling logic to depend on test presence and
    disabled flag.
  • Ensure empty body is omitted from JSON output.
    		• +9/-14   
    Tests
    upstream_test.go
    Add tests for uptime test migration and serialization       

    apidef/oas/upstream_test.go

  • Add tests for empty uptime test migration and JSON serialization.
  • Verify omission of empty body and protocol fields in JSON.
  • Check that Fill produces empty structure when no tests are present.
    		• +44/-0   
    fixtures_test.go
    Assert migration errors in test fixtures                                 

    apidef/oas/fixtures_test.go

    • Update migration call to handle error and assert no error occurs.
    		 +2/-1     
    service_discovery.yml
    Update service discovery fixture for disabled state           

    apidef/oas/testdata/fixtures/service_discovery.yml

    • Update expected output for disabled service discovery to be nil.
    		 +1/-5     
    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
    •

    @shults
    Disabled/empty uptime test fields migrated to Tyk OAS (#7069)
    43f405b 
    ### **User description**
<details open>
<summary><a href="https://tyktech.atlassian.net/browse/TT-14183"
title="TT-14183" target="_blank">TT-14183</a></summary>
  <br />
  <table>
    <tr>
      <th>Summary</th>
      <td>Disabled/empty uptime test fields migrated to Tyk OAS</td>
    </tr>
    <tr>
      <th>Type</th>
      <td>
<img alt="Bug"
src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium"
/>
        Bug
      </td>
    </tr>
    <tr>
      <th>Status</th>
      <td>In Code Review</td>
    </tr>
    <tr>
      <th>Points</th>
      <td>N/A</td>
    </tr>
    <tr>
      <th>Labels</th>
<td><a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%205.8.0Regression%20ORDER%20BY%20created%20DESC"
title="5.8.0Regression">5.8.0Regression</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%205.8.1Refinement%20ORDER%20BY%20created%20DESC"
title="5.8.1Refinement">5.8.1Refinement</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20QA_Fail%20ORDER%20BY%20created%20DESC"
title="QA_Fail">QA_Fail</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC"
title="codilime_refined">codilime_refined</a></td>
    </tr>
  </table>
</details>
<!--
  do not remove this marker as it will break jira-lint's functionality.
  added_by_jira_lint
-->

---

<!-- Provide a general summary of your changes in the Title above -->
Chnage migration rules

<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [x] 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)
- [x] 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


___

### **PR Type**
Bug fix, Tests


___

### **Description**
- Fixes migration of disabled/empty uptime test fields to Tyk OAS.

- Ensures empty uptime test fields are omitted in JSON serialization.

- Updates service discovery cache logic for disabled state.

- Adds and enhances tests for uptime test migration and serialization.


___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Bug
fix</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>upstream.go</strong><dd><code>Refactor uptime test and
cache migration logic</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

apidef/oas/upstream.go

<li>Refactored uptime test migration to omit empty fields in JSON.<br>
<li> Changed cache logic to set cache to nil if not enabled.<br> <li>
Updated uptime test enabling logic to depend on test presence and
<br>disabled flag.<br> <li> Ensured empty body is omitted from JSON
output.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/7069/files#diff-7b0941c7f37fe5a2a23047e0822a65519ca11c371660f36555b59a60f000e3f4">+6/-11</a>&nbsp;
&nbsp; </td>

</tr>
</table></td></tr><tr><td><strong>Tests</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>upstream_test.go</strong><dd><code>Add tests for uptime
test migration and serialization</code>&nbsp; &nbsp; &nbsp; &nbsp;
</dd></summary>
<hr>

apidef/oas/upstream_test.go

<li>Added tests for empty uptime test migration and JSON
serialization.<br> <li> Verified omission of empty body and protocol
fields in JSON.<br> <li> Checked that Fill produces empty structure when
no tests are present.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/7069/files#diff-222cc254c0c6c09fa0cf50087860b837a0873e2aef3c84ec7d80b1014c149057">+44/-0</a>&nbsp;
&nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>fixtures_test.go</strong><dd><code>Assert migration
errors in test fixtures</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; </dd></summary>
<hr>

apidef/oas/fixtures_test.go

- Updated migration call to handle error and assert no error occurs.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/7069/files#diff-2c27bf7208b7414ba0b07325bc834177a424d5348f2ffb4a78d46f428f8e303f">+2/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>service_discovery.yml</strong><dd><code>Update service
discovery fixture for disabled state</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; </dd></summary>
<hr>

apidef/oas/testdata/fixtures/service_discovery.yml

- Updated expected output for disabled service discovery to be nil.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/7069/files#diff-8a41102394fc3160d93fc56f1bcec9d1a03404724ba9e10dd624faf8e1154f65">+1/-5</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr></tr></tbody></table>

___

> <details> <summary> Need help?</summary><li>Type <code>/help how to
...</code> in the comments thread for any questions about PR-Agent
usage.</li><li>Check out the <a
href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a>
for more information.</li></details>

(cherry picked from commit 6495337)
    @buger buger enabled auto-merge (squash) May 27, 2025 15:04
    @github-actions GitHub Actions
    Copy link
    Contributor

    github-actions bot commented May 27, 2025 •
    edited
    Loading

    API Changes

    --- prev.txt	2025-05-28 11:47:07.385699432 +0000
+++ current.txt	2025-05-28 11:47:02.580697071 +0000
@@ -4899,7 +4899,7 @@
 
 	// Timeout declares a timeout for the request. If the test exceeds
 	// this timeout, the check fails.
-	Timeout time.ReadableDuration `bson:"timeout" json:"timeout"`
+	Timeout time.ReadableDuration `bson:"timeout" json:"timeout,omitempty"`
 
 	// Method allows you to customize the HTTP method for the test (`GET`, `POST`,...).
 	Method string `bson:"method" json:"method"`
@@ -4908,7 +4908,7 @@
 	Headers map[string]string `bson:"headers" json:"headers,omitempty"`
 
 	// Body is the body of the test request.
-	Body string `bson:"body" json:"body"`
+	Body string `bson:"body" json:"body,omitempty"`
 
 	// Commands are used for TCP checks.
 	Commands []UptimeTestCommand `bson:"commands" json:"commands,omitempty"`
@@ -4947,11 +4947,11 @@
 	// 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"`
+	HostDownRetestPeriod time.ReadableDuration `bson:"hostDownRetestPeriod" json:"hostDownRetestPeriod,omitempty"`
 
 	// 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"`
+	LogRetentionPeriod time.ReadableDuration `bson:"logRetentionPeriod" json:"logRetentionPeriod,omitempty"`
 }
     UptimeTests configures uptime tests.

    @github-actions GitHub Actions
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    7069 - Fully compliant

    Compliant requirements:

    • Fix migration of disabled/empty uptime test fields to Tyk OAS.
    • Ensure empty uptime test fields are omitted in JSON serialization.
    • Update service discovery cache logic for disabled state.
    • Add and enhance tests for uptime test migration and serialization.
    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    JSON Serialization Omissions

    The changes introduce omitempty for several fields (e.g., Body, Timeout, HostDownRetestPeriod, LogRetentionPeriod) in uptime test structs. Ensure that omitting these fields in JSON does not break any downstream consumers or integrations that expect their presence, even if empty.

     
    	HostDownRetestPeriod time.ReadableDuration `bson:"hostDownRetestPeriod" json:"hostDownRetestPeriod,omitempty"`

	// 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,omitempty"`
}

// UptimeTest configures an uptime test check.
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`
	// - `tcp://127.0.0.1:6379` (for TCP checks).
	CheckURL string `bson:"url" json:"url"`

	// Timeout declares a timeout for the request. If the test exceeds
	// this timeout, the check fails.
	Timeout time.ReadableDuration `bson:"timeout" json:"timeout,omitempty"`

	// 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,omitempty"`

	// Commands are used for TCP checks.
	Commands []UptimeTestCommand `bson:"commands" json:"commands,omitempty"`

	// EnableProxyProtocol enables proxy protocol support when making request.
    Uptime Test Enablement Logic

    The logic for setting u.Enabled in UptimeTests.Fill now depends on the presence of tests and the Disabled flag. Confirm that this change does not introduce regressions in scenarios where uptime tests are dynamically enabled/disabled.

     
    	u.Tests = nil
	for _, v := range uptimeTests.CheckList {
		check := UptimeTest{
			CheckURL:            u.fillCheckURL(v.Protocol, v.CheckURL),
			Timeout:             ReadableDuration(v.Timeout),
			Method:              v.Method,
			Headers:             v.Headers,
			Body:                v.Body,
			EnableProxyProtocol: v.EnableProxyProtocol,
		}
		for _, command := range v.Commands {
			check.AddCommand(command.Name, command.Message)
		}
		u.Tests = append(u.Tests, check)
	}

	u.Enabled = len(u.Tests) > 0 && !uptimeTests.Disabled
}

    @github-actions GitHub Actions
    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @sonarqubecloud SonarQubeCloud
    Copy link

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    100.0% Coverage on New Code
    0.0% Duplication on New Code

    See analysis details on SonarQube Cloud

    @buger buger merged commit 6c67e48 into release-5.8 May 28, 2025
    37 of 38 checks passed
    @buger buger deleted the merge/release-5.8/64953379ed0fa8e1c82fb595a4e4212e527cbf20 branch May 28, 2025 12:10
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers
    No reviews
    Assignees
    No one assigned
    Labels
    Review effort 3/5
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @buger @shults