Merge pull request #4283 from hhovsepy/issue3061_2

Added support of exported DestinationRules for SubsetPresenceChecker of VirtualSevices. Added tests.
kiali · Aug 19, 2021 · 2a717fd · 2a717fd
2 parents f633c43 + 1998d5c
commit 2a717fd
Show file tree

Hide file tree

Showing 5 changed files with 211 additions and 11 deletions.
diff --git a/business/checkers/virtual_service_checker.go b/business/checkers/virtual_service_checker.go
@@ -64,7 +64,7 @@ func (in VirtualServiceChecker) runChecks(virtualService kubernetes.IstioObject)
 
 	enabledCheckers := []Checker{
 		virtualservices.RouteChecker{Route: virtualService},
-		virtualservices.SubsetPresenceChecker{Namespace: in.Namespace, Namespaces: in.Namespaces.GetNames(), DestinationRules: in.DestinationRules, VirtualService: virtualService},
+		virtualservices.SubsetPresenceChecker{Namespace: in.Namespace, Namespaces: in.Namespaces.GetNames(), DestinationRules: in.DestinationRules, VirtualService: virtualService, ExportedDestinationRules: in.ExportedDestinationRules},
 		common.ExportToNamespaceChecker{IstioObject: virtualService, Namespaces: in.Namespaces},
 	}
 

