Skip to content

Commit 311afea

Browse files
author
Abhinandan Prateek
committed
bug 10731: reverting back source cidr changes for loadbalancer and portforwarding
1 parent e466f49 commit 311afea

17 files changed

Lines changed: 23 additions & 127 deletions

api/src/com/cloud/agent/api/to/LoadBalancerTO.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,20 @@ public class LoadBalancerTO {
2727
String srcIp;
2828
int srcPort;
2929
String protocol;
30-
List<String> sourceCidrs;
3130
String algorithm;
3231
boolean revoked;
3332
boolean alreadyAdded;
3433
DestinationTO[] destinations;
3534

3635

37-
public LoadBalancerTO (String srcIp, int srcPort, String protocol, List<String> sourceCidrs, String algorithm, boolean revoked, boolean alreadyAdded, List<LbDestination> destinations) {
36+
public LoadBalancerTO (String srcIp, int srcPort, String protocol, String algorithm, boolean revoked, boolean alreadyAdded, List<LbDestination> destinations) {
3837
this.srcIp = srcIp;
3938
this.srcPort = srcPort;
4039
this.protocol = protocol;
4140
this.algorithm = algorithm;
4241
this.revoked = revoked;
4342
this.alreadyAdded = alreadyAdded;
4443
this.destinations = new DestinationTO[destinations.size()];
45-
this.sourceCidrs = sourceCidrs;
4644
int i = 0;
4745
for (LbDestination destination : destinations) {
4846
this.destinations[i++] = new DestinationTO(destination.getIpAddress(), destination.getDestinationPortStart(), destination.isRevoked(), false);
@@ -59,14 +57,6 @@ public String getSrcIp() {
5957
public int getSrcPort() {
6058
return srcPort;
6159
}
62-
63-
public List<String> getSourceCidrs(){
64-
return sourceCidrs;
65-
}
66-
67-
public String getStringSourceCidrs(){
68-
return StringUtils.join(sourceCidrs, "-");
69-
}
7060

7161
public String getAlgorithm() {
7262
return algorithm;

api/src/com/cloud/agent/api/to/PortForwardingRuleTO.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
public class PortForwardingRuleTO extends FirewallRuleTO {
3434
String dstIp;
3535
int[] dstPortRange;
36-
List<String> sourceCidrs;
3736

3837
protected PortForwardingRuleTO() {
3938
super();
@@ -43,7 +42,6 @@ public PortForwardingRuleTO(PortForwardingRule rule, String srcIp) {
4342
super(rule, srcIp);
4443
this.dstIp = rule.getDestinationIpAddress().addr();
4544
this.dstPortRange = new int[] { rule.getDestinationPortStart(), rule.getDestinationPortEnd() };
46-
this.sourceCidrs = rule.getSourceCidrList();
4745
}
4846

4947
protected PortForwardingRuleTO(long id, String srcIp, int srcPortStart, int srcPortEnd, String dstIp, int dstPortStart, int dstPortEnd, String protocol, boolean revoked, boolean brandNew) {
@@ -64,12 +62,4 @@ public String getStringDstPortRange() {
6462
return NetUtils.portRangeToString(dstPortRange);
6563
}
6664

67-
public List<String> getSourceCidrs(){
68-
return sourceCidrs;
69-
}
70-
71-
public String getStringSourceCidrs(){
72-
return StringUtils.join(sourceCidrs, ",");
73-
}
74-
7565
}

api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,8 +264,4 @@ public Integer getIcmpType() {
264264
return null;
265265
}
266266

267-
@Override
268-
public List<String> getSourceCidrList() {
269-
return null;
270-
}
271267
}

api/src/com/cloud/api/commands/CreateLoadBalancerRuleCmd.java

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,6 @@ public class CreateLoadBalancerRuleCmd extends BaseAsyncCmd /*implements LoadBa
7272
@Parameter(name=ApiConstants.PUBLIC_PORT, type=CommandType.INTEGER, required=true, description="the public port from where the network traffic will be load balanced from")
7373
private Integer publicPort;
7474

75-
@Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, description = "the cidr list to forward traffic from")
76-
private List<String> cidrlist;
77-
7875
@Parameter(name = ApiConstants.OPEN_FIREWALL, type = CommandType.BOOLEAN, description = "if true, firewall rule for source/end pubic port is automatically created; if false - firewall rule has to be created explicitely. Has value true by default")
7976
private Boolean openFirewall;
8077

@@ -120,10 +117,6 @@ public Integer getPublicPort() {
120117
public String getName() {
121118
return loadBalancerRuleName;
122119
}
123-
124-
public List<String> getSourceCidrList() {
125-
return cidrlist;
126-
}
127120

128121
public Boolean getOpenFirewall() {
129122
if (openFirewall != null) {
@@ -143,15 +136,7 @@ public String getCommandName() {
143136
}
144137

145138
@Override
146-
public void execute() throws ResourceAllocationException, ResourceUnavailableException {
147-
if (cidrlist != null){
148-
for (String cidr: cidrlist){
149-
if (!NetUtils.isValidCIDR(cidr)){
150-
throw new ServerApiException(BaseCmd.PARAM_ERROR, "Source cidrs formatting error " + cidr);
151-
}
152-
}
153-
}
154-
139+
public void execute() throws ResourceAllocationException, ResourceUnavailableException {
155140
LoadBalancer result = null;
156141
try {
157142
result = _lbService.createLoadBalancerRule(this, getOpenFirewall());

api/src/com/cloud/network/lb/LoadBalancingRule.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,7 @@ public int getDefaultPortStart() {
6565
public int getDefaultPortEnd() {
6666
return lb.getDefaultPortEnd();
6767
}
68-
69-
@Override
70-
public List<String> getSourceCidrList() {
71-
return lb.getSourceCidrList();
72-
}
73-
68+
7469

7570
@Override
7671
public String getAlgorithm() {

api/src/com/cloud/network/rules/FirewallRule.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,5 +74,4 @@ enum State {
7474

7575
Integer getIcmpType();
7676

77-
List<String> getSourceCidrList();
7877
}

server/src/com/cloud/network/LoadBalancerVO.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ public class LoadBalancerVO extends FirewallRuleVO implements LoadBalancer {
5454
public LoadBalancerVO() {
5555
}
5656

57-
public LoadBalancerVO(String xId, String name, String description, long srcIpId, int srcPort, int dstPort, List<String> sourceCidrs, String algorithm, long networkId, long accountId, long domainId) {
58-
super(xId, srcIpId, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, sourceCidrs, null, null);
57+
public LoadBalancerVO(String xId, String name, String description, long srcIpId, int srcPort, int dstPort, String algorithm, long networkId, long accountId, long domainId) {
58+
super(xId, srcIpId, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, null, null);
5959
this.name = name;
6060
this.description = description;
6161
this.algorithm = algorithm;

server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -176,26 +176,6 @@ public List<FirewallRuleVO> listStaticNatByVmId(long vmId) {
176176
return listBy(sc);
177177
}
178178

179-
@Override @DB
180-
public FirewallRuleVO persist(FirewallRuleVO firewallRule) {
181-
Transaction txn = Transaction.currentTxn();
182-
txn.start();
183-
184-
FirewallRuleVO dbfirewallRule = super.persist(firewallRule);
185-
saveSourceCidrs(firewallRule);
186-
187-
txn.commit();
188-
return dbfirewallRule;
189-
}
190-
191-
192-
public void saveSourceCidrs(FirewallRuleVO firewallRule) {
193-
List<String> cidrlist = firewallRule.getSourceCidrList();
194-
if (cidrlist == null) {
195-
return;
196-
}
197-
_firewallRulesCidrsDao.persist(firewallRule.getId(), cidrlist);
198-
}
199179

200180
@Override
201181
public List<FirewallRuleVO> listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose) {

server/src/com/cloud/network/firewall/FirewallManagerImpl.java

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,13 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
9999
public FirewallRule createFirewallRule(FirewallRule rule) throws NetworkRuleConflictException {
100100
Account caller = UserContext.current().getCaller();
101101

102-
return createFirewallRule(rule.getSourceIpAddressId(), caller, rule.getXid(), rule.getSourcePortStart() ,rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), rule.getIcmpCode(), rule.getIcmpType());
102+
return createFirewallRule(rule.getSourceIpAddressId(), caller, rule.getXid(), rule.getSourcePortStart() ,rule.getSourcePortEnd(), rule.getProtocol(), rule.getIcmpCode(), rule.getIcmpType());
103103
}
104104

105105
@DB
106106
@Override
107107
@ActionEvent(eventType = EventTypes.EVENT_FIREWALL_OPEN, eventDescription = "creating firewll rule", create = true)
108-
public FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart,Integer portEnd, String protocol, List<String> sourceCidrList, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException{
108+
public FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart,Integer portEnd, String protocol, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException{
109109
IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId);
110110

111111
// Validate ip address
@@ -128,7 +128,7 @@ public FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId
128128
Transaction txn = Transaction.currentTxn();
129129
txn.start();
130130

131-
FirewallRuleVO newRule = new FirewallRuleVO (xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountId, domainId, Purpose.Firewall, sourceCidrList, icmpCode, icmpType);
131+
FirewallRuleVO newRule = new FirewallRuleVO (xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountId, domainId, Purpose.Firewall, icmpCode, icmpType);
132132
newRule = _firewallDao.persist(newRule);
133133

134134
detectRulesConflict(newRule, ipAddress);
@@ -334,12 +334,6 @@ public boolean applyFirewallRules(List<FirewallRuleVO> rules, boolean continueOn
334334
return true;
335335
}
336336

337-
for (FirewallRuleVO rule: rules){
338-
// load cidrs if any
339-
rule.setSourceCidrList(_firewallCidrsDao.getSourceCidrs(rule.getId()));
340-
}
341-
342-
343337
if (caller != null) {
344338
_accountMgr.checkAccess(caller, rules.toArray(new FirewallRuleVO[rules.size()]));
345339
}
@@ -463,10 +457,7 @@ public FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer
463457
if (!rules.isEmpty()) {
464458
return rules.get(0);
465459
}
466-
467-
List<String> oneCidr = new ArrayList<String>();
468-
oneCidr.add(NetUtils.ALL_CIDRS);
469-
return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, oneCidr, icmpCode, icmpType);
460+
return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, icmpCode, icmpType);
470461
}
471462

472463
@Override

server/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,7 @@ private void createApplyLoadBalancingRulesCommands(
284284
.addr();
285285
int srcPort = rule.getSourcePortStart();
286286
List<LbDestination> destinations = rule.getDestinations();
287-
LoadBalancerTO lb = new LoadBalancerTO(elbIp, srcPort, protocol, null,
288-
algorithm, revoked, false, destinations);
287+
LoadBalancerTO lb = new LoadBalancerTO(elbIp, srcPort, protocol, algorithm, revoked, false, destinations);
289288
lbs[i++] = lb;
290289
}
291290

0 commit comments

Comments
 (0)