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 <abhishek.mr...@gmail.com> 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 <abhishek.mr...@gmail.com> --- .../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,