Add parameter to not create additional users on cloudstack-setup-databases

[lucas-a-martins]

Description

When using the cloudstack-setup-databases command during the database setup process, some additional users are created. Since the standard procedure involves creating and configuring database users prior to the ACS setup, these extra users are not used. Moreover, the additional users created by cloudstack-setup-databases are granted excessive permissions, requiring operators to manually delete them.

This PR introduces a new optional parameter, --skip-users-auto-creation. By using this parameter, ACS will skip the automatic creation of these additional users.

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

• Major
• Minor

Screenshots (if appropriate):

How Has This Been Tested?

After executing the cloudstack-setup-databases with the new flag, I checked the database users and, as expected, no new users were created. I then repeated the procedure without the new flag, and the extra users were created as usual.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 16.07%. Comparing base (b38ee63) to head (42c6e7a).
Report is 223 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9969      +/-   ##
============================================
+ Coverage     15.80%   16.07%   +0.26%     
- Complexity    12586    13191     +605     
============================================
  Files          5627     5664      +37     
  Lines        492328   497612    +5284     
  Branches      59692    60216     +524     
============================================
+ Hits          77828    79994    +2166     
- Misses       405977   408661    +2684     
- Partials       8523     8957     +434     

Flag	Coverage Δ
uitests	4.00% <ø> (-0.04%)	⬇️
unittests	16.91% <ø> (+0.28%)	⬆️

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.

[sureshanaparti]

@blueorangutan package

[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]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11676

[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]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11728

[DaanHoogland]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

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

Test	Result	Time (s)	Test File
test_01_secure_vm_migration	Error	134.24	test_vm_life_cycle.py
test_01_secure_vm_migration	Error	134.24	test_vm_life_cycle.py

[DaanHoogland]

huh? what's foo?

[lucas-a-martins]

@DaanHoogland it's a local variable

cloudstack/setup/db/create-database.sql

Lines 26 to 43 in 864751d

	DECLARE foo BIGINT DEFAULT 0 ;
	SELECT COUNT(*)
	INTO foo
	FROM `mysql`.`user`
	WHERE `User` = 'cloud' and host = 'localhost';
	IF foo > 0 THEN
	DROP USER 'cloud'@'localhost' ;
	END IF;
	SELECT COUNT(*)
	INTO foo
	FROM `mysql`.`user`
	WHERE `User` = 'cloud' and host = '%';
	IF foo > 0 THEN
	DROP USER 'cloud'@'%' ;
	END IF;

[DaanHoogland]

wouw, this is unreadable code (not your fault per sé)

so as I read this, this means that the lines

"IF foo > 0 THEN"

,

"DROP USER 'cloud'@'localhost' ;"

and

"END IF;"

are being skipped.

That does not seem to be the intention of the code. Should it just (conditionally) add the middle line? Or does it somehow do that?

[lucas-a-martins]

@DaanHoogland I was unable to skip the DROP without skipping the IF and END IF, the command results in an error. Considering that the IF is used only for the query I want to skip, I tought skipping it too wouldn't be a problem.

My goal when removing these queries is because the new flag was removing users from database when host = % || localhost, but the idea of this PR is to not create additional users, not to remove existing ones.

[DaanHoogland]

I am not sure that works this way. The line IF foo > 0 THEN occurs 4 times in the file create-database.sql

I think it is safer to add a line with a noop and only remove the DROP line

[lucas-a-martins]

@DaanHoogland isn't it just 2 times?

Here:

cloudstack/setup/db/create-database.sql

Line 32 in 864751d

IF foo > 0 THEN

And here:

cloudstack/setup/db/create-database.sql

Line 41 in 864751d

IF foo > 0 THEN

Am I missing something?

[DaanHoogland]

sorry, yes. I grep -r'd in a compiled env so I had two results in the target dir.
even so,

I think it is safer to add a line with a noop and only remove the DROP line