[TT-11909]: fix oas bug and add respect expiry

[kofoworola]

User description

TT-11909

Summary	[OAS] Session lifetime
Type	Story
Status	In Dev
Points	N/A
Labels	QA_Fail

---

Description

TT-11909

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, Enhancement

---

Description

• Added RespectExpiry field to KeyRetentionPeriod for session lifetime handling.

• Updated logic to respect key expiration in Fill and ExtractTo methods.

• Introduced unit tests for KeyRetentionPeriod to validate new functionality.

• Enhanced OpenAPI schema with respectExpiry property for keyRetentionPeriod.

---

Changes walkthrough 📝

	Relevant files
Enhancement	authentication.go Add `RespectExpiry` field and update logic                              apidef/oas/authentication.go Added RespectExpiry field to KeyRetentionPeriod struct. Updated Fill and ExtractTo methods to handle RespectExpiry. Adjusted Value logic to align with SessionLifetime. +6/-1      x-tyk-api-gateway.json Update OpenAPI schema with `respectExpiry`                              apidef/oas/schema/x-tyk-api-gateway.json Added respectExpiry property to keyRetentionPeriod schema. Updated OpenAPI schema to reflect new functionality. +3/-0
Tests	authentication_test.go Add tests for `KeyRetentionPeriod` and `RespectExpiry`      apidef/oas/authentication_test.go Added unit tests for KeyRetentionPeriod functionality. Validated RespectExpiry behavior in ExtractTo and Fill methods. +34/-0    oas_test.go Remove redundant test case                                                              apidef/oas/oas_test.go Removed redundant test case for SessionLifetimeRespectsKeyExpiration. +0/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[buger]

Knock Knock! 🔍

Just thought I'd let you know that your PR title and story title look quite different. PR titles that closely resemble the story title make it easier for reviewers to understand the context of the PR.

An easy-to-understand PR title a day makes the reviewer review away! 😛⚡️

Story Title	[OAS] Session lifetime
PR Title	[TT-11909]: fix oas bug and add respect expiry

Check out this guide to learn more about PR best-practices.

[github-actions]

API Changes

--- prev.txt	2025-01-23 08:27:06.743705236 +0000
+++ current.txt	2025-01-23 08:27:01.962678002 +0000
@@ -3083,6 +3083,9 @@
 	//
 	// Tyk classic API definition: `expire_analytics_after`.
 	Value ReadableDuration `bson:"value" json:"value"`
+	// RespectExpiry ensures that Tyk respects the expiry configured in the key when the API level configuration grants a shorter lifetime.
+	// That is, Redis waits until the the key has expired before deleting it.
+	RespectExpiry bool `bson:"respectExpiry,omitempty" json:"respectExpiry,omitempty"`
 }
     KeyRetentionPeriod contains configuration for key retention.
 
@@ -4904,33 +4907,6 @@
 
 TYPES
 
