code review

examples/validators/gatekeeper-validate/invalid-configmap/.expected/results.yaml

@@ -1,6 +1,6 @@
 items:
 - message: |-
-    The following banned keys are being used in the config map: {"private_key"}
+    The following banned keys are being used in the ConfigMap: {"private_key"}
     violatedConstraint: no-secrets-in-configmap
   severity: error
   resourceRef:

examples/validators/gatekeeper-validate/invalid-configmap/README.md

@@ -2,7 +2,7 @@
 
 ## Overview
 
-This example demonstrates how to validate config maps using a constraint.
+This example demonstrates how to validate ConfigMaps using a constraint.
 
 There are 3 resources: a ConstraintTemplate, a K8sBannedConfigMapKeysV1 and a
 ConfigMap.
@@ -34,7 +34,7 @@ $ kpt fn run --results-dir=results .
 You should see the following output:
 
 ```
-The following banned keys are being used in the config map: {"private_key"}
+The following banned keys are being used in the ConfigMap: {"private_key"}
 violatedConstraint: no-secrets-in-configmaperror: exit status 1
 ```
 
@@ -44,7 +44,7 @@ Let's take a look at the structured output:
 $ cat results/results-0.yaml 
 items:
 - message: |-
-    The following banned keys are being used in the config map: {"private_key"}
+    The following banned keys are being used in the ConfigMap: {"private_key"}
     violatedConstraint: no-secrets-in-configmap
   severity: error
   resourceRef:
@@ -68,6 +68,7 @@ To pass validation, let's replace the key `private_key` in the ConfigMap in
 `resources.yaml` with something else e.g. `public_key`.
 Rerun the command. It will succeed (no output).
 
-## Function Reference
+## Function Reference Doc
 
-TODO: add the link
+TODO: replace the following with the link to the reference doc when our site is live.
+https://github.com/GoogleContainerTools/kpt-functions-catalog/blob/master/functions/go/gatekeeper-validate/README.md

examples/validators/gatekeeper-validate/invalid-configmap/resources.yaml

@@ -24,15 +24,14 @@ spec:
           banned = {key | input.parameters.keys[_] = key}
           overlap = keys & banned
           count(overlap) > 0
-          val := sprintf("The following banned keys are being used in the config map: %v", [overlap])
+          val := sprintf("The following banned keys are being used in the ConfigMap: %v", [overlap])
         }
 ---
 apiVersion: constraints.gatekeeper.sh/v1beta1
 kind: K8sBannedConfigMapKeysV1
 metadata:
   name: no-secrets-in-configmap
 spec:
-  enforcementAction: deny
   match:
     kinds:
       - apiGroups:

examples/validators/gatekeeper-validate/warning-only/.expected/results.yaml

@@ -1,6 +1,6 @@
 items:
 - message: |-
-    The following banned keys are being used in the config map: {"private_key"}
+    The following banned keys are being used in the ConfigMap: {"private_key"}
     violatedConstraint: no-secrets-in-configmap
   severity: warning
   resourceRef:

examples/validators/gatekeeper-validate/warning-only/README.md

@@ -38,7 +38,7 @@ warning about the constraint violation.
 $ cat results/results-0.yaml 
 items:
 - message: |-
-    The following banned keys are being used in the config map: {"private_key"}
+    The following banned keys are being used in the ConfigMap map: {"private_key"}
     violatedConstraint: no-secrets-in-configmap
   severity: warning
   resourceRef:
@@ -62,6 +62,7 @@ To pass validation, let's replace the key `private_key` in the ConfigMap in
 `resources.yaml` with something else e.g. `public_key`.
 Rerun the command. It will no longer have the warning.
 
-## Function Reference
+## Function Reference Doc
 
-TODO: add the link
+TODO: replace the following with the link to the reference doc when our site is live.
+https://github.com/GoogleContainerTools/kpt-functions-catalog/blob/master/functions/go/gatekeeper-validate/README.md

examples/validators/gatekeeper-validate/warning-only/resources.yaml

@@ -23,7 +23,7 @@ spec:
         banned = {key | input.parameters.keys[_] = key}
         overlap = keys & banned
         count(overlap) > 0
-        val := sprintf("The following banned keys are being used in the config map: %v", [overlap])
+        val := sprintf("The following banned keys are being used in the ConfigMap: %v", [overlap])
       }
     target: admission.k8s.gatekeeper.sh
 ---

functions/go/gatekeeper-validate/README.md

@@ -4,16 +4,16 @@
 
 <!--mdtogo:Short-->
 
-Validate the KRM resources using the policy controller.
+Validate the KRM resources using [Gatekeeper] constraints.
 
 <!--mdtogo-->
 
 ### Synopsis
 
 <!--mdtogo:Long-->
 
-You can use the policy controller to validate KRM resources. To learn more about
-the policy controller, see: https://cloud.google.com/anthos-config-management/docs/concepts/policy-controller.
+You can use Gatekeeper to validate KRM resources. To learn more about how to use
+the Gatekeeper project, see: https://open-policy-agent.github.io/gatekeeper/website/docs/howto.
 
 The function evaluates constraint policies against KRM resources.
 The function takes 3 types of resources from the input resource list:
@@ -41,3 +41,5 @@ https://cloud.google.com/anthos-config-management/docs/how-to/creating-constrain
 https://github.com/GoogleContainerTools/kpt-functions-catalog/tree/master/examples/validators/gatekeeper-validate/
 
 <!--mdtogo-->
