You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "sureshanaparti (via GitHub)" <gi...@apache.org> on 2023/12/29 08:19:21 UTC

[PR] User data content size validation with actual length only, and some code improvements [cloudstack]

sureshanaparti opened a new pull request, #8418:
URL: https://github.com/apache/cloudstack/pull/8418

   ### Description
   
   This PR validate user data with actual length only, and has some code improvements.
   
   Partially fixes #8415
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ******************************************************************************* -->
   <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ******************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [x] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] build/CI
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   
   ### Screenshots (if appropriate):
   
   
   ### How Has This Been Tested?
   
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   
   #### How did you try to break this feature and the system with this change?
   
   <!-- see how your change affects other areas of the code, etc. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#discussion_r1438096794


##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +90,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {

Review Comment:
   why is userdata obligatory?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1880557433

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1880549878

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8224


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1876894054

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#discussion_r1447253910


##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +88,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {

Review Comment:
   @sureshanaparti Maybe leave it at `trace` level? This way it will only show up when someone really needs to see it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#discussion_r1444852101


##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +88,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {
+            return null;
+        }
 
-            if (!Base64.isBase64(userData)) {
-                throw new InvalidParameterValueException("User data is not base64 encoded");
-            }
-            // If GET, use 4K. If POST, support up to 1M.
-            if (httpmethod.equals(BaseCmd.HTTPMethod.GET)) {
-                decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_GET_LENGTH, BaseCmd.HTTPMethod.GET);
-            } else if (httpmethod.equals(BaseCmd.HTTPMethod.POST)) {
-                decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_POST_LENGTH, BaseCmd.HTTPMethod.POST);
+        if (userData.contains("%")) {
+            try {
+                userData = URLDecoder.decode(userData, "UTF-8");
+            } catch (UnsupportedEncodingException e) {
+                throw new InvalidParameterValueException("Url decoding of userdata failed.");
             }
+        }
 
-            if (decodedUserData == null || decodedUserData.length < 1) {
-                throw new InvalidParameterValueException("User data is too short");
-            }
-            // Re-encode so that the '=' paddings are added if necessary since 'isBase64' does not require it, but python does on the VR.
-            return Base64.encodeBase64String(decodedUserData);
+        if (!Base64.isBase64(userData)) {
+            throw new InvalidParameterValueException("User data is not base64 encoded");

Review Comment:
   ```suggestion
               throw new InvalidParameterValueException("User data is not base64 encoded.");
   ```



##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +88,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {

Review Comment:
   Could you add a log line at the start of the method that logs the userdata that is being validated? something like:
   ```suggestion
           logger.debug(String.format("Validating userdata [%s].", userData));
           
           if (StringUtils.isBlank(userData)) {
   ```



##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +88,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {
+            return null;
+        }
 
-            if (!Base64.isBase64(userData)) {
-                throw new InvalidParameterValueException("User data is not base64 encoded");
-            }
-            // If GET, use 4K. If POST, support up to 1M.
-            if (httpmethod.equals(BaseCmd.HTTPMethod.GET)) {
-                decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_GET_LENGTH, BaseCmd.HTTPMethod.GET);
-            } else if (httpmethod.equals(BaseCmd.HTTPMethod.POST)) {
-                decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_POST_LENGTH, BaseCmd.HTTPMethod.POST);
+        if (userData.contains("%")) {
+            try {
+                userData = URLDecoder.decode(userData, "UTF-8");
+            } catch (UnsupportedEncodingException e) {
+                throw new InvalidParameterValueException("Url decoding of userdata failed.");
             }
+        }
 
-            if (decodedUserData == null || decodedUserData.length < 1) {
-                throw new InvalidParameterValueException("User data is too short");
-            }
-            // Re-encode so that the '=' paddings are added if necessary since 'isBase64' does not require it, but python does on the VR.
-            return Base64.encodeBase64String(decodedUserData);
+        if (!Base64.isBase64(userData)) {
+            throw new InvalidParameterValueException("User data is not base64 encoded");
         }
-        return null;
-    }
 
