Skip to content

Commit

Permalink
Css 10530/filterning resources (#1353)
Browse files Browse the repository at this point in the history
* add filter by name and entity with tests
  • Loading branch information
SimoneDutto authored Sep 6, 2024
1 parent f423bbd commit 4d88a29
Show file tree
Hide file tree
Showing 12 changed files with 472 additions and 109 deletions.
4 changes: 4 additions & 0 deletions internal/common/utils/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ package utils
func IntToPointer(i int) *int {
return &i
}

func StringToPointer(s string) *string {
return &s
}
175 changes: 119 additions & 56 deletions internal/db/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?;
Expand All @@ -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)
Expand All @@ -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
}
Expand All @@ -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
}
103 changes: 97 additions & 6 deletions internal/db/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("[email protected]")
c.Assert(err, qt.IsNil)
c.Assert(database.DB.Create(&u).Error, qt.IsNil)
Expand Down Expand Up @@ -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":
Expand All @@ -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])
}
})
}
}
4 changes: 2 additions & 2 deletions internal/jimm/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion internal/jimm/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions internal/jimmtest/jimm_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 4d88a29

Please sign in to comment.