+
+[Gatekeeper]:https://github.com/open-policy-agent/gatekeeper

functions/go/gatekeeper-validate/main.go

@@ -54,29 +54,28 @@ func main() {
 			objects = append(objects, obj)
 		}
 
-		result, err := Validate(objects)
-		switch {
-		case result != nil && err != nil:
-			resourceList.Result = result
-			return err
-		case result != nil && err == nil:
-			resourceList.Result = result
+		err := Validate(objects)
+		if err == nil {
 			return nil
-		case result == nil && err != nil:
-			resourceList.Result = &framework.Result{
-				Name: "gatekeeper-validate",
-				Items: []framework.Item{
-					{
-						Message:  err.Error(),
-						Severity: framework.Error,
-					},
-				},
+		}
+
+		if result, ok := err.(*framework.Result); ok {
+			resourceList.Result = result
+			if resultContainsError(result) {
+				return result
 			}
-			resourceList.FunctionConfig = nil
-			return resourceList.Result
-		default:
 			return nil
 		}
+
+		resourceList.Result = &framework.Result{
+			Items: []framework.Item{
+				{
+					Message:  err.Error(),
+					Severity: framework.Error,
+				},
+			},
+		}
+		return resourceList.Result
 	})
 	cmd.Short = generated.PolicyControllerValidateShort
 	cmd.Long = generated.PolicyControllerValidateLong
@@ -85,3 +84,12 @@ func main() {
 		os.Exit(1)
 	}
 }
+
+func resultContainsError(result *framework.Result) bool {
+	for _, item := range result.Items {
+		if item.Severity == framework.Error {
+			return true
+		}
+	}
+	return false
+}

functions/go/gatekeeper-validate/validate.go

@@ -80,49 +80,49 @@ func gatherConstraints(objects []runtime.Object) ([]*unstructured.Unstructured,
 
 // Validate makes sure the configs passed to it comply with any Constraints and
 // Constraint Templates present in the list of configs
-func Validate(objects []runtime.Object) (*framework.Result, error) {
+func Validate(objects []runtime.Object) error {
 	client, err := createClient()
 	if err != nil {
-		return nil, err
+		return err
 	}
 	tmpls, err := gatherTemplates(objects)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	ctx := context.Background()
 	for _, t := range tmpls {
 		if _, err = client.AddTemplate(ctx, t); err != nil {
-			return nil, err
+			return err
 		}
 	}
 	cstrs, err := gatherConstraints(objects)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	for _, c := range cstrs {
 		if _, err = client.AddConstraint(ctx, c); err != nil {
-			return nil, err
+			return err
 		}
 	}
 
 	for _, obj := range objects {
 		if _, err = client.AddData(ctx, obj); err != nil {
-			return nil, err
+			return err
 		}
 	}
 
 	resps, err := client.Audit(ctx)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	results := resps.Results()
 	if len(results) > 0 {
 		return parseResults(results)
 	}
-	return nil, nil
+	return nil
 }
 
-func parseResults(results []*opatypes.Result) (*framework.Result, error) {
+func parseResults(results []*opatypes.Result) error {
 	out := &framework.Result{
 		Items: []framework.Item{},
 	}
@@ -131,7 +131,7 @@ func parseResults(results []*opatypes.Result) (*framework.Result, error) {
 	for _, r := range results {
 		u, ok := r.Resource.(*unstructured.Unstructured)
 		if !ok {
-			return nil, fmt.Errorf("could not cast to unstructured: %+v", r.Resource)
+			return fmt.Errorf("could not cast to unstructured: %+v", r.Resource)
 		}
 
 		item := framework.Item{
@@ -171,7 +171,7 @@ func parseResults(results []*opatypes.Result) (*framework.Result, error) {
 			if foundIndex {
 				idx, err := strconv.Atoi(index)
 				if err != nil {
-					return nil, err
+					return err
 				}
 				item.File.Index = idx
 			}
@@ -181,8 +181,8 @@ func parseResults(results []*opatypes.Result) (*framework.Result, error) {
 	}
 
 	if foundError {
-		return out, out
+		return out
 	} else {
-		return out, nil
+		return nil
 	}
 }

tests/gatekeeper-validate/dryrun/.expected/results.yaml

@@ -1,6 +1,6 @@
 items:
 - message: |-
-    The following banned keys are being used in the config map: {"private_key"}
+    The following banned keys are being used in the ConfigMap: {"private_key"}
     violatedConstraint: no-secrets-in-configmap
   severity: info
   resourceRef:

tests/gatekeeper-validate/dryrun/resources.yaml

@@ -23,7 +23,7 @@ spec:
         banned = {key | input.parameters.keys[_] = key}
         overlap = keys & banned
         count(overlap) > 0
-        val := sprintf("The following banned keys are being used in the config map: %v", [overlap])
+        val := sprintf("The following banned keys are being used in the ConfigMap: %v", [overlap])
       }
     target: admission.k8s.gatekeeper.sh
 ---

tests/gatekeeper-validate/valid/resources.yaml

@@ -23,7 +23,7 @@ spec:
         banned = {key | input.parameters.keys[_] = key}
         overlap = keys & banned
         count(overlap) > 0
-        val := sprintf("The following banned keys are being used in the config map: %v", [overlap])
+        val := sprintf("The following banned keys are being used in the ConfigMap: %v", [overlap])
       }
     target: admission.k8s.gatekeeper.sh
 ---