Add proxy settings and support for migration and webhook #16704

lunny · 2021-08-16T13:40:08Z

replace #15234, #16697 and fix #12018

This PR added a proxy supports which should be applied to every request to external http/https URL.
Follow requests will follow the proxy setting if no standalone setting.

Git clone from external http/https URLs
Migrate LFS datas from external git http/https URLs
Migrate other datas via API from other git services http/https URLs
Send webhooks to external URLs

And since webhook has supported standalone proxy before this PR, webhook's proxy settings will override the proxy settings.

This PR also added an option to ignore tls verify on migrations.

⚠️ BREAKING ⚠️

This PR means that by default Gitea will not obey the system proxy unless you set [proxy] ENABLED= true but leave the rest of the proxy configuration blank.

This is in contrast to previous behaviour where Gitea would partially obey system proxies.

custom/conf/app.example.ini

KN4CK3R · 2021-08-16T13:50:44Z

The LFS client is missing. Otherwise lgtm.

docs/content/doc/advanced/config-cheat-sheet.en-us.md

modules/git/command.go

modules/git/repo.go

lunny · 2021-08-17T02:06:17Z

The LFS client is missing. Otherwise lgtm.

done.

codecov-commenter · 2021-08-17T16:27:11Z

Codecov Report

Merging #16704 (6eca452) into main (422c30d) will decrease coverage by 0.01%.
The diff coverage is 50.36%.

@@            Coverage Diff             @@
##             main   #16704      +/-   ##
==========================================
- Coverage   45.37%   45.36%   -0.02%     
==========================================
  Files         758      760       +2     
  Lines       85381    85482     +101     
==========================================
+ Hits        38742    38779      +37     
- Misses      40358    40420      +62     
- Partials     6281     6283       +2

Impacted Files	Coverage Δ
modules/migrations/gitea_downloader.go	`0.90% <0.00%> (-0.03%)`	⬇️
modules/migrations/gitlab.go	`0.97% <0.00%> (-0.03%)`	⬇️
modules/migrations/gogs.go	`2.35% <0.00%> (-0.02%)`	⬇️
services/mirror/mirror_pull.go	`32.74% <0.00%> (ø)`
modules/proxy/proxy.go	`18.75% <18.75%> (ø)`
modules/setting/proxy.go	`45.45% <45.45%> (ø)`
modules/repository/repo.go	`47.32% <66.66%> (ø)`
services/mirror/mirror_push.go	`39.23% <66.66%> (ø)`
modules/migrations/github.go	`68.56% <71.42%> (+0.30%)`	⬆️
modules/git/repo.go	`47.71% <87.50%> (+2.31%)`	⬆️
... and 13 more

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 422c30d...6eca452. Read the comment docs.

zeripath · 2021-08-17T16:46:35Z

docs/content/doc/advanced/config-cheat-sheet.en-us.md

+## Proxy (`proxy`)
+
+- `PROXY_ENABLED`: **false**: Enable the proxy if true, all requests to external via HTTP will be affected, if false, no proxy will be used even environment http_proxy/https_proxy
+- `PROXY_URL`: ****: Proxy server URL, support http://, https//, socks://, blank will follow environment http_proxy/https_proxy


Suggested change

- `PROXY_URL`: ****: Proxy server URL, support http://, https//, socks://, blank will follow environment http_proxy/https_proxy

- `PROXY_URL`: **\<empty\>`**: Proxy server URL, support http://, https//, socks://, blank will follow environment http_proxy/https_proxy

docs/content/doc/advanced/config-cheat-sheet.en-us.md

docs/content/doc/advanced/config-cheat-sheet.zh-cn.md

zeripath · 2021-08-17T16:59:37Z

I think setting the proxy off by default is both breaking and the wrong thing to do.

We should have it on by default so that we defer to the system proxy by default.

KN4CK3R · 2021-08-17T17:27:52Z

Missing:

gitea/modules/migrations/gitlab.go

Line 317 in 274aeb3

resp, err := http.DefaultClient.Do(req)

gitea/modules/migrations/github.go

Line 298 in 274aeb3

resp, err := http.DefaultClient.Do(req)

gitea/modules/migrations/gitea_downloader.go

Line 285 in 274aeb3

resp, err := http.Get(asset.DownloadURL)

lunny · 2021-08-18T01:54:06Z

I think setting the proxy off by default is both breaking and the wrong thing to do.

We should have it on by default so that we defer to the system proxy by default.

I don't think it will break anything. off means follow operating system proxy setting.

lunny · 2021-08-18T04:50:06Z

@zeripath @KN4CK3R Both done.

Co-authored-by: zeripath <art27@cantab.net>

lafriks · 2021-08-18T14:58:43Z

modules/proxy/proxy.go

+		if os.Getenv("http_proxy") != "" {
+			return os.Getenv("http_proxy")
+		}
+		return os.Getenv("https_proxy")


httpproxy.FromEnvironment().ProxyFunc()(url) should be used instead of custom logic as it also contains support for NO_PROXY env varaible

We cannot use that, because we will set it as envs of git clone.

lafriks · 2021-08-18T14:59:31Z

modules/git/repo.go

+	u, err := url.Parse(from)
+	if err == nil && (strings.EqualFold(u.Scheme, "http") || strings.EqualFold(u.Scheme, "https")) {
+		if proxy.Match(u.Host) {
+			envs = append(envs, fmt.Sprintf("https_proxy=%s", proxy.GetProxyURL()))


https_proxy or http_proxy should be used based on either https or http proxy protocol is used

lunny · 2021-08-18T16:05:44Z

@lafriks Could you send a PR to fix them?

lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 16, 2021

lunny added this to the 1.16.0 milestone Aug 16, 2021

6543 reviewed Aug 16, 2021

View reviewed changes

custom/conf/app.example.ini Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 16, 2021

KN4CK3R approved these changes Aug 16, 2021

View reviewed changes

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 16, 2021

zeripath reviewed Aug 16, 2021

View reviewed changes

docs/content/doc/advanced/config-cheat-sheet.en-us.md Outdated Show resolved Hide resolved

zeripath reviewed Aug 16, 2021

View reviewed changes

docs/content/doc/advanced/config-cheat-sheet.en-us.md Outdated Show resolved Hide resolved

zeripath reviewed Aug 16, 2021

View reviewed changes

modules/git/command.go Outdated Show resolved Hide resolved

zeripath reviewed Aug 16, 2021

View reviewed changes

modules/git/repo.go Outdated Show resolved Hide resolved

lunny force-pushed the lunny/proxy branch from a1b9e1b to a633fa0 Compare August 17, 2021 02:06

lunny force-pushed the lunny/proxy branch from 24d7e7f to 368d98e Compare August 17, 2021 16:11

zeripath reviewed Aug 17, 2021

View reviewed changes

docs/content/doc/advanced/config-cheat-sheet.en-us.md Outdated Show resolved Hide resolved

zeripath reviewed Aug 17, 2021

View reviewed changes

docs/content/doc/advanced/config-cheat-sheet.zh-cn.md Outdated Show resolved Hide resolved

lunny added 8 commits August 18, 2021 12:49

Add proxy settings and support for migration and webhook

c1ac622

Fix default value

9aa2dd3

Add newline for example ini

51db33b

Add lfs proxy support

7c6f205

Fix lint

803b7c1

Follow @zeripath's review

264f963

Fix git clone

c0e5bbc

Fix test

634c613

missgin http requests for proxy

6eca452

lunny force-pushed the lunny/proxy branch from 368d98e to 6eca452 Compare August 18, 2021 04:49

use empty

a076155

Co-authored-by: zeripath <art27@cantab.net>

6543 approved these changes Aug 18, 2021

View reviewed changes

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 18, 2021

lunny merged commit f9acad8 into go-gitea:main Aug 18, 2021

lunny deleted the lunny/proxy branch August 18, 2021 13:11

lafriks reviewed Aug 18, 2021

View reviewed changes

lunny mentioned this pull request Aug 18, 2021

Migration will clone repository from the proxy #16697

Closed

zeripath mentioned this pull request Aug 19, 2021

Return nil proxy function if proxy not enabled #16742

Merged

lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Aug 20, 2021

go-gitea locked and limited conversation to collaborators Oct 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add proxy settings and support for migration and webhook #16704

Add proxy settings and support for migration and webhook #16704

lunny commented Aug 16, 2021 •

edited by zeripath

Loading

KN4CK3R commented Aug 16, 2021

lunny commented Aug 17, 2021

codecov-commenter commented Aug 17, 2021 •

edited

Loading

zeripath Aug 17, 2021 •

edited

Loading

zeripath commented Aug 17, 2021

KN4CK3R commented Aug 17, 2021

lunny commented Aug 18, 2021

lunny commented Aug 18, 2021

lafriks Aug 18, 2021

lunny Aug 20, 2021

lafriks Aug 18, 2021

lunny commented Aug 18, 2021

	- `PROXY_URL`: ****: Proxy server URL, support http://, https//, socks://, blank will follow environment http_proxy/https_proxy
	- `PROXY_URL`: \<empty\>`: Proxy server URL, support http://, https//, socks://, blank will follow environment http_proxy/https_proxy

Add proxy settings and support for migration and webhook #16704

Add proxy settings and support for migration and webhook #16704

Conversation

lunny commented Aug 16, 2021 • edited by zeripath Loading

⚠️ BREAKING ⚠️

KN4CK3R commented Aug 16, 2021

lunny commented Aug 17, 2021

codecov-commenter commented Aug 17, 2021 • edited Loading

Codecov Report

zeripath Aug 17, 2021 • edited Loading

Choose a reason for hiding this comment

zeripath commented Aug 17, 2021

KN4CK3R commented Aug 17, 2021

lunny commented Aug 18, 2021

lunny commented Aug 18, 2021

lafriks Aug 18, 2021

Choose a reason for hiding this comment

lunny Aug 20, 2021

Choose a reason for hiding this comment

lafriks Aug 18, 2021

Choose a reason for hiding this comment

lunny commented Aug 18, 2021

lunny commented Aug 16, 2021 •

edited by zeripath

Loading

codecov-commenter commented Aug 17, 2021 •

edited

Loading

zeripath Aug 17, 2021 •

edited

Loading