Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
API Changes --- prev.txt 2025-11-12 14:29:17.975838698 +0000
+++ current.txt 2025-11-12 14:29:07.912835015 +0000
@@ -6988,6 +6988,8 @@
// If you set this value to `true`, then the id parameter in a stored policy (or imported policy using the Dashboard API), will be used instead of the internal ID.
//
// This option should only be used when moving an installation to a new database.
+ //
+ // Deprecated. Is not used in codebase.
AllowExplicitPolicyID bool `json:"allow_explicit_policy_id"`
// This option only applies in OSS deployment when the `policies.policy_source` is either set
// to `file` or an empty string. If `policies.policy_path` is set, then Tyk will load policies
@@ -10130,11 +10132,11 @@
func (gw *Gateway) LoadDefinitionsFromRPCBackup() ([]*APISpec, error)
-func (gw *Gateway) LoadPoliciesFromDashboard(endpoint, secret string, allowExplicit bool) (map[string]user.Policy, error)
+func (gw *Gateway) LoadPoliciesFromDashboard(endpoint, secret string) (map[string]user.Policy, error)
LoadPoliciesFromDashboard will connect and download Policies from a Tyk
Dashboard instance.
-func (gw *Gateway) LoadPoliciesFromRPC(store RPCDataLoader, orgId string, allowExplicit bool) (map[string]user.Policy, error)
+func (gw *Gateway) LoadPoliciesFromRPC(store RPCDataLoader, orgId string) (map[string]user.Policy, error)
func (gw *Gateway) LoadPoliciesFromRPCBackup() (map[string]user.Policy, error)
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
🔍 Code Analysis ResultsThis PR addresses inconsistencies in policy identifier handling by standardizing on A significant security enhancement is the introduction of the Files Changed Analysis
Architecture & Impact Assessment
graph TD
subgraph "Configuration"
A[tyk.conf: allow_explicit_policy_id] -- "Marked as Deprecated" --> B(No longer used in code)
end
subgraph "Policy Loading & Identification"
C[Policy Loaders] -- "Signature simplified (no 'allowExplicit')" --> D{LoadPoliciesFromDashboard}
C -- "Signature simplified (no 'allowExplicit')" --> E{LoadPoliciesFromRPC}
F[ensurePolicyId helper] -- "If policy.ID is empty" --> G[policy.ID = policy.MID.Hex()]
D -- "Uses" --> F
E -- "Uses" --> F
end
subgraph "API Security Enhancement"
H[Gateway Policy API<br>/tyk/policies] -- "File operations" --> I[New osutil package]
I -- "Scopes FS access<br>Prevents Path Traversal" --> J[Policy JSON files]
end
D & E -- "Load policies into map" --> K[Policy Map (keyed by policy.ID)]
Scope Discovery & Context ExpansionThe introduction of the Other parts of the gateway's management API that perform file I/O could also benefit from this enhancement. Specifically, the API Definition handlers in Recommendation: To ensure consistent security across the gateway, the API Definition handlers should be refactored to use the new Metadata
Powered by Visor from Probelabs Last updated: 2025-11-12T14:32:34.531Z | Triggered by: synchronize | Commit: 3d1d1b0 💡 TIP: You can chat with Visor using |
🔍 Code Analysis ResultsSecurity Issues (1)
Architecture Issues (1)
Performance Issues (1)
Quality Issues (1)
✅ Dependency Check PassedNo dependency issues found – changes LGTM. ✅ Connectivity Check PassedNo connectivity issues found – changes LGTM. Powered by Visor from Probelabs Last updated: 2025-11-12T14:32:35.693Z | Triggered by: synchronize | Commit: 3d1d1b0 💡 TIP: You can chat with Visor using |
6eef7e5 to
5fc2a48
Compare
acb0367 to
efc831e
Compare
56c8e93 to
311fddc
Compare
e7ee806 to
a0b1126
Compare
…se-of-policy-identifiers
…se-of-policy-identifiers
🎯 Recommended Merge TargetsBased on JIRA ticket TT-15825: [BE] Address inconsistencies with use of Policy identifiers Fix Version: Tyk 5.11.0
Required:
Fix Version: Tyk 5.8.9Required:
📋 Workflow
|
fa5b341 to
17c8a1d
Compare
…se-of-policy-identifiers
|
|
/release to release-5.8 |
|
/release to release-5.11 |
|
❌ Cherry-pick failed. Please check the workflow logs. |
|
|
#7424) <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-15825" title="TT-15825" target="_blank">TT-15825</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[BE] Address inconsistencies with use of Policy identifiers</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 Dev</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%202025_long_tail%20ORDER%20BY%20created%20DESC" title="2025_long_tail">2025_long_tail</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20AI-Complexity-Medium%20ORDER%20BY%20created%20DESC" title="AI-Complexity-Medium">AI-Complexity-Medium</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20AI-Priority-High%20ORDER%20BY%20created%20DESC" title="AI-Priority-High">AI-Priority-High</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>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20customer_bug%20ORDER%20BY%20created%20DESC" title="customer_bug">customer_bug</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC" title="jira_escalated">jira_escalated</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20stability_theme_auth%20ORDER%20BY%20created%20DESC" title="stability_theme_auth">stability_theme_auth</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 --> <!-- Describe your changes in detail --> <!-- 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. --> <!-- Why is this change required? What problem does it solve? --> <!-- 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. --> <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] 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) <!-- 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 ___ Bug fix, Tests, Documentation ___ - Remove explicit policy ID toggle usage - Standardize policy ID to `policy.ID` - Update RPC/Dashboard loaders signatures - Adjust tests for new behavior ___ ```mermaid flowchart LR Config["Config: deprecate allow_explicit_policy_id"] -- "not used" --> Loaders["Policy loaders"] Loaders["Policy loaders"] -- "use policy.ID only" --> Dashboard["LoadPoliciesFromDashboard"] Loaders -- "use policy.ID only" --> RPC["LoadPoliciesFromRPC/parsePoliciesFromRPC"] API["handleAddOrUpdatePolicy"] -- "backfill ID from MID" --> Filesave["Save policy file"] Tests["Tests updated"] -- "drop allowExplicit param" --> Dashboard Tests -- "expect IDs via policy.ID" --> RPC Backup["RPC backup handlers"] -- "parsePoliciesFromRPC()" --> RPC ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Documentation</strong></td><td><table> <tr> <td> <details> <summary><strong>config.go</strong><dd><code>Deprecate unused AllowExplicitPolicyID config</code> </dd></summary> <hr> config/config.go <ul><li>Mark <code>AllowExplicitPolicyID</code> as deprecated.<br> <li> Note it's unused in codebase.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7424/files#diff-fe44f09c4d5977b5f5eaea29170b6a0748819c9d02271746a20d81a5f3efca17">+2/-0</a> </td> </tr> </table></td></tr><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>api.go</strong><dd><code>Backfill missing policy ID from MID on save</code> </dd></summary> <hr> gateway/api.go <ul><li>Default <code>newPol.ID</code> to <code>newPol.MID.Hex()</code> if empty.<br> <li> Ensure saved filename uses resolved policy ID.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7424/files#diff-644cda3aeb4ac7f325359e85fcddb810f100dd5e6fa480b0d9f9363a743c4e05">+4/-0</a> </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>policy.go</strong><dd><code>Standardize ID handling and simplify loader APIs</code> </dd></summary> <hr> gateway/policy.go <ul><li>Remove <code>allowExplicit</code> parameter from loaders/parsers.<br> <li> Use <code>policy.ID</code> as key; drop MID fallback.<br> <li> Adjust duplicate detection to use <code>policy.ID</code>.<br> <li> Update retry path to new signatures.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7424/files#diff-ec674104322b26b82def55e9be32117753ab66e7840490481eb6eb4c15bc35e7">+8/-18</a> </td> </tr> <tr> <td> <details> <summary><strong>rpc_backup_handlers.go</strong><dd><code>Update RPC backup to new parsing/decrypt API</code> </dd></summary> <hr> gateway/rpc_backup_handlers.go <ul><li>Call <code>parsePoliciesFromRPC</code> without allowExplicit.<br> <li> Use updated crypto decrypt signature.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7424/files#diff-69d9cb8df2bd4296a8e5e5d769009a09bd61ca65b7dbcbf29751af92698bd9ce">+2/-2</a> </td> </tr> <tr> <td> <details> <summary><strong>server.go</strong><dd><code>Remove allowExplicit parameter from sync paths</code> </dd></summary> <hr> gateway/server.go <ul><li>Call loaders without <code>AllowExplicitPolicyID</code>.<br> <li> Keep source selection logic intact.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7424/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+2/-2</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>policy_test.go</strong><dd><code>Align tests with standardized policy ID handling</code> </dd></summary> <hr> gateway/policy_test.go <ul><li>Update tests to new loader signatures.<br> <li> Adjust expectations to lookup by <code>policy.ID</code>.<br> <li> Remove dependency on config <code>AllowExplicitPolicyID</code>.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7424/files#diff-40d701767204255c38c7dd64939d6bb8df621640c4bddfe5f56080380476a18a">+28/-37</a> </td> </tr> </table></td></tr></tr></tbody></table> </details> ___ <!---TykTechnologies/jira-linter starts here--> <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-15825" title="TT-15825" target="_blank">TT-15825</a> </summary> | | | |---------|----| | Status | In Code Review | | Summary | [BE] Address inconsistencies with use of Policy identifiers | Generated at: 2025-11-12 14:28:57 </details> <!---TykTechnologies/jira-linter ends here--> --------- Co-authored-by: Radosław Krawczyk <radoslawkrawczyk@ST11253-radkra.local> (cherry picked from commit 39e1e56)
…use of Policy identifiers (#7424) (#7536) Cherry-pick of `39e1e5660b67c3b62e499bcc3fa5939b1a44e4cb` from `master` to `release-5.8` requires manual resolution. **Conflicts detected:** 3 - gateway/api_test.go Tips: - Check out this branch locally and run: `git cherry-pick -x 39e1e56` - Resolve conflicts (including submodules if any), then push back to this branch. Original commit: 39e1e56 <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-15825" title="TT-15825" target="_blank">TT-15825</a> </summary> | | | |---------|----| | Status | In Code Review | | Summary | [BE] Address inconsistencies with use of Policy identifiers | Generated at: 2025-11-12 17:33:09 </details> <!---TykTechnologies/jira-linter ends here--> --------- Co-authored-by: Tyk Bot <bot@tyk.io> Co-authored-by: Yaroslav Kotsur <yarikkotsur@gmail.com> Co-authored-by: Radosław Krawczyk <radoslawkrawczyk@ST11253-radkra.local>




User description
TT-15825
Description
Related Issue
Motivation and Context
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
Checklist
PR Type
Bug fix, Tests, Documentation
Description
Remove explicit policy ID toggle usage
Standardize policy ID to
policy.IDUpdate RPC/Dashboard loaders signatures
Adjust tests for new behavior
Diagram Walkthrough
File Walkthrough
config.go
Deprecate unused AllowExplicitPolicyID configconfig/config.go
AllowExplicitPolicyIDas deprecated.api.go
Backfill missing policy ID from MID on savegateway/api.go
newPol.IDtonewPol.MID.Hex()if empty.policy.go
Standardize ID handling and simplify loader APIsgateway/policy.go
allowExplicitparameter from loaders/parsers.policy.IDas key; drop MID fallback.policy.ID.rpc_backup_handlers.go
Update RPC backup to new parsing/decrypt APIgateway/rpc_backup_handlers.go
parsePoliciesFromRPCwithout allowExplicit.server.go
Remove allowExplicit parameter from sync pathsgateway/server.go
AllowExplicitPolicyID.policy_test.go
Align tests with standardized policy ID handlinggateway/policy_test.go
policy.ID.AllowExplicitPolicyID.Ticket Details
TT-15825
Generated at: 2025-11-12 14:28:57