From 5a4ddd5655abedb9dc9277ed34117799c795937c Mon Sep 17 00:00:00 2001 From: Adil Fulara Date: Thu, 12 Dec 2024 15:40:14 -0800 Subject: [PATCH] MESH-5878: Admiral does not create envoy filter on client onboardings - Add DeltaUpdate and Delete api - Increase coverage TODO: Implement DeletaUpdate() correctly Signed-off-by: Adil Fulara --- admiral/pkg/clusters/dependency_handler.go | 3 + admiral/pkg/clusters/routingpolicy_handler.go | 132 +++-- .../clusters/routingpolicy_handler_test.go | 520 +++++++++++++++--- 3 files changed, 520 insertions(+), 135 deletions(-) diff --git a/admiral/pkg/clusters/dependency_handler.go b/admiral/pkg/clusters/dependency_handler.go index 8339e396..f98ea41d 100644 --- a/admiral/pkg/clusters/dependency_handler.go +++ b/admiral/pkg/clusters/dependency_handler.go @@ -75,6 +75,9 @@ func (dh *DependencyHandler) HandleDependencyRecord(ctx context.Context, obj *v1 return handleDepRecordErrors } + // process routing policies + _ = dh.RoutingPolicyProcessor.DeltaUpdate(ctx, eventType, obj) + remoteRegistry.AdmiralCache.SourceToDestinations.put(obj) return handleDepRecordErrors } diff --git a/admiral/pkg/clusters/routingpolicy_handler.go b/admiral/pkg/clusters/routingpolicy_handler.go index 57678d79..e412e0c3 100644 --- a/admiral/pkg/clusters/routingpolicy_handler.go +++ b/admiral/pkg/clusters/routingpolicy_handler.go @@ -26,6 +26,8 @@ func NewRoutingPolicyHandler(rr *RemoteRegistry, cId string, rpProcessor Routing type RoutingPolicyProcessor interface { ProcessAddOrUpdate(ctx context.Context, eventType admiral.EventType, routingPolicy *v1.RoutingPolicy, dependents map[string]string) error + DeltaUpdate(ctx context.Context, eventType admiral.EventType, dependency *v1.Dependency) error + Delete(ctx context.Context, eventType admiral.EventType, routingPolicy *v1.RoutingPolicy) error } type RoutingPolicyService struct { @@ -51,6 +53,81 @@ func (r *RoutingPolicyService) ProcessAddOrUpdate(ctx context.Context, eventType return err } +func (r *RoutingPolicyService) DeltaUpdate(ctx context.Context, eventType admiral.EventType, dependency *v1.Dependency) error { + //ns := getEnvoyFilterNamespace() + + //sourceClustersMap := r.RemoteRegistry.AdmiralCache.IdentityDependencyCache.Get(dependency.Spec.Source) + //if sourceClustersMap == nil { + // log.Infof(LogFormat, string(eventType), common.DependencyResourceType, dependency.Name, "", fmt.Sprintf("identity: %s, does not have any clusters. Skipping RoutingPolicy delta update", dependency.Spec.Source)) + // return nil + //} + getDestinationsToBeProcessed(eventType, dependency, r.RemoteRegistry) + + // for each destination, identify the rp. + // for each rp, call ProcessAddOrUpdate to add the envoyfilter + + //sourceClusters := make([]*RemoteController, 0) + //for _, sClusterID := range sourceClustersMap.GetKeys() { + // it := r.RemoteRegistry.GetRemoteController(sClusterID) + // if it != nil { + // sourceClusters = append(sourceClusters, it) + // } + //} + // + //for _, rc := range sourceClusters { + // options := metaV1.ListOptions{} + // options.LabelSelector = fmt.Sprintf("%s=%s", common.GetRoutingPolicyLabel(), dependency.Spec.Source) + // rp, err := rc.RoutingPolicyController.CrdClient.AdmiralV1alpha1().RoutingPolicies(ns).List(ctx, options) + // if err != nil { + // log.Errorf(LogFormat, eventType, "dependency", dependency.Spec.Source, rc.ClusterID, err) + // return err + // } + // d := make(map[string]string) + // for _, it := range destinations { + // d[it] = it + // } + // for _, it := range rp.Items { + // err = r.ProcessAddOrUpdate(ctx, eventType, &it, d) + // if err != nil { + // log.Errorf(LogErrFormat, admiral.Update, "routingpolicy", it.Name, "", "failed to process routing policy") + // return err + // } + // } + //} + + return nil +} + +func (r *RoutingPolicyService) Delete(ctx context.Context, eventType admiral.EventType, routingPolicy *v1.RoutingPolicy) error { + key := routingPolicy.Name + common.GetRoutingPolicyIdentity(routingPolicy) + common.GetRoutingPolicyEnv(routingPolicy) + if r.RemoteRegistry == nil || r.RemoteRegistry.AdmiralCache == nil || r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache == nil { + log.Infof(LogFormat, eventType, "routingpolicy", routingPolicy.Name, "", "skipping delete event as cache is nil") + return nil + } + clusterIdFilterMap := r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache.Get(key) // RoutingPolicyFilterCache key=rpname+rpidentity+environment of the routingPolicy, value is a map [clusterId -> map [filterName -> filterNameSpace]] + var err error + for _, rc := range r.RemoteRegistry.remoteControllers { + if rc != nil { + if filterMap, ok := clusterIdFilterMap[rc.ClusterID]; ok { + for filter, filterNs := range filterMap { + log.Infof(LogFormat, eventType, "envoyfilter", filter, rc.ClusterID, "deleting") + err1 := rc.RoutingPolicyController.IstioClient.NetworkingV1alpha3().EnvoyFilters(filterNs).Delete(ctx, filter, metaV1.DeleteOptions{}) + if err1 != nil { + log.Errorf(LogErrFormat, eventType, "envoyfilter", filter, rc.ClusterID, err1) + err = common.AppendError(err, err1) + } else { + log.Infof(LogFormat, eventType, "envoyfilter", filter, rc.ClusterID, "deleting from cache") + } + } + } + } + } + if err == nil { + r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache.Delete(key) + } + return err +} + func NewRoutingPolicyProcessor(remoteRegistry *RemoteRegistry) RoutingPolicyProcessor { return &RoutingPolicyService{RemoteRegistry: remoteRegistry} } @@ -109,7 +186,7 @@ func (r RoutingPolicyHandler) Added(ctx context.Context, obj *v1.RoutingPolicy) if common.ShouldIgnoreResource(obj.ObjectMeta) { log.Infof("op=%s type=%v name=%v namespace=%s cluster=%s message=%s", "admiralIoIgnoreAnnotationCheck", common.RoutingPolicyResourceType, obj.Name, obj.Namespace, "", "Value=true") - log.Infof(LogFormat, "success", "routingpolicy", obj.Name, "", "Ignored the RoutingPolicy because of the annotation") + log.Infof(LogFormat, "success", "routingpolicy", obj.Name, "", "Ignored the RoutingPolicy because of the annotation or label") return nil } dependents := getDependents(obj, r) @@ -129,25 +206,6 @@ func (r RoutingPolicyHandler) Added(ctx context.Context, obj *v1.RoutingPolicy) return nil } -func (r RoutingPolicyHandler) processroutingPolicy(ctx context.Context, dependents map[string]string, routingPolicy *v1.RoutingPolicy, eventType admiral.EventType) error { - var err error - for _, remoteController := range r.RemoteRegistry.remoteControllers { - for _, dependent := range dependents { - // Check if the dependent exists in this remoteCluster. If so, we create an envoyFilter with dependent identity as workload selector - if _, ok := r.RemoteRegistry.AdmiralCache.IdentityClusterCache.Get(dependent).Copy()[remoteController.ClusterID]; ok { - _, err1 := createOrUpdateEnvoyFilter(ctx, remoteController, routingPolicy, eventType, dependent, r.RemoteRegistry.AdmiralCache) - if err1 != nil { - log.Errorf(LogErrFormat, eventType, "routingpolicy", routingPolicy.Name, remoteController.ClusterID, err) - err = common.AppendError(err, err1) - } else { - log.Infof(LogFormat, eventType, "routingpolicy ", routingPolicy.Name, remoteController.ClusterID, "created envoyfilters") - } - } - } - } - return err -} - func (r RoutingPolicyHandler) Updated(ctx context.Context, obj *v1.RoutingPolicy) error { if commonUtil.IsAdmiralReadOnly() { log.Infof(LogFormat, admiral.Update, "routingpolicy", "", "", "skipping read-only mode") @@ -157,7 +215,7 @@ func (r RoutingPolicyHandler) Updated(ctx context.Context, obj *v1.RoutingPolicy if common.ShouldIgnoreResource(obj.ObjectMeta) { log.Infof("op=%s type=%v name=%v namespace=%s cluster=%s message=%s", "admiralIoIgnoreAnnotationCheck", common.RoutingPolicyResourceType, obj.Name, obj.Namespace, "", "Value=true") - log.Infof(LogFormat, admiral.Update, "routingpolicy", obj.Name, "", "Ignored the RoutingPolicy because of the annotation") + log.Infof(LogFormat, admiral.Update, "routingpolicy", obj.Name, "", "Ignored the RoutingPolicy because of the annotation or label") // We need to process this as a delete event. r.Deleted(ctx, obj) return nil @@ -196,39 +254,9 @@ func getDependents(obj *v1.RoutingPolicy, r RoutingPolicyHandler) map[string]str Deleted - deletes the envoyFilters for the routingPolicy when delete event received for routing policy */ func (r RoutingPolicyHandler) Deleted(ctx context.Context, obj *v1.RoutingPolicy) error { - err := r.deleteEnvoyFilters(ctx, obj, admiral.Delete) + err := r.RoutingPolicyService.Delete(ctx, admiral.Delete, obj) if err != nil { log.Infof(LogFormat, admiral.Delete, "routingpolicy", obj.Name, "", "deleted envoy filter for routing policy") } return err } - -func (r RoutingPolicyHandler) deleteEnvoyFilters(ctx context.Context, obj *v1.RoutingPolicy, eventType admiral.EventType) error { - key := obj.Name + common.GetRoutingPolicyIdentity(obj) + common.GetRoutingPolicyEnv(obj) - if r.RemoteRegistry == nil || r.RemoteRegistry.AdmiralCache == nil || r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache == nil { - log.Infof(LogFormat, eventType, "routingpolicy", obj.Name, "", "skipping delete event as cache is nil") - return nil - } - clusterIdFilterMap := r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache.Get(key) // RoutingPolicyFilterCache key=rpname+rpidentity+environment of the routingPolicy, value is a map [clusterId -> map [filterName -> filterNameSpace]] - var err error - for _, rc := range r.RemoteRegistry.remoteControllers { - if rc != nil { - if filterMap, ok := clusterIdFilterMap[rc.ClusterID]; ok { - for filter, filterNs := range filterMap { - log.Infof(LogFormat, eventType, "envoyfilter", filter, rc.ClusterID, "deleting") - err1 := rc.RoutingPolicyController.IstioClient.NetworkingV1alpha3().EnvoyFilters(filterNs).Delete(ctx, filter, metaV1.DeleteOptions{}) - if err1 != nil { - log.Errorf(LogErrFormat, eventType, "envoyfilter", filter, rc.ClusterID, err1) - err = common.AppendError(err, err1) - } else { - log.Infof(LogFormat, eventType, "envoyfilter", filter, rc.ClusterID, "deleting from cache") - } - } - } - } - } - if err == nil { - r.RemoteRegistry.AdmiralCache.RoutingPolicyFilterCache.Delete(key) - } - return err -} diff --git a/admiral/pkg/clusters/routingpolicy_handler_test.go b/admiral/pkg/clusters/routingpolicy_handler_test.go index 992f612d..933b9a49 100644 --- a/admiral/pkg/clusters/routingpolicy_handler_test.go +++ b/admiral/pkg/clusters/routingpolicy_handler_test.go @@ -3,7 +3,7 @@ package clusters import ( "bytes" "context" - "fmt" + "errors" "os" "reflect" "strings" @@ -24,7 +24,7 @@ import ( metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestRoutingPolicyHandler(t *testing.T) { +func TestNewRoutingPolicyProcessor(t *testing.T) { common.ResetSync() p := common.AdmiralParams{ KubeconfigPath: "testdata/fake.config", @@ -59,7 +59,6 @@ func TestRoutingPolicyHandler(t *testing.T) { }) remoteController.RoutingPolicyController = routingPolicyController - registry.remoteControllers = map[string]*RemoteController{"cluster-1": remoteController} registry.AdmiralCache.RoutingPolicyFilterCache = rpFilterCache @@ -67,16 +66,26 @@ func TestRoutingPolicyHandler(t *testing.T) { registry.AdmiralCache.IdentityDependencyCache.Put("foo", "bar", "bar") registry.AdmiralCache.IdentityClusterCache.Put("bar", remoteController.ClusterID, remoteController.ClusterID) - // foo is also dependent upon bar2 but bar2 is in a different cluster, so this cluster should not have the envoyfilter created + // foo2 is also dependent upon bar2 but bar2 is in a different cluster, so this cluster should not have the envoyfilter created registry.AdmiralCache.IdentityDependencyCache.Put("foo2", "bar2", "bar2") registry.AdmiralCache.IdentityClusterCache.Put("bar2", "differentCluster", "differentCluster") // foo1 is dependent upon bar 1 but bar1 does not have a deployment so it is missing from identityClusterCache registry.AdmiralCache.IdentityDependencyCache.Put("foo1", "bar1", "bar1") - handler := NewRoutingPolicyHandler(registry, "", NewRoutingPolicyProcessor(registry)) - - routingPolicyFoo := &admiralV1.RoutingPolicy{ + type args struct { + rr *RemoteRegistry + et admiral.EventType + rp *admiralV1.RoutingPolicy + dp map[string]string + } + type want struct { + err error + expectedFilterCacheKey string + expectedFilterCount int + expectedEnvoyFilterConfigPatchVal map[string]interface{} + } + fooRP := &admiralV1.RoutingPolicy{ TypeMeta: metaV1.TypeMeta{}, ObjectMeta: metaV1.ObjectMeta{ Name: "rpfoo", @@ -89,16 +98,149 @@ func TestRoutingPolicyHandler(t *testing.T) { Plugin: "test", Hosts: []string{"e2e.testservice.mesh"}, Config: map[string]string{ - "cachePrefix": "cache-v1", - "cachettlSec": "86400", "routingServiceUrl": "e2e.test.routing.service.mesh", - "pathPrefix": "/sayhello,/v1/company/{id}/", }, }, Status: admiralV1.RoutingPolicyStatus{}, } - routingPolicyFooTest := &admiralV1.RoutingPolicy{ + foo1RP := fooRP.DeepCopy() + foo1RP.Labels[common.GetWorkloadIdentifier()] = "foo1" + + foo2RP := fooRP.DeepCopy() + foo1RP.Labels[common.GetWorkloadIdentifier()] = "foo2" + + testCases := []struct { + name string + args args + want want + }{ + { + name: "Valid routing policy and existing deployment", + args: args{ + rr: registry, + et: admiral.Add, + rp: fooRP, + dp: map[string]string{"bar": "bar"}, + }, + want: want{ + expectedFilterCacheKey: "rpfoofoodev", + expectedFilterCount: 1, + expectedEnvoyFilterConfigPatchVal: map[string]interface{}{"name": "dynamicRoutingFilterPatch", "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/udpa.type.v1.TypedStruct", "type_url": "type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm", + "value": map[string]interface{}{ + "config": map[string]interface{}{ + "configuration": map[string]interface{}{ + "@type": "type.googleapis.com/google.protobuf.StringValue", + "value": "routingServiceUrl: e2e.test.routing.service.mesh\nhosts: e2e.testservice.mesh\nplugin: test"}, + "vm_config": map[string]interface{}{"code": map[string]interface{}{"local": map[string]interface{}{"filename": ""}}, "runtime": "envoy.wasm.runtime.v8", "vm_id": "test-dr-532221909d5db54fe5f5-f6ce3712830af1b15625-1.13"}}}}}, + }, + }, + { + name: "Valid routing policy but dependency cluster is missing", + args: args{ + rr: registry, + et: admiral.Add, + rp: foo1RP, + dp: map[string]string{"bar1": "bar1"}, + }, + want: want{ + expectedFilterCacheKey: "rpfoofoodev", + expectedFilterCount: 0, + }, + }, + { + name: "Valid routing policy and known cluster containing the deployment", + args: args{ + rr: registry, + et: admiral.Add, + rp: foo2RP, + dp: map[string]string{"bar2": "bar2"}, + }, + want: want{ + expectedFilterCacheKey: "rpfoofoodev", + expectedFilterCount: 0, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + p := NewRoutingPolicyProcessor(tc.args.rr) + ae := p.ProcessAddOrUpdate(ctx, tc.args.et, tc.args.rp, tc.args.dp) + if ae != nil && !errors.Is(ae, tc.want.err) { + t.Errorf("NewRoutingPolicyProcessor() = %v, want %v", ae, tc.want.err) + t.Fail() + } + list1, _ := remoteController.RoutingPolicyController.IstioClient.NetworkingV1alpha3().EnvoyFilters("istio-system").List(ctx, metaV1.ListOptions{}) + assert.Equal(t, tc.want.expectedFilterCount, len(list1.Items)) + + if tc.want.expectedFilterCount > 0 { + receivedEnvoyFilter, _ := remoteController.RoutingPolicyController.IstioClient.NetworkingV1alpha3().EnvoyFilters("istio-system").Get(ctx, "test-dr-532221909d5db54fe5f5-f6ce3712830af1b15625-1.13", metaV1.GetOptions{}) + eq := reflect.DeepEqual(tc.want.expectedEnvoyFilterConfigPatchVal, receivedEnvoyFilter.Spec.ConfigPatches[0].Patch.Value.AsMap()) + assert.True(t, eq) + } + + // once the routing policy is deleted, the corresponding filter should also be deleted + p.Delete(ctx, admiral.Delete, tc.args.rp) + assert.Nil(t, registry.AdmiralCache.RoutingPolicyFilterCache.Get(tc.want.expectedFilterCacheKey)) + + }) + } +} + +func TestRoutingPolicyHandler(t *testing.T) { + common.ResetSync() + p := common.AdmiralParams{ + KubeconfigPath: "testdata/fake.config", + LabelSet: &common.LabelSet{ + DeploymentAnnotation: "sidecar.istio.io/inject", + }, + EnableSAN: true, + SANPrefix: "prefix", + HostnameSuffix: "mesh", + SyncNamespace: "ns", + CacheReconcileDuration: time.Minute, + ClusterRegistriesNamespace: "default", + DependenciesNamespace: "default", + EnableRoutingPolicy: true, + EnvoyFilterVersion: "1.13", + Profile: common.AdmiralProfileDefault, + } + + p.LabelSet.WorkloadIdentityKey = "identity" + p.LabelSet.EnvKey = "admiral.io/env" + p.LabelSet.AdmiralCRDIdentityLabel = "identity" + + registry, _ := InitAdmiral(context.Background(), p) + + rpFilterCache := &routingPolicyFilterCache{} + rpFilterCache.filterCache = make(map[string]map[string]map[string]string) + rpFilterCache.mutex = &sync.Mutex{} + + routingPolicyController := &admiral.RoutingPolicyController{IstioClient: istiofake.NewSimpleClientset()} + remoteController, _ := createMockRemoteController(func(i interface{}) { + + }) + + remoteController.RoutingPolicyController = routingPolicyController + registry.remoteControllers = map[string]*RemoteController{"cluster-1": remoteController} + registry.AdmiralCache.RoutingPolicyFilterCache = rpFilterCache + + // foo is dependent upon bar and bar has a deployment in the same cluster. + registry.AdmiralCache.IdentityDependencyCache.Put("foo", "bar", "bar") + registry.AdmiralCache.IdentityClusterCache.Put("bar", remoteController.ClusterID, remoteController.ClusterID) + + // foo2 is also dependent upon bar2 but bar2 is in a different cluster, so this cluster should not have the envoyfilter created + registry.AdmiralCache.IdentityDependencyCache.Put("foo2", "bar2", "bar2") + registry.AdmiralCache.IdentityClusterCache.Put("bar2", "differentCluster", "differentCluster") + + // foo1 is dependent upon bar 1 but bar1 does not have a deployment so it is missing from identityClusterCache + registry.AdmiralCache.IdentityDependencyCache.Put("foo1", "bar1", "bar1") + processor := &mockPolicyProcessor{} + oops := errors.New("oops") + errProcessor := &mockPolicyProcessor{err: oops} + fooRP := &admiralV1.RoutingPolicy{ TypeMeta: metaV1.TypeMeta{}, ObjectMeta: metaV1.ObjectMeta{ Name: "rpfoo", @@ -116,95 +258,106 @@ func TestRoutingPolicyHandler(t *testing.T) { }, Status: admiralV1.RoutingPolicyStatus{}, } - - routingPolicyFoo1 := routingPolicyFoo.DeepCopy() - routingPolicyFoo1.Labels[common.GetWorkloadIdentifier()] = "foo1" - - routingPolicyFoo2 := routingPolicyFoo.DeepCopy() - routingPolicyFoo2.Labels[common.GetWorkloadIdentifier()] = "foo2" - + type args struct { + proc *mockPolicyProcessor + eventType admiral.EventType + rp *admiralV1.RoutingPolicy + } + type want struct { + err error + addCount int + delCount int + updateCount int + } + time.Sleep(time.Second * 10) testCases := []struct { - name string - routingPolicy *admiralV1.RoutingPolicy - expectedFilterCacheKey string - expectedFilterCount int - expectedEnvoyFilterConfigPatchVal map[string]interface{} + name string + args args + want want }{ { - name: "If dependent deployment exists, should fetch filter from cache", - routingPolicy: routingPolicyFooTest, - expectedFilterCacheKey: "rpfoofoodev", - expectedFilterCount: 1, - expectedEnvoyFilterConfigPatchVal: map[string]interface{}{"name": "dynamicRoutingFilterPatch", "typed_config": map[string]interface{}{ - "@type": "type.googleapis.com/udpa.type.v1.TypedStruct", "type_url": "type.googleapis.com/envoy.extensions.filters.http.wasm.v3.Wasm", - "value": map[string]interface{}{ - "config": map[string]interface{}{ - "configuration": map[string]interface{}{ - "@type": "type.googleapis.com/google.protobuf.StringValue", - "value": "routingServiceUrl: e2e.test.routing.service.mesh\nhosts: e2e.testservice.mesh\nplugin: test"}, - "vm_config": map[string]interface{}{"code": map[string]interface{}{"local": map[string]interface{}{"filename": ""}}, "runtime": "envoy.wasm.runtime.v8", "vm_id": "test-dr-532221909d5db54fe5f5-f6ce3712830af1b15625-1.13"}}}}}, + name: "Add event is propagated correctly", + args: args{ + proc: processor, + eventType: admiral.Add, + rp: fooRP, + }, + want: want{ + addCount: 1, + }, }, { - name: "If dependent deployment does not exist, the filter should not be created ", - routingPolicy: routingPolicyFoo1, - expectedFilterCacheKey: "rpfoofoodev", - expectedFilterCount: 0, + name: "Update event is propagated correctly", + args: args{ + proc: processor, + eventType: admiral.Update, + rp: fooRP, + }, + want: want{ + updateCount: 1, + }, }, { - name: "If dependent deployment exists in a different cluster, the filter should not be created in cluster where dependency isnt there", - routingPolicy: routingPolicyFoo2, - expectedFilterCacheKey: "rpfoofoodev", - expectedFilterCount: 0, + name: "Delete event is propagated correctly", + args: args{ + proc: processor, + eventType: admiral.Delete, + rp: fooRP, + }, + want: want{ + delCount: 1, + }, + }, + { + name: "Err on delete event", + args: args{ + proc: errProcessor, + eventType: admiral.Delete, + rp: fooRP, + }, + want: want{ + err: oops, + delCount: 0, + }, }, } ctx := context.Background() - time.Sleep(time.Second * 30) for _, c := range testCases { t.Run(c.name, func(t *testing.T) { - handler.Added(ctx, c.routingPolicy) - if c.expectedFilterCount > 0 { - filterCacheValue := registry.AdmiralCache.RoutingPolicyFilterCache.Get(c.expectedFilterCacheKey) - assert.NotNil(t, filterCacheValue) - routingPolicyNameSha, _ := getSha1(c.routingPolicy.Name + common.GetRoutingPolicyEnv(c.routingPolicy) + common.GetRoutingPolicyIdentity(c.routingPolicy)) - dependentIdentitySha, _ := getSha1("bar") - envoyFilterName := fmt.Sprintf("%s-dr-%s-%s-%s", strings.ToLower(c.routingPolicy.Spec.Plugin), routingPolicyNameSha, dependentIdentitySha, "1.13") - - filterMap := filterCacheValue[remoteController.ClusterID] - assert.NotNil(t, filterMap) - assert.NotNil(t, filterMap[envoyFilterName]) - - filter, err := remoteController.RoutingPolicyController.IstioClient.NetworkingV1alpha3(). - EnvoyFilters("istio-system").Get(ctx, envoyFilterName, metaV1.GetOptions{}) - assert.Nil(t, err) - assert.NotNil(t, filter) + handler := NewRoutingPolicyHandler(registry, "", c.args.proc) + switch c.args.eventType { + case admiral.Add: + err := handler.Added(ctx, c.args.rp) + if c.want.err != nil && !errors.Is(err, c.want.err) { + t.Errorf("RoutingPolicyHandler.Added() = %v, want %v", err, c.want.err) + t.Fail() + } + if c.want.addCount > 0 { + assert.Equal(t, c.args.proc.addCount, c.want.addCount) + } + case admiral.Update: + err := handler.Updated(ctx, c.args.rp) + if c.want.err != nil && !errors.Is(err, c.want.err) { + t.Errorf("RoutingPolicyHandler.Updated() = %v, want %v", err, c.want.err) + t.Fail() + } + if c.want.addCount > 0 { + assert.Equal(t, c.args.proc.addCount, c.want.addCount) + } + case admiral.Delete: + err := handler.Deleted(ctx, c.args.rp) + if c.want.err != nil && !errors.Is(err, c.want.err) { + t.Errorf("RoutingPolicyHandler.Deleted() = %v, want %v", err, c.want.err) + t.Fail() + } + if c.want.addCount > 0 { + assert.Equal(t, c.args.proc.addCount, c.want.addCount) + } } - //get envoyfilters from all namespaces - list1, _ := remoteController.RoutingPolicyController.IstioClient.NetworkingV1alpha3().EnvoyFilters("istio-system").List(ctx, metaV1.ListOptions{}) - assert.Equal(t, c.expectedFilterCount, len(list1.Items)) - if c.expectedFilterCount > 0 { - receivedEnvoyFilter, _ := remoteController.RoutingPolicyController.IstioClient.NetworkingV1alpha3().EnvoyFilters("istio-system").Get(ctx, "test-dr-532221909d5db54fe5f5-f6ce3712830af1b15625-1.13", metaV1.GetOptions{}) - eq := reflect.DeepEqual(c.expectedEnvoyFilterConfigPatchVal, receivedEnvoyFilter.Spec.ConfigPatches[0].Patch.Value.AsMap()) - assert.True(t, eq) - } - - // once the routing policy is deleted, the corresponding filter should also be deleted - handler.Deleted(ctx, c.routingPolicy) - assert.Nil(t, registry.AdmiralCache.RoutingPolicyFilterCache.Get(c.expectedFilterCacheKey)) }) } - - // ignore the routing policy - annotations := routingPolicyFoo.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - annotations[common.AdmiralIgnoreAnnotation] = "true" - routingPolicyFoo.SetAnnotations(annotations) - - handler.Updated(ctx, routingPolicyFoo) - assert.Nil(t, registry.AdmiralCache.RoutingPolicyFilterCache.Get("rpfoofoodev")) } func TestRoutingPolicyReadOnly(t *testing.T) { @@ -282,3 +435,204 @@ func TestRoutingPolicyReadOnly(t *testing.T) { }) } } + +func TestRoutingPolicyIgnored(t *testing.T) { + common.ResetSync() + p := common.AdmiralParams{ + KubeconfigPath: "testdata/fake.config", + LabelSet: &common.LabelSet{ + DeploymentAnnotation: "sidecar.istio.io/inject", + }, + EnableSAN: true, + SANPrefix: "prefix", + HostnameSuffix: "mesh", + SyncNamespace: "ns", + CacheReconcileDuration: time.Minute, + ClusterRegistriesNamespace: "default", + DependenciesNamespace: "default", + EnableRoutingPolicy: true, + EnvoyFilterVersion: "1.13", + Profile: common.AdmiralProfileDefault, + } + + p.LabelSet.WorkloadIdentityKey = "identity" + p.LabelSet.EnvKey = "admiral.io/env" + p.LabelSet.AdmiralCRDIdentityLabel = "identity" + + registry, _ := InitAdmiral(context.Background(), p) + + processor := &mockPolicyProcessor{} + handler := NewRoutingPolicyHandler(registry, "", processor) + + type want struct { + err error + addCount int + delCount int + updateCount int + } + testcases := []struct { + name string + rp *admiralV1.RoutingPolicy + want want + }{ + { + name: "Ignore Routing Policy - Annotation", + rp: &admiralV1.RoutingPolicy{ + ObjectMeta: metaV1.ObjectMeta{ + Annotations: map[string]string{ + "admiral.io/ignore": "true", + }, + }, + }, + want: want{}, + }, + { + name: "Ignore Routing Policy - Label", + rp: &admiralV1.RoutingPolicy{ + ObjectMeta: metaV1.ObjectMeta{ + Labels: map[string]string{ + "admiral.io/ignore": "true", + }, + }, + }, + want: want{}, + }, + } + + ctx := context.Background() + + for _, c := range testcases { + t.Run(c.name, func(t *testing.T) { + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + handler.Added(ctx, c.rp) + t.Log(buf.String()) + val := strings.Contains(buf.String(), "Ignored the RoutingPolicy") + assert.True(t, val, "RoutingPolicy should be ignored for 'add' event") + assert.Equal(t, c.want.addCount, 0) + + // Update routing policy test + handler.Updated(ctx, c.rp) + t.Log(buf.String()) + val = strings.Contains(buf.String(), "Ignored the RoutingPolicy") + assert.True(t, val, "RoutingPolicy should be ignored for 'update' event") + assert.Equal(t, c.want.updateCount, 0) + + // Delete routing policy test + handler.Deleted(ctx, c.rp) + t.Log(buf.String()) + val = strings.Contains(buf.String(), "Ignored the RoutingPolicy") + assert.True(t, val, "RoutingPolicy should be ignored for 'delete' event") + assert.Equal(t, c.want.delCount, 0) + }) + } +} + +func TestRoutingPolicyProcessingDisabled(t *testing.T) { + common.ResetSync() + p := common.AdmiralParams{ + KubeconfigPath: "testdata/fake.config", + LabelSet: &common.LabelSet{ + DeploymentAnnotation: "sidecar.istio.io/inject", + }, + EnableSAN: true, + SANPrefix: "prefix", + HostnameSuffix: "mesh", + SyncNamespace: "ns", + CacheReconcileDuration: time.Minute, + ClusterRegistriesNamespace: "default", + DependenciesNamespace: "default", + EnableRoutingPolicy: false, + EnvoyFilterVersion: "1.13", + Profile: common.AdmiralProfileDefault, + } + + p.LabelSet.WorkloadIdentityKey = "identity" + p.LabelSet.EnvKey = "admiral.io/env" + p.LabelSet.AdmiralCRDIdentityLabel = "identity" + + registry, _ := InitAdmiral(context.Background(), p) + + processor := &mockPolicyProcessor{} + handler := NewRoutingPolicyHandler(registry, "", processor) + + type want struct { + err error + addCount int + delCount int + updateCount int + } + testcases := []struct { + name string + rp *admiralV1.RoutingPolicy + want want + }{ + { + name: "Disabled", + rp: &admiralV1.RoutingPolicy{ + ObjectMeta: metaV1.ObjectMeta{ + Annotations: map[string]string{ + "admiral.io/ignore": "true", + }, + }, + }, + want: want{}, + }, + } + + ctx := context.Background() + + for _, c := range testcases { + t.Run(c.name, func(t *testing.T) { + var buf bytes.Buffer + log.SetOutput(&buf) + defer func() { + log.SetOutput(os.Stderr) + }() + handler.Added(ctx, c.rp) + t.Log(buf.String()) + val := strings.Contains(buf.String(), "routingpolicy disabled") + assert.True(t, val, "RoutingPolicy processing should be disabled") + assert.Equal(t, c.want.addCount, 0) + + // Update routing policy test + handler.Updated(ctx, c.rp) + t.Log(buf.String()) + val = strings.Contains(buf.String(), "routingpolicy disabled") + assert.True(t, val, "RoutingPolicy processing should be disabled") + assert.Equal(t, c.want.updateCount, 0) + + // Delete routing policy test + handler.Deleted(ctx, c.rp) + t.Log(buf.String()) + val = strings.Contains(buf.String(), "routingpolicy disabled") + assert.True(t, val, "RoutingPolicy processing should be disabled") + assert.Equal(t, c.want.delCount, 0) + }) + } +} + +type mockPolicyProcessor struct { + addCount int + deltaCount int + deleteCount int + err error +} + +func (m *mockPolicyProcessor) ProcessAddOrUpdate(ctx context.Context, eventType admiral.EventType, routingPolicy *admiralV1.RoutingPolicy, dependents map[string]string) error { + m.addCount++ + return m.err +} + +func (m *mockPolicyProcessor) DeltaUpdate(ctx context.Context, eventType admiral.EventType, dependency *admiralV1.Dependency) error { + m.deltaCount++ + return m.err +} + +func (m *mockPolicyProcessor) Delete(ctx context.Context, eventType admiral.EventType, routingPolicy *admiralV1.RoutingPolicy) error { + m.deleteCount++ + return m.err +}