Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: operator throwing panic when storage is empty #357

Merged
merged 2 commits into from
Feb 9, 2024
Merged

fix: operator throwing panic when storage is empty #357

merged 2 commits into from
Feb 9, 2024

Conversation

csatib02
Copy link
Member

@csatib02 csatib02 commented Feb 2, 2024

Overview

  • I placed a check in vault_types.go so the panic won't occur.
  • Added a check, which creates a new status and produces an error, if the specified spec lacks storage configurations.

Fixes #189

Notes for reviewer

  • The test is quite limited, it specifically addresses the area that previously failed.

Signed-off-by: Bence Csati <csatib02@gmail.com>
@csatib02 csatib02 requested a review from a team as a code owner February 2, 2024 17:41
@csatib02 csatib02 requested review from ramizpolic and removed request for a team February 2, 2024 17:41
@github-actions github-actions bot added the size/M Denotes a PR that changes 100-499 lines label Feb 2, 2024
Signed-off-by: Bence Csati <csatib02@gmail.com>
Copy link
Member

@akijakya akijakya left a comment

Choose a reason for hiding this comment

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

LGTM, nice work, thanks for also adding a test! It would be nice if the Vault api would provide us a way to validate configs, but I couldn't find it so far. If we encounter more panics due to other missing config values, the code could be extended with a validation step in the future.

@ramizpolic ramizpolic merged commit 9a41620 into bank-vaults:main Feb 9, 2024
31 checks passed
@csatib02 csatib02 deleted the fix/no-storage-panic branch February 12, 2024 08:39
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
size/M Denotes a PR that changes 100-499 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Operator panics when no storage is set
3 participants