Repository: appc/spec
Pull request #692 (2017-09-06 18:30:57)

linuxSeccompBase: Fix errno check
errno symbols can contain numeric characters.


alban: LGTM

Examples:
```
$ errno --list|gawk '{print $1}'|grep [0-9]
E2BIG
EL2NSYNC
EL3HLT
EL3RST
EL2HLT
```
lucab: Definitely my bad, I forgot some of them contain numbers. Perhaps a better check would be `starts with uppercase "E" and contains only uppercase or digits`.
glevand: Updated patch to do the checks as suggested.

cc: @lucab

lucab: (go fmt is unhappy about the latest rebase)

Patch:
diff --git a/ace/image_manifest_main.json.in b/ace/image_manifest_main.json.in
index 1e10377..ddfc3ab 100644
--- a/ace/image_manifest_main.json.in
+++ b/ace/image_manifest_main.json.in
@@ -4,8 +4,8 @@
     "name": "coreos.com/ace-validator-main",
     "labels": [
         { "name": "version", "value": "0.8.10" },
-        { "name": "os", "value": "@ACI_OS@" },
-        { "name": "arch", "value": "@ACI_ARCH@" }
+        { "name": "os", "value": "@GOOS@" },
+        { "name": "arch", "value": "@GOARCH@" }
     ],
     "app": {
         "exec": [
diff --git a/ace/image_manifest_sidekick.json.in b/ace/image_manifest_sidekick.json.in
index 7fe3e06..accff02 100644
--- a/ace/image_manifest_sidekick.json.in
+++ b/ace/image_manifest_sidekick.json.in
@@ -4,8 +4,8 @@
     "name": "coreos.com/ace-validator-sidekick",
     "labels": [
         { "name": "version", "value": "0.8.10" },
-        { "name": "os", "value": "@ACI_OS@" },
-        { "name": "arch", "value": "@ACI_ARCH@" }
+        { "name": "os", "value": "@GOOS@" },
+        { "name": "arch", "value": "@GOARCH@" }
     ],
     "app": {
         "exec": [
diff --git a/schema/pod.go b/schema/pod.go
index a834e51..9ed6c9e 100644
--- a/schema/pod.go
+++ b/schema/pod.go
@@ -83,15 +83,6 @@ func (pm *PodManifest) assertValid() error {
 	if pm.ACKind != PodManifestKind {
 		return pmKindError
 	}
-
-	// ensure volumes names are unique (unique key)
-	volNames := make(map[types.ACName]bool, len(pm.Volumes))
-	for _, vol := range pm.Volumes {
-		if volNames[vol.Name] {
-			return fmt.Errorf("duplicate volume name %q", vol.Name)
-		}
-		volNames[vol.Name] = true
-	}
 	return nil
 }
 
diff --git a/schema/types/environment.go b/schema/types/environment.go
index 3e7b18f..f152a6b 100644
--- a/schema/types/environment.go
+++ b/schema/types/environment.go
@@ -21,7 +21,7 @@ import (
 )
 
 var (
-	envPattern = regexp.MustCompile("^[A-Za-z_][A-Za-z_0-9.-]*$")
+	envPattern = regexp.MustCompile("^[A-Za-z_][A-Za-z_0-9]*$")
 )
 
 type Environment []EnvironmentVariable
diff --git a/schema/types/isolator_linux_specific.go b/schema/types/isolator_linux_specific.go
index 3d5dc9f..ad3441a 100644
--- a/schema/types/isolator_linux_specific.go
+++ b/schema/types/isolator_linux_specific.go
@@ -226,9 +226,13 @@ func (l linuxSeccompBase) AssertValid() error {
 	if l.val.Errno == "" {
 		return nil
 	}
-	for _, c := range l.val.Errno {
-		if !unicode.IsUpper(c) {
-			return errors.New("errno must be an upper case string")
+	for i, c := range l.val.Errno {
+		if i == 0 && c != 'E' {
+			s := fmt.Sprintf("errno must start with an 'E' character, got %s", l.val.Errno)
+			return errors.New(s)
+		} else if !(unicode.IsUpper(c) || unicode.IsNumber(c)) {
+			s := fmt.Sprintf("errno can only contain upper case or numeric characters, got %s", l.val.Errno)
+			return errors.New(s)
 		}
 	}
 	return nil
diff --git a/schema/types/labels.go b/schema/types/labels.go
index 25931f9..89fa428 100644
--- a/schema/types/labels.go
+++ b/schema/types/labels.go
@@ -28,11 +28,7 @@ var ValidOSArch = map[string][]string{
 
 type Labels []Label
 
-type labelsSlice Labels
-
-func (l labelsSlice) Len() int           { return len(l) }
-func (l labelsSlice) Swap(i, j int)      { l[i], l[j] = l[j], l[i] }
-func (l labelsSlice) Less(i, j int) bool { return l[i].Name < l[j].Name }
+type labels Labels
 
 type Label struct {
 	Name  ACIdentifier `json:"name"`
@@ -101,11 +97,11 @@ func (l Labels) MarshalJSON() ([]byte, error) {
 	if err := l.assertValid(); err != nil {
 		return nil, err
 	}
-	return json.Marshal(labelsSlice(l))
+	return json.Marshal(labels(l))
 }
 
 func (l *Labels) UnmarshalJSON(data []byte) error {
-	var jl labelsSlice
+	var jl labels
 	if err := json.Unmarshal(data, &jl); err != nil {
 		return err
 	}
@@ -145,7 +141,6 @@ func LabelsFromMap(labelsMap map[ACIdentifier]string) (Labels, error) {
 	if err := labels.assertValid(); err != nil {
 		return nil, err
 	}
-	sort.Sort(labelsSlice(labels))
 	return labels, nil
 }
 
diff --git a/scripts/build-ace-validator-acis b/scripts/build-ace-validator-acis
index 8ace3c2..72a3579 100755
--- a/scripts/build-ace-validator-acis
+++ b/scripts/build-ace-validator-acis
@@ -4,13 +4,8 @@ set -eu
 
 PREFIX="ace"
 : ${NO_SIGNATURE=}
-
-ACI_OS="$(go env GOOS)"
+GOOS="$(go env GOOS)"
 GOARCH="$(go env GOARCH)"
-case $GOARCH in
-	arm64)	ACI_ARCH="aarch64" ;;
-	*)	ACI_ARCH="$GOARCH" ;;
-esac
 
 if ! [[ $0 =~ "scripts/build-ace-validator-acis" ]]; then 
 	echo "invoke from repository root" 1>&2
@@ -25,7 +20,7 @@ for typ in main sidekick; do
 	layoutdir="bin/ace-validator-${typ}-layout"
 	mkdir -p ${layoutdir}/rootfs/opt/acvalidator
 	cp bin/ace-validator ${layoutdir}/rootfs/
-	sed -e "s/@ACI_OS@/$ACI_OS/" -e "s/@ACI_ARCH@/$ACI_ARCH/" < ${PREFIX}/image_manifest_${typ}.json.in > ${layoutdir}/manifest
+	sed -e "s/@GOOS@/$GOOS/" -e "s/@GOARCH@/$GOARCH/" < ${PREFIX}/image_manifest_${typ}.json.in > ${layoutdir}/manifest
 	# now build the tarball, and sign it
 	pushd ${layoutdir} >/dev/null
 		# Set a consistent timestamp so we get a consistent hash
diff --git a/spec/aci.md b/spec/aci.md
index 0bb3295..c26dc5e 100644
--- a/spec/aci.md
+++ b/spec/aci.md
@@ -214,7 +214,7 @@ JSON Schema for the Image Manifest (app image manifest, ACI manifest), conformin
         },
         {
             "name": "appc.io/executor/supports-systemd-notify",
-            "value": "false"
+            "value": false
         }
     ]
 }
@@ -234,7 +234,7 @@ JSON Schema for the Image Manifest (app image manifest, ACI manifest), conformin
         * **pre-start** - executed and must exit before the long running main **exec** binary is launched
         * **post-stop** - executed if the main **exec** process is killed. This can be used to cleanup resources in the case of clean application shutdown, but cannot be relied upon in the face of machine failure.
     * **workingDirectory** (string, optional) working directory of the launched application, relative to the application image's root (must be an absolute path, defaults to "/", ACE can override). If the directory does not exist in the application's assembled rootfs (including any dependent images and mounted volumes), the ACE must fail execution.
-    * **environment** (list of objects, optional) represents the app's environment variables (ACE can append). The listed objects must have two key-value pairs: **name** and **value**. The **name** must consist solely of letters, digits, and underscores '_' as outlined in [IEEE Std 1003.1-2008, 2016 Edition](http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html), with practical considerations dictating that the name may also include periods '.' and hyphens '-'. The **value** is an arbitrary string. These values are not evaluated in any way, and no substitutions are made.
+    * **environment** (list of objects, optional) represents the app's environment variables (ACE can append). The listed objects must have two key-value pairs: **name** and **value**. The **name** must consist solely of letters, digits, and underscores '_' as outlined in [IEEE Std 1003.1-2001](http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html). The **value** is an arbitrary string. These values are not evaluated in any way, and no substitutions are made.
     * **isolators** (list of objects of type [Isolator](types.md#isolator-type), optional) list of isolation steps that SHOULD be applied to the app.
     * **mountPoints** (list of objects, optional) locations where an app is expecting external data to be mounted. The listed objects contain the following key-value pairs: the **name** indicates a label to refer to a mount point (which may be used by the executor when resolving a mount to a volume in the PodManifest), and the **path** stipulates where it is to be mounted inside the rootfs. The name is restricted to the [AC Name](types.md#ac-name-type) Type formatting. **readOnly** is a boolean indicating whether or not the mount point will be read-only (defaults to "false" if unsupplied).
     * **ports** (list of objects, optional) ports that this app will be listening on once started. This field is informational: example uses include helping users to discover the listening ports of the application, or indicating to executors ports that should be exposed on the host. This information could also optionally be used to limit the inbound connections to the container via firewall rules to only ports that are explicitly exposed. Each object can represent either a single port or a port range (contiguous set of ports).
@@ -256,7 +256,7 @@ JSON Schema for the Image Manifest (app image manifest, ACI manifest), conformin
     * **authors** contact details of the people or organization responsible for the image (freeform string)
     * **homepage** URL to find more information on the image (string, must be a URL with scheme HTTP or HTTPS)
     * **documentation** URL to get documentation on the image (string, must be a URL with scheme HTTP or HTTPS)
-    * **appc.io/executor/supports-systemd-notify** (string boolean, optional, defaults to "false" if unset) if set to "true", the application SHOULD use the sd\_notify mechanism to signal when it is ready. Also it SHOULD be able to detect if the executor had not set up the sd\_notify mechanism and skip the notification without error ([sd_notify()](https://www.freedesktop.org/software/systemd/man/sd_notify.html) from libsystemd does that automatically).
+    * **appc.io/executor/supports-systemd-notify** (boolean, optional, defaults to "false" if unset) if set to true, the application SHOULD use the sd\_notify mechanism to signal when it is ready. Also it SHOULD be able to detect if the executor had not set up the sd\_notify mechanism and skip the notification without error ([sd_notify()](https://www.freedesktop.org/software/systemd/man/sd_notify.html) from libsystemd does that automatically).
 
 #### Dependency Matching
 


Test patch:
diff --git a/schema/pod_test.go b/schema/pod_test.go
index 6932bb5..978e99d 100644
--- a/schema/pod_test.go
+++ b/schema/pod_test.go
@@ -71,43 +71,3 @@ func TestAppList(t *testing.T) {
 		t.Errorf("want err, got nil")
 	}
 }
-
-func TestPodManifestUniqueVolumeNames(t *testing.T) {
-	pm := BlankPodManifest()
-	volA, err := types.VolumeFromString("simple,kind=empty")
-	if err != nil {
-		t.Errorf("expected nil error, got %q", err)
-	}
-	volB, err := types.VolumeFromString("simple,kind=host,source=/tmp")
-	if err != nil {
-		t.Errorf("expected nil error, got %q", err)
-	}
-	pm.Volumes = append(pm.Volumes, *volA, *volB)
-	if _, err := pm.MarshalJSON(); err == nil {
-		t.Errorf("expected duplicate volume name error, got nil")
-	}
-
-	pmj := `
-{
-        "acVersion": "0.8.10",
-        "acKind": "PodManifest",
-        "apps": [
-        ],
-        "volumes": [
-                {
-                        "name": "simplename",
-                        "kind": "empty"
-                },
-                {
-                        "name": "simplename",
-                        "kind": "host",
-                        "source": "/tmp"
-                }
-        ]
-}
-`
-	err = pm.UnmarshalJSON([]byte(pmj))
-	if err == nil {
-		t.Errorf("expected duplicate volume name error, got nil")
-	}
-}
diff --git a/schema/types/environment_test.go b/schema/types/environment_test.go
index 9df0c13..8c51bea 100644
--- a/schema/types/environment_test.go
+++ b/schema/types/environment_test.go
@@ -45,10 +45,10 @@ func TestEnvironmentAssertValid(t *testing.T) {
 			},
 			true,
 		},
-		// name with non [A-Za-z0-9_.-] should fail
+		// name with non [A-Za-z0-9_] should fail
 		{
 			Environment{
-				EnvironmentVariable{"VERBOSE|DEBUG", "true"},
+				EnvironmentVariable{"VERBOSE-DEBUG", "true"},
 			},
 			true,
 		},
@@ -65,18 +65,6 @@ func TestEnvironmentAssertValid(t *testing.T) {
 			},
 			false,
 		},
-		{
-			Environment{
-				EnvironmentVariable{"DEBUG.0", "true"},
-			},
-			false,
-		},
-		{
-			Environment{
-				EnvironmentVariable{"DEBUG-0", "true"},
-			},
-			false,
-		},
 	}
 	for i, test := range tests {
 		env := Environment(test.env)
diff --git a/schema/types/labels_test.go b/schema/types/labels_test.go
index 82a1122..31da1d8 100644
--- a/schema/types/labels_test.go
+++ b/schema/types/labels_test.go
@@ -16,8 +16,6 @@ package types
 
 import (
 	"encoding/json"
-	"errors"
-	"reflect"
 	"strings"
 	"testing"
 )
@@ -223,67 +221,3 @@ func TestLabels(t *testing.T) {
 		}
 	}
 }
-
-func TestLabelsFromMap(t *testing.T) {
-	tests := []struct {
-		in          map[ACIdentifier]string
-		expectedOut Labels
-		expectedErr error
-	}{
-		{
-			in: map[ACIdentifier]string{
-				"foo": "bar",
-				"bar": "baz",
-				"baz": "foo",
-			},
-			expectedOut: []Label{
-				Label{
-					Name:  "bar",
-					Value: "baz",
-				},
-				Label{
-					Name:  "baz",
-					Value: "foo",
-				},
-				Label{
-					Name:  "foo",
-					Value: "bar",
-				},
-			},
-		},
-		{
-			in: map[ACIdentifier]string{
-				"foo": "",
-			},
-			expectedOut: []Label{
-				Label{
-					Name:  "foo",
-					Value: "",
-				},
-			},
-		},
-		{
-			in: map[ACIdentifier]string{
-				"name": "foo",
-			},
-			expectedErr: errors.New(`invalid label name: "name"`),
-		},
-	}
-
-	for i, test := range tests {
-		out, err := LabelsFromMap(test.in)
-		if err != nil {
-			if err.Error() != test.expectedErr.Error() {
-				t.Errorf("case %d: expected %v = %v", i, err, test.expectedErr)
-			}
-			continue
-		}
-		if test.expectedErr != nil {
-			t.Errorf("case %d: expected error %v, but got none", i, test.expectedErr)
-			continue
-		}
-		if !reflect.DeepEqual(test.expectedOut, out) {
-			t.Errorf("case %d: expected %v = %v", i, out, test.expectedOut)
-		}
-	}
-}