Skip to content

Commit af8a582

Browse files
weizhouapacheweizhouapache
authored
api/utils/ui: List protocol numbers and icmp types (#8293)
This PR contains the following changes * adds a new API to list network procotols and details/types/codes, etc * get network protocols on UI and add dropdowns for procotol numbers and icmp types/codes * validate icmp types/codes when add network ACL
1 parent 7dffbc6 commit af8a582

File tree

16 files changed

+1054
-52
lines changed

16 files changed

+1054
-52
lines changed

‎api/src/main/java/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ public class ApiConstants {
321321
public static final String IS_DEFAULT_USE = "defaultuse";
322322
public static final String OLD_FORMAT = "oldformat";
323323
public static final String OP = "op";
324+
public static final String OPTION = "option";
324325
public static final String OPTIONS = "options";
325326
public static final String OS_CATEGORY_ID = "oscategoryid";
326327
public static final String OS_CATEGORY_NAME = "oscategoryname";

‎api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworkProtocolsCmd.java‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.api.command.user.network;
18+
19+
import com.cloud.utils.net.NetworkProtocols;
20+
import org.apache.cloudstack.acl.RoleType;
21+
import org.apache.cloudstack.api.APICommand;
22+
import org.apache.cloudstack.api.ApiConstants;
23+
import org.apache.cloudstack.api.BaseCmd;
24+
import org.apache.cloudstack.api.Parameter;
25+
import org.apache.cloudstack.api.response.ListResponse;
26+
import org.apache.cloudstack.api.response.NetworkProtocolResponse;
27+
import org.apache.cloudstack.context.CallContext;
28+
import org.apache.log4j.Logger;
29+
30+
import java.util.ArrayList;
31+
import java.util.List;
32+
33+
@APICommand(name = "listNetworkProtocols", description = "Lists details of network protocols", responseObject = NetworkProtocolResponse.class,
34+
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
35+
authorized = { RoleType.Admin, RoleType.DomainAdmin, RoleType.ResourceAdmin, RoleType.User}, since = "4.19.0")
36+
public class ListNetworkProtocolsCmd extends BaseCmd {
37+
public static final Logger s_logger = Logger.getLogger(ListNetworkProtocolsCmd.class.getName());
38+
39+
40+
/////////////////////////////////////////////////////
41+
//////////////// API parameters /////////////////////
42+
/////////////////////////////////////////////////////
43+
44+
@Parameter(name = ApiConstants.OPTION, type = CommandType.STRING, required = true,
45+
description = "The option of network protocols. Supported values are: protocolnumber, icmptype.")
46+
private String option;
47+
48+
49+
/////////////////////////////////////////////////////
50+
/////////////////// Accessors ///////////////////////
51+
/////////////////////////////////////////////////////
52+
53+
54+
public String getOption() {
55+
return option;
56+
}
57+
58+
/////////////////////////////////////////////////////
59+
/////////////// API Implementation///////////////////
60+
/////////////////////////////////////////////////////
61+
62+
@Override
63+
public void execute() {
64+
ListResponse<NetworkProtocolResponse> response = new ListResponse<>();
65+
List<NetworkProtocolResponse> networkProtocolResponses = new ArrayList<>();
66+
67+
NetworkProtocols.Option option = NetworkProtocols.Option.getOption(getOption());
68+
switch (option) {
69+
case ProtocolNumber:
70+
updateResponseWithProtocolNumbers(networkProtocolResponses);
71+
break;
72+
case IcmpType:
73+
updateResponseWithIcmpTypes(networkProtocolResponses);
74+
break;
75+
default:
76+
break;
77+
}
78+
79+
response.setResponses(networkProtocolResponses);
80+
response.setResponseName(getCommandName());
81+
setResponseObject(response);
82+
}
83+
84+
@Override
85+
public long getEntityOwnerId() {
86+
return CallContext.current().getCallingAccount().getId();
87+
}
88+
89+
private void updateResponseWithProtocolNumbers(List<NetworkProtocolResponse> responses) {
90+
for (NetworkProtocols.ProtocolNumber protocolNumber : NetworkProtocols.ProtocolNumbers) {
91+
NetworkProtocolResponse networkProtocolResponse = new NetworkProtocolResponse(protocolNumber.getNumber(),
92+
protocolNumber.getKeyword(), protocolNumber.getProtocol());
93+
networkProtocolResponse.setObjectName("networkprotocol");
94+
responses.add(networkProtocolResponse);
95+
}
96+
}
97+
98+
private void updateResponseWithIcmpTypes(List<NetworkProtocolResponse> responses) {
99+
for (NetworkProtocols.IcmpType icmpType : NetworkProtocols.IcmpTypes) {
100+
NetworkProtocolResponse networkProtocolResponse = new NetworkProtocolResponse(icmpType.getType(),
101+
null, icmpType.getDescription());
102+
for (NetworkProtocols.IcmpCode code : icmpType.getIcmpCodes()) {
103+
networkProtocolResponse.addDetail(String.valueOf(code.getCode()), code.getDescription());
104+
}
105+
networkProtocolResponse.setObjectName("networkprotocol");
106+
responses.add(networkProtocolResponse);
107+
}
108+
}
109+
}

‎api/src/main/java/org/apache/cloudstack/api/response/NetworkProtocolResponse.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.api.response;
18+
19+
import java.util.LinkedHashMap;
20+
import java.util.Map;
21+
22+
import org.apache.cloudstack.api.ApiConstants;
23+
import org.apache.cloudstack.api.BaseResponse;
24+
25+
import com.cloud.serializer.Param;
26+
import com.google.gson.annotations.SerializedName;
27+
28+
public class NetworkProtocolResponse extends BaseResponse {
29+
@SerializedName(ApiConstants.INDEX)
30+
@Param(description = "the index (ID, Value, Code, Type, Option, etc) of the protocol parameter")
31+
private Integer index;
32+
33+
@SerializedName(ApiConstants.NAME)
34+
@Param(description = "the name of the protocol parameter")
35+
private String name;
36+
37+
@SerializedName(ApiConstants.DESCRIPTION)
38+
@Param(description = "the description of the protocol parameter")
39+
private String description;
40+
41+
@SerializedName(ApiConstants.DETAILS)
42+
@Param(description = "the details of the protocol parameter")
43+
private Map details;
44+
45+
public NetworkProtocolResponse(Integer index, String name, String description) {
46+
this.index = index;
47+
this.name = name;
48+
this.description = description;
49+
}
50+
51+
public Integer getIndex() {
52+
return index;
53+
}
54+
55+
public void setIndex(Integer index) {
56+
this.index = index;
57+
}
58+
59+
public String getName() {
60+
return name;
61+
}
62+
63+
public void setName(String name) {
64+
this.name = name;
65+
}
66+
67+
public String getDescription() {
68+
return description;
69+
}
70+
71+
public void setDescription(String description) {
72+
this.description = description;
73+
}
74+
75+
public Map getDetails() {
76+
return details;
77+
}
78+
79+
public void setDetails(Map details) {
80+
this.details = details;
81+
}
82+
83+
public void addDetail(String key, String value) {
84+
if (this.details == null) {
85+
this.details = new LinkedHashMap();
86+
}
87+
this.details.put(key, value);
88+
}
89+
}

‎api/src/test/java/org/apache/cloudstack/api/command/user/network/ListNetworkProtocolsCmdTest.java‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.cloudstack.api.command.user.network;
19+
20+
import com.cloud.utils.net.NetworkProtocols;
21+
import org.apache.cloudstack.api.BaseCmd;
22+
import org.apache.cloudstack.api.response.ListResponse;
23+
import org.apache.cloudstack.api.response.NetworkProtocolResponse;
24+
import org.junit.Assert;
25+
import org.junit.Test;
26+
import org.junit.runner.RunWith;
27+
28+
import org.mockito.junit.MockitoJUnitRunner;
29+
import org.springframework.test.util.ReflectionTestUtils;
30+
31+
@RunWith(MockitoJUnitRunner.class)
32+
public class ListNetworkProtocolsCmdTest {
33+
34+
@Test
35+
public void testListNetworkProtocolNumbers() {
36+
ListNetworkProtocolsCmd cmd = new ListNetworkProtocolsCmd();
37+
String option = NetworkProtocols.Option.ProtocolNumber.toString();
38+
ReflectionTestUtils.setField(cmd, "option", option);
39+
Assert.assertEquals(cmd.getOption(), option);
40+
41+
try {
42+
cmd.execute();
43+
} catch (Exception e) {
44+
e.printStackTrace();
45+
}
46+
Object response = cmd.getResponseObject();
47+
Assert.assertTrue(response instanceof ListResponse);
48+
ListResponse listResponse = (ListResponse) response;
49+
Assert.assertEquals(BaseCmd.getResponseNameByClass(cmd.getClass()), listResponse.getResponseName());
50+
Assert.assertNotNull(listResponse.getResponses());
51+
Assert.assertNotEquals(0, listResponse.getResponses().size());
52+
Object firstResponse = listResponse.getResponses().get(0);
53+
Assert.assertTrue(firstResponse instanceof NetworkProtocolResponse);
54+
Assert.assertEquals("networkprotocol", ((NetworkProtocolResponse) firstResponse).getObjectName());
55+
Assert.assertEquals(Integer.valueOf(0), ((NetworkProtocolResponse) firstResponse).getIndex());
56+
Assert.assertEquals("HOPOPT", ((NetworkProtocolResponse) firstResponse).getName());
57+
}
58+
59+
@Test
60+
public void testListIcmpTypes() {
61+
ListNetworkProtocolsCmd cmd = new ListNetworkProtocolsCmd();
62+
String option = NetworkProtocols.Option.IcmpType.toString();
63+
ReflectionTestUtils.setField(cmd, "option", option);
64+
Assert.assertEquals(cmd.getOption(), option);
65+
66+
try {
67+
cmd.execute();
68+
} catch (Exception e) {
69+
e.printStackTrace();
70+
}
71+
Object response = cmd.getResponseObject();
72+
Assert.assertTrue(response instanceof ListResponse);
73+
ListResponse listResponse = (ListResponse) response;
74+
Assert.assertEquals(BaseCmd.getResponseNameByClass(cmd.getClass()), listResponse.getResponseName());
75+
Assert.assertNotNull(listResponse.getResponses());
76+
Assert.assertNotEquals(0, listResponse.getResponses().size());
77+
Object firstResponse = listResponse.getResponses().get(0);
78+
Assert.assertTrue(firstResponse instanceof NetworkProtocolResponse);
79+
Assert.assertEquals("networkprotocol", ((NetworkProtocolResponse) firstResponse).getObjectName());
80+
Assert.assertEquals(Integer.valueOf(0), ((NetworkProtocolResponse) firstResponse).getIndex());
81+
Assert.assertNotNull(((NetworkProtocolResponse) firstResponse).getDetails());
82+
System.out.println(((NetworkProtocolResponse) firstResponse).getDetails());
83+
Assert.assertEquals("Echo reply", ((NetworkProtocolResponse) firstResponse).getDetails().get("0"));
84+
}
85+
86+
@Test(expected = IllegalArgumentException.class)
87+
public void testListInvalidOption() {
88+
ListNetworkProtocolsCmd cmd = new ListNetworkProtocolsCmd();
89+
String option = "invalid-option";
90+
ReflectionTestUtils.setField(cmd, "option", option);
91+
Assert.assertEquals(cmd.getOption(), option);
92+
93+
cmd.execute();
94+
}
95+
}

‎server/src/main/java/com/cloud/network/security/SecurityGroupManagerImpl.java‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -658,10 +658,14 @@ public List<SecurityGroupRuleVO> authorizeSecurityGroupRule(final Long securityG
658658
if(StringUtils.isNumeric(protocol)){
659659
int protoNumber = Integer.parseInt(protocol);
660660
// Deal with ICMP(protocol number 1) specially because it need to be paired with icmp type and code
661-
if (protoNumber == 1) {
662-
protocol = "icmp";
663-
icmpCode = -1;
664-
icmpType = -1;
661+
if (protoNumber == NetUtils.ICMP_PROTO_NUMBER) {
662+
protocol = NetUtils.ICMP_PROTO;
663+
if (icmpCode == null) {
664+
icmpCode = -1;
665+
}
666+
if (icmpType == null) {
667+
icmpType = -1;
668+
}
665669
} else if(protoNumber < 0 || protoNumber > 255){
666670
throw new InvalidParameterValueException("Invalid protocol number: " + protoNumber);
667671
}
@@ -673,18 +677,7 @@ public List<SecurityGroupRuleVO> authorizeSecurityGroupRule(final Long securityG
673677
}
674678
}
675679
if (protocol.equals(NetUtils.ICMP_PROTO)) {
676-
if ((icmpType == null) || (icmpCode == null)) {
677-
throw new InvalidParameterValueException("Invalid ICMP type/code specified, icmpType = " + icmpType + ", icmpCode = " + icmpCode);
678-
}
679-
if (icmpType == -1 && icmpCode != -1) {
680-
throw new InvalidParameterValueException("Invalid icmp code");
681-
}
682-
if (icmpType != -1 && icmpCode == -1) {
683-
throw new InvalidParameterValueException("Invalid icmp code: need non-negative icmp code ");
684-
}
685-
if (icmpCode > 255 || icmpType > 255 || icmpCode < -1 || icmpType < -1) {
686-
throw new InvalidParameterValueException("Invalid icmp type/code ");
687-
}
680+
NetUtils.validateIcmpTypeAndCode(icmpType, icmpCode);
688681
startPortOrType = icmpType;
689682
endPortOrCode = icmpCode;
690683
} else if (protocol.equals(NetUtils.ALL_PROTO)) {
@@ -785,6 +778,7 @@ public List<SecurityGroupRuleVO> doInTransaction(TransactionStatus status) {
785778
SecurityGroupRuleVO securityGroupRule = _securityGroupRuleDao.findByProtoPortsAndAllowedGroupId(securityGroup.getId(), protocolFinal, startPortOrTypeFinal,
786779
endPortOrCodeFinal, ngVO.getId());
787780
if ((securityGroupRule != null) && (securityGroupRule.getRuleType() == ruleType)) {
781+
s_logger.warn("The rule already exists. id= " + securityGroupRule.getUuid());
788782
continue; // rule already exists.
789783
}
790784
securityGroupRule = new SecurityGroupRuleVO(ruleType, securityGroup.getId(), startPortOrTypeFinal, endPortOrCodeFinal, protocolFinal, ngVO.getId());

‎server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ protected void validateProtocol(NetworkACLItemVO networkACLItemVO) {
583583
Integer icmpCode = networkACLItemVO.getIcmpCode();
584584
Integer icmpType = networkACLItemVO.getIcmpType();
585585
// icmp code and icmp type can't be passed in for any other protocol rather than icmp
586-
boolean isIcmpProtocol = protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO);
586+
boolean isIcmpProtocol = protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) || protocol.equalsIgnoreCase(String.valueOf(NetUtils.ICMP_PROTO_NUMBER));
587587
if (!isIcmpProtocol && (icmpCode != null || icmpType != null)) {
588588
throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only");
589589
}

‎server/src/main/java/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@
446446
import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
447447
import org.apache.cloudstack.api.command.user.network.ListNetworkOfferingsCmd;
448448
import org.apache.cloudstack.api.command.user.network.ListNetworkPermissionsCmd;
449+
import org.apache.cloudstack.api.command.user.network.ListNetworkProtocolsCmd;
449450
import org.apache.cloudstack.api.command.user.network.ListNetworksCmd;
450451
import org.apache.cloudstack.api.command.user.network.MoveNetworkAclItemCmd;
451452
import org.apache.cloudstack.api.command.user.network.RemoveNetworkPermissionsCmd;
@@ -3621,6 +3622,7 @@ public List<Class<?>> getCommands() {
36213622
cmdList.add(DeleteNetworkCmd.class);
36223623
cmdList.add(ListNetworkACLsCmd.class);
36233624
cmdList.add(ListNetworkOfferingsCmd.class);
3625+
cmdList.add(ListNetworkProtocolsCmd.class);
36243626
cmdList.add(ListNetworksCmd.class);
36253627
cmdList.add(RestartNetworkCmd.class);
36263628
cmdList.add(UpdateNetworkCmd.class);

‎server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -622,8 +622,8 @@ public void validateProtocolTestProtocolIsStringValid() {
622622

623623
@Test(expected = InvalidParameterValueException.class)
624624
public void validateProtocolTestProtocolNotIcmpWithIcmpConfigurations() {
625-
Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(1);
626-
Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(1);
625+
Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(2);
626+
Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(3);
627627

628628
Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("tcp");
629629
networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
@@ -647,8 +647,8 @@ public void validateProtocolTestProtocolNotIcmpWithSourcePorts() {
647647

648648
@Test
649649
public void validateProtocolTestProtocolIcmpWithIcmpConfigurations() {
650-
Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(1);
651-
Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(1);
650+
Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(2);
651+
Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(3);
652652

653653
Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(null);
654654
Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(null);

0 commit comments

Comments
 (0)