From 253320e463c76941487bef1de16efac84078658c Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Tue, 14 Jan 2025 11:00:06 +0200 Subject: [PATCH] chore: move ListResources method to permission manager (#1520) * chore: move ListResources method to permission manager The ListResources method is used solely by the rebac-admin UI to list the resources that are known to JIMM. This method would need to go on a manager/business object eventually, and the permission manager made the most sense with the above in mind. * chore: rename TestGetResources to TestListResources --- internal/jimm/jimm.go | 2 + internal/jimm/permissions/relations.go | 12 ++++ internal/jimm/permissions/relations_test.go | 46 +++++++++++++ internal/jimm/resource.go | 22 ------- internal/jimm/resource_test.go | 65 ------------------- internal/jimmhttp/rebac_admin/resources.go | 4 +- .../jimmhttp/rebac_admin/resources_test.go | 11 +++- internal/jujuapi/interface.go | 3 - .../jimmtest/mocks/jimm_relation_mock.go | 9 +++ 9 files changed, 80 insertions(+), 94 deletions(-) delete mode 100644 internal/jimm/resource.go delete mode 100644 internal/jimm/resource_test.go diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index e0288983d..784c6f5e1 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -186,6 +186,8 @@ type PermissionManager interface { ListRelationshipTuples(ctx context.Context, user *openfga.User, tuple apiparams.RelationshipTuple, pageSize int32, continuationToken string) ([]openfga.Tuple, string, error) // ListObjectRelations lists all the tuples that an object has a direct relation with. ListObjectRelations(ctx context.Context, user *openfga.User, object string, pageSize int32, entitlementToken pagination.EntitlementToken) ([]openfga.Tuple, pagination.EntitlementToken, error) + // ListResources lists all resources known to JIMM. + ListResources(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, namePrefixFilter, typeFilter string) ([]db.Resource, error) // GetJimmControllerAccess returns the user's level of access to JIMM. GetJimmControllerAccess(ctx context.Context, user *openfga.User, tag names.UserTag) (string, error) diff --git a/internal/jimm/permissions/relations.go b/internal/jimm/permissions/relations.go index 000a558d3..302d95bb7 100644 --- a/internal/jimm/permissions/relations.go +++ b/internal/jimm/permissions/relations.go @@ -10,6 +10,7 @@ import ( "go.uber.org/zap" "github.com/canonical/jimm/v3/internal/common/pagination" + "github.com/canonical/jimm/v3/internal/db" "github.com/canonical/jimm/v3/internal/errors" "github.com/canonical/jimm/v3/internal/openfga" ofganames "github.com/canonical/jimm/v3/internal/openfga/names" @@ -127,6 +128,17 @@ func (j *permissionManager) ListObjectRelations(ctx context.Context, user *openf return responseTuples, nextToken, nil } +// ListResources returns a list of resources known to JIMM with a pagination filter. +func (j *permissionManager) ListResources(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, namePrefixFilter, typeFilter string) ([]db.Resource, error) { + const op = errors.Op("jimm.ListResources") + + if !user.JimmAdmin { + return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized") + } + + return j.store.ListResources(ctx, filter.Limit(), filter.Offset(), namePrefixFilter, typeFilter) +} + func (j *permissionManager) getObjectRelationsPage(ctx context.Context, object string, pageSize int32, entitlementToken pagination.EntitlementToken) ([]openfga.Tuple, pagination.EntitlementToken, error) { var err error var e pagination.EntitlementToken diff --git a/internal/jimm/permissions/relations_test.go b/internal/jimm/permissions/relations_test.go index 6f6c4c8f2..6649cd1ed 100644 --- a/internal/jimm/permissions/relations_test.go +++ b/internal/jimm/permissions/relations_test.go @@ -266,3 +266,49 @@ func (s *permissionManagerSuite) TestListObjectRelations(c *qt.C) { }) } } + +func (s *permissionManagerSuite) TestListResources(c *qt.C) { + c.Parallel() + ctx := context.Background() + + _, _, controller, model, applicationOffer, cloud, _, _ := jimmtest.CreateTestControllerEnvironment(ctx, c, s.db) + + ids := []string{applicationOffer.UUID, cloud.Name, controller.UUID, model.UUID.String} + + testCases := []struct { + desc string + limit int + offset int + identities []string + }{ + { + desc: "test with first resources", + limit: 3, + offset: 0, + identities: []string{ids[0], ids[1], ids[2]}, + }, + { + desc: "test with remianing ids", + limit: 3, + offset: 3, + identities: []string{ids[3]}, + }, + { + desc: "test out of range", + limit: 3, + offset: 6, + identities: []string{}, + }, + } + for _, t := range testCases { + c.Run(t.desc, func(c *qt.C) { + filter := pagination.NewOffsetFilter(t.limit, t.offset) + resources, err := s.manager.ListResources(ctx, s.adminUser, filter, "", "") + c.Assert(err, qt.IsNil) + c.Assert(resources, qt.HasLen, len(t.identities)) + for i := range len(t.identities) { + c.Assert(resources[i].ID.String, qt.Equals, t.identities[i]) + } + }) + } +} diff --git a/internal/jimm/resource.go b/internal/jimm/resource.go deleted file mode 100644 index c1734a551..000000000 --- a/internal/jimm/resource.go +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2024 Canonical. -package jimm - -import ( - "context" - - "github.com/canonical/jimm/v3/internal/common/pagination" - "github.com/canonical/jimm/v3/internal/db" - "github.com/canonical/jimm/v3/internal/errors" - "github.com/canonical/jimm/v3/internal/openfga" -) - -// ListResources returns a list of resources known to JIMM with a pagination filter. -func (j *JIMM) ListResources(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, namePrefixFilter, typeFilter string) ([]db.Resource, error) { - const op = errors.Op("jimm.ListResources") - - if !user.JimmAdmin { - return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized") - } - - return j.Database.ListResources(ctx, filter.Limit(), filter.Offset(), namePrefixFilter, typeFilter) -} diff --git a/internal/jimm/resource_test.go b/internal/jimm/resource_test.go deleted file mode 100644 index c44f2b0c5..000000000 --- a/internal/jimm/resource_test.go +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2024 Canonical. -package jimm_test - -import ( - "context" - "testing" - - qt "github.com/frankban/quicktest" - - "github.com/canonical/jimm/v3/internal/common/pagination" - "github.com/canonical/jimm/v3/internal/dbmodel" - "github.com/canonical/jimm/v3/internal/openfga" - "github.com/canonical/jimm/v3/internal/testutils/jimmtest" -) - -func TestGetResources(t *testing.T) { - c := qt.New(t) - ctx := context.Background() - - j := jimmtest.NewJIMM(c, nil) - - _, _, controller, model, applicationOffer, cloud, _, _ := jimmtest.CreateTestControllerEnvironment(ctx, c, j.Database) - - ids := []string{applicationOffer.UUID, cloud.Name, controller.UUID, model.UUID.String} - - u := openfga.NewUser(&dbmodel.Identity{Name: "admin@canonical.com"}, j.OpenFGAClient) - u.JimmAdmin = true - - testCases := []struct { - desc string - limit int - offset int - identities []string - }{ - { - desc: "test with first resources", - limit: 3, - offset: 0, - identities: []string{ids[0], ids[1], ids[2]}, - }, - { - desc: "test with remianing ids", - limit: 3, - offset: 3, - identities: []string{ids[3]}, - }, - { - desc: "test out of range", - limit: 3, - offset: 6, - identities: []string{}, - }, - } - for _, t := range testCases { - c.Run(t.desc, func(c *qt.C) { - filter := pagination.NewOffsetFilter(t.limit, t.offset) - resources, err := j.ListResources(ctx, u, filter, "", "") - c.Assert(err, qt.IsNil) - c.Assert(resources, qt.HasLen, len(t.identities)) - for i := range len(t.identities) { - c.Assert(resources[i].ID.String, qt.Equals, t.identities[i]) - } - }) - } -} diff --git a/internal/jimmhttp/rebac_admin/resources.go b/internal/jimmhttp/rebac_admin/resources.go index 0db979cf9..77f28fcd1 100644 --- a/internal/jimmhttp/rebac_admin/resources.go +++ b/internal/jimmhttp/rebac_admin/resources.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package rebac_admin @@ -40,7 +40,7 @@ func (s *resourcesService) ListResources(ctx context.Context, params *resources. } } - res, err := s.jimm.ListResources(ctx, user, pagination, namePrefixFilter, typeFilter) + res, err := s.jimm.PermissionManager().ListResources(ctx, user, pagination, namePrefixFilter, typeFilter) if err != nil { return nil, err } diff --git a/internal/jimmhttp/rebac_admin/resources_test.go b/internal/jimmhttp/rebac_admin/resources_test.go index 00e54032d..f75f46f3c 100644 --- a/internal/jimmhttp/rebac_admin/resources_test.go +++ b/internal/jimmhttp/rebac_admin/resources_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package rebac_admin_test @@ -13,18 +13,25 @@ import ( "github.com/canonical/jimm/v3/internal/common/pagination" "github.com/canonical/jimm/v3/internal/common/utils" "github.com/canonical/jimm/v3/internal/db" + "github.com/canonical/jimm/v3/internal/jimm" "github.com/canonical/jimm/v3/internal/jimmhttp/rebac_admin" "github.com/canonical/jimm/v3/internal/openfga" "github.com/canonical/jimm/v3/internal/testutils/jimmtest" + "github.com/canonical/jimm/v3/internal/testutils/jimmtest/mocks" ) func TestListResources(t *testing.T) { c := qt.New(t) - jimm := jimmtest.JIMM{ + permissionManager := mocks.PermissionManager{ ListResources_: func(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, nameFilter, typeFilter string) ([]db.Resource, error) { return []db.Resource{}, nil }, } + jimm := jimmtest.JIMM{ + PermissionManager_: func() jimm.PermissionManager { + return &permissionManager + }, + } user := openfga.User{} user.JimmAdmin = true ctx := context.Background() diff --git a/internal/jujuapi/interface.go b/internal/jujuapi/interface.go index 48a9bcecd..149d1bf62 100644 --- a/internal/jujuapi/interface.go +++ b/internal/jujuapi/interface.go @@ -10,8 +10,6 @@ import ( jujuparams "github.com/juju/juju/rpc/params" "github.com/juju/names/v5" - "github.com/canonical/jimm/v3/internal/common/pagination" - "github.com/canonical/jimm/v3/internal/db" "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/jimm" "github.com/canonical/jimm/v3/internal/openfga" @@ -46,7 +44,6 @@ type JIMM interface { InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) ListApplicationOffers(ctx context.Context, user *openfga.User, filters ...jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error) ListModels(ctx context.Context, user *openfga.User) ([]base.UserModel, error) - ListResources(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, namePrefixFilter, typeFilter string) ([]db.Resource, error) Offer(ctx context.Context, user *openfga.User, offer jimm.AddApplicationOfferParams) error PubSubHub() *pubsub.Hub RemoveCloud(ctx context.Context, u *openfga.User, ct names.CloudTag) error diff --git a/internal/testutils/jimmtest/mocks/jimm_relation_mock.go b/internal/testutils/jimmtest/mocks/jimm_relation_mock.go index 0e2f15193..462ef9510 100644 --- a/internal/testutils/jimmtest/mocks/jimm_relation_mock.go +++ b/internal/testutils/jimmtest/mocks/jimm_relation_mock.go @@ -9,6 +9,7 @@ import ( "github.com/juju/names/v5" "github.com/canonical/jimm/v3/internal/common/pagination" + "github.com/canonical/jimm/v3/internal/db" "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/errors" "github.com/canonical/jimm/v3/internal/openfga" @@ -24,6 +25,7 @@ type PermissionManager struct { CheckRelation_ func(ctx context.Context, user *openfga.User, tuple apiparams.RelationshipTuple, trace bool) (_ bool, err error) ListRelationshipTuples_ func(ctx context.Context, user *openfga.User, tuple apiparams.RelationshipTuple, pageSize int32, continuationToken string) ([]openfga.Tuple, string, error) ListObjectRelations_ func(ctx context.Context, user *openfga.User, object string, pageSize int32, continuationToken pagination.EntitlementToken) ([]openfga.Tuple, pagination.EntitlementToken, error) + ListResources_ func(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, namePrefixFilter, typeFilter string) ([]db.Resource, error) GetJimmControllerAccess_ func(ctx context.Context, user *openfga.User, tag names.UserTag) (string, error) GetUserCloudAccess_ func(ctx context.Context, user *openfga.User, cloud names.CloudTag) (string, error) @@ -80,6 +82,13 @@ func (j *PermissionManager) ListObjectRelations(ctx context.Context, user *openf return j.ListObjectRelations_(ctx, user, object, pageSize, entitlementToken) } +func (j *PermissionManager) ListResources(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, namePrefixFilter, typeFilter string) ([]db.Resource, error) { + if j.ListResources_ == nil { + return nil, errors.E(errors.CodeNotImplemented) + } + return j.ListResources_(ctx, user, filter, namePrefixFilter, typeFilter) +} + func (j *PermissionManager) GetJimmControllerAccess(ctx context.Context, user *openfga.User, tag names.UserTag) (string, error) { if j.GetJimmControllerAccess_ == nil { return "", errors.E(errors.CodeNotImplemented)