pr comments

internal/db/resource.go

@@ -13,7 +13,7 @@ import (
 )
 
 const ApplicationOffersQueryKey = "application_offers"
-const SELECT_APPLICATION_OFFERS = `
+const selectApplicationOffers = `
 'application_offer' AS type, 
 application_offers.uuid AS id, 
 application_offers.name AS name, 
@@ -23,7 +23,7 @@ models.name AS parent_name,
 `
 
 const CloudsQueryKey = "clouds"
-const SELECT_CLOUDS = `
+const selectClouds = `
 'cloud' AS type, 
 clouds.name AS id, 
 clouds.name AS name, 
@@ -33,7 +33,7 @@ clouds.name AS name,
 `
 
 const ControllersQueryKey = "controllers"
-const SELECT_CONTROLLERS = `
+const selectControllers = `
 'controller' AS type, 
 controllers.uuid AS id, 
 controllers.name AS name, 
@@ -43,7 +43,7 @@ controllers.name AS name,
 `
 
 const ModelsQueryKey = "models"
-const SELECT_MODELS = `
+const selectModels = ` 
 'model' AS type, 
 models.uuid AS id, 
 models.name AS name, 
@@ -53,7 +53,7 @@ controllers.name AS parent_name,
 `
 
 const ServiceAccountQueryKey = "identities"
-const SELECT_IDENTITIES = `
+const selectIdentities = `
 'service_account' AS type, 
 identities.name AS id, 
 identities.name AS name, 
