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

Revert "backports changes back to feature-rebac (#1003)" #1030

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions internal/db/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,28 @@ func (d *Database) ForEachModel(ctx context.Context, f func(m *dbmodel.Model) er
return nil
}

// GetModelsByUUID retrieves a list of models where the model UUIDs are in
// the provided modelUUIDs slice.
func (d *Database) GetModelsByUUID(ctx context.Context, modelUUIDs []string) ([]dbmodel.Model, error) {
const op = errors.Op("db.GetModelsByUUID")

if err := d.ready(); err != nil {
return nil, errors.E(op, err)
}
var models []dbmodel.Model
db := d.DB.WithContext(ctx)
db = preloadModel("", db)
err := db.Where("uuid IN ?", modelUUIDs).Find(&models).Error
if err != nil {
err = dbError(err)
if errors.ErrorCode(err) == errors.CodeNotFound {
return nil, errors.E(op, err, "model not found")
}
return nil, errors.E(op, dbError(err))
}
return models, nil
}

func preloadModel(prefix string, db *gorm.DB) *gorm.DB {
if len(prefix) > 0 && prefix[len(prefix)-1] != '.' {
prefix += "."
Expand Down
86 changes: 86 additions & 0 deletions internal/db/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package db_test
import (
"context"
"database/sql"
"sort"
"testing"

qt "github.com/frankban/quicktest"
Expand Down Expand Up @@ -718,3 +719,88 @@ func (s *dbSuite) TestForEachModel(c *qt.C) {
"00000002-0000-0000-0000-000000000003",
})
}

const testGetModelsByUUIDEnv = `clouds:
- name: test
type: test
regions:
- name: test-region
cloud-credentials:
- name: test-cred
cloud: test
owner: alice@external
type: empty
controllers:
- name: test
uuid: 00000001-0000-0000-0000-000000000001
cloud: test
region: test-region
models:
- name: test-1
uuid: 00000002-0000-0000-0000-000000000001
owner: alice@external
cloud: test
region: test-region
cloud-credential: test-cred
controller: test
users:
- user: alice@external
access: admin
- user: bob@external
access: write
- name: test-2
uuid: 00000002-0000-0000-0000-000000000002
owner: bob@external
cloud: test
region: test-region
cloud-credential: test-cred
controller: test
users:
- user: bob@external
access: admin
- name: test-3
uuid: 00000002-0000-0000-0000-000000000003
owner: bob@external
cloud: test
region: test-region
cloud-credential: test-cred
controller: test
users:
- user: bob@external
access: admin
`

func TestGetModelsByUUIDlUnconfiguredDatabase(t *testing.T) {
c := qt.New(t)

var d db.Database
_, err := d.GetModelsByUUID(context.Background(), nil)
c.Check(err, qt.ErrorMatches, `database not configured`)
c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeServerConfiguration)
}

func (s *dbSuite) TestGetModelsByUUID(c *qt.C) {
ctx := context.Background()
err := s.Database.Migrate(context.Background(), true)
c.Assert(err, qt.Equals, nil)

env := jimmtest.ParseEnvironment(c, testGetModelsByUUIDEnv)
env.PopulateDB(c, *s.Database, nil)

modelUUIDs := []string{
"00000002-0000-0000-0000-000000000001",
"00000002-0000-0000-0000-000000000002",
"00000002-0000-0000-0000-000000000003",
}
models, err := s.Database.GetModelsByUUID(ctx, modelUUIDs)
c.Assert(err, qt.IsNil)
sort.Slice(models, func(i, j int) bool {
kian99 marked this conversation as resolved.
Show resolved Hide resolved
return models[i].UUID.String < models[j].UUID.String
})
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")
c.Check(models[1].Controller.Name, qt.Not(qt.Equals), "")
c.Check(models[2].UUID.String, qt.Equals, "00000002-0000-0000-0000-000000000003")
c.Check(models[2].Controller.Name, qt.Not(qt.Equals), "")
}
15 changes: 10 additions & 5 deletions internal/jujuapi/jimm.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,14 +497,19 @@ func (r *controllerRoot) RemoveCloudFromController(ctx context.Context, req apip
func (r *controllerRoot) CrossModelQuery(ctx context.Context, req apiparams.CrossModelQueryRequest) (apiparams.CrossModelQueryResponse, error) {
const op = errors.Op("jujuapi.CrossModelQuery")

usersModels, err := r.jimm.Database.GetUserModels(ctx, r.user.User)
modelUUIDs, err := r.user.ListModels(ctx)
if err != nil {
return apiparams.CrossModelQueryResponse{}, errors.E(op, errors.Code("failed to get models for user"))
return apiparams.CrossModelQueryResponse{}, errors.E(op, errors.Code("failed to list user's model access"))
}
models := make([]dbmodel.Model, len(usersModels))
for i, m := range usersModels {
models[i] = m.Model_
// UUIDs are returned in the form "model:uuid"
for i, uuid := range modelUUIDs {
modelUUIDs[i] = strings.Split(uuid, ":")[1]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strings.Split bit assumes we will never change the format.. which might be ok.. also assumes there will be a : in the string..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we need a function that will split the tag type from the value in jaas tags...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can add

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to do this in a follow up PR, I believe we should be returning ofga entities from ofga related queries, not strings. Then we won't have this parsing problem.

}
models, err := r.jimm.Database.GetModelsByUUID(ctx, modelUUIDs)
if err != nil {
return apiparams.CrossModelQueryResponse{}, errors.E(op, errors.Code("failed to get models for user"))
}

switch strings.TrimSpace(strings.ToLower(req.Type)) {
case "jq":
return r.jimm.QueryModelsJq(ctx, models, req.Query)
Expand Down