romana / core

Romana core components - Micro services written in Go.
Apache License 2.0
47 stars 11 forks source link

Incorrect gorm syntax in `findTenantsByName` #91

Closed flashvoid closed 7 years ago

flashvoid commented 8 years ago

Relevant to https://github.com/romana/core/blob/fix/k8s-listener/tenant/store.go#L132

Where not executed after Find so the query always returns all the tenants. Obvious fix is

db := tenantStore.DbStore.Db.Where("name = ?", name).Find(&tenants)

but these will find all the tenants with same name while the only one user of this code in https://github.com/romana/core/blob/fix/k8s-listener/romana/kubernetes/listener.go#L291 can't really process multiple users, so maybe it should be

db := tenantStore.DbStore.Db.Where("name = ?", name).First(&tenants)

and a name of the function to be findTenantByName ?

BTW. Client code is also broken - GET request at https://github.com/romana/core/blob/fix/k8s-listener/romana/kubernetes/listener.go#L291 should be like

err = l.restClient.Get(fmt.Sprintf("%s/find/tenants?tenantName=%s", tenantURL, ns), t)
debedb commented 8 years ago

Actually no - it should be able to find tenant by both name and external ID as things stand right now. However, that'll have to change (https://paninetworks.kanbanize.com/ctrl_board/3/cards/319/details).

Will close this when 319 is done.

flashvoid commented 8 years ago

This is how current code works.

curl http://192.168.99.10:9602/find/tenants?tenantName=tenant-a
[{"ID":1,"ExternalID":"425feed6e46a45b69ccd2c97ba0d320f","Name":"default","Segments":null,"Seq":1},{"ID":2,"ExternalID":"680214b74eb14cd58a30a1c251aa2af6","Name":"tenant-a","Segments":null,"Seq":2}]

This is how proposed change works

curl "http://192.168.99.10:9602/find/tenants?tenantName=tenant-a"
[{"ID":2,"ExternalID":"680214b74eb14cd58a30a1c251aa2af6","Name":"tenant-a","Segments":null,"Seq":2}]
debedb commented 8 years ago

How about instead we use https://github.com/paninetworks/core/pull/9

pritesh commented 7 years ago

Fixed in latest release.