-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Hide CloudStack version from XML response when unauthenticated #10575
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10575 +/- ##
=========================================
Coverage 16.26% 16.26%
+ Complexity 13388 13387 -1
=========================================
Files 5674 5674
Lines 498929 498938 +9
Branches 60338 60341 +3
=========================================
+ Hits 81154 81157 +3
- Misses 408736 408741 +5
- Partials 9039 9040 +1
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:
|
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.
code looks good @gpordeus , but can I ask you to remove all the log.append() calls and only append sb to log, just before returning at line 211?
Not your doing for sure, but the code looks a bit silly like this.
winterhazel
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.
Tested manually:
- Verified that XML responses do not contain the version when the user is not authenticated, independently of
expose.cloudstack.version.api.xml.response - Verified that XML responses contain the version when user is authenticated and
expose.cloudstack.version.api.xml.responseis set to true - Verified that XML responses do not contain the version when user is authenticated and
expose.cloudstack.version.api.xml.responseis set to false - Verified that
listCapabilitiesreturns the version whenexpose.cloudstack.version.api.list.capabilitiesis set to true - Verified that
listCapabilitiesdoes not return the version whenexpose.cloudstack.version.api.list.capabilitiesis set to false
|
@DaanHoogland While I agree it is silly, it would take bigger changes to clean it up and demand more testing. |
|
@blueorangutan package |
|
@JoaoJandre 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 ✔️ debian ✔️ suse15. SL-JID 13583 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-13461) |
|
@blueorangutan package |
|
@JoaoJandre 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 ✔️ debian ✔️ suse15. SL-JID 14044 |
|
@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-13692)
|
|
@blueorangutan package |
|
@nvazquez 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 ✔️ debian ✔️ suse15. SL-JID 14139 |
|
@blueorangutan test |
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13753)
|
|
no response from original author but it looks ready, @nvazquez and had been marked so by @winterhazel. merging. |
This PR hides the cloudstack version field of XML responses when the caller is unauthenticated (checks if it is SYSTEM account).
It also adds the
expose.cloudstack.version.api.list.capabilitiesto allow hiding the field from thelistCapabilitiesAPI call andexpose.cloudstack.version.api.xml.responseto allow hiding the field from XML responses, both are true by default as to not change compatibility.It does not change the version information in system VMs and VRs.
Fixes: #10072
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Unauthenticated:
Authenticated and configs:
expose.cloudstack.version.api.list.capabilities= true;expose.cloudstack.version.api.xml.response= true;expose.cloudstack.version.api.list.capabilities= false;expose.cloudstack.version.api.xml.response= false;How did you try to break this feature and the system with this change?