From 8203b939a9d1f1bca87b4f1e6f384795746e8b63 Mon Sep 17 00:00:00 2001 From: Roy Hadad <39004075+royhadad@users.noreply.github.com> Date: Wed, 25 May 2022 13:11:26 +0300 Subject: [PATCH] fix: exclude defaultSchemaLocations in offline k8s validation (#643) --- bl/validation/k8sValidator.go | 63 ++++++++++++++-------- bl/validation/k8sValidator_test.go | 87 +++++++++++++++++++++--------- 2 files changed, 103 insertions(+), 47 deletions(-) diff --git a/bl/validation/k8sValidator.go b/bl/validation/k8sValidator.go index eac832a82a..572c70818e 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 { diff --git a/bl/validation/k8sValidator_test.go b/bl/validation/k8sValidator_test.go index 2ec62541e6..a4a34240c7 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) +}