Skip to content

Commit

Permalink
return model summaries with new model owner
Browse files Browse the repository at this point in the history 
- refactor ModelInfo slightly to use a separate function where we merge the juju model info with JIMM specific data
- refactor the test for ModelInfo to reduce duplication
- tweak the test dialer to have less surprising behavior
  • Loading branch information
@kian99
kian99 committed Sep 16, 2024
1 parent 5101bf8 commit d0a5be3
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 137 deletions.
34 changes: 23 additions & 11 deletions internal/jimm/model.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,13 +701,7 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model
return nil, errors.E(op, err)
}

modelAccess, err := j.GetUserModelAccess(ctx, user, mt)
if err != nil {
return nil, errors.E(op, err)
}
if modelAccess == "" {
// If the user doesn't have any access on the model return an
// unauthorized error
if ok, err := user.IsModelReader(ctx, mt); !ok || err != nil {
return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}

Expand All @@ -724,6 +718,19 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model
return nil, errors.E(op, err)
}

return j.mergeModelInfo(ctx, user, mi, m)
}

// mergeModelInfo replaces fields on the juju model info object with
// information from JIMM where JIMM specific information should be used.
func (j *JIMM) mergeModelInfo(ctx context.Context, user *openfga.User, modelInfo *jujuparams.ModelInfo, jimmModel dbmodel.Model) (*jujuparams.ModelInfo, error) {
const op = errors.Op("jimm.mergeModelInfo")

jimmSummary := jimmModel.ToJujuModelSummary()
modelInfo.CloudCredentialTag = jimmSummary.CloudCredentialTag
modelInfo.ControllerUUID = jimmSummary.ControllerUUID
modelInfo.OwnerTag = jimmSummary.OwnerTag

userAccess := make(map[string]string)

for _, relation := range []openfga.Relation{
Expand All @@ -733,7 +740,7 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model
ofganames.WriterRelation,
ofganames.ReaderRelation,
} {
usersWithSpecifiedRelation, err := openfga.ListUsersWithAccess(ctx, j.OpenFGAClient, mt, relation)
usersWithSpecifiedRelation, err := openfga.ListUsersWithAccess(ctx, j.OpenFGAClient, jimmModel.ResourceTag(), relation)
if err != nil {
return nil, errors.E(op, err)
}
Expand All @@ -747,6 +754,11 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model
}
}

modelAccess, err := j.GetUserModelAccess(ctx, user, jimmModel.ResourceTag())
if err != nil {
return nil, errors.E(op, err)
}

users := make([]jujuparams.ModelUserInfo, 0, len(userAccess))
for username, access := range userAccess {
// If the user does not contain an "@" sign (no domain), it means
Expand All @@ -762,15 +774,15 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model
})
}
}
mi.Users = users
modelInfo.Users = users

if modelAccess != "admin" && modelAccess != "write" {
// Users need "write" level access (or above) to see machine
// information.
mi.Machines = nil
modelInfo.Machines = nil
}

return mi, nil
return modelInfo, nil
}

// ModelStatus returns a jujuparams.ModelStatus for the given model. If
Expand Down
188 changes: 64 additions & 124 deletions internal/jimm/model_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1230,19 +1230,8 @@ const modelInfoTestEnvWithEveryoneAccess = modelInfoTestEnv + `
access: read
`

