From f423bbd6f98a00aff5c2efe3b6eaadca8b6ebbc2 Mon Sep 17 00:00:00 2001 From: Piotr Kulik <70904851+pkulik0@users.noreply.github.com> Date: Thu, 5 Sep 2024 10:58:00 -0400 Subject: [PATCH 01/10] Use go version from go.mod in golangci-lint workflow (#1354) --- .github/workflows/golangci-lint.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml index 9b24d51e7..7d01493ea 100644 --- a/.github/workflows/golangci-lint.yaml +++ b/.github/workflows/golangci-lint.yaml @@ -17,7 +17,7 @@ jobs: - name: Setup Go uses: actions/setup-go@v5 with: - go-version: stable + go-version-file: 'go.mod' - name: Run Golangci-lint uses: golangci/golangci-lint-action@v6 From 4d88a296957f2e2e11c1c4c4a30a0c222609e28b Mon Sep 17 00:00:00 2001 From: SimoneDutto Date: Fri, 6 Sep 2024 16:50:32 +0200 Subject: [PATCH 02/10] Css 10530/filterning resources (#1353) * add filter by name and entity with tests --- internal/common/utils/test_utils.go | 4 + internal/db/resource.go | 175 ++++++++++++------ internal/db/resource_test.go | 103 ++++++++++- internal/jimm/resource.go | 4 +- internal/jimm/resource_test.go | 2 +- internal/jimmtest/jimm_mock.go | 6 +- internal/jujuapi/controllerroot.go | 2 +- internal/rebac_admin/entitlements.go | 79 ++++---- internal/rebac_admin/resources.go | 34 +++- .../rebac_admin/resources_integration_test.go | 81 +++++++- internal/rebac_admin/resources_test.go | 81 ++++++++ internal/rebac_admin/utils/utils.go | 10 + 12 files changed, 472 insertions(+), 109 deletions(-) create mode 100644 internal/rebac_admin/resources_test.go diff --git a/internal/common/utils/test_utils.go b/internal/common/utils/test_utils.go index 4e0b7fdcc..dc63a2b27 100644 --- a/internal/common/utils/test_utils.go +++ b/internal/common/utils/test_utils.go @@ -4,3 +4,7 @@ package utils func IntToPointer(i int) *int { return &i } + +func StringToPointer(s string) *string { + return &s +} diff --git a/internal/db/resource.go b/internal/db/resource.go index f5dd58e10..7eaab7749 100644 --- a/internal/db/resource.go +++ b/internal/db/resource.go @@ -5,64 +5,65 @@ import ( "context" "database/sql" + "gorm.io/gorm" + + "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/errors" "github.com/canonical/jimm/v3/internal/servermon" ) -// RESOURCES_RAW_SQL contains the raw query fetching entities from multiple tables, with their respective entity parents. -const RESOURCES_RAW_SQL = ` -( - SELECT 'application_offer' AS type, - application_offers.uuid AS id, - application_offers.name AS name, - models.uuid AS parent_id, - models.name AS parent_name, - 'model' AS parent_type - FROM application_offers - JOIN models ON application_offers.model_id = models.id -) -UNION -( - SELECT 'cloud' AS type, - clouds.name AS id, - clouds.name AS name, - '' AS parent_id, - '' AS parent_name, - '' AS parent_type - FROM clouds -) -UNION -( - SELECT 'controller' AS type, - controllers.uuid AS id, - controllers.name AS name, - '' AS parent_id, - '' AS parent_name, - '' AS parent_type - FROM controllers -) -UNION -( - SELECT 'model' AS type, - models.uuid AS id, - models.name AS name, - controllers.uuid AS parent_id, - controllers.name AS parent_name, - 'controller' AS parent_type - FROM models - JOIN controllers ON models.controller_id = controllers.id -) -UNION -( - SELECT 'service_account' AS type, - identities.name AS id, - identities.name AS name, - '' AS parent_id, - '' AS parent_name, - '' AS parent_type - FROM identities - WHERE name LIKE '%@serviceaccount' -) +const ApplicationOffersQueryKey = "application_offers" +const selectApplicationOffers = ` +'application_offer' AS type, +application_offers.uuid AS id, +application_offers.name AS name, +models.uuid AS parent_id, +models.name AS parent_name, +'model' AS parent_type +` + +const CloudsQueryKey = "clouds" +const selectClouds = ` +'cloud' AS type, +clouds.name AS id, +clouds.name AS name, +'' AS parent_id, +'' AS parent_name, +'' AS parent_type +` + +const ControllersQueryKey = "controllers" +const selectControllers = ` +'controller' AS type, +controllers.uuid AS id, +controllers.name AS name, +'' AS parent_id, +'' AS parent_name, +'' AS parent_type +` + +const ModelsQueryKey = "models" +const selectModels = ` +'model' AS type, +models.uuid AS id, +models.name AS name, +controllers.uuid AS parent_id, +controllers.name AS parent_name, +'controller' AS parent_type +` + +const ServiceAccountQueryKey = "identities" +const selectIdentities = ` +'service_account' AS type, +identities.name AS id, +identities.name AS name, +'' AS parent_id, +'' AS parent_name, +'' AS parent_type +` + +const unionQuery = ` +? UNION ? UNION ? UNION ? UNION ? ORDER BY type, id OFFSET ? LIMIT ?; @@ -79,7 +80,7 @@ type Resource struct { // ListResources returns a list of models, clouds, controllers, service accounts, and application offers, with its respective parents. // It has been implemented with a raw query because this is a specific implementation for the ReBAC Admin UI. -func (d *Database) ListResources(ctx context.Context, limit, offset int) (_ []Resource, err error) { +func (d *Database) ListResources(ctx context.Context, limit, offset int, namePrefixFilter, typeFilter string) (_ []Resource, err error) { const op = errors.Op("db.ListResources") if err := d.ready(); err != nil { return nil, errors.E(op, err) @@ -90,7 +91,11 @@ func (d *Database) ListResources(ctx context.Context, limit, offset int) (_ []Re defer servermon.ErrorCounter(servermon.DBQueryErrorCount, &err, string(op)) db := d.DB.WithContext(ctx) - rows, err := db.Raw(RESOURCES_RAW_SQL, offset, limit).Rows() + query, err := buildQuery(db, offset, limit, namePrefixFilter, typeFilter) + if err != nil { + return nil, err + } + rows, err := query.Rows() if err != nil { return nil, err } @@ -106,3 +111,61 @@ func (d *Database) ListResources(ctx context.Context, limit, offset int) (_ []Re } return resources, nil } + +// buildQuery is a utility function to build the database query according to two optional parameters. +// namePrefixFilter: used to match resources name prefix. +// typeFilter: used to match resources type. If this is not empty the resources are fetched from a single table. +func buildQuery(db *gorm.DB, offset, limit int, namePrefixFilter, typeFilter string) (*gorm.DB, error) { + applicationOffersQuery := db.Select(selectApplicationOffers). + Model(&dbmodel.ApplicationOffer{}). + Where("(CASE WHEN ? = '' THEN TRUE ELSE application_offers.name LIKE ? END)", namePrefixFilter, namePrefixFilter+"%"). + Joins("JOIN models ON application_offers.model_id = models.id") + + cloudsQuery := db.Select(selectClouds). + Model(&dbmodel.Cloud{}). + Where("(CASE WHEN ? = '' THEN TRUE ELSE clouds.name LIKE ? END)", namePrefixFilter, namePrefixFilter+"%") + + controllersQuery := db.Select(selectControllers). + Model(&dbmodel.Controller{}). + Where("(CASE WHEN ? = '' THEN TRUE ELSE controllers.name LIKE ? END)", namePrefixFilter, namePrefixFilter+"%") + + modelsQuery := db.Select(selectModels). + Model(&dbmodel.Model{}). + Where("(CASE WHEN ? = '' THEN TRUE ELSE models.name LIKE ? END)", namePrefixFilter, namePrefixFilter+"%"). + Joins("JOIN controllers ON models.controller_id = controllers.id") + + serviceAccountsQuery := db.Select(selectIdentities). + Model(&dbmodel.Identity{}). + Where("name LIKE '%@serviceaccount' AND (CASE WHEN ? = '' THEN TRUE ELSE identities.name LIKE ? END)", namePrefixFilter, namePrefixFilter+"%") + + // if the typeFilter is set we only return the query for that specif entityType, otherwise the union. + if typeFilter == "" { + return db. + Raw(unionQuery, + applicationOffersQuery, + cloudsQuery, + controllersQuery, + modelsQuery, + serviceAccountsQuery, + offset, + limit, + ), nil + } + var query *gorm.DB + switch typeFilter { + case ControllersQueryKey: + query = controllersQuery + case CloudsQueryKey: + query = cloudsQuery + case ApplicationOffersQueryKey: + query = applicationOffersQuery + case ModelsQueryKey: + query = modelsQuery + case ServiceAccountQueryKey: + query = serviceAccountsQuery + default: + // this shouldn't happen because we have validated the entityFilter at API layer + return nil, errors.E("this entityType does not exist") + } + return query.Order("id").Offset(offset).Limit(limit), nil +} diff --git a/internal/db/resource_test.go b/internal/db/resource_test.go index c42ccde1d..6786e5cbf 100644 --- a/internal/db/resource_test.go +++ b/internal/db/resource_test.go @@ -12,7 +12,7 @@ import ( "github.com/canonical/jimm/v3/internal/dbmodel" ) -func SetupDB(c *qt.C, database *db.Database) (dbmodel.Model, dbmodel.Controller, dbmodel.Cloud) { +func SetupDB(c *qt.C, database *db.Database) (dbmodel.Model, dbmodel.Controller, dbmodel.Cloud, dbmodel.Identity) { u, err := dbmodel.NewIdentity("bob@canonical.com") c.Assert(err, qt.IsNil) c.Assert(database.DB.Create(&u).Error, qt.IsNil) @@ -66,21 +66,27 @@ func SetupDB(c *qt.C, database *db.Database) (dbmodel.Model, dbmodel.Controller, } err = database.AddModel(context.Background(), &model) c.Assert(err, qt.Equals, nil) - return model, controller, cloud + clientIDWithDomain := "abda51b2-d735-4794-a8bd-49c506baa4af@serviceaccount" + sa, err := dbmodel.NewIdentity(clientIDWithDomain) + c.Assert(err, qt.Equals, nil) + err = database.GetIdentity(context.Background(), sa) + c.Assert(err, qt.Equals, nil) + + return model, controller, cloud, *sa } func (s *dbSuite) TestGetResources(c *qt.C) { ctx := context.Background() err := s.Database.Migrate(context.Background(), true) c.Assert(err, qt.Equals, nil) - res, err := s.Database.ListResources(ctx, 10, 0) + res, err := s.Database.ListResources(ctx, 10, 0, "", "") c.Assert(err, qt.Equals, nil) c.Assert(res, qt.HasLen, 0) // create one model, one controller, one cloud - model, controller, cloud := SetupDB(c, s.Database) - res, err = s.Database.ListResources(ctx, 10, 0) + model, controller, cloud, sva := SetupDB(c, s.Database) + res, err = s.Database.ListResources(ctx, 10, 0, "", "") c.Assert(err, qt.Equals, nil) - c.Assert(res, qt.HasLen, 3) + c.Assert(res, qt.HasLen, 4) for _, r := range res { switch r.Type { case "model": @@ -90,6 +96,91 @@ func (s *dbSuite) TestGetResources(c *qt.C) { c.Assert(r.ID.String, qt.Equals, controller.UUID) case "cloud": c.Assert(r.ID.String, qt.Equals, cloud.Name) + case "service_account": + c.Assert(r.ID.String, qt.Equals, sva.Name) } } } + +func (s *dbSuite) TestGetResourcesWithNameTypeFilter(c *qt.C) { + ctx := context.Background() + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + // create one model, one controller, one cloud + model, controller, cloud, sva := SetupDB(c, s.Database) + + tests := []struct { + description string + nameFilter string + typeFilter string + limit int + offset int + expectedSize int + expectedUUIDs []string + }{ + { + description: "filter on model name", + nameFilter: model.Name, + limit: 10, + offset: 0, + typeFilter: "", + expectedSize: 1, + expectedUUIDs: []string{model.UUID.String}, + }, + { + description: "filter name test prefix", + nameFilter: "test", + limit: 10, + offset: 0, + typeFilter: "", + expectedSize: 3, + expectedUUIDs: []string{cloud.Name, controller.UUID, model.UUID.String}, + }, + { + description: "filter name controller suffix", + nameFilter: "controller", + limit: 10, + offset: 0, + typeFilter: "", + expectedSize: 0, + expectedUUIDs: []string{}, + }, + { + description: "filter only models", + nameFilter: "test", + limit: 10, + offset: 0, + typeFilter: "models", + expectedSize: 1, + expectedUUIDs: []string{model.UUID.String}, + }, + { + description: "filter only service accounts", + nameFilter: "", + limit: 10, + offset: 0, + typeFilter: "identities", + expectedSize: 1, + expectedUUIDs: []string{sva.Name}, + }, + { + description: "filter only service accounts and name", + nameFilter: "not-found", + limit: 10, + offset: 0, + typeFilter: "identities", + expectedSize: 0, + expectedUUIDs: []string{}, + }, + } + for _, t := range tests { + c.Run(t.description, func(c *qt.C) { + res, err := s.Database.ListResources(ctx, t.limit, t.offset, t.nameFilter, t.typeFilter) + c.Assert(err, qt.Equals, nil) + c.Assert(res, qt.HasLen, t.expectedSize) + for i, r := range res { + c.Assert(r.ID.String, qt.Equals, t.expectedUUIDs[i]) + } + }) + } +} diff --git a/internal/jimm/resource.go b/internal/jimm/resource.go index 134ba56ed..c1734a551 100644 --- a/internal/jimm/resource.go +++ b/internal/jimm/resource.go @@ -11,12 +11,12 @@ import ( ) // 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) ([]db.Resource, error) { +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()) + 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 index a6c78d2ea..f03da279f 100644 --- a/internal/jimm/resource_test.go +++ b/internal/jimm/resource_test.go @@ -69,7 +69,7 @@ func TestGetResources(t *testing.T) { 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) + 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) { diff --git a/internal/jimmtest/jimm_mock.go b/internal/jimmtest/jimm_mock.go index de846f8f0..7371d4c45 100644 --- a/internal/jimmtest/jimm_mock.go +++ b/internal/jimmtest/jimm_mock.go @@ -68,7 +68,7 @@ type JIMM struct { InitiateMigration_ func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) InitiateInternalMigration_ func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error) ListApplicationOffers_ func(ctx context.Context, user *openfga.User, filters ...jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error) - ListResources_ func(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]db.Resource, error) + ListResources_ func(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, namePrefixFilter, typeFilter string) ([]db.Resource, error) Offer_ func(ctx context.Context, user *openfga.User, offer jimm.AddApplicationOfferParams) error PubSubHub_ func() *pubsub.Hub PurgeLogs_ func(ctx context.Context, user *openfga.User, before time.Time) (int64, error) @@ -301,11 +301,11 @@ func (j *JIMM) ListApplicationOffers(ctx context.Context, user *openfga.User, fi } return j.ListApplicationOffers_(ctx, user, filters...) } -func (j *JIMM) ListResources(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]db.Resource, error) { +func (j *JIMM) 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) + return j.ListResources_(ctx, user, filter, namePrefixFilter, typeFilter) } func (j *JIMM) Offer(ctx context.Context, user *openfga.User, offer jimm.AddApplicationOfferParams) error { if j.Offer_ == nil { diff --git a/internal/jujuapi/controllerroot.go b/internal/jujuapi/controllerroot.go index 9406570b6..19cb01507 100644 --- a/internal/jujuapi/controllerroot.go +++ b/internal/jujuapi/controllerroot.go @@ -66,7 +66,7 @@ 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) ListIdentities(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]openfga.User, error) - ListResources(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination) ([]db.Resource, 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 PurgeLogs(ctx context.Context, user *openfga.User, before time.Time) (int64, error) diff --git a/internal/rebac_admin/entitlements.go b/internal/rebac_admin/entitlements.go index 9f0d930a6..073ec18c2 100644 --- a/internal/rebac_admin/entitlements.go +++ b/internal/rebac_admin/entitlements.go @@ -10,56 +10,63 @@ import ( openfgastatic "github.com/canonical/jimm/v3/openfga" ) +const ApplicationOffer = "applicationoffer" +const Cloud = "cloud" +const Controller = "controller" +const Group = "group" +const Model = "model" +const ServiceAccount = "serviceaccount" + // For rebac v1 this list is kept manually. // The reason behind that is we want to decide what relations to expose to rebac admin ui. var EntitlementsList = []resources.EntitlementSchema{ // applicationoffer - {Entitlement: "administrator", ReceiverType: "user", EntityType: "applicationoffer"}, - {Entitlement: "administrator", ReceiverType: "user:*", EntityType: "applicationoffer"}, - {Entitlement: "administrator", ReceiverType: "group#member", EntityType: "applicationoffer"}, - {Entitlement: "consumer", ReceiverType: "user", EntityType: "applicationoffer"}, - {Entitlement: "consumer", ReceiverType: "user:*", EntityType: "applicationoffer"}, - {Entitlement: "consumer", ReceiverType: "group#member", EntityType: "applicationoffer"}, - {Entitlement: "reader", ReceiverType: "user", EntityType: "applicationoffer"}, - {Entitlement: "reader", ReceiverType: "user:*", EntityType: "applicationoffer"}, - {Entitlement: "reader", ReceiverType: "group#member", EntityType: "applicationoffer"}, + {Entitlement: "administrator", ReceiverType: "user", EntityType: ApplicationOffer}, + {Entitlement: "administrator", ReceiverType: "user:*", EntityType: ApplicationOffer}, + {Entitlement: "administrator", ReceiverType: "group#member", EntityType: ApplicationOffer}, + {Entitlement: "consumer", ReceiverType: "user", EntityType: ApplicationOffer}, + {Entitlement: "consumer", ReceiverType: "user:*", EntityType: ApplicationOffer}, + {Entitlement: "consumer", ReceiverType: "group#member", EntityType: ApplicationOffer}, + {Entitlement: "reader", ReceiverType: "user", EntityType: ApplicationOffer}, + {Entitlement: "reader", ReceiverType: "user:*", EntityType: ApplicationOffer}, + {Entitlement: "reader", ReceiverType: "group#member", EntityType: ApplicationOffer}, // cloud - {Entitlement: "administrator", ReceiverType: "user", EntityType: "cloud"}, - {Entitlement: "administrator", ReceiverType: "user:*", EntityType: "cloud"}, - {Entitlement: "administrator", ReceiverType: "group#member", EntityType: "cloud"}, - {Entitlement: "can_addmodel", ReceiverType: "user", EntityType: "cloud"}, - {Entitlement: "can_addmodel", ReceiverType: "user:*", EntityType: "cloud"}, - {Entitlement: "can_addmodel", ReceiverType: "group#member", EntityType: "cloud"}, + {Entitlement: "administrator", ReceiverType: "user", EntityType: Cloud}, + {Entitlement: "administrator", ReceiverType: "user:*", EntityType: Cloud}, + {Entitlement: "administrator", ReceiverType: "group#member", EntityType: Cloud}, + {Entitlement: "can_addmodel", ReceiverType: "user", EntityType: Cloud}, + {Entitlement: "can_addmodel", ReceiverType: "user:*", EntityType: Cloud}, + {Entitlement: "can_addmodel", ReceiverType: "group#member", EntityType: Cloud}, // controller - {Entitlement: "administrator", ReceiverType: "user", EntityType: "controller"}, - {Entitlement: "administrator", ReceiverType: "user:*", EntityType: "controller"}, - {Entitlement: "administrator", ReceiverType: "group#member", EntityType: "controller"}, - {Entitlement: "audit_log_viewer", ReceiverType: "user", EntityType: "controller"}, - {Entitlement: "audit_log_viewer", ReceiverType: "user:*", EntityType: "controller"}, - {Entitlement: "audit_log_viewer", ReceiverType: "group#member", EntityType: "controller"}, + {Entitlement: "administrator", ReceiverType: "user", EntityType: Controller}, + {Entitlement: "administrator", ReceiverType: "user:*", EntityType: Controller}, + {Entitlement: "administrator", ReceiverType: "group#member", EntityType: Controller}, + {Entitlement: "audit_log_viewer", ReceiverType: "user", EntityType: Controller}, + {Entitlement: "audit_log_viewer", ReceiverType: "user:*", EntityType: Controller}, + {Entitlement: "audit_log_viewer", ReceiverType: "group#member", EntityType: Controller}, // group - {Entitlement: "member", ReceiverType: "user", EntityType: "group"}, - {Entitlement: "member", ReceiverType: "user:*", EntityType: "group"}, - {Entitlement: "member", ReceiverType: "group#member", EntityType: "group"}, + {Entitlement: "member", ReceiverType: "user", EntityType: Group}, + {Entitlement: "member", ReceiverType: "user:*", EntityType: Group}, + {Entitlement: "member", ReceiverType: "group#member", EntityType: Group}, // model - {Entitlement: "administrator", ReceiverType: "user", EntityType: "model"}, - {Entitlement: "administrator", ReceiverType: "user:*", EntityType: "model"}, - {Entitlement: "administrator", ReceiverType: "group#member", EntityType: "model"}, - {Entitlement: "reader", ReceiverType: "user", EntityType: "model"}, - {Entitlement: "reader", ReceiverType: "user:*", EntityType: "model"}, - {Entitlement: "reader", ReceiverType: "group#member", EntityType: "model"}, - {Entitlement: "writer", ReceiverType: "user", EntityType: "model"}, - {Entitlement: "writer", ReceiverType: "user:*", EntityType: "model"}, - {Entitlement: "writer", ReceiverType: "group#member", EntityType: "model"}, + {Entitlement: "administrator", ReceiverType: "user", EntityType: Model}, + {Entitlement: "administrator", ReceiverType: "user:*", EntityType: Model}, + {Entitlement: "administrator", ReceiverType: "group#member", EntityType: Model}, + {Entitlement: "reader", ReceiverType: "user", EntityType: Model}, + {Entitlement: "reader", ReceiverType: "user:*", EntityType: Model}, + {Entitlement: "reader", ReceiverType: "group#member", EntityType: Model}, + {Entitlement: "writer", ReceiverType: "user", EntityType: Model}, + {Entitlement: "writer", ReceiverType: "user:*", EntityType: Model}, + {Entitlement: "writer", ReceiverType: "group#member", EntityType: Model}, // serviceaccount - {Entitlement: "administrator", ReceiverType: "user", EntityType: "serviceaccount"}, - {Entitlement: "administrator", ReceiverType: "user:*", EntityType: "serviceaccount"}, - {Entitlement: "administrator", ReceiverType: "group#member", EntityType: "serviceaccount"}, + {Entitlement: "administrator", ReceiverType: "user", EntityType: ServiceAccount}, + {Entitlement: "administrator", ReceiverType: "user:*", EntityType: ServiceAccount}, + {Entitlement: "administrator", ReceiverType: "group#member", EntityType: ServiceAccount}, } // entitlementsService implements the `entitlementsService` interface from rebac-admin-ui-handlers library diff --git a/internal/rebac_admin/resources.go b/internal/rebac_admin/resources.go index baca7aa19..c458de762 100644 --- a/internal/rebac_admin/resources.go +++ b/internal/rebac_admin/resources.go @@ -5,10 +5,12 @@ package rebac_admin import ( "context" + v1 "github.com/canonical/rebac-admin-ui-handlers/v1" "github.com/canonical/rebac-admin-ui-handlers/v1/resources" "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/jujuapi" "github.com/canonical/jimm/v3/internal/rebac_admin/utils" ) @@ -30,7 +32,15 @@ func (s *resourcesService) ListResources(ctx context.Context, params *resources. return nil, err } currentPage, expectedPageSize, pagination := pagination.CreatePaginationWithoutTotal(params.Size, params.Page) - res, err := s.jimm.ListResources(ctx, user, pagination) + namePrefixFilter, typeFilter := utils.GetNameAndTypeResourceFilter(params.EntityName, params.EntityType) + if typeFilter != "" { + typeFilter, err = validateAndConvertResourceFilter(typeFilter) + if err != nil { + return nil, v1.NewInvalidRequestError(err.Error()) + } + } + + res, err := s.jimm.ListResources(ctx, user, pagination, namePrefixFilter, typeFilter) if err != nil { return nil, err } @@ -58,10 +68,30 @@ func (s *resourcesService) ListResources(ctx context.Context, params *resources. // Otherwise we return the records we have and set next page as empty. func getNextPageAndResources(currentPage, expectedPageSize int, resources []db.Resource) (*int, []db.Resource) { var nextPage *int - if len(resources) == expectedPageSize { + if len(resources) > 0 && len(resources) == expectedPageSize { nPage := currentPage + 1 nextPage = &nPage resources = resources[:len(resources)-1] } return nextPage, resources } + +// validateAndConvertResourceFilter checks the typeFilter in the request and converts it to a valid key that is used to retrieved +// the right resources from the db. +func validateAndConvertResourceFilter(typeFilter string) (string, error) { + switch typeFilter { + case ApplicationOffer: + return db.ApplicationOffersQueryKey, nil + case Cloud: + return db.CloudsQueryKey, nil + case Controller: + return db.ControllersQueryKey, nil + case Model: + return db.ModelsQueryKey, nil + case ServiceAccount: + return db.ServiceAccountQueryKey, nil + default: + return "", errors.E("this resource type is not supported") + + } +} diff --git a/internal/rebac_admin/resources_integration_test.go b/internal/rebac_admin/resources_integration_test.go index dbb8bc352..fc2344110 100644 --- a/internal/rebac_admin/resources_integration_test.go +++ b/internal/rebac_admin/resources_integration_test.go @@ -3,6 +3,7 @@ package rebac_admin_test import ( "context" + "strings" rebac_handlers "github.com/canonical/rebac-admin-ui-handlers/v1" "github.com/canonical/rebac-admin-ui-handlers/v1/resources" @@ -69,25 +70,33 @@ func (s *resourcesSuite) TestListResources(c *gc.C) { env.PopulateDB(tester, s.JIMM.Database) type testEntity struct { Id string + Name string ParentId string + Type string } ids := make([]testEntity, 0) for _, c := range env.Clouds { ids = append(ids, testEntity{ Id: c.Name, + Name: c.Name, ParentId: "", + Type: "clouds", }) } for _, c := range env.Controllers { ids = append(ids, testEntity{ Id: c.UUID, + Name: c.Name, ParentId: "", + Type: "controllers", }) } for _, m := range env.Models { ids = append(ids, testEntity{ Id: m.UUID, + Name: m.Name, ParentId: env.Controller(m.Controller).UUID, + Type: "models", }) } @@ -95,6 +104,8 @@ func (s *resourcesSuite) TestListResources(c *gc.C) { desc string size *int page *int + nameFilter string + typeFilter string wantPage int wantSize int wantNextpage *int @@ -127,11 +138,77 @@ func (s *resourcesSuite) TestListResources(c *gc.C) { wantNextpage: nil, ids: []testEntity{ids[4]}, }, + { + desc: "test first page with model name filter", + size: utils.IntToPointer(2), + page: utils.IntToPointer(0), + nameFilter: "model", + wantPage: 0, + wantSize: 2, + wantNextpage: utils.IntToPointer(1), + ids: func() []testEntity { + filteredIds := make([]testEntity, 0) + for _, id := range ids { + if strings.HasPrefix(id.Name, "model") { + filteredIds = append(filteredIds, id) + } + } + return filteredIds + }()[:2], + }, + { + desc: "test first page with model name filter", + size: utils.IntToPointer(2), + page: utils.IntToPointer(1), + nameFilter: "model", + wantPage: 1, + wantSize: 1, + wantNextpage: nil, + ids: func() []testEntity { + filteredIds := make([]testEntity, 0) + for _, id := range ids { + if strings.Contains(id.Name, "model") { + filteredIds = append(filteredIds, id) + } + } + return filteredIds + }()[2:], + }, + { + desc: "test first page with controller entity type", + size: utils.IntToPointer(2), + page: utils.IntToPointer(0), + typeFilter: rebac_admin.Controller, + wantPage: 0, + wantSize: 1, + wantNextpage: nil, + ids: func() []testEntity { + filteredIds := make([]testEntity, 0) + for _, id := range ids { + if id.Type == "controllers" { + filteredIds = append(filteredIds, id) + } + } + return filteredIds + }(), + }, + { + desc: "test big page with model entity type", + size: utils.IntToPointer(10), + page: utils.IntToPointer(0), + typeFilter: rebac_admin.Model, + wantPage: 0, + wantSize: 3, + wantNextpage: nil, + ids: []testEntity{}, + }, } for _, t := range testCases { resources, err := resourcesSvc.ListResources(ctx, &resources.GetResourcesParams{ - Size: t.size, - Page: t.page, + Size: t.size, + Page: t.page, + EntityName: &t.nameFilter, + EntityType: &t.typeFilter, }) c.Assert(err, gc.IsNil) c.Assert(*resources.Meta.Page, gc.Equals, t.wantPage) diff --git a/internal/rebac_admin/resources_test.go b/internal/rebac_admin/resources_test.go new file mode 100644 index 000000000..6b9e21bee --- /dev/null +++ b/internal/rebac_admin/resources_test.go @@ -0,0 +1,81 @@ +// Copyright 2024 Canonical. + +package rebac_admin_test + +import ( + "context" + "testing" + + rebac_handlers "github.com/canonical/rebac-admin-ui-handlers/v1" + "github.com/canonical/rebac-admin-ui-handlers/v1/resources" + qt "github.com/frankban/quicktest" + + "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/jimmtest" + "github.com/canonical/jimm/v3/internal/openfga" + "github.com/canonical/jimm/v3/internal/rebac_admin" +) + +func TestListResources(t *testing.T) { + c := qt.New(t) + jimm := jimmtest.JIMM{ + ListResources_: func(ctx context.Context, user *openfga.User, filter pagination.LimitOffsetPagination, nameFilter, typeFilter string) ([]db.Resource, error) { + return []db.Resource{}, nil + }, + } + user := openfga.User{} + user.JimmAdmin = true + ctx := context.Background() + ctx = rebac_handlers.ContextWithIdentity(ctx, &user) + resourcesSvc := rebac_admin.NewResourcesService(&jimm) + + testCases := []struct { + desc string + size *int + page *int + nameFilter *string + typeFilter *string + expectErrorMatch string + }{ + { + desc: "test good", + size: utils.IntToPointer(2), + page: utils.IntToPointer(0), + nameFilter: utils.StringToPointer(""), + typeFilter: utils.StringToPointer(""), + }, + { + desc: "test good with all params set to nil", + size: nil, + page: nil, + nameFilter: nil, + typeFilter: nil, + }, + { + desc: "test with not valid type filter", + size: nil, + page: nil, + nameFilter: nil, + typeFilter: utils.StringToPointer("type-not-found"), + expectErrorMatch: ".*this resource type is not supported.*", + }, + } + for _, t := range testCases { + c.Run(t.desc, func(c *qt.C) { + _, err := resourcesSvc.ListResources(ctx, &resources.GetResourcesParams{ + Page: t.page, + Size: t.size, + EntityType: t.typeFilter, + EntityName: t.nameFilter, + }) + if t.expectErrorMatch != "" { + c.Assert(err, qt.ErrorMatches, t.expectErrorMatch) + } else { + c.Assert(err, qt.IsNil) + } + }) + } + +} diff --git a/internal/rebac_admin/utils/utils.go b/internal/rebac_admin/utils/utils.go index 3908b3d40..40a3e33d6 100644 --- a/internal/rebac_admin/utils/utils.go +++ b/internal/rebac_admin/utils/utils.go @@ -70,3 +70,13 @@ func ValidateDecomposedTag(kind string, id string) (names.Tag, error) { rawTag := kind + "-" + id return jimmnames.ParseTag(rawTag) } + +func GetNameAndTypeResourceFilter(nFilter, eFilter *string) (nameFilter, typeFilter string) { + if nFilter != nil { + nameFilter = *nFilter + } + if eFilter != nil { + typeFilter = *eFilter + } + return nameFilter, typeFilter +} From 293cb7374d12b7997562e41590b059557c4b2214 Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Mon, 9 Sep 2024 11:25:48 +0200 Subject: [PATCH 03/10] add version method to JIMM facade (#1356) --- internal/jujuapi/jimm.go | 12 ++++++++++++ internal/jujuapi/jimm_test.go | 10 ++++++++++ pkg/api/client.go | 7 +++++++ pkg/api/params/params.go | 6 ++++++ 4 files changed, 35 insertions(+) diff --git a/internal/jujuapi/jimm.go b/internal/jujuapi/jimm.go index 39768247d..f52551eea 100644 --- a/internal/jujuapi/jimm.go +++ b/internal/jujuapi/jimm.go @@ -22,6 +22,7 @@ import ( ofganames "github.com/canonical/jimm/v3/internal/openfga/names" "github.com/canonical/jimm/v3/pkg/api/params" apiparams "github.com/canonical/jimm/v3/pkg/api/params" + "github.com/canonical/jimm/v3/version" ) func init() { @@ -55,6 +56,7 @@ func init() { updateServiceAccountCredentials := rpc.Method(r.UpdateServiceAccountCredentials) listServiceAccountCredentials := rpc.Method(r.ListServiceAccountCredentials) grantServiceAccountAccess := rpc.Method(r.GrantServiceAccountAccess) + version := rpc.Method(r.Version) // JIMM Generic RPC r.AddMethod("JIMM", 4, "AddController", addControllerMethod) @@ -89,6 +91,7 @@ func init() { r.AddMethod("JIMM", 4, "UpdateServiceAccountCredentials", updateServiceAccountCredentials) r.AddMethod("JIMM", 4, "ListServiceAccountCredentials", listServiceAccountCredentials) r.AddMethod("JIMM", 4, "GrantServiceAccountAccess", grantServiceAccountAccess) + r.AddMethod("JIMM", 4, "Version", version) return []int{4} } @@ -503,3 +506,12 @@ func (r *controllerRoot) MigrateModel(ctx context.Context, args apiparams.Migrat Results: results, }, nil } + +// Version is a method on the JIMM facade that returns information on the version of JIMM. +func (r *controllerRoot) Version(ctx context.Context) (apiparams.VersionResponse, error) { + versionInfo := apiparams.VersionResponse{ + Version: version.VersionInfo.Version, + Commit: version.VersionInfo.GitCommit, + } + return versionInfo, nil +} diff --git a/internal/jujuapi/jimm_test.go b/internal/jujuapi/jimm_test.go index 140c97705..c45077ced 100644 --- a/internal/jujuapi/jimm_test.go +++ b/internal/jujuapi/jimm_test.go @@ -888,3 +888,13 @@ func (s *jimmSuite) TestJimmModelMigrationNonSuperuser(c *gc.C) { item := res.Results[0] c.Assert(item.Error.Message, gc.Matches, "unauthorized access") } + +func (s *jimmSuite) TestVersion(c *gc.C) { + conn := s.open(c, nil, "bob") + defer conn.Close() + client := api.NewClient(conn) + versionInfo, err := client.Version() + c.Assert(err, gc.IsNil) + c.Assert(versionInfo.Version, gc.Not(gc.Equals), "") + c.Assert(versionInfo.Commit, gc.Not(gc.Equals), "") +} diff --git a/pkg/api/client.go b/pkg/api/client.go index a736e515e..75b002e78 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -223,3 +223,10 @@ func (c *Client) UpdateServiceAccountCredentials(req *params.UpdateServiceAccoun func (c *Client) GrantServiceAccountAccess(req *params.GrantServiceAccountAccess) error { return c.caller.APICall("JIMM", 4, "", "GrantServiceAccountAccess", req, nil) } + +// Version returns version info of the controller. +func (c *Client) Version() (params.VersionResponse, error) { + var response params.VersionResponse + err := c.caller.APICall("JIMM", 4, "", "Version", nil, &response) + return response, err +} diff --git a/pkg/api/params/params.go b/pkg/api/params/params.go index 4f917f30f..7098f64a4 100644 --- a/pkg/api/params/params.go +++ b/pkg/api/params/params.go @@ -497,3 +497,9 @@ type WhoamiResponse struct { DisplayName string `json:"display-name" yaml:"display-name"` Email string `json:"email" yaml:"email"` } + +// VersionResponse holds the response for a version call. +type VersionResponse struct { + Version string `json:"version" yaml:"version"` + Commit string `json:"commit" yaml:"commit"` +} From 86a20ea44d6d3213d30861cdc5db5b354245581f Mon Sep 17 00:00:00 2001 From: pkulik0 <70904851+pkulik0@users.noreply.github.com> Date: Tue, 10 Sep 2024 11:40:10 +0000 Subject: [PATCH 04/10] Update copyright header in linter with year regexp --- .golangci.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.golangci.yaml b/.golangci.yaml index bd973feb4..d294f77e7 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -69,8 +69,11 @@ linters-settings: gocognit: min-complexity: 30 goheader: + values: + regexp: + year: (\d{4}) template: |- - Copyright 2024 Canonical. + Copyright {{year}} Canonical. importas: no-unaliased: false no-extra-aliases: false From 07eed59e9e300dade03e8d82fe16a9870187604e Mon Sep 17 00:00:00 2001 From: SimoneDutto Date: Wed, 11 Sep 2024 13:42:49 +0200 Subject: [PATCH 05/10] add capabilities endpoint (#1359) * add capabilities endpoint --------- Co-authored-by: Ales Stimec --- internal/rebac_admin/backend.go | 1 + internal/rebac_admin/capabilities.go | 116 ++++++++++++++++++++++ internal/rebac_admin/capabilities_test.go | 55 ++++++++++ internal/rebac_admin/export_test.go | 1 + internal/rebac_admin/identities.go | 11 +- internal/rebac_admin/identities_test.go | 3 +- 6 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 internal/rebac_admin/capabilities.go create mode 100644 internal/rebac_admin/capabilities_test.go diff --git a/internal/rebac_admin/backend.go b/internal/rebac_admin/backend.go index b021ff9ce..3e03de032 100644 --- a/internal/rebac_admin/backend.go +++ b/internal/rebac_admin/backend.go @@ -22,6 +22,7 @@ func SetupBackend(ctx context.Context, jimm jujuapi.JIMM) (*rebac_handlers.ReBAC Groups: newGroupService(jimm), Identities: newidentitiesService(jimm), Resources: newResourcesService(jimm), + Capabilities: newCapabilitiesService(), }) if err != nil { zapctx.Error(ctx, "failed to create rebac admin backend", zap.Error(err)) diff --git a/internal/rebac_admin/capabilities.go b/internal/rebac_admin/capabilities.go new file mode 100644 index 000000000..6b8303436 --- /dev/null +++ b/internal/rebac_admin/capabilities.go @@ -0,0 +1,116 @@ +// Copyright 2024 Canonical. + +package rebac_admin + +import ( + "context" + + "github.com/canonical/rebac-admin-ui-handlers/v1/interfaces" + "github.com/canonical/rebac-admin-ui-handlers/v1/resources" +) + +// capabilitiesService implements the `CapabilitiesService` interface. +type capabilitiesService struct { +} + +// For doc/test sake, to hint that the struct needs to implement a specific interface. +var _ interfaces.CapabilitiesService = &capabilitiesService{} + +func newCapabilitiesService() *capabilitiesService { + return &capabilitiesService{} +} + +var capabilities = []resources.Capability{ + { + Endpoint: "/swagger.json", + Methods: []resources.CapabilityMethods{ + "GET", + }, + }, + { + Endpoint: "/capabilities", + Methods: []resources.CapabilityMethods{ + "GET", + }, + }, + { + Endpoint: "/identities", + Methods: []resources.CapabilityMethods{ + "GET", + "POST", + }, + }, + { + Endpoint: "/identities/{id}", + Methods: []resources.CapabilityMethods{ + "GET", + }, + }, + { + Endpoint: "/identities/{id}/groups", + Methods: []resources.CapabilityMethods{ + "GET", + "PATCH", + }, + }, + { + Endpoint: "/identities/{id}/entitlements", + Methods: []resources.CapabilityMethods{ + "GET", + "PATCH", + }, + }, + { + Endpoint: "/groups", + Methods: []resources.CapabilityMethods{ + "GET", + "POST", + }, + }, + { + Endpoint: "/groups/{id}", + Methods: []resources.CapabilityMethods{ + "GET", + "PUT", + "DELETE", + }, + }, + { + Endpoint: "/groups/{id}/identities", + Methods: []resources.CapabilityMethods{ + "GET", + "PATCH", + }, + }, + { + Endpoint: "/groups/{id}/entitlements", + Methods: []resources.CapabilityMethods{ + "GET", + "PATCH", + }, + }, + { + Endpoint: "/entitlements", + Methods: []resources.CapabilityMethods{ + "GET", + }, + }, + { + Endpoint: "/entitlements/raw", + Methods: []resources.CapabilityMethods{ + "GET", + }, + }, + { + Endpoint: "/resources", + Methods: []resources.CapabilityMethods{ + "GET", + }, + }, +} + +// ListCapabilities returns a list of capabilities supported by this service. +func (s *capabilitiesService) ListCapabilities(ctx context.Context) ([]resources.Capability, error) { + + return capabilities, nil +} diff --git a/internal/rebac_admin/capabilities_test.go b/internal/rebac_admin/capabilities_test.go new file mode 100644 index 000000000..83a45f92e --- /dev/null +++ b/internal/rebac_admin/capabilities_test.go @@ -0,0 +1,55 @@ +// Copyright 2024 Canonical. + +package rebac_admin_test + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + qt "github.com/frankban/quicktest" + + "github.com/canonical/jimm/v3/internal/jimmtest" + "github.com/canonical/jimm/v3/internal/rebac_admin" +) + +// test capabilities are reachable +func TestCapabilities(t *testing.T) { + c := qt.New(t) + jimm := jimmtest.JIMM{} + ctx := context.Background() + handlers, err := rebac_admin.SetupBackend(ctx, &jimm) + c.Assert(err, qt.IsNil) + testServer := httptest.NewServer(handlers.Handler("")) + defer testServer.Close() + + // test not found endpoint + url := fmt.Sprintf("%s/v1%s", testServer.URL, "/not-found") + req, err := http.NewRequest("GET", url, nil) + c.Assert(err, qt.IsNil) + resp, err := http.DefaultClient.Do(req) + c.Assert(err, qt.IsNil) + defer resp.Body.Close() + c.Assert(resp.StatusCode, qt.Equals, 404) + + // test endpoints in capabilities are found + for _, cap := range rebac_admin.Capabilities { + for _, m := range cap.Methods { + c.Run(fmt.Sprintf("%s %s", m, cap.Endpoint), func(c *qt.C) { + url := fmt.Sprintf("%s/v1%s", testServer.URL, cap.Endpoint) + req, err := http.NewRequest(string(m), url, nil) + c.Assert(err, qt.IsNil) + resp, err := http.DefaultClient.Do(req) + c.Assert(err, qt.IsNil) + defer resp.Body.Close() + // 404 is for not found endpoints and 501 is for "not implemented" endpoints in the rebac-admin-ui-handlers library + isNotFound := resp.StatusCode == 404 || resp.StatusCode == 501 + c.Assert(isNotFound, qt.IsFalse) + }) + + } + } + +} diff --git a/internal/rebac_admin/export_test.go b/internal/rebac_admin/export_test.go index b59df4943..3f53ea4f3 100644 --- a/internal/rebac_admin/export_test.go +++ b/internal/rebac_admin/export_test.go @@ -5,6 +5,7 @@ var ( NewGroupService = newGroupService NewidentitiesService = newidentitiesService NewResourcesService = newResourcesService + Capabilities = capabilities ) type GroupsService = groupsService diff --git a/internal/rebac_admin/identities.go b/internal/rebac_admin/identities.go index 7751a9d67..479c9b6a3 100644 --- a/internal/rebac_admin/identities.go +++ b/internal/rebac_admin/identities.go @@ -13,6 +13,7 @@ import ( "go.uber.org/zap" "github.com/canonical/jimm/v3/internal/common/pagination" + "github.com/canonical/jimm/v3/internal/errors" "github.com/canonical/jimm/v3/internal/jujuapi" "github.com/canonical/jimm/v3/internal/openfga" ofganames "github.com/canonical/jimm/v3/internal/openfga/names" @@ -74,7 +75,10 @@ func (s *identitiesService) CreateIdentity(ctx context.Context, identity *resour func (s *identitiesService) GetIdentity(ctx context.Context, identityId string) (*resources.Identity, error) { user, err := s.jimm.FetchIdentity(ctx, identityId) if err != nil { - return nil, v1.NewNotFoundError(fmt.Sprintf("User with id %s not found", identityId)) + if errors.ErrorCode(err) == errors.CodeNotFound { + return nil, v1.NewNotFoundError(fmt.Sprintf("User with id %s not found", identityId)) + } + return nil, err } identity := utils.FromUserToIdentity(*user) return &identity, nil @@ -190,7 +194,10 @@ func (s *identitiesService) GetIdentityEntitlements(ctx context.Context, identit } objUser, err := s.jimm.FetchIdentity(ctx, identityId) if err != nil { - return nil, v1.NewNotFoundError(fmt.Sprintf("User with id %s not found", identityId)) + if errors.ErrorCode(err) == errors.CodeNotFound { + return nil, v1.NewNotFoundError(fmt.Sprintf("User with id %s not found", identityId)) + } + return nil, err } filter := utils.CreateTokenPaginationFilter(params.Size, params.NextToken, params.NextPageToken) diff --git a/internal/rebac_admin/identities_test.go b/internal/rebac_admin/identities_test.go index d039b18f1..f90dec19e 100644 --- a/internal/rebac_admin/identities_test.go +++ b/internal/rebac_admin/identities_test.go @@ -15,6 +15,7 @@ import ( "github.com/canonical/jimm/v3/internal/common/pagination" "github.com/canonical/jimm/v3/internal/common/utils" "github.com/canonical/jimm/v3/internal/dbmodel" + jimmm_errors "github.com/canonical/jimm/v3/internal/errors" "github.com/canonical/jimm/v3/internal/jimmtest" "github.com/canonical/jimm/v3/internal/jimmtest/mocks" "github.com/canonical/jimm/v3/internal/openfga" @@ -29,7 +30,7 @@ func TestGetIdentity(t *testing.T) { if username == "bob@canonical.com" { return openfga.NewUser(&dbmodel.Identity{Name: "bob@canonical.com"}, nil), nil } - return nil, dbmodel.IdentityCreationError + return nil, jimmm_errors.E(jimmm_errors.CodeNotFound) }, } user := openfga.User{} From 8c57ffe9bae7745cfb1ef7aa4f91ff8f9bda5aa0 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 12 Sep 2024 08:55:03 +0100 Subject: [PATCH 06/10] Upgrade `rebac-admin-ui-handlers` to `v0.1.1` (#1362) Signed-off-by: Babak K. Shandiz --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index a8ffed17c..e2dbc9fac 100644 --- a/go.mod +++ b/go.mod @@ -45,7 +45,7 @@ require ( require ( github.com/antonlindstrom/pgstore v0.0.0-20220421113606-e3a6e3fed12a github.com/canonical/ofga v0.10.0 - github.com/canonical/rebac-admin-ui-handlers v0.1.0 + github.com/canonical/rebac-admin-ui-handlers v0.1.1 github.com/coreos/go-oidc/v3 v3.9.0 github.com/dustinkirkland/golang-petname v0.0.0-20231002161417-6a283f1aaaf2 github.com/go-chi/chi/v5 v5.0.12 diff --git a/go.sum b/go.sum index 74face0b1..8d8a65842 100644 --- a/go.sum +++ b/go.sum @@ -156,6 +156,8 @@ github.com/canonical/ofga v0.10.0 h1:DHXhG/DAXWWQT/I+2jzr4qm0uTIYrILmtMxd6ZqmEzE github.com/canonical/ofga v0.10.0/go.mod h1:u4Ou8dbIhO7FmVlT7W3rX2roD9AOGz/CqmGh7AdF0Lo= github.com/canonical/rebac-admin-ui-handlers v0.1.0 h1:Bef1N/RgQine8hHX4ZMksQz/1VKsy4DHK2XdhAzQsZs= github.com/canonical/rebac-admin-ui-handlers v0.1.0/go.mod h1:EIdBoaTHWYPkzNeUeXUBueJkglN9nQz5HLIvaOT7o1k= +github.com/canonical/rebac-admin-ui-handlers v0.1.1 h1:rjsb45diShhwD/uUFpai6gmhFUzT+jTdsnEWcOvcKx4= +github.com/canonical/rebac-admin-ui-handlers v0.1.1/go.mod h1:EIdBoaTHWYPkzNeUeXUBueJkglN9nQz5HLIvaOT7o1k= github.com/cenkalti/backoff/v3 v3.0.0/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs= github.com/cenkalti/backoff/v3 v3.2.2 h1:cfUAAO3yvKMYKPrvhDuHSwQnhZNk/RMHKdZqKTxfm6M= github.com/cenkalti/backoff/v3 v3.2.2/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs= From b4cd3d501b5b706dd4745f3342dc30c1912563fc Mon Sep 17 00:00:00 2001 From: pkulik0 <70904851+pkulik0@users.noreply.github.com> Date: Thu, 12 Sep 2024 13:07:04 +0000 Subject: [PATCH 07/10] Add GetGroup to api client --- pkg/api/client.go | 7 +++++++ pkg/api/params/params.go | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/pkg/api/client.go b/pkg/api/client.go index 75b002e78..80765209c 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -123,6 +123,13 @@ func (c *Client) AddGroup(req *params.AddGroupRequest) (params.AddGroupResponse, return resp, err } +// GetGroup returns the group with the given UUID. +func (c *Client) GetGroup(req *params.GetGroupRequest) (params.Group, error) { + var resp params.GetGroupResponse + err := c.caller.APICall("JIMM", 4, "", "GetGroup", req, &resp) + return resp.Group, err +} + // RenameGroup renames a group in JIMM. func (c *Client) RenameGroup(req *params.RenameGroupRequest) error { return c.caller.APICall("JIMM", 4, "", "RenameGroup", req, nil) diff --git a/pkg/api/params/params.go b/pkg/api/params/params.go index 7098f64a4..60aa855bd 100644 --- a/pkg/api/params/params.go +++ b/pkg/api/params/params.go @@ -285,6 +285,17 @@ type AddGroupResponse struct { Group } +// GetGroupRequest holds a request to get a group. +type GetGroupRequest struct { + // UUID holds the UUID of the group to be retrieved. + UUID string `json:"uuid"` +} + +// GetGroupResponse holds the details of the group. +type GetGroupResponse struct { + Group +} + // RenameGroupRequest holds a request to rename a group. type RenameGroupRequest struct { // Name holds the name of the group. From 4db98984dba4a29d3953299fc168a2745e606086 Mon Sep 17 00:00:00 2001 From: pkulik0 <70904851+pkulik0@users.noreply.github.com> Date: Thu, 12 Sep 2024 13:26:47 +0000 Subject: [PATCH 08/10] Implement GetGroup jimm facade --- internal/jujuapi/access_control.go | 17 +++++++++++++++++ internal/jujuapi/access_control_test.go | 17 +++++++++++++++++ internal/jujuapi/jimm.go | 2 ++ pkg/api/client.go | 4 ++-- 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/internal/jujuapi/access_control.go b/internal/jujuapi/access_control.go index b4de5f247..2f7236e52 100644 --- a/internal/jujuapi/access_control.go +++ b/internal/jujuapi/access_control.go @@ -58,6 +58,23 @@ func (r *controllerRoot) AddGroup(ctx context.Context, req apiparams.AddGroupReq return resp, nil } +// GetGroup retrieves a group within JIMMs DB for reference by OpenFGA. +func (r *controllerRoot) GetGroup(ctx context.Context, req apiparams.GetGroupRequest) (apiparams.Group, error) { + const op = errors.Op("jujuapi.GetGroup") + + groupEntry, err := r.jimm.GetGroupByID(ctx, r.user, req.UUID) + if err != nil { + zapctx.Error(ctx, "failed to get group", zaputil.Error(err)) + return apiparams.Group{}, errors.E(op, err) + } + return apiparams.Group{ + UUID: groupEntry.UUID, + Name: groupEntry.Name, + CreatedAt: groupEntry.CreatedAt.Format(time.RFC3339), + UpdatedAt: groupEntry.UpdatedAt.Format(time.RFC3339), + }, nil +} + // RenameGroup renames a group within JIMMs DB for reference by OpenFGA. func (r *controllerRoot) RenameGroup(ctx context.Context, req apiparams.RenameGroupRequest) error { const op = errors.Op("jujuapi.RenameGroup") diff --git a/internal/jujuapi/access_control_test.go b/internal/jujuapi/access_control_test.go index e1230590b..544a97fed 100644 --- a/internal/jujuapi/access_control_test.go +++ b/internal/jujuapi/access_control_test.go @@ -61,6 +61,23 @@ func (s *accessControlSuite) TestAddGroup(c *gc.C) { c.Assert(err, gc.ErrorMatches, ".*already exists.*") } +func (s *accessControlSuite) TestGetGroup(c *gc.C) { + conn := s.open(c, nil, "alice") + defer conn.Close() + + client := api.NewClient(conn) + + created, err := client.AddGroup(&apiparams.AddGroupRequest{Name: "test-group"}) + c.Assert(err, jc.ErrorIsNil) + + retrieved, err := client.GetGroup(&apiparams.GetGroupRequest{UUID: created.UUID}) + c.Assert(err, jc.ErrorIsNil) + c.Assert(retrieved.Group, gc.DeepEquals, created.Group) + + _, err = client.GetGroup(&apiparams.GetGroupRequest{UUID: "non-existent"}) + c.Assert(err, gc.ErrorMatches, ".*not found.*") +} + func (s *accessControlSuite) TestRemoveGroup(c *gc.C) { conn := s.open(c, nil, "alice") defer conn.Close() diff --git a/internal/jujuapi/jimm.go b/internal/jujuapi/jimm.go index f52551eea..1fad49918 100644 --- a/internal/jujuapi/jimm.go +++ b/internal/jujuapi/jimm.go @@ -41,6 +41,7 @@ func init() { addCloudToControllerMethod := rpc.Method(r.AddCloudToController) removeCloudFromControllerMethod := rpc.Method(r.RemoveCloudFromController) addGroupMethod := rpc.Method(r.AddGroup) + getGroupMethod := rpc.Method(r.GetGroup) renameGroupMethod := rpc.Method(r.RenameGroup) removeGroupMethod := rpc.Method(r.RemoveGroup) listGroupsMethod := rpc.Method(r.ListGroups) @@ -76,6 +77,7 @@ func init() { r.AddMethod("JIMM", 4, "MigrateModel", migrateModel) // JIMM ReBAC RPC r.AddMethod("JIMM", 4, "AddGroup", addGroupMethod) + r.AddMethod("JIMM", 4, "GetGroup", getGroupMethod) r.AddMethod("JIMM", 4, "RenameGroup", renameGroupMethod) r.AddMethod("JIMM", 4, "RemoveGroup", removeGroupMethod) r.AddMethod("JIMM", 4, "ListGroups", listGroupsMethod) diff --git a/pkg/api/client.go b/pkg/api/client.go index 80765209c..79307dc9a 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -124,10 +124,10 @@ func (c *Client) AddGroup(req *params.AddGroupRequest) (params.AddGroupResponse, } // GetGroup returns the group with the given UUID. -func (c *Client) GetGroup(req *params.GetGroupRequest) (params.Group, error) { +func (c *Client) GetGroup(req *params.GetGroupRequest) (params.GetGroupResponse, error) { var resp params.GetGroupResponse err := c.caller.APICall("JIMM", 4, "", "GetGroup", req, &resp) - return resp.Group, err + return resp, err } // RenameGroup renames a group in JIMM. From 16883f89aaa2e6758d8544f6820edf3dac9149f1 Mon Sep 17 00:00:00 2001 From: Piotr Kulik <70904851+pkulik0@users.noreply.github.com> Date: Thu, 12 Sep 2024 08:39:18 -0500 Subject: [PATCH 09/10] Update GetGroup docstring Co-authored-by: Kian Parvin <46668016+kian99@users.noreply.github.com> --- internal/jujuapi/access_control.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/jujuapi/access_control.go b/internal/jujuapi/access_control.go index 2f7236e52..8218734a6 100644 --- a/internal/jujuapi/access_control.go +++ b/internal/jujuapi/access_control.go @@ -58,7 +58,7 @@ func (r *controllerRoot) AddGroup(ctx context.Context, req apiparams.AddGroupReq return resp, nil } -// GetGroup retrieves a group within JIMMs DB for reference by OpenFGA. +// GetGroup returns group information based on a group ID. func (r *controllerRoot) GetGroup(ctx context.Context, req apiparams.GetGroupRequest) (apiparams.Group, error) { const op = errors.Op("jujuapi.GetGroup") From 6ec6f1918cc44240d4b19c9097f28bd0960ca344 Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Mon, 16 Sep 2024 15:48:10 +0200 Subject: [PATCH 10/10] Update channel used for setting up LXD to LTS channel (#1365) --- .github/actions/test-server/action.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/test-server/action.yaml b/.github/actions/test-server/action.yaml index 6878542b9..c789b2524 100644 --- a/.github/actions/test-server/action.yaml +++ b/.github/actions/test-server/action.yaml @@ -89,7 +89,7 @@ runs: uses: charmed-kubernetes/actions-operator@main with: provider: "lxd" - channel: "5.19/stable" + channel: "5.21/stable" juju-channel: ${{ inputs.juju-channel }} bootstrap-options: "--config ${{ github.action_path }}/../../../cloudinit.temp.yaml --config login-token-refresh-url=https://jimm.localhost/.well-known/jwks.json"