Merging to release-5.3: [TT-12865] Rename config parameter, update usage, support mux params on legacy (#6506)

[buger]

User description

TT-12865 Rename config parameter, update usage, support mux params on legacy (#6506)

PR Type

enhancement

---

Description

• Renamed configuration fields EnablePrefixMatching and
  EnableSuffixMatching to EnablePathPrefixMatching and
  EnablePathSuffixMatching respectively, across multiple files.
• Updated comments and documentation to reflect the new naming
  conventions.
• Modified test cases to use the updated configuration fields.
• Refactored middleware and session manager logic to accommodate the new
  path matching configuration.

---

Changes walkthrough 📝

	Relevant files
Enhancement	config.go Rename configuration fields for path matching options        config/config.go Renamed EnablePrefixMatching to EnablePathPrefixMatching. Renamed EnableSuffixMatching to EnablePathSuffixMatching. Updated comments to reflect the new naming conventions. +12/-12  api_definition.go Update API definition to use new path matching config        gateway/api_definition.go Updated variable names to use new path matching configuration fields. +2/-2      mw_granular_access.go Refactor middleware to use updated path matching config    gateway/mw_granular_access.go Updated variable names to use new path matching configuration fields. Refactored pattern preparation using PreparePathRegexp. +3/-9      session_manager.go Update session manager for new path matching config            gateway/session_manager.go Updated variable names to use new path matching configuration fields. +2/-2      schema.json Update JSON schema for path matching configuration              cli/linter/schema.json Renamed JSON schema fields for path matching options. +2/-2
Tests	api_definition_test.go Update test configuration for path prefix matching              gateway/api_definition_test.go Modified test configuration to use EnablePathPrefixMatching. +1/-1      mw_granular_access_test.go Update middleware test for path prefix matching                    gateway/mw_granular_access_test.go Modified test configuration to use EnablePathPrefixMatching. +1/-1

---

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

---

Co-authored-by: Tit Petric [email protected]

---

PR Type

Enhancement, Tests

---

Description

• Introduced new configuration options EnablePathPrefixMatching and EnablePathSuffixMatching to enhance URL path matching capabilities.
• Refactored URLSpec and related logic to support the new path matching configuration.
• Updated middleware and session management to utilize the new path matching options.
• Added comprehensive tests for the new path matching logic, including regression tests for issue 12865.
• Improved logging and error handling in path matching processes.

---

Changes walkthrough 📝

	Relevant files
Enhancement	6 files config.go Add configuration for path prefix and suffix matching        config/config.go Added new configuration fields EnablePathPrefixMatching and EnablePathSuffixMatching. Updated comments to explain the new path matching configuration. +43/-0    api_definition.go Refactor URLSpec and update path matching logic                    gateway/api_definition.go Refactored URLSpec to use private fields and methods. Updated path matching logic to support new configuration. Renamed several functions for clarity. +47/-142 model_apispec.go Refactor and deprecate CheckSpecMatchesStatus                        gateway/model_apispec.go Moved CheckSpecMatchesStatus to a separate file and deprecated it. Added FindSpecMatchesStatus for improved status checking. +75/-0    model_urlspec.go Add path and method matching methods to URLSpec                    gateway/model_urlspec.go Added new methods to URLSpec for matching paths and methods. Deprecated modeSpecificSpec method. +120/-0  mw_granular_access.go Update GranularAccessMiddleware for new path matching        gateway/mw_granular_access.go Updated middleware to use new path matching configuration. Improved logging and error handling in path matching. +74/-20  mux.go Refactor path regex preparation and matching                          internal/httputil/mux.go Refactored path regex preparation and matching functions. Improved caching and handling of mux-style parameters. +78/-18
Tests	4 files api_definition_test.go Update and add test cases for path matching logic                gateway/api_definition_test.go Updated test cases to reflect changes in path matching logic. Added new test cases for path prefix matching. +12/-12  mw_granular_access_test.go Add tests for GranularAccessMiddleware path matching          gateway/mw_granular_access_test.go Added tests for new path matching logic in GranularAccessMiddleware. Updated existing tests to use new configuration. +50/-28  mux_test.go Add tests for path regex preparation and matching                internal/httputil/mux_test.go Added tests for new path matching functions. Updated existing tests to reflect changes in regex preparation. +112/-14 issue_12865_test.go Add regression tests for issue 12865                                          tests/regression/issue_12865_test.go Added regression tests for issue 12865. Tested various configurations of path prefix and suffix matching. +171/-0
Configuration changes	2 files test.yml Update test task configuration                                                      .taskfiles/test.yml Updated test task to exclude the first package in the list. Added a task to merge coverage reports. +2/-1      schema.json Update schema for new path matching configuration                cli/linter/schema.json Added new schema fields for path prefix and suffix matching. +6/-0

---

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

[github-actions]

API Changes

--- prev.txt	2024-09-12 17:32:03.415738583 +0000
+++ current.txt	2024-09-12 17:31:59.983726905 +0000
@@ -5514,6 +5514,49 @@
 	// Regular expressions and parameterized routes will be left alone regardless of this setting.
 	EnableStrictRoutes bool `json:"enable_strict_routes"`
 
+	// EnablePathPrefixMatching changes the URL matching from wildcard mode to prefix mode.
+	// For example, `/json` matches `*/json*` by current default behaviour.
+	// If prefix matching is enabled, the match will be performed as a prefix match (`/json*`).
+	//
+	// The `/json` url would be matched as `^/json` against the following paths:
+	//
+	// - Full listen path and versioning URL (`/listen-path/v4/json`)
+	// - Stripped listen path URL (`/v4/json`)
+	// - Stripped version information (`/json`) - match.
+	//
+	// If versioning is disabled then the following URLs are considered:
+	//
+	// - Full listen path and endpoint (`/listen-path/json`)
+	// - Stripped listen path (`/json`) - match.
+	//
+	// For inputs that start with `/`, a prefix match is ensured by
+	// prepending the start of string `^` caret.
+	//
+	// For all other cases, the pattern remains unmodified.
+	//
+	// Combine this option with `enable_path_suffix_matching` to achieve
+	// exact url matching with `/json` being evaluated as `^/json$`.
+	EnablePathPrefixMatching bool `json:"enable_path_prefix_matching"`
+
+	// EnablePathSuffixMatching changes the URL matching to match as a suffix.
+	// For example: `/json` is matched as `/json$` against the following paths:
+	//
+	// - Full listen path and versioning URL (`/listen-path/v4/json`)
+	// - Stripped listen path URL (`/v4/json`)
+	// - Stripped version information (`/json`) - match.
+	//
+	// If versioning is disabled then the following URLs are considered:
+	//
+	// - Full listen path and endpoint (`/listen-path/json`)
+	// - Stripped listen path (`/json`) - match.
+	//
+	// If the input pattern already ends with a `$` (`/json$`)
+	// then the pattern remains unmodified.
+	//
+	// Combine this option with `enable_path_prefix_matching` to achieve
+	// exact url matching with `/json` being evaluated as `^/json$`.
+	EnablePathSuffixMatching bool `json:"enable_path_suffix_matching"`
+
 	// Disable TLS verification. Required if you are using self-signed certificates.
 	SSLInsecureSkipVerify bool `json:"ssl_insecure_skip_verify"`
 
@@ -7424,10 +7467,16 @@
 func CloneAPI(a *APISpec) *APISpec
 
 func (a *APISpec) CheckSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mode URLStatus) (bool, interface{})
-    CheckSpecMatchesStatus checks if a url spec has a specific status
+    CheckSpecMatchesStatus checks if a URL spec has a specific status.
+    Deprecated: The function doesn't follow go return conventions (T, ok);
+    use FindSpecMatchesStatus;
 
 func (a *APISpec) Expired() bool
 
+func (a *APISpec) FindSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mode URLStatus) (*URLSpec, bool)
+    FindSpecMatchesStatus checks if a URL spec has a specific status and returns
+    the URLSpec for it.
+
 func (s *APISpec) FireEvent(name apidef.TykEvent, meta interface{})
 
 func (a *APISpec) GetSessionLifetimeRespectsKeyExpiration() bool
