Merge pull request #8 from hashtagchris/issue2

Honor repo flag on sync

README.md

@@ -31,7 +31,7 @@ When there are machines which have access to both the public internet and the GH
 - `destination-token` _(required)_
    A personal access token to authenticate against the GHES instance when uploading repositories.
 - `repo-name` _(optional)_
-   A single repository to be synced. In the format of `owner/repo`. Optionally if you wish the repository to be named different on your GHES instance you can provide an aliase in the format: `upstream_owner/up_streamrepo:destination_owner/destination_repo`
+   A single repository to be synced. In the format of `owner/repo`. Optionally if you wish the repository to be named different on your GHES instance you can provide an alias in the format: `upstream_owner/upstream_repo:destination_owner/destination_repo`
 - `repo-name-list` _(optional)_
    A comma-separated list of repositories to be synced. Each entry follows the format of `repo-name`.
 - `repo-name-list-file` _(optional)_

cmd/root.go

@@ -11,8 +11,7 @@ import (
 )
 
 var (
-	cacheDir string
-	rootCmd  = &cobra.Command{
+	rootCmd = &cobra.Command{
 		Use:   "actions-sync",
 		Short: "GHES Actions Sync",
 		Long:  "Sync Actions from github.com to a GHES instance.",
@@ -22,7 +21,7 @@ var (
 		Use:   "version",
 		Short: "The version of actions-sync in use.",
 		Run: func(cmd *cobra.Command, args []string) {
-			fmt.Fprintln(os.Stdout, "GHES Actions Sync v0.1")
+			fmt.Fprintln(os.Stdout, "GHES Actions Sync v0.2")
 		},
 	}
 
@@ -37,7 +36,7 @@ var (
 				os.Exit(1)
 				return
 			}
-			if err := src.Push(cmd.Context(), cacheDir, pushRepoFlags); err != nil {
+			if err := src.Push(cmd.Context(), pushRepoFlags); err != nil {
 				fmt.Fprintln(os.Stderr, err.Error())
 				os.Exit(1)
 				return
@@ -56,7 +55,7 @@ var (
 				os.Exit(1)
 				return
 			}
-			if err := src.Pull(cmd.Context(), cacheDir, pullRepoFlags); err != nil {
+			if err := src.Pull(cmd.Context(), pullRepoFlags); err != nil {
 				fmt.Fprintln(os.Stderr, err.Error())
 				os.Exit(1)
 				return
@@ -75,7 +74,7 @@ var (
 				os.Exit(1)
 				return
 			}
-			if err := src.Sync(cmd.Context(), cacheDir, syncRepoFlags); err != nil {
+			if err := src.Sync(cmd.Context(), syncRepoFlags); err != nil {
 				fmt.Fprintln(os.Stderr, err.Error())
 				os.Exit(1)
 				return
@@ -85,9 +84,6 @@ var (
 )
 
 func Execute(ctx context.Context) error {
-	rootCmd.PersistentFlags().StringVar(&cacheDir, "cache-dir", "", "Directory containing the repopositories cache created by the `pull` command")
-	_ = rootCmd.MarkPersistentFlagRequired("cache-dir")
-
 	rootCmd.AddCommand(versionCmd)
 
 	rootCmd.AddCommand(pushRepoCmd)

go.mod

@@ -9,5 +9,6 @@ require (
 	github.com/gorilla/mux v1.7.4
 	github.com/pkg/errors v0.8.1
 	github.com/spf13/cobra v1.0.0
+	github.com/stretchr/testify v1.6.1
 	golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be
 )

go.sum

@@ -140,10 +140,13 @@ github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg=
 github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
 github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
+github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
 github.com/xanzy/ssh-agent v0.2.1 h1:TCbipTQL2JiiCprBWx9frJ2eJlCYT00NmctrHxVAr70=
@@ -215,4 +218,6 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

script/test-build

@@ -19,6 +19,12 @@ DEST_API_PID=-1
 trap "after_suite" EXIT
 trap "after_suite" SIGINT
 
+function test_version() {
+  version "version shouldn't require any flags"
+
+  echo "all version tests passed"
+}
+
 function test_pull() {
   # Pull new repo
   setup_src "org/repo:heads/main:a5984bb887dd2fcdc2892cd906d6f004844d1142"
@@ -86,6 +92,16 @@ function test_push() {
   assert_dest_sha "org1/repo2" "tags/v1" "e9009d51dd6da2c363d1d14779c53dd27fcb0c52" "updating org1/repo2:tags/v1 to new commit"
   assert_dest_sha "org2/repo1" "heads/main" "e9009d51dd6da2c363d1d14779c53dd27fcb0c52" "updating org2/repo1:tags/v1 to new commit"
 
+  # Honor --repo-name flag, ignore other cache entries
+  setup_cache "org/repo1:heads/main:e9009d51dd6da2c363d1d14779c53dd27fcb0c52" \
+              "org/repo2:heads/main:e9009d51dd6da2c363d1d14779c53dd27fcb0c52"
+  setup_dest "org/repo1:heads/main:a5984bb887dd2fcdc2892cd906d6f004844d1142" \
+               "org/repo2:heads/main:a5984bb887dd2fcdc2892cd906d6f004844d1142"
+
+  push2args --repo-name "org/repo1" "pushing only one of the repos in the cache"
+  assert_dest_sha "org/repo1" "heads/main" "e9009d51dd6da2c363d1d14779c53dd27fcb0c52" "updating org/repo1 passed in repo flag"
+  assert_dest_sha "org/repo2" "heads/main" "a5984bb887dd2fcdc2892cd906d6f004844d1142" "org/repo2 not updated despite cache"
+
   echo "all push tests passed successfully"
 }
 
@@ -142,6 +158,16 @@ function test_sync() {
   sync --repo-name "org/repo:org2/repo2" "syncing org/repo aliased to org2/repo2"
   assert_dest_sha "org2/repo2" "heads/main" "e9009d51dd6da2c363d1d14779c53dd27fcb0c52" "syncing org/repo aliased to org2/repo2"
 
+  # Honor --repo-name flag, ignore other cache entries
+  setup_cache "org/repo1:heads/main:e9009d51dd6da2c363d1d14779c53dd27fcb0c52" \
+              "org/repo2:heads/main:e9009d51dd6da2c363d1d14779c53dd27fcb0c52"
+  setup_dest "org/repo1:heads/main:a5984bb887dd2fcdc2892cd906d6f004844d1142" \
+               "org/repo2:heads/main:a5984bb887dd2fcdc2892cd906d6f004844d1142"
+
+  sync --repo-name "org/repo1" "syncing only one of the repos in the cache"
+  assert_dest_sha "org/repo1" "heads/main" "e9009d51dd6da2c363d1d14779c53dd27fcb0c52" "updating org/repo1 passed in repo flag"
+  assert_dest_sha "org/repo2" "heads/main" "a5984bb887dd2fcdc2892cd906d6f004844d1142" "org/repo2 not updated despite cache"
+
   echo "all sync tests passed successfully"
 }
 
@@ -230,6 +256,12 @@ function setup_dest() {
   done
 }
 
+function version() {
+  bin/actions-sync version \
+    &> $OUTPUT ||
+    fail $1
+}
+
 function pull() {
   bin/actions-sync pull \
     --cache-dir "test/tmp/cache" \
@@ -249,6 +281,17 @@ function push() {
     fail "$1"
 }
 
+function push2args() {
+  bin/actions-sync push \
+    --cache-dir "test/tmp/cache" \
+    --disable-push-git-auth \
+    --destination-token "token" \
+    --destination-url "http://localhost:$DEST_API_PORT" \
+    "$1" "$2" \
+    &> $OUTPUT ||
+    fail $3
+}
+
 function sync() {
   bin/actions-sync sync \
     --cache-dir "test/tmp/cache" \
@@ -298,6 +341,7 @@ function fail() {
   exit 1
 }
 
+test_version
 before_suite
 test_pull
 test_push

src/commonflags.go

@@ -0,0 +1,31 @@
+package src
+
+import (
+	"github.com/spf13/cobra"
+)
+
+// flags common to pull, push and sync operations
+type CommonFlags struct {
+	CacheDir, RepoName, RepoNameList, RepoNameListFile string
+}
+
+func (f *CommonFlags) Init(cmd *cobra.Command) {
+	cmd.Flags().StringVar(&f.CacheDir, "cache-dir", "", "Directory containing the repopositories cache created by the `pull` command")
+	_ = cmd.MarkFlagRequired("cache-dir")
+
+	cmd.Flags().StringVar(&f.RepoName, "repo-name", "", "Single repository name to pull")
+	cmd.Flags().StringVar(&f.RepoNameList, "repo-name-list", "", "Comma delimited list of repository names to pull")
+	cmd.Flags().StringVar(&f.RepoNameListFile, "repo-name-list-file", "", "Path to file containing a list of repository names to pull")
+}
+
+func (f *CommonFlags) Validate(reposRequired bool) Validations {
+	var validations Validations
+	if reposRequired && !f.HasAtLeastOneRepoFlag() {
+		validations = append(validations, "one of --repo-name, --repo-name-list, --repo-name-list-file must be set")
+	}
+	return validations
+}
+
+func (f *CommonFlags) HasAtLeastOneRepoFlag() bool {
+	return f.RepoName != "" || f.RepoNameList != "" || f.RepoNameListFile != ""
+}

src/pull.go

@@ -3,62 +3,49 @@ package src
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path"
-	"regexp"
 	"strings"
 
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
-var (
-	RepoNameRegExp   = regexp.MustCompile(`^[^/]+/\S+$`)
-	ErrEmptyRepoList = errors.New("repo list cannot be empty")
-)
+type PullOnlyFlags struct {
+	SourceURL string
+}
 
 type PullFlags struct {
-	SourceURL, RepoName, RepoNameList, RepoNameListFile string
+	CommonFlags
+	PullOnlyFlags
 }
 
 func (f *PullFlags) Init(cmd *cobra.Command) {
+	f.CommonFlags.Init(cmd)
+	f.PullOnlyFlags.Init(cmd)
+}
+
+func (f *PullOnlyFlags) Init(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&f.SourceURL, "source-url", "https://github.com", "The domain to pull from")
-	cmd.Flags().StringVar(&f.RepoName, "repo-name", "", "Single repository name to pull")
-	cmd.Flags().StringVar(&f.RepoNameList, "repo-name-list", "", "Comma delimited list of repository names to pull")
-	cmd.Flags().StringVar(&f.RepoNameListFile, "repo-name-list-file", "", "Path to file containing a list of repository names to pull")
 }
 
 func (f *PullFlags) Validate() Validations {
-	var validations Validations
-	if !f.HasAtLeastOneRepoFlag() {
-		validations = append(validations, "one of --repo-name, --repo-name-list, --repo-name-list-file must be set")
-	}
-	return validations
+	return f.CommonFlags.Validate(true).Join(f.PullOnlyFlags.Validate())
 }
 
-func (f *PullFlags) HasAtLeastOneRepoFlag() bool {
-	return f.RepoName != "" || f.RepoNameList != "" || f.RepoNameListFile != ""
+func (f *PullOnlyFlags) Validate() Validations {
+	var validations Validations
+	return validations
 }
 
-func Pull(ctx context.Context, cacheDir string, flags *PullFlags) error {
-	if flags.RepoNameList != "" {
-		repoNames, err := getRepoNamesFromCSVString(flags.RepoNameList)
-		if err != nil {
-			return err
-		}
-		return PullManyWithGitImpl(ctx, flags.SourceURL, cacheDir, repoNames, gitImplementation{})
-	}
-	if flags.RepoNameListFile != "" {
-		repoNames, err := getRepoNamesFromFile(flags.RepoNameListFile)
-		if err != nil {
-			return err
-		}
-		return PullManyWithGitImpl(ctx, flags.SourceURL, cacheDir, repoNames, gitImplementation{})
+func Pull(ctx context.Context, flags *PullFlags) error {
+	repoNames, err := getRepoNamesFromRepoFlags(&flags.CommonFlags)
+	if err != nil {
+		return err
 	}
-	return PullWithGitImpl(ctx, flags.SourceURL, cacheDir, flags.RepoName, gitImplementation{})
+
+	return PullManyWithGitImpl(ctx, flags.SourceURL, flags.CacheDir, repoNames, gitImplementation{})
 }
 
 func PullManyWithGitImpl(ctx context.Context, sourceURL, cacheDir string, repoNames []string, gitimpl GitImplementation) error {
@@ -71,18 +58,11 @@ func PullManyWithGitImpl(ctx context.Context, sourceURL, cacheDir string, repoNa
 }
 
 func PullWithGitImpl(ctx context.Context, sourceURL, cacheDir string, repoName string, gitimpl GitImplementation) error {
-	repoNameParts := strings.SplitN(repoName, ":", 2)
-	originRepoName, err := validateRepoName(repoNameParts[0])
+	originRepoName, destRepoName, err := extractSourceDest(repoName)
 	if err != nil {
 		return err
 	}
-	destRepoName := originRepoName
-	if len(repoNameParts) > 1 {
-		destRepoName, err = validateRepoName(repoNameParts[1])
-		if err != nil {
-			return err
-		}
-	}
+
 	_, err = os.Stat(cacheDir)
 	if err != nil {
 		return err
@@ -122,41 +102,3 @@ func PullWithGitImpl(ctx context.Context, sourceURL, cacheDir string, repoName s
 
 	return nil
 }
-
-func getRepoNamesFromCSVString(csv string) ([]string, error) {
-	repos := filterEmptyEntries(strings.Split(csv, ","))
-	if len(repos) == 0 {
-		return nil, ErrEmptyRepoList
-	}
-	return repos, nil
-}
-
-func getRepoNamesFromFile(file string) ([]string, error) {
-	data, err := ioutil.ReadFile(file)
-	if err != nil {
-		return nil, err
-	}
-	repos := filterEmptyEntries(strings.Split(string(data), "\n"))
-	if len(repos) == 0 {
-		return nil, ErrEmptyRepoList
-	}
-	return repos, nil
-}
-
-func filterEmptyEntries(names []string) []string {
-	filtered := []string{}
-	for _, name := range names {
-		if name != "" {
-			filtered = append(filtered, name)
-		}
-	}
-	return filtered
-}
-
-func validateRepoName(name string) (string, error) {
-	s := strings.TrimSpace(name)
-	if RepoNameRegExp.MatchString(s) {
-		return s, nil
-	}
-	return "", fmt.Errorf("`%s` is not a valid repo name", s)
-}