Repository: datreeio/datree
Pull request #871 (2023-01-25 11:12:05)

fix: panic errors in extractConfigurationK8sData
when the resources "annotations" is an array instead of an object, we get this error:

<img width="754" alt="Screenshot 2023-01-25 at 13 11 34" src="https://user-images.githubusercontent.com/39004075/214548676-e20bdade-2a61-4dad-ad06-4a8fbb18add2.png">



Patch:
diff --git a/pkg/extractor/extractor.go b/pkg/extractor/extractor.go
index e272c8c..e65a17b 100644
--- a/pkg/extractor/extractor.go
+++ b/pkg/extractor/extractor.go
@@ -54,7 +54,7 @@ func ExtractConfigurationsFromYamlFile(path string) (*[]Configuration, string, *
 		return nil, "", &InvalidFile{Path: absolutePath, ValidationErrors: []error{&InvalidYamlError{ErrorMessage: err.Error()}}}
 	}
 
-	configurations, err := ParseYaml(content)
+	configurations, err := parseYaml(content)
 	if err != nil {
 		return nil, "", &InvalidFile{Path: absolutePath, ValidationErrors: []error{&InvalidYamlError{ErrorMessage: err.Error()}}}
 	}
@@ -76,7 +76,7 @@ type FileConfigurations struct {
 	Configurations []Configuration `json:"configurations"`
 }
 
