Repository: datreeio/datree
Pull request #643 (2022-05-24 15:39:50)

fix: exclude defaultSchemaLocations in offline k8s validation
### use cases:

**offline mode:** 
uses *only* the schema locations provided by the user
- no `--schema-location` provided: skips k8s validation with warning
- local `--schema-location` provided: validates normally
- remote `--schema-location` provided: will fail because of no internet and display the error message in the correct format

**online mode:** 
gives precedence to schema locations provided by the user, uses Datree's default schema locations as a fallback


Patch:
diff --git a/bl/validation/k8sValidator.go b/bl/validation/k8sValidator.go
index eac832a..572c708 100644
--- a/bl/validation/k8sValidator.go
+++ b/bl/validation/k8sValidator.go
@@ -3,8 +3,10 @@ package validation
 import (
 	"fmt"
 	"io"
+	"net/http"
 	"os"
 	"strings"
+	"time"
 
 	"github.com/datreeio/datree/pkg/extractor"
 	"github.com/datreeio/datree/pkg/utils"
@@ -16,7 +18,9 @@ type ValidationClient interface {
 }
 
 type K8sValidator struct {
-	validationClient ValidationClient
+	validationClient              ValidationClient
+	isOffline                     bool
+	areThereCustomSchemaLocations bool
 }
 
 type K8sValidationWarningPerValidFile map[string]FileWithWarning
@@ -25,8 +29,22 @@ func New() *K8sValidator {
 	return &K8sValidator{}
 }
 
-func (val *K8sValidator) InitClient(k8sVersion string, ignoreMissingSchemas bool, schemaLocations []string) {
-	val.validationClient = newKubeconformValidator(k8sVersion, ignoreMissingSchemas, getAllSchemaLocations(schemaLocations))
+func (val *K8sValidator) InitClient(k8sVersion string, ignoreMissingSchemas bool, userProvidedSchemaLocations []string) {
+	val.isOffline = checkIsOffline()
+	val.areThereCustomSchemaLocations = len(userProvidedSchemaLocations) > 0
+	val.validationClient = newKubeconformValidator(k8sVersion, ignoreMissingSchemas, getAllSchemaLocations(userProvidedSchemaLocations, val.isOffline))
+}
+
+func checkIsOffline() bool {
+	client := http.Client{
+		Timeout: 10 * time.Second,
+	}
+	resp, err := client.Get("https://www.githubstatus.com/api/v2/status.json")
+	if err == nil && resp != nil && resp.StatusCode == 200 {
+		return false
+	} else {
+		return true
+	}
 }
 
 type WarningKind int
@@ -134,6 +152,14 @@ func (val *K8sValidator) validateResource(filepath string) (bool, []error, *vali
 
 	defer f.Close()
 
+	if val.isOffline && !val.areThereCustomSchemaLocations {
+		var noConnectionWarning = &validationWarning{
+			WarningKind:    NetworkError,
+			WarningMessage: "k8s schema validation skipped: no internet connection",
+		}
+		return true, []error{}, noConnectionWarning, nil
+	}
+
 	results := val.validationClient.Validate(filepath, f)
 
 	// Return an error if no valid configurations found
@@ -152,22 +178,15 @@ func (val *K8sValidator) validateResource(filepath string) (bool, []error, *vali
 			isAtLeastOneConfigSkipped = true
 		}
 		if res.Status == kubeconformValidator.Invalid || res.Status == kubeconformValidator.Error {
-			if utils.IsNetworkError(res.Err.Error()) {
-				noConnectionWarning := &validationWarning{
-					WarningKind:    NetworkError,
-					WarningMessage: "k8s schema validation skipped: no internet connection",
-				}
-				return true, []error{}, noConnectionWarning, nil
-			}
 			isValid = false
+			errString := res.Err.Error()
 
-			errorMessages := strings.Split(res.Err.Error(), "-")
-
-			// errorMessages slice is not empty
-			if len(errorMessages) > 0 {
+			if utils.IsNetworkError(errString) {
+				validationErrors = append(validationErrors, &InvalidK8sSchemaError{errString})
+			} else {
+				errorMessages := strings.Split(errString, "-")
 				for _, errorMessage := range errorMessages {
-					msg := strings.Trim(errorMessage, " ")
-					validationErrors = append(validationErrors, &InvalidK8sSchemaError{ErrorMessage: msg})
+					validationErrors = append(validationErrors, &InvalidK8sSchemaError{ErrorMessage: strings.Trim(errorMessage, " ")})
 				}
 			}
 		}
@@ -197,11 +216,13 @@ func isEveryResultStatusEmpty(results []kubeconformValidator.Result) bool {
 	return isEveryResultStatusEmpty
 }
 
-func getAllSchemaLocations(userProvidedSchemaLocations []string) []string {
-	// order matters!
-	// it's important that provided schema locations (from --schema-locations flag) are *before* the default schema locations
-	// this will give them priority and allow using a local schema in offline mode
-	return append(userProvidedSchemaLocations, getDefaultSchemaLocations()...)
+func getAllSchemaLocations(userProvidedSchemaLocations []string, isOffline bool) []string {
+	if isOffline {
+		return userProvidedSchemaLocations
+	} else {
+		// order matters! userProvidedSchemaLocations get priority over defaultSchemaLocations
+		return append(userProvidedSchemaLocations, getDefaultSchemaLocations()...)
+	}
 }
 
 func getDefaultSchemaLocations() []string {


Test patch:
diff --git a/bl/validation/k8sValidator_test.go b/bl/validation/k8sValidator_test.go
index 2ec6254..a4a3424 100644
--- a/bl/validation/k8sValidator_test.go
+++ b/bl/validation/k8sValidator_test.go
@@ -25,11 +25,14 @@ func TestValidateResources(t *testing.T) {
 	test_valid_multiple_configurations(t)
 	test_valid_multiple_configurations_only_k8s_files(t)
 	test_invalid_file(t)
-	test_get_all_schema_locations(t)
+	test_get_all_schema_locations_online(t)
+	test_get_all_schema_locations_offline(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_offline_with_remote_custom_schema_location", test_offline_with_remote_custom_schema_location)
 	t.Run("test missing schema skipped", test_missing_schema_skipped)
+	t.Run("test_validateResource_offline_with_local_schema", test_validateResource_offline_with_local_schema)
+	t.Run("test_validateResource_offline_without_custom_schema_location", test_validateResource_offline_without_custom_schema_location)
 }
 
 func test_valid_multiple_configurations(t *testing.T) {
@@ -128,13 +131,15 @@ func test_empty_file(t *testing.T) {
 	}
 }
 
-func test_no_connection(t *testing.T) {
+func test_offline_with_remote_custom_schema_location(t *testing.T) {
 	validationClient := &mockValidationClient{}
 	validationClient.On("Validate", mock.Anything, mock.Anything).Return([]kubeconformValidator.Result{
 		{Status: kubeconformValidator.Error, Err: fmt.Errorf("no such host")},
 	})
 	k8sValidator := K8sValidator{
-		validationClient: validationClient,
+		validationClient:              validationClient,
+		areThereCustomSchemaLocations: true,
+		isOffline:                     true,
 	}
 
 	path := "../../internal/fixtures/kube/pass-all.yaml"
@@ -145,27 +150,15 @@ func test_no_connection(t *testing.T) {
 		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: no internet connection", k8sValidationWarningPerValidFile[path].Warning)
+	_, invalidFilesChan, filesWithWarningsChan := k8sValidator.ValidateResources(filesConfigurationsChan, 1)
+	for p := range invalidFilesChan {
+		assert.Equal(t, 1, len(p.ValidationErrors))
+		assert.Equal(t, "k8s schema validation error: no such host\n", p.ValidationErrors[0].Error())
+	}
+	for p := range filesWithWarningsChan {
+		panic("expected 0 warnings when custom --schema-location provided, instead got warning: " + p.Warning)
+	}
 }
 
 func test_missing_schema_skipped(t *testing.T) {
@@ -208,14 +201,22 @@ func test_missing_schema_skipped(t *testing.T) {
 	assert.Equal(t, "k8s schema validation skipped: --ignore-missing-schemas flag was used", k8sValidationWarningPerValidFile[path].Warning)
 }
 
-func test_get_all_schema_locations(t *testing.T) {
+func test_get_all_schema_locations_online(t *testing.T) {
 	expectedOutput := []string{
 		"/my-local-schema-location",
 		"default",
 		"https://raw.githubusercontent.com/yannh/kubernetes-json-schema/master/{{ .NormalizedKubernetesVersion }}/{{ .ResourceKind }}{{ .KindSuffix }}.json",
 		"https://raw.githubusercontent.com/datreeio/CRDs-catalog/master/argo/{{ .ResourceKind }}_{{ .ResourceAPIVersion }}.json",
 	}
-	actual := getAllSchemaLocations([]string{"/my-local-schema-location"})
+	actual := getAllSchemaLocations([]string{"/my-local-schema-location"}, false)
+	assert.Equal(t, expectedOutput, actual)
+}
+
+func test_get_all_schema_locations_offline(t *testing.T) {
+	expectedOutput := []string{
+		"/my-local-schema-location",
+	}
+	actual := getAllSchemaLocations([]string{"/my-local-schema-location"}, true)
 	assert.Equal(t, expectedOutput, actual)
 }
 
@@ -228,3 +229,37 @@ func test_get_datree_crd_schema_by_name(t *testing.T) {
 		t.Errorf("Expected: %s, Actual: %s", expectedOutput, actual)
 	}
 }
+
+func test_validateResource_offline_with_local_schema(t *testing.T) {
+	k8sValidator := &K8sValidator{
+		validationClient: newKubeconformValidator("1.21.0", false, getAllSchemaLocations([]string{
+			"some-path-to-non-existing-file-to-get-404.yaml",
+		}, true)),
+		isOffline:                     true,
+		areThereCustomSchemaLocations: true,
+	}
+
+	isValid, validationErrors, validationWarningResult, err := k8sValidator.validateResource("../../internal/fixtures/kube/pass-all.yaml")
+	var nilValidationWarning *validationWarning
+	assert.Equal(t, nil, err)
+	assert.Equal(t, false, isValid)
+	assert.Equal(t, "k8s schema validation error: could not find schema for Deployment\nYou can skip files with missing schemas instead of failing by using the `--ignore-missing-schemas` flag\n", validationErrors[0].Error())
+	assert.Equal(t, nilValidationWarning, validationWarningResult)
+}
+
+func test_validateResource_offline_without_custom_schema_location(t *testing.T) {
+	k8sValidator := &K8sValidator{
+		validationClient:              newKubeconformValidator("1.21.0", false, getAllSchemaLocations([]string{}, true)),
+		isOffline:                     true,
+		areThereCustomSchemaLocations: false,
+	}
+
+	isValid, validationErrors, validationWarningResult, err := k8sValidator.validateResource("../../internal/fixtures/kube/pass-all.yaml")
+	assert.Equal(t, nil, err)
+	assert.Equal(t, true, isValid)
+	assert.Equal(t, 0, len(validationErrors))
+	assert.Equal(t, &validationWarning{
+		WarningKind:    NetworkError,
+		WarningMessage: "k8s schema validation skipped: no internet connection",
+	}, validationWarningResult)
+}