var modelInfoTests = []struct {
name string
env string
username string
uuid string
expectModelInfo *jujuparams.ModelInfo
expectError string
}{{
name: "AdminUser",
env: modelInfoTestEnv,
username: "alice@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
expectModelInfo: &jujuparams.ModelInfo{
func modelInfoTestExpectedModelInfo(canReadMachineInfo bool, limitedExpectedUsers []jujuparams.ModelUserInfo) *jujuparams.ModelInfo {
info := jujuparams.ModelInfo{
Name: "model-1",
Type: "iaas",
UUID: "00000002-0000-0000-0000-000000000001",
Expand Down Expand Up @@ -1290,86 +1279,51 @@ var modelInfoTests = []struct {
Level: "unsupported",
},
AgentVersion: newVersion("1.2.3"),
},
}, {
name: "WriteUser",
env: modelInfoTestEnv,
username: "bob@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
expectModelInfo: &jujuparams.ModelInfo{
Name: "model-1",
Type: "iaas",
UUID: "00000002-0000-0000-0000-000000000001",
ControllerUUID: "00000001-0000-0000-0000-000000000001",
ProviderType: "test-provider",
DefaultSeries: "warty",
CloudTag: names.NewCloudTag("test-cloud").String(),
CloudRegion: "test-cloud-region",
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(),
OwnerTag: names.NewUserTag("alice@canonical.com").String(),
Life: life.Value(state.Alive.String()),
Status: jujuparams.EntityStatus{
Status: "available",
Info: "OK!",
Since: newDate(2020, 2, 20, 20, 2, 20, 0, time.UTC),
},
Users: []jujuparams.ModelUserInfo{{
UserName: "bob@canonical.com",
Access: "write",
}},
Machines: []jujuparams.ModelMachineInfo{{
Id: "0",
Hardware: jimmtest.ParseMachineHardware("arch=amd64 mem=8096 root-disk=10240 cores=1"),
InstanceId: "00000009-0000-0000-0000-0000000000000",
DisplayName: "Machine 0",
Status: "available",
Message: "OK!",
HasVote: true,
}, {
Id: "1",
Hardware: jimmtest.ParseMachineHardware("arch=amd64 mem=8096 root-disk=10240 cores=2"),
InstanceId: "00000009-0000-0000-0000-0000000000001",
DisplayName: "Machine 1",
Status: "available",
Message: "OK!",
HasVote: true,
}},
SLA: &jujuparams.ModelSLAInfo{
Level: "unsupported",
},
AgentVersion: newVersion("1.2.3"),
},
}, {
name: "ReadUser",
env: modelInfoTestEnv,
username: "charlie@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
expectModelInfo: &jujuparams.ModelInfo{
Name: "model-1",
Type: "iaas",
UUID: "00000002-0000-0000-0000-000000000001",
ControllerUUID: "00000001-0000-0000-0000-000000000001",
ProviderType: "test-provider",
DefaultSeries: "warty",
CloudTag: names.NewCloudTag("test-cloud").String(),
CloudRegion: "test-cloud-region",
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(),
OwnerTag: names.NewUserTag("alice@canonical.com").String(),
Life: life.Value(state.Alive.String()),
Status: jujuparams.EntityStatus{
Status: "available",
Info: "OK!",
Since: newDate(2020, 2, 20, 20, 2, 20, 0, time.UTC),
},
Users: []jujuparams.ModelUserInfo{{
UserName: "charlie@canonical.com",
Access: "read",
}},
SLA: &jujuparams.ModelSLAInfo{
Level: "unsupported",
},
AgentVersion: newVersion("1.2.3"),
},
}
if !canReadMachineInfo {
info.Machines = nil
}
if limitedExpectedUsers != nil {
info.Users = limitedExpectedUsers
}
return &info
}

var modelInfoTests = []struct {
name string
env string
username string
uuid string
originModelOwner string
expectModelInfo *jujuparams.ModelInfo
expectError string
}{{
name: "AdminUser",
env: modelInfoTestEnv,
username: "alice@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("alice@canonical.com").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(true, nil),
}, {
name: "WriteUser",
env: modelInfoTestEnv,
username: "bob@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("alice@canonical.com").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(true, []jujuparams.ModelUserInfo{{
UserName: "bob@canonical.com",
Access: "write",
}}),
}, {
name: "ReadUser",
env: modelInfoTestEnv,
username: "charlie@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("alice@canonical.com").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(false, []jujuparams.ModelUserInfo{{
UserName: "charlie@canonical.com",
Access: "read",
}}),
}, {
name: "NoAccess",
env: modelInfoTestEnv,
Expand All @@ -1383,36 +1337,22 @@ var modelInfoTests = []struct {
uuid: "00000002-0000-0000-0000-000000000002",
expectError: "model not found",
}, {
name: "Access through everyone user",
env: modelInfoTestEnvWithEveryoneAccess,
username: "diane@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
expectModelInfo: &jujuparams.ModelInfo{
Name: "model-1",
Type: "iaas",
UUID: "00000002-0000-0000-0000-000000000001",
ControllerUUID: "00000001-0000-0000-0000-000000000001",
ProviderType: "test-provider",
DefaultSeries: "warty",
CloudTag: names.NewCloudTag("test-cloud").String(),
CloudRegion: "test-cloud-region",
CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(),
OwnerTag: names.NewUserTag("alice@canonical.com").String(),
Life: life.Value(state.Alive.String()),
Status: jujuparams.EntityStatus{
Status: "available",
Info: "OK!",
Since: newDate(2020, 2, 20, 20, 2, 20, 0, time.UTC),
},
Users: []jujuparams.ModelUserInfo{{
UserName: "everyone@external",
Access: "read",
}},
SLA: &jujuparams.ModelSLAInfo{
Level: "unsupported",
},
AgentVersion: newVersion("1.2.3"),
},
name: "Access through everyone user",
env: modelInfoTestEnvWithEveryoneAccess,
username: "diane@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("alice@canonical.com").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(false, []jujuparams.ModelUserInfo{{
UserName: "everyone@external",
Access: "read",
}}),
}, {
name: "Owner field is replaced",
env: modelInfoTestEnv,
username: "alice@canonical.com",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("bob").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(true, nil),
},
}

Expand Down Expand Up @@ -1443,7 +1383,7 @@ func TestModelInfo(t *testing.T) {
mi.CloudTag = names.NewCloudTag("test-cloud").String()
mi.CloudRegion = "test-cloud-region"
mi.CloudCredentialTag = names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String()
mi.OwnerTag = names.NewUserTag("alice@canonical.com").String()
mi.OwnerTag = test.originModelOwner
mi.Life = life.Value(state.Alive.String())
mi.Status = jujuparams.EntityStatus{
Status: "available",
Expand Down
7 changes: 5 additions & 2 deletions internal/jimmtest/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,12 @@ func (d *Dialer) Dial(_ context.Context, ctl *dbmodel.Controller, _ names.ModelT
return nil, d.Err
}
atomic.AddInt64(&d.open, 1)
ctl.UUID = d.UUID
if ctl.UUID == "" {
ctl.UUID = DefaultControllerUUID
if d.UUID == "" {
ctl.UUID = DefaultControllerUUID
} else {
ctl.UUID = d.UUID
}
}
ctl.AgentVersion = d.AgentVersion
if ctl.AgentVersion == "" {
Expand Down

0 comments on commit d0a5be3

Please sign in to comment.