-
Notifications
You must be signed in to change notification settings - Fork 304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HPCC-33146 Add backoff if vault authentication fails and check other vaults #19370
Conversation
cee76f5
to
3cbb92f
Compare
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-33146 Jirabot Action Result: |
@afishbeckln please can you check this. I have not had a chance to test it yet. Also are any of the other errors that are returned also candidates for backing off (e.g. 403?) |
(Compare ignoring whitespace) |
3cbb92f
to
cf18ecd
Compare
@ghalliday the general problem with this backoff mechanism is that the failure could have been because the configured approle didn't have permission to access the specific secret for which access was being attempted. I don't think we can know whether vault access, path access, or specific secret access failed. Therefore, we could be disabling access to the particular named vault due to permissions on one particular secret. |
@ghalliday Ok, having looked closer, the way you are catching the exception does catch issues related to login (acquiring token) rather than using token, which makes this level of backoff likely to be ok. The question about other errors is what I meant where we likely can't tell whether the problem is with the permissions over all or just with the item being requested. But to answer your question, when it gets a 403, it actually tries a re-login. If the re-login fails because access has been revoked, or some other issue, it should already trigger your backoff. I think other errors are too vague or potentially caused by permissions on the actual secret being accessed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but needs testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ghalliday - I haven't spotted any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ghalliday - new commit/defaulting to backoff 0 makes sense
…vaults Signed-off-by: Gavin Halliday <[email protected]>
951293c
into
hpcc-systems:candidate-9.6.x
Jirabot Action Result: |
Type of change:
Checklist:
Smoketest:
Testing: