Skip to content

CLOUDSTACK-9905:VPN Gateway with Public Subnet#2086

Merged
karuturi merged 1 commit intomasterfrom
unknown repository
Jun 6, 2017
Merged

CLOUDSTACK-9905:VPN Gateway with Public Subnet#2086
karuturi merged 1 commit intomasterfrom
unknown repository

Conversation

@mrunalinikankariya
Copy link
Contributor

@mrunalinikankariya mrunalinikankariya commented May 8, 2017

CLOUDSTACK-9905 : VPN Gateway with Public Subnet

When we attempt to use a /24 subnet with a public IP ranges, for example,153.97.140.0/24. VPN Customer Gateways can be created with this type of CIDR, but cannot be updated, for example to 153.97.181.0/24 . Attempting to do so produces the error "The customer gateway cidr list 153.97.181.0/24 contains invalid guest cidr!"

REPRO STEPS

Created a new VPN Customer Gateway
Attempted to change the CIDR list entry from 153.97.180.0/24 to 153.97.181.0/24
The UI became unresponsive
The Management-server log shows the following:
2017-03-31 17:10:42,471 WARN [c.c.u.n.NetUtils] (API-Job-Executor-9:ctx-ed9b5816 job-172 ctx-32369258) (logid:3a16f24b) cidr 153.97.181.0/24 is not RFC 1918 compliant
153.97.181.0/24

EXPECTED BEHAVIOR

==================
Users should be able to update existing VPN Customer Gateway CIDR list as needed
Resolution

Removed RFC 1918 compliant check during updation of VPN Customer Gateway CIDR

@PranaliM
Copy link
Contributor

Manually tested the change.

Steps for defect replication :

  1. Added a VPN Customer Gateway in the pubic IP range- It got added successfully.
  2. Then tried editing the CIDR, which resulted in an error: cidr is not RFC 1918 compliant

After applying the fix, could edit the CIDR successfully.

Test LGTM based on manual testing results

@remibergsma
Copy link
Contributor

We did the same in our fork, works fine. LGTM based on code review.

@mrunalinikankariya
Copy link
Contributor Author

tag:This is Ready to Merge

name = "VPN-" + gatewayIp;
}
String guestCidrList = cmd.getGuestCidrList();
if (!NetUtils.validateGuestCidrList(guestCidrList)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrunalinikankariya we need to check for the isValidCIDR cidr here. If you look at validateGuestCidr method it is checking for the isValidCIDR first then checking for RFC compliant.

Also please squash the commits into single commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree...Will update

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jayapal,
I have update code accordingly, can you please check

@jayapalu
Copy link
Contributor

LGTM
@karuturi This is small fix, it has manual test results and working in production.
It seems CI has not triggered in it. I think it can be tagged merge ready.

@karuturi karuturi merged commit 6f5bc89 into apache:master Jun 6, 2017
winterhazel pushed a commit that referenced this pull request Jan 28, 2026
Adição das variáveis de conta e domínio para _tags_ flexíveis

Closes #2086, #2688, and #3140

See merge request scclouds/scclouds!918
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants