Merge pull request #466 from patilpankaj212/skip-comment-and-scan-sum…

…mary-additions

support comment with rule skipping for resource and scan summary modifications

pkg/cli/testdata/run-test/test_pod.yaml

@@ -7,7 +7,9 @@ metadata:
     test: someupdate
     test2: someupdate3
   annotations:
-    terrascanSkip: [accurics.kubernetes.IAM.109]
+    terrascanSkip:
+      - rule: accurics.kubernetes.IAM.109
+        comment: reason to skip the rule
 spec:
   containers:
   - name: myapp-container
@@ -25,7 +27,11 @@ metadata:
     test: someupdate
     test2: someupdate3
   annotations:
-    terrascanSkip: [accurics.kubernetes.IAM.3, accurics.kubernetes.OPS.461]
+    terrascanSkip:
+      - rule: accurics.kubernetes.IAM.3
+        comment: reason to skip the rule
+      - rule: accurics.kubernetes.OPS.461
+        comment: reason to skip the rule
 spec:
   template:
     spec:

pkg/iac-providers/kubernetes/v1/normalize.go

@@ -28,7 +28,11 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-const terrascanSkip = "terrascanSkip"
+const (
+	terrascanSkip        = "terrascanSkip"
+	terrascanSkipRule    = "rule"
+	terrascanSkipComment = "comment"
+)
 
 var (
 	errUnsupportedDoc = fmt.Errorf("unsupported document type")
@@ -146,7 +150,7 @@ func (k *K8sV1) Normalize(doc *utils.IacDocument) (*output.ResourceConfig, error
 	return &resourceConfig, nil
 }
 
-func readSkipRulesFromAnnotations(annotations map[string]interface{}, resourceID string) []string {
+func readSkipRulesFromAnnotations(annotations map[string]interface{}, resourceID string) []output.SkipRule {
 
 	var skipRulesFromAnnotations interface{}
 	var ok bool
@@ -155,18 +159,47 @@ func readSkipRulesFromAnnotations(annotations map[string]interface{}, resourceID
 		return nil
 	}
 
-	skipRules := make([]string, 0)
+	skipRules := make([]output.SkipRule, 0)
 	if rules, ok := skipRulesFromAnnotations.([]interface{}); ok {
 		for _, rule := range rules {
-			if value, ok := rule.(string); ok {
-				skipRules = append(skipRules, value)
+			if value, ok := rule.(map[string]interface{}); ok {
+				skipRule := getSkipRuleObject(value)
+				if skipRule != nil {
+					skipRules = append(skipRules, *skipRule)
+				}
 			} else {
-				zap.S().Debugf("each rule in %s must be of string type", terrascanSkip)
+				zap.S().Debugf("each rule in %s must be of map type", terrascanSkip)
 			}
 		}
 	} else {
-		zap.S().Debugf("%s must be an array of rules to skip", terrascanSkip)
+		zap.S().Debugf("%s must be an array of {rule: ruleID, comment: reason for skipping}", terrascanSkip)
 	}
 
 	return skipRules
 }
+
+func getSkipRuleObject(m map[string]interface{}) *output.SkipRule {
+	var skipRule output.SkipRule
+	var rule, comment interface{}
+	var ok bool
+
+	// get rule, if rule not found return nil
+	if rule, ok = m[terrascanSkipRule]; ok {
+		if _, ok = rule.(string); ok {
+			skipRule.Rule = rule.(string)
+		} else {
+			return nil
+		}
+	} else {
+		return nil
+	}
+
+	// get comment
+	if comment, ok = m[terrascanSkipComment]; ok {
+		if _, ok = comment.(string); ok {
+			skipRule.Comment = comment.(string)
+		}
+	}
+
+	return &skipRule
+}

pkg/iac-providers/kubernetes/v1/normalize_test.go

@@ -55,7 +55,9 @@ kind: Pod
 metadata:
   name: myapp-pod
   annotations:
-    terrascanSkip: [accurics.kubernetes.IAM.109]
+    terrascanSkip:
+      - rule: accurics.kubernetes.IAM.109
+        comment: reason to skip the rule
 spec:
   containers:
     - name: myapp-container
@@ -66,7 +68,9 @@ kind: CRD
 metadata:
   generateName: myapp-pod-prefix-
   annotations:
-    terrascanSkip: [accurics.kubernetes.IAM.109]
+    terrascanSkip:
+      - rule: accurics.kubernetes.IAM.109
+        comment: reason to skip the rule
 spec:
   containers:
     - name: myapp-container
@@ -121,7 +125,10 @@ func TestK8sV1ExtractResource(t *testing.T) {
 				Metadata: k8sMetadata{
 					Name: "myapp-pod",
 					Annotations: map[string]interface{}{
-						terrascanSkip: []interface{}{"accurics.kubernetes.IAM.109"},
+						terrascanSkip: []interface{}{map[string]interface{}{
+							"rule":    "accurics.kubernetes.IAM.109",
+							"comment": "reason to skip the rule",
+						}},
 					},
 				},
 			},
@@ -179,6 +186,12 @@ func TestK8sV1GetNormalizedName(t *testing.T) {
 
 func TestK8sV1Normalize(t *testing.T) {
 	testRule := "accurics.kubernetes.IAM.109"
+	testComment := "reason to skip the rule"
+
+	testSkipRule := output.SkipRule{
+		Rule:    testRule,
+		Comment: testComment,
+	}
 
 	type args struct {
 		doc *utils.IacDocument
@@ -215,7 +228,10 @@ func TestK8sV1Normalize(t *testing.T) {
 					"kind":       "Pod",
 					"metadata": map[string]interface{}{
 						"annotations": map[string]interface{}{
-							terrascanSkip: []interface{}{testRule},
+							terrascanSkip: []interface{}{map[string]interface{}{
+								"rule":    testRule,
+								"comment": testComment,
+							}},
 						},
 						"name": "myapp-pod",
 					},
@@ -228,7 +244,7 @@ func TestK8sV1Normalize(t *testing.T) {
 						},
 					},
 				},
-				SkipRules: []string{testRule},
+				SkipRules: []output.SkipRule{testSkipRule},
 			},
 		},
 		{
@@ -249,7 +265,10 @@ func TestK8sV1Normalize(t *testing.T) {
 					"kind":       "CRD",
 					"metadata": map[string]interface{}{
 						"annotations": map[string]interface{}{
-							terrascanSkip: []interface{}{testRule},
+							terrascanSkip: []interface{}{map[string]interface{}{
+								"rule":    testRule,
+								"comment": testComment,
+							}},
 						},
 						"generateName": "myapp-pod-prefix-",
 					},
@@ -262,7 +281,7 @@ func TestK8sV1Normalize(t *testing.T) {
 						},
 					},
 				},
-				SkipRules: []string{testRule},
+				SkipRules: []output.SkipRule{testSkipRule},
 			},
 		},
 	}
