added a cluster allowlist param for vs based routing

Signed-off-by: Shriram Sharma <[email protected]>

admiral/cmd/admiral/cmd/root.go

@@ -254,6 +254,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().StringArrayVar(&params.VSRoutingEnabledClusters, "vs_routing_enabled_clusters", []string{}, "The source clusters to enable VS based routing on")
 	rootCmd.PersistentFlags().StringArrayVar(&params.VSRoutingGateways, "vs_routing_gateways", []string{}, "The PASSTHROUGH gateways to use for VS based routing")
 	rootCmd.PersistentFlags().StringArrayVar(&params.IngressVSExportToNamespaces, "ingress_vs_export_to_namespaces", []string{"istio-system"}, "List of namespaces where the ingress VS should be exported")
 	rootCmd.PersistentFlags().StringVar(&params.IngressLBPolicy, "ingress_lb_policy", "round_robin", "loadbalancer policy for ingress destination rule (round_robin/random/passthrough/least_request)")

admiral/pkg/clusters/serviceentry.go

@@ -759,7 +759,7 @@ func modifyServiceEntryForNewServiceOrPod(
 			remoteRegistry.AdmiralCache.DependencyNamespaceCache.Put(val, serviceInstance[appType[sourceCluster]].Namespace, localFqdn, cnames)
 		}
 
