Skip to content

Conversation

kstiehl
Copy link
Member

@kstiehl kstiehl commented Jan 27, 2021

Currently this action can't be used as soon as you have at least one go dependency which requires authentication.

Currently the only way in go to allow fetching private repositories is by adding an URL patch to your global gitconfig and setting the GOPRIVATE var.

The github action runner passes the GOPRIVATE variable correctly to the docker container, but the gitconfig of the runner has no effect on the go installation inside the container.

With this change it is now possible to apply git url patches using an environment variables prefixed with GIT_URL_OVERRIDE

In our setup we use the action like this now

  - uses: ourOrg/action-staticcheck@master
     name: Staticcheck
     env:
       GOPRIVATE: github.com/ourOrg/privateRepo
       GIT_URL_OVERRIDE: https://${{ secrets.GO_MODULE_TOKEN }}:x-oauth-basic@github.com;https://github.com
     with:
       reporter: github-pr-review
       filter_mode: nofilter

I also changed the /bin/sh to /bin/bash since /bin/sh is a link to a bash on most linux systems, except for ubuntu where it is a dash shell. So /bin/sh is no guarantee for what shell will be used. (Also this change requires the bash expansion features)

@kstiehl
Copy link
Member Author

kstiehl commented Jan 27, 2021

This should fix #9 since it then allows the use of private repositories

@haya14busa
Copy link
Member

Thank you for your work!

I have 2 opinions.

  1. I'm considering using composite run action instead of docker action for this action-staticcheck as well, similar to https://github.com/reviewdog/action-golangci-lint. Then, users can setup anything including private repo config outside this action. Do you think you can work on this docker->composite run migration instead? WDYT?
  2. If we still want to pass the env variable, I'd like to have 2 environment variable for GIT_URL_OVERRIDE instead of depending on awk and bash expansion internally.

@kstiehl
Copy link
Member Author

kstiehl commented Feb 3, 2021

Hi @haya14busa
Thnx for your feedback.

I'm considering using composite run action instead of docker action for this action-staticcheck as well, similar to https://github.com/reviewdog/action-golangci-lint. Then, users can setup anything including private repo config outside this action. Do you think you can work on this docker->composite run migration instead? WDYT?

That's actually even better. It totally makes sense to migrate this action to a composite action. If you don't mind I will start working on that at the weekend.

If we still want to pass the env variable, I'd like to have 2 environment variable for GIT_URL_OVERRIDE instead of depending on awk and bash expansion internally.

I think that we won't need the env anymore once the migration to a composite action is done plus awk was only an option since it was present in the ubuntu container anyways. We will get rid of this completely then

@haya14busa
Copy link
Member

Yes! I'd appriciate it if you can work on the migration!

@kstiehl
Copy link
Member Author

kstiehl commented Feb 5, 2021

Closing PR in favor of the mentioned changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants