Merge pull request #4 from danwinship/version

Require nft v1.0.1 or later

README.md

@@ -21,12 +21,21 @@ that is quite different from all documented examples of nftables usage
 because there is no easy way to convert the "standard" representation
 of nftables rules into the netlink form.
 
-(Actually, that's not quite true: the `nft` CLI is just a thin wrapper
-around `libnftables`, and it would be possible for knftables to use
-cgo to invoke that library instead of using an external binary.
-However, this would be harder to build and ship, so I'm not bothering
-with that for now. But this could be done in the future without
-needing to change knftables's API.)
+(Actually, it's not quite true that there's no other usable API: the
+`nft` CLI is just a thin wrapper around `libnftables`, and it would be
+possible for knftables to use cgo to invoke that library instead of
+using an external binary. However, this would be harder to build and
+ship, so I'm not bothering with that for now. But this could be done
+in the future without needing to change knftables's API.)
+
+knftables requires nft version 1.0.1 or later, because earlier
+versions would download and process the entire ruleset regardless of
+what you were doing, which, besides being pointlessly inefficient,
+means that in some cases, other people using new features in _their_
+tables could prevent you from modifying _your_ table. (In particular,
+a change in how some rules are generated starting in nft 1.0.3
+triggers a crash in nft 0.9.9 and earlier, _even if you aren't looking
+at the table containing that rule_.)
 
 ## Usage
 

nftables.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os/exec"
+	"strings"
 )
 
 // Interface is an interface for running nftables commands against a given family and table.
@@ -73,7 +74,8 @@ type realNFTables struct {
 	path string
 }
 
-// for unit tests
+// newInternal creates a new nftables.Interface for interacting with the given table; this
+// is split out from New() so it can be used from unit tests with a fakeExec.
 func newInternal(family Family, table string, execer execer) (Interface, error) {
 	var err error
 
@@ -91,17 +93,29 @@ func newInternal(family Family, table string, execer execer) (Interface, error)
 		return nil, fmt.Errorf("could not find nftables binary: %w", err)
 	}
 
-	cmd := exec.Command(nft.path, "--check", "add", "table", string(nft.family), nft.table)
-	_, err = nft.exec.Run(cmd)
+	cmd := exec.Command(nft.path, "--version")
+	out, err := nft.exec.Run(cmd)
 	if err != nil {
 		return nil, fmt.Errorf("could not run nftables command: %w", err)
 	}
+	if strings.HasPrefix(out, "nftables v0.") || strings.HasPrefix(out, "nftables v1.0.0 ") {
+		return nil, fmt.Errorf("nft version must be v1.0.1 or later (got %s)", strings.TrimSpace(out))
+	}
 
+	// Check that (a) nft works, (b) we have permission, (c) the kernel is new enough
+	// to support object comments.
 	cmd = exec.Command(nft.path, "--check", "add", "table", string(nft.family), nft.table,
 		"{", "comment", `"test"`, "}",
 	)
 	_, err = nft.exec.Run(cmd)
 	if err != nil {
+		// Try again, checking just that (a) nft works, (b) we have permission.
+		cmd := exec.Command(nft.path, "--check", "add", "table", string(nft.family), nft.table)
+		_, err = nft.exec.Run(cmd)
+		if err != nil {
+			return nil, fmt.Errorf("could not run nftables command: %w", err)
+		}
+
 		nft.noObjectComments = true
 	}
 

nftables_test.go

@@ -35,7 +35,8 @@ func newTestInterface(t *testing.T, family Family, tableName string) (Interface,
 	}
 	fexec.expected = append(fexec.expected,
 		expectedCmd{
-			args: []string{"/nft", "--check", "add", "table", ip, tableName},
+			args:   []string{"/nft", "--version"},
+			stdout: "nftables v1.0.7 (Old Doc Yak)\n",
 		},
 		expectedCmd{
 			args: []string{"/nft", "--check", "add", "table", ip, tableName,
@@ -420,16 +421,28 @@ func TestFeatures(t *testing.T) {
 	for _, tc := range []struct {
 		name     string
 		commands []expectedCmd
-		result   nftContext
+		result   *nftContext
 	}{
+		{
+			name: "old nftables",
+			commands: []expectedCmd{
+				{
+					args: []string{
+						"/nft", "--version",
+					},
+					stdout: "nftables v0.9.3 (Topsy)\n",
+				},
+			},
+			result: nil,
+		},
 		{
 			name: "all features",
 			commands: []expectedCmd{
 				{
 					args: []string{
-						"/nft", "--check",
-						"add", "table", "ip", "testing",
+						"/nft", "--version",
 					},
+					stdout: "nftables v1.0.7 (Old Doc Yak)\n",
 				},
 				{
 					args: []string{
@@ -439,7 +452,7 @@ func TestFeatures(t *testing.T) {
 					},
 				},
 			},
-			result: nftContext{
+			result: &nftContext{
 				family: IPv4Family,
 				table:  "testing",
 			},
@@ -449,9 +462,9 @@ func TestFeatures(t *testing.T) {
 			commands: []expectedCmd{
 				{
 					args: []string{
-						"/nft", "--check",
-						"add", "table", "ip", "testing",
+						"/nft", "--version",
 					},
+					stdout: "nftables v1.0.7 (Old Doc Yak)\n",
 				},
 				{
 					args: []string{
@@ -461,8 +474,14 @@ func TestFeatures(t *testing.T) {
 					},
 					err: fmt.Errorf("Error: syntax error, unexpected comment"),
 				},
+				{
+					args: []string{
+						"/nft", "--check",
+						"add", "table", "ip", "testing",
+					},
+				},
 			},
-			result: nftContext{
+			result: &nftContext{
 				family: IPv4Family,
 				table:  "testing",
 
@@ -475,11 +494,18 @@ func TestFeatures(t *testing.T) {
 			fexec.expected = tc.commands
 			nft, err := newInternal(IPv4Family, "testing", fexec)
 			if err != nil {
-				t.Fatalf("Unexpected error creating Interface: %v", err)
-			}
-			result := nft.(*realNFTables).nftContext
-			if !reflect.DeepEqual(tc.result, result) {
-				t.Errorf("Expected %#v, got %#v", tc.result, result)
+				if tc.result != nil {
+					t.Fatalf("Unexpected error creating Interface: %v", err)
+				}
+			} else {
+				result := nft.(*realNFTables).nftContext
+				if tc.result != nil {
+					if !reflect.DeepEqual(*tc.result, result) {
+						t.Errorf("Expected %#v, got %#v", *tc.result, result)
+					}
+				} else {
+					t.Fatalf("Expected failure, got %#v", result)
+				}
 			}
 		})
 	}