Repository: appc/spec
Pull request #691 (2017-07-16 12:19:26)

[issue/690] allow . and - in environment variable names.
Files touched:  schema/types/environment.go, schema/types/environment_test.go

Allow additional characters in the set considered valid in environment variable names.  Prompted by
https://github.com/appc/docker2aci/issues/244 and https://github.com/rkt/rkt/issues/3532.

ae6rt: @lucab PTAL, taking you up on your offer:  https://github.com/rkt/rkt/issues/3532#issuecomment-279013222
ae6rt: The IEEE spec from which the original conservative character set is taken, as a reference:

http://pubs.opengroup.org/onlinepubs/9699919799/
lucab: @ae6rt thanks for the PR, can you please also update `SPEC.md` for the new characters?

Patch:
diff --git a/schema/types/environment.go b/schema/types/environment.go
index f152a6b..3e7b18f 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/spec/aci.md b/spec/aci.md
index 00a757f..0bb3295 100644
--- a/spec/aci.md
+++ b/spec/aci.md
@@ -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-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.
+    * **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.
     * **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).


Test patch:
diff --git a/schema/types/environment_test.go b/schema/types/environment_test.go
index 8c51bea..9df0c13 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,6 +65,18 @@ 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)