Skip to content

Commit

Permalink
chore: pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ale8k committed Dec 13, 2024
1 parent 265c2b9 commit 0141ef3
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 18 deletions.
3 changes: 3 additions & 0 deletions internal/db/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ func (d *Database) ForEachModel(ctx context.Context, f func(m *dbmodel.Model) er

// GetModelsByUUID retrieves a list of models where the model UUIDs are in
// the provided modelUUIDs slice.
//
// If the UUID cannot be resolved to a model, it is skipped from the result and
// no error is returned.
func (d *Database) GetModelsByUUID(ctx context.Context, modelUUIDs []string) (_ []dbmodel.Model, err error) {
const op = errors.Op("db.GetModelsByUUID")

Expand Down
1 change: 1 addition & 0 deletions internal/db/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ func (s *dbSuite) TestGetModelsByUUID(c *qt.C) {
sort.Slice(models, func(i, j int) bool {
return models[i].UUID.String < models[j].UUID.String
})
c.Assert(models, qt.HasLen, 3)
c.Check(models[0].UUID.String, qt.Equals, "00000002-0000-0000-0000-000000000001")
c.Check(models[0].Controller.Name, qt.Not(qt.Equals), "")
c.Check(models[1].UUID.String, qt.Equals, "00000002-0000-0000-0000-000000000002")
Expand Down
6 changes: 2 additions & 4 deletions internal/jimm/jimm.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,10 +495,8 @@ type API interface {
// ListStorageDetails lists all storage.
ListStorageDetails(ctx context.Context) ([]jujuparams.StorageDetails, error)

// ListModels returns the models that the specified user
// has access to in the current server. Only that controller owner
// can list models for any user (at this stage). Other users
// can only ask about their own models.
// ListModels returns UserModel's for the user that is logged in. If the user logged
// in is "admin" they may specify another user's models.
//
// In our wrapper, we ask as the controller admin. So expect ALL models from
// the controller.
Expand Down
15 changes: 12 additions & 3 deletions internal/jimm/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -1402,9 +1402,18 @@ func (j *JIMM) ListModels(ctx context.Context, user *openfga.User) ([]base.UserM
// NOTE: We skip controller models as ListModels is used for login and register.
// The models returned are stored locally and used for reference. In the case of JIMM,
// we do not want to show the controller models.
for _, m := range ums {
if slices.Contains(uuids, m.UUID) {
userModels = append(userModels, m)
for _, um := range ums {
// Filter models that match authorised uuids list
if slices.Contains(uuids, um.UUID) {
// Find that model in the db models
index := slices.IndexFunc(models, func(m dbmodel.Model) bool {
return m.UUID.String == um.UUID
})
if index != -1 {
// Override owner and append to result
um.Owner = models[index].OwnerIdentityName
userModels = append(userModels, um)
}
}
}
return nil
Expand Down
14 changes: 7 additions & 7 deletions internal/jimm/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3901,9 +3901,9 @@ var modelListTests = []struct {
env: listModelsTestEnv,
username: "[email protected]",
expectedUserModels: []base.UserModel{
{UUID: "00000002-0000-0000-0000-000000000001"},
{UUID: "00000002-0000-0000-0000-000000000002"},
{UUID: "00000002-0000-0000-0000-000000000003"},
{UUID: "00000002-0000-0000-0000-000000000001", Owner: "[email protected]"},
{UUID: "00000002-0000-0000-0000-000000000002", Owner: "[email protected]"},
{UUID: "00000002-0000-0000-0000-000000000003", Owner: "[email protected]"},
},
listModelsMockByControllerName: map[string]func(context.Context) ([]base.UserModel, error){
"controller-1": func(ctx context.Context) ([]base.UserModel, error) {
Expand All @@ -3924,10 +3924,10 @@ var modelListTests = []struct {
env: listModelsTestEnv,
username: "[email protected]",
expectedUserModels: []base.UserModel{
{UUID: "00000002-0000-0000-0000-000000000001"},
{UUID: "00000002-0000-0000-0000-000000000002"},
{UUID: "00000002-0000-0000-0000-000000000003"},
{UUID: "00000002-0000-0000-0000-000000000004"},
{UUID: "00000002-0000-0000-0000-000000000001", Owner: "[email protected]"},
{UUID: "00000002-0000-0000-0000-000000000002", Owner: "[email protected]"},
{UUID: "00000002-0000-0000-0000-000000000003", Owner: "[email protected]"},
{UUID: "00000002-0000-0000-0000-000000000004", Owner: "[email protected]"},
},
listModelsMockByControllerName: map[string]func(context.Context) ([]base.UserModel, error){
"controller-1": func(ctx context.Context) ([]base.UserModel, error) {
Expand Down
6 changes: 2 additions & 4 deletions internal/jujuclient/modelmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,8 @@ func (c Connection) ChangeModelCredential(ctx context.Context, model names.Model
return out.OneError()
}

// ListModels returns the models that the specified user
// has access to in the current server. Only that controller owner
// can list models for any user (at this stage). Other users
// can only ask about their own models.
// ListModels returns UserModel's for the user that is logged in. If the user logged
// in is "admin" they may specify another user's models.
//
// In our wrapper, we ask as the controller admin. So expect ALL models from
// the controller.
Expand Down

0 comments on commit 0141ef3

Please sign in to comment.