You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by zh...@apache.org on 2022/03/02 13:18:43 UTC
[apisix-ingress-controller] branch master updated: fix: avoid create pluginconfig in the tranlsation of route (#845)
This is an automated email from the ASF dual-hosted git repository.
zhangjintao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new 5f6a7c1 fix: avoid create pluginconfig in the tranlsation of route (#845)
5f6a7c1 is described below
commit 5f6a7c10b97c5eb575d23140db433bc8dfe60a2c
Author: nevercase <10...@qq.com>
AuthorDate: Wed Mar 2 21:18:08 2022 +0800
fix: avoid create pluginconfig in the tranlsation of route (#845)
---
pkg/ingress/apisix_route.go | 44 +++++++--------
pkg/kube/translation/apisix_route.go | 28 ++--------
pkg/kube/translation/apisix_route_test.go | 93 +++++++++++++++++++++++++++++--
test/e2e/ingress/resourcepushing.go | 2 +-
4 files changed, 113 insertions(+), 54 deletions(-)
diff --git a/pkg/ingress/apisix_route.go b/pkg/ingress/apisix_route.go
index 7b6ed53..5f62b76 100644
--- a/pkg/ingress/apisix_route.go
+++ b/pkg/ingress/apisix_route.go
@@ -25,6 +25,7 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
+ apisixcache "github.com/apache/apisix-ingress-controller/pkg/apisix/cache"
"github.com/apache/apisix-ingress-controller/pkg/kube"
"github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2beta3"
"github.com/apache/apisix-ingress-controller/pkg/kube/translation"
@@ -92,9 +93,8 @@ func (c *apisixRouteController) sync(ctx context.Context, ev *types.Event) error
return err
}
var (
- ar kube.ApisixRoute
- replaced *v2beta3.ApisixRoute
- tctx *translation.TranslateContext
+ ar kube.ApisixRoute
+ tctx *translation.TranslateContext
)
switch obj.GroupVersion {
case kube.ApisixRouteV2beta1:
@@ -164,8 +164,8 @@ func (c *apisixRouteController) sync(ctx context.Context, ev *types.Event) error
}
case kube.ApisixRouteV2beta3:
if ev.Type != types.EventDelete {
- if replaced, err = c.replacePluginNameWithIdIfNotEmptyV2beta3(ctx, ar.V2beta3()); err == nil {
- tctx, err = c.controller.translator.TranslateRouteV2beta3(replaced)
+ if err = c.checkPluginNameIfNotEmptyV2beta3(ctx, ar.V2beta3()); err == nil {
+ tctx, err = c.controller.translator.TranslateRouteV2beta3(ar.V2beta3())
}
} else {
tctx, err = c.controller.translator.TranslateRouteV2beta3NotStrictly(ar.V2beta3())
@@ -211,9 +211,7 @@ func (c *apisixRouteController) sync(ctx context.Context, ev *types.Event) error
case kube.ApisixRouteV2beta2:
oldCtx, err = c.controller.translator.TranslateRouteV2beta2(obj.OldObject.V2beta2())
case kube.ApisixRouteV2beta3:
- if replaced, err = c.replacePluginNameWithIdIfNotEmptyV2beta3(ctx, obj.OldObject.V2beta3()); err == nil {
- oldCtx, err = c.controller.translator.TranslateRouteV2beta3(replaced)
- }
+ oldCtx, err = c.controller.translator.TranslateRouteV2beta3(obj.OldObject.V2beta3())
}
if err != nil {
log.Errorw("failed to translate old ApisixRoute",
@@ -237,27 +235,27 @@ func (c *apisixRouteController) sync(ctx context.Context, ev *types.Event) error
return c.controller.syncManifests(ctx, added, updated, deleted)
}
-func (c *apisixRouteController) replacePluginNameWithIdIfNotEmptyV2beta3(ctx context.Context, in *v2beta3.ApisixRoute) (*v2beta3.ApisixRoute, error) {
- clusterName := c.controller.cfg.APISIX.DefaultClusterName
- news := make([]v2beta3.ApisixRouteHTTP, 0)
+func (c *apisixRouteController) checkPluginNameIfNotEmptyV2beta3(ctx context.Context, in *v2beta3.ApisixRoute) error {
for _, v := range in.Spec.HTTP {
- pluginConfigId := ""
if v.PluginConfigName != "" {
- pc, err := c.controller.apisix.Cluster(clusterName).PluginConfig().Get(ctx, apisixv1.ComposePluginConfigName(in.Namespace, v.PluginConfigName))
+ _, err := c.controller.apisix.Cluster(c.controller.cfg.APISIX.DefaultClusterName).PluginConfig().Get(ctx, apisixv1.ComposePluginConfigName(in.Namespace, v.PluginConfigName))
if err != nil {
- log.Errorw("replacePluginNameWithIdIfNotEmptyV2beta3 error: plugin_config not found",
- zap.String("name", apisixv1.ComposePluginConfigName(in.Namespace, v.PluginConfigName)),
- zap.Any("obj", in),
- zap.Error(err))
- } else {
- pluginConfigId = pc.ID
+ if err == apisixcache.ErrNotFound {
+ log.Errorw("checkPluginNameIfNotEmptyV2beta3 error: plugin_config not found",
+ zap.String("name", apisixv1.ComposePluginConfigName(in.Namespace, v.PluginConfigName)),
+ zap.Any("obj", in),
+ zap.Error(err))
+ } else {
+ log.Errorw("checkPluginNameIfNotEmptyV2beta3 PluginConfig get failed",
+ zap.String("name", apisixv1.ComposePluginConfigName(in.Namespace, v.PluginConfigName)),
+ zap.Any("obj", in),
+ zap.Error(err))
+ }
+ return err
}
}
- v.PluginConfigName = pluginConfigId
- news = append(news, v)
}
- in.Spec.HTTP = news
- return in, nil
+ return nil
}
func (c *apisixRouteController) handleSyncErr(obj interface{}, errOrigin error) {
diff --git a/pkg/kube/translation/apisix_route.go b/pkg/kube/translation/apisix_route.go
index 8e92485..cb0df7d 100644
--- a/pkg/kube/translation/apisix_route.go
+++ b/pkg/kube/translation/apisix_route.go
@@ -530,17 +530,8 @@ func (t *translator) translateHTTPRouteV2beta3(ctx *TranslateContext, ar *config
route.EnableWebsocket = part.Websocket
route.Plugins = pluginMap
route.Timeout = timeout
- pluginConfig := apisixv1.NewDefaultPluginConfig()
- if len(pluginMap) > 0 {
- pluginConfigName := apisixv1.ComposePluginConfigName(ar.Namespace, backend.ServiceName)
- route.PluginConfigId = id.GenID(pluginConfigName)
- pluginConfig.ID = route.PluginConfigId
- pluginConfig.Name = pluginConfigName
- pluginConfig.Plugins = pluginMap
- } else {
- if part.PluginConfigName != "" {
- route.PluginConfigId = part.PluginConfigName
- }
+ if part.PluginConfigName != "" {
+ route.PluginConfigId = id.GenID(apisixv1.ComposePluginConfigName(ar.Namespace, part.PluginConfigName))
}
if len(backends) > 0 {
@@ -566,9 +557,6 @@ func (t *translator) translateHTTPRouteV2beta3(ctx *TranslateContext, ar *config
}
ctx.addUpstream(ups)
}
- if len(pluginMap) > 0 {
- ctx.addPluginConfig(pluginConfig)
- }
}
return nil
}
@@ -851,7 +839,6 @@ func (t *translator) translateStreamRoute(ctx *TranslateContext, ar *configv2bet
if !ctx.checkUpstreamExist(ups.Name) {
ctx.addUpstream(ups)
}
-
}
return nil
}
@@ -977,12 +964,8 @@ func (t *translator) translateHTTPRouteV2beta3NotStrictly(ctx *TranslateContext,
route := apisixv1.NewDefaultRoute()
route.Name = apisixv1.ComposeRouteName(ar.Namespace, ar.Name, part.Name)
route.ID = id.GenID(route.Name)
- pluginConfig := apisixv1.NewDefaultPluginConfig()
- if len(pluginMap) > 0 {
- pluginConfigName := apisixv1.ComposePluginConfigName(ar.Namespace, backend.ServiceName)
- route.PluginConfigId = id.GenID(pluginConfigName)
- pluginConfig.ID = route.PluginConfigId
- pluginConfig.Name = pluginConfigName
+ if part.PluginConfigName != "" {
+ route.PluginConfigId = id.GenID(apisixv1.ComposePluginConfigName(ar.Namespace, part.PluginConfigName))
}
ctx.addRoute(route)
@@ -993,9 +976,6 @@ func (t *translator) translateHTTPRouteV2beta3NotStrictly(ctx *TranslateContext,
}
ctx.addUpstream(ups)
}
- if len(pluginMap) > 0 {
- ctx.addPluginConfig(pluginConfig)
- }
}
return nil
}
diff --git a/pkg/kube/translation/apisix_route_test.go b/pkg/kube/translation/apisix_route_test.go
index ed15ea8..56517df 100644
--- a/pkg/kube/translation/apisix_route_test.go
+++ b/pkg/kube/translation/apisix_route_test.go
@@ -33,6 +33,7 @@ import (
fakeapisix "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/fake"
apisixinformers "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions"
_const "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/const"
+ apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
)
func TestRouteMatchExpr(t *testing.T) {
@@ -185,7 +186,7 @@ func TestRouteMatchExpr(t *testing.T) {
assert.Equal(t, []string{"foo.com"}, results[9][2].SliceVal)
}
-func TestTranslateApisixRouteV2alpha1WithDuplicatedName(t *testing.T) {
+func mockTranslator(t *testing.T) (*translator, <-chan struct{}) {
svc := &corev1.Service{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
@@ -278,6 +279,11 @@ func TestTranslateApisixRouteV2alpha1WithDuplicatedName(t *testing.T) {
go epInformer.Run(stopCh)
cache.WaitForCacheSync(stopCh, svcInformer.HasSynced)
+ return tr, processCh
+}
+
+func TestTranslateApisixRouteV2beta3WithDuplicatedName(t *testing.T) {
+ tr, processCh := mockTranslator(t)
<-processCh
<-processCh
@@ -324,12 +330,86 @@ func TestTranslateApisixRouteV2alpha1WithDuplicatedName(t *testing.T) {
},
}
- _, err = tr.TranslateRouteV2beta3(ar)
+ _, err := tr.TranslateRouteV2beta3(ar)
assert.NotNil(t, err)
assert.Equal(t, "duplicated route rule name", err.Error())
}
-func TestTranslateApisixRouteV2alpha1NotStrictly(t *testing.T) {
+func TestTranslateApisixRouteV2beta3WithEmptyPluginConfigName(t *testing.T) {
+ tr, processCh := mockTranslator(t)
+ <-processCh
+ <-processCh
+
+ ar := &configv2beta3.ApisixRoute{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "ar",
+ Namespace: "test",
+ },
+ Spec: configv2beta3.ApisixRouteSpec{
+ HTTP: []configv2beta3.ApisixRouteHTTP{
+ {
+ Name: "rule1",
+ Match: configv2beta3.ApisixRouteHTTPMatch{
+ Paths: []string{
+ "/*",
+ },
+ },
+ Backends: []configv2beta3.ApisixRouteHTTPBackend{
+ {
+ ServiceName: "svc",
+ ServicePort: intstr.IntOrString{
+ IntVal: 80,
+ },
+ },
+ },
+ },
+ {
+ Name: "rule2",
+ Match: configv2beta3.ApisixRouteHTTPMatch{
+ Paths: []string{
+ "/*",
+ },
+ },
+ Backends: []configv2beta3.ApisixRouteHTTPBackend{
+ {
+ ServiceName: "svc",
+ ServicePort: intstr.IntOrString{
+ IntVal: 80,
+ },
+ },
+ },
+ PluginConfigName: "test-PluginConfigName-1",
+ },
+ {
+ Name: "rule3",
+ Match: configv2beta3.ApisixRouteHTTPMatch{
+ Paths: []string{
+ "/*",
+ },
+ },
+ Backends: []configv2beta3.ApisixRouteHTTPBackend{
+ {
+ ServiceName: "svc",
+ ServicePort: intstr.IntOrString{
+ IntVal: 80,
+ },
+ },
+ },
+ },
+ },
+ },
+ }
+ res, err := tr.TranslateRouteV2beta3(ar)
+ assert.NoError(t, err)
+ assert.Len(t, res.PluginConfigs, 0)
+ assert.Len(t, res.Routes, 3)
+ assert.Equal(t, "", res.Routes[0].PluginConfigId)
+ expectedPluginId := id.GenID(apisixv1.ComposePluginConfigName(ar.Namespace, ar.Spec.HTTP[1].PluginConfigName))
+ assert.Equal(t, expectedPluginId, res.Routes[1].PluginConfigId)
+ assert.Equal(t, "", res.Routes[2].PluginConfigId)
+}
+
+func TestTranslateApisixRouteV2beta3NotStrictly(t *testing.T) {
tr := &translator{
&TranslatorOptions{},
}
@@ -365,6 +445,7 @@ func TestTranslateApisixRouteV2alpha1NotStrictly(t *testing.T) {
},
},
},
+ PluginConfigName: "echo-and-cors-apc",
},
{
Name: "rule2",
@@ -391,16 +472,16 @@ func TestTranslateApisixRouteV2alpha1NotStrictly(t *testing.T) {
assert.NoError(t, err, "translateRoute not strictly should be no error")
assert.Equal(t, 2, len(tx.Routes), "There should be 2 routes")
assert.Equal(t, 2, len(tx.Upstreams), "There should be 2 upstreams")
- assert.Equal(t, 1, len(tx.PluginConfigs), "There should be 1 pluginConfigs")
assert.Equal(t, "test_ar_rule1", tx.Routes[0].Name, "route1 name error")
assert.Equal(t, "test_ar_rule2", tx.Routes[1].Name, "route2 name error")
assert.Equal(t, "test_svc1_81", tx.Upstreams[0].Name, "upstream1 name error")
assert.Equal(t, "test_svc2_82", tx.Upstreams[1].Name, "upstream2 name error")
- assert.Equal(t, "test_svc1", tx.PluginConfigs[0].Name, "pluginConfig1 name error")
assert.Equal(t, id.GenID("test_ar_rule1"), tx.Routes[0].ID, "route1 id error")
assert.Equal(t, id.GenID("test_ar_rule2"), tx.Routes[1].ID, "route2 id error")
+ assert.Equal(t, id.GenID(apisixv1.ComposePluginConfigName(ar.Namespace, ar.Spec.HTTP[0].PluginConfigName)), tx.Routes[0].PluginConfigId, "route1 PluginConfigId error")
+ assert.Equal(t, "", tx.Routes[1].PluginConfigId, "route2 PluginConfigId error ")
+
assert.Equal(t, id.GenID("test_svc1_81"), tx.Upstreams[0].ID, "upstream1 id error")
assert.Equal(t, id.GenID("test_svc2_82"), tx.Upstreams[1].ID, "upstream2 id error")
- assert.Equal(t, id.GenID("test_svc1"), tx.PluginConfigs[0].ID, "pluginConfig1 id error")
}
diff --git a/test/e2e/ingress/resourcepushing.go b/test/e2e/ingress/resourcepushing.go
index 66f8069..9a1fd6b 100644
--- a/test/e2e/ingress/resourcepushing.go
+++ b/test/e2e/ingress/resourcepushing.go
@@ -423,7 +423,7 @@ spec:
assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(apisixRoute), "creating ApisixRoute")
assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(2))
assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(1))
- assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixPluginConfigCreated(1))
+ assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixPluginConfigCreated(0))
// Hit rule1
resp := s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.com").Expect()