From 40200e853611cf4c3d312c28ee0fdcee66baa98d Mon Sep 17 00:00:00 2001 From: Alex McGrath Date: Tue, 15 Mar 2022 12:43:07 +0000 Subject: [PATCH] Reslove comments, move all occurences of teleport.dev to use a constant --- api/types/constants.go | 4 +- lib/asciitable/table.go | 13 +++-- lib/srv/desktop/discovery.go | 12 ++--- lib/srv/desktop/discovery_test.go | 10 ++-- lib/utils/jsontools_test.go | 5 +- tool/tctl/common/app_command.go | 2 +- tool/tctl/common/collection.go | 73 ++++++++++++++++++----------- tool/tctl/common/db_command.go | 2 +- tool/tctl/common/desktop_command.go | 2 +- tool/tctl/common/kube_command.go | 2 +- tool/tctl/common/node_command.go | 17 ++++--- tool/tsh/tsh.go | 4 +- tool/tsh/tsh_test.go | 1 + 13 files changed, 84 insertions(+), 63 deletions(-) diff --git a/api/types/constants.go b/api/types/constants.go index 28013622a2d78..52a4d64210e98 100644 --- a/api/types/constants.go +++ b/api/types/constants.go @@ -299,11 +299,11 @@ const ( const ( // TeleportNamespace is used as the namespace prefix for any // labels defined by teleport - TeleportNamespace = "teleport.dev/" + TeleportNamespace = "teleport.dev" // OriginLabel is a resource metadata label name used to identify a source // that the resource originates from. - OriginLabel = "teleport.dev/origin" + OriginLabel = TeleportNamespace + "/origin" // OriginConfigFile is an origin value indicating that the resource was // constructed as a default value. diff --git a/lib/asciitable/table.go b/lib/asciitable/table.go index 893d2af0d81e1..16c1d0f916975 100644 --- a/lib/asciitable/table.go +++ b/lib/asciitable/table.go @@ -53,24 +53,23 @@ func MakeHeadlessTable(columnCount int) Table { } } -// MakeTable creates a new instance of the table with given column names. -func MakeTable(headers []string) Table { +// MakeTable creates a new instance of the table with given column +// names. Optionally rows to be added to the table may be included. +func MakeTable(headers []string, rows ...[]string) Table { t := MakeHeadlessTable(len(headers)) for i := range t.columns { t.columns[i].Title = headers[i] t.columns[i].width = len(headers[i]) } - return t -} - -func MakeTableWithRows(headers []string, rows [][]string) Table { - t := MakeTable(headers) for _, row := range rows { t.AddRow(row) } return t } +// MakeTableWithTruncatedColumn creates a table where the column +// matching truncatedColumn will be shortened to account for terminal +// width. func MakeTableWithTruncatedColumn(columnOrder []string, rows [][]string, truncatedColumn string) Table { width, _, err := term.GetSize(int(os.Stdin.Fd())) if err != nil { diff --git a/lib/srv/desktop/discovery.go b/lib/srv/desktop/discovery.go index 21f2f208c3051..d89a3135a79c1 100644 --- a/lib/srv/desktop/discovery.go +++ b/lib/srv/desktop/discovery.go @@ -177,14 +177,14 @@ func (s *WindowsService) deleteDesktop(ctx context.Context, r types.ResourceWith } func applyLabelsFromLDAP(entry *ldap.Entry, labels map[string]string) { - labels["teleport.dev/dns_host_name"] = entry.GetAttributeValue(attrDNSHostName) - labels["teleport.dev/computer_name"] = entry.GetAttributeValue(attrName) - labels["teleport.dev/os"] = entry.GetAttributeValue(attrOS) - labels["teleport.dev/os_version"] = entry.GetAttributeValue(attrOSVersion) + labels[types.TeleportNamespace+"/dns_host_name"] = entry.GetAttributeValue(attrDNSHostName) + labels[types.TeleportNamespace+"/computer_name"] = entry.GetAttributeValue(attrName) + labels[types.TeleportNamespace+"/os"] = entry.GetAttributeValue(attrOS) + labels[types.TeleportNamespace+"/os_version"] = entry.GetAttributeValue(attrOSVersion) labels[types.OriginLabel] = types.OriginDynamic switch entry.GetAttributeValue(attrPrimaryGroupID) { case writableDomainControllerGroupID, readOnlyDomainControllerGroupID: - labels["teleport.dev/is_domain_controller"] = "true" + labels[types.TeleportNamespace+"/is_domain_controller"] = "true" } } @@ -193,7 +193,7 @@ func applyLabelsFromLDAP(entry *ldap.Entry, labels map[string]string) { func (s *WindowsService) ldapEntryToWindowsDesktop(ctx context.Context, entry *ldap.Entry, getHostLabels func(string) map[string]string) (types.ResourceWithLabels, error) { hostname := entry.GetAttributeValue(attrDNSHostName) labels := getHostLabels(hostname) - labels["teleport.dev/windows_domain"] = s.cfg.Domain + labels[types.TeleportNamespace+"/windows_domain"] = s.cfg.Domain applyLabelsFromLDAP(entry, labels) addrs, err := s.dnsResolver.LookupHost(ctx, hostname) diff --git a/lib/srv/desktop/discovery_test.go b/lib/srv/desktop/discovery_test.go index 5635f8fc3190e..6ad26a602a197 100644 --- a/lib/srv/desktop/discovery_test.go +++ b/lib/srv/desktop/discovery_test.go @@ -71,10 +71,10 @@ func TestAppliesLDAPLabels(t *testing.T) { applyLabelsFromLDAP(entry, l) require.Equal(t, l[types.OriginLabel], types.OriginDynamic) - require.Equal(t, l["teleport.dev/dns_host_name"], "foo.example.com") - require.Equal(t, l["teleport.dev/computer_name"], "foo") - require.Equal(t, l["teleport.dev/os"], "Windows Server") - require.Equal(t, l["teleport.dev/os_version"], "6.1") + require.Equal(t, l[types.TeleportNamespace+"/dns_host_name"], "foo.example.com") + require.Equal(t, l[types.TeleportNamespace+"/computer_name"], "foo") + require.Equal(t, l[types.TeleportNamespace+"/os"], "Windows Server") + require.Equal(t, l[types.TeleportNamespace+"/os_version"], "6.1") } func TestLabelsDomainControllers(t *testing.T) { @@ -109,7 +109,7 @@ func TestLabelsDomainControllers(t *testing.T) { l := make(map[string]string) applyLabelsFromLDAP(test.entry, l) - b, _ := strconv.ParseBool(l["teleport.dev/is_domain_controller"]) + b, _ := strconv.ParseBool(l[types.TeleportNamespace+"/is_domain_controller"]) test.assert(t, b) }) } diff --git a/lib/utils/jsontools_test.go b/lib/utils/jsontools_test.go index c4389e51f1011..8b55ffe814c31 100644 --- a/lib/utils/jsontools_test.go +++ b/lib/utils/jsontools_test.go @@ -19,6 +19,7 @@ import ( "bytes" "testing" + "github.com/gravitational/teleport/api/types" "github.com/stretchr/testify/require" ) @@ -28,8 +29,8 @@ import ( // operations that depend on the byte ordering fail (e.g. CompareAndSwap). func TestMarshalMapConsistency(t *testing.T) { value := map[string]string{ - "teleport.dev/foo": "1234", - "teleport.dev/bar": "5678", + types.TeleportNamespace + "/foo": "1234", + types.TeleportNamespace + "/bar": "5678", } compareTo, err := FastMarshal(value) diff --git a/tool/tctl/common/app_command.go b/tool/tctl/common/app_command.go index 56e52aead1fe5..711e18b7d01ce 100644 --- a/tool/tctl/common/app_command.go +++ b/tool/tctl/common/app_command.go @@ -58,7 +58,7 @@ func (c *AppsCommand) Initialize(app *kingpin.Application, config *service.Confi apps := app.Command("apps", "Operate on applications registered with the cluster.") c.appsList = apps.Command("ls", "List all applications registered with the cluster.") - c.appsList.Flag("format", "Output format, 'text', 'json', or 'yaml'").Default("text").StringVar(&c.format) + c.appsList.Flag("format", "Output format, 'text', 'json', or 'yaml'").Default(teleport.Text).StringVar(&c.format) c.appsList.Arg("labels", labelHelp).StringVar(&c.labels) c.appsList.Flag("search", searchHelp).StringVar(&c.searchKeywords) c.appsList.Flag("query", queryHelp).StringVar(&c.predicateExpr) diff --git a/tool/tctl/common/collection.go b/tool/tctl/common/collection.go index b603faf994b39..0a5bed166ff29 100644 --- a/tool/tctl/common/collection.go +++ b/tool/tctl/common/collection.go @@ -68,7 +68,7 @@ func (r *roleCollection) writeText(w io.Writer) error { headers := []string{"Role", "Allowed to login as", "Node Labels", "Access to resources"} var t asciitable.Table if r.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Access to resources") } @@ -138,19 +138,16 @@ func (s *serverCollection) resources() (r []types.Resource) { func (s *serverCollection) writeText(w io.Writer) error { var rows [][]string - for _, s := range s.servers { - addr := s.GetPublicAddr() - if addr == "" { - addr = s.GetAddr() - } + for _, se := range s.servers { + labels := stripInternalTeleportLabels(s.verbose, se.GetAllLabels()) rows = append(rows, []string{ - s.GetHostname(), s.GetName(), addr, s.LabelsString(), s.GetTeleportVersion(), + se.GetHostname(), se.GetName(), se.GetAddr(), labels, se.GetTeleportVersion(), }) } headers := []string{"Host", "UUID", "Public Address", "Labels", "Version"} var t asciitable.Table if s.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Labels") @@ -163,6 +160,15 @@ func (s *serverCollection) writeYaml(w io.Writer) error { return utils.WriteYAML(w, s.servers) } +func (s *serverCollection) writeJSON(w io.Writer) error { + data, err := json.MarshalIndent(s.resources(), "", " ") + if err != nil { + return trace.Wrap(err) + } + _, err = w.Write(data) + return trace.Wrap(err) +} + type userCollection struct { users []types.User } @@ -482,13 +488,14 @@ func (a *appServerCollection) writeText(w io.Writer) error { var rows [][]string for _, server := range a.servers { app := server.GetApp() + labels := stripInternalTeleportLabels(a.verbose, app.GetAllLabels()) rows = append(rows, []string{ - server.GetHostname(), app.GetName(), app.GetPublicAddr(), app.GetURI(), app.LabelsString(), server.GetTeleportVersion()}) + server.GetHostname(), app.GetName(), app.GetPublicAddr(), app.GetURI(), labels, server.GetTeleportVersion()}) } var t asciitable.Table headers := []string{"Host", "Name", "Public Address", "URI", "Labels", "Version"} if a.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Labels") } @@ -529,13 +536,14 @@ func (c *appCollection) resources() (r []types.Resource) { func (c *appCollection) writeText(w io.Writer) error { var rows [][]string for _, app := range c.apps { + labels := stripInternalTeleportLabels(c.verbose, app.GetAllLabels()) rows = append(rows, []string{ - app.GetName(), app.GetDescription(), app.GetURI(), app.GetPublicAddr(), app.LabelsString(), app.GetVersion()}) + app.GetName(), app.GetDescription(), app.GetURI(), app.GetPublicAddr(), labels, app.GetVersion()}) } headers := []string{"Name", "Description", "URI", "Public Address", "Labels", "Version"} var t asciitable.Table if c.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Labels") } @@ -645,19 +653,20 @@ func (c *databaseServerCollection) resources() (r []types.Resource) { func (c *databaseServerCollection) writeText(w io.Writer) error { var rows [][]string for _, server := range c.servers { + labels := stripInternalTeleportLabels(c.verbose, server.GetDatabase().GetAllLabels()) rows = append(rows, []string{ server.GetHostname(), server.GetDatabase().GetName(), server.GetDatabase().GetProtocol(), server.GetDatabase().GetURI(), - server.GetDatabase().LabelsString(), + labels, server.GetTeleportVersion(), }) } headers := []string{"Host", "Name", "Protocol", "URI", "Labels", "Version"} var t asciitable.Table if c.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Labels") } @@ -697,14 +706,15 @@ func (c *databaseCollection) resources() (r []types.Resource) { func (c *databaseCollection) writeText(w io.Writer) error { var rows [][]string for _, database := range c.databases { + labels := stripInternalTeleportLabels(c.verbose, database.GetAllLabels()) rows = append(rows, []string{ - database.GetName(), database.GetProtocol(), database.GetURI(), database.LabelsString(), + database.GetName(), database.GetProtocol(), database.GetURI(), labels, }) } headers := []string{"Name", "Protocol", "URI", "Labels"} var t asciitable.Table if c.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Labels") } @@ -791,24 +801,29 @@ type windowsDesktopAndServiceCollection struct { verbose bool } +func stripInternalTeleportLabels(verbose bool, labels map[string]string) string { + if verbose { // remove teleport.dev labels unless we're in verbose mode. + return types.LabelsAsString(labels, nil) + } + for key := range labels { + if strings.HasPrefix(key, types.TeleportNamespace+"/") { + delete(labels, key) + } + } + return types.LabelsAsString(labels, nil) +} + func (c *windowsDesktopAndServiceCollection) writeText(w io.Writer) error { var rows [][]string for _, d := range c.desktops { - labels := d.desktop.GetAllLabels() - if !c.verbose { // remove teleport.dev labels unless we're in verbose mode. - for key := range labels { - if strings.HasPrefix(key, types.TeleportNamespace) { - delete(labels, key) - } - } - } + labels := stripInternalTeleportLabels(c.verbose, d.desktop.GetAllLabels()) rows = append(rows, []string{d.service.GetHostname(), d.desktop.GetAddr(), - d.desktop.GetDomain(), types.LabelsAsString(labels, nil), d.service.GetTeleportVersion()}) + d.desktop.GetDomain(), labels, d.service.GetTeleportVersion()}) } headers := []string{"Host", "Address", "AD Domain", "Labels", "Version"} var t asciitable.Table if c.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Labels") } @@ -860,9 +875,11 @@ func (c *kubeServerCollection) writeText(w io.Writer) error { for _, server := range c.servers { kubes := server.GetKubernetesClusters() for _, kube := range kubes { + labels := stripInternalTeleportLabels(c.verbose, + types.CombineLabels(kube.StaticLabels, kube.DynamicLabels)) rows = append(rows, []string{ kube.Name, - types.LabelsAsString(kube.StaticLabels, kube.DynamicLabels), + labels, server.GetTeleportVersion(), }) } @@ -870,7 +887,7 @@ func (c *kubeServerCollection) writeText(w io.Writer) error { headers := []string{"Cluster", "Labels", "Version"} var t asciitable.Table if c.verbose { - t = asciitable.MakeTableWithRows(headers, rows) + t = asciitable.MakeTable(headers, rows...) } else { t = asciitable.MakeTableWithTruncatedColumn(headers, rows, "Labels") } diff --git a/tool/tctl/common/db_command.go b/tool/tctl/common/db_command.go index e13f4f2a8ccac..11426be1b081d 100644 --- a/tool/tctl/common/db_command.go +++ b/tool/tctl/common/db_command.go @@ -58,7 +58,7 @@ func (c *DBCommand) Initialize(app *kingpin.Application, config *service.Config) db := app.Command("db", "Operate on databases registered with the cluster.") c.dbList = db.Command("ls", "List all databases registered with the cluster.") - c.dbList.Flag("format", "Output format, 'text', 'json', or 'yaml'").Default("text").StringVar(&c.format) + c.dbList.Flag("format", "Output format, 'text', 'json', or 'yaml'").Default(teleport.Text).StringVar(&c.format) c.dbList.Arg("labels", labelHelp).StringVar(&c.labels) c.dbList.Flag("search", searchHelp).StringVar(&c.searchKeywords) c.dbList.Flag("query", queryHelp).StringVar(&c.predicateExpr) diff --git a/tool/tctl/common/desktop_command.go b/tool/tctl/common/desktop_command.go index 4c5d3ea23a3d5..ad92a1b443d6e 100644 --- a/tool/tctl/common/desktop_command.go +++ b/tool/tctl/common/desktop_command.go @@ -49,7 +49,7 @@ func (c *DesktopCommand) Initialize(app *kingpin.Application, config *service.Co desktop := app.Command("desktops", "Operate on registered desktops.") c.desktopList = desktop.Command("ls", "List all desktops registered with the cluster.") - c.desktopList.Flag("format", "Output format, 'text', 'json' or 'yaml'").Default("text").StringVar(&c.format) + c.desktopList.Flag("format", "Output format, 'text', 'json' or 'yaml'").Default(teleport.Text).StringVar(&c.format) c.desktopList.Flag("verbose", "Verbose table output, shows full label output").Short('v').BoolVar(&c.verbose) } diff --git a/tool/tctl/common/kube_command.go b/tool/tctl/common/kube_command.go index 5ac95f934851f..079eab31df597 100644 --- a/tool/tctl/common/kube_command.go +++ b/tool/tctl/common/kube_command.go @@ -48,7 +48,7 @@ func (c *KubeCommand) Initialize(app *kingpin.Application, config *service.Confi kube := app.Command("kube", "Operate on registered kubernetes clusters.") c.kubeList = kube.Command("ls", "List all kubernetes clusters registered with the cluster.") - c.kubeList.Flag("format", "Output format, 'text', 'json', or 'yaml'").Default("text").StringVar(&c.format) + c.kubeList.Flag("format", "Output format, 'text', 'json', or 'yaml'").Default(teleport.Text).StringVar(&c.format) c.kubeList.Flag("verbose", "Verbose table output, shows full label output").Short('v').BoolVar(&c.verbose) } diff --git a/tool/tctl/common/node_command.go b/tool/tctl/common/node_command.go index 0f70ccf1cf83b..5208268d7bca3 100644 --- a/tool/tctl/common/node_command.go +++ b/tool/tctl/common/node_command.go @@ -26,6 +26,7 @@ import ( "time" "github.com/gravitational/kingpin" + "github.com/gravitational/teleport" "github.com/gravitational/trace" log "github.com/sirupsen/logrus" @@ -81,12 +82,12 @@ func (c *NodeCommand) Initialize(app *kingpin.Application, config *service.Confi c.nodeAdd.Flag("roles", "Comma-separated list of roles for the new node to assume [node]").Default("node").StringVar(&c.roles) c.nodeAdd.Flag("ttl", "Time to live for a generated token").Default(defaults.ProvisioningTokenTTL.String()).DurationVar(&c.ttl) c.nodeAdd.Flag("token", "Custom token to use, autogenerated if not provided").StringVar(&c.token) - c.nodeAdd.Flag("format", "Output format, 'text' or 'json'").Hidden().Default("text").StringVar(&c.format) + c.nodeAdd.Flag("format", "Output format, 'text' or 'json'").Hidden().Default(teleport.Text).StringVar(&c.format) c.nodeAdd.Alias(AddNodeHelp) c.nodeList = nodes.Command("ls", "List all active SSH nodes within the cluster") c.nodeList.Flag("namespace", "Namespace of the nodes").Default(apidefaults.Namespace).StringVar(&c.namespace) - c.nodeList.Flag("format", "Output format, 'text', or 'yaml'").Default("text").StringVar(&c.lsFormat) + c.nodeList.Flag("format", "Output format, 'text', or 'yaml'").Default(teleport.Text).StringVar(&c.lsFormat) c.nodeList.Flag("verbose", "Verbose table output, shows full label output").Short('v').BoolVar(&c.verbose) c.nodeList.Alias(ListNodesHelp) c.nodeList.Arg("labels", labelHelp).StringVar(&c.labels) @@ -164,7 +165,7 @@ func (c *NodeCommand) Invite(client auth.ClientI) error { } // output format switch: - if c.format == "text" { + if c.format == teleport.Text { if roles.Include(types.RoleTrustedCluster) { fmt.Printf(trustedClusterMessage, token, int(c.ttl.Minutes())) } else { @@ -245,16 +246,20 @@ func (c *NodeCommand) ListActive(clt auth.ClientI) error { coll := &serverCollection{servers: nodes, verbose: c.verbose} switch c.lsFormat { - case "text": + case teleport.Text: if err := coll.writeText(os.Stdout); err != nil { return trace.Wrap(err) } - case "yaml": + case teleport.YAML: if err := coll.writeYaml(os.Stdout); err != nil { return trace.Wrap(err) } + case teleport.JSON: + if err := coll.writeJSON(os.Stdout); err != nil { + return trace.Wrap(err) + } default: - return trace.Errorf("Invalid format %s, only text and yaml are supported", c.lsFormat) + return trace.Errorf("Invalid format %s, only text, json and yaml are supported", c.lsFormat) } return nil } diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index f4663d6d10306..282622c9bd81a 100644 --- a/tool/tsh/tsh.go +++ b/tool/tsh/tsh.go @@ -358,8 +358,6 @@ const ( // establishment of a TCP connection, rather than the full HTTP round- // trip that we measure against, so some tweaking may be needed. proxyDefaultResolutionTimeout = 2 * time.Second - - teleportNamespace = "teleport.dev" ) // cliOption is used in tests to inject/override configuration within Run @@ -1474,7 +1472,7 @@ func sortedLabels(labels map[string]string) string { var namespaced []string var result []string for key, val := range labels { - if strings.HasPrefix(key, teleportNamespace+"/") { + if strings.HasPrefix(key, types.TeleportNamespace+"/") { teleportNamespaced = append(teleportNamespaced, key) continue } diff --git a/tool/tsh/tsh_test.go b/tool/tsh/tsh_test.go index 6c3796bcae6a2..31ba3c70cae09 100644 --- a/tool/tsh/tsh_test.go +++ b/tool/tsh/tsh_test.go @@ -25,6 +25,7 @@ import ( "net" "os" "path/filepath" + "strings" "testing" "time"