Dogface2k opened a new pull request, #13765:
URL: https://github.com/apache/cloudstack/pull/13765

   ### Description
   
   This PR stops the built-in `VPC offering with NSX - NAT Mode` offering from 
advertising Site-to-Site VPN through the NSX provider when the provider does 
not implement that service.
   
   The seeded offering currently maps `Vpn` to `Nsx`, but `NsxElement` does not 
implement `Site2SiteVpnServiceProvider`; the only production implementation is 
`VpcVirtualRouterElement`. `Site2SiteVpnManagerImpl` then selects a VPN 
endpoint through logic hard-coded to `VPCVirtualRouter`. On an NSX NAT-mode 
VPC, that legacy lookup sees both the NSX source-NAT IP and the system-VM 
helper source-NAT IP and fails with `Cannot found source nat ip of vpc <id>`, 
hiding the unsupported provider mapping.
   
   The changes align the advertised capability with the implementation:
   
   - new copies of the built-in NSX NAT offering omit the unsupported `Vpn/Nsx` 
mapping;
   - the 4.22.1.0-to-4.22.2.0 data migration removes only that mapping from the 
exact built-in offering and from VPC service snapshots created from it;
   - `createVpnGateway` now verifies that the VPC maps `Vpn` to 
`VPCVirtualRouter` before any IP lookup and returns a clear 
unsupported-capability error;
   - focused tests cover offering seeding, upgrade-path registration, migration 
scope/failure behavior, fail-fast validation, and both existing virtual-router 
IP-selection paths.
   
   This does not change the NSX VPC router public NIC, source-NAT IP 
allocation, custom VPC offerings, `Vpn/VPCVirtualRouter` mappings, or implement 
NSX-native Site-to-Site VPN.
   
   Fixes: #13764
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] Build/CI
   - [ ] Test (unit or integration test code)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [x] Major
   - [ ] Minor
   - [ ] Trivial
   
   ### Screenshots (if appropriate):
   
   Not applicable.
   
   ### How Has This Been Tested?
   
   The failure was reproduced on Apache CloudStack 4.22.1.0 in an NSX 
4.2.4-backed VMware environment through the public API. A VPC created from the 
built-in NSX NAT offering had `Vpn/Nsx` in its service snapshot. 
`createVpnGateway` reached the legacy exactly-one source-NAT lookup and failed 
with:
   
   ```text
   Request failed. (530)
   Cannot found source nat ip of vpc <id>
   ```
   
   Source and negative-control tests verified the same unsupported seed and 
pre-fix behavior on 4.22.0.0, 4.22.1.0, and main at 
`4f117071c9397b1e4714c8fb61c384883c872842`. On unmodified main, the new 
fail-fast regression test fails with the old `Cannot found source nat ip` 
exception while the other 45 focused tests pass. With the patch applied to 
main, all 47 focused tests pass.
   
   The actual patched `4.22` PR branch was tested with Java 17 and is green:
   
   ```text
   Schema and upgrade tests:
   Tests run: 21, Failures: 0, Errors: 0, Skipped: 0
   
   VPC and Site-to-Site VPN manager tests:
   Tests run: 21, Failures: 0, Errors: 0, Skipped: 0
   ```
   
   The Maven reactor built all 15 modules required by the schema test run and 
all 25 modules required by the server test run successfully, with zero 
Checkstyle violations.
   
   The data migration SQL was also executed against an isolated MariaDB 11.8 
schema. It removed the target built-in offering and VPC-snapshot `Vpn/Nsx` rows 
while preserving:
   
   - `Vpn/VPCVirtualRouter` mappings;
   - `SourceNat/Nsx` mappings;
   - `Vpn/Nsx` mappings on custom offerings;
   - all rows on transaction rollback.
   
   A committed run removed the two target rows, and a second run removed zero 
rows without error.
   
   ### How did you try to break this feature and the system with this change?
   
   The regression coverage and database checks exercise the boundaries 
introduced by this PR:
   
   - an NSX VPC without `Vpn/VPCVirtualRouter` is rejected before any public-IP 
lookup or gateway persistence;
   - a supported VPC with virtual-router VPN and virtual-router source NAT 
still uses its single source-NAT IP;
   - a supported VPC with virtual-router VPN but without virtual-router source 
NAT still uses the existing router-IP/static-NAT path;
   - the seeded NSX NAT service map still assigns every previously supported 
service to the same provider and omits only `Vpn`;
   - the upgrade path is registered only for 4.22.1.0 to 4.22.2.0;
   - migration errors fail the upgrade rather than leaving a partial silent 
result;
   - migration queries are parameterized, scoped by the exact built-in offering 
`unique_name`, ordered to remove VPC snapshots before the offering mapping, and 
idempotent.
   
   No generic exception handling, public-IP filtering workaround, router 
behavior change, or provider-specific branch was added to the VPN endpoint 
selector.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to