From 6c0b02bbc59043071642e38e6b7710f6cad14077 Mon Sep 17 00:00:00 2001 From: charlie Date: Wed, 7 Dec 2022 15:28:58 +0800 Subject: [PATCH 1/5] opt: optimize redundant code Signed-off-by: charlie --- pkg/ingress/mcp/generator.go | 129 ++++++----------------------------- 1 file changed, 19 insertions(+), 110 deletions(-) diff --git a/pkg/ingress/mcp/generator.go b/pkg/ingress/mcp/generator.go index ab2ca491d..e5f14f879 100644 --- a/pkg/ingress/mcp/generator.go +++ b/pkg/ingress/mcp/generator.go @@ -14,17 +14,17 @@ package mcp +// nolint import ( "path" "github.com/gogo/protobuf/types" "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/any" - extensions "istio.io/api/extensions/v1alpha1" mcp "istio.io/api/mcp/v1alpha1" - networking "istio.io/api/networking/v1alpha3" "istio.io/istio/pilot/pkg/model" "istio.io/istio/pilot/pkg/xds" + cfg "istio.io/istio/pkg/config" ) type ServiceEntryGenerator struct { @@ -72,31 +72,7 @@ type VirtualServiceGenerator struct { func (c VirtualServiceGenerator) Generate(proxy *model.Proxy, push *model.PushContext, w *model.WatchedResource, updates *model.PushRequest) ([]*any.Any, model.XdsLogDetails, error) { - resources := make([]*any.Any, 0) - configs := push.AllVirtualServices - for _, config := range configs { - body, err := types.MarshalAny(config.Spec.(*networking.VirtualService)) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - createTime, err := types.TimestampProto(config.CreationTimestamp) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resource := &mcp.Resource{ - Body: body, - Metadata: &mcp.Metadata{ - Name: path.Join(config.Namespace, config.Name), - CreateTime: createTime, - }, - } - mcpAny, err := ptypes.MarshalAny(resource) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resources = append(resources, mcpAny) - } - return resources, model.DefaultXdsLogDetails, nil + return generate(proxy, push.AllVirtualServices, w, updates) } func (c VirtualServiceGenerator) GenerateDeltas(proxy *model.Proxy, push *model.PushContext, updates *model.PushRequest, @@ -111,31 +87,7 @@ type DestinationRuleGenerator struct { func (c DestinationRuleGenerator) Generate(proxy *model.Proxy, push *model.PushContext, w *model.WatchedResource, updates *model.PushRequest) ([]*any.Any, model.XdsLogDetails, error) { - resources := make([]*any.Any, 0) - configs := push.AllDestinationRules - for _, config := range configs { - body, err := types.MarshalAny(config.Spec.(*networking.DestinationRule)) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - createTime, err := types.TimestampProto(config.CreationTimestamp) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resource := &mcp.Resource{ - Body: body, - Metadata: &mcp.Metadata{ - Name: path.Join(config.Namespace, config.Name), - CreateTime: createTime, - }, - } - mcpAny, err := ptypes.MarshalAny(resource) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resources = append(resources, mcpAny) - } - return resources, model.DefaultXdsLogDetails, nil + return generate(proxy, push.AllDestinationRules, w, updates) } func (c DestinationRuleGenerator) GenerateDeltas(proxy *model.Proxy, push *model.PushContext, updates *model.PushRequest, @@ -150,31 +102,7 @@ type EnvoyFilterGenerator struct { func (c EnvoyFilterGenerator) Generate(proxy *model.Proxy, push *model.PushContext, w *model.WatchedResource, updates *model.PushRequest) ([]*any.Any, model.XdsLogDetails, error) { - resources := make([]*any.Any, 0) - configs := push.AllEnvoyFilters - for _, config := range configs { - body, err := types.MarshalAny(config.Spec.(*networking.EnvoyFilter)) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - createTime, err := types.TimestampProto(config.CreationTimestamp) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resource := &mcp.Resource{ - Body: body, - Metadata: &mcp.Metadata{ - Name: path.Join(config.Namespace, config.Name), - CreateTime: createTime, - }, - } - mcpAny, err := ptypes.MarshalAny(resource) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resources = append(resources, mcpAny) - } - return resources, model.DefaultXdsLogDetails, nil + return generate(proxy, push.AllEnvoyFilters, w, updates) } func (c EnvoyFilterGenerator) GenerateDeltas(proxy *model.Proxy, push *model.PushContext, updates *model.PushRequest, @@ -189,31 +117,7 @@ type GatewayGenerator struct { func (c GatewayGenerator) Generate(proxy *model.Proxy, push *model.PushContext, w *model.WatchedResource, updates *model.PushRequest) ([]*any.Any, model.XdsLogDetails, error) { - resources := make([]*any.Any, 0) - configs := push.AllGateways - for _, config := range configs { - body, err := types.MarshalAny(config.Spec.(*networking.Gateway)) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - createTime, err := types.TimestampProto(config.CreationTimestamp) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resource := &mcp.Resource{ - Body: body, - Metadata: &mcp.Metadata{ - Name: path.Join(config.Namespace, config.Name), - CreateTime: createTime, - }, - } - mcpAny, err := ptypes.MarshalAny(resource) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resources = append(resources, mcpAny) - } - return resources, model.DefaultXdsLogDetails, nil + return generate(proxy, push.AllGateways, w, updates) } func (c GatewayGenerator) GenerateDeltas(proxy *model.Proxy, push *model.PushContext, updates *model.PushRequest, @@ -227,11 +131,21 @@ type WasmpluginGenerator struct { } func (c WasmpluginGenerator) Generate(proxy *model.Proxy, push *model.PushContext, w *model.WatchedResource, + updates *model.PushRequest) ([]*any.Any, model.XdsLogDetails, error) { + return generate(proxy, push.AllWasmplugins, w, updates) +} + +func (c WasmpluginGenerator) GenerateDeltas(proxy *model.Proxy, push *model.PushContext, updates *model.PushRequest, + w *model.WatchedResource) ([]*any.Any, []string, model.XdsLogDetails, bool, error) { + // TODO: delta implement + return nil, nil, model.DefaultXdsLogDetails, false, nil +} + +func generate(proxy *model.Proxy, configs []cfg.Config, w *model.WatchedResource, updates *model.PushRequest) ([]*any.Any, model.XdsLogDetails, error) { resources := make([]*any.Any, 0) - configs := push.AllWasmplugins for _, config := range configs { - body, err := types.MarshalAny(config.Spec.(*extensions.WasmPlugin)) + body, err := cfg.ToProtoGogo(config.Spec) if err != nil { return nil, model.DefaultXdsLogDetails, err } @@ -246,6 +160,7 @@ func (c WasmpluginGenerator) Generate(proxy *model.Proxy, push *model.PushContex CreateTime: createTime, }, } + // nolint mcpAny, err := ptypes.MarshalAny(resource) if err != nil { return nil, model.DefaultXdsLogDetails, err @@ -254,9 +169,3 @@ func (c WasmpluginGenerator) Generate(proxy *model.Proxy, push *model.PushContex } return resources, model.DefaultXdsLogDetails, nil } - -func (c WasmpluginGenerator) GenerateDeltas(proxy *model.Proxy, push *model.PushContext, updates *model.PushRequest, - w *model.WatchedResource) ([]*any.Any, []string, model.XdsLogDetails, bool, error) { - // TODO: delta implement - return nil, nil, model.DefaultXdsLogDetails, false, nil -} From 0357d9c62cbe746bbd76b6dbc08503f9bad7a1e1 Mon Sep 17 00:00:00 2001 From: charlie Date: Wed, 7 Dec 2022 17:39:32 +0800 Subject: [PATCH 2/5] ut: add the unit tests for pkg/ingress/mcp Signed-off-by: charlie --- pkg/ingress/mcp/generator_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/ingress/mcp/generator_test.go b/pkg/ingress/mcp/generator_test.go index 63b86f031..a6287976c 100644 --- a/pkg/ingress/mcp/generator_test.go +++ b/pkg/ingress/mcp/generator_test.go @@ -101,6 +101,19 @@ func TestGenerate(t *testing.T) { generator: WasmpluginGenerator{}, isErr: false, }, + { + name: "ServiceEntry", + fn: func() (*model.PushContext, any) { + ctx := model.NewPushContext() + cfg := config.Config{ + Spec: &networking.ServiceEntry{}, + } + ctx.AllServiceEntries = []config.Config{cfg} + return ctx, cfg.Spec + }, + generator: ServiceEntryGenerator{}, + isErr: false, + }, { name: "WasmPlugin with wrong config", fn: func() (*model.PushContext, any) { From f84655796554b364bbdaaf2f874a646996482e82 Mon Sep 17 00:00:00 2001 From: charlie Date: Thu, 8 Dec 2022 20:21:31 +0800 Subject: [PATCH 3/5] replace generate with McpResourceGenerator.Generate Signed-off-by: charlie --- pkg/ingress/mcp/generator_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/pkg/ingress/mcp/generator_test.go b/pkg/ingress/mcp/generator_test.go index a6287976c..63b86f031 100644 --- a/pkg/ingress/mcp/generator_test.go +++ b/pkg/ingress/mcp/generator_test.go @@ -101,19 +101,6 @@ func TestGenerate(t *testing.T) { generator: WasmpluginGenerator{}, isErr: false, }, - { - name: "ServiceEntry", - fn: func() (*model.PushContext, any) { - ctx := model.NewPushContext() - cfg := config.Config{ - Spec: &networking.ServiceEntry{}, - } - ctx.AllServiceEntries = []config.Config{cfg} - return ctx, cfg.Spec - }, - generator: ServiceEntryGenerator{}, - isErr: false, - }, { name: "WasmPlugin with wrong config", fn: func() (*model.PushContext, any) { From c7fd187f139d663a6f3c6faa2b8e412354a423ba Mon Sep 17 00:00:00 2001 From: charlie Date: Wed, 1 Feb 2023 13:49:43 +0800 Subject: [PATCH 4/5] reactor ServiceEntryGenerator Signed-off-by: charlie --- pkg/ingress/mcp/generator.go | 26 +------------------------- pkg/ingress/mcp/generator_test.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/pkg/ingress/mcp/generator.go b/pkg/ingress/mcp/generator.go index e5f14f879..37b98dc7b 100644 --- a/pkg/ingress/mcp/generator.go +++ b/pkg/ingress/mcp/generator.go @@ -33,31 +33,7 @@ type ServiceEntryGenerator struct { func (c ServiceEntryGenerator) Generate(proxy *model.Proxy, push *model.PushContext, w *model.WatchedResource, updates *model.PushRequest) ([]*any.Any, model.XdsLogDetails, error) { - resources := make([]*any.Any, 0) - configs := push.AllServiceEntries - for _, config := range configs { - body, err := types.MarshalAny(config.Spec.(*networking.ServiceEntry)) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - createTime, err := types.TimestampProto(config.CreationTimestamp) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resource := &mcp.Resource{ - Body: body, - Metadata: &mcp.Metadata{ - Name: path.Join(config.Namespace, config.Name), - CreateTime: createTime, - }, - } - mcpAny, err := ptypes.MarshalAny(resource) - if err != nil { - return nil, model.DefaultXdsLogDetails, err - } - resources = append(resources, mcpAny) - } - return resources, model.DefaultXdsLogDetails, nil + return generate(proxy, push.AllServiceEntries, w, updates) } func (c ServiceEntryGenerator) GenerateDeltas(proxy *model.Proxy, push *model.PushContext, updates *model.PushRequest, diff --git a/pkg/ingress/mcp/generator_test.go b/pkg/ingress/mcp/generator_test.go index 63b86f031..f7d16723b 100644 --- a/pkg/ingress/mcp/generator_test.go +++ b/pkg/ingress/mcp/generator_test.go @@ -101,6 +101,19 @@ func TestGenerate(t *testing.T) { generator: WasmpluginGenerator{}, isErr: false, }, + { + name: "ServiceEntry", + fn: func() *model.PushContext { + ctx := model.NewPushContext() + cfg := config.Config{ + Spec: &networking.ServiceEntry{}, + } + ctx.AllServiceEntries = []config.Config{cfg} + return ctx + }, + generator: ServiceEntryGenerator{}, + isErr: false, + }, { name: "WasmPlugin with wrong config", fn: func() (*model.PushContext, any) { From 59b725534aeae38f6eedbb7997265bb454452a73 Mon Sep 17 00:00:00 2001 From: charlie Date: Wed, 1 Feb 2023 15:10:44 +0800 Subject: [PATCH 5/5] resolve the conflict Signed-off-by: charlie --- pkg/ingress/mcp/generator_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ingress/mcp/generator_test.go b/pkg/ingress/mcp/generator_test.go index f7d16723b..a6287976c 100644 --- a/pkg/ingress/mcp/generator_test.go +++ b/pkg/ingress/mcp/generator_test.go @@ -103,13 +103,13 @@ func TestGenerate(t *testing.T) { }, { name: "ServiceEntry", - fn: func() *model.PushContext { + fn: func() (*model.PushContext, any) { ctx := model.NewPushContext() cfg := config.Config{ Spec: &networking.ServiceEntry{}, } ctx.AllServiceEntries = []config.Config{cfg} - return ctx + return ctx, cfg.Spec }, generator: ServiceEntryGenerator{}, isErr: false,