[nft PATCH RFC 0/2] JSON schema for nftables.py

[nft PATCH RFC 0/2] JSON schema for nftables.py

[Phil Sutter]

This is an initial implementation of support for jsonschema in
nftables.py. The goal is to have a schema definition which may be used
by users as well as our testsuite to make sure any JSON we accept or
generate is valid.

The added schema.json in patch 1 is very minimal for now - I have a more
detailed version but it is not complete yet. Also it is quite large
already, so for the sake of reviews the small one serves better.

A few aspects of the implementation I'm unsure of:

Keeping the schema in a "pure" JSON file makes things a bit complex: It
has to be shipped as data file and loaded by the validator using
json.load(). OTOH the content may be fed into json_verify and my editor
provides nicer syntax highlighting. An alternative would be to name it
schema.py, prefix the content with 'nftschema = ' and simply import it
into nftables.py. I don't think inlining the content is a good option
simply due to how large the file will get once definitions for all
statements and expressions are in there.

Introducing that SchemaValidator class is not really required, either.
Though squeezing everything into json_validate() method felt clumsy.
Also I wanted to avoid the explicit schema loading mentioned above upon
each call to json_validate(), so having an instance of a validator class
seemed like how one is supposed to do things in an object-oriented
language.

Note that SchemaValidator imports jsonschema upon instantiation. This
may be a bad idea to begin with, but the intention is to not introduce a
hard dependency on jsonschema in nftables.py. Same argument holds for
conditional import of traceback module in nft-test.py, although
validator errors are practically useless without it.

Phil Sutter (2):
  py: Implement JSON validation in nftables module
  tests/py: Support JSON validation

 py/Makefile.am       |  2 +-
 py/nftables.py       | 30 ++++++++++++++++++++++++++++++
 py/schema.json       | 17 +++++++++++++++++
 py/setup.py          |  1 +
 tests/py/nft-test.py | 29 ++++++++++++++++++++++++++++-
 5 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 py/schema.json

-- 
2.21.0

[nft PATCH RFC 1/2] py: Implement JSON validation in nftables module

[Phil Sutter]

Using jsonschema it is possible to validate any JSON input to make sure
it formally conforms with libnftables JSON API requirements.

Implement a simple validator class for use within a new Nftables class
method 'json_validate' and ship a minimal schema definition along with
the package.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 py/Makefile.am |  2 +-
 py/nftables.py | 30 ++++++++++++++++++++++++++++++
 py/schema.json | 17 +++++++++++++++++
 py/setup.py    |  1 +
 4 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 py/schema.json

diff --git a/py/Makefile.am b/py/Makefile.am
index 0963535d068dc..9fce7c9e54c38 100644
--- a/py/Makefile.am
+++ b/py/Makefile.am
@@ -1,4 +1,4 @@
-EXTRA_DIST = setup.py __init__.py nftables.py
+EXTRA_DIST = setup.py __init__.py nftables.py schema.json
 
 if HAVE_PYTHON
 
diff --git a/py/nftables.py b/py/nftables.py
index f07163573f9a6..e1955d98151ef 100644
--- a/py/nftables.py
+++ b/py/nftables.py
@@ -17,9 +17,24 @@
 import json
 from ctypes import *
 import sys
+import os
 
 NFTABLES_VERSION = "0.1"
 
+class SchemaValidator:
+    """Libnftables JSON validator using jsonschema"""
+
+    def __init__(self):
+        schema_path = os.path.join(os.path.dirname(__file__), "schema.json")
+        schema_file = file(schema_path)
+        self.schema = json.load(schema_file)
+        schema_file.close()
+        import jsonschema
+        self.jsonschema = jsonschema
+
+    def validate(self, json):
+        self.jsonschema.validate(instance=json, schema=self.schema)
+
 class Nftables:
     """A class representing libnftables interface"""
 
@@ -46,6 +61,8 @@ class Nftables:
         "numeric_symbol": (1 << 9),
     }
 
