replaced allow list with exclusion list for vs based routing (#378)

admiral/cmd/admiral/cmd/root.go

@@ -258,7 +258,7 @@ func GetRootCmd(args []string) *cobra.Command {
 
 	// Flags pertaining to VS based routing
 	rootCmd.PersistentFlags().BoolVar(&params.EnableVSRouting, "enable_vs_routing", false, "Enable/Disable VS Based Routing")
-	rootCmd.PersistentFlags().StringSliceVar(&params.VSRoutingEnabledClusters, "vs_routing_enabled_clusters", []string{}, "The source clusters to enable VS based routing on")
+	rootCmd.PersistentFlags().StringSliceVar(&params.VSRoutingDisabledClusters, "vs_routing_disabled_clusters", []string{}, "The source clusters to disable VS based routing on")
 	rootCmd.PersistentFlags().StringSliceVar(&params.VSRoutingSlowStartEnabledClusters, "vs_routing_slow_start_enabled_clusters", []string{}, "The source clusters to where VS routing is enabled and require slow start")
 	rootCmd.PersistentFlags().StringSliceVar(&params.VSRoutingGateways, "vs_routing_gateways", []string{}, "The PASSTHROUGH gateways to use for VS based routing")
 	rootCmd.PersistentFlags().StringSliceVar(&params.IngressVSExportToNamespaces, "ingress_vs_export_to_namespaces", []string{"istio-system"}, "List of namespaces where the ingress VS should be exported")

admiral/pkg/clusters/serviceentry.go

@@ -772,7 +772,7 @@ func modifyServiceEntryForNewServiceOrPod(
 			remoteRegistry.AdmiralCache.DependencyNamespaceCache.Put(val, serviceInstance[appType[sourceCluster]].Namespace, localFqdn, cnames)
 		}
 
-		if common.DoVSRoutingForCluster(sourceCluster) {
+		if !common.IsVSRoutingDisabledForCluster(sourceCluster) {
 			eventNamespace := sourceClusterToEventNsCache[sourceCluster]
 			ctxLogger.Infof(common.CtxLogFormat, "VSBasedRouting",
 				deploymentOrRolloutName, eventNamespace, sourceCluster,

admiral/pkg/clusters/serviceentry_test.go

@@ -78,6 +78,7 @@ func admiralParamsForServiceEntryTests() common.AdmiralParams {
 		Profile:                           common.AdmiralProfileDefault,
 		DependentClusterWorkerConcurrency: 5,
 		PreventSplitBrain:                 true,
+		VSRoutingGateways:                 []string{"istio-system/passthrough-gateway"},
 	}
 }
 

admiral/pkg/clusters/shard_handler_test.go

@@ -4,6 +4,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"sync"
+	"testing"
+	"time"
+
 	"github.com/golang/protobuf/ptypes/duration"
 	"github.com/google/go-cmp/cmp"
 	"github.com/istio-ecosystem/admiral/admiral/pkg/client/loader"
@@ -13,9 +17,6 @@ import (
 	"istio.io/client-go/pkg/apis/networking/v1alpha3"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"sync"
-	"testing"
-	"time"
 
 	admiralapiv1 "github.com/istio-ecosystem/admiral-api/pkg/apis/admiral/v1"
 	"github.com/istio-ecosystem/admiral/admiral/pkg/controller/common"
@@ -42,6 +43,7 @@ func setupForShardTests() common.AdmiralParams {
 	admiralParams.AdditionalEndpointSuffixes = []string{"org"}
 	admiralParams.SecretFilterTags = "admiral/sync"
 	admiralParams.OperatorSecretFilterTags = "admiral/syncoperator"
+	admiralParams.VSRoutingGateways = []string{"passthrough-gateway"}
 	shardTestSingleton.Do(func() {
 		common.ResetSync()
 		initHappened = true

admiral/pkg/clusters/virtualservice_routing.go

@@ -468,7 +468,10 @@ func addUpdateVirtualServicesForIngress(
 
 	for sourceCluster, destination := range sourceClusterToDestinations {
 
-		if !common.DoVSRoutingForCluster(sourceCluster) {
+		if common.IsVSRoutingDisabledForCluster(sourceCluster) {
+			ctxLogger.Infof(common.CtxLogFormat, "VSBasedRouting",
+				"", "", sourceCluster,
+				"Writing phase: addUpdateVirtualServicesForIngress VS based routing disabled for cluster")
 			continue
 		}
 
@@ -487,7 +490,7 @@ func addUpdateVirtualServicesForIngress(
 		virtualService, err := getBaseVirtualServiceForIngress()
 		if err != nil {
 			ctxLogger.Errorf(common.CtxLogFormat, "addUpdateVirtualServicesForIngress",
-				virtualService.Name, virtualService.Namespace, sourceCluster, err.Error())
+				vsName, util.IstioSystemNamespace, sourceCluster, err.Error())
 			return err
 		}
 
@@ -740,7 +743,10 @@ func addUpdateDestinationRuleForSourceIngress(
 
 	for sourceCluster, drHosts := range sourceClusterToDRHosts {
 
-		if !common.DoVSRoutingForCluster(sourceCluster) {
+		if common.IsVSRoutingDisabledForCluster(sourceCluster) {
+			ctxLogger.Infof(common.CtxLogFormat, "VSBasedRouting",
+				"", "", sourceCluster,
+				"Writing phase: addUpdateVirtualServicesForIngress VS based routing disabled for cluster")
 			continue
 		}
 

admiral/pkg/clusters/virtualservice_routing_test.go

@@ -69,7 +69,6 @@ func TestAddUpdateVirtualServicesForIngress(t *testing.T) {
 		IngressVSExportToNamespaces: []string{"istio-system"},
 		VSRoutingGateways:           []string{"istio-system/passthrough-gateway"},
 		EnableVSRouting:             true,
-		VSRoutingEnabledClusters:    []string{"cluster-1"},
 	}
 	common.ResetSync()
 	common.InitializeConfig(admiralParams)
@@ -2381,7 +2380,6 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
 		SANPrefix:                         "test-san-prefix",
 		IngressVSExportToNamespaces:       []string{"istio-system"},
 		EnableVSRouting:                   true,
-		VSRoutingEnabledClusters:          []string{"cluster-1", "cluster-2"},
 		VSRoutingSlowStartEnabledClusters: []string{"cluster-1"},
 	}
 	common.ResetSync()

admiral/pkg/controller/common/config.go

@@ -444,13 +444,13 @@ func GetIngressVSExportToNamespace() []string {
 	return wrapper.params.IngressVSExportToNamespaces
 }
 
-func DoVSRoutingForCluster(cluster string) bool {
+func IsVSRoutingDisabledForCluster(cluster string) bool {
 	wrapper.RLock()
 	defer wrapper.RUnlock()
 	if !wrapper.params.EnableVSRouting {
 		return false
 	}
-	for _, c := range wrapper.params.VSRoutingEnabledClusters {
+	for _, c := range wrapper.params.VSRoutingDisabledClusters {
 		if c == "*" {
 			return true
 		}

admiral/pkg/controller/common/config_test.go

@@ -168,71 +168,71 @@ func TestDoRoutingPolicyForCluster(t *testing.T) {
 	}
 }
 
-func TestDoVSRoutingForCluster(t *testing.T) {
+func TestIsVSRoutingDisabledForCluster(t *testing.T) {
 	p := AdmiralParams{}
 
 	testCases := []struct {
-		name                      string
-		cluster                   string
-		enableVSRouting           bool
-		enableVSRoutingForCluster []string
-		expected                  bool
+		name                        string
+		cluster                     string
+		enableVSRouting             bool
+		disabledVSRoutingForCluster []string
+		expected                    bool
 	}{
 		{
-			name: "Given enableVSRouting is false, enableVSRoutingForCluster is empty" +
-				"When DoVSRoutingForCluster is called" +
+			name: "Given enableVSRouting is false, disabledVSRoutingForCluster is empty" +
+				"When IsVSRoutingDisabledForCluster is called" +
 				"Then it should return false",
-			cluster:                   "cluster1",
-			enableVSRouting:           false,
-			enableVSRoutingForCluster: []string{},
-			expected:                  false,
+			cluster:                     "cluster1",
+			enableVSRouting:             false,
+			disabledVSRoutingForCluster: []string{},
+			expected:                    false,
 		},
 		{
-			name: "Given enableVSRouting is true, enableVSRoutingForCluster is empty" +
-				"When DoVSRoutingForCluster is called" +
+			name: "Given enableVSRouting is true, disabledVSRoutingForCluster is empty" +
+				"When IsVSRoutingDisabledForCluster is called" +
 				"Then it should return false",
-			cluster:                   "cluster1",
-			enableVSRouting:           true,
-			enableVSRoutingForCluster: []string{},
-			expected:                  false,
+			cluster:                     "cluster1",
+			enableVSRouting:             true,
+			disabledVSRoutingForCluster: []string{},
+			expected:                    false,
 		},
 		{
 			name: "Given enableVSRouting is true, and given cluster doesn't exists in the list" +
-				"When DoVSRoutingForCluster is called" +
+				"When IsVSRoutingDisabledForCluster is called" +
 				"Then it should return false",
-			cluster:                   "cluster2",
-			enableVSRouting:           true,
-			enableVSRoutingForCluster: []string{"cluster1"},
-			expected:                  false,
+			cluster:                     "cluster2",
+			enableVSRouting:             true,
+			disabledVSRoutingForCluster: []string{"cluster1"},
+			expected:                    false,
 		},
 		{
 			name: "Given enableVSRouting is true, and given cluster does exists in the list" +
-				"When DoVSRoutingForCluster is called" +
-				"Then it should return false",
-			cluster:                   "cluster1",
-			enableVSRouting:           true,
-			enableVSRoutingForCluster: []string{"cluster1"},
-			expected:                  true,
+				"When IsVSRoutingDisabledForCluster is called" +
+				"Then it should return true",
+			cluster:                     "cluster1",
+			enableVSRouting:             true,
+			disabledVSRoutingForCluster: []string{"cluster1"},
+			expected:                    true,
 		},
 		{
 			name: "Given enableVSRouting is true, and all VS routing is enabled in all clusters using '*'" +
-				"When DoVSRoutingForCluster is called" +
-				"Then it should return false",
-			cluster:                   "cluster1",
-			enableVSRouting:           true,
-			enableVSRoutingForCluster: []string{"*"},
-			expected:                  true,
+				"When IsVSRoutingDisabledForCluster is called" +
+				"Then it should return true",
+			cluster:                     "cluster1",
+			enableVSRouting:             true,
+			disabledVSRoutingForCluster: []string{"*"},
+			expected:                    true,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			p.EnableVSRouting = tc.enableVSRouting
-			p.VSRoutingEnabledClusters = tc.enableVSRoutingForCluster
+			p.VSRoutingDisabledClusters = tc.disabledVSRoutingForCluster
 			ResetSync()
 			InitializeConfig(p)
 
-			assert.Equal(t, tc.expected, DoVSRoutingForCluster(tc.cluster))
+			assert.Equal(t, tc.expected, IsVSRoutingDisabledForCluster(tc.cluster))
 		})
 	}
 

admiral/pkg/controller/common/types.go

@@ -135,7 +135,7 @@ type AdmiralParams struct {
 	VSRoutingGateways                 []string
 	IngressVSExportToNamespaces       []string
 	IngressLBPolicy                   string
-	VSRoutingEnabledClusters          []string
+	VSRoutingDisabledClusters         []string
 	VSRoutingSlowStartEnabledClusters []string
 
 	//Client discovery (types requiring mesh egress only)