Repository: cerbos/cerbos
Pull request #1526 (2023-04-12 11:07:24)

feat: Validate policy and test files with JSON schema

codecov[bot]: ## [Codecov](https://codecov.io/gh/cerbos/cerbos/pull/1526?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cerbos) Report
> Merging [#1526](https://codecov.io/gh/cerbos/cerbos/pull/1526?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cerbos) (b515328) into [main](https://codecov.io/gh/cerbos/cerbos/commit/df020fbe466f0d77ad00cc6980377fbdf2a43914?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cerbos) (df020fb) will **increase** coverage by `5.73%`.
> The diff coverage is `n/a`.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://codecov.io/gh/cerbos/cerbos/pull/1526/graphs/tree.svg?width=650&height=150&src=pr&token=tGaxiUZUzL&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cerbos)](https://codecov.io/gh/cerbos/cerbos/pull/1526?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cerbos)

```diff
@@            Coverage Diff             @@
##             main    #1526      +/-   ##
==========================================
+ Coverage   53.94%   59.67%   +5.73%     
==========================================
  Files         122        9     -113     
  Lines       13585     1679   -11906     
==========================================
- Hits         7328     1002    -6326     
+ Misses       5634      590    -5044     
+ Partials      623       87     -536     
```


[see 113 files with indirect coverage changes](https://codecov.io/gh/cerbos/cerbos/pull/1526/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cerbos)

</details>

Patch:
diff --git a/cmd/cerbos/compile/compile.go b/cmd/cerbos/compile/compile.go
index d270d188..8c52cd42 100644
--- a/cmd/cerbos/compile/compile.go
+++ b/cmd/cerbos/compile/compile.go
@@ -26,7 +26,7 @@ import (
 	"github.com/cerbos/cerbos/internal/engine"
 	"github.com/cerbos/cerbos/internal/outputcolor"
 	"github.com/cerbos/cerbos/internal/printer"
-	"github.com/cerbos/cerbos/internal/schema"
+	internalschema "github.com/cerbos/cerbos/internal/schema"
 	"github.com/cerbos/cerbos/internal/storage/disk"
 	"github.com/cerbos/cerbos/internal/storage/index"
 	"github.com/cerbos/cerbos/internal/util"
@@ -97,11 +97,11 @@ func (c *Cmd) Run(k *kong.Kong) error {
 
 	store := disk.NewFromIndexWithConf(idx, &disk.Conf{})
 
-	enforcement := schema.EnforcementReject
+	enforcement := internalschema.EnforcementReject
 	if c.IgnoreSchemas {
-		enforcement = schema.EnforcementNone
+		enforcement = internalschema.EnforcementNone
 	}
-	schemaMgr := schema.NewFromConf(ctx, store, schema.NewConf(enforcement))
+	schemaMgr := internalschema.NewFromConf(ctx, store, internalschema.NewConf(enforcement))
 
 	if err := compile.BatchCompile(idx.GetAllCompilationUnits(ctx), schemaMgr); err != nil {
 		compErr := new(compile.ErrorList)
@@ -141,6 +141,7 @@ func (c *Cmd) Run(k *kong.Kong) error {
 		if err != nil {
 			return err
 		}
+
 		results, err := verify.Verify(ctx, testFsys, eng, verifyConf)
 		if err != nil {
 			return fmt.Errorf("failed to run tests: %w", err)
diff --git a/go.mod b/go.mod
index 7796cc1d..279f0d06 100644
--- a/go.mod
+++ b/go.mod
@@ -94,7 +94,7 @@ require (
 	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1
 	google.golang.org/grpc v1.54.0
 	google.golang.org/protobuf v1.30.0
-	gopkg.in/yaml.v2 v2.4.0
+	gopkg.in/yaml.v3 v3.0.1
 	helm.sh/helm/v3 v3.11.2
 	modernc.org/sqlite v1.21.1
 )
@@ -268,7 +268,7 @@ require (
 	google.golang.org/appengine v1.6.7 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
+	gopkg.in/yaml.v2 v2.4.0 // indirect
 	lukechampine.com/uint128 v1.2.0 // indirect
 	modernc.org/cc/v3 v3.40.0 // indirect
 	modernc.org/ccgo/v3 v3.16.13 // indirect
diff --git a/internal/audit/conf.go b/internal/audit/conf.go
index 25fe8f80..819ba10f 100644
--- a/internal/audit/conf.go
+++ b/internal/audit/conf.go
@@ -6,8 +6,9 @@ package audit
 import (
 	"fmt"
 
+	"gopkg.in/yaml.v3"
+
 	"github.com/cerbos/cerbos/internal/config"
-	"gopkg.in/yaml.v2"
 )
 
 const (
diff --git a/internal/jsonschema/jsonschema.go b/internal/jsonschema/jsonschema.go
new file mode 100644
index 00000000..a387a491
--- /dev/null
+++ b/internal/jsonschema/jsonschema.go
@@ -0,0 +1,129 @@
+// Copyright 2021-2023 Zenauth Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+package jsonschema
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"io/fs"
+	"log"
+	"strings"
+
+	"github.com/santhosh-tekuri/jsonschema/v5"
+	"gopkg.in/yaml.v3"
+
+	"github.com/cerbos/cerbos/schema"
+)
+
+var (
+	policySchema *jsonschema.Schema
+	testSchema   *jsonschema.Schema
+)
+
+func init() {
+	var err error
+	if policySchema, err = jsonschema.CompileString("Policy.schema.json", schema.PolicyJSONSchema); err != nil {
+		log.Fatalf("failed to compile policy schema: %v", err)
+	}
+
+	if testSchema, err = jsonschema.CompileString("TestSuite.schema.json", schema.TestSuiteJSONSchema); err != nil {
+		log.Fatalf("failed to compile test schema: %v", err)
+	}
+}
+
+// ValidatePolicy validates the policy in the fsys with the JSON schema.
+func ValidatePolicy(fsys fs.FS, path string) error {
+	return validate(policySchema, fsys, path)
+}
+
+// ValidateTest validates the test in the fsys with the JSON schema.
+func ValidateTest(fsys fs.FS, path string) error {
+	return validate(testSchema, fsys, path)
+}
+
+func validate(s *jsonschema.Schema, fsys fs.FS, path string) error {
+	f, err := fsys.Open(path)
+	if err != nil {
+		return fmt.Errorf("failed to open file %s: %w", path, err)
+	}
+	defer f.Close()
+
+	data, err := io.ReadAll(f)
+	if err != nil {
+		return fmt.Errorf("failed to read file %s: %w", path, err)
+	}
+
+	var y any
+	if err := yaml.Unmarshal(data, &y); err != nil {
+		return fmt.Errorf("failed to unmarshal file %s: %w", path, err)
+	}
+
+	if err := s.Validate(y); err != nil {
+		var validationErr *jsonschema.ValidationError
+		if ok := errors.As(err, &validationErr); !ok {
+			return fmt.Errorf("unable to validate file %s: %w", path, err)
+		}
+
+		return newValidationErrorList(validationErr)
+	}
+
+	return nil
+}
+
+func newValidationError(err *jsonschema.ValidationError) validationError {
+	path := "/"
+	if err.InstanceLocation != "" {
+		path = err.InstanceLocation
+	}
+	return validationError{
+		Path:    path,
+		Message: err.Message,
+	}
+}
+
+func newValidationErrorList(validationErr *jsonschema.ValidationError) validationErrorList {
+	if validationErr == nil {
+		return nil
+	}
+
+	if len(validationErr.Causes) == 0 {
+		return validationErrorList{newValidationError(validationErr)}
+	}
+
+	var errs validationErrorList
+	for _, err := range validationErr.Causes {
+		errs = append(errs, newValidationErrorList(err)...)
+	}
+
+	return errs
+}
+
+type validationError struct {
+	Path    string
+	Message string
+}
+
+func (e validationError) Error() string {
+	return fmt.Sprintf("%s: %s", e.Path, e.Message)
+}
+
+type validationErrorList []validationError
+
+func (e validationErrorList) Error() string {
+	return fmt.Sprintf("file is not valid: [%s]", strings.Join(e.ErrorMessages(), ", "))
+}
+
+func (e validationErrorList) ErrorMessages() []string {
+	if len(e) == 0 {
+		return nil
+	}
+
+	msgs := make([]string, len(e))
+	for i, err := range e {
+		msgs[i] = err.Error()
+	}
+
+	return msgs
+}
diff --git a/internal/storage/conf.go b/internal/storage/conf.go
index 338deb8d..7217c29d 100644
--- a/internal/storage/conf.go
+++ b/internal/storage/conf.go
@@ -6,7 +6,7 @@ package storage
 import (
 	"fmt"
 
-	"gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v3"
 
 	"github.com/cerbos/cerbos/internal/config"
 )
diff --git a/internal/storage/index/builder.go b/internal/storage/index/builder.go
index 22fc9572..614f7852 100644
--- a/internal/storage/index/builder.go
+++ b/internal/storage/index/builder.go
@@ -15,6 +15,7 @@ import (
 
 	policyv1 "github.com/cerbos/cerbos/api/genpb/cerbos/policy/v1"
 	runtimev1 "github.com/cerbos/cerbos/api/genpb/cerbos/runtime/v1"
+	internaljsonschema "github.com/cerbos/cerbos/internal/jsonschema"
 	"github.com/cerbos/cerbos/internal/namer"
 	"github.com/cerbos/cerbos/internal/observability/metrics"
 	"github.com/cerbos/cerbos/internal/policy"
@@ -104,6 +105,11 @@ func build(ctx context.Context, fsys fs.FS, opts buildOptions) (Index, error) {
 			return nil
 		}
 
+		if err := internaljsonschema.ValidatePolicy(fsys, filePath); err != nil {
+			ib.addLoadFailure(filePath, err)
+			return nil
+		}
+
 		p := &policyv1.Policy{}
 		if err := util.LoadFromJSONOrYAML(fsys, filePath, p); err != nil {
 			ib.addLoadFailure(filePath, err)
diff --git a/internal/verify/verify.go b/internal/verify/verify.go
index 70125495..e75fe734 100644
--- a/internal/verify/verify.go
+++ b/internal/verify/verify.go
@@ -15,6 +15,7 @@ import (
 	enginev1 "github.com/cerbos/cerbos/api/genpb/cerbos/engine/v1"
 	policyv1 "github.com/cerbos/cerbos/api/genpb/cerbos/policy/v1"
 	"github.com/cerbos/cerbos/internal/engine"
+	internaljsonschema "github.com/cerbos/cerbos/internal/jsonschema"
 	"github.com/cerbos/cerbos/internal/util"
 )
 
@@ -97,6 +98,17 @@ func Verify(ctx context.Context, fsys fs.FS, eng Checker, conf Config) (*policyv
 	}
 
 	runTestSuite := func(file string) *policyv1.TestResults_Suite {
+		if err := internaljsonschema.ValidateTest(fsys, file); err != nil {
+			return &policyv1.TestResults_Suite{
+				File: file,
+				Name: "Unknown",
+				Summary: &policyv1.TestResults_Summary{
+					OverallResult: policyv1.TestResults_RESULT_ERRORED,
+				},
+				Error: err.Error(),
+			}
+		}
+
 		suite := &policyv1.TestSuite{}
 		err := util.LoadFromJSONOrYAML(fsys, file, suite)
 		if err == nil {
diff --git a/schema/schema.go b/schema/schema.go
index 0de33181..c9a9dc1d 100644
--- a/schema/schema.go
+++ b/schema/schema.go
@@ -21,6 +21,12 @@ var svcSwaggerRaw []byte
 //go:embed assets/ui.html
 var rapidocHTML []byte
 
+//go:embed jsonschema/cerbos/policy/v1/Policy.schema.json
+var PolicyJSONSchema string
+
+//go:embed jsonschema/cerbos/policy/v1/TestSuite.schema.json
+var TestSuiteJSONSchema string
+
 func ServeSvcSwagger(w http.ResponseWriter, r *http.Request) {
 	defer cleanup(r)
 


Test patch:
diff --git a/internal/test/testdata/index/corrupt_files.yaml b/internal/test/testdata/index/corrupt_files.yaml
index 3a55bf66..125de423 100644
--- a/internal/test/testdata/index/corrupt_files.yaml
+++ b/internal/test/testdata/index/corrupt_files.yaml
@@ -2,10 +2,10 @@
 wantErrList: 
   loadFailures: 
     - error: |-
-        failed to unmarshal JSON: proto: (line 1:2): unknown field "key"
+        file is not valid: [/: missing properties: 'apiVersion', /: additionalProperties 'key' not allowed, /: missing properties: 'resourcePolicy', /: missing properties: 'principalPolicy', /: missing properties: 'derivedRoles']
       file: principal.json
     - error: |-
-        failed to unmarshal JSON: proto: (line 1:2): unknown field "some"
+        file is not valid: [/: missing properties: 'apiVersion', /: additionalProperties 'some' not allowed, /: missing properties: 'resourcePolicy', /: missing properties: 'principalPolicy', /: missing properties: 'derivedRoles']
       file: resource.yaml
 files:
   "resource.yaml": |-
diff --git a/internal/test/testdata/index/incomplete_files.yaml b/internal/test/testdata/index/incomplete_files.yaml
index 6f15f097..8d373b88 100644
--- a/internal/test/testdata/index/incomplete_files.yaml
+++ b/internal/test/testdata/index/incomplete_files.yaml
@@ -1,9 +1,9 @@
 ---
 wantErrList:
   loadFailures:
-    - error: "invalid Policy.DerivedRoles: embedded message failed validation | caused by: invalid DerivedRoles.Definitions: value must contain at least 1 item(s)"
+    - error: "file is not valid: [/derivedRoles: missing properties: 'definitions']"
       file: derived.yaml
-    - error: "invalid Policy.PolicyType: value is required"
+    - error: "file is not valid: [/: missing properties: 'resourcePolicy', /: missing properties: 'principalPolicy', /: missing properties: 'derivedRoles']"
       file: resource.yaml
 files:
   "resource.yaml": |-
diff --git a/internal/test/testdata/server/playground/evaluate/pge_invalid_case_01.yaml b/internal/test/testdata/server/playground/evaluate/pge_invalid_case_01.yaml
index 8c041c59..30e2f9b9 100644
--- a/internal/test/testdata/server/playground/evaluate/pge_invalid_case_01.yaml
+++ b/internal/test/testdata/server/playground/evaluate/pge_invalid_case_01.yaml
@@ -36,11 +36,11 @@ playgroundEvaluate:
       "errors": [
         {
           "file": "resource.yaml",
-          "error": "Failed to read: failed to convert YAML to JSON: yaml: invalid leading UTF-8 octet"
+          "error": "Failed to read: failed to unmarshal file resource.yaml: yaml: invalid leading UTF-8 octet"
         },
         {
           "file": "common_roles.yaml",
-          "error": "Failed to read: failed to convert YAML to JSON: yaml: invalid leading UTF-8 octet"
+          "error": "Failed to read: failed to unmarshal file common_roles.yaml: yaml: invalid leading UTF-8 octet"
         }
       ]
     }
diff --git a/internal/test/testdata/server/playground/test/pgt_invalid_case_01.yaml b/internal/test/testdata/server/playground/test/pgt_invalid_case_01.yaml
index 735bf056..bee4e89f 100644
--- a/internal/test/testdata/server/playground/test/pgt_invalid_case_01.yaml
+++ b/internal/test/testdata/server/playground/test/pgt_invalid_case_01.yaml
@@ -27,11 +27,11 @@ playgroundTest:
       "errors": [
         {
           "file": "resource.yaml",
-          "error": "Failed to read: failed to convert YAML to JSON: yaml: invalid leading UTF-8 octet"
+          "error": "Failed to read: failed to unmarshal file resource.yaml: yaml: invalid leading UTF-8 octet"
         },
         {
           "file": "common_roles.yaml",
-          "error": "Failed to read: failed to convert YAML to JSON: yaml: invalid leading UTF-8 octet"
+          "error": "Failed to read: failed to unmarshal file common_roles.yaml: yaml: invalid leading UTF-8 octet"
         }
       ]
     }
diff --git a/internal/test/testdata/server/playground/test/pgt_invalid_case_02.yaml b/internal/test/testdata/server/playground/test/pgt_invalid_case_02.yaml
index 6bb97a3a..5a003803 100644
--- a/internal/test/testdata/server/playground/test/pgt_invalid_case_02.yaml
+++ b/internal/test/testdata/server/playground/test/pgt_invalid_case_02.yaml
@@ -29,7 +29,7 @@ playgroundTest:
           {
             "file": "policy_04_test.yaml",
             "name": "Unknown",
-            "error": "failed to load test suite: failed to convert YAML to JSON: yaml: invalid leading UTF-8 octet",
+            "error": "failed to unmarshal file policy_04_test.yaml: yaml: invalid leading UTF-8 octet",
             "summary": {
               "overallResult": "RESULT_ERRORED"
             }
diff --git a/internal/test/testdata/server/playground/validate/pgv_invalid_case_01.yaml b/internal/test/testdata/server/playground/validate/pgv_invalid_case_01.yaml
index cfb15bb4..4ac641c9 100644
--- a/internal/test/testdata/server/playground/validate/pgv_invalid_case_01.yaml
+++ b/internal/test/testdata/server/playground/validate/pgv_invalid_case_01.yaml
@@ -23,11 +23,11 @@ playgroundValidate:
       "errors": [
         {
           "file": "resource.yaml",
-          "error": "Failed to read: failed to convert YAML to JSON: yaml: invalid leading UTF-8 octet"
+          "error": "Failed to read: failed to unmarshal file resource.yaml: yaml: invalid leading UTF-8 octet"
         },
         {
           "file": "common_roles.yaml",
-          "error": "Failed to read: failed to convert YAML to JSON: yaml: invalid leading UTF-8 octet"
+          "error": "Failed to read: failed to unmarshal file common_roles.yaml: yaml: invalid leading UTF-8 octet"
         }
       ]
     }
diff --git a/internal/test/testdata/store/derived_roles/common_roles.yaml b/internal/test/testdata/store/derived_roles/common_roles.yaml
index ec34c288..7ecc42b4 100644
--- a/internal/test/testdata/store/derived_roles/common_roles.yaml
+++ b/internal/test/testdata/store/derived_roles/common_roles.yaml
@@ -2,7 +2,7 @@
 apiVersion: "api.cerbos.dev/v1"
 description: |-
   Common dynamic roles used within the Apatr app
-derived_roles:
+derivedRoles:
   name: apatr_common_roles
   definitions:
     - name: owner
diff --git a/internal/test/testdata/store_archive/policies.tar b/internal/test/testdata/store_archive/policies.tar
index a889ef17..f2c2cd56 100644
--- a/internal/test/testdata/store_archive/policies.tar
+++ b/internal/test/testdata/store_archive/policies.tar
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:c06cbca64541c5df699390546c3ed0991a4daf0858e040f0487c8b6831443cea
-size 41472
+oid sha256:79322a54aab09a790f4d06b74a8f023f0f20a9b66d3b51e316f9b8ae874cdbd6
+size 37888
diff --git a/internal/test/testdata/store_archive/policies.tgz b/internal/test/testdata/store_archive/policies.tgz
index 20a1af9b..85ffddc0 100644
--- a/internal/test/testdata/store_archive/policies.tgz
+++ b/internal/test/testdata/store_archive/policies.tgz
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:c8da8c0bc2d1901fd9c4f2203113790565a7c3f9e33ee1847e673e0698d40e73
-size 2956
+oid sha256:b2eccc7c634da0ee2428d2fedc225ccaaa6d5adb7e24315bf4b8aa52d9776b31
+size 3900
diff --git a/internal/test/testdata/store_archive/policies.zip b/internal/test/testdata/store_archive/policies.zip
index d8fcb18a..b3035acd 100644
--- a/internal/test/testdata/store_archive/policies.zip
+++ b/internal/test/testdata/store_archive/policies.zip
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:381c0da22603d01f43567b9a36a832ff00c80ff0719c3f99e4e0b634513ecdcc
-size 13428
+oid sha256:7d2f30d888850398d72de193516c4e3735766ae31415b74ce5b1a1d2fe142d87
+size 13954
diff --git a/internal/test/testdata/verify/cases/case_004.yaml.golden b/internal/test/testdata/verify/cases/case_004.yaml.golden
index 3ec74ccb..935bed56 100644
--- a/internal/test/testdata/verify/cases/case_004.yaml.golden
+++ b/internal/test/testdata/verify/cases/case_004.yaml.golden
@@ -6,7 +6,7 @@
       "summary":  {
         "overallResult":  "RESULT_ERRORED"
       },
-      "error":  "failed to load test suite: failed to unmarshal JSON: proto: syntax error (line 1:1): unexpected token \"x\""
+      "error":  "file is not valid: [/: expected object, but got string]"
     }
   ],
   "summary":  {
diff --git a/internal/test/testdata/verify/cases/case_005.yaml.golden b/internal/test/testdata/verify/cases/case_005.yaml.golden
index 1f07073d..442d30cd 100644
--- a/internal/test/testdata/verify/cases/case_005.yaml.golden
+++ b/internal/test/testdata/verify/cases/case_005.yaml.golden
@@ -6,7 +6,7 @@
       "summary":  {
         "overallResult":  "RESULT_ERRORED"
       },
-      "error":  "failed to load test suite: invalid TestSuite.Tests: value must contain at least 1 item(s)"
+      "error":  "file is not valid: [/: missing properties: 'tests']"
     }
   ],
   "summary":  {
diff --git a/internal/test/testdata/verify/cases/case_007.yaml.golden b/internal/test/testdata/verify/cases/case_007.yaml.golden
index 317ef48e..9842cd5a 100644
--- a/internal/test/testdata/verify/cases/case_007.yaml.golden
+++ b/internal/test/testdata/verify/cases/case_007.yaml.golden
@@ -6,7 +6,7 @@
       "summary":  {
         "overallResult":  "RESULT_ERRORED"
       },
-      "error":  "failed to load test suite: failed to unmarshal JSON: proto: (line 1:84): invalid google.protobuf.Timestamp value \"blah\""
+      "error":  "file is not valid: [/options/now: 'blah' is not valid 'date-time']"
     }
   ],
   "summary":  {