From a41cd1694988859cf0d463ceb61a5f52191ae67e Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Fri, 8 Dec 2023 14:54:13 -0500 Subject: [PATCH 1/5] refactor and add --exclude-column --- tool/tsh/common/db.go | 6 +- tool/tsh/common/db_print.go | 72 ++++++++++++++++++++ tool/tsh/common/db_print_test.go | 112 +++++++++++++++++++++++++++++++ tool/tsh/common/tsh.go | 108 +++++++++++++---------------- 4 files changed, 233 insertions(+), 65 deletions(-) create mode 100644 tool/tsh/common/db_print.go create mode 100644 tool/tsh/common/db_print_test.go diff --git a/tool/tsh/common/db.go b/tool/tsh/common/db.go index 12a5fa51332d2..e513e186979bd 100644 --- a/tool/tsh/common/db.go +++ b/tool/tsh/common/db.go @@ -96,7 +96,7 @@ func onListDatabases(cf *CLIConf) error { } sort.Sort(types.Databases(databases)) - return trace.Wrap(showDatabases(cf.Stdout(), cf.SiteName, databases, activeDatabases, accessChecker, cf.Format, cf.Verbose)) + return trace.Wrap(showDatabases(cf, databases, activeDatabases, accessChecker)) } func accessCheckerForRemoteCluster(ctx context.Context, profile *client.ProfileStatus, proxy *client.ProxyClient, clusterName string) (services.AccessChecker, error) { @@ -232,7 +232,7 @@ func listDatabasesAllClusters(cf *CLIConf) error { format := strings.ToLower(cf.Format) switch format { case teleport.Text, "": - printDatabasesWithClusters(cf.SiteName, dbListings, active, cf.Verbose) + printDatabasesWithClusters(cf, dbListings, active) case teleport.JSON, teleport.YAML: out, err := serializeDatabasesAllClusters(dbListings, format) if err != nil { @@ -1669,7 +1669,7 @@ func formatAmbiguousDB(cf *CLIConf, selectors resourceSelectors, matchedDBs type var checker services.AccessChecker var sb strings.Builder verbose := true - showDatabasesAsText(&sb, cf.SiteName, matchedDBs, activeDBs, checker, verbose) + showDatabasesAsText(cf, &sb, matchedDBs, activeDBs, checker, verbose) listCommand := formatDatabaseListCommand(cf.SiteName) fullNameExample := matchedDBs[0].GetName() diff --git a/tool/tsh/common/db_print.go b/tool/tsh/common/db_print.go new file mode 100644 index 0000000000000..bd51122026fe0 --- /dev/null +++ b/tool/tsh/common/db_print.go @@ -0,0 +1,72 @@ +/* + * Teleport + * Copyright (C) 2023 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package common + +import ( + "fmt" + "io" + + "golang.org/x/exp/slices" + + "github.com/gravitational/teleport/lib/asciitable" +) + +type printDatabaseTableConfig struct { + writer io.Writer + rows [][]string + showProxyAndCluster bool + verbose bool + excludeColumns []string +} + +func printDatabaseTable(cfg printDatabaseTableConfig) { + allColumns := databaseTableColumns() + if !cfg.showProxyAndCluster { + cfg.excludeColumns = append(cfg.excludeColumns, "Proxy", "Cluster") + } + if !cfg.verbose { + cfg.excludeColumns = append(cfg.excludeColumns, "Protocol", "Type", "URI") + } + + var printColumns []string + printRows := make([][]string, len(cfg.rows)) + + for columnIndex, column := range allColumns { + if slices.Contains(cfg.excludeColumns, column) { + continue + } + + printColumns = append(printColumns, column) + for rowIndex, row := range cfg.rows { + printRows[rowIndex] = append(printRows[rowIndex], row[columnIndex]) + } + } + + var t asciitable.Table + if cfg.verbose { + t = asciitable.MakeTable(printColumns, printRows...) + } else { + t = asciitable.MakeTableWithTruncatedColumn(printColumns, printRows, "Labels") + } + fmt.Fprintln(cfg.writer, t.AsBuffer().String()) +} + +func databaseTableColumns() []string { + return []string{"Proxy", "Cluster", "Name", "Description", "Protocol", "Type", "URI", "Allowed Users", "Labels", "Connect"} +} diff --git a/tool/tsh/common/db_print_test.go b/tool/tsh/common/db_print_test.go new file mode 100644 index 0000000000000..74bc33bd49671 --- /dev/null +++ b/tool/tsh/common/db_print_test.go @@ -0,0 +1,112 @@ +/* + * Teleport + * Copyright (C) 2023 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package common + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_printDatabaseTable(t *testing.T) { + t.Parallel() + + rows := [][]string{ + {"proxy", "cluster1", "db1", "describe db1", "postgres", "self-hosted", "localhost:5432", "[*]", "Env=dev", "tsh db connect db1"}, + {"proxy", "cluster1", "db2", "describe db2", "mysql", "self-hosted", "localhost:3306", "[alice] (Auto-provisioned)", "Env=prod", ""}, + } + + tests := []struct { + name string + cfg printDatabaseTableConfig + expect string + }{ + { + name: "tsh db ls", + cfg: printDatabaseTableConfig{ + rows: rows, + showProxyAndCluster: false, + verbose: false, + }, + // os.Stdin.Fd() fails during go test, so width is defaulted to 80 for truncated table. + expect: `Name Description Allowed Users Labels Connect +---- ------------ ------------------- -------- ------------------- +db1 describe db1 [*] Env=dev tsh db connect d... +db2 describe db2 [alice] (Auto-pr... Env=prod + +`, + }, + { + name: "tsh db ls -E Description -E Labels -E Connect", + cfg: printDatabaseTableConfig{ + rows: rows, + showProxyAndCluster: false, + verbose: false, + excludeColumns: []string{"Description", "Labels", "Connect"}, + }, + expect: `Name Allowed Users +---- -------------------------- +db1 [*] +db2 [alice] (Auto-provisioned) + +`, + }, + { + name: "tsh db ls -v", + cfg: printDatabaseTableConfig{ + rows: rows, + showProxyAndCluster: false, + verbose: true, + }, + expect: `Name Description Protocol Type URI Allowed Users Labels Connect +---- ------------ -------- ----------- -------------- -------------------------- -------- ------------------ +db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 +db2 describe db2 mysql self-hosted localhost:3306 [alice] (Auto-provisioned) Env=prod + +`, + }, + { + name: "tsh db ls -v -A", + cfg: printDatabaseTableConfig{ + rows: rows, + showProxyAndCluster: true, + verbose: true, + }, + expect: `Proxy Cluster Name Description Protocol Type URI Allowed Users Labels Connect +----- -------- ---- ------------ -------- ----------- -------------- -------------------------- -------- ------------------ +proxy cluster1 db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 +proxy cluster1 db2 describe db2 mysql self-hosted localhost:3306 [alice] (Auto-provisioned) Env=prod + +`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var sb strings.Builder + + cfg := test.cfg + cfg.writer = &sb + + printDatabaseTable(cfg) + require.Equal(t, test.expect, sb.String()) + }) + } +} diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index 1c2ca1a0e4769..ba9b11632325b 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -496,6 +496,9 @@ type CLIConf struct { // SSHLogDir is the directory to log the output of multiple SSH commands to. // If not set, no logs will be created. SSHLogDir string + + // ExcludeTableColumns is a list of columns to hide when printing tables. + ExcludeTableColumns []string } // Stdout returns the stdout writer. @@ -852,6 +855,7 @@ func Run(ctx context.Context, args []string, opts ...CliOption) error { dbList.Flag("query", queryHelp).StringVar(&cf.PredicateExpression) dbList.Flag("format", defaults.FormatFlagDescription(defaults.DefaultFormats...)).Short('f').Default(teleport.Text).EnumVar(&cf.Format, defaults.DefaultFormats...) dbList.Flag("all", "List databases from all clusters and proxies.").Short('R').BoolVar(&cf.ListAll) + dbList.Flag("exclude-column", "Exclude columns from table (e.g --exclude-column Labels -E Description).").Short('E').StringsVar(&cf.ExcludeTableColumns) dbList.Arg("labels", labelHelp).StringVar(&cf.Labels) dbLogin := db.Command("login", "Retrieve credentials for a database.") // don't require positional argument, user can select with --labels/--query alone. @@ -2753,17 +2757,17 @@ func showAppsAsText(apps []types.Application, active []tlsca.RouteToApp, verbose fmt.Println(t.AsBuffer().String()) } -func showDatabases(w io.Writer, clusterFlag string, databases []types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker, format string, verbose bool) error { - format = strings.ToLower(format) +func showDatabases(cf *CLIConf, databases []types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker) error { + format := strings.ToLower(cf.Format) switch format { case teleport.Text, "": - showDatabasesAsText(w, clusterFlag, databases, active, accessChecker, verbose) + showDatabasesAsText(cf, cf.Stdout(), databases, active, accessChecker, cf.Verbose) case teleport.JSON, teleport.YAML: - out, err := serializeDatabases(databases, format, accessChecker) + out, err := serializeDatabases(databases, cf.Format, accessChecker) if err != nil { return trace.Wrap(err) } - fmt.Fprintln(w, out) + fmt.Fprintln(cf.Stdout(), out) default: return trace.BadParameter("unsupported format %q", format) } @@ -2901,17 +2905,16 @@ func formatUsersForDB(database types.Database, accessChecker services.AccessChec return fmt.Sprintf("%v, except: %v", dbUsers.Allowed, dbUsers.Denied) } +// TODO(greedy52) more refactoring on db printing and move them to db_print.go. + func getDatabaseRow(proxy, cluster, clusterFlag string, database types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker, verbose bool) []string { - name := database.GetName() displayName := common.FormatResourceName(database, verbose) var connect string for _, a := range active { - if a.ServiceName == name { - a.ServiceName = displayName + if a.ServiceName == database.GetName() { // format the db name with the display name - displayName = formatActiveDB(a) + displayName = formatActiveDB(a, displayName) // then revert it for connect string - a.ServiceName = name switch a.Protocol { case defaults.ProtocolDynamoDB: // DynamoDB does not support "tsh db connect", so print the proxy command instead. @@ -2923,81 +2926,62 @@ func getDatabaseRow(proxy, cluster, clusterFlag string, database types.Database, } } - row := make([]string, 0) - if proxy != "" && cluster != "" { - row = append(row, proxy, cluster) + // Must match databaseTableColumns(). + return []string{ + proxy, + cluster, + displayName, + database.GetDescription(), + database.GetProtocol(), + database.GetType(), + database.GetURI(), + formatUsersForDB(database, accessChecker), + common.FormatLabels(database.GetAllLabels(), verbose), + connect, } - - labels := common.FormatLabels(database.GetAllLabels(), verbose) - if verbose { - row = append(row, - displayName, - database.GetDescription(), - database.GetProtocol(), - database.GetType(), - database.GetURI(), - formatUsersForDB(database, accessChecker), - labels, - connect, - ) - } else { - row = append(row, - displayName, - database.GetDescription(), - formatUsersForDB(database, accessChecker), - labels, - connect, - ) - } - - return row } -func showDatabasesAsText(w io.Writer, clusterFlag string, databases []types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker, verbose bool) { +func showDatabasesAsText(cf *CLIConf, w io.Writer, databases []types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker, verbose bool) { var rows [][]string for _, database := range databases { rows = append(rows, getDatabaseRow("", "", - clusterFlag, + cf.SiteName, database, active, accessChecker, verbose)) } - var t asciitable.Table - if verbose { - t = asciitable.MakeTable([]string{"Name", "Description", "Protocol", "Type", "URI", "Allowed Users", "Labels", "Connect"}, rows...) - } else { - t = asciitable.MakeTableWithTruncatedColumn([]string{"Name", "Description", "Allowed Users", "Labels", "Connect"}, rows, "Labels") - } - fmt.Fprintln(w, t.AsBuffer().String()) + printDatabaseTable(printDatabaseTableConfig{ + writer: w, + rows: rows, + verbose: verbose, + excludeColumns: cf.ExcludeTableColumns, + }) } -func printDatabasesWithClusters(clusterFlag string, dbListings []databaseListing, active []tlsca.RouteToDatabase, verbose bool) { +func printDatabasesWithClusters(cf *CLIConf, dbListings []databaseListing, active []tlsca.RouteToDatabase) { var rows [][]string for _, listing := range dbListings { rows = append(rows, getDatabaseRow( listing.Proxy, listing.Cluster, - clusterFlag, + cf.SiteName, listing.Database, active, listing.accessChecker, - verbose)) - } - var t asciitable.Table - if verbose { - t = asciitable.MakeTable([]string{"Proxy", "Cluster", "Name", "Description", "Protocol", "Type", "URI", "Allowed Users", "Labels", "Connect", "Expires"}, rows...) - } else { - t = asciitable.MakeTableWithTruncatedColumn( - []string{"Proxy", "Cluster", "Name", "Description", "Allowed Users", "Labels", "Connect"}, - rows, - "Labels", - ) - } - fmt.Println(t.AsBuffer().String()) + cf.Verbose)) + } + printDatabaseTable(printDatabaseTableConfig{ + writer: cf.Stdout(), + rows: rows, + showProxyAndCluster: true, + verbose: cf.Verbose, + excludeColumns: cf.ExcludeTableColumns, + }) } -func formatActiveDB(active tlsca.RouteToDatabase) string { +func formatActiveDB(active tlsca.RouteToDatabase, displayName string) string { + active.ServiceName = displayName switch { case active.Username != "" && active.Database != "": return fmt.Sprintf("> %v (user: %v, db: %v)", active.ServiceName, active.Username, active.Database) From 4df07d669f2799fcd842486d5fe70e7fda2cebce Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Fri, 8 Dec 2023 16:43:26 -0500 Subject: [PATCH 2/5] add Database Roles column --- tool/tsh/common/db_print.go | 31 ++++++-- tool/tsh/common/db_print_test.go | 118 ++++++++++++++++++++++++++----- tool/tsh/common/tsh.go | 3 +- 3 files changed, 125 insertions(+), 27 deletions(-) diff --git a/tool/tsh/common/db_print.go b/tool/tsh/common/db_print.go index bd51122026fe0..e255fcb6958c8 100644 --- a/tool/tsh/common/db_print.go +++ b/tool/tsh/common/db_print.go @@ -21,10 +21,11 @@ package common import ( "fmt" "io" + "slices" - "golang.org/x/exp/slices" - + "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/asciitable" + "github.com/gravitational/teleport/lib/services" ) type printDatabaseTableConfig struct { @@ -35,19 +36,22 @@ type printDatabaseTableConfig struct { excludeColumns []string } +func (cfg printDatabaseTableConfig) allColumnTitles() []string { + return []string{"Proxy", "Cluster", "Name", "Description", "Protocol", "Type", "URI", "Allowed Users", "Database Roles", "Labels", "Connect"} +} + func printDatabaseTable(cfg printDatabaseTableConfig) { - allColumns := databaseTableColumns() if !cfg.showProxyAndCluster { cfg.excludeColumns = append(cfg.excludeColumns, "Proxy", "Cluster") } if !cfg.verbose { - cfg.excludeColumns = append(cfg.excludeColumns, "Protocol", "Type", "URI") + cfg.excludeColumns = append(cfg.excludeColumns, "Protocol", "Type", "URI", "Database Roles") } var printColumns []string printRows := make([][]string, len(cfg.rows)) - for columnIndex, column := range allColumns { + for columnIndex, column := range cfg.allColumnTitles() { if slices.Contains(cfg.excludeColumns, column) { continue } @@ -67,6 +71,19 @@ func printDatabaseTable(cfg printDatabaseTableConfig) { fmt.Fprintln(cfg.writer, t.AsBuffer().String()) } -func databaseTableColumns() []string { - return []string{"Proxy", "Cluster", "Name", "Description", "Protocol", "Type", "URI", "Allowed Users", "Labels", "Connect"} +func formatDatabaseRolesForDB(database types.Database, accessChecker services.AccessChecker) string { + if database.SupportsAutoUsers() && database.GetAdminUser().Name != "" { + // may happen if fetching the role set failed for any reason. + if accessChecker == nil { + return "(unknown)" + } + + autoUser, roles, err := accessChecker.CheckDatabaseRoles(database) + if err != nil { + log.Warnf("Failed to CheckDatabaseRoles for database %v: %v.", database.GetName(), err) + } else if autoUser.IsEnabled() { + return fmt.Sprintf("%v", roles) + } + } + return "" } diff --git a/tool/tsh/common/db_print_test.go b/tool/tsh/common/db_print_test.go index 74bc33bd49671..4c5811b48a9a5 100644 --- a/tool/tsh/common/db_print_test.go +++ b/tool/tsh/common/db_print_test.go @@ -23,14 +23,18 @@ import ( "testing" "github.com/stretchr/testify/require" + + apidefaults "github.com/gravitational/teleport/api/defaults" + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/services" ) func Test_printDatabaseTable(t *testing.T) { t.Parallel() rows := [][]string{ - {"proxy", "cluster1", "db1", "describe db1", "postgres", "self-hosted", "localhost:5432", "[*]", "Env=dev", "tsh db connect db1"}, - {"proxy", "cluster1", "db2", "describe db2", "mysql", "self-hosted", "localhost:3306", "[alice] (Auto-provisioned)", "Env=prod", ""}, + {"proxy", "cluster1", "db1", "describe db1", "postgres", "self-hosted", "localhost:5432", "[*]", "", "Env=dev", "tsh db connect db1"}, + {"proxy", "cluster1", "db2", "describe db2", "mysql", "self-hosted", "localhost:3306", "[alice]", "[readonly]", "Env=prod", ""}, } tests := []struct { @@ -46,10 +50,10 @@ func Test_printDatabaseTable(t *testing.T) { verbose: false, }, // os.Stdin.Fd() fails during go test, so width is defaulted to 80 for truncated table. - expect: `Name Description Allowed Users Labels Connect ----- ------------ ------------------- -------- ------------------- -db1 describe db1 [*] Env=dev tsh db connect d... -db2 describe db2 [alice] (Auto-pr... Env=prod + expect: `Name Description Allowed Users Labels Connect +---- ------------ ------------- -------- ------------------- +db1 describe db1 [*] Env=dev tsh db connect d... +db2 describe db2 [alice] Env=prod `, }, @@ -61,10 +65,10 @@ db2 describe db2 [alice] (Auto-pr... Env=prod verbose: false, excludeColumns: []string{"Description", "Labels", "Connect"}, }, - expect: `Name Allowed Users ----- -------------------------- -db1 [*] -db2 [alice] (Auto-provisioned) + expect: `Name Allowed Users +---- ------------- +db1 [*] +db2 [alice] `, }, @@ -75,24 +79,25 @@ db2 [alice] (Auto-provisioned) showProxyAndCluster: false, verbose: true, }, - expect: `Name Description Protocol Type URI Allowed Users Labels Connect ----- ------------ -------- ----------- -------------- -------------------------- -------- ------------------ -db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 -db2 describe db2 mysql self-hosted localhost:3306 [alice] (Auto-provisioned) Env=prod + expect: `Name Description Protocol Type URI Allowed Users Database Roles Labels Connect +---- ------------ -------- ----------- -------------- ------------- -------------- -------- ------------------ +db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 +db2 describe db2 mysql self-hosted localhost:3306 [alice] [readonly] Env=prod `, }, { - name: "tsh db ls -v -A", + name: "tsh db ls -v -R -E 'Database Roles'", cfg: printDatabaseTableConfig{ rows: rows, showProxyAndCluster: true, verbose: true, + excludeColumns: []string{"Database Roles"}, }, - expect: `Proxy Cluster Name Description Protocol Type URI Allowed Users Labels Connect ------ -------- ---- ------------ -------- ----------- -------------- -------------------------- -------- ------------------ -proxy cluster1 db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 -proxy cluster1 db2 describe db2 mysql self-hosted localhost:3306 [alice] (Auto-provisioned) Env=prod + expect: `Proxy Cluster Name Description Protocol Type URI Allowed Users Labels Connect +----- -------- ---- ------------ -------- ----------- -------------- ------------- -------- ------------------ +proxy cluster1 db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 +proxy cluster1 db2 describe db2 mysql self-hosted localhost:3306 [alice] Env=prod `, }, @@ -110,3 +115,78 @@ proxy cluster1 db2 describe db2 mysql self-hosted localhost:3306 [alice] (Au }) } } + +func Test_formatDatabaseRolesForDB(t *testing.T) { + t.Parallel() + + db, err := types.NewDatabaseV3(types.Metadata{ + Name: "db", + }, types.DatabaseSpecV3{ + Protocol: "postgres", + URI: "localhost:5432", + }) + require.NoError(t, err) + + dbWithAutoUser, err := types.NewDatabaseV3(types.Metadata{ + Name: "dbWithAutoUser", + Labels: map[string]string{"env": "prod"}, + }, types.DatabaseSpecV3{ + Protocol: "postgres", + URI: "localhost:5432", + AdminUser: &types.DatabaseAdminUser{ + Name: "teleport-admin", + }, + }) + require.NoError(t, err) + + roleAutoUser := &types.RoleV6{ + Metadata: types.Metadata{Name: "auto-user", Namespace: apidefaults.Namespace}, + Spec: types.RoleSpecV6{ + Options: types.RoleOptions{ + CreateDatabaseUserMode: types.CreateDatabaseUserMode_DB_USER_MODE_KEEP, + }, + Allow: types.RoleConditions{ + Namespaces: []string{apidefaults.Namespace}, + DatabaseLabels: types.Labels{"env": []string{"prod"}}, + DatabaseRoles: []string{"roleA", "roleB"}, + DatabaseNames: []string{"*"}, + DatabaseUsers: []string{types.Wildcard}, + }, + }, + } + + tests := []struct { + name string + database types.Database + accessChecker services.AccessChecker + expect string + }{ + { + name: "nil accessChecker", + database: dbWithAutoUser, + expect: "(unknown)", + }, + { + name: "roles", + database: dbWithAutoUser, + accessChecker: services.NewAccessCheckerWithRoleSet(&services.AccessInfo{ + Username: "alice", + }, "clustername", services.RoleSet{roleAutoUser}), + expect: "[roleA roleB]", + }, + { + name: "db without admin user", + database: db, + accessChecker: services.NewAccessCheckerWithRoleSet(&services.AccessInfo{ + Username: "alice", + }, "clustername", services.RoleSet{roleAutoUser}), + expect: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.expect, formatDatabaseRolesForDB(test.database, test.accessChecker)) + }) + } +} diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index ba9b11632325b..4ea798ef99788 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -2926,7 +2926,7 @@ func getDatabaseRow(proxy, cluster, clusterFlag string, database types.Database, } } - // Must match databaseTableColumns(). + // Must match printDatabaseTableConfig.allColumnTitles(). return []string{ proxy, cluster, @@ -2936,6 +2936,7 @@ func getDatabaseRow(proxy, cluster, clusterFlag string, database types.Database, database.GetType(), database.GetURI(), formatUsersForDB(database, accessChecker), + formatDatabaseRolesForDB(database, accessChecker), common.FormatLabels(database.GetAllLabels(), verbose), connect, } From d09f1b9650d1ecbf6a46b7d7542747fcaf837f1f Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Fri, 8 Dec 2023 17:02:28 -0500 Subject: [PATCH 3/5] add database_roles to json,yaml --- tool/tsh/common/tsh.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index 4ea798ef99788..eaedd26fb26fa 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -2814,7 +2814,8 @@ type databaseWithUsers struct { // *DatabaseV3 is used instead of types.Database because we want the db fields marshaled to JSON inline. // An embedded interface (like types.Database) does not inline when marshaled to JSON. *types.DatabaseV3 - Users *dbUsers `json:"users"` + Users *dbUsers `json:"users"` + DatabaseRoles []string `json:"database_roles,omitempty"` } func getDBUsers(db types.Database, accessChecker services.AccessChecker) *dbUsers { @@ -2847,6 +2848,15 @@ func newDatabaseWithUsers(db types.Database, accessChecker services.AccessChecke default: return nil, trace.BadParameter("unrecognized database type %T", db) } + + if db.SupportsAutoUsers() && db.GetAdminUser().Name != "" { + autoUser, roles, err := accessChecker.CheckDatabaseRoles(db) + if err != nil { + log.Warnf("Failed to CheckDatabaseRoles for database %v: %v.", db.GetName(), err) + } else if autoUser.IsEnabled() { + dbWithUsers.DatabaseRoles = roles + } + } return dbWithUsers, nil } From 0b9baeafc025d8163a66bdb8c917e681fbf6fe53 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Tue, 12 Dec 2023 11:01:34 -0500 Subject: [PATCH 4/5] remove --exclude-column and refactor databaseTableRow --- tool/tsh/common/db_print.go | 75 ++++++++++++++++++++++++++------ tool/tsh/common/db_print_test.go | 56 +++++++++++++----------- tool/tsh/common/tsh.go | 43 ++++++++---------- 3 files changed, 110 insertions(+), 64 deletions(-) diff --git a/tool/tsh/common/db_print.go b/tool/tsh/common/db_print.go index e255fcb6958c8..34fdc89fcd278 100644 --- a/tool/tsh/common/db_print.go +++ b/tool/tsh/common/db_print.go @@ -21,43 +21,90 @@ package common import ( "fmt" "io" - "slices" + "reflect" + "regexp" + + "golang.org/x/exp/slices" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/asciitable" "github.com/gravitational/teleport/lib/services" ) +type databaseTableRow struct { + Proxy string + Cluster string + DisplayName string `title:"Name"` + Description string + Protocol string + Type string + URI string + AllowedUsers string + DatabaseRoles string + Labels string + Connect string +} + +func makeTableColumnTitles(row any) (out []string) { + // Regular expression to convert from "DatabaseRoles" to "Database Roles" etc. + re := regexp.MustCompile(`([a-z])([A-Z])`) + + t := reflect.TypeOf(row) + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + title := field.Tag.Get("title") + if title == "" { + title = re.ReplaceAllString(field.Name, "${1} ${2}") + } + out = append(out, title) + } + return out +} + +func makeTableRows[T any](rows []T) [][]string { + out := make([][]string, 0, len(rows)) + for _, row := range rows { + var columnValues []string + v := reflect.ValueOf(row) + for i := 0; i < v.NumField(); i++ { + columnValues = append(columnValues, fmt.Sprintf("%v", v.Field(i))) + } + out = append(out, columnValues) + } + return out +} + type printDatabaseTableConfig struct { writer io.Writer - rows [][]string + rows []databaseTableRow showProxyAndCluster bool verbose bool - excludeColumns []string } -func (cfg printDatabaseTableConfig) allColumnTitles() []string { - return []string{"Proxy", "Cluster", "Name", "Description", "Protocol", "Type", "URI", "Allowed Users", "Database Roles", "Labels", "Connect"} -} - -func printDatabaseTable(cfg printDatabaseTableConfig) { +func (cfg printDatabaseTableConfig) excludeColumns() (out []string) { if !cfg.showProxyAndCluster { - cfg.excludeColumns = append(cfg.excludeColumns, "Proxy", "Cluster") + out = append(out, "Proxy", "Cluster") } if !cfg.verbose { - cfg.excludeColumns = append(cfg.excludeColumns, "Protocol", "Type", "URI", "Database Roles") + out = append(out, "Protocol", "Type", "URI", "Database Roles") } + return out +} + +func printDatabaseTable(cfg printDatabaseTableConfig) { + allColumns := makeTableColumnTitles(databaseTableRow{}) + rowsWithAllColumns := makeTableRows(cfg.rows) + excludeColumns := cfg.excludeColumns() var printColumns []string printRows := make([][]string, len(cfg.rows)) - - for columnIndex, column := range cfg.allColumnTitles() { - if slices.Contains(cfg.excludeColumns, column) { + for columnIndex, column := range allColumns { + if slices.Contains(excludeColumns, column) { continue } printColumns = append(printColumns, column) - for rowIndex, row := range cfg.rows { + for rowIndex, row := range rowsWithAllColumns { printRows[rowIndex] = append(printRows[rowIndex], row[columnIndex]) } } diff --git a/tool/tsh/common/db_print_test.go b/tool/tsh/common/db_print_test.go index 4c5811b48a9a5..4ce92d0fdbcbe 100644 --- a/tool/tsh/common/db_print_test.go +++ b/tool/tsh/common/db_print_test.go @@ -32,9 +32,31 @@ import ( func Test_printDatabaseTable(t *testing.T) { t.Parallel() - rows := [][]string{ - {"proxy", "cluster1", "db1", "describe db1", "postgres", "self-hosted", "localhost:5432", "[*]", "", "Env=dev", "tsh db connect db1"}, - {"proxy", "cluster1", "db2", "describe db2", "mysql", "self-hosted", "localhost:3306", "[alice]", "[readonly]", "Env=prod", ""}, + rows := []databaseTableRow{ + databaseTableRow{ + Proxy: "proxy", + Cluster: "cluster1", + DisplayName: "db1", + Description: "describe db1", + Protocol: "postgres", + Type: "self-hosted", + URI: "localhost:5432", + AllowedUsers: "[*]", + Labels: "Env=dev", + Connect: "tsh db connect db1", + }, + databaseTableRow{ + Proxy: "proxy", + Cluster: "cluster1", + DisplayName: "db2", + Description: "describe db2", + Protocol: "mysql", + Type: "self-hosted", + URI: "localhost:3306", + AllowedUsers: "[alice]", + DatabaseRoles: "[readonly]", + Labels: "Env=prod", + }, } tests := []struct { @@ -58,22 +80,7 @@ db2 describe db2 [alice] Env=prod `, }, { - name: "tsh db ls -E Description -E Labels -E Connect", - cfg: printDatabaseTableConfig{ - rows: rows, - showProxyAndCluster: false, - verbose: false, - excludeColumns: []string{"Description", "Labels", "Connect"}, - }, - expect: `Name Allowed Users ----- ------------- -db1 [*] -db2 [alice] - -`, - }, - { - name: "tsh db ls -v", + name: "tsh db ls --verbose", cfg: printDatabaseTableConfig{ rows: rows, showProxyAndCluster: false, @@ -87,17 +94,16 @@ db2 describe db2 mysql self-hosted localhost:3306 [alice] [readonly] `, }, { - name: "tsh db ls -v -R -E 'Database Roles'", + name: "tsh db ls --verbose --all", cfg: printDatabaseTableConfig{ rows: rows, showProxyAndCluster: true, verbose: true, - excludeColumns: []string{"Database Roles"}, }, - expect: `Proxy Cluster Name Description Protocol Type URI Allowed Users Labels Connect ------ -------- ---- ------------ -------- ----------- -------------- ------------- -------- ------------------ -proxy cluster1 db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 -proxy cluster1 db2 describe db2 mysql self-hosted localhost:3306 [alice] Env=prod + expect: `Proxy Cluster Name Description Protocol Type URI Allowed Users Database Roles Labels Connect +----- -------- ---- ------------ -------- ----------- -------------- ------------- -------------- -------- ------------------ +proxy cluster1 db1 describe db1 postgres self-hosted localhost:5432 [*] Env=dev tsh db connect db1 +proxy cluster1 db2 describe db2 mysql self-hosted localhost:3306 [alice] [readonly] Env=prod `, }, diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index eaedd26fb26fa..8aef6cabff6f3 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -496,9 +496,6 @@ type CLIConf struct { // SSHLogDir is the directory to log the output of multiple SSH commands to. // If not set, no logs will be created. SSHLogDir string - - // ExcludeTableColumns is a list of columns to hide when printing tables. - ExcludeTableColumns []string } // Stdout returns the stdout writer. @@ -855,7 +852,6 @@ func Run(ctx context.Context, args []string, opts ...CliOption) error { dbList.Flag("query", queryHelp).StringVar(&cf.PredicateExpression) dbList.Flag("format", defaults.FormatFlagDescription(defaults.DefaultFormats...)).Short('f').Default(teleport.Text).EnumVar(&cf.Format, defaults.DefaultFormats...) dbList.Flag("all", "List databases from all clusters and proxies.").Short('R').BoolVar(&cf.ListAll) - dbList.Flag("exclude-column", "Exclude columns from table (e.g --exclude-column Labels -E Description).").Short('E').StringsVar(&cf.ExcludeTableColumns) dbList.Arg("labels", labelHelp).StringVar(&cf.Labels) dbLogin := db.Command("login", "Retrieve credentials for a database.") // don't require positional argument, user can select with --labels/--query alone. @@ -2917,7 +2913,7 @@ func formatUsersForDB(database types.Database, accessChecker services.AccessChec // TODO(greedy52) more refactoring on db printing and move them to db_print.go. -func getDatabaseRow(proxy, cluster, clusterFlag string, database types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker, verbose bool) []string { +func getDatabaseRow(proxy, cluster, clusterFlag string, database types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker, verbose bool) databaseTableRow { displayName := common.FormatResourceName(database, verbose) var connect string for _, a := range active { @@ -2936,24 +2932,23 @@ func getDatabaseRow(proxy, cluster, clusterFlag string, database types.Database, } } - // Must match printDatabaseTableConfig.allColumnTitles(). - return []string{ - proxy, - cluster, - displayName, - database.GetDescription(), - database.GetProtocol(), - database.GetType(), - database.GetURI(), - formatUsersForDB(database, accessChecker), - formatDatabaseRolesForDB(database, accessChecker), - common.FormatLabels(database.GetAllLabels(), verbose), - connect, + return databaseTableRow{ + Proxy: proxy, + Cluster: cluster, + DisplayName: displayName, + Description: database.GetDescription(), + Protocol: database.GetProtocol(), + Type: database.GetType(), + URI: database.GetURI(), + AllowedUsers: formatUsersForDB(database, accessChecker), + DatabaseRoles: formatDatabaseRolesForDB(database, accessChecker), + Labels: common.FormatLabels(database.GetAllLabels(), verbose), + Connect: connect, } } func showDatabasesAsText(cf *CLIConf, w io.Writer, databases []types.Database, active []tlsca.RouteToDatabase, accessChecker services.AccessChecker, verbose bool) { - var rows [][]string + var rows []databaseTableRow for _, database := range databases { rows = append(rows, getDatabaseRow("", "", cf.SiteName, @@ -2963,15 +2958,14 @@ func showDatabasesAsText(cf *CLIConf, w io.Writer, databases []types.Database, a verbose)) } printDatabaseTable(printDatabaseTableConfig{ - writer: w, - rows: rows, - verbose: verbose, - excludeColumns: cf.ExcludeTableColumns, + writer: w, + rows: rows, + verbose: verbose, }) } func printDatabasesWithClusters(cf *CLIConf, dbListings []databaseListing, active []tlsca.RouteToDatabase) { - var rows [][]string + var rows []databaseTableRow for _, listing := range dbListings { rows = append(rows, getDatabaseRow( listing.Proxy, @@ -2987,7 +2981,6 @@ func printDatabasesWithClusters(cf *CLIConf, dbListings []databaseListing, activ rows: rows, showProxyAndCluster: true, verbose: cf.Verbose, - excludeColumns: cf.ExcludeTableColumns, }) } From 801e36ee2443ce1cf1ba95a2274663fcb7a21836 Mon Sep 17 00:00:00 2001 From: STeve Huang Date: Tue, 2 Jan 2024 10:34:01 -0500 Subject: [PATCH 5/5] change slices import --- tool/tsh/common/db_print.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tool/tsh/common/db_print.go b/tool/tsh/common/db_print.go index 34fdc89fcd278..a2d26c1f28586 100644 --- a/tool/tsh/common/db_print.go +++ b/tool/tsh/common/db_print.go @@ -23,8 +23,7 @@ import ( "io" "reflect" "regexp" - - "golang.org/x/exp/slices" + "slices" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/asciitable"