fix: added consul policy list check #4164
Conversation
| case http.StatusOK: | ||
| for _, policy := range policyList { | ||
| if policyName == policy.Name { | ||
| isExistingPolicy = true | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
If we already found policy here, we can just return policy? why we need to find policy by name again below? (IMO, kind of redundant logic below)
There was a problem hiding this comment.
No, it's not. The list API only returns the names of the policies. Still need another call to actually get the policy (i.e. it is just chaining off to the original code).
There was a problem hiding this comment.
that is not true: see this: https://www.consul.io/api-docs/acl/policies#list-policies
There was a problem hiding this comment.
@jim-wang-intel I understand, I think, what you were saying. We only really need the name and the ID, not the text of the policy. I think I like the solution as presented because it addresses the bug as stated (error in consul log due to get of non-existent policy.) Since this is bootstrapping logic, it is not performance critical that we get the answer we need in one or two REST API calls. I think we should implement the simplest and most readable fix. We can refactor later if we want.
ItsRico
force-pushed
the
acl_policy_check
branch
from
247dd7a
to
4167d64
Compare
Added a check in getPolicyByName to see whether the policy exists in the policy list before attempting to get the policy. This prevents erroneous or unclear logging. closes: edgexfoundry#4108 Signed-off-by: Rico Chavez-Lopez <rchavezlopez@ucdavis.edu>
ItsRico
force-pushed
the
acl_policy_check
branch
from
4167d64
to
aa55db8
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Added a check in getPolicyByName to see whether the policy exists in the policy list before attempting to get the policy. This prevents erroneous or unclear logging.
closes: #4108
Signed-off-by: Rico Chavez-Lopez rchavezlopez@ucdavis.edu
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:describing the break)Testing Instructions