Fix injection of preset variables into the JS interpreter #12515
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12515 +/- ##
============================================
+ Coverage 16.24% 16.27% +0.02%
- Complexity 13396 13487 +91
============================================
Files 5658 5658
Lines 499169 503837 +4668
Branches 60579 62882 +2303
============================================
+ Hits 81087 81990 +903
- Misses 409041 412715 +3674
- Partials 9041 9132 +91
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:
|
|
@blueorangutan package |
|
@winterhazel 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 16519 |
|
@blueorangutan package |
|
@winterhazel 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 16520 |
DaanHoogland
requested review from
BryanMLima,
Copilot and
shwstppr
and removed request for
Copilot and
shwstppr
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
@blueorangutan test |
|
@RosiKyu a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15273)
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
Show resolved
Hide resolved
shwstppr
left a comment
shwstppr
left a comment
There was a problem hiding this comment.
code lgtm. Copilot highlighted a logging issue
utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java
Show resolved
Hide resolved
|
[SF] Trillian test result (tid-15283)
|
|
@blueorangutan package |
|
@winterhazel 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 16566 |
5 participants
Description
Before commit 03a4b9f, preset variables were injected into the JS interpreter by directly prepending them as a string to the script. 03a4b9f changed the interpreter so that preset variables are injected via bindings instead. However, all preset variables are still injected as a string. Due to this, variables that were previously interpreted as objects are now interpreted as strings. This broke the Quota activation rules and secondary storage selectors features.
This PR changes the code to inject the preset variables using their expected types. Also, the test files of most preset variables have been removed because the tests do not make sense anymore with the new injection mechanism.
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
How Has This Been Tested?
org.apache.cloudstack.utils.jsinterpreter.JsInterpreter#executeScriptInThreadto validate that the preset variables had their expected type in the JS interpreter.org.apache.cloudstack.utils.jsinterpreter.JsInterpreter#executeScriptInThreadto validate that the preset variables had their expected type in the JS interpreter.org.apache.cloudstack.utils.jsinterpreter.JsInterpreter#executeScriptInThreadto validate that the offering's host tags were correctly injected into the JS interpreter as a list.