go/libraries/doltcore/remotestorage: Be more permissive in allowed path, only pass repo_path in requests if repo_id cannot be constructed.

go/cmd/dolt/commands/clone.go

@@ -26,7 +26,6 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions"
-	"github.com/dolthub/dolt/go/libraries/doltcore/remotestorage"
 	"github.com/dolthub/dolt/go/libraries/events"
 	"github.com/dolthub/dolt/go/libraries/utils/argparser"
 	"github.com/dolthub/dolt/go/libraries/utils/earl"
@@ -203,15 +202,8 @@ func createRemote(ctx context.Context, remoteName, remoteUrl string, params map[
 
 	r := env.NewRemote(remoteName, remoteUrl, params)
 	ddb, err := r.GetRemoteDB(ctx, types.Format_Default, dEnv)
-
 	if err != nil {
 		bdr := errhand.BuildDError("error: failed to get remote db").AddCause(err)
-
-		if err == remotestorage.ErrInvalidDoltSpecPath {
-			urlObj, _ := earl.Parse(remoteUrl)
-			bdr.AddDetails("'%s' should be in the format 'organization/repo'", urlObj.Path)
-		}
-
 		return env.NoRemote, nil, bdr.Build()
 	}
 

go/cmd/dolt/commands/push.go

@@ -111,9 +111,7 @@ func (cmd PushCmd) Exec(ctx context.Context, commandStr string, args []string, d
 
 	remoteDB, err := opts.Remote.GetRemoteDB(ctx, dEnv.DoltDB.ValueReadWriter().Format(), dEnv)
 	if err != nil {
-		if err == remotestorage.ErrInvalidDoltSpecPath {
-			err = actions.HandleInvalidDoltSpecPathErr(opts.Remote.Name, opts.Remote.Url, err)
-		}
+		err = actions.HandleInitRemoteStorageClientErr(opts.Remote.Name, opts.Remote.Url, err)
 		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
 	}
 

go/libraries/doltcore/dbfactory/grpc.go

@@ -103,12 +103,8 @@ func (fact DoltRemoteFactory) newChunkStore(ctx context.Context, nbf *types.Noms
 
 	csClient := remotesapi.NewChunkStoreServiceClient(conn)
 	cs, err := remotestorage.NewDoltChunkStoreFromPath(ctx, nbf, urlObj.Path, urlObj.Host, csClient)
-
-	if err == remotestorage.ErrInvalidDoltSpecPath {
-		return nil, fmt.Errorf("invalid dolt url '%s'", urlObj.String())
-	} else if err != nil {
-		// TODO: Make this error more expressive
-		return nil, err
+	if err != nil {
+		return nil, fmt.Errorf("could not access dolt url '%s': %w", urlObj.String(), err)
 	}
 
 	if _, ok := params[NoCachingParameter]; ok {

go/libraries/doltcore/dtestutils/testcommands/multienv.go

@@ -26,7 +26,6 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/dtestutils"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions"
-	"github.com/dolthub/dolt/go/libraries/doltcore/remotestorage"
 	"github.com/dolthub/dolt/go/libraries/doltcore/row"
 	"github.com/dolthub/dolt/go/libraries/doltcore/schema"
 	"github.com/dolthub/dolt/go/libraries/doltcore/table/editor"
@@ -322,10 +321,7 @@ func (mr *MultiRepoTestSetup) PushToRemote(dbName, remoteName, branchName string
 
 	remoteDB, err := opts.Remote.GetRemoteDB(ctx, dEnv.DoltDB.ValueReadWriter().Format(), mr.MrEnv.GetEnv(dbName))
 	if err != nil {
-		if err == remotestorage.ErrInvalidDoltSpecPath {
-			mr.Errhand(actions.HandleInvalidDoltSpecPathErr(opts.Remote.Name, opts.Remote.Url, err))
-		}
-		mr.Errhand(fmt.Sprintf("Failed to get remote database: %s", err.Error()))
+		mr.Errhand(actions.HandleInitRemoteStorageClientErr(opts.Remote.Name, opts.Remote.Url, err))
 	}
 
 	tmpDir, err := dEnv.TempTableFilesDir()

go/libraries/doltcore/env/actions/remotes.go

@@ -471,13 +471,7 @@ func SyncRoots(ctx context.Context, srcDb, destDb *doltdb.DoltDB, tempTableDir s
 	return nil
 }
 
-func HandleInvalidDoltSpecPathErr(name, url string, err error) error {
-	urlObj, _ := earl.Parse(url)
-	path := urlObj.Path
-	if path[0] == '/' {
-		path = path[1:]
-	}
-
-	var detail = fmt.Sprintf("the remote: %s %s '%s' should be in the format 'organization/repo'", name, url, path)
+func HandleInitRemoteStorageClientErr(name, url string, err error) error {
+	var detail = fmt.Sprintf("the remote: %s '%s' could not be accessed", name, url)
 	return fmt.Errorf("%w; %s; %s", ErrFailedToGetRemoteDb, detail, err.Error())
 }

go/libraries/doltcore/remotestorage/chunk_store.go

@@ -58,7 +58,6 @@ func init() {
 }
 
 var ErrUploadFailed = errors.New("upload failed")
-var ErrInvalidDoltSpecPath = errors.New("invalid dolt spec path")
 
 var globalHttpFetcher HTTPFetcher = &http.Client{}
 
@@ -104,8 +103,7 @@ type ConcurrencyParams struct {
 }
 
 type DoltChunkStore struct {
-	org         string
-	repoName    string
+	repoId      *remotesapi.RepoId
 	repoPath    string
 	repoToken   *atomic.Value // string
 	host        string
@@ -120,34 +118,32 @@ type DoltChunkStore struct {
 }
 
 func NewDoltChunkStoreFromPath(ctx context.Context, nbf *types.NomsBinFormat, path, host string, csClient remotesapi.ChunkStoreServiceClient) (*DoltChunkStore, error) {
+	var repoId *remotesapi.RepoId
+
 	tokens := strings.Split(strings.Trim(path, "/"), "/")
-	if len(tokens) != 2 {
-		return nil, ErrInvalidDoltSpecPath
-	}
-
-	// todo:
-	// this may just be a dolthub thing.  Need to revisit how we do this.
-	org := tokens[0]
-	repoName := tokens[1]
-
-	metadata, err := csClient.GetRepoMetadata(ctx, &remotesapi.GetRepoMetadataRequest{
-		RepoId: &remotesapi.RepoId{
+	if len(tokens) == 2 {
+		org := tokens[0]
+		repoName := tokens[1]
+		repoId = &remotesapi.RepoId{
 			Org:      org,
 			RepoName: repoName,
-		},
+		}
+	}
+
+	metadata, err := csClient.GetRepoMetadata(ctx, &remotesapi.GetRepoMetadataRequest{
+		RepoId:   repoId,
+		RepoPath: path,
 		ClientRepoFormat: &remotesapi.ClientRepoFormat{
 			NbfVersion: nbf.VersionString(),
 			NbsVersion: nbs.StorageVersion,
 		},
 	})
-
 	if err != nil {
 		return nil, err
 	}
 
 	cs := &DoltChunkStore{
-		org:         org,
-		repoName:    repoName,
+		repoId:      repoId,
 		repoPath:    path,
 		repoToken:   new(atomic.Value),
 		host:        host,
@@ -163,8 +159,7 @@ func NewDoltChunkStoreFromPath(ctx context.Context, nbf *types.NomsBinFormat, pa
 
 func (dcs *DoltChunkStore) WithHTTPFetcher(fetcher HTTPFetcher) *DoltChunkStore {
 	return &DoltChunkStore{
-		org:         dcs.org,
-		repoName:    dcs.repoName,
+		repoId:      dcs.repoId,
 		repoPath:    dcs.repoPath,
 		repoToken:   new(atomic.Value),
 		host:        dcs.host,
@@ -180,8 +175,7 @@ func (dcs *DoltChunkStore) WithHTTPFetcher(fetcher HTTPFetcher) *DoltChunkStore
 
 func (dcs *DoltChunkStore) WithNoopChunkCache() *DoltChunkStore {
 	return &DoltChunkStore{
-		org:         dcs.org,
-		repoName:    dcs.repoName,
+		repoId:      dcs.repoId,
 		repoPath:    dcs.repoPath,
 		repoToken:   new(atomic.Value),
 		host:        dcs.host,
@@ -198,8 +192,7 @@ func (dcs *DoltChunkStore) WithNoopChunkCache() *DoltChunkStore {
 
 func (dcs *DoltChunkStore) WithChunkCache(cache ChunkCache) *DoltChunkStore {
 	return &DoltChunkStore{
-		org:         dcs.org,
-		repoName:    dcs.repoName,
+		repoId:      dcs.repoId,
 		repoPath:    dcs.repoPath,
 		repoToken:   new(atomic.Value),
 		host:        dcs.host,
@@ -216,8 +209,7 @@ func (dcs *DoltChunkStore) WithChunkCache(cache ChunkCache) *DoltChunkStore {
 
 func (dcs *DoltChunkStore) WithDownloadConcurrency(concurrency ConcurrencyParams) *DoltChunkStore {
 	return &DoltChunkStore{
-		org:         dcs.org,
-		repoName:    dcs.repoName,
+		repoId:      dcs.repoId,
 		repoPath:    dcs.repoPath,
 		repoToken:   new(atomic.Value),
 		host:        dcs.host,
@@ -248,10 +240,7 @@ func (dcs *DoltChunkStore) getRepoId() (*remotesapi.RepoId, string) {
 	if curToken != nil {
 		token = curToken.(string)
 	}
-	return &remotesapi.RepoId{
-		Org:      dcs.org,
-		RepoName: dcs.repoName,
-	}, token
+	return dcs.repoId, token
 }
 
 type cacheStats struct {
@@ -538,6 +527,12 @@ func (l *dlLocations) Add(resp *remotesapi.DownloadLoc) {
 	}
 }
 
+type RepoRequest interface {
+	SetRepoId(*remotesapi.RepoId)
+	SetRepoToken(string)
+	SetRepoPath(string)
+}
+
 func (dcs *DoltChunkStore) getDLLocs(ctx context.Context, hashes []hash.Hash) (dlLocations, error) {
 	ctx, span := tracer.Start(ctx, "remotestorage.getDLLocs", trace.WithAttributes(attribute.Int("num_hashes", len(hashes))))
 	defer span.End()
@@ -573,7 +568,7 @@ func (dcs *DoltChunkStore) getDLLocs(ctx context.Context, hashes []hash.Hash) (d
 		batchItr(len(hashesBytes), getLocsBatchSize, func(st, end int) (stop bool) {
 			batch := hashesBytes[st:end]
 			id, token := dcs.getRepoId()
-			req := &remotesapi.GetDownloadLocsRequest{RepoId: id, ChunkHashes: batch, RepoToken: token, RepoPath: dcs.repoPath}
+			req := &remotesapi.GetDownloadLocsRequest{RepoId: id, RepoPath: dcs.repoPath, RepoToken: token, ChunkHashes: batch}
 			reqs = append(reqs, req)
 			return false
 		})
@@ -809,10 +804,8 @@ func (dcs *DoltChunkStore) Rebase(ctx context.Context) error {
 
 func (dcs *DoltChunkStore) refreshRepoMetadata(ctx context.Context) error {
 	mdReq := &remotesapi.GetRepoMetadataRequest{
-		RepoId: &remotesapi.RepoId{
-			Org:      dcs.org,
-			RepoName: dcs.repoName,
-		},
+		RepoId:   dcs.repoId,
+		RepoPath: dcs.repoPath,
 		ClientRepoFormat: &remotesapi.ClientRepoFormat{
 			NbfVersion: dcs.nbf.VersionString(),
 			NbsVersion: nbs.StorageVersion,

go/libraries/doltcore/sqle/database_provider.go

@@ -29,12 +29,10 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions"
 	"github.com/dolthub/dolt/go/libraries/doltcore/ref"
-	"github.com/dolthub/dolt/go/libraries/doltcore/remotestorage"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dfunctions"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dprocedures"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
 	"github.com/dolthub/dolt/go/libraries/doltcore/table/editor"
-	"github.com/dolthub/dolt/go/libraries/utils/earl"
 	"github.com/dolthub/dolt/go/libraries/utils/filesys"
 	"github.com/dolthub/dolt/go/store/types"
 )
@@ -513,15 +511,8 @@ func (p DoltDatabaseProvider) cloneDatabaseFromRemote(
 // TODO: this method only adds error handling. Remove?
 func getRemoteDb(ctx *sql.Context, r env.Remote, dialer dbfactory.GRPCDialProvider) (*doltdb.DoltDB, error) {
 	ddb, err := r.GetRemoteDB(ctx, types.Format_Default, dialer)
-
 	if err != nil {
 		bdr := errhand.BuildDError("error: failed to get remote db").AddCause(err)
-
-		if err == remotestorage.ErrInvalidDoltSpecPath {
-			urlObj, _ := earl.Parse(r.Url)
-			bdr.AddDetails("'%s' should be in the format 'organization/repo'", urlObj.Path)
-		}
-
 		return nil, bdr.Build()
 	}
 

go/libraries/doltcore/sqle/dfunctions/dolt_push.go

@@ -25,7 +25,6 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env"
 	"github.com/dolthub/dolt/go/libraries/doltcore/env/actions"
-	"github.com/dolthub/dolt/go/libraries/doltcore/remotestorage"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
 	"github.com/dolthub/dolt/go/store/datas"
 )
@@ -94,10 +93,7 @@ func DoDoltPush(ctx *sql.Context, args []string) (int, error) {
 	}
 	remoteDB, err := sess.Provider().GetRemoteDB(ctx, dbData.Ddb, opts.Remote, true)
 	if err != nil {
-		if err == remotestorage.ErrInvalidDoltSpecPath {
-			return 1, actions.HandleInvalidDoltSpecPathErr(opts.Remote.Name, opts.Remote.Url, err)
-		}
-		return 1, err
+		return 1, actions.HandleInitRemoteStorageClientErr(opts.Remote.Name, opts.Remote.Url, err)
 	}
 
 	tmpDir, err := dbData.Rsw.TempTableFilesDir()