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]

Reply via email to