Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable importing models created by local users #1037

Merged
121 changes: 96 additions & 25 deletions internal/jimm/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@ func (j *JIMM) ImportModel(ctx context.Context, u *dbmodel.User, controllerName
if err != nil {
return errors.E(op, err)
}

model := dbmodel.Model{}
// fill in data from model info
err = model.FromJujuModelInfo(modelInfo)
Expand All @@ -471,31 +470,79 @@ func (j *JIMM) ImportModel(ctx context.Context, u *dbmodel.User, controllerName
model.ControllerID = controller.ID
model.Controller = controller

// fetch the model owner user
ownerTag, err := names.ParseUserTag(modelInfo.OwnerTag)
if err != nil {
return errors.E(op, err)
}
owner := dbmodel.User{}
owner.SetTag(ownerTag)
err = j.Database.GetUser(ctx, &owner)
if err != nil {
return errors.E(op, err)
userHasModelAccess := false
for _, user := range model.Users {
if user.User.Username == u.Username {
userHasModelAccess = true
break
}
}
model.OwnerUsername = owner.Username
model.Owner = owner
if !userHasModelAccess {
zapctx.Debug(ctx, "User doesn't have model access, adding it")
// Ensure the current user gets access to the model
// This will be applied to JIMM's access table lower down.
model.Users = append(model.Users, dbmodel.UserModelAccess{User: *u, Access: string(jujuparams.ModelAdminAccess)})
}

var cloudCredential dbmodel.CloudCredential
originalOwnerIsLocalUser := !strings.Contains(modelInfo.OwnerTag, "@")
kian99 marked this conversation as resolved.
Show resolved Hide resolved
if originalOwnerIsLocalUser {
// Switch the model to be owned by the user making the request.
model.OwnerUsername = u.Username
model.Owner = *u
for _, user := range model.Users {
if user.User.Username == u.Username {
userHasModelAccess = true
break
}
}

// fetch cloud credential used by the model
credentialTag, err := names.ParseCloudCredentialTag(modelInfo.CloudCredentialTag)
if err != nil {
return errors.E(op, err)
}
cloudCredential := dbmodel.CloudCredential{}
cloudCredential.SetTag(credentialTag)
err = j.Database.GetCloudCredential(ctx, &cloudCredential)
if err != nil {
return errors.E(op, err)
cloudTag, err := names.ParseCloudTag(modelInfo.CloudTag)
if err != nil {
return err
}
// Note that the model already has a cloud credential configured which it will use when deploying new
// applications. JIMM needs some cloud credential reference to be able to import the model so use any
// credential against the cloud the model is deployed against. Even using the correct cloud for the
// credential is not strictly necessary, but will help prevent the user think they can create new
// models on the incoming cloud.
allCredentials, err := j.Database.GetUserCloudCredentials(ctx, u, cloudTag.Id())
if err != nil {
return err
}
if len(allCredentials) == 0 {
return errors.E(op, errors.CodeNotFound, fmt.Sprintf("Failed to find cloud credential for user %s on cloud %s", u.Username, cloudTag.Id()))
}
cloudCredential = allCredentials[0]
} else {
// fetch the model owner user
ownerTag, err := names.ParseUserTag(modelInfo.OwnerTag)
if err != nil {
return errors.E(op, err)
}
owner := dbmodel.User{}
owner.SetTag(ownerTag)
err = j.Database.GetUser(ctx, &owner)
if err != nil {
return errors.E(op, err)
}
model.OwnerUsername = owner.Username
model.Owner = owner

// fetch cloud credential used by the model
credentialTag, err := names.ParseCloudCredentialTag(modelInfo.CloudCredentialTag)
if err != nil {
return errors.E(op, err)
}
cred := dbmodel.CloudCredential{}
cred.SetTag(credentialTag)
err = j.Database.GetCloudCredential(ctx, &cred)
if err != nil {
return errors.E(op, err)
}
cloudCredential = cred
}

model.CloudCredentialID = cloudCredential.ID
model.CloudCredential = cloudCredential

Expand All @@ -505,6 +552,7 @@ func (j *JIMM) ImportModel(ctx context.Context, u *dbmodel.User, controllerName
}
err = j.Database.GetCloud(ctx, &cloud)
if err != nil {
zapctx.Error(ctx, "failed to get cloud", zap.Any("cloud credential", cloudCredential))
kian99 marked this conversation as resolved.
Show resolved Hide resolved
return errors.E(op, err)
}

Expand All @@ -521,10 +569,20 @@ func (j *JIMM) ImportModel(ctx context.Context, u *dbmodel.User, controllerName
return errors.E(op, "cloud region not found")
}

var usersExcludingLocalUsers []dbmodel.UserModelAccess
for _, userAccess := range model.Users {
if !strings.Contains(userAccess.User.Username, "@") {
kian99 marked this conversation as resolved.
Show resolved Hide resolved
// If the username doesn't contain an "@" the user is local
// to the controller and we don't want to propagate it.
continue
}
usersExcludingLocalUsers = append(usersExcludingLocalUsers, userAccess)
}
model.Users = usersExcludingLocalUsers

for i, userAccess := range model.Users {
u := userAccess.User
err = j.Database.GetUser(ctx, &u)
if err != nil {
if err = j.Database.GetUser(ctx, &u); err != nil {
return errors.E(op, err)
}
model.Users[i].Username = u.Username
Expand All @@ -539,6 +597,19 @@ func (j *JIMM) ImportModel(ctx context.Context, u *dbmodel.User, controllerName
return errors.E(op, err)
}

if !userHasModelAccess {
// Here we finally grant the user doing the import, access to the underlying model.
err = j.doModelAdmin(ctx, u, modelTag, func(m *dbmodel.Model, api API) error {
if err := api.GrantModelAccess(ctx, modelTag, u.Tag().(names.UserTag), jujuparams.ModelAdminAccess); err != nil {
return err
}
return nil
})
if err != nil {
return errors.E(op, err, "Failed to grant user %s admin access on the model", u.Username)
}
}

modelAPI, err := j.dial(ctx, &controller, modelTag)
if err != nil {
return errors.E(op, err)
Expand Down
95 changes: 95 additions & 0 deletions internal/jimm/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,101 @@ func TestImportModel(t *testing.T) {
Access: "read",
}},
},
}, {
about: "model from local user imported",
user: "alice@external",
controllerName: "test-controller",
modelUUID: "00000002-0000-0000-0000-000000000001",
modelInfo: func(_ context.Context, info *jujuparams.ModelInfo) error {
info.Name = "test-model"
info.Type = "test-type"
info.UUID = "00000002-0000-0000-0000-000000000001"
info.ControllerUUID = "00000001-0000-0000-0000-000000000001"
info.DefaultSeries = "test-series"
info.CloudTag = names.NewCloudTag("test-cloud").String()
info.CloudRegion = "test-region"
info.CloudCredentialTag = names.NewCloudCredentialTag("test-cloud/local-user/test-credential").String()
info.CloudCredentialValidity = &trueValue
info.OwnerTag = names.NewUserTag("local-user").String()
info.Life = life.Alive
info.Status = jujuparams.EntityStatus{
Status: status.Status("available"),
Info: "test-info",
Since: &now,
}
info.Users = []jujuparams.ModelUserInfo{{
UserName: "local-user",
Access: jujuparams.ModelAdminAccess,
}, {
UserName: "another-user",
Access: jujuparams.ModelReadAccess,
}}
info.Machines = []jujuparams.ModelMachineInfo{{
Id: "test-machine",
DisplayName: "Test machine",
Status: "test-status",
Message: "test-message",
}}
info.SLA = &jujuparams.ModelSLAInfo{
Level: "essential",
Owner: "local-user",
}
info.AgentVersion = newVersion("2.1.0")
return nil
},
expectedModel: dbmodel.Model{
Name: "test-model",
UUID: sql.NullString{
String: "00000002-0000-0000-0000-000000000001",
Valid: true,
},
Owner: dbmodel.User{
Username: "alice@external", // Owner will switch to the user doing the import
kian99 marked this conversation as resolved.
Show resolved Hide resolved
DisplayName: "Alice",
ControllerAccess: "superuser",
},
Controller: dbmodel.Controller{
Name: "test-controller",
UUID: "00000001-0000-0000-0000-000000000001",
CloudName: "test-cloud",
CloudRegion: "test-region-1",
AgentVersion: "3.2.1",
},
CloudRegion: dbmodel.CloudRegion{
Cloud: dbmodel.Cloud{
Name: "test-cloud",
Type: "test",
},
Name: "test-region",
},
CloudCredential: dbmodel.CloudCredential{
Name: "test-credential",
},
Type: "test-type",
DefaultSeries: "test-series",
Life: "alive",
Status: dbmodel.Status{
Status: "available",
Info: "test-info",
Since: sql.NullTime{
Valid: true,
Time: now,
},
Version: "2.1.0",
},
SLA: dbmodel.SLA{
Level: "essential",
Owner: "local-user",
},
Users: []dbmodel.UserModelAccess{{
User: dbmodel.User{
Username: "alice@external",
DisplayName: "Alice",
ControllerAccess: "superuser",
},
Access: "admin",
}},
},
}, {
about: "model not found",
user: "alice@external",
Expand Down
7 changes: 7 additions & 0 deletions local/candid/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,12 @@ identity-providers:
groups:
- group1
- group2
joe:
name: Joe user (non superuser)
email: [email protected]
password: joe
groups:
- group1
- group2
hidden: false
require-mfa: false
kian99 marked this conversation as resolved.
Show resolved Hide resolved