-type AccessLogsConfig struct {
-	// Enabled controls enabling the transaction logs. Default: false.
-	Enabled bool `json:"enabled"`
-
-	// Template configures which fields to log into the access log.
-	// If unconfigured, all fields listed will be logged.
-	//
-	// Example: ["client_ip", "path"].
-	//
-	// Template Options:
-	//
-	// - `api_key` will include they obfuscated or hashed key.
-	// - `client_ip` will include the ip of the request.
-	// - `host` will include the host of the request.
-	// - `method` will include the request method.
-	// - `path` will include the path of the request.
-	// - `protocol` will include the protocol of the request.
-	// - `remote_addr` will include the remote address of the request.
-	// - `upstream_address` will include the upstream address (scheme, host and path)
-	// - `upstream_latency` will include the upstream latency of the request.
-	// - `latency_total` will include the total latency of the request.
-	// - `user_agent` will include the user agent of the request.
-	// - `status` will include the response status code.
-	Template []string `json:"template"`
-}
-    AccessLogsConfig defines the type of transactions logs printed to stdout.
-
 type AnalyticsConfigConfig struct {
 	// Set empty for a Self-Managed installation or `rpc` for multi-cloud.
 	Type string `json:"type"`
@@ -5383,10 +5359,6 @@
 	// If not set or left empty, it will default to `standard`.
 	LogFormat string `json:"log_format"`
 
-	// AccessLogs configures the output for access logs.
-	// If not configured, the access log is disabled.
-	AccessLogs AccessLogsConfig `json:"access_logs"`
-
 	// Section for configuring OpenTracing support
 	// Deprecated: use OpenTelemetry instead.
 	Tracer Tracer `json:"tracing"`
@@ -8243,10 +8215,6 @@
 
 func (t *BaseMiddleware) OrgSessionExpiry(orgid string) int64
 
-func (t *BaseMiddleware) RecordAccessLog(req *http.Request, resp *http.Response, latency analytics.Latency)
-    RecordAccessLog is used for Success/Error handler logging. It emits a log
-    entry with populated access log fields.
-
 func (t *BaseMiddleware) SetName(name string)
 
 func (t *BaseMiddleware) SetOrgExpiry(orgid string, expiry int64)
@@ -12642,8 +12610,6 @@
     manipulate
 
 func MyPluginReturningError(rw http.ResponseWriter, r *http.Request)
-# Package: ./tests/accesslog
-
 # Package: ./tests/coprocess
 
 package coprocess // import "github.com/TykTechnologies/tyk/tests/coprocess"

[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 new RespectExpiry field and its logic in Fill and ExtractTo methods should be carefully reviewed to ensure it correctly handles the session lifetime and key expiration scenarios without introducing unexpected behavior. // RespectExpiry respects the key expiration time when the session lifetime is less than the key expiration. //That is, Redis waits the key expiration for physical removal. RespectExpiry bool `bson:"respectExpiry,omitempty" json:"respectExpiry,omitempty"` } // Fill fills *KeyRetentionPeriod from apidef.APIDefinition. func (k *KeyRetentionPeriod) Fill(api apidef.APIDefinition) { k.Enabled = !api.SessionLifetimeDisabled k.Value = ReadableDuration(time.Duration(api.SessionLifetime) * time.Second) k.RespectExpiry = api.SessionLifetimeRespectsKeyExpiration } // ExtractTo extracts *Authentication into *apidef.APIDefinition. func (k *KeyRetentionPeriod) ExtractTo(api *apidef.APIDefinition) { api.SessionLifetimeDisabled = !k.Enabled api.SessionLifetime = int64(k.Value.Seconds()) api.SessionLifetimeRespectsKeyExpiration = k.RespectExpiry Test Coverage The new TestKeyRetentionPeriod function should be validated to ensure it provides adequate coverage for the RespectExpiry field and its integration with Fill and ExtractTo methods. func TestKeyRetentionPeriod(t *testing.T) { t.Run("empty", func(t *testing.T) { var emptyKeyRetentionPeriod KeyRetentionPeriod var convertedAPI apidef.APIDefinition var resultKeyRetentionPeriod KeyRetentionPeriod convertedAPI.SetDisabledFlags() emptyKeyRetentionPeriod.ExtractTo(&convertedAPI) resultKeyRetentionPeriod.Fill(convertedAPI) assert.Equal(t, emptyKeyRetentionPeriod, resultKeyRetentionPeriod) }) t.Run("filled", func(t *testing.T) { var keyRetentionPeriod = KeyRetentionPeriod{ Enabled: true, Value: ReadableDuration(5 * time.Minute), RespectExpiry: true, } var convertedAPI apidef.APIDefinition var resultKeyRetentionPeriod KeyRetentionPeriod keyRetentionPeriod.ExtractTo(&convertedAPI) assert.Equal(t, int64(300), convertedAPI.SessionLifetime) assert.True(t, convertedAPI.SessionLifetimeRespectsKeyExpiration) resultKeyRetentionPeriod.Fill(convertedAPI) assert.Equal(t, keyRetentionPeriod, resultKeyRetentionPeriod) }) }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
General	Add test for RespectExpiry false case Add a test case to verify the behavior when RespectExpiry is false to ensure all scenarios are covered. apidef/oas/authentication_test.go [147] -RespectExpiry: true, +RespectExpiry: false, // Add test case for false scenario Suggestion importance[1-10]: 8 Why: Adding a test case for the RespectExpiry field when it is set to false would enhance test coverage and ensure the feature behaves as expected in all scenarios. This suggestion is relevant and actionable.	8
	Clarify respectExpiry field requirement Include the respectExpiry field in the required array if it is mandatory for the schema, or document its optional nature clearly. apidef/oas/schema/x-tyk-api-gateway.json [1217-1218] "respectExpiry": { "type": "boolean" -} +}, +"required": [ + "enabled", + "respectExpiry" +] Suggestion importance[1-10]: 7 Why: Including the respectExpiry field in the required array or documenting its optional nature would improve schema clarity. However, the suggestion assumes the field is mandatory without verifying its necessity, slightly reducing its impact.	7

[andyo-tyk]

Suggested improvements to the GoDocs

[sonarqubecloud]

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
83.9% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarQube Cloud

[andyo-tyk]

Happy to approve the change made to the comment, but I can't review the code changes.