@@ -7450,6 +7499,12 @@
 func (a *APISpec) StopSessionManagerPool()
 
 func (a *APISpec) StripListenPath(reqPath string) string
+    StripListenPath will strip the listen path from the URL, keeping version in
+    tact.
+
+func (a *APISpec) StripVersionPath(reqPath string) string
+    StripVersionPath will strip the version from the URL. The input URL should
+    already have listen path stripped.
 
 func (a *APISpec) URLAllowedAndIgnored(r *http.Request, rxPaths []URLSpec, whiteListStatus bool) (RequestStatus, interface{})
     URLAllowedAndIgnored checks if a url is allowed and ignored.
@@ -10074,7 +10129,6 @@
     system, return an error to have the chain fail
 
 type URLSpec struct {
-	Spec                      *regexp.Regexp
 	Status                    URLStatus
 	MethodActions             map[string]apidef.EndpointMethodMeta
 	Whitelist                 apidef.EndPointMeta
@@ -10102,6 +10156,7 @@
 	PersistGraphQL            apidef.PersistGraphQLMeta
 
 	IgnoreCase bool
+	// Has unexported fields.
 }
     URLSpec represents a flattened specification for URLs, used to check if
     a proxy URL path is on any of the white, black or ignored lists. This is

[github-actions]

