Clean up some utils (#57423)

- Remove some unused code
- Move some code that is only used once to the calling package
- Use testify assertions for checking the presence of files/directories

integration/hostuser_test.go

@@ -173,9 +173,9 @@ func TestRootHostUsersBackend(t *testing.T) {
 		require.NoError(t, sudoersbk.WriteSudoersFile("user.name", validSudoersEntry))
 		_, err = os.Stat(filepath.Join(sudoersTestDir, "teleport-hostuuid-user_name"))
 		require.NoError(t, err)
+
 		require.NoError(t, sudoersbk.RemoveSudoersFile("user.name"))
-		_, err = os.Stat(filepath.Join(sudoersTestDir, "teleport-hostuuid-user_name"))
-		require.True(t, os.IsNotExist(err))
+		require.NoFileExists(t, filepath.Join(sudoersTestDir, "teleport-hostuuid-user_name"))
 	})
 
 	t.Run("Test CreateHomeDirectory does not follow symlinks", func(t *testing.T) {
@@ -344,16 +344,14 @@ func TestRootHostUsers(t *testing.T) {
 		// delete the user and ensure the sudoers file got deleted
 		require.NoError(t, closer.Close())
 		require.NoError(t, sudoers.RemoveSudoers(testuser))
-		_, err = os.Stat(sudoersPath(testuser, uuid))
-		require.True(t, os.IsNotExist(err))
+		require.NoFileExists(t, sudoersPath(testuser, uuid))
 
 		// ensure invalid sudoers entries dont get written
 		err = sudoers.WriteSudoers(testuser,
 			[]string{"badsudoers entry!!!"},
 		)
 		require.Error(t, err)
-		_, err = os.Stat(sudoersPath(testuser, uuid))
-		require.True(t, os.IsNotExist(err))
+		require.NoFileExists(t, sudoersPath(testuser, uuid))
 	})
 
 	t.Run("test delete all users in teleport service group", func(t *testing.T) {

lib/client/sso/redirector.go

@@ -305,7 +305,32 @@ func (rd *Redirector) clickableURL(redirectURL, postForm string) string {
 		}
 	})
 
-	return utils.ClickableURL(rd.baseURL() + shortPath)
+	return clickableURL(rd.baseURL() + shortPath)
+}
+
+// clickableURL fixes the address in a URL to make sure
+// it's clickable, e.g. it replaces "undefined" addresses like
+// 0.0.0.0 used in network listeners with the loopback address.
+func clickableURL(in string) string {
+	out, err := url.Parse(in)
+	if err != nil {
+		return in
+	}
+	host, port, err := net.SplitHostPort(out.Host)
+	if err != nil {
+		return in
+	}
+	ip := net.ParseIP(host)
+	// If address is not an IP address, return it unchanged.
+	if ip == nil && out.Host != "" {
+		return out.String()
+	}
+	// if address is unspecified, e.g. all interfaces 0.0.0.0 or multicast,
+	// replace with localhost that is clickable
+	if len(ip) == 0 || ip.IsUnspecified() || ip.IsMulticast() {
+		out.Host = fmt.Sprintf("127.0.0.1:%v", port)
+	}
+	return out.String()
 }
 
 func (rd *Redirector) baseURL() string {

lib/client/sso/redirector_internal_test.go

@@ -0,0 +1,49 @@
+/*
+ * Teleport
+ * Copyright (C) 2025  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 <http://www.gnu.org/licenses/>.
+ */
+
+package sso
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestClickableURL tests clickable URL conversions
+func TestClickableURL(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		info string
+		in   string
+		out  string
+	}{
+		{info: "original URL is OK", in: "http://127.0.0.1:3000/hello", out: "http://127.0.0.1:3000/hello"},
+		{info: "unspecified IPV6", in: "http://[::]:5050/howdy", out: "http://127.0.0.1:5050/howdy"},
+		{info: "unspecified IPV4", in: "http://0.0.0.0:5050/howdy", out: "http://127.0.0.1:5050/howdy"},
+		{info: "specified IPV4", in: "http://192.168.1.1:5050/howdy", out: "http://192.168.1.1:5050/howdy"},
+		{info: "specified IPV6", in: "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5050/howdy", out: "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5050/howdy"},
+		{info: "hostname", in: "http://example.com:3000/howdy", out: "http://example.com:3000/howdy"},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.info, func(t *testing.T) {
+			out := clickableURL(testCase.in)
+			require.Equal(t, testCase.out, out)
+		})
+	}
+}

