feature(backend) Tenant ID support #2980
Conversation
| "github.com/kubeshop/tracetest/server/http/middleware" | ||
| ) | ||
|
|
||
| func Tenant(ctx context.Context, query string, params ...any) (string, []any) { |
There was a problem hiding this comment.
From here we can grab add the tenant query to the operation and grab the tenant id from the context
There was a problem hiding this comment.
Can we be more specific on this method naming? By calling it a Tenant we don't provide a hint about what is happening inside of this function and what it does.
Can we call it GetQueryWithTenant or something similar?
|
|
||
| const HeaderTenantID = "X-Tracetest-TenantID" | ||
|
|
||
| func TenantMiddleware(next http.Handler) http.Handler { |
There was a problem hiding this comment.
Validates that the tenant id has the proper format, if all is correct, we add it to the context so the rest of the app can use it
| @@ -0,0 +1,69 @@ | |||
| BEGIN; | |||
There was a problem hiding this comment.
You should also create an index for those columns:
CREATE INDEX idx_config_tenant_id
ON config(tenant_id);Otherwise this will be very costly to query after a few weeks of saas being released.
Note: you don't have to delete the index in the down migration. Postgres deletes the index when you drop the column
| func TenantID(ctx context.Context) *string { | ||
| tenantID := ctx.Value(middleware.TenantIDKey) | ||
|
|
||
| if tenantID == "" || tenantID == nil { |
There was a problem hiding this comment.
You are creating each resource with:
COLUMN tenant_id uuid DEFAULT '00000000-0000-0000-0000-000000000000';Shouldn't you inject 00000000-0000-0000-0000-00000000000 into the context when user passes no tenant information?
There was a problem hiding this comment.
I removed the default value because that might cause problems with backward compatibility, right now I think is better to have it always be NULL. Good catch!
| return query, params | ||
| } | ||
|
|
||
| queryPrefix := getQueryPrefix(query) |
There was a problem hiding this comment.
There is a tricky thing to think about here if the query has subqueries: (it doesn't affect us now, but maybe we can add a comment here as a warning)
SELECT * FROM (select * from mytable where id = 1)The resulting queryPrefix will be AND instead of WHERE, making the entire function return an invalid SQL query in the future.
| "github.com/kubeshop/tracetest/server/http/middleware" | ||
| ) | ||
|
|
||
| func Tenant(ctx context.Context, query string, params ...any) (string, []any) { |
There was a problem hiding this comment.
Can we be more specific on this method naming? By calling it a Tenant we don't provide a hint about what is happening inside of this function and what it does.
Can we call it GetQueryWithTenant or something similar?
| if err != nil { | ||
| return Environment{}, err | ||
| } | ||
| defer tx.Rollback() |
This PR adds support for tenant ID to all resources in the application
Changes
Checklist