cloud: move list delimiter into options struct

This commit updates the `ExternalStorage` `List` interface to take in an
options struct and moves `delimiter` into the struct. This is to
facilitate further work in adding more options to the `List` call.

Epic: None

Release note: None

Author: kev-cao

pkg/backup/backup_job.go

@@ -1889,9 +1889,12 @@ func (b *backupResumer) deleteCheckpoint(
 		defer exportStore.Close()
 		// Delete will not delete a nonempty directory, so we have to go through
 		// all files and delete each file one by one.
-		return exportStore.List(ctx, backupinfo.BackupProgressDirectory, "", func(p string) error {
-			return exportStore.Delete(ctx, backupinfo.BackupProgressDirectory+p)
-		})
+		return exportStore.List(
+			ctx, backupinfo.BackupProgressDirectory, cloud.ListOptions{},
+			func(p string) error {
+				return exportStore.Delete(ctx, backupinfo.BackupProgressDirectory+p)
+			},
+		)
 	}(); err != nil {
 		log.Dev.Warningf(ctx, "unable to delete checkpointed backup descriptor file in progress directory: %+v", err)
 	}

pkg/backup/backup_test.go

@@ -10083,16 +10083,18 @@ func TestBackupNoOverwriteCheckpoint(t *testing.T) {
 	r.Close(ctx)
 
 	var actualNumCheckpointsWritten int
-	require.NoError(t, store.List(ctx, latestFilePath+"/progress/", "", func(f string) error {
-		// Don't double count checkpoints as there will be the manifest and
-		// the checksum.
-		if !strings.HasSuffix(f, backupinfo.BackupManifestChecksumSuffix) {
-			if strings.HasPrefix(f, backupinfo.BackupManifestCheckpointName) {
-				actualNumCheckpointsWritten++
+	require.NoError(t, store.List(
+		ctx, latestFilePath+"/progress/", cloud.ListOptions{},
+		func(f string) error {
+			// Don't double count checkpoints as there will be the manifest and
+			// the checksum.
+			if !strings.HasSuffix(f, backupinfo.BackupManifestChecksumSuffix) {
+				if strings.HasPrefix(f, backupinfo.BackupManifestCheckpointName) {
+					actualNumCheckpointsWritten++
+				}
 			}
-		}
-		return nil
-	}))
+			return nil
+		}))
 
 	// numCheckpointWritten only accounts for checkpoints written in the
 	// progress loop, each time we Resume we write another checkpoint.
@@ -10217,7 +10219,7 @@ func TestBackupTimestampedCheckpointsAreLexicographical(t *testing.T) {
 			}
 			require.NoError(t, err)
 			var actual string
-			err = store.List(ctx, "/progress/", "", func(f string) error {
+			err = store.List(ctx, "/progress/", cloud.ListOptions{}, func(f string) error {
 				actual = f
 				return cloud.ErrListingDone
 			})
@@ -10248,13 +10250,15 @@ func TestBackupNoOverwriteLatest(t *testing.T) {
 	findNumLatestFiles := func() (int, string) {
 		var numLatestFiles int
 		var latestFile string
-		err = store.List(ctx, backupbase.LatestHistoryDirectory, "", func(p string) error {
-			if numLatestFiles == 0 {
-				latestFile = p
-			}
-			numLatestFiles++
-			return nil
-		})
+		err = store.List(
+			ctx, backupbase.LatestHistoryDirectory, cloud.ListOptions{},
+			func(p string) error {
+				if numLatestFiles == 0 {
+					latestFile = p
+				}
+				numLatestFiles++
+				return nil
+			})
 		require.NoError(t, err)
 		return numLatestFiles, latestFile
 	}

pkg/backup/backupdest/backup_destination.go

@@ -303,18 +303,21 @@ func FindLatestFile(
 	// empty object with a trailing '/'. The latter is never created by our code
 	// but can be created by other tools, e.g., AWS DataSync to transfer an existing backup to
 	// another bucket. (See https://github.com/cockroachdb/cockroach/issues/106070.)
-	err := exportStore.List(ctx, backupbase.LatestHistoryDirectory, "", func(p string) error {
-		p = strings.TrimPrefix(p, "/")
-		if p == "" {
-			// N.B. skip the empty object with a trailing '/', created by a third-party tool.
-			return nil
-		}
-		latestFile = p
-		latestFileFound = true
-		// We only want the first latest file so return an error that it is
-		// done listing.
-		return cloud.ErrListingDone
-	})
+	err := exportStore.List(
+		ctx, backupbase.LatestHistoryDirectory, cloud.ListOptions{},
+		func(p string) error {
+			p = strings.TrimPrefix(p, "/")
+			if p == "" {
+				// N.B. skip the empty object with a trailing '/', created by a third-party tool.
+				return nil
+			}
+			latestFile = p
+			latestFileFound = true
+			// We only want the first latest file so return an error that it is
+			// done listing.
+			return cloud.ErrListingDone
+		},
+	)
 	// If the list failed because the storage used does not support listing,
 	// such as http, we can try reading the non-timestamped backup latest
 	// file directly. This can still fail if it is a mixed cluster and the
@@ -478,12 +481,15 @@ func ListFullBackupsInCollection(
 	ctx context.Context, store cloud.ExternalStorage,
 ) ([]string, error) {
 	var backupPaths []string
-	if err := store.List(ctx, "", backupbase.ListingDelimDataSlash, func(f string) error {
-		if deprecatedBackupPathRE.MatchString(f) {
-			backupPaths = append(backupPaths, f)
-		}
-		return nil
-	}); err != nil {
+	if err := store.List(
+		ctx, "", cloud.ListOptions{Delimiter: backupbase.ListingDelimDataSlash},
+		func(f string) error {
+			if deprecatedBackupPathRE.MatchString(f) {
+				backupPaths = append(backupPaths, f)
+			}
+			return nil
+		},
+	); err != nil {
 		// Can't happen, just required to handle the error for lint.
 		return nil, err
 	}

pkg/backup/backupdest/incrementals.go

@@ -131,26 +131,29 @@ func LegacyFindPriorBackups(
 	defer sp.Finish()
 
 	var prev []string
-	if err := store.List(ctx, "", backupbase.ListingDelimDataSlash, func(p string) error {
-		matchesGlob, err := path.Match(incBackupSubdirGlob+backupbase.DeprecatedBackupManifestName, p)
-		if err != nil {
-			return err
-		} else if !matchesGlob {
-			matchesGlob, err = path.Match(incBackupSubdirGlobWithSuffix+backupbase.DeprecatedBackupManifestName, p)
+	if err := store.List(
+		ctx, "", cloud.ListOptions{Delimiter: backupbase.ListingDelimDataSlash},
+		func(p string) error {
+			matchesGlob, err := path.Match(incBackupSubdirGlob+backupbase.DeprecatedBackupManifestName, p)
 			if err != nil {
 				return err
+			} else if !matchesGlob {
+				matchesGlob, err = path.Match(incBackupSubdirGlobWithSuffix+backupbase.DeprecatedBackupManifestName, p)
+				if err != nil {
+					return err
+				}
 			}
-		}
 
-		if matchesGlob {
-			if !includeManifest {
-				p = strings.TrimSuffix(p, "/"+backupbase.DeprecatedBackupManifestName)
+			if matchesGlob {
+				if !includeManifest {
+					p = strings.TrimSuffix(p, "/"+backupbase.DeprecatedBackupManifestName)
+				}
+				prev = append(prev, p)
+				return nil
 			}
-			prev = append(prev, p)
 			return nil
-		}
-		return nil
-	}); err != nil {
+		},
+	); err != nil {
 		return nil, errors.Wrap(err, "reading previous backup layers")
 	}
 	sort.Strings(prev)

pkg/backup/backupencryption/encryption.go

@@ -479,15 +479,18 @@ func GetEncryptionInfoFiles(ctx context.Context, dest cloud.ExternalStorage) ([]
 	var files []string
 	// Look for all files in dest that start with "/ENCRYPTION-INFO"
 	// and return them.
-	err := dest.List(ctx, "", backupbase.ListingDelimDataSlash, func(p string) error {
-		paths := strings.Split(p, "/")
-		p = paths[len(paths)-1]
-		if match := strings.HasPrefix(p, backupEncryptionInfoFile); match {
-			files = append(files, p)
-		}
+	err := dest.List(
+		ctx, "", cloud.ListOptions{Delimiter: backupbase.ListingDelimDataSlash},
+		func(p string) error {
+			paths := strings.Split(p, "/")
+			p = paths[len(paths)-1]
+			if match := strings.HasPrefix(p, backupEncryptionInfoFile); match {
+				files = append(files, p)
+			}
 
-		return nil
-	})
+			return nil
+		},
+	)
 	if len(files) < 1 {
 		return nil, errors.New("no ENCRYPTION-INFO files found")
 	}

pkg/backup/backupinfo/backup_index.go

@@ -138,7 +138,7 @@ func IndexExists(ctx context.Context, store cloud.ExternalStorage, subdir string
 	if err := store.List(
 		ctx,
 		indexDir,
-		"/",
+		cloud.ListOptions{Delimiter: "/"},
 		func(file string) error {
 			indexExists = true
 			// Because we delimit on `/` and the index subdir does not contain a
@@ -176,7 +176,7 @@ func ListIndexes(
 	if err := store.List(
 		ctx,
 		indexDir+"/",
-		"",
+		cloud.ListOptions{},
 		func(file string) error {
 			// We assert that if a file ends with .pb in the index, it should be a
 			// parsable index file. Otherwise, we ignore it. This circumvents any temp

pkg/backup/backupinfo/manifest_handling.go

@@ -1334,21 +1334,24 @@ func CheckForPreviousBackup(
 
 	// Check for the presence of a BACKUP-LOCK file with a job ID different from
 	// that of our job.
-	if err := defaultStore.List(ctx, "", backupbase.ListingDelimDataSlash, func(s string) error {
-		s = strings.TrimPrefix(s, "/")
-		if strings.HasPrefix(s, BackupLockFilePrefix) {
-			jobIDSuffix := strings.TrimPrefix(s, BackupLockFilePrefix)
-			if len(jobIDSuffix) == 0 {
-				return errors.AssertionFailedf("malformed BACKUP-LOCK file %s, expected a job ID suffix", s)
-			}
-			if jobIDSuffix != strconv.FormatInt(int64(jobID), 10) {
-				return pgerror.Newf(pgcode.FileAlreadyExists,
-					"%s already contains a `BACKUP-LOCK` file written by job %s",
-					redactedURI, jobIDSuffix)
+	if err := defaultStore.List(
+		ctx, "", cloud.ListOptions{Delimiter: backupbase.ListingDelimDataSlash},
+		func(s string) error {
+			s = strings.TrimPrefix(s, "/")
+			if strings.HasPrefix(s, BackupLockFilePrefix) {
+				jobIDSuffix := strings.TrimPrefix(s, BackupLockFilePrefix)
+				if len(jobIDSuffix) == 0 {
+					return errors.AssertionFailedf("malformed BACKUP-LOCK file %s, expected a job ID suffix", s)
+				}
+				if jobIDSuffix != strconv.FormatInt(int64(jobID), 10) {
+					return pgerror.Newf(pgcode.FileAlreadyExists,
+						"%s already contains a `BACKUP-LOCK` file written by job %s",
+						redactedURI, jobIDSuffix)
+				}
 			}
-		}
-		return nil
-	}); err != nil {
+			return nil
+		},
+	); err != nil {
 		// HTTP external storage does not support listing, and so we skip checking
 		// for a BACKUP-LOCK file.
 		if !errors.Is(err, cloud.ErrListingUnsupported) {
@@ -1536,7 +1539,7 @@ func readLatestCheckpointFile(
 
 	// We name files such that the most recent checkpoint will always
 	// be at the top, so just grab the first filename.
-	err = exportStore.List(ctx, BackupProgressDirectory, "", func(p string) error {
+	err = exportStore.List(ctx, BackupProgressDirectory, cloud.ListOptions{}, func(p string) error {
 		// The first file returned by List could be either the checkpoint or
 		// checksum file, but we are only concerned with the timestamped prefix.
 		// We resolve if it is a checkpoint or checksum file separately below.

pkg/ccl/changefeedccl/sink_cloudstorage_test.go

@@ -997,7 +997,9 @@ func (n *mockSinkStorage) Writer(_ context.Context, _ string) (io.WriteCloser, e
 	return n.writer(), nil
 }
 
-func (n *mockSinkStorage) List(_ context.Context, _, _ string, _ cloud.ListingFn) error {
+func (n *mockSinkStorage) List(
+	_ context.Context, _ string, _ cloud.ListOptions, _ cloud.ListingFn,
+) error {
 	return nil
 }
 

pkg/ccl/workloadccl/fixture.go

@@ -694,7 +694,7 @@ func listDir(
 	if log.V(1) {
 		log.Dev.Infof(ctx, "Listing %s", dir)
 	}
-	return es.List(ctx, dir, "/", lsFn)
+	return es.List(ctx, dir, cloud.ListOptions{Delimiter: "/"}, lsFn)
 }
 
 // ListFixtures returns the object paths to all fixtures stored in a FixtureConfig.

pkg/cli/userfile.go

@@ -248,7 +248,7 @@ func runUserFileGet(cmd *cobra.Command, args []string) (resErr error) {
 	defer f.Close()
 
 	var files []string
-	if err := f.List(ctx, "", "", func(s string) error {
+	if err := f.List(ctx, "", cloud.ListOptions{}, func(s string) error {
 		if pattern != "" {
 			if ok, err := path.Match(pattern, s); err != nil || !ok {
 				return err
@@ -442,7 +442,7 @@ func listUserFile(ctx context.Context, conn clisqlclient.Conn, glob string) ([]s
 
 	displayPrefix := strings.TrimPrefix(conf.Path, "/")
 	var res []string
-	if err := f.List(ctx, "", "", func(s string) error {
+	if err := f.List(ctx, "", cloud.ListOptions{}, func(s string) error {
 		if pattern != "" {
 			ok, err := path.Match(pattern, s)
 			if err != nil || !ok {
@@ -519,7 +519,7 @@ func deleteUserFile(ctx context.Context, conn clisqlclient.Conn, glob string) ([
 	displayRoot := strings.TrimPrefix(userFileTableConf.FileTableConfig.Path, "/")
 	var deleted []string
 
-	if err := f.List(ctx, "", "", func(s string) error {
+	if err := f.List(ctx, "", cloud.ListOptions{}, func(s string) error {
 		if pattern != "" {
 			ok, err := path.Match(pattern, s)
 			if err != nil || !ok {