AlinsRan commented on code in PR #2878:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2878#discussion_r4216939360
##########
internal/adc/translator/gateway.go:
##########
@@ -237,11 +241,12 @@ func (t *Translator) translateFrontendValidation(tctx
*provider.TranslateContext
}
// fillPluginsFromGatewayProxy fill plugins from GatewayProxy to given plugins
-func (t *Translator) fillPluginsFromGatewayProxy(plugins adctypes.GlobalRule,
gatewayProxy *v1alpha1.GatewayProxy) {
+func (t *Translator) fillPluginsFromGatewayProxy(plugins adctypes.GlobalRule,
gatewayProxy *v1alpha1.GatewayProxy) error {
if gatewayProxy == nil {
- return
+ return nil
}
+ translated := make(adctypes.GlobalRule)
Review Comment:
nit: the `translated` staging map (here and in
`fillPluginMetadataFromGatewayProxy`) doesn't protect anything with the current
callers. Both `TranslateGateway` and `TranslateIngressClass` pass a map they
just created and drop it on error:
```go
globalRules := make(adctypes.GlobalRule)
pluginMetadata := make(adctypes.PluginMetadata)
if err := t.fillPluginsFromGatewayProxy(globalRules, &gatewayProxy); err !=
nil {
return nil, err
}
if err := t.fillPluginMetadataFromGatewayProxy(pluginMetadata,
&gatewayProxy); err != nil {
return nil, err
}
result.GlobalRules = globalRules
```
A partially filled `globalRules` / `pluginMetadata` can't be seen by anyone,
so writing straight into `plugins` / `pluginMetadata` gives the same result
with less code. If the "leave the input untouched on error" guarantee is meant
for future callers, a short note in the doc comment would make that explicit.
--
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]