-    private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, BaseCmd.HTTPMethod httpMethod) {
         byte[] decodedUserData = null;
 
-        if (userData.length() >= maxHTTPLength) {
-            throw new InvalidParameterValueException(String.format("User data is too long for an http %s request", httpMethod.toString()));
+        // If GET, use 4K. If POST, support up to 1M.
+        if (httpmethod.equals(BaseCmd.HTTPMethod.GET)) {
+            decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_GET_LENGTH, BaseCmd.HTTPMethod.GET);
+        } else if (httpmethod.equals(BaseCmd.HTTPMethod.POST)) {
+            decodedUserData = validateAndDecodeByHTTPMethod(userData, MAX_HTTP_POST_LENGTH, BaseCmd.HTTPMethod.POST);
         }
-        if (userData.length() > ConfigurationManager.VM_USERDATA_MAX_LENGTH.value()) {
-            throw new InvalidParameterValueException("User data has exceeded configurable max length : " + ConfigurationManager.VM_USERDATA_MAX_LENGTH.value());
+
+        // Re-encode so that the '=' paddings are added if necessary since 'isBase64' does not require it, but python does on the VR.
+        return Base64.encodeBase64String(decodedUserData);
+    }
+
+    private byte[] validateAndDecodeByHTTPMethod(String userData, int maxHTTPLength, BaseCmd.HTTPMethod httpMethod) {
+        byte[] decodedUserData = Base64.decodeBase64(userData.getBytes());
+        if (decodedUserData == null || decodedUserData.length < 1) {
+            throw new InvalidParameterValueException("User data is too short");

Review Comment:
   ```suggestion
               throw new InvalidParameterValueException("User data is too short.");
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#discussion_r1446943616


##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +88,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {

Review Comment:
   @JoaoJandre may not be good to log user data here (can be of more length sometimes).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1881913312

   <b>[SF] Trillian test result (tid-8750)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 48273 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8418-t8750-kvm-centos7.zip
   Smoke tests completed. 118 look OK, 3 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_list_cpvm_vm | `Failure` | 0.03 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.04 | test_ssvm.py
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 614.36 | test_kubernetes_clusters.py
   test_08_migrate_vm | `Error` | 0.06 | test_vm_life_cycle.py
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1871832696

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1871835961

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8418?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Comparison is base [(`6d916ca`)](https://app.codecov.io/gh/apache/cloudstack/commit/6d916cad348f5833a567c17f5c9dbccaf2135448?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 30.85% compared to head [(`d7d93e1`)](https://app.codecov.io/gh/apache/cloudstack/pull/8418?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 4.39%.
   > Report is 7 commits behind head on main.
   
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               main   #8418       +/-   ##
   ============================================
   - Coverage     30.85%   4.39%   -26.46%     
   ============================================
     Files          5341     361     -4980     
     Lines        374861   28622   -346239     
     Branches      54518    4992    -49526     
   ============================================
   - Hits         115659    1258   -114401     
   + Misses       243973   27225   -216748     
   + Partials      15229     139    -15090     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.39% <ø> (ø)` | |
   | [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8418?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1889300411

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✖️ el9 ✔️ debian ✖️ suse15. SL-JID 8303


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1893839109

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-2036417471

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-2036423262

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8418?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `50.00000%` with `12 lines` in your changes are missing coverage. Please review.
   > Project coverage is 30.80%. Comparing base [(`6d916ca`)](https://app.codecov.io/gh/apache/cloudstack/commit/6d916cad348f5833a567c17f5c9dbccaf2135448?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`c95f2ce`)](https://app.codecov.io/gh/apache/cloudstack/pull/8418?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 151 commits behind head on 4.19.
   
   | [Files](https://app.codecov.io/gh/apache/cloudstack/pull/8418?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...pache/cloudstack/userdata/UserDataManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8418?src=pr&el=tree&filepath=engine%2Fuserdata%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fuserdata%2FUserDataManagerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3VzZXJkYXRhL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3VzZXJkYXRhL1VzZXJEYXRhTWFuYWdlckltcGwuamF2YQ==) | 40.00% | [7 Missing and 5 partials :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8418?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               4.19    #8418      +/-   ##
   ============================================
   - Coverage     30.85%   30.80%   -0.05%     
   + Complexity    34048    33979      -69     
   ============================================
     Files          5341     5341              
     Lines        374861   374888      +27     
     Branches      54518    54520       +2     
   ============================================
   - Hits         115659   115491     -168     
   - Misses       243973   244127     +154     
   - Partials      15229    15270      +41     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.72% <33.33%> (-0.04%)` | :arrow_down: |
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.39% <ø> (ø)` | |
   | [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8418/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `16.46% <41.66%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8418?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#discussion_r1450380154


##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +88,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {

Review Comment:
   we can log it later, may be before validation, in case needed @JoaoJandre I think, logger obj for only this trace is unnecessary.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1893840501

   @sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#discussion_r1441565124


##########
engine/userdata/src/main/java/org/apache/cloudstack/userdata/UserDataManagerImpl.java:
##########
@@ -90,49 +90,50 @@ public String concatenateUserData(String userdata1, String userdata2, String use
 
     @Override
     public String validateUserData(String userData, BaseCmd.HTTPMethod httpmethod) {
-        byte[] decodedUserData = null;
-        if (userData != null) {
-
-            if (userData.contains("%")) {
-                try {
-                    userData = URLDecoder.decode(userData, "UTF-8");
-                } catch (UnsupportedEncodingException e) {
-                    throw new InvalidParameterValueException("Url decoding of userdata failed.");
-                }
-            }
+        if (StringUtils.isBlank(userData)) {

Review Comment:
   > why is userdata obligatory?
   
   userdata is not mandatory, updated it to return null as earlier.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1945560259

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1875305346

   @sureshanaparti can you review the outstanding comments?
   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1900101219

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1876893306

   > @sureshanaparti, along with these changes is it better to call the registerUserData API using POST from UI ?
   
   changes done


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1876981680

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8199


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1880565763

   @sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1877028818

   @sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1871833135

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1875422883

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8189


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1871871814

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8164


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1875306092

   @rohityadavcloud 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1881243019

   @sureshanaparti can you rebase with 4.18.2? As the related issue is also aimed at 4.18.2
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1889093621

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1889090095

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1893855787

   <b>[SF] Trillian Build Failed (tid-8854)<b/>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1876895785

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1945568700

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1945663951

   Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8662


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1880487773

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, register managed user data using POST call from UI, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1880486897

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1877019500

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] User data content size validation with actual length only, and some code improvements [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8418:
URL: https://github.com/apache/cloudstack/pull/8418#issuecomment-1900103482

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org