Allow modification of user vm details if user.vm.readonly.details is empty

[Pearl1594]

Description

This PR fixes: #10305
Currently when the global setting 'user.vm.readonly.details' is cleared, it sets the global setting value to NULL. Querying a global setting whose value is null returns the default value. Hence, updating this specific global setting value to "" as opposed to NULL.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• build/CI
• test (unit or integration test code)

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?

How did you try to break this feature and the system with this change?

[codecov]

Codecov Report

❌ Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.56%. Comparing base (50fe265) to head (2e631b5).

Files with missing lines	Patch %	Lines
.../apache/cloudstack/framework/config/ConfigKey.java	71.42%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.22   #10456   +/-   ##
=========================================
  Coverage     17.55%   17.56%           
- Complexity    15535    15536    +1     
=========================================
  Files          5909     5909           
  Lines        529053   529066   +13     
  Branches      64619    64622    +3     
=========================================
+ Hits          92879    92908   +29     
+ Misses       425719   425703   -16     
  Partials      10455    10455           

Flag	Coverage Δ
uitests	3.58% <ø> (ø)
unittests	18.62% <73.33%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Pearl1594]

@blueorangutan package

[blueorangutan]

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

[weizhouapache]

@Pearl1594
would be good to add a parameter to ConfigKey to indicate whether use default value if empty ?

[Pearl1594]

@Pearl1594 would be good to add a parameter to ConfigKey to indicate whether use default value if empty ?

Currently when value is null, it picks up the default value, but in this case, we do not want it to use the default value. Unless I understood you wrong @weizhouapache

[DaanHoogland]

@Pearl1594 would be good to add a parameter to ConfigKey to indicate whether use default value if empty ?

Currently when value is null, it picks up the default value, but in this case, we do not want it to use the default value. Unless I understood you wrong @weizhouapache

Yes, @Pearl1594 , but I think it is still good to use a ConfigKey and let users set it explicitely to "".

[weizhouapache]

@Pearl1594 would be good to add a parameter to ConfigKey to indicate whether use default value if empty ?

Currently when value is null, it picks up the default value, but in this case, we do not want it to use the default value. Unless I understood you wrong @weizhouapache

@Pearl1594 would be good to add a parameter to ConfigKey to indicate whether use default value if empty ?

Currently when value is null, it picks up the default value, but in this case, we do not want it to use the default value. Unless I understood you wrong @weizhouapache

Yes, @Pearl1594 , but I think it is still good to use a ConfigKey and let users set it explicitely to "".

yes. using ConfigKey is more generic, it is easier to support more configurations in the future, for example user.vm.denied.details.

[blueorangutan]

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

[JoaoJandre]

I believe that this is not the correct script for this update 😄

[DaanHoogland]

@Pearl1594 I think we want this line in the (to be created) 4.19.2 -> 4.19.3 and in the 4.20.0 -> 4.20.1 scripts. We must also test if this upgrade will then work from 4.19.3 to 4.20.1 (it should according to theory but testing is still advised (by me))

code looks good btw

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[DaanHoogland]

@Pearl1594 , can you revisit?

[Pearl1594]

Will look into it tomorrow @DaanHoogland .

[Pearl1594]

@weizhouapache do you think this is a better / generic solution. I've taken into advisement your suggestion from earlier..

[sonarqubecloud]

Quality Gate failed

Failed conditions
22.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube Cloud

[Pearl1594]

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

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