@@ -284,8 +303,13 @@ func TestK8sV1Normalize(t *testing.T) {
 func TestReadSkipRulesFromAnnotations(t *testing.T) {
 	// test data
 	testRuleA := "RuleA"
+	testCommentA := "RuleA can be skipped"
 	testRuleB := "RuleB"
+	testCommentB := "RuleB must be skipped"
 	testRuleC := "RuleC"
+	testCommentC := "RuleC skipped"
+
+	testSkipRule := output.SkipRule{Rule: testRuleA}
 
 	type args struct {
 		annotations map[string]interface{}
@@ -294,7 +318,7 @@ func TestReadSkipRulesFromAnnotations(t *testing.T) {
 	tests := []struct {
 		name string
 		args args
-		want []string
+		want []output.SkipRule
 	}{
 		{
 			name: "nil annotations",
@@ -317,34 +341,101 @@ func TestReadSkipRulesFromAnnotations(t *testing.T) {
 					terrascanSkip: "test",
 				},
 			},
-			want: []string{},
+			want: []output.SkipRule{},
 		},
 		{
-			name: "annotations with invalid terrascanSkipRules rule value",
+			name: "annotations with invalid SkipRule object",
 			args: args{
 				annotations: map[string]interface{}{
 					terrascanSkip: []interface{}{1},
 				},
 			},
-			want: []string{},
+			want: []output.SkipRule{},
+		},
+		{
+			name: "annotations with invalid terrascanSkipRules rule value",
+			args: args{
+				annotations: map[string]interface{}{
+					terrascanSkip: []interface{}{map[string]interface{}{
+						terrascanSkipRule: 1,
+					}},
+				},
+			},
+			want: []output.SkipRule{},
 		},
 		{
 			name: "annotations with one terrascanSkipRules",
 			args: args{
 				annotations: map[string]interface{}{
-					terrascanSkip: []interface{}{testRuleA},
+					terrascanSkip: []interface{}{map[string]interface{}{
+						terrascanSkipRule: testRuleA,
+					}},
+				},
+			},
+			want: []output.SkipRule{
+				{
+					Rule: testRuleA,
 				},
 			},
-			want: []string{testRuleA},
 		},
 		{
 			name: "annotations with multiple terrascanSkipRules",
 			args: args{
 				annotations: map[string]interface{}{
-					terrascanSkip: []interface{}{testRuleA, testRuleB, testRuleC},
+					terrascanSkip: []interface{}{
+						map[string]interface{}{
+							terrascanSkipRule:    testRuleA,
+							terrascanSkipComment: testCommentA,
+						},
+						map[string]interface{}{
+							terrascanSkipRule:    testRuleB,
+							terrascanSkipComment: testCommentB,
+						},
+						map[string]interface{}{
+							terrascanSkipRule:    testRuleC,
+							terrascanSkipComment: testCommentC,
+						}},
+				},
+			},
+			want: []output.SkipRule{
+				{
+					Rule:    testRuleA,
+					Comment: testCommentA,
+				},
+				{
+					Rule:    testRuleB,
+					Comment: testCommentB,
+				},
+				{
+					Rule:    testRuleC,
+					Comment: testCommentC,
+				},
+			},
+		},
+		{
+			name: "annotations with invalid rule key in terrascanSkipRules",
+			args: args{
+				annotations: map[string]interface{}{
+					terrascanSkip: []interface{}{
+						map[string]interface{}{
+							"skip-rule":          testRuleA,
+							terrascanSkipComment: testCommentA,
+						}},
+				},
+			},
+			want: []output.SkipRule{},
+		},
+		{
+			name: "annotations with no comment key in terrascanSkipRules",
+			args: args{
+				annotations: map[string]interface{}{
+					terrascanSkip: []interface{}{
+						map[string]interface{}{
+							terrascanSkipRule: testRuleA,
+						}},
 				},
 			},
-			want: []string{testRuleA, testRuleB, testRuleC},
+			want: []output.SkipRule{testSkipRule},
 		},
 	}
 	for _, tt := range tests {

pkg/iac-providers/kubernetes/v1/testdata/file-test-data/test_pod_skip_rules.yaml

@@ -7,7 +7,9 @@ metadata:
     test: someupdate
     test2: someupdate3
   annotations:
-    terrascanSkip: [accurics.kubernetes.IAM.109]
+    terrascanSkip:
+      - rule: accurics.kubernetes.IAM.109
+        comment: reason to skip the rule
 spec:
   containers:
   - name: myapp-container
@@ -25,7 +27,11 @@ metadata:
     test: someupdate
     test2: someupdate3
   annotations:
-    terrascanSkip: [accurics.kubernetes.IAM.3, accurics.kubernetes.OPS.461]
+    terrascanSkip:
+      - rule: accurics.kubernetes.IAM.3
+        comment: reason to skip the rule
+      - rule: accurics.kubernetes.OPS.461
+        comment: reason to skip the rule
 spec:
   template:
     spec:

pkg/iac-providers/output/types.go

@@ -27,7 +27,13 @@ type ResourceConfig struct {
 	// SkipRules will hold the rules to be skipped for the resource.
 	// Each iac provider should append the rules to be skipped for a resource,
 	// while extracting resource from the iac files
-	SkipRules []string `json:"skip_rules"`
+	SkipRules []SkipRule `json:"skip_rules" yaml:"skip_rules"`
+}
+
+// SkipRule struct will hold the skipped rule and any comment for the skipped rule
+type SkipRule struct {
+	Rule    string `json:"rule"`
+	Comment string `json:"comment"`
 }
 
 // AllResourceConfigs is a list/slice of resource configs present in IaC