Skip to content

Commit

Permalink
fix: avoid create pluginconfig in the tranlsation of route (#845)
Browse files Browse the repository at this point in the history
  • Loading branch information
@neverCase @tao12345666333
neverCase authored and tao12345666333 committed Apr 22, 2022
1 parent e0518a4 commit cbcae44
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 54 deletions.
44 changes: 21 additions & 23 deletions pkg/ingress/apisix_route.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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",
Expand All @@ -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) {
Expand Down
28 changes: 4 additions & 24 deletions pkg/kube/translation/apisix_route.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -566,9 +557,6 @@ func (t *translator) translateHTTPRouteV2beta3(ctx *TranslateContext, ar *config
}
ctx.addUpstream(ups)
}
if len(pluginMap) > 0 {
ctx.addPluginConfig(pluginConfig)
}
}
return nil
}
Expand Down Expand Up @@ -851,7 +839,6 @@ func (t *translator) translateStreamRoute(ctx *TranslateContext, ar *configv2bet
if !ctx.checkUpstreamExist(ups.Name) {
ctx.addUpstream(ups)
}

}
return nil
}
Expand Down Expand Up @@ -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)
Expand All @@ -993,9 +976,6 @@ func (t *translator) translateHTTPRouteV2beta3NotStrictly(ctx *TranslateContext,
}
ctx.addUpstream(ups)
}
if len(pluginMap) > 0 {
ctx.addPluginConfig(pluginConfig)
}
}
return nil
}
Expand Down
93 changes: 87 additions & 6 deletions pkg/kube/translation/apisix_route_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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{},
}
Expand Down Expand Up @@ -365,6 +445,7 @@ func TestTranslateApisixRouteV2alpha1NotStrictly(t *testing.T) {
},
},
},
PluginConfigName: "echo-and-cors-apc",
},
{
Name: "rule2",
Expand All @@ -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")
}
2 changes: 1 addition & 1 deletion test/e2e/ingress/resourcepushing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit cbcae44

Please sign in to comment.