diff --git a/business/checkers/virtualservices/subset_presence_checker.go b/business/checkers/virtualservices/subset_presence_checker.go
@@ -9,10 +9,11 @@ import (
 )
 
 type SubsetPresenceChecker struct {
-	Namespace        string
-	Namespaces       []string
-	DestinationRules []kubernetes.IstioObject
-	VirtualService   kubernetes.IstioObject
+	Namespace                string
+	Namespaces               []string
+	DestinationRules         []kubernetes.IstioObject
+	ExportedDestinationRules []kubernetes.IstioObject
+	VirtualService           kubernetes.IstioObject
 }
 
 func (checker SubsetPresenceChecker) Check() ([]*models.IstioCheck, bool) {
@@ -93,9 +94,9 @@ func (checker SubsetPresenceChecker) subsetPresent(host string, subset string) b
 }
 
 func (checker SubsetPresenceChecker) getDestinationRules(virtualServiceHost string) ([]kubernetes.IstioObject, bool) {
-	drs := make([]kubernetes.IstioObject, 0, len(checker.DestinationRules))
+	drs := make([]kubernetes.IstioObject, 0, len(checker.DestinationRules)+len(checker.ExportedDestinationRules))
 
-	for _, destinationRule := range checker.DestinationRules {
+	for _, destinationRule := range append(checker.DestinationRules, checker.ExportedDestinationRules...) {
 		host, ok := destinationRule.GetSpec()["host"]
 		if !ok {
 			continue

diff --git a/business/checkers/virtualservices/subset_presence_checker_test.go b/business/checkers/virtualservices/subset_presence_checker_test.go
@@ -14,6 +14,19 @@ func TestCheckerWithSubsetMatching(t *testing.T) {
 	testNoSubsetPresenceValidationsFound("subset-presence-matching-subsets-1.yaml", t)
 }
 
+func TestCheckerWithExportSubsetMatching(t *testing.T) {
+	testNoSubsetPresenceValidationsFound("subset-presence-export-subset.yaml", t)
+}
+
+func TestCheckerWithExportSubsetNotMatching(t *testing.T) {
+	vals, valid := subsetPresenceCheckerPrep("subset-presence-no-matching-export-subset.yaml", t)
+
+	tb := validations.IstioCheckTestAsserter{T: t, Validations: vals, Valid: valid}
+	tb.AssertValidationsPresent(2, true)
+	tb.AssertValidationAt(0, models.WarningSeverity, "spec/http[0]/route[0]/destination", "virtualservices.subsetpresent.subsetnotfound")
+	tb.AssertValidationAt(1, models.WarningSeverity, "spec/http[0]/route[1]/destination", "virtualservices.subsetpresent.subsetnotfound")
+}
+
 func TestCheckerWithSubsetsMatchingShortHostname(t *testing.T) {
 	testNoSubsetPresenceValidationsFound("subset-presence-matching-subsets-2.yaml", t)
 }
@@ -67,10 +80,11 @@ func subsetPresenceCheckerPrep(scenario string, t *testing.T) ([]*models.IstioCh
 	err := loader.Load()
 
 	vals, valid := SubsetPresenceChecker{
-		Namespace:        "bookinfo",
-		Namespaces:       namespaceNames(loader.GetResources("Namespace")),
-		DestinationRules: loader.GetResources("DestinationRule"),
-		VirtualService:   loader.GetFirstResource("VirtualService"),
+		Namespace:                "bookinfo",
+		Namespaces:               namespaceNames(loader.GetResources("Namespace")),
+		DestinationRules:         loader.GetResourcesIn("DestinationRule", "bookinfo"),
+		ExportedDestinationRules: loader.GetResourcesNotIn("DestinationRule", "bookinfo"),
+		VirtualService:           loader.GetFirstResource("VirtualService"),
 	}.Check()
 
 	if err != nil {

diff --git a/tests/data/validations/virtualservices/subset-presence-export-subset.yaml b/tests/data/validations/virtualservices/subset-presence-export-subset.yaml
@@ -0,0 +1,89 @@
+# No validations found
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: bookinfo
+  labels:
+    istio-injection: "enabled"
+spec: {}
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: bookinfo2
+  labels:
+    istio-injection: "enabled"
+spec: {}
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: bookinfo3
+  labels:
+    istio-injection: "enabled"
+spec: {}
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: testrule
+  namespace: bookinfo
+spec:
+  host: reviews.bookinfo.svc.cluster.local
+  subsets:
+    - labels:
+        version: v2
+      name: v2
+  exportTo:
+    - '.'
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: testrule
+  namespace: bookinfo2
+spec:
+  host: reviews
+  subsets:
+    - labels:
+        version: v1
+      name: v1
+  exportTo:
+    - bookinfo
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: testrule
+  namespace: bookinfo3
+spec:
+  host: reviews
+  subsets:
+    - labels:
+        version: v3
+      name: v3
+  exportTo:
+    - '*'
+---
+apiVersion: networking.istio.io/v1beta1
+kind: VirtualService
+metadata:
+  name: reviews-vs
+  namespace: bookinfo
+spec:
+  hosts:
+    - reviews.bookinfo.svc.cluster.local
+  http:
+    - route:
+        - destination:
+            host: reviews.bookinfo2.svc.cluster.local
+            subset: v1
+          weight: 33
+        - destination:
+            host: reviews.bookinfo.svc.cluster.local
+            subset: v2
+          weight: 33
+        - destination:
+            host: reviews.bookinfo3.svc.cluster.local
+            subset: v3
+          weight: 34
diff --git a/tests/data/validations/virtualservices/subset-presence-no-matching-export-subset.yaml b/tests/data/validations/virtualservices/subset-presence-no-matching-export-subset.yaml
@@ -0,0 +1,96 @@
+# No validations found
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: bookinfo
+  labels:
+    istio-injection: "enabled"
+spec: {}
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: bookinfo2
+  labels:
+    istio-injection: "enabled"
+spec: {}
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: bookinfo3
+  labels:
+    istio-injection: "enabled"
+spec: {}
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: testrule
+  namespace: bookinfo
+spec:
+  host: reviews.bookinfo.svc.cluster.local
+  subsets:
+    - labels:
+        version: v2
+      name: v2
+    - labels:
+        version: v3
+      name: v3
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: testrule
+  namespace: bookinfo2
+spec:
+  host: reviews
+  subsets:
+    - labels:
+        version: v4
+      name: v4
+  exportTo:
+    - '*'
+---
+apiVersion: networking.istio.io/v1alpha3
+kind: DestinationRule
+metadata:
+  name: testrule
+  namespace: bookinfo3
+spec:
+  host: reviews
+  subsets:
+    - labels:
+        version: v5
+      name: v5
+    - labels:
+        version: v6
+      name: v6
+    - labels:
+        version: v7
+      name: v7
+  exportTo:
+    - '*'
+---
+apiVersion: networking.istio.io/v1beta1
+kind: VirtualService
+metadata:
+  name: reviews-vs
+  namespace: bookinfo
+spec:
+  hosts:
+    - reviews.bookinfo.svc.cluster.local
+  http:
+    - route:
+        - destination:
+            host: reviews.bookinfo2.svc.cluster.local
+            subset: v1
+          weight: 33
+        - destination:
+            host: reviews.bookinfo3.svc.cluster.local
+            subset: v3
+          weight: 34
+        - destination:
+            host: reviews.bookinfo.svc.cluster.local
+            subset: v2
+          weight: 33