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..1dfc739a10f0 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 @@ -150,7 +150,7 @@ public void execute() { ConfigurationResponse response = _responseGenerator.createConfigurationResponse(cfg); response.setResponseName(getCommandName()); response = setResponseScopes(response); - response = setResponseValue(response, cfg); + setResponseValue(response, cfg); this.setResponseObject(response); } else { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update config"); @@ -161,15 +161,13 @@ public void execute() { * Sets the configuration value in the response. If the configuration is in the `Hidden` or `Secure` categories, the value is encrypted before being set in the response. * @param response to be set with the configuration `cfg` value * @param cfg to be used in setting the response value - * @return the response with the configuration's value */ - public ConfigurationResponse setResponseValue(ConfigurationResponse response, Configuration cfg) { + public void setResponseValue(ConfigurationResponse response, Configuration cfg) { + String value = cfg.getValue(); if (cfg.isEncrypted()) { - response.setValue(DBEncryptionUtil.encrypt(getValue())); - } else { - response.setValue(getValue()); + value = DBEncryptionUtil.encrypt(value); } - return response; + response.setValue(value); } /** diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmdTest.java new file mode 100644 index 000000000000..51b1cd9e14b7 --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmdTest.java @@ -0,0 +1,81 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.admin.config; + +import org.apache.cloudstack.api.response.ConfigurationResponse; +import org.apache.cloudstack.config.Configuration; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.crypt.DBEncryptionUtil; + +@RunWith(MockitoJUnitRunner.class) +public class UpdateCfgCmdTest { + + private UpdateCfgCmd updateCfgCmd; + + private MockedStatic mockedStatic; + + @Before + public void setUp() { + updateCfgCmd = new UpdateCfgCmd(); + mockedStatic = Mockito.mockStatic(DBEncryptionUtil.class); + } + + @After + public void tearDown() { + mockedStatic.close(); + } + + @Test + public void setResponseValueSetsEncryptedValueWhenConfigurationIsEncrypted() { + ConfigurationResponse response = new ConfigurationResponse(); + Configuration cfg = Mockito.mock(Configuration.class); + Mockito.when(cfg.isEncrypted()).thenReturn(true); + Mockito.when(cfg.getValue()).thenReturn("testValue"); + Mockito.when(DBEncryptionUtil.encrypt("testValue")).thenReturn("encryptedValue"); + updateCfgCmd.setResponseValue(response, cfg); + Assert.assertEquals("encryptedValue", response.getValue()); + } + + @Test + public void setResponseValueSetsPlainValueWhenConfigurationIsNotEncrypted() { + ConfigurationResponse response = new ConfigurationResponse(); + Configuration cfg = Mockito.mock(Configuration.class); + Mockito.when(cfg.isEncrypted()).thenReturn(false); + Mockito.when(cfg.getValue()).thenReturn("testValue"); + updateCfgCmd.setResponseValue(response, cfg); + Assert.assertEquals("testValue", response.getValue()); + } + + @Test + public void setResponseValueHandlesNullConfigurationValueGracefully() { + ConfigurationResponse response = new ConfigurationResponse(); + Configuration cfg = Mockito.mock(Configuration.class); + Mockito.when(cfg.isEncrypted()).thenReturn(false); + Mockito.when(cfg.getValue()).thenReturn(null); + updateCfgCmd.setResponseValue(response, cfg); + Assert.assertNull(response.getValue()); + } + +} diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 2c5a931a8319..184f8a13ea03 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -982,6 +982,19 @@ private void updateCustomDisplayNameOnHypervisorsList(String previousValue, Stri } } + protected String getNormalizedEmptyValueForConfig(final String name, final String inputValue, + final Long configStorageId) { + String value = inputValue.trim(); + if (!value.isEmpty() && !value.equals("null")) { + return value; + } + if (configStorageId != null) { + return ""; + } + ConfigKey key = _configDepot.get(name); + return (key != null && key.type() == String.class) ? "" : null; + } + @Override @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, eventDescription = "updating configuration") public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidParameterValueException { @@ -1076,11 +1089,7 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP throw new InvalidParameterValueException("cannot handle multiple IDs, provide only one ID corresponding to the scope"); } - value = value.trim(); - - if (value.isEmpty() || value.equals("null")) { - value = (id == null) ? null : ""; - } + value = getNormalizedEmptyValueForConfig(name, value, id); String currentValueInScope = getConfigurationValueInScope(config, name, scope, id); final String updatedValue = updateConfiguration(userId, name, category, value, scope, id); diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java index 1ffd7967eec3..fbdc75f0b503 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java @@ -1072,4 +1072,42 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenDetailsMatch Assert.assertFalse(result); } + + @Test + public void normalizedEmptyValueForConfigReturnsTrimmedValueWhenInputIsValid() { + String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", " validValue ", null); + Assert.assertEquals("validValue", result); + } + + @Test + public void normalizedEmptyValueForConfigReturnsNullWhenInputIsNullAndNoConfigStorageId() { + String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "null", null); + Assert.assertNull(result); + } + + @Test + public void normalizedEmptyValueForConfigReturnsEmptyStringWhenInputIsNullAndConfigStorageIdProvided() { + String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "null", 123L); + Assert.assertEquals("", result); + } + + @Test + public void normalizedEmptyValueForConfigReturnsEmptyStringWhenKeyTypeIsStringAndInputIsEmpty() { + ConfigKey mockKey = Mockito.mock(ConfigKey.class); + Mockito.when(mockKey.type()).thenReturn(String.class); + Mockito.doReturn(mockKey).when(configDepot).get("someConfig"); + + String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "", null); + Assert.assertEquals("", result); + } + + @Test + public void normalizedEmptyValueForConfigReturnsNullWhenKeyTypeIsNotStringAndInputIsEmpty() { + ConfigKey mockKey = Mockito.mock(ConfigKey.class); + Mockito.when(mockKey.type()).thenReturn(Integer.class); + Mockito.doReturn(mockKey).when(configDepot).get("someConfig"); + + String result = configurationManagerImplSpy.getNormalizedEmptyValueForConfig("someConfig", "", null); + Assert.assertNull(result); + } }