-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Removal of internal ID in backup event descriptions #12197
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
Conversation
DaanHoogland
left a comment
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.
clgtm
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12197 +/- ##
============================================
- Coverage 17.60% 17.60% -0.01%
Complexity 15669 15669
============================================
Files 5915 5915
Lines 529964 529967 +3
Branches 64734 64734
============================================
Hits 93305 93305
- Misses 426191 426194 +3
Partials 10468 10468
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
abh1sar
left a comment
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.
code lgtm
sureshanaparti
left a comment
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.
clgtm
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15946 |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@blueorangutan package |
|
@weizhouapache @winterhazel , do you think we need more than smoke tests for this one? |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16194 |
@DaanHoogland smoke tests should be enough here |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15080)
|
|
Awesome work, congrats on your first merged pull request! |
Description
Currently, the description of backup events contains the backup's internal ID, displaying sensitive content to users. This PR changes the events description to use the backup's UUID instead.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
Before the changes:
After the changes:
How Has This Been Tested?
After changing the code, I compiled the code and installed the packages to my lab. Then, I created a VM to perform the tests and assigned it a backup offering. After that, I created a test backup, restored it, and deleted it. When the
Eventstab was accessed, I validated that the events displayed the backup's UUID, instead of its internal ID.How did you try to break this feature and the system with this change?