+    validator = None
+
     def __init__(self, sofile="libnftables.so"):
         """Instantiate a new Nftables class object.
 
@@ -375,3 +392,16 @@ class Nftables:
         if len(output):
             output = json.loads(output)
         return (rc, output, error)
+
+    def json_validate(self, json_root):
+        """Validate JSON object against libnftables schema.
+
+        Accepts a hash object as input.
+
+        Returns True if JSON is valid, raises an exception otherwise.
+        """
+        if not self.validator:
+            self.validator = SchemaValidator()
+
+        self.validator.validate(json_root)
+        return True
diff --git a/py/schema.json b/py/schema.json
new file mode 100644
index 0000000000000..6cb731a228bf4
--- /dev/null
+++ b/py/schema.json
@@ -0,0 +1,17 @@
+{
+	"$schema": "http://json-schema.org/schema#",
+	"id": "http://netfilter.org/nftables/ruleset-schema.json",
+	"description": "libnftables JSON API schema",
+
+	"type": "object",
+        "properties": {
+		"nftables": {
+			"type": "array",
+			"minitems": 0,
+			"items": {
+				"type": "object"
+			}
+		}
+	},
+	"required": [ "nftables" ]
+}
diff --git a/py/setup.py b/py/setup.py
index ef143c42a21b0..72fc8fd98b269 100755
--- a/py/setup.py
+++ b/py/setup.py
@@ -11,6 +11,7 @@ setup(name='nftables',
       packages=['nftables'],
       provides=['nftables'],
       package_dir={'nftables':'.'},
+      package_data={'nftables':['schema.json']},
       classifiers=[
           'Development Status :: 4 - Beta',
           'Environment :: Console',
-- 
2.21.0

[nft PATCH RFC 2/2] tests/py: Support JSON validation

[Phil Sutter]

Introduce a new flag -s/--schema to nft-test.py which enables validation
of any JSON input and output against our schema.

If validation was requested on command line, try to import traceback
module. Upon validation errors, this provides more insight into what
went wrong. If unavailable, simply continue without it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/py/nft-test.py | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
index 1c0afd0ec0eb3..bc038629befd3 100755
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -687,6 +687,14 @@ def json_dump_normalize(json_string, human_readable = False):
     else:
         return json.dumps(json_obj, sort_keys = True)
 
+def json_validate(json_string):
+    json_obj = json.loads(json_string)
+    try:
+        nftables.json_validate(json_obj)
+    except Exception:
+        print_error("schema validation failed for input '%s'" % json_string)
+        if not traceback is None:
+            print_error(traceback.format_exc())
 
 def rule_add(rule, filename, lineno, force_all_family_option, filename_path):
     '''
@@ -912,6 +920,9 @@ def rule_add(rule, filename, lineno, force_all_family_option, filename_path):
                     "expr": json.loads(json_input),
             }}}]})
 
+            if enable_json_schema:
+                json_validate(cmd)
+
             json_old = nftables.set_json_output(True)
             ret = execute_cmd(cmd, filename, lineno, payload_log, debug="netlink")
             nftables.set_json_output(json_old)
@@ -945,6 +956,9 @@ def rule_add(rule, filename, lineno, force_all_family_option, filename_path):
             nftables.set_numeric_proto_output(numeric_proto_old)
             nftables.set_stateless_output(stateless_old)
 
+            if enable_json_schema:
+                json_validate(json_output)
+
             json_output = json.loads(json_output)
             for item in json_output["nftables"]:
                 if "rule" in item:
@@ -1341,12 +1355,17 @@ def main():
                         dest='enable_json',
                         help='test JSON functionality as well')
 
+    parser.add_argument('-s', '--schema', action='store_true',
+                        dest='enable_schema',
+                        help='verify json input/output against schema')
+
     args = parser.parse_args()
-    global debug_option, need_fix_option, enable_json_option
+    global debug_option, need_fix_option, enable_json_option, enable_json_schema
     debug_option = args.debug
     need_fix_option = args.need_fix_line
     force_all_family_option = args.force_all_family
     enable_json_option = args.enable_json
+    enable_json_schema = args.enable_schema
     specific_file = False
 
     signal.signal(signal.SIGINT, signal_handler)
@@ -1367,6 +1386,14 @@ def main():
     global nftables
     nftables = Nftables(sofile = 'src/.libs/libnftables.so')
 
+    global traceback
+    traceback = None
+    if enable_json_schema:
+        try:
+            import traceback
+        except:
+            pass
+
     test_files = files_ok = run_total = 0
     tests = passed = warnings = errors = 0
     global log_file
-- 
2.21.0

Re: [nft PATCH RFC 0/2] JSON schema for nftables.py

[Eric Garver]

Hi Phil,

Thanks for working on this.

On Thu, Apr 25, 2019 at 04:05:00PM +0200, Phil Sutter wrote:
> This is an initial implementation of support for jsonschema in
> nftables.py. The goal is to have a schema definition which may be used
> by users as well as our testsuite to make sure any JSON we accept or
> generate is valid.
> 
> The added schema.json in patch 1 is very minimal for now - I have a more
> detailed version but it is not complete yet. Also it is quite large
> already, so for the sake of reviews the small one serves better.
> 
> A few aspects of the implementation I'm unsure of:
> 
> Keeping the schema in a "pure" JSON file makes things a bit complex: It
> has to be shipped as data file and loaded by the validator using
> json.load(). OTOH the content may be fed into json_verify and my editor
> provides nicer syntax highlighting. An alternative would be to name it
> schema.py, prefix the content with 'nftschema = ' and simply import it
> into nftables.py. I don't think inlining the content is a good option
> simply due to how large the file will get once definitions for all
> statements and expressions are in there.

I very much prefer the external JSON file. Other projects can then use
it to validate the JSON they generate without going through libnftables.

> Introducing that SchemaValidator class is not really required, either.
> Though squeezing everything into json_validate() method felt clumsy.
> Also I wanted to avoid the explicit schema loading mentioned above upon
> each call to json_validate(), so having an instance of a validator class
> seemed like how one is supposed to do things in an object-oriented
> language.
> 
> Note that SchemaValidator imports jsonschema upon instantiation. This
> may be a bad idea to begin with, but the intention is to not introduce a
> hard dependency on jsonschema in nftables.py. Same argument holds for
> conditional import of traceback module in nft-test.py, although
> validator errors are practically useless without it.

I agree with the optional jsonschema dependency.

traceback is part of the python standard library. No reason to make it a
conditional import unless you're worried about the cost of importing it.

> 
> Phil Sutter (2):
>   py: Implement JSON validation in nftables module
>   tests/py: Support JSON validation
> 
>  py/Makefile.am       |  2 +-
>  py/nftables.py       | 30 ++++++++++++++++++++++++++++++
>  py/schema.json       | 17 +++++++++++++++++
>  py/setup.py          |  1 +
>  tests/py/nft-test.py | 29 ++++++++++++++++++++++++++++-
>  5 files changed, 77 insertions(+), 2 deletions(-)
>  create mode 100644 py/schema.json
> 
> -- 
> 2.21.0
> 

Re: [nft PATCH RFC 0/2] JSON schema for nftables.py

[Phil Sutter]

Hi Eric,

On Thu, Apr 25, 2019 at 12:35:46PM -0400, Eric Garver wrote:
> On Thu, Apr 25, 2019 at 04:05:00PM +0200, Phil Sutter wrote:
[...]
> > Keeping the schema in a "pure" JSON file makes things a bit complex: It
> > has to be shipped as data file and loaded by the validator using
> > json.load(). OTOH the content may be fed into json_verify and my editor
> > provides nicer syntax highlighting. An alternative would be to name it
> > schema.py, prefix the content with 'nftschema = ' and simply import it
> > into nftables.py. I don't think inlining the content is a good option
> > simply due to how large the file will get once definitions for all
> > statements and expressions are in there.
> 
> I very much prefer the external JSON file. Other projects can then use
> it to validate the JSON they generate without going through libnftables.

Valid point, although the file will be hidden in
/lib64/python2.7/site-packages/nftables/schema.json after installation.

> > Introducing that SchemaValidator class is not really required, either.
> > Though squeezing everything into json_validate() method felt clumsy.
> > Also I wanted to avoid the explicit schema loading mentioned above upon
> > each call to json_validate(), so having an instance of a validator class
> > seemed like how one is supposed to do things in an object-oriented
> > language.
> > 
> > Note that SchemaValidator imports jsonschema upon instantiation. This
> > may be a bad idea to begin with, but the intention is to not introduce a
> > hard dependency on jsonschema in nftables.py. Same argument holds for
> > conditional import of traceback module in nft-test.py, although
> > validator errors are practically useless without it.
> 
> I agree with the optional jsonschema dependency.
> 
> traceback is part of the python standard library. No reason to make it a
> conditional import unless you're worried about the cost of importing it.

Thanks for pointing this out! For the testsuite, unconditionally
importing traceback should be perfectly fine.

Thanks, Phil