diff --git a/api/units.go b/api/units.go index 84aa85ac9..8f6e1fc84 100644 --- a/api/units.go +++ b/api/units.go @@ -71,7 +71,7 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str sendError(rw, http.StatusBadRequest, fmt.Errorf("name in URL %q differs from unit name in request body %q", item, su.Name)) return } - if err := validateName(su.Name); err != nil { + if err := ValidateName(su.Name); err != nil { sendError(rw, http.StatusBadRequest, err) return } @@ -130,10 +130,10 @@ var validUnitTypes = pkg.NewUnsafeSet( "scope", ) -// validateName ensures that a given unit name is valid; if not, an error is +// ValidateName ensures that a given unit name is valid; if not, an error is // returned describing the first issue encountered. // systemd reference: `unit_name_is_valid` in `unit-name.c` -func validateName(name string) error { +func ValidateName(name string) error { length := len(name) if length == 0 { return errors.New("unit name cannot be empty") diff --git a/api/units_test.go b/api/units_test.go index d289a7b7e..f4da95929 100644 --- a/api/units_test.go +++ b/api/units_test.go @@ -657,7 +657,7 @@ func TestValidateName(t *testing.T) { "@this.mount", } for _, name := range badTestCases { - if err := validateName(name); err == nil { + if err := ValidateName(name); err == nil { t.Errorf("name %q: validation did not fail as expected!", name) } } @@ -676,7 +676,7 @@ func TestValidateName(t *testing.T) { fmt.Sprintf("%0"+strconv.Itoa(unitNameMax)+"s", ".service"), } for _, name := range goodTestCases { - if err := validateName(name); err != nil { + if err := ValidateName(name); err != nil { t.Errorf("name %q: validation failed unexpectedly! err=%v", name, err) } } diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index b229f3853..0abccb438 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -424,6 +424,9 @@ func createUnit(name string, uf *unit.UnitFile) (*schema.Unit, error) { // redundant with the check in api.unitsResource.set, but it is a // workaround to implementing the same check in the RegistryClient. It // will disappear once RegistryClient is deprecated. + if err := api.ValidateName(name); err != nil { + return nil, err + } if err := api.ValidateOptions(u.Options); err != nil { return nil, err } diff --git a/fleetctl/fleetctl_test.go b/fleetctl/fleetctl_test.go index 77f4df1a2..5859507f6 100644 --- a/fleetctl/fleetctl_test.go +++ b/fleetctl/fleetctl_test.go @@ -119,33 +119,72 @@ func TestCreateUnitFails(t *testing.T) { } cAPI = fakeAPI{} var i int + var un string var uf *unit.UnitFile defer func() { if r := recover(); r != nil { t.Errorf("case %d: unexpectedly called API!", i) + t.Logf("unit name: %q", un) t.Logf("unit file: %#v", uf) } }() - for i, uf = range []*unit.UnitFile{ - nil, - newUnitFile(t, `[X-Fleet] + testCases := []struct { + name string + uf *unit.UnitFile + }{ + { + "foo@{1,3}.service", + newUnitFile(t, ``), + }, + { + "foo@{1..3}.service", + newUnitFile(t, ``), + }, + { + "foo.{1-3}.service", + newUnitFile(t, ``), + }, + { + "foo.service", + nil, + }, + { + "foo.service", + newUnitFile(t, `[X-Fleet] + MachineOf=abcd + Conflicts=abcd`), + }, + { + "foo.service", + newUnitFile(t, `[X-Fleet] MachineOf=abcd Conflicts=abcd`), - newUnitFile(t, `[X-Fleet] -MachineOf=abcd -Conflicts=abcd`), - newUnitFile(t, `[X-Fleet] + }, + { + "foo.service", + newUnitFile(t, `[X-Fleet] Global=true MachineOf=abcd`), - newUnitFile(t, `[X-Fleet] + }, + { + "foo.service", + newUnitFile(t, `[X-Fleet] Global=true MachineOf=zxcvq`), - newUnitFile(t, `[X-Fleet] + }, + { + "foo.service", + newUnitFile(t, `[X-Fleet] Global=true Conflicts=bar`), - } { - if _, err := createUnit("foo.service", uf); err == nil { + }, + } + for i, tt := range testCases { + un = tt.name + uf = tt.uf + if _, err := createUnit(un, uf); err == nil { t.Errorf("case %d did not return error as expected!", i) + t.Logf("unit name: %v", un) t.Logf("unit file: %#v", uf) } }