state: check unit/service name validity

state: check unit/service name validity

I originally intended to just move the regexps, but then I realised that we
don't get any of this checking "free" with mongo in the way we did with ZK,
and it is probably a good idea to maintain naming restrictions (given that
there is at least one place we expect to infer entity kind from name
structure (possibly status? possibly set-/get-constraints?)).

https://code.launchpad.net/~fwereade/juju-core/almost-trivial-check-valid-names/+merge/125433

(do not edit description out of merge proposal)

[fwereade, 2012-09-20 09:07:33]

Please take a look.

[niemeyer, 2012-09-20 09:39:41]

Great catch, thanks. A couple of details:

https://codereview.appspot.com/6551044/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/6551044/diff/1/cmd/juju/deploy.go#newcode97
cmd/juju/deploy.go:97: // TODO check for valid service name *before* touching
state?
s/?//; we don't want to upload the charm if the service name is wrong.

https://codereview.appspot.com/6551044/diff/1/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6551044/diff/1/state/state.go#newcode30
state/state.go:30: ValidService =
regexp.MustCompile("^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*$")
Feels ok to use regexes to implement them, but I'd prefer to have actual methods
rather than exposing the patterns, and using them even internally.

func IsServiceName(name string) bool
func IsUnitName(name string) bool

[fwereade, 2012-09-20 09:52:55]

Please take a look.

[niemeyer, 2012-09-20 10:35:34]

LGTM with the trivial:

https://codereview.appspot.com/6551044/diff/5001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6551044/diff/5001/state/state.go#newcode34
state/state.go:34: // IsServiceName returns true if name is a valid service
name.
s/true if/whether/ (both are of course right.. it's just upstream Go convention)

https://codereview.appspot.com/6551044/diff/5001/state/state.go#newcode39
state/state.go:39: // IsUnitName returns true if name is a valid service name.
s/true if/whether/
s/service/unit/

[fwereade, 2012-09-20 10:41:32]

*** Submitted:

state: check unit/service name validity

I originally intended to just move the regexps, but then I realised that we
don't get any of this checking "free" with mongo in the way we did with ZK,
and it is probably a good idea to maintain naming restrictions (given that
there is at least one place we expect to infer entity kind from name
structure (possibly status? possibly set-/get-constraints?)).

R=niemeyer
CC=
https://codereview.appspot.com/6551044

https://codereview.appspot.com/6551044/diff/5001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6551044/diff/5001/state/state.go#newcode34
state/state.go:34: // IsServiceName returns true if name is a valid service
name.
On 2012/09/20 10:35:34, niemeyer wrote:
> s/true if/whether/ (both are of course right.. it's just upstream Go
convention)

Done.

https://codereview.appspot.com/6551044/diff/5001/state/state.go#newcode39
state/state.go:39: // IsUnitName returns true if name is a valid service name.
On 2012/09/20 10:35:34, niemeyer wrote:
> s/true if/whether/
> s/service/unit/

Done.