[
https://issues.apache.org/jira/browse/CLOUDSTACK-10207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16447679#comment-16447679
]
ASF GitHub Bot commented on CLOUDSTACK-10207:
---------------------------------------------
resmo closed pull request #2377: CLOUDSTACK-10207: updateVpnCustomerGateway:
fix defaulting for option…
URL: https://github.com/apache/cloudstack/pull/2377
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/server/src/com/cloud/network/vpn/Site2SiteVpnManagerImpl.java
b/server/src/com/cloud/network/vpn/Site2SiteVpnManagerImpl.java
index aebc8717485..8717a6a38f8 100644
--- a/server/src/com/cloud/network/vpn/Site2SiteVpnManagerImpl.java
+++ b/server/src/com/cloud/network/vpn/Site2SiteVpnManagerImpl.java
@@ -192,7 +192,7 @@ public Site2SiteCustomerGateway
createCustomerGateway(CreateVpnCustomerGatewayCm
String ikePolicy = cmd.getIkePolicy();
String espPolicy = cmd.getEspPolicy();
if (!NetUtils.isValidS2SVpnPolicy("ike", ikePolicy)) {
- throw new InvalidParameterValueException("The customer gateway IKE
policy " + ikePolicy + " is invalid! Verify the required Diffie Hellman (DH)
group is specified.");
+ throw new InvalidParameterValueException("Error while creating a
customer gateway. The IKE policy " + ikePolicy + " is invalid! Verify whether
the required Diffie Hellman (DH) group is specified.");
}
if (!NetUtils.isValidS2SVpnPolicy("esp", espPolicy)) {
throw new InvalidParameterValueException("The customer gateway ESP
policy " + espPolicy + " is invalid!");
@@ -427,73 +427,103 @@ public Site2SiteCustomerGateway
updateCustomerGateway(UpdateVpnCustomerGatewayCm
}
}
}
- String name = cmd.getName();
String gatewayIp = cmd.getGatewayIp();
-
- if (!NetUtils.isValidIp4(gatewayIp) &&
!NetUtils.verifyDomainName(gatewayIp)) {
- throw new InvalidParameterValueException("The customer gateway
ip/Domain " + gatewayIp + " is invalid!");
- }
- if (name == null) {
- name = "VPN-" + gatewayIp;
+ if (gatewayIp != null) {
+ if (!NetUtils.isValidIp(gatewayIp) &&
!NetUtils.verifyDomainName(gatewayIp)) {
+ throw new InvalidParameterValueException("The customer gateway
IP/domain " + gatewayIp + " is invalid!");
+ }
+ gw.setGatewayIp(gatewayIp);
+ } else {
+ // TODO: Change API to make this param optional on update
+ throw new InvalidParameterValueException("The customer gateway
IP/domain param is required");
}
+
String guestCidrList = cmd.getGuestCidrList();
- if (!NetUtils.isValidCidrList(guestCidrList)) {
- throw new InvalidParameterValueException("The customer gateway
peer cidr list " + guestCidrList + " contains an invalid cidr!");
+ if (guestCidrList != null) {
+ if (!NetUtils.isValidCidrList(guestCidrList)) {
+ throw new InvalidParameterValueException("The customer gateway
peer cidr list " + guestCidrList + " contains an invalid cidr!");
+ }
+ checkCustomerGatewayCidrList(guestCidrList);
+ gw.setGuestCidrList(guestCidrList);
+ } else {
+ // TODO: Change API to make this param optional on update
+ throw new InvalidParameterValueException("The customer gateway
peer cidr list param is required");
}
- String ipsecPsk = cmd.getIpsecPsk();
+
String ikePolicy = cmd.getIkePolicy();
- String espPolicy = cmd.getEspPolicy();
- if (!NetUtils.isValidS2SVpnPolicy("ike", ikePolicy)) {
- throw new InvalidParameterValueException("The customer gateway IKE
policy" + ikePolicy + " is invalid! Verify the required Diffie Hellman (DH)
group is specified.");
- }
- if (!NetUtils.isValidS2SVpnPolicy("esp", espPolicy)) {
- throw new InvalidParameterValueException("The customer gateway ESP
policy" + espPolicy + " is invalid!");
+ if (ikePolicy != null) {
+ if (!NetUtils.isValidS2SVpnPolicy("ike", ikePolicy)) {
+ throw new InvalidParameterValueException("Error while updating
a customer gateway. The IKE policy" + ikePolicy + " is invalid! Verify whether
the required Diffie Hellman (DH) group is specified.");
+ }
+ gw.setIkePolicy(ikePolicy);
+ } else {
+ // TODO: Change API to make this param optional on update
+ throw new InvalidParameterValueException("The customer gateway IKE
policy param is required");
}
+
+ String espPolicy = cmd.getEspPolicy();
+ if (espPolicy != null) {
+ if (!NetUtils.isValidS2SVpnPolicy("esp", espPolicy)) {
+ throw new InvalidParameterValueException("The customer gateway
ESP policy" + espPolicy + " is invalid!");
+ }
+ gw.setEspPolicy(espPolicy);
+ } else {
+ // TODO: Change API to make this param optional on update
+ throw new InvalidParameterValueException("The customer gateway
ESP policy param is required");
+ }
+
Long ikeLifetime = cmd.getIkeLifetime();
- if (ikeLifetime == null) {
- // Default value of lifetime is 1 day
- ikeLifetime = (long)86400;
- }
- if (ikeLifetime > 86400) {
- throw new InvalidParameterValueException("The IKE lifetime " +
ikeLifetime + " of vpn connection is invalid!");
+ if (ikeLifetime != null) {
+ if (ikeLifetime > 86400) {
+ throw new InvalidParameterValueException("The IKE lifetime " +
ikeLifetime + " of vpn connection is invalid!");
+ }
+ gw.setIkeLifetime(ikeLifetime);
}
+
Long espLifetime = cmd.getEspLifetime();
- if (espLifetime == null) {
- // Default value of lifetime is 1 hour
- espLifetime = (long)3600;
+ if (espLifetime != null {
+ if espLifetime > 86400) {
+ throw new InvalidParameterValueException("The ESP lifetime " +
espLifetime + " of vpn connection is invalid!");
+ }
+ gw.setEspLifetime(espLifetime);
}
- if (espLifetime > 86400) {
- throw new InvalidParameterValueException("The ESP lifetime " +
espLifetime + " of vpn connection is invalid!");
+
+ Long espLifetime = cmd.getEspLifetime();
+ if (espLifetime != null {
+ if espLifetime > 86400) {
+ throw new InvalidParameterValueException("The ESP lifetime " +
espLifetime + " of vpn connection is invalid!");
+ }
+ gw.setEspLifetime(espLifetime);
}
- Boolean dpd = cmd.getDpd();
- if (dpd == null) {
- dpd = false;
+ String ipsecPsk = cmd.getIpsecPsk();
+ if (ipsecPsk != null) {
+ gw.setIpsecPsk(ipsecPsk);
+ } else {
+ // TODO: Change API to make this param optional on update
+ throw new InvalidParameterValueException("The customer gateway
IPSec PSK param is required");
}
- Boolean encap = cmd.getEncap();
- if (encap == null) {
- encap = false;
+ String name = cmd.getName();
+ if (name != null) {
+ long accountId = gw.getAccountId();
+ Site2SiteCustomerGatewayVO existedGw =
_customerGatewayDao.findByNameAndAccountId(name, accountId);
+ if (existedGw != null && existedGw.getId() != gw.getId()) {
+ throw new InvalidParameterValueException("A customer gateway
with name " + name + " already exists. Please choose a different name.");
+ }
+ gw.setName(name);
}
- checkCustomerGatewayCidrList(guestCidrList);
+ Boolean dpd = cmd.getDpd();
+ if (dpd != null) {
+ gw.setDpd(dpd);
+ }
- long accountId = gw.getAccountId();
- Site2SiteCustomerGatewayVO existedGw =
_customerGatewayDao.findByNameAndAccountId(name, accountId);
- if (existedGw != null && existedGw.getId() != gw.getId()) {
- throw new InvalidParameterValueException("The customer gateway
with name " + name + " already existed!");
+ Boolean encap = cmd.getEncap();
+ if (encap != null) {
+ gw.setEncap(encap);
}
- gw.setName(name);
- gw.setGatewayIp(gatewayIp);
- gw.setGuestCidrList(guestCidrList);
- gw.setIkePolicy(ikePolicy);
- gw.setEspPolicy(espPolicy);
- gw.setIpsecPsk(ipsecPsk);
- gw.setIkeLifetime(ikeLifetime);
- gw.setEspLifetime(espLifetime);
- gw.setDpd(dpd);
- gw.setEncap(encap);
_customerGatewayDao.persist(gw);
return gw;
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> updateVpnCustomerGateway: unexpected changes on optional keys
> -------------------------------------------------------------
>
> Key: CLOUDSTACK-10207
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10207
> Project: CloudStack
> Issue Type: Bug
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Components: API
> Reporter: René Moser
> Assignee: René Moser
> Priority: Minor
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
