Implement PushToNamedFork in the git v2 client.

prow/git/v2/fakes.go

@@ -31,6 +31,10 @@ func (r *fakeResolver) Resolve() (string, error) {
 	return r.out, r.err
 }
 
+func (r *fakeResolver) ForkResolver(_ string) (string, error) {
+	return r.out, r.err
+}
+
 type execResponse struct {
 	out []byte
 	err error

prow/git/v2/publisher.go

@@ -17,7 +17,6 @@ limitations under the License.
 package git
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/sirupsen/logrus"
@@ -39,7 +38,7 @@ type Publisher interface {
 type GitUserGetter func() (name, email string, err error)
 
 type remotes struct {
-	publishRemote RemoteResolver
+	publishRemote ForkRemoteResolver
 	centralRemote RemoteResolver
 }
 
@@ -70,12 +69,7 @@ func (p *publisher) Commit(title, body string) error {
 }
 
 func (p *publisher) PushToNamedFork(forkName, branch string, force bool) error {
-	return errors.New("pushToNamedFork is not implemented in the v2 client")
-}
-
-// PublishPush pushes the local state to the publish remote
-func (p *publisher) PushToFork(branch string, force bool) error {
-	remote, err := p.remotes.publishRemote()
+	remote, err := p.remotes.publishRemote(forkName)
 	if err != nil {
 		return err
 	}
@@ -93,6 +87,11 @@ func (p *publisher) PushToFork(branch string, force bool) error {
 	return nil
 }
 
+// PublishPush pushes the local state to the publish remote
+func (p *publisher) PushToFork(branch string, force bool) error {
+	return p.PushToNamedFork("", branch, force)
+}
+
 // CentralPush pushes the local state to the central remote
 func (p *publisher) PushToCentral(branch string, force bool) error {
 	remote, err := p.remotes.centralRemote()

prow/git/v2/publisher_test.go

@@ -222,7 +222,7 @@ func TestPublisher_PushToFork(t *testing.T) {
 			}
 			p := publisher{
 				executor: &e,
-				remotes:  remotes{publishRemote: r.Resolve},
+				remotes:  remotes{publishRemote: r.ForkResolver},
 				logger:   logrus.WithField("test", testCase.name),
 			}
 			actualErr := p.PushToFork(testCase.branch, testCase.force)

prow/git/v2/remote.go

@@ -35,14 +35,19 @@ type RemoteResolverFactory interface {
 	// is useful for fetching refs and cloning.
 	CentralRemote(org, repo string) RemoteResolver
 	// PublishRemote returns a resolver for a remote server with a
-	// personal fork of the repository. This type of remote is most
+	// personal fork of the central repository. This type of remote is most
 	// useful for publishing local changes.
-	PublishRemote(org, repo string) RemoteResolver
+	PublishRemote(org, centralRepo string) ForkRemoteResolver
 }
 
 // RemoteResolver knows how to construct a remote URL for git calls
 type RemoteResolver func() (string, error)
 
+// ForkRemoteResolver knows how to construct a remote URL for git calls
+// It accepts a fork name since this may be different than the parent
+// repo name. If the forkName is "", the parent repo name is assumed.
+type ForkRemoteResolver func(forkName string) (string, error)
+
 // LoginGetter fetches a GitHub login on-demand
 type LoginGetter func() (login string, err error)
 
@@ -65,8 +70,12 @@ func (f *sshRemoteResolverFactory) CentralRemote(org, repo string) RemoteResolve
 
 // PublishRemote creates a remote resolver that refers to a user's remote
 // for the repository that can be published to.
-func (f *sshRemoteResolverFactory) PublishRemote(_, repo string) RemoteResolver {
-	return func() (string, error) {
+func (f *sshRemoteResolverFactory) PublishRemote(_, centralRepo string) ForkRemoteResolver {
+	return func(forkName string) (string, error) {
+		repo := centralRepo
+		if forkName != "" {
+			repo = forkName
+		}
 		org, err := f.username()
 		if err != nil {
 			return "", err
@@ -87,56 +96,59 @@ type httpResolverFactory struct {
 // CentralRemote creates a remote resolver that refers to an authoritative remote
 // for the repository.
 func (f *httpResolverFactory) CentralRemote(org, repo string) RemoteResolver {
-	return HttpResolver(func() (*url.URL, error) {
-		scheme := "https"
-		if f.http {
-			scheme = "http"
-		}
-		return &url.URL{Scheme: scheme, Host: f.host, Path: fmt.Sprintf("%s/%s", org, repo)}, nil
-	}, f.username, f.token, org)
+	return func() (string, error) {
+		return f.resolve(org, repo)
+	}
 }
 
 // PublishRemote creates a remote resolver that refers to a user's remote
 // for the repository that can be published to.
-func (f *httpResolverFactory) PublishRemote(org, repo string) RemoteResolver {
-	return HttpResolver(func() (*url.URL, error) {
-		scheme := "https"
-		if f.http {
-			scheme = "http"
+func (f *httpResolverFactory) PublishRemote(_, centralRepo string) ForkRemoteResolver {
+	return func(forkName string) (string, error) {
+		// For the publsh remote we use:
+		// - the user login rather than the central org
+		// - the forkName rather than the central repo name, if specified.
+		repo := centralRepo
+		if forkName != "" {
+			repo = forkName
 		}
 		if f.username == nil {
-			return nil, errors.New("username not configured, no publish repo available")
+			return "", errors.New("username not configured, no publish repo available")
 		}
-		o, err := f.username()
+		org, err := f.username()
 		if err != nil {
-			return nil, err
+			return "", fmt.Errorf("could not resolve username: %w", err)
 		}
-		return &url.URL{Scheme: scheme, Host: f.host, Path: fmt.Sprintf("%s/%s", o, repo)}, nil
-	}, f.username, f.token, org)
+		remote, err := f.resolve(org, repo)
+		if err != nil {
+			err = fmt.Errorf("could not resolve remote: %w", err)
+		}
+		return remote, err
+	}
 }
 
-// HttpResolver builds http URLs that may optionally contain simple auth credentials, resolved dynamically.
-func HttpResolver(remote func() (*url.URL, error), username LoginGetter, tokenGetter TokenGetter, org string) RemoteResolver {
-	return func() (string, error) {
-		remote, err := remote()
+// resolve builds the URL string for the given org/repo remote identifier, it
+// respects the configured scheme, and the dynamic username and credentials.
+func (f *httpResolverFactory) resolve(org, repo string) (string, error) {
+	scheme := "https"
+	if f.http {
+		scheme = "http"
+	}
+	remote := &url.URL{Scheme: scheme, Host: f.host, Path: fmt.Sprintf("%s/%s", org, repo)}
+
+	if f.username != nil {
+		name, err := f.username()
 		if err != nil {
-			return "", fmt.Errorf("could not resolve remote: %w", err)
+			return "", fmt.Errorf("could not resolve username: %w", err)
 		}
-
-		if username != nil {
-			name, err := username()
-			if err != nil {
-				return "", fmt.Errorf("could not resolve username: %w", err)
-			}
-			token, err := tokenGetter(org)
-			if err != nil {
-				return "", fmt.Errorf("could not resolve token: %w", err)
-			}
-			remote.User = url.UserPassword(name, token)
+		token, err := f.token(org)
+		if err != nil {
+			return "", fmt.Errorf("could not resolve token: %w", err)
 		}
-
-		return remote.String(), nil
+		remote.User = url.UserPassword(name, token)
 	}
+
+	return remote.String(), nil
 }
 
 // pathResolverFactory generates resolvers for local path-based repositories,
@@ -155,9 +167,9 @@ func (f *pathResolverFactory) CentralRemote(org, repo string) RemoteResolver {
 
 // PublishRemote creates a remote resolver that refers to a user's remote
 // for the repository that can be published to.
-func (f *pathResolverFactory) PublishRemote(org, repo string) RemoteResolver {
-	return func() (string, error) {
-		return path.Join(f.baseDir, org, repo), nil
+func (f *pathResolverFactory) PublishRemote(org, centralRepo string) ForkRemoteResolver {
+	return func(_ string) (string, error) {
+		return path.Join(f.baseDir, org, centralRepo), nil
 	}
 }
 
@@ -173,8 +185,8 @@ func (f *gerritResolverFactory) CentralRemote(org, repo string) RemoteResolver {
 	}
 }
 
-func (f *gerritResolverFactory) PublishRemote(org, repo string) RemoteResolver {
-	return func() (string, error) {
+func (f *gerritResolverFactory) PublishRemote(org, repo string) ForkRemoteResolver {
+	return func(_ string) (string, error) {
 		return gerritsource.CloneURIFromOrgRepo(org, repo), nil
 	}
 }