diff --git a/pkg/ingress/apisix_route.go b/pkg/ingress/apisix_route.go index 7b6ed53ac5..5f62b7684d 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 8e92485ffd..cb0df7d62b 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 ed15ea8b1b..56517df7cd 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 66f8069432..9a1fd6baf4 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()