[TT-12634] Add oas contract and migrations to/from classic apidef

titpetric commented 1 month ago

User description

https://tyktech.atlassian.net/browse/TT-12634

PR Type

Enhancement, Tests

Description

Added RateLimit field to the Operation struct in operation.go.
Implemented methods to handle rate limit configurations: fillRateLimitEndpoints and extractRateLimitEndpointTo.
Integrated rate limit handling in fillPathsAndOperations and extractPathsAndOperations methods.
Added RateLimitEndpoint struct and implemented Fill and ExtractTo methods for it in upstream.go.
Updated X-Tyk-Operation schema definition to include rateLimit field.
Enhanced linter and operation tests to handle rate limit configurations.

Changes walkthrough 📝

Relevant files

Tests

linter_test.go `Add rate limit configuration handling in linter test` apidef/oas/linter_test.go - Added rate limit configuration handling in the `Lint` test.	+3/-0
operation_test.go `Add rate limit configuration handling in operation test` apidef/oas/operation_test.go Added rate limit configuration handling in `PathsAndOperations` test.	+4/-0

Enhancement

operation.go `Add rate limit handling in OAS operations` apidef/oas/operation.go Added `RateLimit` field to `Operation` struct. Implemented `fillRateLimitEndpoints` and `extractRateLimitEndpointTo` methods. Integrated rate limit handling in `fillPathsAndOperations` and `extractPathsAndOperations`.	+30/-0
upstream.go `Add RateLimitEndpoint struct and methods` apidef/oas/upstream.go Added `RateLimitEndpoint` struct. Implemented `Fill` and `ExtractTo` methods for `RateLimitEndpoint`.	+17/-0
x-tyk-api-gateway.json `Add rateLimit field to X-Tyk-Operation schema` apidef/oas/schema/x-tyk-api-gateway.json - Added `rateLimit` field to `X-Tyk-Operation` schema definition.	+3/-0

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 1 month ago

API Changes

--- prev.txt    2024-07-15 14:14:30.365866423 +0000
+++ current.txt 2024-07-15 14:14:27.101873520 +0000
@@ -3853,6 +3853,9 @@

    // RequestSizeLimit limits the maximum allowed size of the request body in bytes.
    RequestSizeLimit *RequestSizeLimit `bson:"requestSizeLimit,omitempty" json:"requestSizeLimit,omitempty"`
+
+   // RateLimit contains endpoint level rate limit configuration.
+   RateLimit *RateLimitEndpoint `bson:"rateLimit,omitempty" json:"rateLimit,omitempty"`
 }
     Operation holds a request operation configuration, allowances,
     tranformations, caching, timeouts and validation.
@@ -4086,7 +4089,7 @@
    // be considered as 0s/empty.
    //
    // Tyk classic API definition: `global_rate_limit.per`.
-   Per time.ReadableDuration `json:"per" bson:"per"`
+   Per ReadableDuration `json:"per" bson:"per"`
 }
     RateLimit holds the configurations related to rate limit. The API-level rate
     limit applies a base-line limit on the frequency of requests to the upstream
@@ -4100,6 +4103,15 @@
 func (r *RateLimit) Fill(api apidef.APIDefinition)
     Fill fills *RateLimit from apidef.APIDefinition.

+type RateLimitEndpoint RateLimit
+    RateLimitEndpoint carries same settings as RateLimit but for endpoints.
+
+func (r *RateLimitEndpoint) ExtractTo(meta *apidef.RateLimitMeta)
+    ExtractTo extracts *Ratelimit into *apidef.RateLimitMeta.
+
+func (r *RateLimitEndpoint) Fill(api apidef.RateLimitMeta)
+    Fill fills *RateLimit from apidef.RateLimitMeta.
+
 type ReadableDuration = time.ReadableDuration
     ReadableDuration is an alias maintained to be used in imports.

@@ -4773,7 +4785,7 @@
    // An empty value is interpreted as "0s", implying no cool-down.
    // It's important to format the string correctly, as invalid formats will
    // be considered as 0s/empty.
-   CoolDownPeriod time.ReadableDuration `json:"cooldownPeriod" bson:"cooldownPeriod"`
+   CoolDownPeriod ReadableDuration `json:"cooldownPeriod" bson:"cooldownPeriod"`
    // BodyTemplate is the template to be used for request payload.
    BodyTemplate string `json:"bodyTemplate,omitempty" bson:"bodyTemplate,omitempty"`
    // Headers are the list of request headers to be used.

github-actions[bot] commented 1 month ago

PR Reviewer Guide 🔍

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review

Rate Limit Logic
Ensure that the logic for setting the rate limit per minute is correctly implemented and tested, especially the use of `time.ReadableDuration(time.Minute)`. Data Conversion
Verify the correctness of data conversion in `RateLimitEndpoint.Fill` and `RateLimitEndpoint.ExtractTo`, particularly the transformation of rate and period values.

github-actions[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for nil returns from `getOperation` ___ Add error handling for `GetTykExtension().getOperation(operationID)` to handle potential nil returns which could lead to runtime panics. [apidef/oas/operation.go [797-799]](https://github.com/TykTechnologies/tyk/pull/6416/files#diff-6d92d2d5b09a5fa7129609bb7cd0d383d015250ec07062b6a93a83257be51fb5R797-R799) ```diff operation := s.GetTykExtension().getOperation(operationID) +if operation == nil { + return // or handle error appropriately +} if operation.RateLimit == nil { operation.RateLimit = &RateLimitEndpoint{} } ``` Suggestion importance[1-10]: 10 Why: Adding error handling for potential nil returns is critical to prevent runtime panics, thus improving the robustness of the code.	10
Possible issue	Avoid overwriting existing rate limit duration if already set ___ Consider checking if `op.RateLimit.Per` is already set before assigning a new value to avoid overwriting existing configurations unintentionally. [apidef/oas/linter_test.go [37]](https://github.com/TykTechnologies/tyk/pull/6416/files#diff-b92239afd81e77a829fe7fe8410044dfd4dfda525d17dbf5f8811714a9c986d3R37-R37) ```diff -if op.RateLimit != nil { +if op.RateLimit != nil && op.RateLimit.Per == 0 { op.RateLimit.Per = time.ReadableDuration(time.Minute) } ``` Suggestion importance[1-10]: 9 Why: This suggestion prevents unintentional overwriting of existing configurations, which is crucial for maintaining the integrity of the rate limit settings.	9
Maintainability	Use a constant for the rate limit duration to improve readability ___ Consider using a constant for `time.Minute` to avoid magic numbers and improve code readability and maintainability. [apidef/oas/operation_test.go [57]](https://github.com/TykTechnologies/tyk/pull/6416/files#diff-cd234db716d6d2edc97c135ef546021c9ab4fa9282d63964bd155d41635cf964R57-R57) ```diff -operation.RateLimit.Per = time.ReadableDuration(time.Minute) +const defaultRateLimitDuration = time.Minute +operation.RateLimit.Per = time.ReadableDuration(defaultRateLimitDuration) ``` Suggestion importance[1-10]: 6 Why: Using a constant improves code readability and maintainability, but it is a minor improvement and not crucial for functionality.	6

sonarcloud[bot] commented 1 month ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

TykTechnologies / tyk