Merge pull request #19263 from jfrazelle/update-aa-parser

refactor aaparser pkg, add unit tests

daemon/execdriver/native/apparmor.go

@@ -4,7 +4,6 @@ package native
 
 import (
 	"bufio"
-	"fmt"
 	"io"
 	"os"
 	"os/exec"
@@ -137,15 +136,10 @@ func installAppArmorProfile() error {
 	}
 	f.Close()
 
-	cmd := exec.Command("/sbin/apparmor_parser", "-r", "-W", "docker")
-	// to use the parser directly we have to make sure we are in the correct
-	// dir with the profile
-	cmd.Dir = "/etc/apparmor.d"
-
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		return fmt.Errorf("Error loading docker apparmor profile: %s (%s)", err, output)
+	if err := aaparser.LoadProfile(apparmorProfilePath); err != nil {
+		return err
 	}
+
 	return nil
 }
 

pkg/aaparser/aaparser.go

@@ -1,35 +1,66 @@
+// Package aaparser is a convenience package interacting with `apparmor_parser`.
 package aaparser
 
 import (
 	"fmt"
-	"log"
 	"os/exec"
+	"path/filepath"
 	"strconv"
 	"strings"
 )
 
-// GetVersion returns the major and minor version of apparmor_parser
+const (
+	binary = "apparmor_parser"
+)
+
+// GetVersion returns the major and minor version of apparmor_parser.
 func GetVersion() (int, int, error) {
-	// get the apparmor_version version
-	cmd := exec.Command("apparmor_parser", "--version")
+	output, err := cmd("", "--version")
+	if err != nil {
+		return -1, -1, err
+	}
+
+	return parseVersion(string(output))
+}
 
-	output, err := cmd.CombinedOutput()
+// LoadProfile runs `apparmor_parser -r -W` on a specified apparmor profile to
+// replace and write it to disk.
+func LoadProfile(profilePath string) error {
+	_, err := cmd(filepath.Dir(profilePath), "-r", "-W", filepath.Base(profilePath))
 	if err != nil {
-		log.Fatalf("getting apparmor_parser version failed: %s (%s)", err, output)
+		return err
 	}
+	return nil
+}
 
-	// parse the version from the output
+// cmd runs `apparmor_parser` with the passed arguments.
+func cmd(dir string, arg ...string) (string, error) {
+	c := exec.Command(binary, arg...)
+	c.Dir = dir
+
+	output, err := c.CombinedOutput()
+	if err != nil {
+		return "", fmt.Errorf("running `%s %s` failed with output: %s\nerror: %v", c.Path, strings.Join(c.Args, " "), string(output), err)
+	}
+
+	return string(output), nil
+}
+
+// parseVersion takes the output from `apparmor_parser --version` and returns
+// the major and minor version for `apparor_parser`.
+func parseVersion(output string) (int, int, error) {
 	// output is in the form of the following:
 	// AppArmor parser version 2.9.1
 	// Copyright (C) 1999-2008 Novell Inc.
 	// Copyright 2009-2012 Canonical Ltd.
-	lines := strings.SplitN(string(output), "\n", 2)
+	lines := strings.SplitN(output, "\n", 2)
 	words := strings.Split(lines[0], " ")
 	version := words[len(words)-1]
+
 	// split by major minor version
 	v := strings.Split(version, ".")
 	if len(v) < 2 {
-		return -1, -1, fmt.Errorf("parsing major minor version failed for %q", version)
+		return -1, -1, fmt.Errorf("parsing major minor version failed for output: `%s`", output)
 	}
 
 	majorVersion, err := strconv.Atoi(v[0])

pkg/aaparser/aaparser_test.go

@@ -0,0 +1,65 @@
+package aaparser
+
+import (
+	"testing"
+)
+
+type versionExpected struct {
+	output string
+	major  int
+	minor  int
+}
+
+func TestParseVersion(t *testing.T) {
+	versions := []versionExpected{
+		{
+			output: `AppArmor parser version 2.10
+Copyright (C) 1999-2008 Novell Inc.
+Copyright 2009-2012 Canonical Ltd.
+
+`,
+			major: 2,
+			minor: 10,
+		},
+		{
+			output: `AppArmor parser version 2.8
+Copyright (C) 1999-2008 Novell Inc.
+Copyright 2009-2012 Canonical Ltd.
+
+`,
+			major: 2,
+			minor: 8,
+		},
+		{
+			output: `AppArmor parser version 2.20
+Copyright (C) 1999-2008 Novell Inc.
+Copyright 2009-2012 Canonical Ltd.
+
+`,
+			major: 2,
+			minor: 20,
+		},
+		{
+			output: `AppArmor parser version 2.05
+Copyright (C) 1999-2008 Novell Inc.
+Copyright 2009-2012 Canonical Ltd.
+
+`,
+			major: 2,
+			minor: 5,
+		},
+	}
+
+	for _, v := range versions {
+		major, minor, err := parseVersion(v.output)
+		if err != nil {
+			t.Fatalf("expected error to be nil for %#v, got: %v", v, err)
+		}
+		if major != v.major {
+			t.Fatalf("expected major version to be %d, was %d, for: %#v\n", v.major, major, v)
+		}
+		if minor != v.minor {
+			t.Fatalf("expected minor version to be %d, was %d, for: %#v\n", v.minor, minor, v)
+		}
+	}
+}