PR Reviewer Guide 🔍

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Performance Concern The implementation of GranularAccessMiddleware.ProcessRequest may lead to performance issues due to the repeated compilation of regex patterns within a loop (lines 100-105). Consider caching compiled regex patterns to avoid recompilation on every request. Error Handling The function MatchPaths does not properly handle or log errors when regex matching fails. This could lead to missed debugging information or unhandled exceptions. Consider improving error handling and logging in this function.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Handle errors from regex compilation explicitly to avoid security risks Ensure that the error from regexp.Compile is not just logged but also handled appropriately to prevent potential security issues or incorrect behavior if the regex is not compiled correctly. gateway/mw_granular_access.go [104-109] asRegex, err := regexp.Compile(pattern) if err != nil { logger.WithError(err).Error("error compiling regex") - continue + return errors.New("regex compilation failed"), http.StatusInternalServerError } Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential security issue by ensuring that errors from regex compilation are handled explicitly, preventing incorrect behavior or vulnerabilities. It is a significant improvement for robustness.	9
Performance	Precompile regex patterns during initialization to enhance performance Use a more efficient approach for matching URL paths by compiling regex patterns once during initialization instead of recompiling them for every request, which can significantly improve performance. gateway/mw_granular_access.go [100-111] -pattern := httputil.PreparePathRegexp(accessSpec.URL, isPrefixMatch, isSuffixMatch) -asRegex, err := regexp.Compile(pattern) +// Assume regex patterns are compiled and stored in accessSpec during initialization +match := accessSpec.CompiledRegex.MatchString(urlPath) Suggestion importance[1-10]: 8 Why: Precompiling regex patterns can significantly improve performance by avoiding repeated compilation during each request. This is a valuable optimization, especially for high-traffic applications.	8
Maintainability	Replace slices.Contains with a custom method existence check function Replace the use of slices.Contains with a custom function to check for method existence in accessSpec.Methods. This avoids importing the slices package which is not necessary for such a simple operation, improving the maintainability and reducing external dependencies. gateway/mw_granular_access.go [56-58] -if !slices.Contains(accessSpec.Methods, r.Method) { +if !methodExists(accessSpec.Methods, r.Method) { continue } Suggestion importance[1-10]: 7 Why: This suggestion improves maintainability by reducing external dependencies, which is beneficial for a simple operation like checking method existence. However, the performance gain is minimal, so it is not crucial.	7
	Refactor logging logic into a separate function to enhance readability and reduce redundancy Refactor the logging logic to a separate function to reduce redundancy and improve code readability. This function can handle both debug and error logging based on parameters. gateway/mw_granular_access.go [69-77] -if gwConfig.LogLevel == "debug" || err != nil { - logger = logger.WithError(err).WithField("pattern", pattern).WithField("match", match) - if err != nil { - logger.Error("error matching endpoint") - } else { - logger.Debug("matching endpoint") - } -} +logEndpointMatching(gwConfig.LogLevel, logger, err, pattern, match) Suggestion importance[1-10]: 6 Why: Refactoring the logging logic into a separate function improves code readability and maintainability by reducing redundancy. This is a minor improvement and not critical to the functionality.	6
Enhancement	Optimize conditional logic for pattern manipulation Refactor the PreparePathRegexp function to avoid redundant checks and concatenations by structuring the conditional logic more efficiently. internal/httputil/mux.go [48-57] -if prefix && strings.HasPrefix(pattern, "/") { - pattern = "^" + pattern +if prefix { + pattern = "^" + strings.TrimLeft(pattern, "^") } -if suffix && !strings.HasSuffix(pattern, "$") { - pattern = pattern + "$" +if suffix { + pattern = strings.TrimRight(pattern, "$") + "$" } Suggestion importance[1-10]: 7 Why: The suggestion improves the code by reducing redundancy and making the logic for adding prefix and suffix more efficient. This enhances code readability and maintainability without altering functionality. However, it is not a critical change, hence a moderate score.	7

[sonarqubecloud]

Quality Gate failed

Failed conditions
74.1% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

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

[titpetric]

/release to release-5.3.5

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@titpetric Seems like there is conflict and it require manual merge.