Merge lp:~frankban/juju-core/bug-1147138-annotations-api into lp:~juju/juju-core/trunk

[Francesco Banconi]

Annotations API functionality

Annotations are now exposed through the API via 'GetAnnotations' and
'SetAnnotation'. These allow a user to set an annotation on an Entity (service,
'unit', 'machine', with environment coming in a future branch), and retrieve
all annotations on an entity. Tests were added around the added functionality.
Note that now the service is an Entity, as suggested by Roger.

https://codereview.appspot.com/7526043/

[Francesco Banconi (frankban)]

Reviewers: mp+152037_code.launchpad.net,

Message:
Please take a look.

Description:
Annotations API functionality

Annotations are now exposed through the API via 'GetAnnotations' and
'SetAnnotation'. These allow a user to set an annotation on an Entity
(service,
'unit', 'machine', with environment coming in a future branch), and
retrieve
all annotations on an entity. Tests were added around the added
functionality.
Note that now the service is an Entity, as suggested by Roger.

https://code.launchpad.net/~frankban/juju-core/bug-1147138-annotations-api/+merge/152037

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7526043/

Affected files:
   A [revision details]
   M state/api/apiclient.go
   M state/apiserver/api_test.go
   M state/apiserver/apiserver.go
   M state/machine_test.go
   M state/service.go
   M state/service_test.go
   M state/state.go
   M state/state_test.go
   M state/unit_test.go
   M state/user.go
   M state/user_test.go

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/7526043/

[Dimiter Naydenov (dimitern)]

Overall LGTM, just a couple of comments.

https://codereview.appspot.com/7526043/diff/2001/state/api/apiclient.go
File state/api/apiclient.go (right):

https://codereview.appspot.com/7526043/diff/2001/state/api/apiclient.go#newcode127
state/api/apiclient.go:127: args := params.GetAnnotations{Id: id}
can you skip the keys here? If it's only one {id} will work.

https://codereview.appspot.com/7526043/diff/2001/state/api/apiclient.go#newcode138
state/api/apiclient.go:138: args := params.SetAnnotation{Id: id, Key:
key, Value: value}
similarly - see above

https://codereview.appspot.com/7526043/

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/7526043/diff/2001/state/api/apiclient.go
File state/api/apiclient.go (right):

https://codereview.appspot.com/7526043/diff/2001/state/api/apiclient.go#newcode127
state/api/apiclient.go:127: args := params.GetAnnotations{Id: id}
On 2013/03/07 14:28:01, dimitern wrote:
> can you skip the keys here? If it's only one {id} will work.

Done.

https://codereview.appspot.com/7526043/diff/2001/state/api/apiclient.go#newcode138
state/api/apiclient.go:138: args := params.SetAnnotation{Id: id, Key:
key, Value: value}
On 2013/03/07 14:28:01, dimitern wrote:
> similarly - see above

Done.

https://codereview.appspot.com/7526043/

[Roger Peppe (rogpeppe)]

LGTM with the below trivial points addressed.
really good stuff, thanks!

https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go
File state/api/apiclient.go (right):

