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.

Sign up for GitHub

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

return model summaries with new model owner #1367

Merged
merged 3 commits into from
Sep 18, 2024
Merged

return model summaries with new model owner #1367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec44b6b
return model summaries with new model owner
kian99 Sep 16, 2024
92ddb15
Merge branch 'v3' into return-model-info-with-jimm-data
kian99 Sep 17, 2024
d325df3
Merge branch 'v3' into return-model-info-with-jimm-data
kian99 Sep 18, 2024
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
34 changes: 23 additions & 11 deletions internal/jimm/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,13 +710,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 @@ -733,6 +727,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 @@ -742,7 +749,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 @@ -756,6 +763,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 @@ -771,15 +783,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
Open in desktop
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: "[email protected]",
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: "[email protected]",
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/[email protected]/cred-1").String(),
OwnerTag: names.NewUserTag("[email protected]").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: "[email protected]",
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: "[email protected]",
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/[email protected]/cred-1").String(),
OwnerTag: names.NewUserTag("[email protected]").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: "[email protected]",
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: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("[email protected]").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(true, nil),
}, {
name: "WriteUser",
env: modelInfoTestEnv,
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("[email protected]").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(true, []jujuparams.ModelUserInfo{{
UserName: "[email protected]",
Access: "write",
}}),
}, {
name: "ReadUser",
env: modelInfoTestEnv,
username: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("[email protected]").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(false, []jujuparams.ModelUserInfo{{
UserName: "[email protected]",
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: "[email protected]",
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/[email protected]/cred-1").String(),
OwnerTag: names.NewUserTag("[email protected]").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: "[email protected]",
uuid: "00000002-0000-0000-0000-000000000001",
originModelOwner: names.NewUserTag("[email protected]").String(),
expectModelInfo: modelInfoTestExpectedModelInfo(false, []jujuparams.ModelUserInfo{{
UserName: "everyone@external",
Access: "read",
}}),
}, {
name: "Owner field is replaced",
env: modelInfoTestEnv,
username: "[email protected]",
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/[email protected]/cred-1").String()
mi.OwnerTag = names.NewUserTag("[email protected]").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
Open in desktop
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
Loading