-		if common.IsVSBasedRoutingEnabled() {
+		if common.DoVSRoutingForCluster(sourceCluster) {
 			// Discovery phase: This is where we build a map of all the svc.cluster.local destinations
 			// for the identity's source cluster. This map will contain the RouteDestination of all svc.cluster.local
 			// endpoints.
@@ -779,7 +779,6 @@ func modifyServiceEntryForNewServiceOrPod(
 					"No RouteDestinations generated for VS based routing ")
 			} else {
 				sourceClusterToDestinations[sourceCluster] = destinations
-				// Get the hosts to populate the DR
 				drHost := fmt.Sprintf("*.%s.%s", deploymentOrRolloutNS, common.DotLocalDomainSuffix)
 				sourceClusterToDRHosts[sourceCluster] = map[string]string{
 					deploymentOrRolloutNS + common.DotLocalDomainSuffix: drHost,
@@ -802,28 +801,26 @@ func modifyServiceEntryForNewServiceOrPod(
 		ctxLogger.Infof(common.CtxLogFormat, "updateRegistryConfigForClusterPerEnvironment", deploymentOrRolloutName, deploymentOrRolloutNS, "", "done writing")
 		return nil, nil
 	}
-	// If VS based routing is enabled, then generate VirtualServices for the source cluster's ingress
-	// This is done after the ServiceEntries are created for the source cluster
-	if common.IsVSBasedRoutingEnabled() {
-		// Writing phase: We update the base ingress virtualservices with the RouteDestinations
-		// gathered during the discovery phase and write them to the source cluster
-		err := addUpdateVirtualServicesForSourceIngress(ctx, ctxLogger, remoteRegistry, sourceClusterToDestinations)
+
+	// VS Based Routing
+	// Writing phase: We update the base ingress virtualservices with the RouteDestinations
+	// gathered during the discovery phase and write them to the source cluster
+	err = addUpdateVirtualServicesForSourceIngress(ctx, ctxLogger, remoteRegistry, sourceClusterToDestinations)
+	if err != nil {
+		ctxLogger.Errorf(common.CtxLogFormat, "addUpdateVirtualServicesForSourceIngress",
+			deploymentOrRolloutName, deploymentOrRolloutNS, "", err)
+		modifySEerr = common.AppendError(modifySEerr, err)
+	} else {
+		err := addUpdateDestinationRuleForSourceIngress(
+			ctx,
+			ctxLogger,
+			remoteRegistry,
+			sourceClusterToDRHosts,
+			sourceIdentity)
 		if err != nil {
-			ctxLogger.Errorf(common.CtxLogFormat, "addUpdateVirtualServicesForSourceIngress",
+			ctxLogger.Errorf(common.CtxLogFormat, "addUpdateDestinationRuleForSourceIngress",
 				deploymentOrRolloutName, deploymentOrRolloutNS, "", err)
 			modifySEerr = common.AppendError(modifySEerr, err)
-		} else {
-			err := addUpdateDestinationRuleForSourceIngress(
-				ctx,
-				ctxLogger,
-				remoteRegistry,
-				sourceClusterToDRHosts,
-				sourceIdentity)
-			if err != nil {
-				ctxLogger.Errorf(common.CtxLogFormat, "addUpdateDestinationRuleForSourceIngress",
-					deploymentOrRolloutName, deploymentOrRolloutNS, "", err)
-				modifySEerr = common.AppendError(modifySEerr, err)
-			}
 		}
 	}
 

admiral/pkg/clusters/virtualservice_routing.go

@@ -454,15 +454,16 @@ func addUpdateVirtualServicesForSourceIngress(
 	remoteRegistry *RemoteRegistry,
 	sourceClusterToDestinations map[string]map[string][]*networkingV1Alpha3.RouteDestination) error {
 
-	if remoteRegistry == nil {
-		return fmt.Errorf("remoteRegistry is nil")
-	}
+	for sourceCluster, destination := range sourceClusterToDestinations {
 
-	if len(sourceClusterToDestinations) == 0 {
-		return fmt.Errorf("no route destination found for the ingress virtualservice")
-	}
+		if !common.DoVSRoutingForCluster(sourceCluster) {
+			continue
+		}
+
+		if remoteRegistry == nil {
+			return fmt.Errorf("remoteRegistry is nil")
+		}
 
-	for sourceCluster, destination := range sourceClusterToDestinations {
 		rc := remoteRegistry.GetRemoteController(sourceCluster)
 
 		if rc == nil {
@@ -632,13 +633,17 @@ func addUpdateDestinationRuleForSourceIngress(
 	sourceClusterToDRHosts map[string]map[string]string,
 	sourceIdentity string) error {
 
-	if sourceIdentity == "" {
-		return fmt.Errorf("sourceIdentity is empty")
-	}
+	for sourceCluster, drHosts := range sourceClusterToDRHosts {
+
+		if !common.DoVSRoutingForCluster(sourceCluster) {
+			continue
+		}
 
-	san := fmt.Sprintf("%s%s/%s", common.SpiffePrefix, common.GetSANPrefix(), sourceIdentity)
+		if sourceIdentity == "" {
+			return fmt.Errorf("sourceIdentity is empty")
+		}
 
-	for sourceCluster, drHosts := range sourceClusterToDRHosts {
+		san := fmt.Sprintf("%s%s/%s", common.SpiffePrefix, common.GetSANPrefix(), sourceIdentity)
 
 		rc := remoteRegistry.GetRemoteController(sourceCluster)
 		if rc == nil {
@@ -689,6 +694,9 @@ func addUpdateDestinationRuleForSourceIngress(
 				existingDR = nil
 			}
 
+			ctxLogger.Infof(common.CtxLogFormat, "addUpdateDestinationRuleForSourceIngress",
+				drName, util.IstioSystemNamespace, sourceCluster, "Add/Update ingress destinationrule")
+
 			err = addUpdateDestinationRule(ctxLogger, ctx, newDR, existingDR, util.IstioSystemNamespace, rc, remoteRegistry)
 			if err != nil {
 				return err

admiral/pkg/clusters/virtualservice_routing_test.go

@@ -64,6 +64,8 @@ func TestAddUpdateVirtualServicesForSourceIngress(t *testing.T) {
 		EnableSWAwareNSCaches:       true,
 		IngressVSExportToNamespaces: []string{"istio-system"},
 		VSRoutingGateways:           []string{"istio-system/passthrough-gateway"},
+		EnableVSRouting:             true,
+		VSRoutingEnabledClusters:    []string{"cluster-1"},
 	}
 	common.ResetSync()
 	common.InitializeConfig(admiralParams)
@@ -174,15 +176,8 @@ func TestAddUpdateVirtualServicesForSourceIngress(t *testing.T) {
 			name: "Given a nil remoteRegistry, " +
 				"When addUpdateVirtualServicesForSourceIngress is invoked, " +
 				"Then it should return an error",
-			expectedError: fmt.Errorf("remoteRegistry is nil"),
-		},
-		{
-			name: "Given a no sourceClusterToDestinations, " +
-				"When addUpdateVirtualServicesForSourceIngress is invoked, " +
-				"Then it should return an error",
-			remoteRegistry:              rr,
-			sourceClusterToDestinations: map[string]map[string][]*networkingV1Alpha3.RouteDestination{},
-			expectedError:               fmt.Errorf("no route destination found for the ingress virtualservice"),
+			sourceClusterToDestinations: sourceDestinationsWithSingleDestinationSvc,
+			expectedError:               fmt.Errorf("remoteRegistry is nil"),
 		},
 		{
 			name: "Given a valid sourceClusterToDestinations " +
@@ -2178,6 +2173,8 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
 	admiralParams := common.AdmiralParams{
 		SANPrefix:                   "test-san-prefix",
 		IngressVSExportToNamespaces: []string{"istio-system"},
+		EnableVSRouting:             true,
+		VSRoutingEnabledClusters:    []string{"cluster-1"},
 	}
 	common.ResetSync()
 	common.InitializeConfig(admiralParams)
@@ -2206,6 +2203,11 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
 			name: "Given a empty sourceIdentity " +
 				"When addUpdateDestinationRuleForSourceIngress is invoked, " +
 				"Then it should return an error",
+			sourceClusterToDRHosts: map[string]map[string]string{
+				"cluster-1": {
+					"test-ns.svc.cluster.local": "*.test-ns.svc.cluster.local",
+				},
+			},
 			sourceIdentity: "",
 			expectedError:  fmt.Errorf("sourceIdentity is empty"),
 		},

admiral/pkg/controller/common/config.go

@@ -438,10 +438,18 @@ func GetIngressVSExportToNamespace() []string {
 	return wrapper.params.IngressVSExportToNamespaces
 }
 
-func IsVSBasedRoutingEnabled() bool {
+func DoVSRoutingForCluster(cluster string) bool {
 	wrapper.RLock()
 	defer wrapper.RUnlock()
-	return wrapper.params.EnableVSRouting
+	if !wrapper.params.EnableVSRouting {
+		return false
+	}
+	for _, c := range wrapper.params.VSRoutingEnabledClusters {
+		if c == cluster {
+			return true
+		}
+	}
+	return false
 }
 
 func GetVSRoutingGateways() []string {

admiral/pkg/controller/common/types.go

@@ -125,6 +125,7 @@ type AdmiralParams struct {
 	VSRoutingGateways           []string
 	IngressVSExportToNamespaces []string
 	IngressLBPolicy             string
+	VSRoutingEnabledClusters    []string
 }
 
 func (b AdmiralParams) String() string {