diff --git a/api/units.go b/api/units.go index f6f06328b..84aa85ac9 100644 --- a/api/units.go +++ b/api/units.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "path" + "strings" "github.com/coreos/fleet/client" "github.com/coreos/fleet/job" @@ -103,12 +104,61 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str ur.update(rw, su.Name, su.DesiredState) } +const ( + // These constants taken from systemd + unitNameMax = 256 + digits = "0123456789" + lowercase = "abcdefghijklmnopqrstuvwxyz" + uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + alphanumerical = digits + lowercase + uppercase + validChars = alphanumerical + `:=+.\@` +) + +var validUnitTypes = pkg.NewUnsafeSet( + "service", + "socket", + "busname", + "target", + "snapshot", + "device", + "mount", + "automount", + "swap", + "timer", + "path", + "slice", + "scope", +) + // validateName ensures that a given unit name is valid; if not, an error is -// returned detailing the issue encountered. +// returned describing the first issue encountered. +// systemd reference: `unit_name_is_valid` in `unit-name.c` func validateName(name string) error { - if len(name) == 0 { + length := len(name) + if length == 0 { return errors.New("unit name cannot be empty") } + if length > unitNameMax { + return fmt.Errorf("unit name exceeds maximum length (%d)", unitNameMax) + } + dot := strings.Index(name, ".") + if dot == -1 { + return errors.New(`unit name must contain "."`) + } + if dot == length-1 { + return errors.New(`unit name cannot end in "."`) + } + if suffix := name[dot+1:]; !validUnitTypes.Contains(suffix) { + return fmt.Errorf("invalid unit type: %q", suffix) + } + for _, char := range name[:dot] { + if !strings.ContainsRune(validChars, char) { + return fmt.Errorf("invalid character %q in unit name", char) + } + } + if strings.HasPrefix(name, "@") { + return errors.New(`unit name cannot start in "@"`) + } return nil } diff --git a/api/units_test.go b/api/units_test.go index ba4607293..d289a7b7e 100644 --- a/api/units_test.go +++ b/api/units_test.go @@ -48,8 +48,8 @@ func TestUnitsSubResourceNotFound(t *testing.T) { func TestUnitsList(t *testing.T) { fr := registry.NewFakeRegistry() fr.SetJobs([]job.Job{ - {Name: "XXX"}, - {Name: "YYY"}, + {Name: "XXX.service"}, + {Name: "YYY.service"}, }) fAPI := &client.RegistryClient{fr} resource := &unitsResource{fAPI, "/units"} @@ -80,7 +80,7 @@ func TestUnitsList(t *testing.T) { t.Fatalf("Received unparseable body: %v", err) } - if len(page.Units) != 2 || page.Units[0].Name != "XXX" || page.Units[1].Name != "YYY" { + if len(page.Units) != 2 || page.Units[0].Name != "XXX.service" || page.Units[1].Name != "YYY.service" { t.Errorf("Received incorrect UnitPage entity: %v", page) } } @@ -160,14 +160,14 @@ func TestUnitGet(t *testing.T) { item string code int }{ - {item: "XXX", code: http.StatusOK}, + {item: "XXX.service", code: http.StatusOK}, {item: "ZZZ", code: http.StatusNotFound}, } fr := registry.NewFakeRegistry() fr.SetJobs([]job.Job{ - {Name: "XXX"}, - {Name: "YYY"}, + {Name: "XXX.service"}, + {Name: "YYY.service"}, }) fAPI := &client.RegistryClient{fr} resource := &unitsResource{fAPI, "/units"} @@ -208,17 +208,17 @@ func TestUnitsDestroy(t *testing.T) { }{ // Deletion of an existing unit should succeed { - init: []job.Job{job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}}, - arg: "XXX", + init: []job.Job{job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}}, + arg: "XXX.service", code: http.StatusNoContent, remaining: []string{}, }, // Deletion of a nonexistent unit should fail { - init: []job.Job{job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}}, - arg: "YYY", + init: []job.Job{job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}}, + arg: "YYY.service", code: http.StatusNotFound, - remaining: []string{"XXX"}, + remaining: []string{"XXX.service"}, }, } @@ -281,35 +281,35 @@ func TestUnitsSetDesiredState(t *testing.T) { }{ // Modify the desired State of an existing Job { - initJobs: []job.Job{job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}}, - initStates: map[string]job.JobState{"XXX": "inactive"}, - item: "XXX", - arg: schema.Unit{Name: "XXX", DesiredState: "launched"}, + initJobs: []job.Job{job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}}, + initStates: map[string]job.JobState{"XXX.service": "inactive"}, + item: "XXX.service", + arg: schema.Unit{Name: "XXX.service", DesiredState: "launched"}, code: http.StatusNoContent, - finalStates: map[string]job.JobState{"XXX": "launched"}, + finalStates: map[string]job.JobState{"XXX.service": "launched"}, }, // Create a new Unit { initJobs: []job.Job{}, initStates: map[string]job.JobState{}, - item: "YYY", + item: "YYY.service", arg: schema.Unit{ - Name: "YYY", + Name: "YYY.service", DesiredState: "loaded", Options: []*schema.UnitOption{ &schema.UnitOption{Section: "Service", Name: "Foo", Value: "Baz"}, }, }, code: http.StatusNoContent, - finalStates: map[string]job.JobState{"YYY": "loaded"}, + finalStates: map[string]job.JobState{"YYY.service": "loaded"}, }, // Creating a new Unit without Options fails { initJobs: []job.Job{}, initStates: map[string]job.JobState{}, - item: "YYY", + item: "YYY.service", arg: schema.Unit{ - Name: "YYY", + Name: "YYY.service", DesiredState: "loaded", Options: []*schema.UnitOption{}, }, @@ -319,42 +319,42 @@ func TestUnitsSetDesiredState(t *testing.T) { // Referencing a Unit where the name is inconsistent with the path should fail { initJobs: []job.Job{ - job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}, - job.Job{Name: "YYY", Unit: newUnit(t, "[Service]\nFoo=Baz")}, + job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}, + job.Job{Name: "YYY.service", Unit: newUnit(t, "[Service]\nFoo=Baz")}, }, initStates: map[string]job.JobState{ - "XXX": "inactive", - "YYY": "inactive", + "XXX.service": "inactive", + "YYY.service": "inactive", }, - item: "XXX", + item: "XXX.service", arg: schema.Unit{ - Name: "YYY", + Name: "YYY.service", DesiredState: "loaded", }, code: http.StatusBadRequest, finalStates: map[string]job.JobState{ - "XXX": "inactive", - "YYY": "inactive", + "XXX.service": "inactive", + "YYY.service": "inactive", }, }, // Referencing a Unit where the name is omitted should substitute the name from the path { initJobs: []job.Job{ - job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}, - job.Job{Name: "YYY", Unit: newUnit(t, "[Service]\nFoo=Baz")}, + job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}, + job.Job{Name: "YYY.service", Unit: newUnit(t, "[Service]\nFoo=Baz")}, }, initStates: map[string]job.JobState{ - "XXX": "inactive", - "YYY": "inactive", + "XXX.service": "inactive", + "YYY.service": "inactive", }, - item: "XXX", + item: "XXX.service", arg: schema.Unit{ DesiredState: "loaded", }, code: http.StatusNoContent, finalStates: map[string]job.JobState{ - "XXX": "loaded", - "YYY": "inactive", + "XXX.service": "loaded", + "YYY.service": "inactive", }, }, } @@ -628,14 +628,56 @@ func TestValidateOptions(t *testing.T) { } func TestValidateName(t *testing.T) { - testCases := map[string]bool{ - "asdf": true, - "": false, + badTestCases := []string{ + // cannot be empty + "", + // cannot be longer than unitNameMax + fmt.Sprintf("%0"+strconv.Itoa(unitNameMax+1)+"s", ".service"), + fmt.Sprintf("%0"+strconv.Itoa(unitNameMax*2)+"s", ".mount"), + // must contain "." + "fooservice", + "barmount", + "bar@foo", + // cannot end in "." + "foo.", + "foo.service.", + "foo@foo.service.", + // must have valid unit suffix + "foo.bar", + "hello.cerveza", + "foo.servICE", + "yo.yo.service", + // cannot have invalid characters + "foo%.service", + "foo$asd.service", + "hello##.mount", + "yes/no.service", + // cannot start in "@" + "@foo.service", + "@this.mount", } - for name, want := range testCases { - err := validateName(name) - if (err == nil) != want { - t.Errorf("name %q: bad validation: want %t, got err=%v", name, want, err) + for _, name := range badTestCases { + if err := validateName(name); err == nil { + t.Errorf("name %q: validation did not fail as expected!", name) + } + } + + goodTestCases := []string{ + "foo.service", + "hello.mount", + "foo@123.service", + "foo@.service", + "hello@world.path", + "hello:world.service", + "this+that.mount", + "yes@no\\.service", + "dog=woof@.mount", + // generate a name the exact length of unitNameMax + fmt.Sprintf("%0"+strconv.Itoa(unitNameMax)+"s", ".service"), + } + for _, name := range goodTestCases { + if err := validateName(name); err != nil { + t.Errorf("name %q: validation failed unexpectedly! err=%v", name, err) } } }