lib/config/configuration.go

@@ -69,6 +69,7 @@ import (
 	"github.com/gravitational/teleport/lib/utils"
 	awsregion "github.com/gravitational/teleport/lib/utils/aws/region"
 	logutils "github.com/gravitational/teleport/lib/utils/log"
+	libslices "github.com/gravitational/teleport/lib/utils/slices"
 )
 
 // CommandLineFlags stores command line flag values, it's a much simplified subset
@@ -2550,19 +2551,20 @@ func Configure(clf *CommandLineFlags, cfg *servicecfg.Config, legacyAppFlags boo
 	// 140-2 compliant.
 	if clf.FIPS {
 		// Make sure all cryptographic primitives are FIPS compliant.
-		err = utils.UintSliceSubset(defaults.FIPSCipherSuites, cfg.CipherSuites)
+		//
+		err = libslices.ContainsAll(defaults.FIPSCipherSuites, cfg.CipherSuites)
 		if err != nil {
 			return trace.BadParameter("non-FIPS compliant TLS cipher suite selected: %v", err)
 		}
-		err = utils.StringSliceSubset(defaults.FIPSCiphers, cfg.Ciphers)
+		err = libslices.ContainsAll(defaults.FIPSCiphers, cfg.Ciphers)
 		if err != nil {
 			return trace.BadParameter("non-FIPS compliant SSH cipher selected: %v", err)
 		}
-		err = utils.StringSliceSubset(defaults.FIPSKEXAlgorithms, cfg.KEXAlgorithms)
+		err = libslices.ContainsAll(defaults.FIPSKEXAlgorithms, cfg.KEXAlgorithms)
 		if err != nil {
 			return trace.BadParameter("non-FIPS compliant SSH kex algorithm selected: %v", err)
 		}
-		err = utils.StringSliceSubset(defaults.FIPSMACAlgorithms, cfg.MACAlgorithms)
+		err = libslices.ContainsAll(defaults.FIPSMACAlgorithms, cfg.MACAlgorithms)
 		if err != nil {
 			return trace.BadParameter("non-FIPS compliant SSH mac algorithm selected: %v", err)
 		}

lib/events/filesessions/filestream_test.go

@@ -165,10 +165,7 @@ func TestCompleteUpload(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, test.expectedContent, contents)
 
-			// Part files directory should no longer exists.
-			_, err = os.ReadDir(handler.uploadRootPath(*upload))
-			require.Error(t, err)
-			require.True(t, os.IsNotExist(err))
+			require.NoDirExists(t, handler.uploadRootPath(*upload))
 		})
 	}
 }

lib/release/release.go

@@ -27,11 +27,11 @@ import (
 	"net/http"
 	"os"
 
+	"github.com/dustin/go-humanize"
 	"github.com/gravitational/roundtrip"
 	"github.com/gravitational/trace"
 
 	"github.com/gravitational/teleport/api/types"
-	"github.com/gravitational/teleport/lib/utils"
 )
 
 // ClientConfig contains configuration for the release client
@@ -124,7 +124,7 @@ func (c *Client) ListReleases(ctx context.Context) ([]*types.Release, error) {
 				OS:          a.OS,
 				SHA256:      a.SHA256,
 				AssetSize:   a.Size,
-				DisplaySize: utils.ByteCount(a.Size),
+				DisplaySize: humanize.Bytes(uint64(a.Size)),
 				ReleaseIDs:  a.ReleaseIDs,
 				PublicURL:   a.PublicURL,
 			})

lib/service/service.go

@@ -1823,6 +1823,18 @@ func (process *TeleportProcess) exportUpgradeWindows(ctx context.Context, req pr
 	return clt.ExportUpgradeWindows(ctx, req)
 }
 