@@ -62,6 +62,13 @@ identities.name AS name,
 '' AS parent_type
 `
 
+const unionQuery = `
+? UNION ? UNION ? UNION ? UNION ?
+ORDER BY type, id
+OFFSET ?
+LIMIT  ?;
+`
+
 type Resource struct {
 	Type       string
 	ID         sql.NullString
@@ -73,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, nameFilter, typeFilter string) (_ []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)
@@ -84,7 +91,7 @@ func (d *Database) ListResources(ctx context.Context, limit, offset int, nameFil
 	defer servermon.ErrorCounter(servermon.DBQueryErrorCount, &err, string(op))
 
 	db := d.DB.WithContext(ctx)
-	query, err := buildQuery(db, offset, limit, nameFilter, typeFilter)
+	query, err := buildQuery(db, offset, limit, namePrefixFilter, typeFilter)
 	if err != nil {
 		return nil, err
 	}
@@ -105,52 +112,40 @@ func (d *Database) ListResources(ctx context.Context, limit, offset int, nameFil
 	return resources, nil
 }
 
-// buildQuery is an utility function to build the database query according to two optional parameters.
-// nameFilter: used to match resources name.
-// 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, nameFilter, typeFilter string) (*gorm.DB, error) {
-	query := `
-	? UNION ? UNION ? UNION ? UNION ?
-	ORDER BY type, id
-	OFFSET ?
-	LIMIT  ?;
-	`
-	applicationOffersQuery := db.Select(SELECT_APPLICATION_OFFERS).
+// 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) {
+	// if namePrefixEmpty set to true we have a 'TRUE IS TRUE' in the SQL WHERE statement, which disable the filtering.
+	namePrefixEmpty := namePrefixFilter == ""
+	namePrefixFilter = namePrefixFilter + "%"
+
+	applicationOffersQuery := db.Select(selectApplicationOffers).
 		Model(&dbmodel.ApplicationOffer{}).
+		Where("? IS TRUE OR application_offers.name LIKE ?", namePrefixEmpty, namePrefixFilter).
 		Joins("JOIN models ON application_offers.model_id = models.id")
 
-	cloudsQuery := db.Select(SELECT_CLOUDS).
-		Model(&dbmodel.Cloud{})
+	cloudsQuery := db.Select(selectClouds).
+		Model(&dbmodel.Cloud{}).
+		Where("? IS TRUE OR clouds.name LIKE ?", namePrefixEmpty, namePrefixFilter)
 
-	controllersQuery := db.Select(SELECT_CONTROLLERS).
-		Model(&dbmodel.Controller{})
+	controllersQuery := db.Select(selectControllers).
+		Model(&dbmodel.Controller{}).
+		Where("? IS TRUE OR controllers.name LIKE ?", namePrefixEmpty, namePrefixFilter)
 
-	modelsQuery := db.Select(SELECT_MODELS).
+	modelsQuery := db.Select(selectModels).
 		Model(&dbmodel.Model{}).
+		Where("? IS TRUE OR models.name LIKE ?", namePrefixEmpty, namePrefixFilter).
 		Joins("JOIN controllers ON models.controller_id = controllers.id")
 
-	serviceAccountsQuery := db.Select(SELECT_IDENTITIES).
+	serviceAccountsQuery := db.Select(selectIdentities).
 		Model(&dbmodel.Identity{}).
-		Where("name LIKE '%@serviceaccount'")
-
-	queries := map[string]*gorm.DB{
-		ApplicationOffersQueryKey: applicationOffersQuery,
-		CloudsQueryKey:            cloudsQuery,
-		ControllersQueryKey:       controllersQuery,
-		ModelsQueryKey:            modelsQuery,
-		ServiceAccountQueryKey:    serviceAccountsQuery,
-	}
-	// we add the where clause only if the nameFilter is filled
-	if nameFilter != "" {
-		nameFilter = "%" + nameFilter + "%"
-		for k := range queries {
-			queries[k] = queries[k].Where(k+".name LIKE ?", nameFilter)
-		}
-	}
+		Where("name LIKE '%@serviceaccount' AND (? IS TRUE OR identities.name LIKE ?)", namePrefixEmpty, namePrefixFilter)
+
 	// if the typeFilter is set we only return the query for that specif entityType, otherwise the union.
 	if typeFilter == "" {
 		return db.
-			Raw(query,
+			Raw(unionQuery,
 				applicationOffersQuery,
 				cloudsQuery,
 				controllersQuery,
@@ -159,13 +154,22 @@ func buildQuery(db *gorm.DB, offset, limit int, nameFilter, typeFilter string) (
 				offset,
 				limit,
 			), nil
-	} else {
-		query, ok := queries[typeFilter]
-		if !ok {
-			// 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("name").Offset(offset).Limit(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
 }

internal/db/resource_test.go

@@ -128,14 +128,23 @@ func (s *dbSuite) TestGetResourcesWithNameTypeFilter(c *qt.C) {
 			expectedUUIDs: []string{model.UUID.String},
 		},
 		{
-			description:   "filter name test",
+			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",

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, nameFilter, typeFilter string) ([]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(), nameFilter, typeFilter)
+	return j.Database.ListResources(ctx, filter.Limit(), filter.Offset(), namePrefixFilter, typeFilter)
 }

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, nameFilter, typeFilter string) ([]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, nameFilter, typeFilter string) ([]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, nameFilter, typeFilter)
+	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 {

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, nameFilter, typeFilter string) ([]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)

internal/rebac_admin/resources.go

@@ -32,15 +32,15 @@ func (s *resourcesService) ListResources(ctx context.Context, params *resources.
 		return nil, err
 	}
 	currentPage, expectedPageSize, pagination := pagination.CreatePaginationWithoutTotal(params.Size, params.Page)
-	nameFilter, typeFilter := utils.GetNameAndTypeResourceFilter(params.EntityName, params.EntityType)
+	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, nameFilter, typeFilter)
+	res, err := s.jimm.ListResources(ctx, user, pagination, namePrefixFilter, typeFilter)
 	if err != nil {
 		return nil, err
 	}

internal/rebac_admin/resources_integration_test.go

@@ -149,7 +149,7 @@ func (s *resourcesSuite) TestListResources(c *gc.C) {
 			ids: func() []testEntity {
 				filteredIds := make([]testEntity, 0)
 				for _, id := range ids {
-					if strings.Contains(id.Name, "model") {
+					if strings.HasPrefix(id.Name, "model") {
 						filteredIds = append(filteredIds, id)
 					}
 				}