From eab86b950ae9284fe175d7c6ba0736f1c46758d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Tue, 29 Apr 2025 14:56:23 -0300 Subject: [PATCH 1/9] make value parameter of the updateConfiguration API required --- .../cloudstack/api/command/admin/config/UpdateCfgCmd.java | 2 +- .../com/cloud/configuration/ConfigurationManagerImpl.java | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java index dbf478df7012..57d144fb5d8e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java @@ -48,7 +48,7 @@ public class UpdateCfgCmd extends BaseCmd { @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the configuration") private String cfgName; - @Parameter(name = ApiConstants.VALUE, type = CommandType.STRING, description = "the value of the configuration", length = 4096) + @Parameter(name = ApiConstants.VALUE, type = CommandType.STRING, required = true, description = "the value of the configuration", length = 4096) private String value; @Parameter(name = ApiConstants.ZONE_ID, diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 81970b0f8a79..2ff35ed4c07e 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -1038,10 +1038,6 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP throw new CloudRuntimeException("Only Root Admin is allowed to edit this configuration."); } - if (value == null) { - return _configDao.findByName(name); - } - ConfigKey.Scope scope = null; Long id = null; int paramCountCheck = 0; From 8cfc5eb14be9da8fcf7228ce15942178a3141efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Wed, 30 Apr 2025 11:44:31 -0300 Subject: [PATCH 2/9] clean up config values --- ui/src/views/setting/ConfigurationValue.vue | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/ui/src/views/setting/ConfigurationValue.vue b/ui/src/views/setting/ConfigurationValue.vue index e438f0eb8315..381234fbb917 100644 --- a/ui/src/views/setting/ConfigurationValue.vue +++ b/ui/src/views/setting/ConfigurationValue.vue @@ -275,6 +275,15 @@ export default { if (configrecord.type === 'WhitespaceSeparatedListWithOptions') { newValue = newValue.join(' ') } + + // The updateConfiguration API expects a blank string to clean up the configuration value + if ( + (['CSV', 'Order', 'WhitespaceSeparatedListWithOptions'].includes(configrecord.type) && Object.keys(newValue).length === 0) || + (configrecord.type === 'String' && newValue.length === 0) + ) { + newValue = ' ' + } + const params = { [this.scopeKey]: this.$route.params?.id, name: configrecord.name, @@ -362,6 +371,17 @@ export default { } return 0 } + + if (configrecord.value?.trim().length === 0) { + if (['CSV', 'Order', 'WhitespaceSeparatedListWithOptions'].includes(configrecord.type)) { + return [] + } + + if (configrecord.type === 'String') { + return '' + } + } + if (['Order', 'CSV'].includes(configrecord.type)) { if (configrecord.value && configrecord.value.length > 0) { return String(configrecord.value).split(',') From 373df59a84f0a9550d6e33bc8e0ff6a6d8b3af13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Wed, 30 Apr 2025 13:42:56 -0300 Subject: [PATCH 3/9] remove unecessary validation --- ui/src/views/setting/ConfigurationValue.vue | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/src/views/setting/ConfigurationValue.vue b/ui/src/views/setting/ConfigurationValue.vue index 381234fbb917..d93e94d79419 100644 --- a/ui/src/views/setting/ConfigurationValue.vue +++ b/ui/src/views/setting/ConfigurationValue.vue @@ -278,8 +278,7 @@ export default { // The updateConfiguration API expects a blank string to clean up the configuration value if ( - (['CSV', 'Order', 'WhitespaceSeparatedListWithOptions'].includes(configrecord.type) && Object.keys(newValue).length === 0) || - (configrecord.type === 'String' && newValue.length === 0) + (['CSV', 'Order', 'WhitespaceSeparatedListWithOptions', 'String'].includes(configrecord.type) && newValue.length === 0) ) { newValue = ' ' } From 37a83213539eee55bb88b978af5b5b14d418993b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Fri, 6 Jun 2025 14:26:50 -0300 Subject: [PATCH 4/9] fix test_vm_strict_host_tags.py integration tests --- test/integration/smoke/test_vm_strict_host_tags.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/integration/smoke/test_vm_strict_host_tags.py b/test/integration/smoke/test_vm_strict_host_tags.py index 2377e9a76185..b03f6a9295bc 100644 --- a/test/integration/smoke/test_vm_strict_host_tags.py +++ b/test/integration/smoke/test_vm_strict_host_tags.py @@ -97,6 +97,9 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): + if len(value) == 0: + value = ' ' + cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value @@ -265,6 +268,9 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): + if len(value) == 0: + value = ' ' + cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value @@ -385,6 +391,9 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): + if len(value) == 0: + value = ' ' + cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value @@ -509,6 +518,9 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): + if len(value) == 0: + value = ' ' + cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value From 696cd1d1ee4dbba161d81f1ccbe9d07375c15a45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Fri, 6 Jun 2025 14:28:50 -0300 Subject: [PATCH 5/9] fix test_global_settings.py integration tests --- test/integration/smoke/test_global_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/smoke/test_global_settings.py b/test/integration/smoke/test_global_settings.py index 53f55736d4f6..f866a1f44ddf 100644 --- a/test/integration/smoke/test_global_settings.py +++ b/test/integration/smoke/test_global_settings.py @@ -79,7 +79,7 @@ def tearDown(self): updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() updateConfigurationCmd.name = "commands.timeout" - updateConfigurationCmd.value = "" + updateConfigurationCmd.value = " " self.apiClient.updateConfiguration(updateConfigurationCmd) class TestListConfigurations(cloudstackTestCase): From 7b5a14d8eecc277a91800ddf937bd8ccf8a4f419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Fri, 6 Jun 2025 14:30:41 -0300 Subject: [PATCH 6/9] fix test_deploy_vm_extra_config_data.py integration tests --- test/integration/smoke/test_deploy_vm_extra_config_data.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/smoke/test_deploy_vm_extra_config_data.py b/test/integration/smoke/test_deploy_vm_extra_config_data.py index 98e60f9d287c..cd67119a0ea5 100644 --- a/test/integration/smoke/test_deploy_vm_extra_config_data.py +++ b/test/integration/smoke/test_deploy_vm_extra_config_data.py @@ -125,6 +125,9 @@ def setUp(self): # Ste Global Config value def add_global_config(self, name, value): + if len(value) == 0: + value = ' ' + self.apiclient = self.testClient.getApiClient() self.hypervisor = self.testClient.getHypervisorInfo() self.dbclient = self.testClient.getDbConnection() From 2d7e0c2a9610b783e16b719a903b002e645e87f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Fri, 31 Oct 2025 12:23:16 -0300 Subject: [PATCH 7/9] trying new approach --- .../command/admin/config/UpdateCfgCmd.java | 2 +- .../ConfigurationManagerImpl.java | 4 ++++ .../smoke/test_deploy_vm_extra_config_data.py | 3 --- .../integration/smoke/test_global_settings.py | 2 +- .../smoke/test_vm_strict_host_tags.py | 12 ------------ ui/src/views/setting/ConfigurationValue.vue | 19 ------------------- 6 files changed, 6 insertions(+), 36 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java index 57d144fb5d8e..dbf478df7012 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java @@ -48,7 +48,7 @@ public class UpdateCfgCmd extends BaseCmd { @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the configuration") private String cfgName; - @Parameter(name = ApiConstants.VALUE, type = CommandType.STRING, required = true, description = "the value of the configuration", length = 4096) + @Parameter(name = ApiConstants.VALUE, type = CommandType.STRING, description = "the value of the configuration", length = 4096) private String value; @Parameter(name = ApiConstants.ZONE_ID, diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 2ff35ed4c07e..6989e1cfca2e 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -1030,6 +1030,10 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP category = config.getCategory(); } + if (value == null) { + throw new InvalidParameterValueException(String.format("A value for the [%s] configuration must be informed.", name)); + } + validateIpAddressRelatedConfigValues(name, value); validateConflictingConfigValue(name, value); diff --git a/test/integration/smoke/test_deploy_vm_extra_config_data.py b/test/integration/smoke/test_deploy_vm_extra_config_data.py index cd67119a0ea5..98e60f9d287c 100644 --- a/test/integration/smoke/test_deploy_vm_extra_config_data.py +++ b/test/integration/smoke/test_deploy_vm_extra_config_data.py @@ -125,9 +125,6 @@ def setUp(self): # Ste Global Config value def add_global_config(self, name, value): - if len(value) == 0: - value = ' ' - self.apiclient = self.testClient.getApiClient() self.hypervisor = self.testClient.getHypervisorInfo() self.dbclient = self.testClient.getDbConnection() diff --git a/test/integration/smoke/test_global_settings.py b/test/integration/smoke/test_global_settings.py index f866a1f44ddf..53f55736d4f6 100644 --- a/test/integration/smoke/test_global_settings.py +++ b/test/integration/smoke/test_global_settings.py @@ -79,7 +79,7 @@ def tearDown(self): updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() updateConfigurationCmd.name = "commands.timeout" - updateConfigurationCmd.value = " " + updateConfigurationCmd.value = "" self.apiClient.updateConfiguration(updateConfigurationCmd) class TestListConfigurations(cloudstackTestCase): diff --git a/test/integration/smoke/test_vm_strict_host_tags.py b/test/integration/smoke/test_vm_strict_host_tags.py index b03f6a9295bc..2377e9a76185 100644 --- a/test/integration/smoke/test_vm_strict_host_tags.py +++ b/test/integration/smoke/test_vm_strict_host_tags.py @@ -97,9 +97,6 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): - if len(value) == 0: - value = ' ' - cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value @@ -268,9 +265,6 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): - if len(value) == 0: - value = ' ' - cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value @@ -391,9 +385,6 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): - if len(value) == 0: - value = ' ' - cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value @@ -518,9 +509,6 @@ def expunge_vm(self, vm): @classmethod def updateConfiguration(self, name, value): - if len(value) == 0: - value = ' ' - cmd = updateConfiguration.updateConfigurationCmd() cmd.name = name cmd.value = value diff --git a/ui/src/views/setting/ConfigurationValue.vue b/ui/src/views/setting/ConfigurationValue.vue index d93e94d79419..e438f0eb8315 100644 --- a/ui/src/views/setting/ConfigurationValue.vue +++ b/ui/src/views/setting/ConfigurationValue.vue @@ -275,14 +275,6 @@ export default { if (configrecord.type === 'WhitespaceSeparatedListWithOptions') { newValue = newValue.join(' ') } - - // The updateConfiguration API expects a blank string to clean up the configuration value - if ( - (['CSV', 'Order', 'WhitespaceSeparatedListWithOptions', 'String'].includes(configrecord.type) && newValue.length === 0) - ) { - newValue = ' ' - } - const params = { [this.scopeKey]: this.$route.params?.id, name: configrecord.name, @@ -370,17 +362,6 @@ export default { } return 0 } - - if (configrecord.value?.trim().length === 0) { - if (['CSV', 'Order', 'WhitespaceSeparatedListWithOptions'].includes(configrecord.type)) { - return [] - } - - if (configrecord.type === 'String') { - return '' - } - } - if (['Order', 'CSV'].includes(configrecord.type)) { if (configrecord.value && configrecord.value.length > 0) { return String(configrecord.value).split(',') From 2deab676a33a10ca07c069366f13b300eb716522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Wed, 5 Nov 2025 08:46:37 -0300 Subject: [PATCH 8/9] Update server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Co-authored-by: dahn --- .../java/com/cloud/configuration/ConfigurationManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 6989e1cfca2e..a4d46cbd73dd 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -1031,7 +1031,7 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP } if (value == null) { - throw new InvalidParameterValueException(String.format("A value for the [%s] configuration must be informed.", name)); + throw new InvalidParameterValueException(String.format(“The new value for the [%s] configuration must be given.", name)); } validateIpAddressRelatedConfigValues(name, value); From fb158ea3938402ee1323db08b4a8145af23d8c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernardo=20De=20Marco=20Gon=C3=A7alves?= Date: Wed, 5 Nov 2025 08:58:53 -0300 Subject: [PATCH 9/9] Update server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java --- .../java/com/cloud/configuration/ConfigurationManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index a4d46cbd73dd..6b70301974bd 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -1031,7 +1031,7 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP } if (value == null) { - throw new InvalidParameterValueException(String.format(“The new value for the [%s] configuration must be given.", name)); + throw new InvalidParameterValueException(String.format("The new value for the [%s] configuration must be given.", name)); } validateIpAddressRelatedConfigValues(name, value);