Repository: lindell/multi-gitter
Pull request #262 (2022-07-18 21:28:09)

fix(github): allow only pull permission for print command
Without configuring platform run options. Not all repos are downloaded due to conditions.

# What does this change

Instead of:
DEBU[0000] Skipping repository since the token does not have push permissions and the run will not fork  repo=jenkinsci/opslevel-plugin
INFO[0000] Running on 0 repositories

You get:
INFO[0000] Running on 1 repositories
INFO[0000] Cloning and running script                    repo=jenkinsci/opslevel-plugin

Since jenkinsci/opslevel-plugin is a fork I have no push permissions and g.Fork always being false because `configureRunPlatform` is never run.

# Notes for the reviewer
_Put any questions or notes for the reviewer here._

# Checklist
- [x] Made sure the PR follows the [CONTRIBUTING.md](CONTRIBUTING.md) guidelines
- [x] Tests if something new is added


lindell: Thanks for the PR. The bug is definitely real and needs to be addressed. The `print` command should only require pull permissions. 

Unfortunately, this does more than that. It does add the fork option (as the title suggests), but that option is intentionally kept out of the print command since there are no forks to be considered when running `print`. The fix should be to ensure that only pull permissions are required and nothing else. 
jetersen: Well even though there is no fork. It filters out the repository. So I wonder what the conditional logic should say.
lindell: It would have to be indicated to each [VersionController](https://github.com/lindell/multi-gitter/tree/master/internal/scm) that push permission is not needed. 

Adding something like "readOnly" and change this 
https://github.com/lindell/multi-gitter/blob/60bbbdf526ce6e87d0f952fdae4858e4c6954952/internal/scm/github/github.go#L146

to
```go
case !g.Fork && !g.ReadOnly && !permissions["push"]:
```
jetersen: I'll see if I can get around to it in the weekend to add readonly.
jetersen: Hopefully this weekend!
jetersen: @lindell I added `flag` that is set programmatically in the `printCmd()` method. To not change the signature of other methods.

Patch:
diff --git a/cmd/cmd-close.go b/cmd/cmd-close.go
index b195801..a14eaf0 100644
--- a/cmd/cmd-close.go
+++ b/cmd/cmd-close.go
@@ -32,7 +32,7 @@ func close(cmd *cobra.Command, args []string) error {
 
 	branchName, _ := flag.GetString("branch")
 
-	vc, err := getVersionController(flag, true)
+	vc, err := getVersionController(flag, true, false)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/cmd-merge.go b/cmd/cmd-merge.go
index f309dc4..f98ee40 100644
--- a/cmd/cmd-merge.go
+++ b/cmd/cmd-merge.go
@@ -33,7 +33,7 @@ func merge(cmd *cobra.Command, args []string) error {
 
 	branchName, _ := flag.GetString("branch")
 
-	vc, err := getVersionController(flag, true)
+	vc, err := getVersionController(flag, true, false)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/cmd-print.go b/cmd/cmd-print.go
index 896bdeb..32139ae 100755
--- a/cmd/cmd-print.go
+++ b/cmd/cmd-print.go
@@ -62,7 +62,7 @@ func print(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	vc, err := getVersionController(flag, true)
+	vc, err := getVersionController(flag, true, true)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/cmd-run.go b/cmd/cmd-run.go
index 275917d..142b5a8 100755
--- a/cmd/cmd-run.go
+++ b/cmd/cmd-run.go
@@ -135,7 +135,7 @@ func run(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	vc, err := getVersionController(flag, true)
+	vc, err := getVersionController(flag, true, false)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/cmd-status.go b/cmd/cmd-status.go
index cb60b9c..b015734 100644
--- a/cmd/cmd-status.go
+++ b/cmd/cmd-status.go
@@ -35,7 +35,7 @@ func status(cmd *cobra.Command, args []string) error {
 	branchName, _ := flag.GetString("branch")
 	strOutput, _ := flag.GetString("output")
 
-	vc, err := getVersionController(flag, true)
+	vc, err := getVersionController(flag, true, false)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/platform.go b/cmd/platform.go
index baceb43..cf1e944 100644
--- a/cmd/platform.go
+++ b/cmd/platform.go
@@ -94,7 +94,7 @@ var OverrideVersionController multigitter.VersionController = nil
 
 // getVersionController gets the complete version controller
 // the verifyFlags parameter can be set to false if a complete vc is not required (during autocompletion)
-func getVersionController(flag *flag.FlagSet, verifyFlags bool) (multigitter.VersionController, error) {
+func getVersionController(flag *flag.FlagSet, verifyFlags bool, readOnly bool) (multigitter.VersionController, error) {
 	if OverrideVersionController != nil {
 		return OverrideVersionController, nil
 	}
@@ -102,7 +102,7 @@ func getVersionController(flag *flag.FlagSet, verifyFlags bool) (multigitter.Ver
 	platform, _ := flag.GetString("platform")
 	switch platform {
 	case "github":
-		return createGithubClient(flag, verifyFlags)
+		return createGithubClient(flag, verifyFlags, readOnly)
 	case "gitlab":
 		return createGitlabClient(flag, verifyFlags)
 	case "gitea":
@@ -114,7 +114,7 @@ func getVersionController(flag *flag.FlagSet, verifyFlags bool) (multigitter.Ver
 	}
 }
 
-func createGithubClient(flag *flag.FlagSet, verifyFlags bool) (multigitter.VersionController, error) {
+func createGithubClient(flag *flag.FlagSet, verifyFlags bool, readOnly bool) (multigitter.VersionController, error) {
 	gitBaseURL, _ := flag.GetString("base-url")
 	orgs, _ := flag.GetStringSlice("org")
 	users, _ := flag.GetStringSlice("user")
@@ -149,7 +149,7 @@ func createGithubClient(flag *flag.FlagSet, verifyFlags bool) (multigitter.Versi
 		Organizations: orgs,
 		Users:         users,
 		Repositories:  repoRefs,
-	}, mergeTypes, forkMode, forkOwner, sshAuth)
+	}, mergeTypes, forkMode, forkOwner, sshAuth, readOnly)
 	if err != nil {
 		return nil, err
 	}
@@ -294,7 +294,7 @@ func versionControllerCompletion(cmd *cobra.Command, flagName string, fn func(vc
 		// Make sure config files are loaded
 		_ = initializeConfig(cmd)
 
-		vc, err := getVersionController(cmd.Flags(), false)
+		vc, err := getVersionController(cmd.Flags(), false, false)
 		if err != nil {
 			return nil, cobra.ShellCompDirectiveError
 		}
diff --git a/go.mod b/go.mod
index 73ecd81..fe0b507 100644
--- a/go.mod
+++ b/go.mod
@@ -10,11 +10,11 @@ require (
 	github.com/google/go-github/v39 v39.2.0
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/pkg/errors v0.9.1
-	github.com/sirupsen/logrus v1.8.1
+	github.com/sirupsen/logrus v1.9.0
 	github.com/spf13/cobra v1.5.0
 	github.com/spf13/pflag v1.0.5
 	github.com/spf13/viper v1.12.0
-	github.com/stretchr/testify v1.7.1
+	github.com/stretchr/testify v1.8.0
 	github.com/xanzy/go-gitlab v0.68.2
-	golang.org/x/oauth2 v0.0.0-20220630143837-2104d58473e0
+	golang.org/x/oauth2 v0.0.0-20220722155238-128564f6959c
 )
diff --git a/go.sum b/go.sum
index c02d645..443e49f 100644
--- a/go.sum
+++ b/go.sum
@@ -413,8 +413,8 @@ github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPx
 github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q=
 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
 github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88=
-github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
-github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
+github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0=
+github.com/sirupsen/logrus v1.9.0/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
 github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
 github.com/spf13/afero v1.8.2 h1:xehSyVa0YnHWsJ49JFljMpg1HX19V6NDZ1fkm1Xznbo=
 github.com/spf13/afero v1.8.2/go.mod h1:CtAatgMJh6bJEIs48Ay/FOnkljP3WeGUG0MC1RfAqwo=
@@ -430,14 +430,16 @@ github.com/spf13/viper v1.12.0 h1:CZ7eSOd3kZoaYDLbXnmzgQI5RlciuXBMA+18HwHRfZQ=
 github.com/spf13/viper v1.12.0/go.mod h1:b6COn30jlNxbm/V2IqWiNWkJ+vZNiMNksliPCiuKtSI=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/subosito/gotenv v1.3.0 h1:mjC+YW8QpAdXibNi+vNWgzmgBH4+5l5dCXv8cNysBLI=
 github.com/subosito/gotenv v1.3.0/go.mod h1:YzJjq/33h7nrwdY+iHMhEOEEbW0ovIz0tB6t6PwAXzs=
 github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
@@ -586,8 +588,8 @@ golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b/go.mod h1:DAh4E804XQdzx2j
 golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
 golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
 golang.org/x/oauth2 v0.0.0-20220608161450-d0670ef3b1eb/go.mod h1:jaDAt6Dkxork7LmZnYtzbRWj0W47D86a3TGe0YHBvmE=
-golang.org/x/oauth2 v0.0.0-20220630143837-2104d58473e0 h1:VnGaRqoLmqZH/3TMLJwYCEWkR4j1nuIU1U9TvbqsDUw=
-golang.org/x/oauth2 v0.0.0-20220630143837-2104d58473e0/go.mod h1:h4gKUeWbJ4rQPri7E0u6Gs4e9Ri2zaLxzw5DI5XGrYg=
+golang.org/x/oauth2 v0.0.0-20220722155238-128564f6959c h1:q3gFqPqH7NVofKo3c3yETAP//pPI+G5mvB7qqj1Y5kY=
+golang.org/x/oauth2 v0.0.0-20220722155238-128564f6959c/go.mod h1:h4gKUeWbJ4rQPri7E0u6Gs4e9Ri2zaLxzw5DI5XGrYg=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
@@ -683,8 +685,9 @@ golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220502124256-b6088ccd6cba/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220610221304-9f5ed59c137d h1:Zu/JngovGLVi6t2J3nmAf3AoTDwuzw85YZ3b9o4yU7s=
 golang.org/x/sys v0.0.0-20220610221304-9f5ed59c137d/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JCXC4ykBgQG4Fe06QRhQ=
+golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
@@ -968,8 +971,9 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gopkg.in/yaml.v3 v3.0.0 h1:hjy8E9ON/egN1tAYqKb61G10WtihqetD4sz2H+8nIeA=
 gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
diff --git a/internal/scm/github/github.go b/internal/scm/github/github.go
index 2a5a9c0..68a9cf5 100755
--- a/internal/scm/github/github.go
+++ b/internal/scm/github/github.go
@@ -26,6 +26,7 @@ func New(
 	forkMode bool,
 	forkOwner string,
 	sshAuth bool,
+	readOnly bool,
 ) (*Github, error) {
 	ctx := context.Background()
 	ts := oauth2.StaticTokenSource(
@@ -54,6 +55,7 @@ func New(
 		ForkOwner:         forkOwner,
 		SSHAuth:           sshAuth,
 		ghClient:          client,
+		ReadOnly:          readOnly,
 		httpClient: &http.Client{
 			Transport: transportMiddleware(http.DefaultTransport),
 		},
@@ -71,6 +73,9 @@ type Github struct {
 	// In this package, it mainly determines which repos are possible to make changes on
 	Fork bool
 
+	// This determines when we are running in read only mode.
+	ReadOnly bool
+
 	// If set, the fork will happen to the ForkOwner value, and not the logged in user
 	ForkOwner string
 
@@ -143,7 +148,7 @@ func (g *Github) GetRepositories(ctx context.Context) ([]scm.Repository, error)
 		case !permissions["pull"]:
 			log.Debug("Skipping repository since the token does not have pull permissions")
 			continue
-		case !g.Fork && !permissions["push"]:
+		case !g.Fork && !g.ReadOnly && !permissions["push"]:
 			log.Debug("Skipping repository since the token does not have push permissions and the run will not fork")
 			continue
 		}


Test patch:
diff --git a/internal/scm/github/github_test.go b/internal/scm/github/github_test.go
index b162fdc..39d5e42 100644
--- a/internal/scm/github/github_test.go
+++ b/internal/scm/github/github_test.go
@@ -117,7 +117,7 @@ func Test_GetRepositories(t *testing.T) {
 	{
 		gh, err := github.New("", "", transport.Wrapper, github.RepositoryListing{
 			Organizations: []string{"test-org"},
-		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false)
+		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false, false)
 		require.NoError(t, err)
 
 		repos, err := gh.GetRepositories(context.Background())
@@ -137,7 +137,7 @@ func Test_GetRepositories(t *testing.T) {
 					Name:      "test1",
 				},
 			},
-		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false)
+		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false, false)
 		require.NoError(t, err)
 
 		repos, err := gh.GetRepositories(context.Background())
@@ -152,7 +152,7 @@ func Test_GetRepositories(t *testing.T) {
 	{
 		gh, err := github.New("", "", transport.Wrapper, github.RepositoryListing{
 			Users: []string{"test-user"},
-		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false)
+		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false, false)
 		require.NoError(t, err)
 
 		repos, err := gh.GetRepositories(context.Background())
@@ -174,7 +174,7 @@ func Test_GetRepositories(t *testing.T) {
 					Name:      "test1",
 				},
 			},
-		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false)
+		}, []scm.MergeType{scm.MergeTypeMerge}, false, "", false, false)
 		require.NoError(t, err)
 
 		repos, err := gh.GetRepositories(context.Background())