Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use GOPROXY and disable download on some steps #7745

Merged
merged 2 commits into from
Aug 4, 2019

Conversation

sapk
Copy link
Member

@sapk sapk commented Aug 4, 2019

This PR set GOPROXY to speed-up CI process and serve as cache in case if gitea.com is down.

I setup it to use https://goproxy.cn since it is the only available worldwide.

I didn't change the Makefile to not impact build by user but only build on CI.

@sapk sapk changed the title build: use GOPROXY build: use GOPROXY and disable download on some steps Aug 4, 2019
@codecov-io
Copy link

codecov-io commented Aug 4, 2019

Codecov Report

Merging #7745 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7745      +/-   ##
==========================================
- Coverage   41.27%   41.27%   -0.01%     
==========================================
  Files         472      472              
  Lines       63854    63854              
==========================================
- Hits        26356    26353       -3     
- Misses      34061    34063       +2     
- Partials     3437     3438       +1
Impacted Files Coverage Δ
modules/avatar/avatar.go 48% <0%> (-6%) ⬇️
modules/log/event.go 64.61% <0%> (-1.03%) ⬇️
models/repo_list.go 73.09% <0%> (+1.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b902e2...f533ca1. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 4, 2019
@lafriks lafriks added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label Aug 4, 2019
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 4, 2019
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 4, 2019
@lafriks lafriks merged commit cd238bc into go-gitea:master Aug 4, 2019
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate that we have to work around the damned great firewall of China.

@sapk
Copy link
Member Author

sapk commented Aug 4, 2019

Go1.13 will have the possibility to define multiple proxy source and we could set first the "standard" one and in second the China one. Maybe I should create an issue to track this needed change ?

@sapk sapk deleted the use-goproxy branch August 4, 2019 22:55
@atomi
Copy link

atomi commented Aug 7, 2019

I can't trust builds using a private GOPROXY hosted in China.
I don't think Go can guarantee the code being downloaded from the proxy is the same code hosted on the repo.
Just my two cents. I do run my own builds using my own local GOPROXY via athens for what it's worth.

@lunny
Copy link
Member

lunny commented Aug 7, 2019

I don't trust proxy.golang.org either which hosted in U.S. :)

@lafriks
Copy link
Member

lafriks commented Aug 7, 2019

@atomi https://github.com/go-gitea/gitea/blob/master/go.sum has all dependency checksums that are checked on dependency download that are verified so no tampering code in proxy is possible

@sapk
Copy link
Member Author

sapk commented Aug 7, 2019

I specificaly only set the GOPROXY only for CI so that go.sum are not generated via proxy.

@atomi
Copy link

atomi commented Aug 7, 2019

@lafriks Thanks. I don't know if sum.golang.org is also blocked by China.

The module system uses a 'trust on first use' see relevant issue golang/go#25530

Since CI is already being provided a sum file this may be okay if we're sure the sum file has not been tampered with. 🤷‍♂️
Again, I will be building my own binary/docker image as needed anyway.

Edit: Here is a proposal with some background if you're interested. https://go.googlesource.com/proposal/+/master/design/25530-sumdb.md

@atomi
Copy link

atomi commented Aug 7, 2019

@lunny Sorry. I meant any private proxy. Not because it's China specifically. We should not trust proxy.golang.org as you say either which is what the sumdb is supposed to address.

@sapk
Copy link
Member Author

sapk commented Aug 7, 2019

The advantage of the go.sum file is that everyone can check it from it own point of view. For the GOSUM, I planned to add it as an extra step and it should not be blocked from china. If I am not mistaken the main concern for China is the ability to share data via those kind of platform (similar to play.golang.org) but that is not the case for GOSUM. Like always (for every software) deps are a security issue. This PR does not change local build process to let everyone build gitea like they want. But if your risk model, include go.sum to be tempered by any methods be sure to recheck it before build since the deps are vendored the build may blindly compile with tempered data depending on how you compile. From gitea point of view risk model, If something can temper go.sum and vendor it means that it have enough access to directly upload binary to releases (which is far more easy and silent). On other side, the more people build them self gitea the more it can be potential contributor later and that always a good point so what ever the reason I can always recommand to build gitea yourself.

@lunny
Copy link
Member

lunny commented Aug 7, 2019

@atomi never mind. I really don't trust any government-like sites.
@sapk That's a good idea. I think we should only do gosum when releases but not PRs.

@lafriks
Copy link
Member

lafriks commented Aug 7, 2019

@lunny we need to check them always also as otherwise how we will know if PR is correct

@sapk
Copy link
Member Author

sapk commented Aug 7, 2019

Now we only download at build step so it will only checked at this step.

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants