state/api: ApiInfo.Tag now takes a Tag

zhatin · Jul 10, 2014 · b20a3d1 · b20a3d1
1 parent 4adeb75
commit b20a3d1
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 31 deletions.
diff --git a/agent/agent.go b/agent/agent.go
@@ -640,7 +640,7 @@ func (c *configInternal) APIInfo() *api.Info {
 		Addrs:    addrs,
 		Password: c.apiDetails.password,
 		CACert:   c.caCert,
-		Tag:      c.tag.String(),
+		Tag:      c.tag,
 		Nonce:    c.nonce,
 	}
 }

diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -488,10 +488,13 @@ func (*suite) TestSetPassword(c *gc.C) {
 	conf, err := agent.NewStateMachineConfig(attrParams, servingInfo)
 	c.Assert(err, gc.IsNil)
 
+	tag, err := names.ParseTag(attrParams.Tag)
+	c.Assert(err, gc.IsNil)
+
 	expectAPIInfo := &api.Info{
 		Addrs:    attrParams.APIAddresses,
 		CACert:   attrParams.CACert,
-		Tag:      attrParams.Tag,
+		Tag:      tag,
 		Password: "",
 		Nonce:    attrParams.Nonce,
 	}

diff --git a/environmentserver/authentication/authentication.go b/environmentserver/authentication/authentication.go
@@ -94,7 +94,7 @@ func (auth *simpleAuth) SetupAuthentication(machine TaggedPasswordChanger) (*Con
 	stateInfo.Tag = machine.Tag()
 	stateInfo.Password = password
 	apiInfo := *auth.apiInfo
-	apiInfo.Tag = machine.Tag().String()
+	apiInfo.Tag = machine.Tag()
 	apiInfo.Password = password
 	return &stateInfo, &apiInfo, nil
 }
diff --git a/environs/cloudinit/cloudinit.go b/environs/cloudinit/cloudinit.go
@@ -594,8 +594,8 @@ func verifyConfig(cfg *MachineConfig) (err error) {
 		if cfg.StateInfo.Tag != nil {
 			return fmt.Errorf("entity tag must be nil when starting a state server")
 		}
-		if cfg.APIInfo.Tag != "" {
-			return fmt.Errorf("entity tag must be blank when starting a state server")
+		if cfg.APIInfo.Tag != nil {
+			return fmt.Errorf("entity tag must be nil when starting a state server")
 		}
 		if cfg.StateServingInfo == nil {
 			return fmt.Errorf("missing state serving info")
@@ -628,7 +628,7 @@ func verifyConfig(cfg *MachineConfig) (err error) {
 		if len(cfg.APIInfo.Addrs) == 0 {
 			return fmt.Errorf("missing API hosts")
 		}
-		if cfg.APIInfo.Tag != names.NewMachineTag(cfg.MachineId).String() {
+		if cfg.APIInfo.Tag != names.NewMachineTag(cfg.MachineId) {
 			return fmt.Errorf("entity tag must match started machine")
 		}
 		if cfg.StateServingInfo != nil {

diff --git a/juju/api.go b/juju/api.go
@@ -257,7 +257,7 @@ func apiInfoConnect(store configstore.Storage, info configstore.EnvironInfo, api
 	apiInfo := &api.Info{
 		Addrs:      endpoint.Addresses,
 		CACert:     endpoint.CACert,
-		Tag:        names.NewUserTag(info.APICredentials().User).String(),
+		Tag:        names.NewUserTag(info.APICredentials().User),
 		Password:   info.APICredentials().Password,
 		EnvironTag: environTag,
 	}
@@ -319,7 +319,7 @@ func environAPIInfo(environ environs.Environ) (*api.Info, error) {
 	if err != nil {
 		return nil, err
 	}
-	info.Tag = "user-admin"
+	info.Tag = names.NewUserTag("admin")
 	password := environ.Config().AdminSecret()
 	if password == "" {
 		return nil, fmt.Errorf("cannot connect without admin-secret")
@@ -335,22 +335,21 @@ func cacheAPIInfo(info configstore.EnvironInfo, apiInfo *api.Info) (err error) {
 	defer errors.Contextf(&err, "failed to cache API credentials")
 	var environUUID string
 	if apiInfo.EnvironTag != nil {
-		tag, err := names.ParseEnvironTag(apiInfo.Tag)
-		if err != nil {
-			return err
-		}
-		environUUID = tag.Id()
+		environUUID = apiInfo.Tag.Id()
 	}
 	info.SetAPIEndpoint(configstore.APIEndpoint{
 		Addresses:   apiInfo.Addrs,
 		CACert:      string(apiInfo.CACert),
 		EnvironUUID: environUUID,
 	})
-	tag, err := names.ParseUserTag(apiInfo.Tag)
-	if err != nil {
-		return err
+	tag, ok := apiInfo.Tag.(names.UserTag)
+	if !ok {
+		return errors.Errorf("apiInfo.Tag was of type %T, expecting names.UserTag", apiInfo.Tag)
 	}
 	info.SetAPICredentials(configstore.APICredentials{
+		// This looks questionable. We have a tag, say "user-admin", but then only
+		// the Id portion of the tag is recorded, "admin", so this is really a
+		// username, not a tag, and cannot be reconstructed accurately.
 		User:     tag.Id(),
 		Password: apiInfo.Password,
 	})

diff --git a/juju/testing/conn.go b/juju/testing/conn.go
@@ -116,14 +116,14 @@ func (s *JujuConnSuite) StateInfo(c *gc.C) *authentication.ConnectionInfo {
 func (s *JujuConnSuite) APIInfo(c *gc.C) *api.Info {
 	_, apiInfo, err := s.APIConn.Environ.StateInfo()
 	c.Assert(err, gc.IsNil)
-	apiInfo.Tag = "user-admin"
+	apiInfo.Tag = names.NewUserTag("admin")
 	apiInfo.Password = "dummy-secret"
 	return apiInfo
 }
 
 // openAPIAs opens the API and ensures that the *api.State returned will be
 // closed during the test teardown by using a cleanup function.
-func (s *JujuConnSuite) openAPIAs(c *gc.C, tag, password, nonce string) *api.State {
+func (s *JujuConnSuite) openAPIAs(c *gc.C, tag names.Tag, password, nonce string) *api.State {
 	_, info, err := s.APIConn.Environ.StateInfo()
 	c.Assert(err, gc.IsNil)
 	info.Tag = tag
@@ -139,14 +139,14 @@ func (s *JujuConnSuite) openAPIAs(c *gc.C, tag, password, nonce string) *api.Sta
 // OpenAPIAs opens the API using the given identity tag and password for
 // authentication.  The returned *api.State should not be closed by the caller
 // as a cleanup function has been registered to do that.
-func (s *JujuConnSuite) OpenAPIAs(c *gc.C, tag, password string) *api.State {
+func (s *JujuConnSuite) OpenAPIAs(c *gc.C, tag names.Tag, password string) *api.State {
 	return s.openAPIAs(c, tag, password, "")
 }
 
 // OpenAPIAsMachine opens the API using the given machine tag, password and
 // nonce for authentication. The returned *api.State should not be closed by
 // the caller as a cleanup function has been registered to do that.
-func (s *JujuConnSuite) OpenAPIAsMachine(c *gc.C, tag, password, nonce string) *api.State {
+func (s *JujuConnSuite) OpenAPIAsMachine(c *gc.C, tag names.Tag, password, nonce string) *api.State {
 	return s.openAPIAs(c, tag, password, nonce)
 }
 
@@ -166,7 +166,7 @@ func (s *JujuConnSuite) OpenAPIAsNewMachine(c *gc.C, jobs ...state.MachineJob) (
 	c.Assert(err, gc.IsNil)
 	err = machine.SetProvisioned("foo", "fake_nonce", nil)
 	c.Assert(err, gc.IsNil)
-	return s.openAPIAs(c, machine.Tag().String(), password, "fake_nonce"), machine
+	return s.openAPIAs(c, machine.Tag(), password, "fake_nonce"), machine
 }
 
 func PreferredDefaultVersions(conf *config.Config, template version.Binary) []version.Binary {

diff --git a/juju/testing/instance.go b/juju/testing/instance.go
@@ -41,7 +41,7 @@ func FakeStateInfo(machineId string) *authentication.ConnectionInfo {
 func FakeAPIInfo(machineId string) *api.Info {
 	return &api.Info{
 		Addrs:    []string{"0.1.2.3:1234"},
-		Tag:      names.NewMachineTag(machineId).String(),
+		Tag:      names.NewMachineTag(machineId),
 		Password: "unimportant",
 		CACert:   testing.CACert,
 	}

diff --git a/provider/dummy/environs.go b/provider/dummy/environs.go
@@ -781,8 +781,7 @@ func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Ins
 	if args.MachineConfig.StateInfo.Tag != names.NewMachineTag(machineId) {
 		return nil, nil, nil, fmt.Errorf("entity tag must match started machine")
 	}
-	// TODO(dfc) APIInfo.Tag should be a Tag
-	if args.MachineConfig.APIInfo.Tag != names.NewMachineTag(machineId).String() {
+	if args.MachineConfig.APIInfo.Tag != names.NewMachineTag(machineId) {
 		return nil, nil, nil, fmt.Errorf("entity tag must match started machine")
 	}
 	logger.Infof("would pick tools from %s", args.Tools)

diff --git a/state/api/apiclient.go b/state/api/apiclient.go
@@ -82,10 +82,10 @@ type Info struct {
 	CACert string
 
 	// Tag holds the name of the entity that is connecting.
-	// If this and the password are empty, no login attempt will be made
+	// If this is nil, and the password are empty, no login attempt will be made.
 	// (this is to allow tests to access the API to check that operations
 	// fail when not logged in).
-	Tag string
+	Tag names.Tag
 
 	// Password holds the password for the administrator or connecting entity.
 	Password string
@@ -181,12 +181,14 @@ func Open(info *Info, opts DialOpts) (*State, error) {
 		conn:       conn,
 		addr:       conn.Config().Location.Host,
 		serverRoot: "https://" + conn.Config().Location.Host,
-		tag:        info.Tag,
-		password:   info.Password,
-		certPool:   pool,
+		// why are the contents of the tag (username and password) written into the
+		// state structure BEFORE login ?!?
+		tag:      toString(info.Tag),
+		password: info.Password,
+		certPool: pool,
 	}
-	if info.Tag != "" || info.Password != "" {
-		if err := st.Login(info.Tag, info.Password, info.Nonce); err != nil {
+	if info.Tag != nil || info.Password != "" {
+		if err := st.Login(info.Tag.String(), info.Password, info.Nonce); err != nil {
 			conn.Close()
 			return nil, err
 		}
@@ -197,6 +199,14 @@ func Open(info *Info, opts DialOpts) (*State, error) {
 	return st, nil
 }
 
+// toString returns the value of a tag's String method, or "" if the tag is nil.
+func toString(tag names.Tag) string {
+	if tag == nil {
+		return ""
+	}
+	return tag.String()
+}
+
 func dialWebsocket(addr, environUUID string, opts DialOpts, rootCAs *x509.CertPool, try *parallel.Try) error {
 	cfg, err := setUpWebsocket(addr, environUUID, rootCAs)
 	if err != nil {