lingsamuel commented on code in PR #1486:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/1486#discussion_r1039088293


##########
test/e2e/suite-plugins/suite-plugins-general/echo.go:
##########
@@ -57,6 +57,62 @@ spec:
          X-Foo: v1
          X-Foo2: v2
        
+`, backendSvc, backendPorts[0])
+
+                       assert.Nil(ginkgo.GinkgoT(), 
s.CreateVersionedApisixResource(ar))
+
+                       err := s.EnsureNumApisixUpstreamsCreated(1)
+                       assert.Nil(ginkgo.GinkgoT(), err, "Checking number of 
upstreams")
+                       err = s.EnsureNumApisixRoutesCreated(1)
+                       assert.Nil(ginkgo.GinkgoT(), err, "Checking number of 
routes")
+
+                       resp := 
s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.org").Expect()
+                       resp.Status(http.StatusOK)
+                       resp.Header("X-Foo").Equal("v1")
+                       resp.Header("X-Foo2").Equal("v2")
+                       resp.Body().Contains("This is the preface")

Review Comment:
   Any way to check if it actually appears before the later one?



##########
pkg/kube/apisix/apis/config/v2beta3/types.go:
##########
@@ -161,6 +161,8 @@ type ApisixRouteHTTPPlugin struct {
        Enable bool `json:"enable" yaml:"enable"`
        // Plugin configuration.
        Config ApisixRouteHTTPPluginConfig `json:"config" yaml:"config"`
+       // Plugin configuration secretRef.

Review Comment:
   We should have a document/field comment that explains how the priority work 
if this conflicts with the Config fields.



##########
pkg/kube/apisix/apis/config/v2beta3/types.go:
##########
@@ -161,6 +161,8 @@ type ApisixRouteHTTPPlugin struct {
        Enable bool `json:"enable" yaml:"enable"`
        // Plugin configuration.
        Config ApisixRouteHTTPPluginConfig `json:"config" yaml:"config"`
+       // Plugin configuration secretRef.
+       SecretConfig string `json:"secretConfig" yaml:"secretConfig"`

Review Comment:
   Are we still adding features for v2beta3? @tao12345666333 



##########
pkg/providers/apisix/translation/apisix_pluginconfig.go:
##########
@@ -42,6 +42,11 @@ func (t *translator) TranslatePluginConfigV2beta3(config 
*configv2beta3.ApisixPl
                                                zap.Any("new", plugin.Config),
                                        )
                                }
+                               if sec, err := 
t.SecretLister.Secrets(config.Namespace).Get(plugin.SecretConfig); err == nil {

Review Comment:
   Why are errors being ignored here? There is no way for the user to know why 
their configuration (although wrong) is being ignored. At least we should print 
a log or add an error message to the sync status field.



-- 
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