Copilot commented on code in PR #13032:
URL: https://github.com/apache/cloudstack/pull/13032#discussion_r3528550155
##########
ui/src/views/infra/network/ServiceProvidersTab.vue:
##########
@@ -1148,6 +1171,51 @@ export default {
return
}
this.fetchServiceProvider()
+ this.fetchRegisteredExtensions()
+ },
+ fetchRegisteredExtensions () {
+ // Load NetworkOrchestrator extensions registered to this physical
network
+ getAPI('listExtensions', {
+ type: 'NetworkOrchestrator',
+ resourceid: this.resource.id,
+ resourcetype: 'PhysicalNetwork'
+ }).then(json => {
+ this.registeredExtensions = (json.listextensionsresponse &&
json.listextensionsresponse.extension) || []
+ }).catch(() => {
+ this.registeredExtensions = []
+ })
Review Comment:
`fetchRegisteredExtensions()` calls `listExtensions` with
`resourceid`/`resourcetype`, but `ListExtensionsCmd` only defines `name`, `id`,
`details`, and `type`. CloudStack API commands generally reject unknown
parameters, so this request is likely to fail and the dynamic extension tabs
will never populate. Consider listing extensions by type and filtering
client-side using the returned `resources` list (or add explicit
`resourceid`/`resourcetype` parameters to the API).
##########
ui/src/views/offering/AddNetworkOffering.vue:
##########
@@ -739,6 +739,13 @@ export default {
isSupportedServiceObject (obj) {
return (obj !== null && obj !== undefined && Object.keys(obj).length > 0
&& obj.constructor === Object && 'provider' in obj)
},
+ isVpcCoreProvider (providerName) {
+ return ['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf',
'ConfigDrive'].includes(providerName)
+ },
+ isDynamicExtensionProvider (providerName) {
+ const knownProviders = ['VirtualRouter', 'VpcVirtualRouter',
'InternalLbVm', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive', 'Nsx', 'Netris']
+ return !knownProviders.includes(providerName)
+ },
Review Comment:
`isDynamicExtensionProvider()` currently treats *any* provider not in a
short hardcoded list as an extension provider. That will misclassify many
built-in CloudStack providers (e.g. JuniperContrailVpcRouter, PaloAlto,
Opendaylight, Tungsten, etc.) as “dynamic extensions” and enable them in the
VPC provider selection, which contradicts the intent expressed in the comment
above and can lead to invalid offering configurations.
##########
ui/src/views/offering/AddVpcOffering.vue:
##########
@@ -431,6 +431,13 @@ export default {
this.zoneLoading = false
})
},
+ isVpcCoreProvider (providerName) {
+ return ['VpcVirtualRouter', 'Netscaler', 'BigSwitchBcf',
'ConfigDrive'].includes(providerName)
+ },
+ isDynamicExtensionProvider (providerName) {
+ const knownProviders = ['VirtualRouter', 'VpcVirtualRouter',
'InternalLbVm', 'Netscaler', 'BigSwitchBcf', 'ConfigDrive', 'Nsx', 'Netris']
+ return !knownProviders.includes(providerName)
+ },
Review Comment:
`isDynamicExtensionProvider()` currently assumes any provider not in a short
list is an extension provider. CloudStack has many built-in providers beyond
that list (e.g. JuniperContrailVpcRouter, PaloAlto, Opendaylight, Tungsten,
etc.), so they would be incorrectly enabled as “dynamic extensions” during VPC
offering service/provider selection.
--
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]