-func ParseYaml(content string) (*[]Configuration, error) {
+func parseYaml(content string) (*[]Configuration, error) {
 	configurations, err := extractYamlConfigurations(content)
 	if err != nil {
 		return nil, err
@@ -131,22 +131,22 @@ func extractConfigurationK8sData(content []byte, yamlNode yaml.Node) Configurati
 		return configuration
 	}
 
-	if metadata := jsonObject["metadata"]; metadata != nil {
-		if metadataName := metadata.(map[string]interface{})["name"]; metadataName != nil {
-			configuration.MetadataName = metadataName.(string)
+	if metadata, ok := jsonObject["metadata"].(map[string]interface{}); ok {
+		if metadataName, ok := metadata["name"].(string); ok {
+			configuration.MetadataName = metadataName
 		}
 
-		if annotations := metadata.(map[string]interface{})["annotations"]; annotations != nil {
-			configuration.Annotations = annotations.(map[string]interface{})
+		if annotations, ok := metadata["annotations"].(map[string]interface{}); ok {
+			configuration.Annotations = annotations
 		}
 	}
 
-	if apiVersion := jsonObject["apiVersion"]; apiVersion != nil {
-		configuration.ApiVersion = apiVersion.(string)
+	if apiVersion, ok := jsonObject["apiVersion"].(string); ok {
+		configuration.ApiVersion = apiVersion
 	}
 
-	if kind := jsonObject["kind"]; kind != nil {
-		configuration.Kind = kind.(string)
+	if kind, ok := jsonObject["kind"].(string); ok {
+		configuration.Kind = kind
 	}
 	configuration.YamlNode = yamlNode
 	return configuration


Test patch:
diff --git a/pkg/extractor/extractorTestFiles/annotationsAreArray.yaml b/pkg/extractor/extractorTestFiles/annotationsAreArray.yaml
new file mode 100644
index 0000000..c1c74d6
--- /dev/null
+++ b/pkg/extractor/extractorTestFiles/annotationsAreArray.yaml
@@ -0,0 +1,6 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: rss-site
+  annotations:
+    - aaa: bbb
diff --git a/pkg/extractor/extractorTestFiles/invalidYaml.yaml b/pkg/extractor/extractorTestFiles/invalidYaml.yaml
new file mode 100644
index 0000000..28f9de5
--- /dev/null
+++ b/pkg/extractor/extractorTestFiles/invalidYaml.yaml
@@ -0,0 +1,3 @@
+apiVersion: v1
+policies:
+  - name: "Default"a
diff --git a/pkg/extractor/extractorTestFiles/noMetadata.yaml b/pkg/extractor/extractorTestFiles/noMetadata.yaml
new file mode 100644
index 0000000..49d9df0
--- /dev/null
+++ b/pkg/extractor/extractorTestFiles/noMetadata.yaml
@@ -0,0 +1,2 @@
+apiVersion: apps/v1
+kind: Deployment
diff --git a/pkg/extractor/extractorTestFiles/skipAnnotations.yaml b/pkg/extractor/extractorTestFiles/skipAnnotations.yaml
new file mode 100644
index 0000000..07d58e8
--- /dev/null
+++ b/pkg/extractor/extractorTestFiles/skipAnnotations.yaml
@@ -0,0 +1,7 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: rss-site
+  annotations:
+    datree.skip/WORKLOAD_INVALID_LABELS_VALUE: skip 1 rule
+    datree.skip/CONTAINERS_MISSING_LIVENESSPROBE_KEY: skip 2 rule
diff --git a/pkg/extractor/extractorTestFiles/validYaml.yaml b/pkg/extractor/extractorTestFiles/validYaml.yaml
new file mode 100644
index 0000000..80b6891
--- /dev/null
+++ b/pkg/extractor/extractorTestFiles/validYaml.yaml
@@ -0,0 +1,1 @@
+testKey: testValue
diff --git a/pkg/extractor/extractor_test.go b/pkg/extractor/extractor_test.go
index 1bb8c7c..3bb9dc1 100644
--- a/pkg/extractor/extractor_test.go
+++ b/pkg/extractor/extractor_test.go
@@ -18,11 +18,11 @@ type toAbsolutePathsTestCase struct {
 }
 
 func test_existed_file() *toAbsolutePathsTestCase {
-	p, _ := filepath.Abs("../../internal/fixtures/kube/pass-all.yaml")
+	p, _ := filepath.Abs("./extractorTestFiles/validYaml.yaml")
 	return &toAbsolutePathsTestCase{
 		name: "existed file, should return abs path with no errors",
 		args: struct{ path string }{
-			path: "../../internal/fixtures/kube/pass-all.yaml",
+			path: "./extractorTestFiles/validYaml.yaml",
 		},
 		expected: struct {
 			path string
@@ -36,7 +36,7 @@ func test_not_existed_file() *toAbsolutePathsTestCase {
 	return &toAbsolutePathsTestCase{
 		name: "test not existed file, should return an error",
 		args: struct{ path string }{
-			path: "../../internal/fixtures/kube/bla.yaml",
+			path: "./extractorTestFiles/fileThatDoesNotExist.yaml",
 		},
 		expected: struct {
 			path string
@@ -49,7 +49,7 @@ func test_not_existed_file() *toAbsolutePathsTestCase {
 func test_directory_file() *toAbsolutePathsTestCase {
 	return &toAbsolutePathsTestCase{
 		args: struct{ path string }{
-			path: "../../internal/fixtures/kube",
+			path: "./extractorTestFiles",
 		},
 		expected: struct {
 			path string
@@ -83,7 +83,7 @@ func TestExtractConfigurationsFromYamlFile(t *testing.T) {
 	})
 
 	t.Run("invalid yaml file, should return an error", func(t *testing.T) {
-		path := "../../internal/fixtures/policyAsCode/invalid-yaml.yaml"
+		path := "./extractorTestFiles/invalidYaml.yaml"
 		configurations, absolutePath, err := ExtractConfigurationsFromYamlFile(path)
 
 		assert.Empty(t, configurations)
@@ -92,11 +92,42 @@ func TestExtractConfigurationsFromYamlFile(t *testing.T) {
 	})
 
 	t.Run("valid yaml file, should return a configuration and no error", func(t *testing.T) {
-		path := "../../internal/fixtures/jsonSchema/yamlSchema.yaml"
+		path := "./extractorTestFiles/validYaml.yaml"
 		configurations, absolutePath, err := ExtractConfigurationsFromYamlFile(path)
 
 		assert.NotEmpty(t, configurations)
 		assert.NotEmpty(t, absolutePath)
 		assert.Nil(t, err)
 	})
+	t.Run("should not panic even though annotations is an array instead of object", func(t *testing.T) {
+		path := "./extractorTestFiles/annotationsAreArray.yaml"
+		configurations, _, _ := ExtractConfigurationsFromYamlFile(path)
+
+		firstConfiguration := (*configurations)[0]
+		assert.Nil(t, firstConfiguration.Annotations)
+		assert.Equal(t, "rss-site", firstConfiguration.MetadataName)
+		assert.Equal(t, "Deployment", firstConfiguration.Kind)
+		assert.Equal(t, "apps/v1", firstConfiguration.ApiVersion)
+	})
+	t.Run("should not panic even though metadata does not exist", func(t *testing.T) {
+		path := "./extractorTestFiles/noMetadata.yaml"
+		configurations, _, _ := ExtractConfigurationsFromYamlFile(path)
+
+		firstConfiguration := (*configurations)[0]
+		assert.Nil(t, firstConfiguration.Annotations)
+		assert.Equal(t, "", firstConfiguration.MetadataName)
+		assert.Equal(t, "Deployment", firstConfiguration.Kind)
+		assert.Equal(t, "apps/v1", firstConfiguration.ApiVersion)
+	})
+	t.Run("should extract skip annotations", func(t *testing.T) {
+		path := "./extractorTestFiles/skipAnnotations.yaml"
+		configurations, _, _ := ExtractConfigurationsFromYamlFile(path)
+
+		firstConfiguration := (*configurations)[0]
+		assert.Equal(t, "skip 1 rule", firstConfiguration.Annotations["datree.skip/WORKLOAD_INVALID_LABELS_VALUE"])
+		assert.Equal(t, "skip 2 rule", firstConfiguration.Annotations["datree.skip/CONTAINERS_MISSING_LIVENESSPROBE_KEY"])
+		assert.Equal(t, "rss-site", firstConfiguration.MetadataName)
+		assert.Equal(t, "Deployment", firstConfiguration.Kind)
+		assert.Equal(t, "apps/v1", firstConfiguration.ApiVersion)
+	})
 }