Re: [I] BGP APIs hidden by flag named `routed.network.vpc.enabled`, return misleading 'not found' error [cloudstack]

["Dogface2k (via GitHub)" <gi...@apache.org>]

Dogface2k commented on issue #13745:
URL: https://github.com/apache/cloudstack/issues/13745#issuecomment-5200137928

   I traced this through the current `4.22` branch and back through the change that introduced `routed.network.vpc.enabled`.
   
   The issue is confirmed.
   
   `RoutedNetworkVpcEnabled` is declared as a dynamic, `Scope.Zone` configuration. However, `RoutedIpv4ManagerImpl.getCommands()` calls `RoutedNetworkVpcEnabled.value()`, which resolves only the global configuration value. `ApiServer` builds its API command map from each `PluggableService.getCommands()` result, so when the global value is false these commands are never registered and execution never reaches any of the existing zone-aware checks.
   
   This also makes the following valid scope combination ineffective:
   
   - global value: `false`
   - zone override: `true`
   
   `valueIn(zoneId)` would correctly resolve true for that zone, but the APIs have already been removed globally during command discovery.
   
   The affected surface on current `4.22` is 26 commands, not approximately 42. The existing `RoutedIpv4ManagerImplTest.testGetCommands()` confirms that count, but it also currently asserts that the list becomes empty when the global default is false, so the test is explicitly preserving the defective behaviour.
   
   History confirms this is a registration regression introduced by #9697. Immediately before that change, the 21 routed-subnet/BGP/routing-firewall commands were always returned by `RoutedIpv4ManagerImpl`, while the five ASN commands were always registered by `ManagementServerImpl`. #9697 grouped the 26 commands and placed them behind the global-value check while defining the setting itself as zone-scoped.
   
   For the 4.22.2 fix, I believe the safe and contained scope is:
   
   1. Register all 26 commands unconditionally.
   2. Keep feature enforcement at the existing zone-aware execution/provisioning paths.
   3. Make disabled-zone errors name `routed.network.vpc.enabled` and the affected zone.
   4. Clarify that the setting controls ROUTED-mode/static-or-dynamic-routing resources; NATTED networks and VPCs are unaffected.
   5. Update `testGetCommands()` to expect the exact 26-command set regardless of the global value, and add coverage for:
      - global false + zone true;
      - global true + zone false;
      - stable `listApis` discovery;
      - useful zone-specific execution errors.
   
   I would not add blanket checks to all 26 operations in this bug fix, because list/delete/release and maintenance of existing routed resources must remain available when provisioning is disabled. Likewise, renaming or splitting the key should remain a separate change because it needs a configuration-migration and compatibility design.
   
   This can therefore be fixed cleanly on `4.22` without a database migration or unrelated refactor.


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org