Skip to content
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

Improve connect cluster vault error handling #494

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

ThomasCAI-mlv
Copy link
Collaborator

@ThomasCAI-mlv ThomasCAI-mlv commented Dec 11, 2024

For the connect cluster vault API, I simplified the validateConnectClusterVault method check:

  • If the provided connect cluster is not found (the connect cluster does not exist or the namespace does not have the OWNER or WRITE ACL on this connect cluster), throw an "not found" error
  • If the provided connect cluster is found but is missing AES config fields, throw an "no config" error. I updated the error message to be more explicit, because previously, it was confusing to display the list of other usable vault connect clusters, since the user explicitly gave the connect cluster he wants to use to vault his secrets. In any case, they can use the LIST vaults API to list the available vaults connect clusters.

I also removed the "allowed namespace" check because it was weird to provide information of another namespaces to a given namespace.

@ThomasCAI-mlv ThomasCAI-mlv added the bug Something isn't working label Dec 11, 2024
@ThomasCAI-mlv ThomasCAI-mlv self-assigned this Dec 11, 2024
Copy link
Collaborator

@loicgreffier loicgreffier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasCAI-mlv Good improvements 👍

@loicgreffier loicgreffier added enhancement This issue or pull request improves a feature and removed bug Something isn't working labels Dec 20, 2024
@loicgreffier loicgreffier merged commit b37432f into master Dec 20, 2024
4 checks passed
@loicgreffier loicgreffier deleted the fix/connect-vault-error-message branch December 20, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue or pull request improves a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants