From 65b72e0a627cf0b6bbc729bfb334960bf1ae682a Mon Sep 17 00:00:00 2001 From: Kyle Wong <37189875+kyle-a-wong@users.noreply.github.com> Date: Tue, 24 Sep 2024 11:13:31 -0400 Subject: [PATCH 1/2] server: refactor databases_metadata api In preparation for an upcoming API, this commit refactors the query and mapping of rows to dbMetadata structs to separate methods to be reused. Release note: None Epic: CRDB-37558 --- pkg/server/api_v2_databases_metadata.go | 113 +++++++++++++----------- 1 file changed, 63 insertions(+), 50 deletions(-) diff --git a/pkg/server/api_v2_databases_metadata.go b/pkg/server/api_v2_databases_metadata.go index 2dc2b99773a9..0e364cfadb9a 100644 --- a/pkg/server/api_v2_databases_metadata.go +++ b/pkg/server/api_v2_databases_metadata.go @@ -664,7 +664,6 @@ func (a *apiV2Server) GetDBMetadata(w http.ResponseWriter, r *http.Request) { }, } apiutil.WriteJSONResponse(ctx, w, 200, resp) - } func (a *apiV2Server) getDBMetadata( @@ -677,39 +676,8 @@ func (a *apiV2Server) getDBMetadata( limit int, offset int, ) (dbms []dbMetadata, totalRowCount int64, retErr error) { - sqlUserStr := sqlUser.Normalized() dbms = make([]dbMetadata, 0) - query := safesql.NewQuery() - - // Base query aggregates table metadata by db_id. It joins on a subquery which flattens - // and deduplicates all store ids for tables in a database into a single array. This query - // will only return databases that the provided sql user has CONNECT privileges to. If they - // are an admin, they have access to all databases. - query.Append(`SELECT - n.id as db_id, - n.name as db_name, - COALESCE(sum(tbm.replication_size_bytes)::INT, 0) as size_bytes, - count(CASE WHEN tbm.table_type = 'TABLE' THEN 1 ELSE NULL END) as table_count, - max(tbm.last_updated) as last_updated, - COALESCE(s.store_ids, ARRAY[]) as store_ids, - count(*) OVER() as total_row_count - FROM system.namespace n - LEFT JOIN system.table_metadata tbm ON n.id = tbm.db_id - LEFT JOIN system.role_members rm ON rm.role = 'admin' AND member = $ - LEFT JOIN ( - SELECT db_id, array_agg(DISTINCT unnested_ids) as store_ids - FROM system.table_metadata, unnest(store_ids) as unnested_ids - GROUP BY db_id - ) s ON s.db_id = tbm.db_id - WHERE (rm.role = 'admin' OR n.name in ( - SELECT cdp.database_name - FROM "".crdb_internal.cluster_database_privileges cdp - WHERE grantee = $ - AND privilege_type = 'CONNECT' - )) - AND n."parentID" = 0 - AND n."parentSchemaID" = 0 -`, sqlUserStr, sqlUserStr) + query := getDatabaseMetadataBaseQuery(sqlUser.Normalized()) if dbName != "" { query.Append("AND n.name ILIKE $ ", dbName) @@ -762,7 +730,6 @@ func (a *apiV2Server) getDBMetadata( // If ok == false, the query returned 0 rows. scanner := makeResultScanner(it.Types()) for ; ok; ok, err = it.Next(ctx) { - var dbm dbMetadata row := it.Cur() if setTotalRowCount { if err := scanner.Scan(row, "total_row_count", &totalRowCount); err != nil { @@ -770,22 +737,8 @@ func (a *apiV2Server) getDBMetadata( } setTotalRowCount = false } - if err := scanner.Scan(row, "db_id", &dbm.DbId); err != nil { - return nil, 0, err - } - if err := scanner.Scan(row, "db_name", &dbm.DbName); err != nil { - return nil, 0, err - } - if err := scanner.Scan(row, "size_bytes", &dbm.SizeBytes); err != nil { - return nil, 0, err - } - if err := scanner.Scan(row, "table_count", &dbm.TableCount); err != nil { - return nil, 0, err - } - if err := scanner.Scan(row, "store_ids", &dbm.StoreIds); err != nil { - return nil, totalRowCount, err - } - if err := scanner.Scan(row, "last_updated", &dbm.LastUpdated); err != nil { + dbm, err := rowToDatabaseMetadata(scanner, row) + if err != nil { return nil, 0, err } dbms = append(dbms, dbm) @@ -798,6 +751,66 @@ func (a *apiV2Server) getDBMetadata( return dbms, totalRowCount, nil } +func getDatabaseMetadataBaseQuery(userName string) *safesql.Query { + query := safesql.NewQuery() + + // Base query aggregates table metadata by db_id. It joins on a subquery which flattens + // and deduplicates all store ids for tables in a database into a single array. This query + // will only return databases that the provided sql user has CONNECT privileges to. If they + // are an admin, they have access to all databases. + query.Append(`SELECT + n.id as db_id, + n.name as db_name, + COALESCE(sum(tbm.replication_size_bytes)::INT, 0) as size_bytes, + count(CASE WHEN tbm.table_type = 'TABLE' THEN 1 ELSE NULL END) as table_count, + max(tbm.last_updated) as last_updated, + COALESCE(s.store_ids, ARRAY[]) as store_ids, + count(*) OVER() as total_row_count + FROM system.namespace n + LEFT JOIN system.table_metadata tbm ON n.id = tbm.db_id + LEFT JOIN system.role_members rm ON rm.role = 'admin' AND member = $ + LEFT JOIN ( + SELECT db_id, array_agg(DISTINCT unnested_ids) as store_ids + FROM system.table_metadata, unnest(store_ids) as unnested_ids + GROUP BY db_id + ) s ON s.db_id = tbm.db_id + WHERE (rm.role = 'admin' OR n.name in ( + SELECT cdp.database_name + FROM "".crdb_internal.cluster_database_privileges cdp + WHERE grantee = $ + AND privilege_type = 'CONNECT' + )) + AND n."parentID" = 0 + AND n."parentSchemaID" = 0 +`, userName, userName) + + return query +} + +func rowToDatabaseMetadata(scanner resultScanner, row tree.Datums) (dbm dbMetadata, err error) { + var emptyMetadata dbMetadata + if err = scanner.Scan(row, "db_id", &dbm.DbId); err != nil { + return emptyMetadata, err + } + if err = scanner.Scan(row, "db_name", &dbm.DbName); err != nil { + return emptyMetadata, err + } + if err = scanner.Scan(row, "size_bytes", &dbm.SizeBytes); err != nil { + return emptyMetadata, err + } + if err = scanner.Scan(row, "table_count", &dbm.TableCount); err != nil { + return emptyMetadata, err + } + if err = scanner.Scan(row, "store_ids", &dbm.StoreIds); err != nil { + return emptyMetadata, err + } + if err = scanner.Scan(row, "last_updated", &dbm.LastUpdated); err != nil { + return emptyMetadata, err + } + + return dbm, nil +} + // TableMetadataJob routes to the necessary receiver based on the http method of the request. Requires // The user making the request must have the CONNECT database grant on at least one database or admin privilege. // --- From 3790e561f875f32539f774b5dd2c23a6b9913b7b Mon Sep 17 00:00:00 2001 From: Kyle Wong <37189875+kyle-a-wong@users.noreply.github.com> Date: Tue, 24 Sep 2024 14:41:56 -0400 Subject: [PATCH 2/2] server: add API to fetch database metadata for database id Adds a new API v2 endpoint, `GET /api/v2/database_metadata/` where id is a specific database id. This API will return database metadata Resolves: #131304 Epic: CRDB-37558 Release note: None --- pkg/server/api_v2.go | 3 +- pkg/server/api_v2_databases_metadata.go | 93 ++++++++++++++- pkg/server/api_v2_databases_metadata_test.go | 118 +++++++++++++++---- 3 files changed, 185 insertions(+), 29 deletions(-) diff --git a/pkg/server/api_v2.go b/pkg/server/api_v2.go index c6bff7b7c779..6a0cc326a424 100644 --- a/pkg/server/api_v2.go +++ b/pkg/server/api_v2.go @@ -187,7 +187,8 @@ func registerRoutes( {"rules/", a.listRules, false, authserver.RegularRole, true}, {"sql/", a.execSQL, true, authserver.RegularRole, true}, - {"database_metadata/", a.GetDBMetadata, true, authserver.RegularRole, true}, + {"database_metadata/", a.GetDbMetadata, true, authserver.RegularRole, true}, + {"database_metadata/{database_id:[0-9]+}/", a.GetDbMetadataForId, true, authserver.RegularRole, true}, {"table_metadata/", a.GetTableMetadata, true, authserver.RegularRole, true}, {"table_metadata/{table_id:[0-9]+}/", a.GetTableMetadataWithDetails, true, authserver.RegularRole, true}, {"table_metadata/updatejob/", a.TableMetadataJob, true, authserver.RegularRole, true}, diff --git a/pkg/server/api_v2_databases_metadata.go b/pkg/server/api_v2_databases_metadata.go index 0e364cfadb9a..34ade446822f 100644 --- a/pkg/server/api_v2_databases_metadata.go +++ b/pkg/server/api_v2_databases_metadata.go @@ -57,6 +57,9 @@ const ( const ( TableNotFound string = "table not found" InvalidTableId string = "invalid table ID" + + DatabaseNotFound string = "database not found" + InvalidDatabaseId string = "invalid database ID" ) // GetTableMetadata returns a paginated response of table metadata and statistics. This is not a live view of @@ -520,7 +523,7 @@ func rowToTableMetadata(scanner resultScanner, row tree.Datums) (tmd tableMetada return tmd, nil } -// GetDBMetadata returns a paginated response of database metadata and statistics. This is not a live view of +// GetDbMetadata returns a paginated response of database metadata and statistics. This is not a live view of // the database data but instead is cached data that had been precomputed at an earlier time. // // The user making the request will receive database metadata based on the CONNECT database grant and admin privilege. @@ -578,7 +581,7 @@ func rowToTableMetadata(scanner resultScanner, row tree.Datums) (tmd tableMetada // description: A paginated response of dbMetadata results. // "400": // description: Bad request. If the provided query parameters are invalid. -func (a *apiV2Server) GetDBMetadata(w http.ResponseWriter, r *http.Request) { +func (a *apiV2Server) GetDbMetadata(w http.ResponseWriter, r *http.Request) { ctx := r.Context() ctx = a.sqlServer.AnnotateCtx(ctx) sqlUser := authserver.UserFromHTTPAuthInfoContext(ctx) @@ -648,7 +651,7 @@ func (a *apiV2Server) GetDBMetadata(w http.ResponseWriter, r *http.Request) { dbNameFilter = fmt.Sprintf("%%%s%%", dbName) } - dbm, totalRowCount, err := a.getDBMetadata(ctx, sqlUser, dbNameFilter, storeIds, sortBy, sortOrder, pageSize, offset) + dbm, totalRowCount, err := a.getDbMetadata(ctx, sqlUser, dbNameFilter, storeIds, sortBy, sortOrder, pageSize, offset) if err != nil { srverrors.APIV2InternalError(ctx, err, w) @@ -666,7 +669,61 @@ func (a *apiV2Server) GetDBMetadata(w http.ResponseWriter, r *http.Request) { apiutil.WriteJSONResponse(ctx, w, 200, resp) } -func (a *apiV2Server) getDBMetadata( +// GetDbMetadataForId fetches database metadata for a specific database id. +// +// The user making the request must have the CONNECT database grant for the database, or the admin privilege. +// +// --- +// parameters: +// +// - name: database_id +// type: integer +// description: The id of the database to fetch database metadata. +// in: path +// required: false +// +// produces: +// - application/json +// +// responses: +// +// "200": +// description: A dbMetadataWithDetailsResponse containing the database metadata. +// "404": +// description: If the database for the provided id doesn't exist or the user doesn't have necessary permissions +// to access the database +func (a *apiV2Server) GetDbMetadataForId(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) + return + } + + ctx := a.sqlServer.AnnotateCtx(r.Context()) + sqlUser := authserver.UserFromHTTPAuthInfoContext(ctx) + pathVars := mux.Vars(r) + databaseId, err := strconv.Atoi(pathVars["database_id"]) + if err != nil { + http.Error(w, InvalidDatabaseId, http.StatusBadRequest) + return + } + dbm, err := a.getDbMetadataForId(ctx, sqlUser, databaseId) + if err != nil { + srverrors.APIV2InternalError(ctx, err, w) + return + } + + // No db id means table couldn't be found or user doesn't have access to the table + if dbm.DbId == 0 { + http.Error(w, DatabaseNotFound, http.StatusNotFound) + return + } + resp := dbMetadataWithDetailsResponse{ + Metadata: dbm, + } + apiutil.WriteJSONResponse(ctx, w, 200, resp) +} + +func (a *apiV2Server) getDbMetadata( ctx context.Context, sqlUser username.SQLUsername, dbName string, @@ -706,7 +763,7 @@ func (a *apiV2Server) getDBMetadata( query.Append("LIMIT $ ", limit) query.Append("OFFSET $ ", offset) - it, err := a.admin.internalExecutor.QueryIteratorEx( + it, err := a.sqlServer.internalExecutor.QueryIteratorEx( ctx, "get-database-metadata", nil, /* txn */ sessiondata.NodeUserSessionDataOverride, query.String(), query.QueryArguments()..., @@ -751,6 +808,28 @@ func (a *apiV2Server) getDBMetadata( return dbms, totalRowCount, nil } +func (a *apiV2Server) getDbMetadataForId( + ctx context.Context, sqlUser username.SQLUsername, dbId int, +) (dbMetadata, error) { + query := getDatabaseMetadataBaseQuery(sqlUser.Normalized()) + query.Append("AND n.id = $ ", dbId) + query.Append("GROUP BY n.id, n.name, s.store_ids ") + + row, types, err := a.sqlServer.internalExecutor.QueryRowExWithCols(ctx, "get-db-metadata-for-id", nil, + sessiondata.NodeUserSessionDataOverride, query.String(), query.QueryArguments()...) + + if err != nil { + return dbMetadata{}, err + } + + if row == nil { + return dbMetadata{}, nil + } + + scanner := makeResultScanner(types) + return rowToDatabaseMetadata(scanner, row) +} + func getDatabaseMetadataBaseQuery(userName string) *safesql.Query { query := safesql.NewQuery() @@ -1036,3 +1115,7 @@ type tableMetadataWithDetailsResponse struct { Metadata tableMetadata `json:"metadata"` CreateStatement string `json:"create_statement"` } + +type dbMetadataWithDetailsResponse struct { + Metadata dbMetadata `json:"metadata"` +} diff --git a/pkg/server/api_v2_databases_metadata_test.go b/pkg/server/api_v2_databases_metadata_test.go index c782709b7a36..53816b68d227 100644 --- a/pkg/server/api_v2_databases_metadata_test.go +++ b/pkg/server/api_v2_databases_metadata_test.go @@ -407,7 +407,7 @@ func TestGetTableMetadataForId(t *testing.T) { }) } -func TestGetDBMetadata(t *testing.T) { +func TestGetDbMetadata(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) testCluster := serverutils.StartCluster(t, 1, base.TestClusterArgs{}) @@ -616,6 +616,90 @@ func TestGetDBMetadata(t *testing.T) { }) } +func TestGetDbMetadataForId(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + testCluster := serverutils.StartCluster(t, 1, base.TestClusterArgs{}) + ctx := context.Background() + defer testCluster.Stopper().Stop(ctx) + conn := testCluster.ServerConn(0) + defer conn.Close() + runner := sqlutils.MakeSQLRunner(conn) + db1Name := "new_test_db_1" + db1Id, _ := setupTest(t, conn, db1Name, "new_test_db_2") + + ts := testCluster.Server(0) + client, err := ts.GetAdminHTTPClient() + require.NoError(t, err) + + t.Run("get database metadata", func(t *testing.T) { + uri := fmt.Sprintf("/api/v2/database_metadata/%d/", db1Id) + resp := makeApiRequest[dbMetadataWithDetailsResponse]( + t, client, ts.AdminURL().WithPath(uri).String(), http.MethodGet) + require.Equal(t, int64(db1Id), resp.Metadata.DbId) + }) + + t.Run("no tables in db", func(t *testing.T) { + runner.Exec(t, "CREATE DATABASE empty_db") + row := runner.QueryRow(t, "SELECT crdb_internal.get_database_id('empty_db') AS database_id;") + var emptyDbId int64 + row.Scan(&emptyDbId) + uri := fmt.Sprintf("/api/v2/database_metadata/%d/", emptyDbId) + resp := makeApiRequest[dbMetadataWithDetailsResponse]( + t, client, ts.AdminURL().WithPath(uri).String(), http.MethodGet) + require.Equal(t, emptyDbId, resp.Metadata.DbId) + }) + + t.Run("authorization", func(t *testing.T) { + sessionUsername := username.TestUserName() + userClient, _, err := ts.GetAuthenticatedHTTPClientAndCookie(sessionUsername, false, 1) + require.NoError(t, err) + + uri := fmt.Sprintf("/api/v2/database_metadata/%d/", db1Id) + failed := makeApiRequest[string]( + t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet) + require.Equal(t, DatabaseNotFound, failed) + + // grant connect access to db1 to allow request to succeed + runner.Exec(t, fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s", db1Name, sessionUsername.Normalized())) + resp := makeApiRequest[dbMetadataWithDetailsResponse]( + t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet) + require.Equal(t, int64(db1Id), resp.Metadata.DbId) + + // revoke access to db1. + runner.Exec(t, fmt.Sprintf("REVOKE CONNECT ON DATABASE %s FROM %s", db1Name, sessionUsername.Normalized())) + failed = makeApiRequest[string]( + t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet) + require.Equal(t, DatabaseNotFound, failed) + + // grant admin access to the user + runner.Exec(t, fmt.Sprintf("GRANT ADMIN TO %s", sessionUsername.Normalized())) + resp = makeApiRequest[dbMetadataWithDetailsResponse]( + t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet) + require.Equal(t, int64(db1Id), resp.Metadata.DbId) + }) + + t.Run("non GET method 405 error", func(t *testing.T) { + req, err := http.NewRequest("POST", ts.AdminURL().WithPath("/api/v2/database_metadata/1/").String(), nil) + require.NoError(t, err) + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + require.NoError(t, err) + require.NotNil(t, resp) + require.Equal(t, 405, resp.StatusCode) + respBytes, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Contains(t, string(respBytes), http.StatusText(http.StatusMethodNotAllowed)) + }) + + t.Run("database doesnt exist", func(t *testing.T) { + failed := makeApiRequest[string]( + t, client, ts.AdminURL().WithPath("/api/v2/database_metadata/1000000000/").String(), http.MethodGet) + require.Equal(t, DatabaseNotFound, failed) + }) +} + func TestGetTableMetadataUpdateJobStatus(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -777,29 +861,18 @@ func assertJobTriggered(t *testing.T, client http.Client, url string, c chan int } func setupTest(t *testing.T, conn *gosql.DB, db1 string, db2 string) (dbId1 int, dbId2 int) { - _, err := conn.Exec(`CREATE DATABASE IF NOT EXISTS ` + db1) - require.NoError(t, err) + runner := sqlutils.MakeSQLRunner(conn) + runner.Exec(t, `CREATE DATABASE IF NOT EXISTS `+db1) - _, err = conn.Exec(`CREATE DATABASE IF NOT EXISTS ` + db2) - require.NoError(t, err) - result, err := conn.Query(fmt.Sprintf(`SELECT crdb_internal.get_database_id('%s') AS database_id;`, db1)) - require.NoError(t, err) - if result.Next() { - err = result.Scan(&dbId1) - require.NoError(t, err) - } else { - t.Fail() - } + runner.Exec(t, `CREATE DATABASE IF NOT EXISTS `+db2) - result, err = conn.Query(fmt.Sprintf(`SELECT crdb_internal.get_database_id('%s') AS database_id;`, db2)) - require.NoError(t, err) - if result.Next() { - err = result.Scan(&dbId2) - require.NoError(t, err) - } else { - t.Fail() - } - _, err = conn.Exec(fmt.Sprintf(` + row := runner.QueryRow(t, fmt.Sprintf(`SELECT crdb_internal.get_database_id('%s') AS database_id;`, db1)) + row.Scan(&dbId1) + + row = runner.QueryRow(t, fmt.Sprintf(`SELECT crdb_internal.get_database_id('%s') AS database_id;`, db2)) + row.Scan(&dbId2) + + runner.Exec(t, fmt.Sprintf(` INSERT INTO system.table_metadata (db_id, db_name, @@ -833,7 +906,6 @@ func setupTest(t *testing.T, conn *gosql.DB, db1 string, db2 string) (dbId1 int, (%[2]d, '%[4]s', 13, 'mySchema', 'myTable13', 'TABLE', 10001, 19, 509, 1000, .509, 11, 1, ARRAY[1, 2, 3], 'some error', '2025-06-20T00:00:12Z'), (%[1]d, '%[3]s', 14, 'mySchema1', 'myView1', 'VIEW', 0, 0, 0, 0, 0, 11, 0, ARRAY[], null, '2025-06-20T00:00:00Z') `, dbId1, dbId2, db1, db2)) - require.NoError(t, err) return dbId1, dbId2 }