You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "shreemaan-abhishek (via GitHub)" <gi...@apache.org> on 2026/09/16 11:42:11 UTC

[PR] fix: reject invalid GatewayProxy plugin configuration [apisix-ingress-controller]

shreemaan-abhishek opened a new pull request, #2878:
URL: https://github.com/apache/apisix-ingress-controller/pull/2878

   ### Type of change:
   
   - [x] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   - [ ] Documentation
   - [ ] Refactor
   - [ ] Chore
   - [ ] CI/CD or Tests
   
   ### What this PR does / why we need it:
   
   GatewayProxy plugin configurations and plugin metadata must translate to JSON objects. Previously, values with other shapes were logged and omitted while translation continued.
   
   This change returns translation errors for those values and propagates them through both Gateway and IngressClass translation before the provider applies translated resources. The current configuration therefore remains unchanged after an invalid update.
   
   Part of #2842
   
   ### Pre-submission checklist:
   
   - [x] Did you explain what problem does this PR solve? Or what new features have been added?
   - [x] Have you added corresponding test cases?
   - [ ] Have you modified the corresponding document?
   - [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix-ingress-controller#community) first**


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: reject invalid GatewayProxy plugin configuration [apisix-ingress-controller]

Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek merged PR #2878:
URL: https://github.com/apache/apisix-ingress-controller/pull/2878


-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: reject invalid GatewayProxy plugin configuration [apisix-ingress-controller]

Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org