+// isGroupMember returns whether currently logged user is a member of a group
+func isGroupMember(gid int) (bool, error) {
+	groups, err := os.Getgroups()
+	if err != nil {
+		return false, trace.ConvertSystemError(err)
+	}
+	if slices.Contains(groups, gid) {
+		return true, nil
+	}
+	return false, nil
+}
+
 // adminCreds returns admin UID and GID settings based on the OS
 func adminCreds() (*int, *int, error) {
 	if runtime.GOOS != constants.LinuxOS {
@@ -1830,7 +1842,7 @@ func adminCreds() (*int, *int, error) {
 	}
 	// if the user member of adm linux group,
 	// make audit log folder readable by admins
-	isAdmin, err := utils.IsGroupMember(teleport.LinuxAdminGID)
+	isAdmin, err := isGroupMember(teleport.LinuxAdminGID)
 	if err != nil {
 		return nil, nil, trace.Wrap(err)
 	}

lib/utils/fs_test.go

@@ -408,8 +408,7 @@ func TestRemoveAllSecure(t *testing.T) {
 
 	require.NoError(t, RemoveAllSecure(""))
 	require.NoError(t, RemoveAllSecure(tempDir))
-	_, err = os.Stat(tempDir)
-	require.True(t, os.IsNotExist(err), "Directory should be removed: %v", err)
+	require.NoDirExists(t, tempDir)
 }
 
 func TestRemoveSecure(t *testing.T) {
@@ -419,8 +418,7 @@ func TestRemoveSecure(t *testing.T) {
 	require.NoError(t, f.Close())
 
 	require.NoError(t, RemoveSecure(f.Name()))
-	_, err = os.Stat(tempFile)
-	require.True(t, os.IsNotExist(err), "File should be removed: %v", err)
+	require.NoFileExists(t, tempFile)
 }
 
 func TestRemoveSecure_symlink(t *testing.T) {
@@ -428,6 +426,5 @@ func TestRemoveSecure_symlink(t *testing.T) {
 	require.NoError(t, os.Symlink("/tmp", symlink))
 
 	require.NoError(t, RemoveSecure(symlink))
-	_, err := os.Stat(symlink)
-	require.True(t, os.IsNotExist(err), "Symlink should be removed: %v", err)
+	require.NoFileExists(t, symlink)
 }

lib/utils/slices/slices.go

@@ -18,6 +18,12 @@
 
 package slices
 
+import (
+	"slices"
+
+	"github.com/gravitational/trace"
+)
+
 // FilterMapUnique applies a function to all elements of a slice and collects them.
 // The function returns the value to collect and whether the current element should be included.
 // Returned values are deduplicated.
@@ -54,6 +60,17 @@ func ToPointers[T any](in []T) []*T {
 	return out
 }
 
+// ContainsAll checks whether haystack contains all needles, reporting
+// error indicating the first unmatched value.
+func ContainsAll[S ~[]E, E comparable](haystack S, needles S) error {
+	for _, needle := range needles {
+		if !slices.Contains(haystack, needle) {
+			return trace.BadParameter("%v not in set", needle)
+		}
+	}
+	return nil
+}
+
 // FromPointers converts a slice of pointers to values to a slice of values.
 // Nil pointers are converted to zero-values.
 func FromPointers[T any](in []*T) []T {

lib/utils/utils.go

@@ -21,13 +21,11 @@ package utils
 import (
 	"context"
 	"errors"
-	"fmt"
 	"io"
 	"io/fs"
 	"log/slog"
 	"math/rand/v2"
 	"net"
-	"net/url"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -146,52 +144,6 @@ func ThisFunction() string {
 	return runtime.FuncForPC(pc[0]).Name()
 }
 
-// SyncString is a string value
-// that can be concurrently accessed
-type SyncString struct {
-	sync.Mutex
-	string
-}
-
-// Value returns value of the string
-func (s *SyncString) Value() string {
-	s.Lock()
-	defer s.Unlock()
-	return s.string
-}
-
-// Set sets the value of the string
-func (s *SyncString) Set(v string) {
-	s.Lock()
-	defer s.Unlock()
-	s.string = v
-}
-
-// ClickableURL fixes address in url to make sure
-// it's clickable, e.g. it replaces "undefined" address like
-// 0.0.0.0 used in network listeners format with loopback 127.0.0.1
-func ClickableURL(in string) string {
-	out, err := url.Parse(in)
-	if err != nil {
-		return in
-	}
-	host, port, err := net.SplitHostPort(out.Host)
-	if err != nil {
-		return in
-	}
-	ip := net.ParseIP(host)
-	// If address is not an IP address, return it unchanged.
-	if ip == nil && out.Host != "" {
-		return out.String()
-	}
-	// if address is unspecified, e.g. all interfaces 0.0.0.0 or multicast,
-	// replace with localhost that is clickable
-	if len(ip) == 0 || ip.IsUnspecified() || ip.IsMulticast() {
-		out.Host = fmt.Sprintf("127.0.0.1:%v", port)
-	}
-	return out.String()
-}
-
 // AsBool converts string to bool, in case of the value is empty
 // or unknown, defaults to false
 func AsBool(v string) bool {
@@ -258,19 +210,8 @@ func StringsSet(in []string) map[string]struct{} {
 	return out
 }
 
-// IsGroupMember returns whether currently logged user is a member of a group
-func IsGroupMember(gid int) (bool, error) {
-	groups, err := os.Getgroups()
-	if err != nil {
-		return false, trace.ConvertSystemError(err)
-	}
-	if slices.Contains(groups, gid) {
-		return true, nil
-	}
-	return false, nil
-}
-
-// DNSName extracts DNS name from host:port string.
+// DNSName extracts DNS name from host:port string,
+// returning an error if the hostname is an IP address.
 func DNSName(hostport string) (string, error) {
 	host, err := Host(hostport)
 	if err != nil {
@@ -485,58 +426,17 @@ func GetFreeTCPPorts(n int, offset ...int) (PortList, error) {
 	return PortList{ports: list}, nil
 }
 
-// StringSliceSubset returns true if b is a subset of a.
-func StringSliceSubset(a []string, b []string) error {
-	aset := make(map[string]bool)
-	for _, v := range a {
-		aset[v] = true
-	}
-
-	for _, v := range b {
-		_, ok := aset[v]
-		if !ok {
-			return trace.BadParameter("%v not in set", v)
-		}
-
-	}
-	return nil
-}
-
-// UintSliceSubset returns true if b is a subset of a.
-func UintSliceSubset(a []uint16, b []uint16) error {
-	aset := make(map[uint16]bool)
-	for _, v := range a {
-		aset[v] = true
-	}
-
-	for _, v := range b {
-		_, ok := aset[v]
-		if !ok {
-			return trace.BadParameter("%v not in set", v)
-		}
-
-	}
-	return nil
-}
-
 // RemoveFromSlice makes a copy of the slice and removes the passed in values from the copy.
 func RemoveFromSlice(slice []string, values ...string) []string {
-	output := make([]string, 0, len(slice))
-
-	remove := make(map[string]bool)
+	remove := make(map[string]struct{})
 	for _, value := range values {
-		remove[value] = true
+		remove[value] = struct{}{}
 	}
 
-	for _, s := range slice {
+	return slices.DeleteFunc(slice, func(s string) bool {
 		_, ok := remove[s]
-		if ok {
-			continue
-		}
-		output = append(output, s)
-	}
-
-	return output
+		return ok
+	})
 }
 
 // ChooseRandomString returns a random string from the given slice.
@@ -577,10 +477,7 @@ func AddrsFromStrings(s apiutils.Strings, defaultPort int) ([]NetAddr, error) {
 // FileExists checks whether a file exists at a given path
 func FileExists(fp string) bool {
 	_, err := os.Stat(fp)
-	if err != nil && os.IsNotExist(err) {
-		return false
-	}
-	return true
+	return !errors.Is(err, fs.ErrNotExist)
 }
 
 // StoreErrorOf stores the error returned by f within *err.
@@ -618,32 +515,6 @@ func ReadAtMost(r io.Reader, limit int64) ([]byte, error) {
 	return data, err
 }
 
-// HasPrefixAny determines if any of the string values have the given prefix.
-func HasPrefixAny(prefix string, values []string) bool {
-	for _, val := range values {
-		if strings.HasPrefix(val, prefix) {
-			return true
-		}
-	}
-
-	return false
-}
-
-// ByteCount converts a size in bytes to a human-readable string.
-func ByteCount(b int64) string {
-	const unit = 1000
-	if b < unit {
-		return fmt.Sprintf("%d B", b)
-	}
-	div, exp := int64(unit), 0
-	for n := b / unit; n >= unit; n /= unit {
-		div *= unit
-		exp++
-	}
-	return fmt.Sprintf("%.1f %cB",
-		float64(b)/float64(div), "kMGTPE"[exp])
-}
-
 // ErrLimitReached means that the read limit is reached.
 //
 // TODO(gavin): this should be converted to a 413 StatusRequestEntityTooLarge

lib/utils/utils_test.go

@@ -152,30 +152,6 @@ func TestMaxVersions(t *testing.T) {
 	}
 }
 
-// TestClickableURL tests clickable URL conversions
-func TestClickableURL(t *testing.T) {
-	t.Parallel()
-
-	testCases := []struct {
-		info string
-		in   string
-		out  string
-	}{
-		{info: "original URL is OK", in: "http://127.0.0.1:3000/hello", out: "http://127.0.0.1:3000/hello"},
-		{info: "unspecified IPV6", in: "http://[::]:5050/howdy", out: "http://127.0.0.1:5050/howdy"},
-		{info: "unspecified IPV4", in: "http://0.0.0.0:5050/howdy", out: "http://127.0.0.1:5050/howdy"},
-		{info: "specified IPV4", in: "http://192.168.1.1:5050/howdy", out: "http://192.168.1.1:5050/howdy"},
-		{info: "specified IPV6", in: "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5050/howdy", out: "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:5050/howdy"},
-		{info: "hostname", in: "http://example.com:3000/howdy", out: "http://example.com:3000/howdy"},
-	}
-	for _, testCase := range testCases {
-		t.Run(testCase.info, func(t *testing.T) {
-			out := ClickableURL(testCase.in)
-			require.Equal(t, testCase.out, out)
-		})
-	}
-}
-
 // TestParseAdvertiseAddr tests parsing of advertise address
 func TestParseAdvertiseAddr(t *testing.T) {
 	t.Parallel()
@@ -580,53 +556,3 @@ func TestReadAtMost(t *testing.T) {
 		})
 	}
 }
-
-func TestByteCount(t *testing.T) {
-	tt := []struct {
-		name     string
-		size     int64
-		expected string
-	}{
-		{
-			name:     "1 byte",
-			size:     1,
-			expected: "1 B",
-		},
-		{
-			name:     "2 byte2",
-			size:     2,
-			expected: "2 B",
-		},
-		{
-			name:     "1kb",
-			size:     1000,
-			expected: "1.0 kB",
-		},
-		{
-			name:     "1mb",
-			size:     1000_000,
-			expected: "1.0 MB",
-		},
-		{
-			name:     "1gb",
-			size:     1000_000_000,
-			expected: "1.0 GB",
-		},
-		{
-			name:     "1tb",
-			size:     1000_000_000_000,
-			expected: "1.0 TB",
-		},
-		{
-			name:     "1.6 kb",
-			size:     1600,
-			expected: "1.6 kB",
-		},
-	}
-
-	for _, tc := range tt {
-		t.Run(tc.name, func(t *testing.T) {
-			assert.Equal(t, tc.expected, ByteCount(tc.size))
-		})
-	}
-}

tool/tbot/db.go

@@ -20,13 +20,14 @@ package main
 
 import (
 	"path/filepath"
+	"slices"
+	"strings"
 
 	"github.com/gravitational/trace"
 
 	"github.com/gravitational/teleport/lib/tbot/cli"
 	"github.com/gravitational/teleport/lib/tbot/config"
 	"github.com/gravitational/teleport/lib/tbot/tshwrap"
-	"github.com/gravitational/teleport/lib/utils"
 )
 
 func onDBCommand(globalCfg *cli.GlobalArgs, dbCmd *cli.DBCommand) error {
@@ -60,7 +61,7 @@ func onDBCommand(globalCfg *cli.GlobalArgs, dbCmd *cli.DBCommand) error {
 	if dbCmd.Cluster != "" {
 		// If we caught --cluster in our args, pass it through.
 		args = append(args, "--cluster="+dbCmd.Cluster)
-	} else if !utils.HasPrefixAny("--cluster", *dbCmd.RemainingArgs) {
+	} else if !slices.ContainsFunc(*dbCmd.RemainingArgs, func(s string) bool { return strings.HasPrefix(s, "--cluster") }) {
 		// If no `--cluster` was provided after a `--`, pass along the cluster
 		// name in the identity.
 		args = append(args, "--cluster="+identity.RouteToCluster)

tool/tsh/common/db_exec_test.go

@@ -250,7 +250,7 @@ func TestDatabaseExec(t *testing.T) {
 				read, err = utils.ReadPath(filepath.Join(dir, "test-output", "mysql.output"))
 				require.NoError(t, err)
 				require.Equal(t, "db-query executed", strings.TrimSpace(string(read)))
-				require.True(t, utils.FileExists(filepath.Join(dir, "test-output", "summary.json")))
+				require.FileExists(t, filepath.Join(dir, "test-output", "summary.json"))
 			},
 		},
 	}