toolmanwyj opened a new pull request, #10224:
URL: https://github.com/apache/cloudstack/pull/10224
### Description
In CloudStack, the security group selection box is displayed when editing a
VM, regardless of whether the zone supports security groups or not. This causes
the `securitygroupids` parameter to be sent with the request even if no
security group is selected. If the zone is of type `NetworkType.Advanced` and
does not support security groups, this triggers a `NullPointerException` in the
`UserVmManagerImpl.updateVirtualMachine` method when calling
`isSecurityGroupSupportedInNetwork`.
Upon investigation, it was found that the `listZones` API call, which is
used to fetch the zone details, incorrectly uses `zoneid` instead of `id` for
the zone identifier. This prevents the correct zone information from being
retrieved. Additionally, since the `listZones` API does not properly filter
zones, it fetches all zones and always takes the first one in the list (index
0) to determine whether security groups are enabled. Consequently, if that
first zone supports security groups, the security group selection box is
displayed in the UI—even if the zone actually used by the VM does not support
them.
Moreover, the logic for checking whether the security group selection box is
empty is not strict enough, which causes the `securitygroupids` parameter to be
included in the request even when the selection box is empty.
#### Proposed Solution
1. Correct the parameter name for the zone identifier when calling the
`listZones` API by using `id` instead of `zoneid`.
2. Filter the zones correctly in the `listZones` API call to ensure that
only the relevant zone is considered when checking for security group support.
3. Improve the logic for checking if the security group selection box is
empty before attaching the `securitygroupids` parameter to the request.
#### Modified Files
- `EditVM.vue`
#### PR Changes
- Fix incorrect usage of `zoneid` in `listZones` calls to properly retrieve
the VM’s zone details.
- Only consider the matching zone (instead of just taking the first item) to
determine whether security groups are enabled.
- Enhance the empty-check logic for the security group selection, preventing
`securitygroupids` from being included if no security group is actually
selected.
- Prevent a `NullPointerException` when editing VMs in
`NetworkType.Advanced` zones that do not support security groups.
### 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
- [ ] Major
- [x] Minor
- [ ] Trivial
### How Has This Been Tested?
1. **Local Dev Environment**
- Configured multiple zones: one supporting security groups and one not
supporting security groups (VPC).
- Edited a VM in each zone to confirm that:
1. When the zone supports SG, the security group selection is displayed
and functions correctly.
2. When the zone does not support SG, no security group selection is
displayed or an empty list is properly handled, and no `securitygroupids` is
sent to the backend.
- Verified no `NullPointerException` is thrown even if `securitygroupids`
is `null` or empty.
2. **UI Validation**
- Checked that the modified logic in `EditVM.vue` properly filters the
correct zone from `listZones`.
- Ensured that if the user does not select a security group, the request
does not include `securitygroupids`.
#### How did you try to break this feature and the system with this change?
- Attempted to edit a VM in a zone that does not support SG and artificially
forced the frontend to pass `securitygroupids`, verifying that no NPE occurs.
- Made sure any new changes do not affect zones where security groups are
legitimately supported.
--
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]
