Skip to content

Commit

Permalink
feat(appeal): add query filters for ListAppeals and ListUserAppeals (#86
Browse files Browse the repository at this point in the history 
)

* feat(appeal): populate resource in list appeals

* feat(appeal): filter appeals by statuses and role

* feat(cmd): add filter flags for list appeals

* feat(appeal): add filter by resource properties

* chore: preload appeal's resource in list user approvals
  • Loading branch information
@rahmatrhd
rahmatrhd authored Dec 6, 2021
1 parent d12b161 commit 189ac12
Show file tree
Hide file tree
Showing 7 changed files with 854 additions and 595 deletions.
36 changes: 36 additions & 0 deletions api/handler/v1beta1/grpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,24 @@ func (s *GRPCServer) ListUserAppeals(ctx context.Context, req *guardianv1beta1.L
filters := map[string]interface{}{
"account_id": user,
}
if req.GetStatuses() != nil {
filters["statuses"] = req.GetStatuses()
}
if req.GetRole() != "" {
filters["role"] = req.GetRole()
}
if req.GetProviderTypes() != nil {
filters["provider_types"] = req.GetProviderTypes()
}
if req.GetProviderUrns() != nil {
filters["provider_urns"] = req.GetProviderUrns()
}
if req.GetResourceTypes() != nil {
filters["resource_types"] = req.GetResourceTypes()
}
if req.GetResourceUrns() != nil {
filters["resource_urns"] = req.GetResourceUrns()
}
appeals, err := s.listAppeals(filters)
if err != nil {
return nil, err
Expand All @@ -377,6 +395,24 @@ func (s *GRPCServer) ListAppeals(ctx context.Context, req *guardianv1beta1.ListA
if req.GetAccountId() != "" {
filters["account_id"] = req.GetAccountId()
}
if req.GetStatuses() != nil {
filters["statuses"] = req.GetStatuses()
}
if req.GetRole() != "" {
filters["role"] = req.GetRole()
}
if req.GetProviderTypes() != nil {
filters["provider_types"] = req.GetProviderTypes()
}
if req.GetProviderUrns() != nil {
filters["provider_urns"] = req.GetProviderUrns()
}
if req.GetResourceTypes() != nil {
filters["resource_types"] = req.GetResourceTypes()
}
if req.GetResourceUrns() != nil {
filters["resource_urns"] = req.GetResourceUrns()
}
appeals, err := s.listAppeals(filters)
if err != nil {
return nil, err
Expand Down
1,276 changes: 699 additions & 577 deletions api/proto/odpf/guardian/v1beta1/guardian.pb.go
View file

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions api/proto/odpf/guardian/v1beta1/guardian.pb.gw.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 19 additions & 1 deletion appeal/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ type findFilters struct {
Statuses []string `mapstructure:"statuses" validate:"omitempty,min=1"`
ExpirationDateLessThan time.Time `mapstructure:"expiration_date_lt" validate:"omitempty,required"`
ExpirationDateGreaterThan time.Time `mapstructure:"expiration_date_gt" validate:"omitempty,required"`
ProviderTypes []string `mapstructure:"provider_types" validate:"omitempty,min=1"`
ProviderURNs []string `mapstructure:"provider_urns" validate:"omitempty,min=1"`
ResourceTypes []string `mapstructure:"resource_types" validate:"omitempty,min=1"`
ResourceURNs []string `mapstructure:"resource_urns" validate:"omitempty,min=1"`
}

// Repository talks to the store to read or insert data
Expand Down Expand Up @@ -85,8 +89,22 @@ func (r *Repository) Find(filters map[string]interface{}) ([]*domain.Appeal, err
db = db.Where(`"options" -> 'expiration_date' > ?`, conditions.ExpirationDateGreaterThan)
}

db = db.Joins("Resource")
if conditions.ProviderTypes != nil {
db = db.Where(`"Resource"."provider_type" IN ?`, conditions.ProviderTypes)
}
if conditions.ProviderURNs != nil {
db = db.Where(`"Resource"."provider_urn" IN ?`, conditions.ProviderURNs)
}
if conditions.ResourceTypes != nil {
db = db.Where(`"Resource"."type" IN ?`, conditions.ResourceTypes)
}
if conditions.ResourceURNs != nil {
db = db.Where(`"Resource"."urn" IN ?`, conditions.ResourceURNs)
}

var models []*model.Appeal
if err := db.Debug().Find(&models).Error; err != nil {
if err := db.Find(&models).Error; err != nil {
return nil, err
}

Expand Down
72 changes: 58 additions & 14 deletions appeal/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"database/sql"
"database/sql/driver"
"errors"
"fmt"
"regexp"
"testing"
"time"
Expand All @@ -26,6 +27,7 @@ type RepositoryTestSuite struct {
columnNames []string
approvalColumnNames []string
approverColumnNames []string
resourceColumnNames []string
}

func (s *RepositoryTestSuite) SetupTest() {
Expand Down Expand Up @@ -69,6 +71,17 @@ func (s *RepositoryTestSuite) SetupTest() {
"created_at",
"updated_at",
}
s.resourceColumnNames = []string{
"id",
"provider_type",
"provider_urn",
"type",
"urn",
"details",
"labels",
"created_at",
"updated_at",
}
}

func (s *RepositoryTestSuite) TearDownTest() {
Expand Down Expand Up @@ -232,6 +245,7 @@ func (s *RepositoryTestSuite) TestFind() {
})

s.Run("should run query based on filters", func() {
selectAppealsJoinsWithResourceSql := `SELECT "appeals"."id","appeals"."resource_id","appeals"."policy_id","appeals"."policy_version","appeals"."status","appeals"."account_id","appeals"."account_type","appeals"."created_by","appeals"."creator","appeals"."role","appeals"."options","appeals"."labels","appeals"."details","appeals"."revoked_by","appeals"."revoked_at","appeals"."revoke_reason","appeals"."created_at","appeals"."updated_at","appeals"."deleted_at","Resource"."id" AS "Resource__id","Resource"."provider_type" AS "Resource__provider_type","Resource"."provider_urn" AS "Resource__provider_urn","Resource"."type" AS "Resource__type","Resource"."urn" AS "Resource__urn","Resource"."name" AS "Resource__name","Resource"."details" AS "Resource__details","Resource"."labels" AS "Resource__labels","Resource"."created_at" AS "Resource__created_at","Resource"."updated_at" AS "Resource__updated_at","Resource"."deleted_at" AS "Resource__deleted_at","Resource"."is_deleted" AS "Resource__is_deleted" FROM "appeals" LEFT JOIN "resources" "Resource" ON "appeals"."resource_id" = "Resource"."id"`
timeNow := time.Now()
testCases := []struct {
filters map[string]interface{}
Expand All @@ -240,41 +254,41 @@ func (s *RepositoryTestSuite) TestFind() {
}{
{
filters: map[string]interface{}{},
expectedQuery: regexp.QuoteMeta(`SELECT * FROM "appeals" WHERE "appeals"."deleted_at" IS NULL`),
expectedQuery: regexp.QuoteMeta(selectAppealsJoinsWithResourceSql + ` WHERE "appeals"."deleted_at" IS NULL`),
},
{
filters: map[string]interface{}{
"account_id": "[email protected]",
},
expectedQuery: regexp.QuoteMeta(`SELECT * FROM "appeals" WHERE "account_id" = $1 AND "appeals"."deleted_at" IS NULL`),
expectedQuery: regexp.QuoteMeta(selectAppealsJoinsWithResourceSql + ` WHERE "account_id" = $1 AND "appeals"."deleted_at" IS NULL`),
expectedArgs: []driver.Value{"[email protected]"},
},
{
filters: map[string]interface{}{
"statuses": []string{domain.AppealStatusActive, domain.AppealStatusTerminated},
},
expectedQuery: regexp.QuoteMeta(`SELECT * FROM "appeals" WHERE "status" IN ($1,$2) AND "appeals"."deleted_at" IS NULL`),
expectedQuery: regexp.QuoteMeta(selectAppealsJoinsWithResourceSql + ` WHERE "status" IN ($1,$2) AND "appeals"."deleted_at" IS NULL`),
expectedArgs: []driver.Value{domain.AppealStatusActive, domain.AppealStatusTerminated},
},
{
filters: map[string]interface{}{
"resource_id": uint(1),
},
expectedQuery: regexp.QuoteMeta(`SELECT * FROM "appeals" WHERE "resource_id" = $1 AND "appeals"."deleted_at" IS NULL`),
expectedQuery: regexp.QuoteMeta(selectAppealsJoinsWithResourceSql + ` WHERE "resource_id" = $1 AND "appeals"."deleted_at" IS NULL`),
expectedArgs: []driver.Value{uint(1)},
},
{
filters: map[string]interface{}{
"role": "test-role",
},
expectedQuery: regexp.QuoteMeta(`SELECT * FROM "appeals" WHERE "role" = $1 AND "appeals"."deleted_at" IS NULL`),
expectedQuery: regexp.QuoteMeta(selectAppealsJoinsWithResourceSql + ` WHERE "role" = $1 AND "appeals"."deleted_at" IS NULL`),
expectedArgs: []driver.Value{"test-role"},
},
{
filters: map[string]interface{}{
"expiration_date_lt": timeNow,
},
expectedQuery: regexp.QuoteMeta(`SELECT * FROM "appeals" WHERE "options" -> 'expiration_date' < $1 AND "appeals"."deleted_at" IS NULL`),
expectedQuery: regexp.QuoteMeta(selectAppealsJoinsWithResourceSql + ` WHERE "options" -> 'expiration_date' < $1 AND "appeals"."deleted_at" IS NULL`),
expectedArgs: []driver.Value{timeNow},
},
}
Expand All @@ -292,31 +306,50 @@ func (s *RepositoryTestSuite) TestFind() {
})

s.Run("should return records on success", func() {
expectedQuery := regexp.QuoteMeta(`SELECT * FROM "appeals" WHERE "appeals"."deleted_at" IS NULL`)
expectedQuery := regexp.QuoteMeta(`SELECT "appeals"."id","appeals"."resource_id","appeals"."policy_id","appeals"."policy_version","appeals"."status","appeals"."account_id","appeals"."account_type","appeals"."created_by","appeals"."creator","appeals"."role","appeals"."options","appeals"."labels","appeals"."details","appeals"."revoked_by","appeals"."revoked_at","appeals"."revoke_reason","appeals"."created_at","appeals"."updated_at","appeals"."deleted_at","Resource"."id" AS "Resource__id","Resource"."provider_type" AS "Resource__provider_type","Resource"."provider_urn" AS "Resource__provider_urn","Resource"."type" AS "Resource__type","Resource"."urn" AS "Resource__urn","Resource"."name" AS "Resource__name","Resource"."details" AS "Resource__details","Resource"."labels" AS "Resource__labels","Resource"."created_at" AS "Resource__created_at","Resource"."updated_at" AS "Resource__updated_at","Resource"."deleted_at" AS "Resource__deleted_at","Resource"."is_deleted" AS "Resource__is_deleted" FROM "appeals" LEFT JOIN "resources" "Resource" ON "appeals"."resource_id" = "Resource"."id" WHERE "appeals"."deleted_at" IS NULL`)
expectedFilters := map[string]interface{}{}
expectedRecords := []*domain.Appeal{
{
ID: 1,
ResourceID: 1,
ID: 1,
ResourceID: 1,
Resource: &domain.Resource{
ID: 1,
ProviderType: "provider_type",
ProviderURN: "provider_urn",
Type: "resource_type",
URN: "resource_urn",
},
PolicyID: "policy_1",
PolicyVersion: 1,
Status: domain.AppealStatusPending,
AccountID: "[email protected]",
Role: "role_name",
},
{
ID: 2,
ResourceID: 2,
ID: 2,
ResourceID: 2,
Resource: &domain.Resource{
ID: 2,
ProviderType: "provider_type",
ProviderURN: "provider_urn",
Type: "resource_type",
URN: "resource_urn",
},
PolicyID: "policy_1",
PolicyVersion: 1,
Status: domain.AppealStatusPending,
AccountID: "[email protected]",
Role: "role_name",
},
}
expecterRows := sqlmock.NewRows(s.columnNames)
aggregatedColumns := s.columnNames
for _, c := range s.resourceColumnNames {
aggregatedColumns = append(aggregatedColumns, fmt.Sprintf("Resource__%s", c))
}
expectedRows := sqlmock.NewRows(aggregatedColumns)
for _, r := range expectedRecords {
expecterRows.AddRow(
expectedRows.AddRow(
// appeal
r.ID,
r.ResourceID,
r.PolicyID,
Expand All @@ -332,11 +365,22 @@ func (s *RepositoryTestSuite) TestFind() {
"null",
r.CreatedAt,
r.UpdatedAt,

// resource
r.Resource.ID,
r.Resource.ProviderType,
r.Resource.ProviderURN,
r.Resource.Type,
r.Resource.URN,
"null",
"null",
r.Resource.CreatedAt,
r.Resource.UpdatedAt,
)
}
s.dbmock.
ExpectQuery(expectedQuery).
WillReturnRows(expecterRows)
WillReturnRows(expectedRows)

actualRecords, actualError := s.repository.Find(expectedFilters)

Expand Down
3 changes: 2 additions & 1 deletion approval/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ func (r *repository) ListApprovals(conditions *domain.ListApprovalsFilter) ([]*d
approvalIDs = append(approvalIDs, a.ApprovalID)
}

db = r.db.Joins("Appeal")
db = r.db.Preload("Appeal.Resource")
db = db.Joins("Appeal")
if conditions.Statuses != nil {
db = db.Where(`"approvals"."status" IN ?`, conditions.Statuses)
}
Expand Down
24 changes: 22 additions & 2 deletions cmd/appeals.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"os"

"github.com/MakeNowJust/heredoc"
guardianv1beta1 "github.com/odpf/guardian/api/proto/odpf/guardian/v1beta1"
"github.com/odpf/guardian/app"
"github.com/odpf/salt/printer"
Expand All @@ -29,9 +30,18 @@ func appealsCommand(c *app.CLIConfig) *cobra.Command {
}

func listAppealsCommand(c *app.CLIConfig) *cobra.Command {
return &cobra.Command{
var statuses []string
var role string
var accountID string

cmd := &cobra.Command{
Use: "list",
Short: "list appeals",
Example: heredoc.Doc(`
$ guardian appeals list
$ guardian appeals list --status=pending
$ guardian appeals list --role=viewer
`),
RunE: func(cmd *cobra.Command, args []string) error {
cs := term.NewColorScheme()

Expand All @@ -42,7 +52,11 @@ func listAppealsCommand(c *app.CLIConfig) *cobra.Command {
}
defer cancel()

res, err := client.ListAppeals(ctx, &guardianv1beta1.ListAppealsRequest{})
res, err := client.ListAppeals(ctx, &guardianv1beta1.ListAppealsRequest{
Statuses: statuses,
Role: role,
AccountId: accountID,
})
if err != nil {
return err
}
Expand All @@ -66,6 +80,12 @@ func listAppealsCommand(c *app.CLIConfig) *cobra.Command {
return nil
},
}

cmd.Flags().StringArrayVar(&statuses, "status", nil, "Filter appeals by status(es)")
cmd.Flags().StringVar(&role, "role", "", "Filter appeals by role")
cmd.Flags().StringVar(&accountID, "account-id", "", "Filter appeals by account_id")

return cmd
}

func createAppealCommand(c *app.CLIConfig) *cobra.Command {
Expand Down

0 comments on commit 189ac12

Please sign in to comment.