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
Error Handling The removal of the conditional block that allows continuation in the loop when multiple policies are being processed might lead to unintended behavior where a single missing policy could halt the entire operation, even if other policies could be applied successfully.
Improve error handling when a policy is not found in a list of multiple policies
Consider handling the error more gracefully when multiple policies are involved. Instead of skipping the current iteration when a policy is not found, log the error and continue checking other policies. This ensures that all policies are evaluated, and the error handling is consistent.
if !ok {
err := fmt.Errorf("policy not found: %q", polID)
t.Logger().Error(err)
+ if len(policyIDs) > 1 {+ continue+ }
return err
}
Suggestion importance[1-10]: 9
Why: The suggestion correctly identifies a potential issue with error handling when multiple policies are involved. By reintroducing the condition to continue the loop when a policy is not found, it ensures that all policies are evaluated, which is a more robust and consistent approach.
--- prev.txt 2024-08-30 14:25:20.822441511 +0000+++ current.txt 2024-08-30 14:25:17.770426502 +0000@@ -5252,10 +5252,7 @@
type CertificatesConfig struct {
API []string `json:"apis"`
- // Upstream is used to specify the certificates to be used in mutual TLS connections to upstream services. These are set at gateway level as a map of domain -> certificate id or path.- // For example if you want Tyk to use the certificate `ab23ef123` for requests to the `example.com` upstream and `/certs/default.pem` for all other upstreams then:- // In `tyk.conf` you would configure `"security": {"certificates": {"upstream": {"*": "/certs/default.pem", "example.com": "ab23ef123"}}}`- // And if using environment variables you would set this to `*:/certs/default.pem,example.com:ab23ef123`.+ // Specify upstream mutual TLS certificates at a global level in the following format: `{ "<host>": "<cert>" }``
Upstream map[string]string `json:"upstream"`
// Certificates used for Control API Mutual TLS
ControlAPI []string `json:"control_api"`
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
Fixed case in which trying to apply a non-existing policy error would be swallowed when having partitioned keys.
Description
Related Issue
Motivation and Context
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
Checklist
PR Type
Bug fix
Description
Applymethod of theServiceto prevent silent failures when policies are missing.Changes walkthrough 📝
apply.go
Fix error handling for non-existing policies in Apply methodinternal/policy/apply.go
policy was encountered.
found.