Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements and bug fixes #47

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat: allows to check for resource review permission against OCM AMS
gdbranco committed Jun 13, 2024
commit 949db1596fb4ca8ab317034fbca482be470a0356
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -37,6 +37,9 @@

# Ignore editor config
.vscode
# for VS Code debug sessions
**/__debug_bin*
**/ginkgo.report

# Ignore IntelliJ config
.idea/
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -361,3 +361,7 @@ crc/login:
@crc console --credentials -ojson | jq -r .clusterConfig.adminCredentials.password | oc login --username kubeadmin --insecure-skip-tls-verify=true https://api.crc.testing:6443
@oc whoami --show-token | $(container_tool) login --username kubeadmin --password-stdin "$(external_image_registry)" --tls-verify=false
.PHONY: crc/login

.PHONY: fmt-imports
fmt-imports: $(GCI)
find . -name '*.go' -not -path './vendor/*' | xargs $(GCI) write -s standard -s default -s "prefix(k8s)" -s "prefix(sigs.k8s)" -s "prefix(github.com)" -s "prefix(gitlab)" -s "prefix(github.com/openshift-online/rh-trex)" --custom-order --skip-generated
4 changes: 3 additions & 1 deletion cmd/trex/environments/service_types.go
Original file line number Diff line number Diff line change
@@ -22,7 +22,9 @@ type GenericServiceLocator func() services.GenericService

func NewGenericServiceLocator(env *Env) GenericServiceLocator {
return func() services.GenericService {
return services.NewGenericService(dao.NewGenericDao(&env.Database.SessionFactory))
return services.NewGenericService(
dao.NewGenericDao(&env.Database.SessionFactory),
env.Clients.OCM)
}
}

