New EnvUser #553
New EnvUser #553
Conversation
| if err != nil { | ||
| return nil, errors.Annotate(err, "failed to create admin user") | ||
| } | ||
| return st.AddUser(admin.Id(), "", password, "") |
There was a problem hiding this comment.
This should be admin.Name() not Id()
|
Unless I'm missing something this pr is supposed to the envuser.go and envuser_test.go files in it? |
| if params.DisplayName == "" { | ||
| params.DisplayName = factory.UniqueString("display name") | ||
| } | ||
| if params.CreatedBy == "" { |
There was a problem hiding this comment.
Is there a chance of running into problems with createdBy not being a valid user id? Maybe this should be "admin"? Or, even better, make User and CreatedBy in EnvUserParams be *state.User - that way valid users can be inserted, or created on the fly.
There was a problem hiding this comment.
I think that if the CreatedBy isn't specified, we should create a new User, and then use them as the creator.
In the same way that we should create a User for the params.User in order for the name to be used in the environment user.
| func (st *State) AddEnvironmentUser(user, createdBy names.UserTag, displayName, alias string) (*EnvironmentUser, error) { | ||
| envuuid := st.EnvironTag().Id() | ||
| username := user.Name() | ||
| creatorname := createdBy.Name() |
There was a problem hiding this comment.
For both of these, we'd want the fully qualified name: "user@provider"
We know we'll need it later, and it makes sense to get it right from the start.
|
We also need an upgrade step that creates the env user for the admin user (and any other users there may be). |
|
Comments addressed, tests passing. Putting up for review while I add the upgrade step. |
|
PTAL |
| } | ||
|
|
||
| // EnvUUID returns the environment UUIID of the environment user. | ||
| func (e *EnvironmentUser) EnvUUID() string { |
There was a problem hiding this comment.
I've been thinking of this since I have done the changes for @davecheney in my branch. I think we should be returning a names.EnvironTag here not a bare string for UUID. The UUID identifies and environment, and the tag can be used to get the environment.
Please change to:
// EnvironmentTag identifies the environment for the environment user.
func (e *EnvironmentUser) EnvironmentTag() names.EnvironTag {
return names.NewEnvironTag(e.doc.EnvUUID)
}
|
PTAL |
| Assert: txn.DocMissing, | ||
| Insert: &envUser.doc, | ||
| }} | ||
| err = st.ResumeTransactions() |
There was a problem hiding this comment.
I'm pretty sure you don't need the ResumeTransactions here...
|
PTAL |
|
Let's Get This Merged! There are minors, but we can address those in the follow up with state environ tag. |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
New EnvUser Add: - the EnvUser doc - support for setting the last login time for the EnvUser - factory methods for adding the envuser Create the Admin EnvUser at bootstrap time.
This is a follow up branch to juju#553 (New EnvUser). Get the environ UUID from the state environ tag.
This is a follow up branch to juju#553 (New EnvUser). Get the environ UUID from the state environ tag.
This is a follow up branch to juju#553 (New EnvUser). Get the environ UUID from the state environ tag.
This is a follow up branch to juju#553 (New EnvUser). Get the environ UUID from the state environ tag.
This is a follow up branch to juju#553 (New EnvUser). Get the environ UUID from the state environ tag.
This is a follow up branch to juju#553 (New EnvUser). Get the environ UUID from the state environ tag.
This is a follow up branch to juju#553 (New EnvUser). Get the environ UUID from the state environ tag.
Use environ tag This is a follow up branch to #553 (New EnvUser). Get the environ UUID from the state environ tag.
Add:
Create the Admin EnvUser at bootstrap time.