Repository: dragonflyoss/Dragonfly2
Pull request #965 (2021-12-29 11:36:32)

Add useProxies to registryMirror allowing to mirror more anything
## Description

Add a new field in registryMirror, allowing to use proxies list to decide whether to use dragonfly.
By doing this, it is now possible to cache more than only blobs/sha256 urls, allowing to use dragonfly2 for other usage thatn docker registries, like yum repositories.

Also fix the whiteList test which is tested on direct request, although host is empty and canno tbe tested.

Please update the documentation in Chinese, I just can't do it.

## Related Issue

#959 

## Motivation and Context

This PR now allows to use dragonfly to mirror yum repositories, at least.

## Screenshots (if appropriate):

## Types of changes

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist:

- [x] Code compiles correctly.
- [x] My code follows the code style of this project.
- [x] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly.
- [x] I have read the **CONTRIBUTING** document.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.


codecov-commenter: # [Codecov](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) Report
> Merging [#965](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) (8220f05) into [main](https://codecov.io/gh/dragonflyoss/Dragonfly2/commit/abfa0a3fb0753cfad3b121e53149ab5ff871a379?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) (abfa0a3) will **increase** coverage by `0.09%`.
> The diff coverage is `75.00%`.

[![Impacted file tree graph](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965/graphs/tree.svg?width=650&height=150&src=pr&token=PUICJYSAT7&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss)](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss)

```diff
@@            Coverage Diff             @@
##             main     #965      +/-   ##
==========================================
+ Coverage   42.19%   42.29%   +0.09%     
==========================================
  Files         116      116              
  Lines        8783     8789       +6     
==========================================
+ Hits         3706     3717      +11     
+ Misses       4609     4607       -2     
+ Partials      468      465       -3     
```

| Flag | Coverage Δ | |
|---|---|---|
| Object-compatibility-e2etests | `?` | |
| e2etests | `?` | |
| unittests | `42.29% <75.00%> (+0.09%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) | Coverage Δ | |
|---|---|---|
| [client/config/peerhost.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2NvbmZpZy9wZWVyaG9zdC5nbw==) | `35.29% <ø> (ø)` | |
| [client/daemon/proxy/proxy.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9wcm94eS9wcm94eS5nbw==) | `8.81% <75.00%> (+1.54%)` | :arrow_up: |
| [client/daemon/peer/peertask\_base.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-Y2xpZW50L2RhZW1vbi9wZWVyL3BlZXJ0YXNrX2Jhc2UuZ28=) | `45.38% <0.00%> (+0.57%)` | :arrow_up: |
| [pkg/sync/krwmutex.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-cGtnL3N5bmMva3J3bXV0ZXguZ28=) | `86.36% <0.00%> (+4.54%)` | :arrow_up: |
| [pkg/sync/kmutex.go](https://codecov.io/gh/dragonflyoss/Dragonfly2/pull/965/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-cGtnL3N5bmMva211dGV4Lmdv) | `100.00% <0.00%> (+16.66%)` | :arrow_up: |

momiji: I apologize as I've never done this before, but I guess once used to the PR process, it will be easier and less a pain for me :)

I've tested the fix and confirm is it working perfectly on my real environments, using the docker image built for this purpose on my github account.

Will there be a v2.0.2 or v2.1.0 version including the fix soon? Not sure if including the alpha changes is necessary.

jim3ma: > I apologize as I've never done this before, but I guess once used to the PR process, it will be easier and less a pain for me :)
> 
> I've tested the fix and confirm is it working perfectly on my real environments, using the docker image built for this purpose on my github account.
> 
> Will there be a v2.0.2 or v2.1.0 version including the fix soon? Not sure if including the alpha changes is necessary.

Next release is v2.0.2. This change will be included.

Patch:
diff --git a/client/config/peerhost.go b/client/config/peerhost.go
index 211e11d8..aa6b5c58 100644
--- a/client/config/peerhost.go
+++ b/client/config/peerhost.go
@@ -527,6 +527,9 @@ type RegistryMirror struct {
 
 	// Request the remote registry directly.
 	Direct bool `yaml:"direct" mapstructure:"direct"`
+
+	// Whether to use proxies to decide when to use dragonfly
+	UseProxies bool `yaml:"useProxies" mapstructure:"useProxies"`
 }
 
 // TLSConfig returns the tls.Config used to communicate with the mirror.
diff --git a/client/daemon/proxy/proxy.go b/client/daemon/proxy/proxy.go
index dd3e2bb8..6bf2155d 100644
--- a/client/daemon/proxy/proxy.go
+++ b/client/daemon/proxy/proxy.go
@@ -269,8 +269,11 @@ func (proxy *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// check direct request
+	directRequest := r.Method != http.MethodConnect && r.URL.Scheme == ""
+
 	// check whiteList
-	if !proxy.checkWhiteList(r) {
+	if !directRequest && !proxy.checkWhiteList(r) {
 		status := http.StatusUnauthorized
 		http.Error(w, http.StatusText(status), status)
 		logger.Debugf("not in whitelist: %s, url：%s", r.Host, r.URL.String())
@@ -556,7 +559,13 @@ func (proxy *Proxy) shouldUseDragonfly(req *http.Request) bool {
 // shouldUseDragonflyForMirror returns whether we should use dragonfly to proxy a request
 // when we use registry mirror.
 func (proxy *Proxy) shouldUseDragonflyForMirror(req *http.Request) bool {
-	return proxy.registry != nil && !proxy.registry.Direct && transport.NeedUseDragonfly(req)
+	if proxy.registry == nil || proxy.registry.Direct {
+		return false
+	}
+	if proxy.registry.UseProxies {
+		return proxy.shouldUseDragonfly(req)
+	}
+	return transport.NeedUseDragonfly(req)
 }
 
 // tunnelHTTPS handles a CONNECT request and proxy an https request through an
diff --git a/docs/en/deployment/configuration/dfget.yaml b/docs/en/deployment/configuration/dfget.yaml
index 6f42a2ae..b640a435 100644
--- a/docs/en/deployment/configuration/dfget.yaml
+++ b/docs/en/deployment/configuration/dfget.yaml
@@ -221,6 +221,8 @@ proxy:
     certs: []
     # whether to request the remote registry directly
     direct: false
+    # whether to use proxies to decide if dragonfly should be used
+    useProxies: false
 
   proxies:
     # proxy all http image layer download requests with dfget
diff --git a/docs/zh-CN/deployment/configuration/dfget.yaml b/docs/zh-CN/deployment/configuration/dfget.yaml
index 53a41ce1..62fcd72a 100644
--- a/docs/zh-CN/deployment/configuration/dfget.yaml
+++ b/docs/zh-CN/deployment/configuration/dfget.yaml
@@ -194,6 +194,8 @@ proxy:
     certs: []
     # 是否直连镜像中心，true 的话，流量不再走 p2p
     direct: false
+    # whether to use proxies to decide if dragonfly should be used
+    useProxies: false
 
   proxies:
     # 代理镜像 blobs 信息


Test patch:
diff --git a/client/daemon/proxy/proxy_test.go b/client/daemon/proxy/proxy_test.go
index 582f9ad6..63990129 100644
--- a/client/daemon/proxy/proxy_test.go
+++ b/client/daemon/proxy/proxy_test.go
@@ -56,7 +56,7 @@ func (tc *testCase) WithRule(regx string, direct bool, useHTTPS bool, redirect s
 	return tc
 }
 
-func (tc *testCase) WithRegistryMirror(rawURL string, direct bool, dynamic bool) *testCase {
+func (tc *testCase) WithRegistryMirror(rawURL string, direct bool, dynamic bool, useProxies bool) *testCase {
 	if tc.Error != nil {
 		return tc
 	}
@@ -67,6 +67,7 @@ func (tc *testCase) WithRegistryMirror(rawURL string, direct bool, dynamic bool)
 		Remote:        &config.URL{URL: u},
 		DynamicRemote: dynamic,
 		Direct:        direct,
+		UseProxies:    useProxies,
 	}
 	return tc
 }
@@ -109,7 +110,7 @@ func (tc *testCase) TestMirror(t *testing.T) {
 	if !a.Nil(tc.Error) {
 		return
 	}
-	tp, err := NewProxy(WithRegistryMirror(tc.RegistryMirror))
+	tp, err := NewProxy(WithRegistryMirror(tc.RegistryMirror), WithRules(tc.Rules))
 	if !a.Nil(err) {
 		return
 	}
@@ -154,17 +155,39 @@ func TestMatch(t *testing.T) {
 		TestMirror(t)
 
 	newTestCase().
-		WithRegistryMirror("http://index.docker.io", false, false).
+		WithRegistryMirror("http://index.docker.io", false, false, false).
 		WithTest("http://h/a", true, false, "").
 		TestMirror(t)
 
 	newTestCase().
-		WithRegistryMirror("http://index.docker.io", false, false).
+		WithRegistryMirror("http://index.docker.io", false, false, false).
 		WithTest("http://index.docker.io/v2/blobs/sha256/xxx", false, false, "").
 		TestMirror(t)
 
 	newTestCase().
-		WithRegistryMirror("http://index.docker.io", true, false).
+		WithRegistryMirror("http://index.docker.io", true, false, false).
+		WithTest("http://index.docker.io/v2/blobs/sha256/xxx", true, false, "").
+		TestMirror(t)
+}
+
+func TestMatchWithUseProxies(t *testing.T) {
+	// should direct as registry is set with direct=false and no proxies are defined
+	newTestCase().
+		WithRegistryMirror("http://index.docker.io", false, false, true).
+		WithTest("http://index.docker.io/v2/blobs/sha256/xxx", true, false, "").
+		TestMirror(t)
+
+	// should cache as registry is set with direct=false, and one proxy matches
+	newTestCase().
+		WithRegistryMirror("http://index.docker.io", false, false, true).
+		WithRule("/blobs/sha256/", false, false, "").
+		WithTest("http://index.docker.io/v2/blobs/sha256/xxx", false, false, "").
+		TestMirror(t)
+
+	// should direct as registry is set with direct=true, even if one proxy matches
+	newTestCase().
+		WithRegistryMirror("http://index.docker.io", true, false, true).
+		WithRule("/blobs/sha256/", false, false, "").
 		WithTest("http://index.docker.io/v2/blobs/sha256/xxx", true, false, "").
 		TestMirror(t)
 }