https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go#newcode146
state/api/apiclient.go:146: func (c *Client) GetAnnotations(id string)
(*Annotations, error) {
i think there's not really a point in having a new type for annotations
here, just to contain a map.
i'd just return the map directly, like the state Annotations method
does.

something like this, perhaps? (note the variable name change,
just to make it obvious what kind of id we're talking about here).

// SetAnnotations returns annotations that have been
// set on the given entity.
func (c *Client) SetAnnotations(entityId string) (map[string]string,
error)

https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go#newcode156
state/api/apiclient.go:156: // SetAnnotation stores an annotation about
a given entity.
// SetAnnotation sets the annotation with the given key
// on the given entity to the given value.
// Currently annotations are supported on machines,
// services, units and the environment itself.
func (c *Client) SetAnnotation(entityId, key, value string) error {

?

https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go#newcode76
state/api/params/params.go:76: Id string
s/Id/EntityId/

https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go#newcode81
state/api/params/params.go:81: Id string
s/Id/EntityId/

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go
File state/apiserver/api_test.go (right):

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode301
state/apiserver/api_test.go:301: return func() {}, nil
the returned function should reset the annotation to its original value
- all these op* functions should leave the state unchanged.

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode641
state/apiserver/api_test.go:641: // Annotations are correctly set.
s/Annotations/Check annotations/ ?

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode642
state/apiserver/api_test.go:642: entity.Refresh()
check error

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode647
state/apiserver/api_test.go:647: // Annotations are correctly returned.
s/Annotations/Check annotations/ ?

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode648
state/apiserver/api_test.go:648: entity.Refresh()
check error

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode652
state/apiserver/api_test.go:652: err = entity.SetAnnotation(key, "")
check error

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/apiserver.go
File state/apiserver/apiserver.go (right):

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/apiserver.go#newcode308
state/apiserver/apiserver.go:308: func (c *srvClient)
GetA...

[Francesco Banconi (frankban)]

*** Submitted:

Annotations API functionality

Annotations are now exposed through the API via 'GetAnnotations' and
'SetAnnotation'. These allow a user to set an annotation on an Entity
(service,
'unit', 'machine', with environment coming in a future branch), and
retrieve
all annotations on an entity. Tests were added around the added
functionality.
Note that now the service is an Entity, as suggested by Roger.

R=dimitern, rog
CC=
https://codereview.appspot.com/7526043

https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go
File state/api/apiclient.go (right):

https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go#newcode146
state/api/apiclient.go:146: func (c *Client) GetAnnotations(id string)
(*Annotations, error) {
On 2013/03/08 20:52:37, rog wrote:
> i think there's not really a point in having a new type for
annotations here,
> just to contain a map.
> i'd just return the map directly, like the state Annotations method
does.

> something like this, perhaps? (note the variable name change,
> just to make it obvious what kind of id we're talking about here).

> // SetAnnotations returns annotations that have been
> // set on the given entity.
> func (c *Client) SetAnnotations(entityId string) (map[string]string,
error)

Done.

https://codereview.appspot.com/7526043/diff/8001/state/api/apiclient.go#newcode156
state/api/apiclient.go:156: // SetAnnotation stores an annotation about
a given entity.
On 2013/03/08 20:52:37, rog wrote:
> // SetAnnotation sets the annotation with the given key
> // on the given entity to the given value.
> // Currently annotations are supported on machines,
> // services, units and the environment itself.
> func (c *Client) SetAnnotation(entityId, key, value string) error {

> ?

Done.

https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go#newcode76
state/api/params/params.go:76: Id string
On 2013/03/08 20:52:37, rog wrote:
> s/Id/EntityId/

Done.

https://codereview.appspot.com/7526043/diff/8001/state/api/params/params.go#newcode81
state/api/params/params.go:81: Id string
On 2013/03/08 20:52:37, rog wrote:
> s/Id/EntityId/

Done.

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go
File state/apiserver/api_test.go (right):

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode301
state/apiserver/api_test.go:301: return func() {}, nil
On 2013/03/08 20:52:37, rog wrote:
> the returned function should reset the annotation to its original
value - all
> these op* functions should leave the state unchanged.

Done.

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode641
state/apiserver/api_test.go:641: // Annotations are correctly set.
On 2013/03/08 20:52:37, rog wrote:
> s/Annotations/Check annotations/ ?

Done.

https://codereview.appspot.com/7526043/diff/8001/state/apiserver/api_test.go#newcode642
state/apiserver/api_test.go:642: entity.Refresh()
On 2013/03/08 20:52:37, rog wrote:
> check error

Done.

https://codereview.appspot.com/7526043/diff/8001...

[Francesco Banconi (frankban)]

Hi Dimiter and Roger,
thanks for the reviews.

https://codereview.appspot.com/7526043/

=== modified file 'state/api/apiclient.go'
--- state/api/apiclient.go	2013-03-08 20:52:12 +0000
+++ state/api/apiclient.go	2013-03-08 21:34:19 +0000
@@ -179,6 +179,29 @@
 	return newAllWatcher(c, &info.AllWatcherId), nil
 }
 
+// GetAnnotations returns annotations that have been set on the given entity.
+func (c *Client) GetAnnotations(entityId string) (map[string]string, error) {
+	args := params.GetAnnotations{entityId}
+	ann := new(params.GetAnnotationsResults)
+	err := c.st.client.Call("Client", "", "GetAnnotations", args, ann)
+	if err != nil {
+		return nil, clientError(err)
+	}
+	return ann.Annotations, nil
+}
+
+// SetAnnotation sets the annotation with the given key on the given entity to
+// the given value. Currently annotations are supported on machines, services,
+// units and the environment itself.
+func (c *Client) SetAnnotation(entityId, key, value string) error {
+	args := params.SetAnnotation{entityId, key, value}
+	err := c.st.client.Call("Client", "", "SetAnnotation", args, nil)
+	if err != nil {
+		return clientError(err)
+	}
+	return nil
+}
+
 // Machine returns a reference to the machine with the given id.
 func (st *State) Machine(id string) (*Machine, error) {
 	m := &Machine{

=== modified file 'state/api/params/params.go'
--- state/api/params/params.go	2013-03-08 20:36:42 +0000
+++ state/api/params/params.go	2013-03-08 21:34:19 +0000
@@ -93,6 +93,23 @@
 	// future.
 }
 
+// GetAnnotationsResults holds annotations associated with an entity.
+type GetAnnotationsResults struct {
+	Annotations map[string]string
+}
+
+// GetAnnotations stores parameters for making the GetAnnotations call.
+type GetAnnotations struct {
+	EntityId string
+}
+
+// SetAnnotation stores parameters for making the SetAnnotation call.
+type SetAnnotation struct {
+	EntityId string
+	Key      string
+	Value    string
+}
+
 // Delta holds details of a change to the environment.
 type Delta struct {
 	// If Removed is true, the entity has been removed;

=== modified file 'state/apiserver/api_test.go'
--- state/apiserver/api_test.go	2013-03-08 20:36:42 +0000
+++ state/apiserver/api_test.go	2013-03-08 21:34:19 +0000
@@ -85,6 +85,14 @@
 	op:    opClientServiceUnexpose,
 	allow: []string{"user-admin", "user-other"},
 }, {
+	about: "Client.GetAnnotations",
+	op:    opClientGetAnnotations,
+	allow: []string{"user-admin", "user-other"},
+}, {
+	about: "Client.SetAnnotation",
+	op:    opClientSetAnnotation,
+	allow: []string{"user-admin", "user-other"},
+}, {
 	about: "Client.ServiceAddUnits",
 	op:    opClientServiceAddUnits,
 	allow: []string{"user-admin", "user-other"},
@@ -282,6 +290,27 @@
 	return func() {}, nil
 }
 
+func opClientGetAnnotations(c *C, st *api.State) (func(), error) {
+	ann, err := st.Client().GetAnnotations("service-wordpress")
+	if err != nil {
+		return func() {}, err
+	}
+	c.Assert(err, IsNil)
+	c.Assert(ann, DeepEquals, make(map[string]string))
+	return func() {}, nil
+}
+
+func opClientSetAnnotation(c *C, st *api.State) (func(), error) {
+	err := st.Client().SetAnnotation("service-wordpress", "key", "value")
+	if err != nil {
+		return func() {}, err
+	}
+	c.Assert(err, IsNil)
+	return func() {
+		st.Client().SetAnnotation("service-wordpress", "key", "")
+	}, nil
+}
+
 func opClientServiceAddUnits(c *C, st *api.State) (func(), error) {
 	// This test only checks that the call is made without error, ensuring the
 	// signatures match.
@@ -353,7 +382,7 @@
 // environment manager (bootstrap machine), so is
 // hopefully easier to remember as such.
 func (s *suite) setUpScenario(c *C) (entities []string) {
-	add := func(e state.AuthEntity) {
+	add := func(e state.Entity) {
 		entities = append(entities, e.EntityName())
 	}
 	u, err := s.State.User("admin")
@@ -427,9 +456,10 @@
 	return
 }
 
-// AuthEntity is the same as state.AuthEntity but
-// without PasswordValid, which is implemented
-// by state entities but not by api entities.
+// AuthEntity is the same as state.Entity but
+// without PasswordValid and annotations handling
+// which are implemented by state entities but not
+// by api entities.
 type AuthEntity interface {
 	EntityName() string
 	SetPassword(pass string) error
@@ -579,6 +609,95 @@
 	c.Assert(info.ProviderType, Equals, conf.Type())
 }
 
+var clientAnnotationsTests = []struct {
+	about    string
+	initial  map[string]string
+	input    map[string]string
+	expected map[string]string
+	err      string
+}{
+	{
+		about:    "test setting an annotation",
+		input:    map[string]string{"mykey": "myvalue"},
+		expected: map[string]string{"mykey": "myvalue"},
+	},
+	{
+		about:    "test setting multiple annotations",
+		input:    map[string]string{"key1": "value1", "key2": "value2"},
+		expected: map[string]string{"key1": "value1", "key2": "value2"},
+	},
+	{
+		about:    "test overriding annotations",
+		initial:  map[string]string{"mykey": "myvalue"},
+		input:    map[string]string{"mykey": "another-value"},
+		expected: map[string]string{"mykey": "another-value"},
+	},
+	{
+		about: "test setting an invalid annotation",
+		input: map[string]string{"invalid.key": "myvalue"},
+		err:   `invalid key "invalid.key"`,
+	},
+}
+
+func (s *suite) TestClientAnnotations(c *C) {
+	// Set up entities.
+	service, err := s.State.AddService("dummy", s.AddTestingCharm(c, "dummy"))
+	c.Assert(err, IsNil)
+	unit, err := service.AddUnit()
+	c.Assert(err, IsNil)
+	machine, err := s.State.AddMachine("series", state.JobHostUnits)
+	c.Assert(err, IsNil)
+	entities := []state.Entity{service, unit, machine}
+	for i, t := range clientAnnotationsTests {
+	loop:
+		for _, entity := range entities {
+			id := entity.EntityName()
+			c.Logf("test %d. %s. entity %s", i, t.about, id)
+			// Set initial entity annotations.
+			for key, value := range t.initial {
+				err := entity.SetAnnotation(key, value)
+				c.Assert(err, IsNil)
+			}
+			// Add annotations using the API call.
+			for key, value := range t.input {
+				err := s.APIState.Client().SetAnnotation(id, key, value)
+				if t.err != "" {
+					c.Assert(err, ErrorMatches, t.err)
+					continue loop
+				}
+				c.Assert(err, IsNil)
+			}
+			// Check annotations are correctly set.
+			err := entity.Refresh()
+			c.Assert(err, IsNil)
+			c.Assert(entity.Annotations(), DeepEquals, t.expected)
+			// Retrieve annotations using the API call.
+			ann, err := s.APIState.Client().GetAnnotations(id)
+			c.Assert(err, IsNil)
+			// Check annotations are correctly returned.
+			err = entity.Refresh()
+			c.Assert(err, IsNil)
+			c.Assert(ann, DeepEquals, entity.Annotations())
+			// Clean up annotations on the current entity.
+			for key := range entity.Annotations() {
+				err = entity.SetAnnotation(key, "")
+				c.Assert(err, IsNil)
+			}
+		}
+	}
+}
+
+func (s *suite) TestClientAnnotationsBadEntity(c *C) {
+	bad := []string{"", "machine", "-foo", "foo-", "---", "machine-jim", "unit-123", "unit-foo", "service-", "service-foo/bar"}
+	expected := `invalid entity name ".*"`
+	for _, id := range bad {
+		err := s.APIState.Client().SetAnnotation(id, "mykey", "myvalue")
+		c.Assert(err, ErrorMatches, expected)
+		_, err = s.APIState.Client().GetAnnotations(id)
+		c.Assert(err, ErrorMatches, expected)
+	}
+}
+
 func (s *suite) TestMachineLogin(c *C) {
 	stm, err := s.State.AddMachine("series", state.JobHostUnits)
 	c.Assert(err, IsNil)

=== modified file 'state/apiserver/apiserver.go'
--- state/apiserver/apiserver.go	2013-03-08 20:36:42 +0000
+++ state/apiserver/apiserver.go	2013-03-08 21:34:19 +0000
@@ -346,6 +346,24 @@
 	return info, nil
 }
 
+// GetAnnotations returns annotations about a given entity.
+func (c *srvClient) GetAnnotations(args params.GetAnnotations) (params.GetAnnotationsResults, error) {
+	entity, err := c.root.srv.state.Entity(args.EntityId)
+	if err != nil {
+		return params.GetAnnotationsResults{}, err
+	}
+	return params.GetAnnotationsResults{Annotations: entity.Annotations()}, nil
+}
+
+// SetAnnotation stores an annotation about a given entity.
+func (c *srvClient) SetAnnotation(args params.SetAnnotation) error {
+	entity, err := c.root.srv.state.Entity(args.EntityId)
+	if err != nil {
+		return err
+	}
+	return entity.SetAnnotation(args.Key, args.Value)
+}
+
 // Login logs in with the provided credentials.
 // All subsequent requests on the connection will
 // act as the authenticated user.
@@ -370,7 +388,7 @@
 	}, nil
 }
 
-func setPassword(e state.AuthEntity, password string) error {
+func setPassword(e state.Entity, password string) error {
 	// Catch expected common case of mispelled
 	// or missing Password parameter.
 	if password == "" {
@@ -433,14 +451,14 @@
 // its methods concurrently.
 type authUser struct {
 	mu      sync.Mutex
-	_entity state.AuthEntity // logged-in entity (access only when mu is locked)
+	_entity state.Entity // logged-in entity (access only when mu is locked)
 }
 
 // login authenticates as entity with the given name,.
 func (u *authUser) login(st *state.State, entityName, password string) error {
 	u.mu.Lock()
 	defer u.mu.Unlock()
-	entity, err := st.AuthEntity(entityName)
+	entity, err := st.Entity(entityName)
 	if err != nil && !state.IsNotFound(err) {
 		return err
 	}
@@ -463,7 +481,7 @@
 // entity returns the currently logged-in entity, or nil if not
 // currently logged on.  The returned entity should not be modified
 // because it may be used concurrently.
-func (u *authUser) entity() state.AuthEntity {
+func (u *authUser) entity() state.Entity {
 	u.mu.Lock()
 	defer u.mu.Unlock()
 	return u._entity
@@ -471,7 +489,7 @@
 
 // isMachineWithJob returns whether the given entity is a machine that
 // is configured to run the given job.
-func isMachineWithJob(e state.AuthEntity, j state.MachineJob) bool {
+func isMachineWithJob(e state.Entity, j state.MachineJob) bool {
 	m, ok := e.(*state.Machine)
 	if !ok {
 		return false
@@ -485,7 +503,7 @@
 }
 
 // isAgent returns whether the given entity is an agent.
-func isAgent(e state.AuthEntity) bool {
+func isAgent(e state.Entity) bool {
 	_, isUser := e.(*state.User)
 	return !isUser
 }

=== modified file 'state/machine_test.go'
--- state/machine_test.go	2013-03-05 15:26:26 +0000
+++ state/machine_test.go	2013-03-08 21:34:19 +0000
@@ -111,7 +111,7 @@
 }
 
 func (s *MachineSuite) TestSetPassword(c *C) {
-	testSetPassword(c, func() (state.AuthEntity, error) {
+	testSetPassword(c, func() (state.Entity, error) {
 		return s.State.Machine(s.machine.Id())
 	})
 }

=== modified file 'state/service.go'
--- state/service.go	2013-03-08 17:56:51 +0000
+++ state/service.go	2013-03-08 21:34:19 +0000
@@ -52,6 +52,25 @@
 	return s.doc.Name
 }
 
+// EntityName returns a name identifying the service that is safe to use
+// as a file name.  The returned name will be different from other
+// EntityName values returned by any other entities from the same state.
+func (s *Service) EntityName() string {
+	return "service-" + s.Name()
+}
+
+// PasswordValid currently just returns false. Implemented here so that
+// a service can be used as an Entity.
+func (s *Service) PasswordValid(password string) bool {
+	return false
+}
+
+// SetPassword currently just returns an error. Implemented here so that
+// a service can be used as an Entity.
+func (s *Service) SetPassword(password string) error {
+	return fmt.Errorf("cannot set password of service")
+}
+
 // Annotations returns the service annotations.
 func (s *Service) Annotations() map[string]string {
 	return s.doc.Annotations

=== modified file 'state/service_test.go'
--- state/service_test.go	2013-03-08 19:55:44 +0000
+++ state/service_test.go	2013-03-08 21:34:19 +0000
@@ -64,6 +64,10 @@
 	}
 }
 
+func (s *ServiceSuite) TestEntityName(c *C) {
+	c.Assert(s.mysql.EntityName(), Equals, "service-mysql")
+}
+
 func (s *ServiceSuite) TestMysqlEndpoints(c *C) {
 	_, err := s.mysql.Endpoint("mysql")
 	c.Assert(err, ErrorMatches, `service "mysql" has no "mysql" relation`)

=== modified file 'state/state.go'
--- state/state.go	2013-03-08 19:27:06 +0000
+++ state/state.go	2013-03-08 21:34:19 +0000
@@ -274,17 +274,19 @@
 	return newMachine(st, mdoc), nil
 }
 
-// AuthEntity represents an entity that has
-// a password that can be authenticated against.
-type AuthEntity interface {
+// Entity represents an entity capabable of handling password authentication
+// and annotations.
+type Entity interface {
 	EntityName() string
 	SetPassword(pass string) error
 	PasswordValid(pass string) bool
 	Refresh() error
+	SetAnnotation(key, value string) error
+	Annotations() map[string]string
 }
 
-// AuthEntity returns the entity for the given name.
-func (st *State) AuthEntity(entityName string) (AuthEntity, error) {
+// Entity returns the entity for the given name.
+func (st *State) Entity(entityName string) (Entity, error) {
 	i := strings.Index(entityName, "-")
 	if i <= 0 || i >= len(entityName)-1 {
 		return nil, fmt.Errorf("invalid entity name %q", entityName)
@@ -308,6 +310,11 @@
 		return st.Unit(name)
 	case "user":
 		return st.User(id)
+	case "service":
+		if !IsServiceName(id) {
+			return nil, fmt.Errorf("invalid entity name %q", entityName)
+		}
+		return st.Service(id)
 	}
 	return nil, fmt.Errorf("invalid entity name %q", entityName)
 }

=== modified file 'state/state_test.go'
--- state/state_test.go	2013-03-07 16:05:22 +0000
+++ state/state_test.go	2013-03-08 21:34:19 +0000
@@ -1194,7 +1194,7 @@
 	c.Assert(err, ErrorMatches, "no reachable servers")
 }
 
-func testSetPassword(c *C, getEntity func() (state.AuthEntity, error)) {
+func testSetPassword(c *C, getEntity func() (state.Entity, error)) {
 	e, err := getEntity()
 	c.Assert(err, IsNil)
 
@@ -1317,30 +1317,30 @@
 	c.Assert(err, IsNil)
 }
 
-func (s *StateSuite) TestAuthEntity(c *C) {
-	bad := []string{"", "machine", "-foo", "foo-", "---", "machine-jim", "unit-123", "unit-foo"}
+func (s *StateSuite) TestEntity(c *C) {
+	bad := []string{"", "machine", "-foo", "foo-", "---", "machine-jim", "unit-123", "unit-foo", "service-", "service-foo/bar"}
 	for _, name := range bad {
-		e, err := s.State.AuthEntity(name)
+		e, err := s.State.Entity(name)
 		c.Check(e, IsNil)
 		c.Assert(err, ErrorMatches, `invalid entity name ".*"`)
 	}
 
-	e, err := s.State.AuthEntity("machine-1234")
+	e, err := s.State.Entity("machine-1234")
 	c.Check(e, IsNil)
 	c.Assert(err, ErrorMatches, `machine 1234 not found`)
 	c.Assert(state.IsNotFound(err), Equals, true)
 
-	e, err = s.State.AuthEntity("unit-foo-654")
+	e, err = s.State.Entity("unit-foo-654")
 	c.Check(e, IsNil)
 	c.Assert(err, ErrorMatches, `unit "foo/654" not found`)
 	c.Assert(state.IsNotFound(err), Equals, true)
 
-	e, err = s.State.AuthEntity("unit-foo-bar-654")
+	e, err = s.State.Entity("unit-foo-bar-654")
 	c.Check(e, IsNil)
 	c.Assert(err, ErrorMatches, `unit "foo-bar/654" not found`)
 	c.Assert(state.IsNotFound(err), Equals, true)
 
-	e, err = s.State.AuthEntity("user-arble")
+	e, err = s.State.Entity("user-arble")
 	c.Check(e, IsNil)
 	c.Assert(err, ErrorMatches, `user "arble" not found`)
 	c.Assert(state.IsNotFound(err), Equals, true)
@@ -1348,17 +1348,23 @@
 	m, err := s.State.AddMachine("series", state.JobHostUnits)
 	c.Assert(err, IsNil)
 
-	e, err = s.State.AuthEntity(m.EntityName())
+	e, err = s.State.Entity(m.EntityName())
 	c.Assert(err, IsNil)
 	c.Assert(e, FitsTypeOf, m)
 	c.Assert(e.EntityName(), Equals, m.EntityName())
 
 	svc, err := s.State.AddService("ser-vice1", s.AddTestingCharm(c, "dummy"))
 	c.Assert(err, IsNil)
+
+	service, err := s.State.Entity(svc.EntityName())
+	c.Assert(err, IsNil)
+	c.Assert(service, FitsTypeOf, svc)
+	c.Assert(service.EntityName(), Equals, svc.EntityName())
+
 	u, err := svc.AddUnit()
 	c.Assert(err, IsNil)
 
-	e, err = s.State.AuthEntity(u.EntityName())
+	e, err = s.State.Entity(u.EntityName())
 	c.Assert(err, IsNil)
 	c.Assert(e, FitsTypeOf, u)
 	c.Assert(e.EntityName(), Equals, u.EntityName())
@@ -1366,7 +1372,7 @@
 	user, err := s.State.AddUser("arble", "pass")
 	c.Assert(err, IsNil)
 
-	e, err = s.State.AuthEntity(user.EntityName())
+	e, err = s.State.Entity(user.EntityName())
 	c.Assert(err, IsNil)
 	c.Assert(e, FitsTypeOf, user)
 	c.Assert(e.EntityName(), Equals, user.EntityName())

=== modified file 'state/unit_test.go'
--- state/unit_test.go	2013-03-05 15:26:26 +0000
+++ state/unit_test.go	2013-03-08 21:34:19 +0000
@@ -174,7 +174,7 @@
 }
 
 func (s *UnitSuite) TestSetPassword(c *C) {
-	testSetPassword(c, func() (state.AuthEntity, error) {
+	testSetPassword(c, func() (state.Entity, error) {
 		return s.State.Unit(s.unit.Name())
 	})
 }

=== modified file 'state/user.go'
--- state/user.go	2013-02-18 17:03:00 +0000
+++ state/user.go	2013-03-08 21:34:19 +0000
@@ -79,6 +79,18 @@
 	return "user-" + u.doc.Name
 }
 
+// Annotations currently just returns an empty map. Implemented here so that
+// a user can be used as an Entity.
+func (u *User) Annotations() map[string]string {
+	return map[string]string{}
+}
+
+// SetAnnotation currently just returns an error. Implemented here so that
+// a user can be used as an Entity.
+func (u *User) SetAnnotation(key, value string) error {
+	return fmt.Errorf("cannot set annotation of user")
+}
+
 // SetPassword sets the password associated with the user.
 func (u *User) SetPassword(password string) error {
 	hp := trivial.PasswordHash(password)

=== modified file 'state/user_test.go'
--- state/user_test.go	2013-02-06 19:57:33 +0000
+++ state/user_test.go	2013-03-08 21:34:19 +0000
@@ -43,7 +43,7 @@
 	u, err := s.State.AddUser("someuser", "")
 	c.Assert(err, IsNil)
 
-	testSetPassword(c, func() (state.AuthEntity, error) {
+	testSetPassword(c, func() (state.Entity, error) {
 		return s.State.User(u.Name())
 	})
 }