Repository: plexsystems/konstraint
Pull request #14 (2020-07-06 22:18:45)

Only generate constraints for policies with violation[] rules
Gatekeeper only supports policies with `violation[msg]` rules and rejects any `ConstraintTemplate`s that do not have any. As such, we should skip generating the constraint resources if a policy does not have any. 

This is a WIP, it needs some more work and additional documentation before merging.

jalseth: @jpreese I think it's in a much better state now. There's less unnecessary code, and the only real inefficiency is that we call `ast.Module.Parse()` twice when loading the policy files. However, given that the contents have already been loaded from disk into memory, this is a fast operation so the overhead is negligible. Anecdotally, my MacBook takes about 1ms per processed policy. 

Patch:
diff --git a/README.md b/README.md
index b190f87..4c74e90 100644
--- a/README.md
+++ b/README.md
@@ -92,47 +92,8 @@ This will generate a single `policies.md` file that contains a description of al
 
 _Example: konstraint doc --output examples/policies.md_
 
-## How template and constraint naming works
+## FAQ
 
-The name of the ConstraintTemplate is derived from the name of the folder that the policy was found in.
+**Konstraint ran without error, but I don't see any new files.**
 
-For example, a policy found in: `policies/pod-volume-size-limits/src.rego` generates the following in the `policies/pod-volume-size-limits` directory:
-
-- `template.yaml` (defining a ConstraintTemplate)
-  - kind: _ConstraintTemplate_
-  - name: _podvolumesizelimits_
-  - kind (to add to Kubernetes API): _PodVolumeSizeLimits_
-
-- `constraint.yaml` (implementing the above ConstraintTemplate)
-  - kind: _PodVolumeSizeLimits_
-  - name: _podvolumesizelimits_
-
-_While not technically required, the tool works best with a folder structure similar to how Gatekeeper itself [structures policies and templates](https://github.com/open-policy-agent/gatekeeper/tree/master/library)._
-
-## How library importing works
-
-Libraries can be stored in a folder named `lib` or the name of the folder set with the `--lib` flag.
-
-Libraries will be added to the `ConstraintTemplate` if and only if the associated policy imports the library. This helps prevent importing Rego code that will go unused.
-
-## Experimental
-
-To further promote that the `.rego` file is the source of truth for policy, a form of block comments on each `violation` rule can be added that includes a human readable description of what the policy does. This comment block is also used to set the matchers on the `Constraint` itself.
-
-```rego
-# All images deployed to the cluster must not contain a latest tag.
-# @Kinds apps/DaemonSet apps/Deployment apps/StatefulSet core/Pod
-violation[msg] {
-  has_latest_tag
-
-  msg := k8s.format(sprintf("(%s) %s: Images must not use the latest tag", [k8s.kind, k8s.name]))
-}
-```
-
-The [examples/policies.md](examples/policies.md) file in this repository was generated by running:
-
-```shell
-$ konstraint doc --output examples/policies.md
-```
-
-At the root of the repository.
+This typically means no policies were found, or the policies did not have any `violation[]` rules so they are not compatible with Gatekeeper. For more information, see [How Constraints are Created](docs/constraint_creation.md).
diff --git a/docs/constraint_creation.md b/docs/constraint_creation.md
new file mode 100644
index 0000000..0601048
--- /dev/null
+++ b/docs/constraint_creation.md
@@ -0,0 +1,44 @@
+# How Constraints are Created
+
+## Policy File Parsing
+
+Konstraint only creates constraints for policies with `violation[]` rules in them, as that is required by Gatekeeper. If a policy does not have a violation rule in it, it is skipped.
+
+## Importing Libraries
+
+Libraries can be stored in a folder named `lib` or the name of the folder set with the `--lib` flag.
+
+Libraries will be added to the `ConstraintTemplate` if and only if the associated policy imports the library. This helps prevent importing Rego code that will go unused.
+
+## Resource Naming
+
+The name of the ConstraintTemplate and Constraint are derived from the name of the folder that the policy was found in.
+
+For example, a policy found in: `policies/pod-volume-size-limits/src.rego` generates the following in the `policies/pod-volume-size-limits` directory:
+
+- `template.yaml` (defining a ConstraintTemplate)
+  - kind: _ConstraintTemplate_
+  - name: _podvolumesizelimits_
+  - CRD kind (to add to Kubernetes API): _PodVolumeSizeLimits_
+
+- `constraint.yaml` (implementing the above ConstraintTemplate)
+  - kind: _PodVolumeSizeLimits_
+  - name: _podvolumesizelimits_
+
+While not technically required, the tool works best with a folder structure similar to how Gatekeeper itself [structures policies and templates](https://github.com/open-policy-agent/gatekeeper/tree/master/library).
+
+## Annotating Rules
+
+**Note: This feature is experimental.**
+
+To further promote that the `.rego` file is the source of truth for policy, a form of block comments on each `violation` rule can be added that includes a human readable description of what the policy does. This comment block is used to set the matchers on the `Constraint` itself and to generate the policy documentation with `konstraint doc`.
+
+```rego
+# All images deployed to the cluster must not contain a latest tag.
+# @Kinds apps/DaemonSet apps/Deployment apps/StatefulSet core/Pod
+violation[msg] {
+  has_latest_tag
+
+  msg := k8s.format(sprintf("(%s) %s: Images must not use the latest tag", [k8s.kind, k8s.name]))
+}
+```
diff --git a/examples/container-images/src.rego b/examples/container-images/src.rego
index e4e0653..2ecfc80 100644
--- a/examples/container-images/src.rego
+++ b/examples/container-images/src.rego
@@ -13,3 +13,7 @@ violation[msg] {
 has_latest_tag {
   endswith(k8s.container_images[_], ":latest")
 }
+
+has_latest_tag {
+  contains(k8s.container_images[_], ":") == false
+}
diff --git a/examples/container-images/template.yaml b/examples/container-images/template.yaml
index 8b4d4e1..56591ea 100644
--- a/examples/container-images/template.yaml
+++ b/examples/container-images/template.yaml
@@ -308,5 +308,9 @@ spec:
       has_latest_tag {
         endswith(k8s.container_images[_], ":latest")
       }
+
+      has_latest_tag {
+        contains(k8s.container_images[_], ":") == false
+      }
     target: admission.k8s.gatekeeper.sh
 status: {}
diff --git a/internal/commands/create.go b/internal/commands/create.go
index 985eb5c..d55d0bb 100644
--- a/internal/commands/create.go
+++ b/internal/commands/create.go
@@ -46,6 +46,15 @@ func newRegoFile(filePath string, contents string) (regoFile, error) {
 	return regoFile, nil
 }
 
+func moduleHasViolationRule(module *ast.Module) bool {
+	for _, rule := range module.Rules {
+		if strings.HasPrefix(rule.Head.String(), "violation[") {
+			return true
+		}
+	}
+	return false
+}
+
 func newCreateCommand() *cobra.Command {
 	cmd := cobra.Command{
 		Use:   "create <dir>",
@@ -93,12 +102,20 @@ func runCreateCommand(path string) error {
 		}
 	}
 
-	policies, err := loadRegoFiles(policyFilePaths)
+	policyContents, err := readFilesContents(policyFilePaths)
+	if err != nil {
+		return fmt.Errorf("read policy contents: %w", err)
+	}
+	policies, err := loadPolicyFiles(policyContents)
 	if err != nil {
 		return fmt.Errorf("load policies: %w", err)
 	}
 
-	libraries, err := loadRegoFiles(libraryFilePaths)
+	libraryContents, err := readFilesContents(libraryFilePaths)
+	if err != nil {
+		return fmt.Errorf("read library contents: %w", err)
+	}
+	libraries, err := loadLibraryFiles(libraryContents)
 	if err != nil {
 		return fmt.Errorf("load libraries: %w", err)
 	}
@@ -289,23 +306,55 @@ func getRegoFilePaths(path string) ([]string, error) {
 	return regoFilePaths, nil
 }
 
-func loadRegoFiles(filePaths []string) ([]regoFile, error) {
-	var regoFiles []regoFile
-	for _, filePath := range filePaths {
-		data, err := ioutil.ReadFile(filePath)
+func loadPolicyFiles(policyContents map[string]string) ([]regoFile, error) {
+	var policyFiles []regoFile
+	for path, contents := range policyContents {
+		module, err := ast.ParseModule(path, contents)
 		if err != nil {
-			return nil, fmt.Errorf("read policy file: %w", err)
+			return nil, fmt.Errorf("parse module: %w", err)
+		}
+
+		if !moduleHasViolationRule(module) {
+			continue
 		}
 
-		regoFile, err := newRegoFile(filePath, string(data))
+		policyFile, err := newRegoFile(path, contents)
 		if err != nil {
 			return nil, fmt.Errorf("new rego file: %w", err)
 		}
 
-		regoFiles = append(regoFiles, regoFile)
+		policyFiles = append(policyFiles, policyFile)
+	}
+
+	return policyFiles, nil
+}
+
+func loadLibraryFiles(libraryContents map[string]string) ([]regoFile, error) {
+	var libraryFiles []regoFile
+	for path, contents := range libraryContents {
+		libraryFile, err := newRegoFile(path, contents)
+		if err != nil {
+			return nil, fmt.Errorf("new rego file: %w", err)
+		}
+
+		libraryFiles = append(libraryFiles, libraryFile)
+	}
+
+	return libraryFiles, nil
+}
+
+func readFilesContents(filePaths []string) (map[string]string, error) {
+	filesContents := make(map[string]string)
+	for _, filePath := range filePaths {
+		data, err := ioutil.ReadFile(filePath)
+		if err != nil {
+			return nil, fmt.Errorf("read file: %w", err)
+		}
+
+		filesContents[filePath] = string(data)
 	}
 
-	return regoFiles, nil
+	return filesContents, nil
 }
 
 func getKindFromPath(path string) string {


Test patch:
diff --git a/examples/container-images/src_test.rego b/examples/container-images/src_test.rego
index 1a9b38d..f3e344b 100644
--- a/examples/container-images/src_test.rego
+++ b/examples/container-images/src_test.rego
@@ -21,3 +21,14 @@ test_input_as_image_with_latest_tag {
 
   has_latest_tag with input as input
 }
+
+test_input_as_image_with_no_tag {
+  input := {
+    "kind": "Pod",
+    "spec": {
+      "containers": [{"image": "image"}]
+    }
+  }
+
+  has_latest_tag with input as input
+}
diff --git a/internal/commands/create_test.go b/internal/commands/create_test.go
index 5e03d6e..c9954dd 100644
--- a/internal/commands/create_test.go
+++ b/internal/commands/create_test.go
@@ -104,6 +104,44 @@ import data.lib.foo`
 	}
 }
 
+func TestLoadPolicyFiles(t *testing.T) {
+	policyContents := make(map[string]string)
+	policyContents["missingViolation.rego"] = `package test
+default a = true`
+	policyContents["withViolation.rego"] = `package test
+violation[msg] {
+	msg = "test"
+}`
+
+	policies, err := loadPolicyFiles(policyContents)
+	if err != nil {
+		t.Fatal("load policy files:", err)
+	}
+
+	if len(policies) != 1 {
+		t.Error("incorrect number of policies loaded")
+	}
+}
+
+func TestLoadLibraryFiles(t *testing.T) {
+	libraryContents := make(map[string]string)
+	libraryContents["missingViolation.rego"] = `package test
+default a = true`
+	libraryContents["withViolation.rego"] = `package test
+violation[msg] {
+	msg = "test"
+}`
+
+	policies, err := loadLibraryFiles(libraryContents)
+	if err != nil {
+		t.Fatal("load library files:", err)
+	}
+
+	if len(policies) != 2 {
+		t.Error("incorrect number of libraries loaded")
+	}
+}
+
 func TestGetKindFromPath(t *testing.T) {
 	path := "/path/to/rego/container-resource-limits/something.rego"