Repository: coreos/fleet
Pull request #1653 (2016-07-28 09:55:05)

api: reject templates for particular unit types
Following unit types don't support templates:

* automount
* busname
* device
* mount
* scope
* slice
* snapshot
* swap

On the other hand, only the following types are allowed for templates:

* path
* service
* socket
* target
* timer

As it's not that simple to support templates for particular unit types on systemd, reject templates for such units from the fleet side.

Fixes https://github.com/coreos/fleet/issues/1390


jonboulle: LGTM after a couple of minor ones

Patch:
diff --git a/Documentation/unit-files-and-scheduling.md b/Documentation/unit-files-and-scheduling.md
index badaf5e3..6daec1c2 100644
--- a/Documentation/unit-files-and-scheduling.md
+++ b/Documentation/unit-files-and-scheduling.md
@@ -10,7 +10,8 @@ The unit name must be of the form `string.suffix` or `string@instance.suffix`, w
 
 * `string` must not be an empty string and can only contain alphanumeric characters and any of `:_.@-`. Formally, it must match the regular expression `[a-zA-Z0-9:_.@-]+`
 * `instance` can be empty, and can only contain the same characters as are valid for `string`. Formally, it must match the regular expression `[a-zA-Z0-9:_.@-]*`
-* `suffix` must be one of the following unit types: `service`, `socket`, `device`, `mount`, `automount`, `timer`, `path`
+* `suffix` must be one of the following unit types: `automount`, `busname`, `device`, `mount`, `path`, `scope`, `service`, `slice`, `snapshot`, `socket`, `swap`, `target`, `timer`
+* `suffix` of templates must be one of the following unit types: `path`, `service`, `socket`, `target`, `timer`. Other types such as `mount` are not allowed to be used for templates.
 
 Note that these requirements are derived directly from systemd, with the only exception that the unit types are a subset of those supported by systemd.
 
diff --git a/api/units.go b/api/units.go
index 316fb661..73c331d4 100644
--- a/api/units.go
+++ b/api/units.go
@@ -160,19 +160,27 @@ const (
 )
 
 var validUnitTypes = pkg.NewUnsafeSet(
-	"service",
-	"socket",
+	"automount",
 	"busname",
-	"target",
-	"snapshot",
 	"device",
 	"mount",
-	"automount",
+	"path",
+	"scope",
+	"service",
+	"slice",
+	"snapshot",
+	"socket",
 	"swap",
+	"target",
 	"timer",
+)
+
+var validTemplateTypes = pkg.NewUnsafeSet(
 	"path",
-	"slice",
-	"scope",
+	"service",
+	"socket",
+	"target",
+	"timer",
 )
 
 // ValidateName ensures that a given unit name is valid; if not, an error is
@@ -193,9 +201,13 @@ func ValidateName(name string) error {
 	if dot == length-1 {
 		return errors.New(`unit name cannot end in "."`)
 	}
-	if suffix := name[dot+1:]; !validUnitTypes.Contains(suffix) {
+	suffix := name[dot+1:]
+	if !validUnitTypes.Contains(suffix) {
 		return fmt.Errorf("invalid unit type: %q", suffix)
 	}
+	if strings.Contains(name, "@") && !validTemplateTypes.Contains(suffix) {
+		return fmt.Errorf("invalid unit type for template: %q", suffix)
+	}
 	for _, char := range name[:dot] {
 		if !strings.ContainsRune(validChars, char) {
 			return fmt.Errorf("invalid character %q in unit name", char)


Test patch:
diff --git a/api/units_test.go b/api/units_test.go
index 62d58c59..308d9bc7 100644
--- a/api/units_test.go
+++ b/api/units_test.go
@@ -759,6 +759,15 @@ func TestValidateName(t *testing.T) {
 		// cannot start in "@"
 		"@foo.service",
 		"@this.mount",
+		// template cannot be used for particular types
+		"foo@1.automount",
+		"foo@1.busname",
+		"foo@1.device",
+		"foo@1.mount",
+		"foo@1.scope",
+		"foo@1.slice",
+		"foo@1.snapshot",
+		"foo@1.swap",
 	}
 	for _, name := range badTestCases {
 		if err := ValidateName(name); err == nil {
@@ -779,6 +788,12 @@ func TestValidateName(t *testing.T) {
 		"jalapano_chips.service",
 		// generate a name the exact length of unitNameMax
 		fmt.Sprintf("%0"+strconv.Itoa(unitNameMax)+"s", ".service"),
+		// template can be used for particular types
+		"foo@1.path",
+		"foo@1.service",
+		"foo@1.socket",
+		"foo@1.target",
+		"foo@1.timer",
 	}
 	for _, name := range goodTestCases {
 		if err := ValidateName(name); err != nil {