Repository: datreeio/datree
Pull request #589 (2022-04-27 13:33:35)

feat: print files that skipped k8s validation


Patch:
diff --git a/bl/evaluation/printer.go b/bl/evaluation/printer.go
index dc063c0..f05a2d1 100644
--- a/bl/evaluation/printer.go
+++ b/bl/evaluation/printer.go
@@ -282,7 +282,7 @@ func parseToPrinterWarnings(results *EvaluationResults, invalidYamlFiles []*extr
 				SkippedRules:    skippedRules,
 				InvalidYamlInfo: printer.InvalidYamlInfo{},
 				InvalidK8sInfo: printer.InvalidK8sInfo{
-					ValidationWarning: k8sValidationWarnings[filename],
+					ValidationWarning: k8sValidationWarnings[filename].Warning,
 				},
 			})
 		}
diff --git a/bl/validation/k8sValidator.go b/bl/validation/k8sValidator.go
index 165d5ca..f789076 100644
--- a/bl/validation/k8sValidator.go
+++ b/bl/validation/k8sValidator.go
@@ -18,7 +18,7 @@ type K8sValidator struct {
 	validationClient ValidationClient
 }
 
-type K8sValidationWarningPerValidFile map[string]string
+type K8sValidationWarningPerValidFile map[string]FileWithWarning
 
 func New() *K8sValidator {
 	return &K8sValidator{}
@@ -28,9 +28,18 @@ func (val *K8sValidator) InitClient(k8sVersion string, ignoreMissingSchemas bool
 	val.validationClient = newKubeconformValidator(k8sVersion, ignoreMissingSchemas, append(getDefaultSchemaLocations(), schemaLocations...))
 }
 
+type WarningKind int
+
+const (
+	_            WarningKind = iota
+	NetworkError             // a network error while validating the resource
+	Skipped                  // resource has been skipped, for example if its kind was not found and the user added the --ignore-missing-schemas flag
+)
+
 type FileWithWarning struct {
-	Filename string
-	Warning  string
+	Filename    string
+	Warning     string
+	WarningKind WarningKind
 }
 
 func (val *K8sValidator) ValidateResources(filesConfigurationsChan chan *extractor.FileConfigurations, concurrency int) (chan *extractor.FileConfigurations, chan *extractor.InvalidFile, chan *FileWithWarning) {
@@ -57,10 +66,11 @@ func (val *K8sValidator) ValidateResources(filesConfigurationsChan chan *extract
 			}
 			if isValid {
 				validK8sFilesConfigurationsChan <- fileConfigurations
-				if validationWarning != "" {
+				if validationWarning != nil {
 					k8sValidationWarningPerValidFileChan <- &FileWithWarning{
-						Filename: fileConfigurations.FileName,
-						Warning:  validationWarning,
+						Filename:    fileConfigurations.FileName,
+						Warning:     validationWarning.WarningMessage,
+						WarningKind: validationWarning.WarningKind,
 					}
 				}
 			} else {
@@ -110,10 +120,15 @@ func (val *K8sValidator) isK8sFile(fileConfigurations []extractor.Configuration)
 	return true
 }
 
-func (val *K8sValidator) validateResource(filepath string) (bool, []error, string, error) {
+type validationWarning struct {
+	WarningKind    WarningKind
+	WarningMessage string
+}
+
+func (val *K8sValidator) validateResource(filepath string) (bool, []error, *validationWarning, error) {
 	f, err := os.Open(filepath)
 	if err != nil {
-		return false, []error{}, "", fmt.Errorf("failed opening %s: %s", filepath, &InvalidK8sSchemaError{ErrorMessage: err.Error()})
+		return false, []error{}, nil, fmt.Errorf("failed opening %s: %s", filepath, &InvalidK8sSchemaError{ErrorMessage: err.Error()})
 	}
 
 	defer f.Close()
@@ -123,18 +138,25 @@ func (val *K8sValidator) validateResource(filepath string) (bool, []error, strin
 	// Return an error if no valid configurations found
 	// Empty files are throwing errors in k8s
 	if isEveryResultStatusEmpty(results) {
-		return false, []error{&InvalidK8sSchemaError{ErrorMessage: "empty file"}}, "", nil
+		return false, []error{&InvalidK8sSchemaError{ErrorMessage: "empty file"}}, nil, nil
 	}
 
 	isValid := true
+	isAtLeastOneConfigSkipped := false
 	var validationErrors []error
 	for _, res := range results {
 		// A file might contain multiple resources
 		// File starts with ---, the parser assumes a first empty resource
+		if res.Status == kubeconformValidator.Skipped {
+			isAtLeastOneConfigSkipped = true
+		}
 		if res.Status == kubeconformValidator.Invalid || res.Status == kubeconformValidator.Error {
 			if val.isNetworkError(res.Err.Error()) {
-				noConnectionWarning := "k8s schema validation skipped: no internet connection"
-				return isValid, []error{}, noConnectionWarning, nil
+				noConnectionWarning := &validationWarning{
+					WarningKind:    NetworkError,
+					WarningMessage: "k8s schema validation skipped: no internet connection",
+				}
+				return true, []error{}, noConnectionWarning, nil
 			}
 			isValid = false
 
@@ -149,8 +171,14 @@ func (val *K8sValidator) validateResource(filepath string) (bool, []error, strin
 			}
 		}
 	}
-
-	return isValid, validationErrors, "", nil
+	var warning *validationWarning = nil
+	if isAtLeastOneConfigSkipped && isValid {
+		warning = &validationWarning{
+			WarningKind:    Skipped,
+			WarningMessage: "k8s schema validation skipped: --ignore-missing-schemas flag was used",
+		}
+	}
+	return isValid, validationErrors, warning, nil
 }
 
 func (val *K8sValidator) isNetworkError(errorString string) bool {
diff --git a/internal/fixtures/kube/combinations.yaml b/internal/fixtures/kube/combinations.yaml
new file mode 100644
index 0000000..c9b07b3
--- /dev/null
+++ b/internal/fixtures/kube/combinations.yaml
@@ -0,0 +1,103 @@
+# Pass all
+apiVersion: networking.k8s.io/v1beta1
+kind: Ingress
+metadata:
+  name: ingress-myservicea
+  annotations:
+    kubernetes.io/ingress.class: "nginx"
+spec:
+  rules:
+    - host: myservicea.foo.org
+      http:
+        paths:
+          - path: /
+            backend:
+              serviceName: myservicea
+              servicePort: 80
+---
+# Skipped K8s schema validation
+apiVersion: v1
+kind: CustomResourceDefinitiona
+metadata:
+  name: my-service
+spec:
+  selector:
+    app: MyApp
+  ports:
+    - protocol: TCP
+      port: 80
+      targetPort: 9376
+---
+# Invalid k8s schema
+apiversion: v1
+kind: Service
+metadata:
+  name: my-service
+spec:
+  selector:
+    app: MyApp
+  ports:
+    - protocol: TCP
+      port: 80
+      targetPort: 9376
+---
+# Fail policy check
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: example-depl
+  namespace: exmpl
+  labels:
+    environment: prod
+    app: web
+  annotations:
+    datree.io/skip/CONTAINERS_MISSING_MEMORY_LIMIT_KEY:  ''
+spec:
+  replicas: 2
+  selector:
+    matchLabels:
+      app: web
+  template:
+    metadata:
+      namespace: exmpl
+      labels:
+        app: web
+    spec:
+      containers:
+        - name: front-end
+          image: nginx:latest
+          readinessProbe:
+            tcpSocket:
+              port: 8080
+            initialDelaySeconds: 5
+            periodSeconds: 10
+          resources:
+            requests:
+              cpu: "64m"
+            limits:
+              cpu: "500m"
+          ports:
+            - containerPort: 80
+        - name: rss-reader
+          image: datree/nginx@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2
+          livenessProbe:
+            httpGet:
+              path: /healthz
+              port: 8080
+              httpHeaders:
+                - name: Custom-Header
+                  value: Awesome
+          readinessProbe:
+            tcpSocket:
+              port: 8080
+            initialDelaySeconds: 5
+            periodSeconds: 10
+          resources:
+            requests:
+              cpu: "64m"
+              memory: "128Mi"
+            limits:
+              memory: "128Mi"
+              cpu: "500m"
+          ports:
+            - containerPort: 88
diff --git a/internal/fixtures/kube/invalid-kind.yaml b/internal/fixtures/kube/invalid-kind.yaml
new file mode 100644
index 0000000..c85e96c
--- /dev/null
+++ b/internal/fixtures/kube/invalid-kind.yaml
@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Serviceee # not a real kind
+metadata:
+  name: my-service
+spec:
+  selector:
+    app: MyApp
+  ports:
+    - protocol: TCP
+      port: 80
+      targetPort: 9376
diff --git a/pkg/printer/printer.go b/pkg/printer/printer.go
index 4860060..611de28 100644
--- a/pkg/printer/printer.go
+++ b/pkg/printer/printer.go
@@ -102,11 +102,9 @@ func (p *Printer) printK8sValidationError(warning Warning) {
 }
 
 func (p *Printer) printK8sValidationWarning(warning Warning) {
-	fmt.Println("[?] Kubernetes schema validation")
 	fmt.Fprintln(out)
-
+	fmt.Println("[?] Kubernetes schema validation")
 	fmt.Println(warning.InvalidK8sInfo.ValidationWarning)
-	fmt.Fprintln(out)
 }
 
 func (p *Printer) PrintYamlSchemaResults(errorsResult []jsonschema.Detailed, error error) {


Test patch:
diff --git a/bl/validation/k8sValidator_test.go b/bl/validation/k8sValidator_test.go
index be9d01e..31e3de4 100644
--- a/bl/validation/k8sValidator_test.go
+++ b/bl/validation/k8sValidator_test.go
@@ -29,6 +29,7 @@ func TestValidateResources(t *testing.T) {
 	test_get_datree_crd_schema_by_name(t)
 	t.Run("test empty file", test_empty_file)
 	t.Run("test no internet connection", test_no_connection)
+	t.Run("test missing schema skipped", test_missing_schema_skipped)
 }
 
 func test_valid_multiple_configurations(t *testing.T) {
@@ -157,14 +158,54 @@ func test_no_connection(t *testing.T) {
 			_ = p
 		}
 		for p := range filesWithWarningsChan {
-			k8sValidationWarningPerValidFile[p.Filename] = p.Warning
+			k8sValidationWarningPerValidFile[p.Filename] = *p
 		}
 		wg.Done()
 	}()
 	wg.Wait()
 
 	assert.Equal(t, 1, len(k8sValidationWarningPerValidFile))
-	assert.Equal(t, "k8s schema validation skipped: no internet connection", k8sValidationWarningPerValidFile["../../internal/fixtures/kube/pass-all.yaml"])
+	assert.Equal(t, "k8s schema validation skipped: no internet connection", k8sValidationWarningPerValidFile[path].Warning)
+}
+
+func test_missing_schema_skipped(t *testing.T) {
+	validationClient := &mockValidationClient{}
+	validationClient.On("Validate", mock.Anything, mock.Anything).Return([]kubeconformValidator.Result{
+		{Status: kubeconformValidator.Skipped, Err: nil},
+	})
+	k8sValidator := K8sValidator{
+		validationClient: validationClient,
+	}
+
+	path := "../../internal/fixtures/kube/invalid-kind.yaml"
+
+	filesConfigurationsChan := make(chan *extractor.FileConfigurations, 1)
+	filesConfigurationsChan <- &extractor.FileConfigurations{
+		FileName:       path,
+		Configurations: []extractor.Configuration{},
+	}
+	close(filesConfigurationsChan)
+	k8sValidationWarningPerValidFile := make(K8sValidationWarningPerValidFile)
+
+	var wg sync.WaitGroup
+	filesConfigurationsChanRes, invalidFilesChan, filesWithWarningsChan := k8sValidator.ValidateResources(filesConfigurationsChan, 1)
+	wg.Add(1)
+	go func() {
+		for p := range filesConfigurationsChanRes {
+			_ = p
+		}
+		for p := range invalidFilesChan {
+			_ = p
+		}
+		for p := range filesWithWarningsChan {
+			k8sValidationWarningPerValidFile[p.Filename] = *p
+		}
+		wg.Done()
+	}()
+	wg.Wait()
+
+	assert.Equal(t, 1, len(k8sValidationWarningPerValidFile))
+	assert.Equal(t, "k8s schema validation skipped: --ignore-missing-schemas flag was used", k8sValidationWarningPerValidFile[path].Warning)
 }
 
 func test_default_schema_location(t *testing.T) {
diff --git a/cmd/test/main_test.go b/cmd/test/main_test.go
index 8d0b30c..5af025e 100644
--- a/cmd/test/main_test.go
+++ b/cmd/test/main_test.go
@@ -809,7 +809,11 @@ func TestTestCommandNoInternetConnection(t *testing.T) {
 	path := "valid/path"
 	filesConfigurationsChan := newFilesConfigurationsChan(path)
 	invalidK8sFilesChan := newInvalidK8sFilesChan()
-	K8sValidationWarnings := validation.K8sValidationWarningPerValidFile{"valid/path": "Validation warning message - no internet"}
+	K8sValidationWarnings := validation.K8sValidationWarningPerValidFile{"valid/path": validation.FileWithWarning{
+		Filename:    "valid/path",
+		Warning:     "Validation warning message - no internet",
+		WarningKind: validation.NetworkError,
+	}}
 
 	k8sValidatorMock.On("ValidateResources", mock.Anything, mock.Anything).Return(filesConfigurationsChan, invalidK8sFilesChan, K8sValidationWarnings, newErrorsChan())
 
diff --git a/cmd/test/validationManager.go b/cmd/test/validationManager.go
index f982303..cace472 100644
--- a/cmd/test/validationManager.go
+++ b/cmd/test/validationManager.go
@@ -64,23 +64,32 @@ func (v *ValidationManager) ValidK8sFilesConfigurations() []*extractor.FileConfi
 }
 
 func (v *ValidationManager) GetK8sValidationSummaryStr(filesCount int) string {
-	if v.hasFilesWithWarnings() {
+	if v.hasFilesWithWarningsOfKind(validation.NetworkError) {
 		return "skipped since there is no internet connection"
 	}
 
-	return fmt.Sprintf("%v/%v", v.ValidK8sFilesConfigurationsCount(), filesCount)
+	return fmt.Sprintf("%v/%v", v.ValidK8sFilesConfigurationsCount()-v.countFilesWithWarningsOfKind(validation.Skipped), filesCount)
 }
 
-func (v *ValidationManager) hasFilesWithWarnings() bool {
+func (v *ValidationManager) hasFilesWithWarningsOfKind(warningKind validation.WarningKind) bool {
 	for _, value := range v.k8sValidationWarningPerValidFile {
-		if value != "" {
+		if value.WarningKind == warningKind {
 			return true
 		}
 	}
-
 	return false
 }
 
+func (v *ValidationManager) countFilesWithWarningsOfKind(warningKind validation.WarningKind) int {
+	count := 0
+	for _, value := range v.k8sValidationWarningPerValidFile {
+		if value.WarningKind == warningKind {
+			count++
+		}
+	}
+	return count
+}
+
 func (v *ValidationManager) ValidK8sFilesConfigurationsCount() int {
 	return len(v.validK8sFilesConfigurations)
 }
@@ -97,7 +106,9 @@ func (v *ValidationManager) ValidK8sConfigurationsCount() int {
 
 func (v *ValidationManager) AggregateK8sValidationWarningsPerValidFile(filesWithWarningsChan chan *validation.FileWithWarning, wg *sync.WaitGroup) {
 	for fileWithWarning := range filesWithWarningsChan {
-		v.k8sValidationWarningPerValidFile[fileWithWarning.Filename] = fileWithWarning.Warning
+		if fileWithWarning != nil {
+			v.k8sValidationWarningPerValidFile[fileWithWarning.Filename] = *fileWithWarning
+		}
 	}
 	wg.Done()
 }