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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
