Fix *.smtp.useAuth, quota.usage.smtp.useStartTLS and *.smtp.enabledSecurityProtocols settings definitions

[bernardodemarco]

Description

The global settings *.smtp.useAuth and quota.usage.smtp.useStartTLS are of boolean type, but the UI provides a standard text input field to manipulate them and does not perform any input validation. This PR, therefore, adds the toggle component to be rendered to manipulate these configurations.

Besides that, the global settings *.smtp.enabledSecurityProtocols expect a white-space separated list of predefined values, but the UI also provides a text input field without any proper validation. As a consequence of that, this PR also adds a new component that enables operators to select and deselect values that belong to a list of predefined values.

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

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Screenshots (if appropriate):

Available options to be selected - *.smtp.enabledSecurityProtocols

Display of selected options - *.smtp.enabledSecurityProtocols

How Has This Been Tested?

Tests of *.smtp.useAuth and quota.usage.smtp.useStartTLS

1. In order to test the boolean settings, I imagined a scenario prior to the proposed changes, in which the operator could change the settings' values without any validation.

SELECT name, value
FROM `cloud`.`configuration`
WHERE name IN ("quota.usage.smtp.useAuth", "alert.smtp.useAuth", "project.smtp.useAuth", "quota.usage.smtp.useStartTLS");

Query output:

name	value
alert.smtp.useAuth	on
project.smtp.useAuth	TRUE
quota.usage.smtp.useAuth	1
quota.usage.smtp.useStartTLS	false

2. I verified the source code and the values of these settings are retrieved through BooleanUtils.toBoolean(String). It returns true if the value is true, y, t, 1, on or yes (case-insensitive).

   Hence, I executed the SQL query that has been added to the update script. The query converts the values that are interpreted as true to true. Values that are not interpreted as true, such as false and 0, are converted to false.

Normalization output:

name	value
alert.smtp.useAuth	true
project.smtp.useAuth	true
quota.usage.smtp.useAuth	true
quota.usage.smtp.useStartTLS	false

3. After this, I manipulated the settings through UI and I checked that when the toggle component was on, the parameter value was sent to the API as true. When the toggle was off, on the other hand, the parameter was sent as false.

Tests of *.smtp.enabledSecurityProtocols

1. When the component is focused, all the available options are displayed to the operator.

2. After selecting the desired options and confirming the update, the value parameter is sent to the API as a white-space separated list with the selected values.

3. If no option is selected, value is sent as an empty string ("").

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 90.90909% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 15.28%. Comparing base (d5241d3) to head (22097b2).
Report is 6 commits behind head on main.

Files	Patch %	Lines
...in/java/com/cloud/projects/ProjectManagerImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9031      +/-   ##
============================================
- Coverage     15.28%   15.28%   -0.01%     
+ Complexity    11522    11521       -1     
============================================
  Files          5425     5425              
  Lines        474005   474016      +11     
  Branches      61507    61782     +275     
============================================
+ Hits          72447    72448       +1     
- Misses       393516   393524       +8     
- Partials       8042     8044       +2     

Flag	Coverage Δ
uitests	4.26% <ø> (-0.01%)	⬇️
unittests	16.01% <90.90%> (+<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.

[DaanHoogland]

@bernardodemarco , I think your change makes total sense functionally. One technical concern though

    public enum Kind {
        CSV, Order, Select, WhitespaceSeparatedListWithOptions
    }

As there is already a CSV kind of configuration, do we need to add the WhitespaceSeparatedListWithOptions ? It seems to me it adds no functional extras in comparison. Isn't converting the ..SecurityProtocol settings to Kind.CSV good enough?

[bernardodemarco]

@DaanHoogland, thanks for your review!

When I started developing this PR I had also considered converting the *.smtp.enabledSecurityProtocols settings to Kind.CSV. However, I've noticed some issues with that.

The settings expect a whitespace-separated list of predefined values. The Kind.CSV type, on the other hand, expects a comma-separated list of values that are not predefined. This means that the operator is able to add any additional values, for example:

As a consequence of this, the Kind.CSV UI component could not be reused.

Another problem that I've identified is that the current settings' values would need to be normalized into a comma-separated list. Additionally, the source code would need to be reviewed and adjusted in order to interpret correctly the new structure of values.

On top of that, I've identified that the Kind.Order UI component could properly be reused to display the Kind.WhitespaceSeparatedListWithOptions settings without introducing big changes to the code. As shown in the screenshots available in the PR's description, the operator can easily select and browse through the available options, but it is not able to add additional values.