From 4762115b749aa3aa3ca2fc1709b791bd0cf4b2c1 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 20 Jan 2025 10:53:23 +0530 Subject: [PATCH 1/6] ui: fix empty source cidr value for firewall rule Fixes #10002 Signed-off-by: Abhishek Kumar --- ui/src/views/network/FirewallRules.vue | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ui/src/views/network/FirewallRules.vue b/ui/src/views/network/FirewallRules.vue index 787f5c2d8e2e..3b4c3f1e3be7 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, From 5cdbc39010c457525e139e026b240285212d0d95 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 20 Jan 2025 11:44:02 +0530 Subject: [PATCH 2/6] fix Signed-off-by: Abhishek Kumar --- ui/src/views/network/FirewallRules.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/views/network/FirewallRules.vue b/ui/src/views/network/FirewallRules.vue index 3b4c3f1e3be7..2d19bdd1f93a 100644 --- a/ui/src/views/network/FirewallRules.vue +++ b/ui/src/views/network/FirewallRules.vue @@ -405,7 +405,7 @@ export default { if (this.loading) return this.loading = true if (this.newRule.cidrlist == null || this.newRule.cidrlist.trim?.() === '') { - delete this.newRule['cidrlist'] + delete this.newRule.cidrlist } api('createFirewallRule', { ...this.newRule }).then(response => { this.$pollJob({ From 2b4e5b5122d38e9b3f910c88fc07754c60eddc12 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 22 Jan 2025 16:28:28 +0530 Subject: [PATCH 3/6] api: fix empty cidr list Signed-off-by: Abhishek Kumar --- .../api/command/user/firewall/CreateFirewallRuleCmd.java | 7 ++++--- 1 file changed, 4 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 b77041ee1748..d375c78c6400 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 @@ 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 String getProtocol() { @Override public List getSourceCidrList() { - if (cidrlist != null) { + if (CollectionUtils.isNotEmpty(cidrlist) && (cidrlist.size() == 1 && StringUtils.isNotBlank(cidrlist.get(0)))) { return cidrlist; } else { - List oneCidrList = new ArrayList(); + List oneCidrList = new ArrayList<>(); oneCidrList.add(NetUtils.ALL_IP4_CIDRS); return oneCidrList; } - } // /////////////////////////////////////////////////// From 2b48ce361e2d1a1336e7be20188d40ffb8a34ff3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 22 Jan 2025 18:19:27 +0530 Subject: [PATCH 4/6] Update CreateFirewallRuleCmd.java --- .../api/command/user/firewall/CreateFirewallRuleCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d375c78c6400..f316d942d368 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 @@ -105,7 +105,7 @@ public String getProtocol() { @Override public List getSourceCidrList() { - if (CollectionUtils.isNotEmpty(cidrlist) && (cidrlist.size() == 1 && StringUtils.isNotBlank(cidrlist.get(0)))) { + if (CollectionUtils.isNotEmpty(cidrlist) && !(cidrlist.size() == 1 && StringUtils.isNotBlank(cidrlist.get(0)))) { return cidrlist; } else { List oneCidrList = new ArrayList<>(); From cfa4a4cf5b8ae66d5a7029691fd5e4a556b22b29 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 22 Jan 2025 18:20:41 +0530 Subject: [PATCH 5/6] Update CreateFirewallRuleCmd.java --- .../api/command/user/firewall/CreateFirewallRuleCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f316d942d368..0809c935c4b7 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 @@ -105,7 +105,7 @@ public String getProtocol() { @Override public List getSourceCidrList() { - if (CollectionUtils.isNotEmpty(cidrlist) && !(cidrlist.size() == 1 && StringUtils.isNotBlank(cidrlist.get(0)))) { + if (CollectionUtils.isNotEmpty(cidrlist) && !(cidrlist.size() == 1 && StringUtils.isBlank(cidrlist.get(0)))) { return cidrlist; } else { List oneCidrList = new ArrayList<>(); From 20244abfc73cfd03f79b138dd1c2fa5fff601486 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 23 Jan 2025 11:42:58 +0530 Subject: [PATCH 6/6] add tests Signed-off-by: Abhishek Kumar --- .../firewall/CreateFirewallRuleCmdTest.java | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java 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 000000000000..c905974b2be9 --- /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 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)); + } +}