You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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] Granular control over client>gateway HTTP
PR Title
[TT-13657]: Add protocol and port to oas
Check out this guide to learn more about PR best-practices.
--- prev.txt 2025-01-27 10:17:58.088197873 +0000+++ current.txt 2025-01-27 10:17:53.220165159 +0000@@ -3820,6 +3820,14 @@
//
// Tyk classic API definition: `allowed_ips` and `blacklisted_ips`.
IPAccessControl *IPAccessControl `bson:"ipAccessControl,omitempty" json:"ipAccessControl,omitempty"`
+ // Protocol configures the HTTP protocol used by the API.+ // Possible values are:+ // - "http": Standard HTTP/1.1 protocol+ // - "http2": HTTP/2 protocol with TLS+ // - "h2c": HTTP/2 protocol without TLS (cleartext).+ Protocol string `bson:"protocol,omitempty" json:"protocol,omitempty"`+ // Port Setting this value will change the port that Tyk listens on. Default: 8080.+ Port int `bson:"port,omitempty" json:"port,omitempty"`
}
Server contains the configuration that sets Tyk up to receive requests from
the client applications.
The Protocol and Port fields were added to the Server struct. Ensure that these fields are properly validated and handled in all relevant parts of the codebase to avoid potential misconfigurations or runtime errors.
// Protocol configures the HTTP protocol used by the API.// Possible values are:// - "http": Standard HTTP/1.1 protocol// - "http2": HTTP/2 protocol with TLS// - "h2c": HTTP/2 protocol without TLS (cleartext).Protocolstring`bson:"protocol" json:"protocol"`// Port Setting this value will change the port that Tyk listens on. Default: 8080.Portint`bson:"port" json:"port"`
New test cases were added for Protocol and Port in the Server struct. Verify that these tests cover all edge cases and scenarios, including invalid values.
-t.Run("port protocol", func(t *testing.T) {+t.Run("port protocol validation", func(t *testing.T) {
var server = Server{
Port: 3000,
Protocol: "http",
+ }+ var invalidServer = Server{+ Port: -1,+ Protocol: "invalid",
}
var convertedAPI apidef.APIDefinition
var resultServer Server
server.ExtractTo(&convertedAPI)
-
assert.Equal(t, server.Port, convertedAPI.ListenPort)
assert.Equal(t, server.Protocol, convertedAPI.Protocol)
resultServer.Fill(convertedAPI)
assert.Equal(t, server, resultServer)
++ // Add assertions for invalid cases+ assert.Panics(t, func() { invalidServer.ExtractTo(&convertedAPI) })
})
Suggestion importance[1-10]: 7
Why: The suggestion proposes adding test cases to validate the Protocol and Port fields, which is relevant and actionable. The improved code includes meaningful test cases for invalid values, enhancing the robustness of the PR. However, the suggestion could be more impactful if it also addressed the lack of validation logic in the implementation.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
TT-13657
Description
TT-13657
Related Issue
Motivation and Context
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
Checklist
PR Type
Enhancement, Tests
Description
Added
protocolandportfields to theServerstruct.Updated JSON schema to include
protocolandportdefinitions.Enhanced tests to validate
protocolandportfunctionality.Removed unused fields from migration tests.
Changes walkthrough 📝
linter_test.go
Add default `protocol` value in linter testapidef/oas/linter_test.go
protocolfield in test settings.oas_test.go
Remove unused fields from migration testsapidef/oas/oas_test.go
ListenPortandProtocolfrom the list of non-migrated fields.server_test.go
Add tests for `protocol` and `port` in Serverapidef/oas/server_test.go
protocolandportfunctionality.server.go
Add `protocol` and `port` fields to Server structapidef/oas/server.go
protocolandportfields to theServerstruct.FillandExtractTomethods to handleprotocolandport.x-tyk-api-gateway.json
Update JSON schema with `protocol` and `port` definitionsapidef/oas/schema/x-tyk-api-gateway.json
protocolandportdefinitions to the JSON schema.protocolfield.