3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -18,13 +18,15 @@ require (
github.com/jinzhu/inflection v1.0.0
github.com/lib/pq v1.10.5
github.com/mendsley/gojwk v0.0.0-20141217222730-4d5ec6e58103
github.com/onsi/ginkgo/v2 v2.8.1
github.com/onsi/gomega v1.27.1
github.com/openshift-online/ocm-sdk-go v0.1.334
github.com/prometheus/client_golang v1.16.0
github.com/segmentio/ksuid v1.0.2
github.com/spf13/cobra v0.0.5
github.com/spf13/pflag v1.0.5
github.com/yaacov/tree-search-language v0.0.0-20190923184055-1c2dad2e354b
go.uber.org/mock v0.4.0
gopkg.in/resty.v1 v1.12.0
gorm.io/driver/postgres v1.0.5
gorm.io/gorm v1.20.5
@@ -38,6 +40,7 @@ require (
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect
github.com/docker/distribution v2.8.1+incompatible // indirect
github.com/go-logr/logr v1.2.3 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/go-cmp v0.5.9 // indirect
github.com/gorilla/css v1.0.0 // indirect
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
@@ -384,7 +384,6 @@ github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+W
github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
github.com/onsi/ginkgo v1.10.1/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc=
github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c=
github.com/onsi/ginkgo/v2 v2.1.4/go.mod h1:um6tUpWM/cxCK3/FK8BXqEiUMUwRgSM4JXG47RKZmLU=
@@ -511,6 +510,8 @@ go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=
go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=
go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU=
go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc=
go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4=
go.uber.org/multierr v1.5.0/go.mod h1:FeouvMocqHpRaaGuG9EjoKcStLC43Zu/fmqdUMPcKYU=
4 changes: 4 additions & 0 deletions pkg/api/dinosaur_types.go
Original file line number Diff line number Diff line change
@@ -5,6 +5,10 @@ import "gorm.io/gorm"
type Dinosaur struct {
Meta
Species string
// This is to illustrate resource review in action
// It passes integration tests as it's mocked
// does not work for local envs pointing to integration AMS via proxy
OrganizationId string
}

type DinosaurList []*Dinosaur
9 changes: 9 additions & 0 deletions pkg/auth/actions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package auth

const (
GetAction = "get"
ListAction = "list"
UpdateAction = "update"
CreateAction = "create"
DeleteAction = "delete"
)
17 changes: 17 additions & 0 deletions pkg/client/ocm/authorization.go
Original file line number Diff line number Diff line change
@@ -7,9 +7,11 @@ import (
azv1 "github.com/openshift-online/ocm-sdk-go/authorizations/v1"
)

//go:generate mockgen -source=authorization.go -package=ocm -destination=mock_authorization.go
type OCMAuthorization interface {
SelfAccessReview(ctx context.Context, action, resourceType, organizationID, subscriptionID, clusterID string) (allowed bool, err error)
AccessReview(ctx context.Context, username, action, resourceType, organizationID, subscriptionID, clusterID string) (allowed bool, err error)
ResourceReview(ctx context.Context, username string, action string, resource string) (*azv1.ResourceReview, error)
}

type authorization service
@@ -75,3 +77,18 @@ func (a authorization) AccessReview(ctx context.Context, username, action, resou

return response.Allowed(), nil
}

func (a authorization) ResourceReview(ctx context.Context, username string, action string, resource string) (*azv1.ResourceReview, error) {
con := a.client.connection
resourceReviewClient := con.Authorizations().V1().ResourceReview()

request, err := azv1.NewResourceReviewRequest().AccountUsername(username).Action(action).ResourceType(resource).Build()
if err != nil {
return nil, err
}
response, err := resourceReviewClient.Post().Request(request).SendContext(ctx)
if err != nil {
return nil, err
}
return response.Review(), nil
}
10 changes: 10 additions & 0 deletions pkg/client/ocm/authorization_mock.go
Original file line number Diff line number Diff line change
@@ -2,6 +2,8 @@ package ocm

import (
"context"

azv1 "github.com/openshift-online/ocm-sdk-go/authorizations/v1"
)

// authorizationMock returns allowed=true for every request
@@ -16,3 +18,11 @@ func (a authorizationMock) SelfAccessReview(ctx context.Context, action, resourc
func (a authorizationMock) AccessReview(ctx context.Context, username, action, resourceType, organizationID, subscriptionID, clusterID string) (allowed bool, err error) {
return true, nil
}

func (a authorizationMock) ResourceReview(ctx context.Context, username string, action string, resource string) (*azv1.ResourceReview, error) {
response, err := azv1.NewResourceReview().AccountUsername(username).Action(action).ResourceType(resource).OrganizationIDs("*").Build()
if err != nil {
return nil, err
}
return response, nil
}
85 changes: 85 additions & 0 deletions pkg/client/ocm/mock_authorization.go

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

10 changes: 10 additions & 0 deletions pkg/client/ocm/resource_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package ocm

type Resource string

const (
ClusterResource Resource = "Cluster"
SubscriptionResource Resource = "Subscription"
OrganizationResource Resource = "Organization"
Dinosaur Resource = "Dinosaur"
)
3 changes: 3 additions & 0 deletions pkg/db/db_session/default.go
Original file line number Diff line number Diff line change
@@ -158,6 +158,9 @@ func (f *Default) CheckConnection() error {
// THIS MUST **NOT** BE CALLED UNTIL THE SERVER/PROCESS IS EXITING!!
// This should only ever be called once for the entire duration of the application and only at the end.
func (f *Default) Close() error {
if f.db == nil {
return nil
}
return f.db.Close()
}

3 changes: 3 additions & 0 deletions pkg/db/db_session/test.go
Original file line number Diff line number Diff line change
@@ -209,6 +209,9 @@ func (f *Test) CheckConnection() error {
}

func (f *Test) Close() error {
if f.db == nil {
return nil
}
return f.db.Close()
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package migrations

import (
"gorm.io/gorm"

"github.com/go-gormigrate/gormigrate/v2"
)

func addDinosaurOrganizationIdColumn() *gormigrate.Migration {
return &gormigrate.Migration{
ID: "202406131012",
Migrate: func(tx *gorm.DB) error {
if err := tx.Exec("ALTER TABLE dinosaurs ADD COLUMN IF NOT EXISTS organization_id text NULL;").Error; err != nil {
return err
}
return nil
},
Rollback: func(tx *gorm.DB) error {
return nil
},
}
}
43 changes: 43 additions & 0 deletions pkg/db/migrations/migrations_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package migrations

import (
"fmt"
"os"
"strings"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestMigration(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Migration Suite")
}

var _ = Describe("Migrate", func() {
It("Expects same amount of files and migrationList", func() {
cwd, err := os.Getwd()
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
os.Exit(1)
}
files, err := os.ReadDir(cwd)
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
os.Exit(1)
}
amountGoFiles := []string{}
for _, file := range files {
if !strings.Contains(file.Name(), ".go") {
continue
}
if strings.Contains(file.Name(), "migration_structs") || strings.Contains(file.Name(), "migrations_test") {
continue
}
amountGoFiles = append(amountGoFiles, file.Name())
}
// Disconsiders migration_structs.go and test files
Expect(amountGoFiles).To(HaveLen(len(MigrationList)))
})
})
2 changes: 1 addition & 1 deletion pkg/handlers/dinosaur.go
Original file line number Diff line number Diff line change
@@ -82,7 +82,7 @@ func (h dinosaurHandler) List(w http.ResponseWriter, r *http.Request) {

listArgs := services.NewListArguments(r.URL.Query())
var dinosaurs = []api.Dinosaur{}
paging, err := h.generic.List(ctx, "username", listArgs, &dinosaurs)
paging, err := h.generic.List(ctx, listArgs, &dinosaurs)
if err != nil {
return nil, err
}
63 changes: 56 additions & 7 deletions pkg/services/generic.go
Original file line number Diff line number Diff line change
@@ -15,29 +15,35 @@ import (
sqlFilter "github.com/yaacov/tree-search-language/pkg/walkers/sql"

"github.com/openshift-online/rh-trex/pkg/api"
"github.com/openshift-online/rh-trex/pkg/auth"
"github.com/openshift-online/rh-trex/pkg/client/ocm"
"github.com/openshift-online/rh-trex/pkg/dao"
"github.com/openshift-online/rh-trex/pkg/db"
"github.com/openshift-online/rh-trex/pkg/errors"
"github.com/openshift-online/rh-trex/pkg/logger"
)

type GenericService interface {
List(ctx context.Context, username string, args *ListArguments, resourceList interface{}) (*api.PagingMeta, *errors.ServiceError)
List(ctx context.Context, args *ListArguments, resourceList interface{}) (*api.PagingMeta, *errors.ServiceError)
}

func NewGenericService(genericDao dao.GenericDao) GenericService {
return &sqlGenericService{genericDao: genericDao}
func NewGenericService(genericDao dao.GenericDao, ocmClient *ocm.Client) GenericService {
return &sqlGenericService{genericDao: genericDao, ocmClient: ocmClient}
}

var _ GenericService = &sqlGenericService{}

type sqlGenericService struct {
genericDao dao.GenericDao
ocmClient *ocm.Client
}

var (
SearchDisallowedFields = map[string]map[string]string{}
allFieldsAllowed = map[string]string{}
// Some mappings are not required as they match AMS resource 1:1
// Such as Organization
modelToAmsResource = map[string]string{}
)

// wrap all needed pieces for the LIST funciton
@@ -55,7 +61,8 @@ type listContext struct {
set map[string]bool
}

func (s *sqlGenericService) newListContext(ctx context.Context, username string, args *ListArguments, resourceList interface{}) (*listContext, interface{}, *errors.ServiceError) {
func newListContext(ctx context.Context, args *ListArguments, resourceList interface{}) (*listContext, interface{}, *errors.ServiceError) {
username := auth.GetUsernameFromContext(ctx)
log := logger.NewOCMLogger(ctx)
resourceModel := reflect.TypeOf(resourceList).Elem().Elem()
resourceTypeStr := resourceModel.Name()
@@ -79,13 +86,55 @@ func (s *sqlGenericService) newListContext(ctx context.Context, username string,
}, reflect.New(resourceModel).Interface(), nil
}

func resourceIncludesOrgId(model interface{}) bool {
resourceModel := reflect.TypeOf(model).Elem()
_, found := resourceModel.FieldByName("OrganizationId")
return found
}

func isAllowedToAllOrgs(allowedOrgs []string) bool {
return len(allowedOrgs) == 1 && allowedOrgs[0] == "*"
}

func (s *sqlGenericService) populateSearchRestriction(listCtx *listContext, model any) *errors.ServiceError {
ctx := listCtx.ctx
resourceName := listCtx.resourceType
if name, ok := modelToAmsResource[resourceName]; ok {
resourceName = string(name)
}
if resourceIncludesOrgId(model) {
resourceReview, err := s.ocmClient.Authorization.ResourceReview(ctx, listCtx.username, auth.GetAction, resourceName)
if err != nil {
return errors.GeneralError("Failed to verify resource review for user '%s' on resource '%s': %v", listCtx.username, listCtx.resourceType, err)
}

// TODO setup a search builder
allowedOrgs := resourceReview.OrganizationIDs()
// If user doesn't have access to all orgs include search for allowed only
if !isAllowedToAllOrgs(allowedOrgs) {
if listCtx.args.Search != "" {
listCtx.args.Search += " and "
}
for i := range allowedOrgs {
allowedOrgs[i] = fmt.Sprintf("'%s'", allowedOrgs[i])
}
listCtx.args.Search += fmt.Sprintf("organization_id in (%s)", strings.Join(allowedOrgs, ","))
}
}
return nil
}

// resourceList must be a pointer to a slice of database resource objects
func (s *sqlGenericService) List(ctx context.Context, username string, args *ListArguments, resourceList interface{}) (*api.PagingMeta, *errors.ServiceError) {
listCtx, model, err := s.newListContext(ctx, username, args, resourceList)
func (s *sqlGenericService) List(ctx context.Context, args *ListArguments, resourceList interface{}) (*api.PagingMeta, *errors.ServiceError) {
listCtx, model, err := newListContext(ctx, args, resourceList)
if err != nil {
return nil, err
}

if err = s.populateSearchRestriction(listCtx, model); err != nil {
return nil, err
}

// the ordering for the sub functions matters.
builders := []listBuilder{
// build SQL to load related resource. for now, it delegates to gorm.preload.
@@ -100,7 +149,7 @@ func (s *sqlGenericService) List(ctx context.Context, username string, args *Lis
// TODO: add any custom builder functions
}

d := s.genericDao.GetInstanceDao(ctx, model)
d := s.genericDao.GetInstanceDao(listCtx.ctx, model)

// run all the "builders". they cumulatively add constructs to gorm by the context.
// it stops when a builder function raises error or signals finished.
184 changes: 140 additions & 44 deletions pkg/services/generic_test.go
Original file line number Diff line number Diff line change
@@ -2,72 +2,167 @@ package services

import (
"context"
"testing"
"net/url"
"reflect"

"github.com/openshift-online/rh-trex/pkg/auth"
"github.com/openshift-online/rh-trex/pkg/client/ocm"
"github.com/openshift-online/rh-trex/pkg/dao"
"github.com/openshift-online/rh-trex/pkg/db"
"go.uber.org/mock/gomock"

"github.com/onsi/gomega/types"
"github.com/yaacov/tree-search-language/pkg/tsl"

azv1 "github.com/openshift-online/ocm-sdk-go/authorizations/v1"
"github.com/openshift-online/rh-trex/pkg/api"
"github.com/openshift-online/rh-trex/pkg/config"
"github.com/openshift-online/rh-trex/pkg/db/db_session"
"github.com/openshift-online/rh-trex/pkg/errors"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestSQLTranslation(t *testing.T) {
RegisterTestingT(t)
dbConfig := config.NewDatabaseConfig()
err := dbConfig.ReadFiles()
Expect(err).ToNot(HaveOccurred())
var dbFactory db.SessionFactory = db_session.NewProdFactory(dbConfig)
defer dbFactory.Close()
var _ = Describe("populates search restriction", func() {
var ctx context.Context
var ctrl *gomock.Controller
var genericService sqlGenericService
var genericDao dao.GenericDao
var authorizationMock *ocm.MockOCMAuthorization
var ocmClientMock *ocm.Client
username := "test-user"
BeforeEach(func() {
ctx = context.Background()
ctx = auth.SetUsernameContext(ctx, username)
ctrl = gomock.NewController(GinkgoT())
dbConfig := config.NewDatabaseConfig()
err := dbConfig.ReadFiles()
Expect(err).ToNot(HaveOccurred())
var dbFactory db.SessionFactory = db_session.NewTestFactory(dbConfig)
defer dbFactory.Close()

authorizationMock = ocm.NewMockOCMAuthorization(ctrl)
ocmClientMock = &ocm.Client{
Authorization: authorizationMock,
}
genericDao = dao.NewGenericDao(&dbFactory)
genericService = sqlGenericService{
genericDao: genericDao,
ocmClient: ocmClientMock,
}
})
Context("Resource includes organization ID field", func() {
When("Auth allows all orgs", func() {
It("Allows all orgs", func() {
args := NewListArguments(url.Values{})
listCtx, model, serviceErr := newListContext(ctx, args, &[]api.Dinosaur{})
resourceModel := reflect.TypeOf(&api.Dinosaur{}).Elem()
Expect(model).To(Equal(reflect.New(resourceModel).Interface()))
Expect(serviceErr).ToNot(HaveOccurred())
response, err := azv1.NewResourceReview().
AccountUsername(listCtx.username).
Action(auth.GetAction).
ResourceType(string(ocm.Dinosaur)).
OrganizationIDs("*").
Build()
Expect(err).ToNot(HaveOccurred())
authorizationMock.EXPECT().
ResourceReview(listCtx.ctx, listCtx.username, auth.GetAction, string(ocm.Dinosaur)).
Return(response, nil)
serviceErr = genericService.populateSearchRestriction(listCtx, model)
Expect(serviceErr).ToNot(HaveOccurred())
Expect(listCtx.args.Search).To(BeEmpty())
})
})
When("Auth restricts orgs", func() {
It("Allows only returned orgs", func() {
args := NewListArguments(url.Values{})
listCtx, model, serviceErr := newListContext(ctx, args, &[]api.Dinosaur{})
resourceModel := reflect.TypeOf(&api.Dinosaur{}).Elem()
Expect(model).To(Equal(reflect.New(resourceModel).Interface()))
Expect(serviceErr).ToNot(HaveOccurred())
response, err := azv1.NewResourceReview().
AccountUsername(listCtx.username).
Action(auth.GetAction).
ResourceType(string(ocm.Dinosaur)).
OrganizationIDs("123", "124").
Build()
Expect(err).ToNot(HaveOccurred())
authorizationMock.EXPECT().
ResourceReview(listCtx.ctx, listCtx.username, auth.GetAction, string(ocm.Dinosaur)).
Return(response, nil)
serviceErr = genericService.populateSearchRestriction(listCtx, model)
Expect(serviceErr).ToNot(HaveOccurred())
Expect(listCtx.args.Search).ToNot(BeEmpty())
Expect(listCtx.args.Search).To(Equal("organization_id in ('123','124')"))
})
It("Includes pre existing search", func() {
args := NewListArguments(url.Values{})
args.Search = "justification like '%test%'"
listCtx, model, serviceErr := newListContext(ctx, args, &[]api.Dinosaur{})
resourceModel := reflect.TypeOf(&api.Dinosaur{}).Elem()
Expect(model).To(Equal(reflect.New(resourceModel).Interface()))
Expect(serviceErr).ToNot(HaveOccurred())
response, err := azv1.NewResourceReview().
AccountUsername(listCtx.username).
Action(auth.GetAction).
ResourceType(string(ocm.Dinosaur)).
OrganizationIDs("123", "124").
Build()
Expect(err).ToNot(HaveOccurred())
authorizationMock.EXPECT().
ResourceReview(listCtx.ctx, listCtx.username, auth.GetAction, string(ocm.Dinosaur)).
Return(response, nil)
serviceErr = genericService.populateSearchRestriction(listCtx, model)
Expect(serviceErr).ToNot(HaveOccurred())
Expect(listCtx.args.Search).ToNot(BeEmpty())
Expect(
listCtx.args.Search,
).To(Equal("justification like '%test%' and organization_id in ('123','124')"))
})
})
})
})

g := dao.NewGenericDao(&dbFactory)
genericService := sqlGenericService{genericDao: g}
var _ = Describe("Sql Translation", func() {
var genericService sqlGenericService
var genericDao dao.GenericDao
BeforeEach(func() {
dbConfig := config.NewDatabaseConfig()
err := dbConfig.ReadFiles()
Expect(err).ToNot(HaveOccurred())
var dbFactory db.SessionFactory = db_session.NewTestFactory(dbConfig)
defer dbFactory.Close()

// ill-formatted search or disallowed fields should be rejected
tests := []map[string]interface{}{
{
"search": "garbage",
"error": "rh-trex-21: Failed to parse search query: garbage",
},
{
"search": "id in ('123')",
"error": "rh-trex-21: dinosaurs.id is not a valid field name",
},
}
for _, test := range tests {
list := []api.Dinosaur{}
search := test["search"].(string)
errorMsg := test["error"].(string)
listCtx, model, serviceErr := genericService.newListContext(context.Background(), "", &ListArguments{Search: search}, &list)
genericDao = dao.NewGenericDao(&dbFactory)
genericService = sqlGenericService{genericDao: genericDao}
})
DescribeTable("Errors", func(
search string, errorMsg string) {
listCtx, model, serviceErr := newListContext(
context.Background(),
&ListArguments{Search: search},
&[]api.Dinosaur{},
)
Expect(serviceErr).ToNot(HaveOccurred())
d := g.GetInstanceDao(context.Background(), model)
d := genericDao.GetInstanceDao(context.Background(), model)
(*listCtx.disallowedFields)["id"] = "id"
_, serviceErr = genericService.buildSearch(listCtx, &d)
Expect(serviceErr).To(HaveOccurred())
Expect(serviceErr.Code).To(Equal(errors.ErrorBadRequest))
Expect(serviceErr.Error()).To(Equal(errorMsg))
}
},
Entry("Garbage", "garbage", "rh-trex-21: Failed to parse search query: garbage"),
Entry("Invalid field name", "id in ('123')", "rh-trex-21: dinosaurs.id is not a valid field name"))

// tests for sql parsing
tests = []map[string]interface{}{
{
"search": "username in ('ooo.openshift')",
"sql": "username IN (?)",
"values": ConsistOf("ooo.openshift"),
},
}
for _, test := range tests {
list := []api.Dinosaur{}
search := test["search"].(string)
sqlReal := test["sql"].(string)
valuesReal := test["values"].(types.GomegaMatcher)
listCtx, _, serviceErr := genericService.newListContext(context.Background(), "", &ListArguments{Search: search}, &list)
DescribeTable("Sql Parsing", func(
search string, sqlReal string, valuesReal types.GomegaMatcher) {
listCtx, _, serviceErr := newListContext(
context.Background(),
&ListArguments{Search: search},
&[]api.Dinosaur{},
)
Expect(serviceErr).ToNot(HaveOccurred())
tslTree, err := tsl.ParseTSL(search)
Expect(err).ToNot(HaveOccurred())
@@ -77,5 +172,6 @@ func TestSQLTranslation(t *testing.T) {
Expect(err).ToNot(HaveOccurred())
Expect(sql).To(Equal(sqlReal))
Expect(values).To(valuesReal)
}
}
},
Entry("Valid search", "username in ('ooo.openshift')", "username IN (?)", ConsistOf("ooo.openshift")))
})
13 changes: 13 additions & 0 deletions pkg/services/suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package services

import (
"testing"

. "github.com/onsi/ginkgo/v2/dsl/core"
. "github.com/onsi/gomega"
)

func TestServices(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Services Suite")
}
2 changes: 1 addition & 1 deletion templates/generate-handlers.txt
Original file line number Diff line number Diff line change
@@ -83,7 +83,7 @@ func (h {{.KindLowerSingular}}Handler) List(w http.ResponseWriter, r *http.Reque

listArgs := services.NewListArguments(r.URL.Query())
var {{.KindLowerPlural}} = []api.{{.Kind}}{}
paging, err := h.generic.List(ctx, "username", listArgs, &{{.KindLowerPlural}})
paging, err := h.generic.List(ctx, listArgs, &{{.KindLowerPlural}})
if err != nil {
return nil, err
}
19 changes: 0 additions & 19 deletions test/mocks/mocks.go

This file was deleted.

65 changes: 0 additions & 65 deletions test/mocks/ocm.go

This file was deleted.