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

[bernardodemarco]

trafficstars

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)
• [X] 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
• [X] 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

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 4.20%. Comparing base (ee39104) to head (89eb61d).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #9031       +/-   ##
============================================
- Coverage     15.32%   4.20%   -11.12%     
============================================
  Files          5452     368     -5084     
  Lines        476525   30161   -446364     
  Branches      61192    5307    -55885     
============================================
- Hits          73010    1269    -71741     
+ Misses       395447   28748   -366699     
+ Partials       8068     144     -7924     

Flag	Coverage Δ
uitests	4.20% <ø> (-0.01%)	:arrow_down:
unittests	?

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: 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.

[github-actions[bot]]

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

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9804

[DaanHoogland]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

[SF] Trillian test result (tid-10376) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 45401 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9031-t10376-kvm-centos7.zip Smoke tests completed. 132 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[github-actions[bot]]

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