This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new 27efc779eac api,ui: fix empty source cidr value for firewall rule
(#10208)
27efc779eac is described below
commit 27efc779eac984c250227203d1f58c864109ffe3
Author: Abhishek Kumar <[email protected]>
AuthorDate: Fri Jan 31 18:00:16 2025 +0530
api,ui: fix empty source cidr value for firewall rule (#10208)
Signed-off-by: Abhishek Kumar <[email protected]>
---
.../user/firewall/CreateFirewallRuleCmd.java | 7 +-
.../user/firewall/CreateFirewallRuleCmdTest.java | 91 ++++++++++++++++++++++
ui/src/views/network/FirewallRules.vue | 3 +
3 files changed, 98 insertions(+), 3 deletions(-)
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java
index b77041ee174..0809c935c4b 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java
@@ -19,6 +19,8 @@ package org.apache.cloudstack.api.command.user.firewall;
import java.util.ArrayList;
import java.util.List;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.RoleType;
@@ -103,14 +105,13 @@ public class CreateFirewallRuleCmd extends
BaseAsyncCreateCmd implements Firewal
@Override
public List<String> getSourceCidrList() {
- if (cidrlist != null) {
+ if (CollectionUtils.isNotEmpty(cidrlist) && !(cidrlist.size() == 1 &&
StringUtils.isBlank(cidrlist.get(0)))) {
return cidrlist;
} else {
- List<String> oneCidrList = new ArrayList<String>();
+ List<String> oneCidrList = new ArrayList<>();
oneCidrList.add(NetUtils.ALL_IP4_CIDRS);
return oneCidrList;
}
-
}
// ///////////////////////////////////////////////////
diff --git
a/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java
b/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java
new file mode 100644
index 00000000000..c905974b2be
--- /dev/null
+++
b/api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java
@@ -0,0 +1,91 @@
+// 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.user.firewall;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import com.cloud.utils.net.NetUtils;
+
+@RunWith(MockitoJUnitRunner.class)
+public class CreateFirewallRuleCmdTest {
+
+ private void validateAllIp4Cidr(final CreateFirewallRuleCmd cmd) {
+ Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList()));
+ Assert.assertEquals(1, cmd.getSourceCidrList().size());
+ Assert.assertEquals(NetUtils.ALL_IP4_CIDRS,
cmd.getSourceCidrList().get(0));
+ }
+
+ @Test
+ public void testGetSourceCidrList_Null() {
+ final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd();
+ ReflectionTestUtils.setField(cmd, "cidrlist", null);
+ validateAllIp4Cidr(cmd);
+ }
+
+ @Test
+ public void testGetSourceCidrList_Empty() {
+ final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd();
+ ReflectionTestUtils.setField(cmd, "cidrlist", new ArrayList<>());
+ validateAllIp4Cidr(cmd);
+ }
+
+ @Test
+ public void testGetSourceCidrList_NullFirstElement() {
+ final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd();
+ List<String> list = new ArrayList<>();
+ list.add(null);
+ ReflectionTestUtils.setField(cmd, "cidrlist", list);
+ validateAllIp4Cidr(cmd);
+ }
+
+ @Test
+ public void testGetSourceCidrList_EmptyFirstElement() {
+ final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd();
+ ReflectionTestUtils.setField(cmd, "cidrlist",
Collections.singletonList(" "));
+ validateAllIp4Cidr(cmd);
+ }
+
+ @Test
+ public void testGetSourceCidrList_Valid() {
+ final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd();
+ String cidr = "10.1.1.1/22";
+ ReflectionTestUtils.setField(cmd, "cidrlist",
Collections.singletonList(cidr));
+ Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList()));
+ Assert.assertEquals(1, cmd.getSourceCidrList().size());
+ Assert.assertEquals(cidr, cmd.getSourceCidrList().get(0));
+ }
+
+ @Test
+ public void testGetSourceCidrList_EmptyFirstElementButMore() {
+ final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd();
+ String cidr = "10.1.1.1/22";
+ ReflectionTestUtils.setField(cmd, "cidrlist", Arrays.asList(" ",
cidr));
+ Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList()));
+ Assert.assertEquals(2, cmd.getSourceCidrList().size());
+ Assert.assertEquals(cidr, cmd.getSourceCidrList().get(1));
+ }
+}
diff --git a/ui/src/views/network/FirewallRules.vue
b/ui/src/views/network/FirewallRules.vue
index 787f5c2d8e2..2d19bdd1f93 100644
--- a/ui/src/views/network/FirewallRules.vue
+++ b/ui/src/views/network/FirewallRules.vue
@@ -404,6 +404,9 @@ export default {
addRule () {
if (this.loading) return
this.loading = true
+ if (this.newRule.cidrlist == null || this.newRule.cidrlist.trim?.() ===
'') {
+ delete this.newRule.cidrlist
+ }
api('createFirewallRule', { ...this.newRule }).then(response => {
this.$pollJob({
jobId: response.createfirewallruleresponse.jobid,