Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support of exported DestinationRules for SubsetPresenceChecker of VirtualSevices. Added tests. #4283

Merged
merged 1 commit into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion business/checkers/virtual_service_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
}

Expand Down
13 changes: 7 additions & 6 deletions business/checkers/virtualservices/subset_presence_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
22 changes: 18 additions & 4 deletions business/checkers/virtualservices/subset_presence_checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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