Repository: favonia/cloudflare-ddns
Pull request #65 (2021-08-08 16:34:06)

refactor(detector): generic http and dns-over-https detectors

codecov[bot]: # [Codecov](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) Report
> Merging [#65](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) (43a3059) into [main](https://codecov.io/gh/favonia/cloudflare-ddns/commit/8463fab23fe3c7d8992d5dad2bd0a2afb1ed18b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) (8463fab) will **increase** coverage by `7.43%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65/graphs/tree.svg?width=650&height=150&src=pr&token=90Cwhy7OkI&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia)](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia)

```diff
@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
+ Coverage   35.43%   42.86%   +7.43%     
==========================================
  Files          20       20              
  Lines        1188     1199      +11     
==========================================
+ Hits          421      514      +93     
+ Misses        766      684      -82     
  Partials        1        1              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `42.86% <100.00%> (+7.43%)` | :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=favonia#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia) | Coverage Δ | |
|---|---|---|
| [internal/ipnet/ipnet.go](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#diff-aW50ZXJuYWwvaXBuZXQvaXBuZXQuZ28=) | `100.00% <100.00%> (ø)` | |
| [internal/updator/updator.go](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia#diff-aW50ZXJuYWwvdXBkYXRvci91cGRhdG9yLmdv) | `100.00% <0.00%> (+100.00%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia). Last update [8463fab...43a3059](https://codecov.io/gh/favonia/cloudflare-ddns/pull/65?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=favonia).


Patch:
diff --git a/internal/config/config.go b/internal/config/config.go
index ba0bf3a..a11c287 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -33,8 +33,8 @@ func Default() *Config {
 		Quiet: quiet.VERBOSE,
 		Auth:  nil,
 		Policy: map[ipnet.Type]detector.Policy{
-			ipnet.IP4: &detector.Cloudflare{},
-			ipnet.IP6: &detector.Cloudflare{},
+			ipnet.IP4: detector.NewCloudflare(),
+			ipnet.IP6: detector.NewCloudflare(),
 		},
 		Domains: map[ipnet.Type][]api.FQDN{
 			ipnet.IP4: nil,
@@ -257,7 +257,7 @@ func (c *Config) Normalize(indent pp.Indent) bool {
 	// change useless policies to
 	for ipNet, domains := range c.Domains {
 		if len(domains) == 0 && c.Policy[ipNet].IsManaged() {
-			c.Policy[ipNet] = &detector.Unmanaged{}
+			c.Policy[ipNet] = detector.NewUnmanaged()
 			pp.Printf(indent, pp.EmojiUserWarning, "IP%v_POLICY was changed to %q because no domains were set for %v.",
 				ipNet.Int(), c.Policy[ipNet], ipNet)
 		}
diff --git a/internal/config/env.go b/internal/config/env.go
index 6c024f4..f9f9f98 100644
--- a/internal/config/env.go
+++ b/internal/config/env.go
@@ -125,16 +125,16 @@ func ReadPolicy(quiet quiet.Quiet, indent pp.Indent, key string, field *detector
 		}
 		return true
 	case "cloudflare":
-		*field = &detector.Cloudflare{}
+		*field = detector.NewCloudflare()
 		return true
 	case "ipify":
-		*field = &detector.Ipify{}
+		*field = detector.NewIpify()
 		return true
 	case "local":
-		*field = &detector.Local{}
+		*field = detector.NewLocal()
 		return true
 	case "unmanaged":
-		*field = &detector.Unmanaged{}
+		*field = detector.NewUnmanaged()
 		return true
 	default:
 		pp.Printf(indent, pp.EmojiUserError, "Failed to parse %q: not a valid policy.", val)
diff --git a/internal/detector/cloudflare.go b/internal/detector/cloudflare.go
index 8e40c0d..fee8f74 100644
--- a/internal/detector/cloudflare.go
+++ b/internal/detector/cloudflare.go
@@ -1,51 +1,21 @@
 package detector
 
 import (
-	"context"
-	"net"
-
 	"golang.org/x/net/dns/dnsmessage"
 
 	"github.com/favonia/cloudflare-ddns/internal/ipnet"
-	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-type Cloudflare struct{}
-
-func (p *Cloudflare) IsManaged() bool {
-	return true
-}
-
-func (p *Cloudflare) String() string {
-	return "cloudflare"
-}
-
-func (p *Cloudflare) getIP4(ctx context.Context, indent pp.Indent) (net.IP, bool) {
-	ip, ok := getIPFromDNS(ctx, indent, "https://1.1.1.1/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS)
-	if !ok {
-		return nil, false
-	}
-
-	return ip.To4(), true
-}
-
-func (p *Cloudflare) getIP6(ctx context.Context, indent pp.Indent) (net.IP, bool) {
-	ip, ok := getIPFromDNS(ctx, indent,
-		"https://[2606:4700:4700::1111]/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS)
-	if !ok {
-		return nil, false
-	}
-
-	return ip.To16(), true
-}
-
-func (p *Cloudflare) GetIP(ctx context.Context, indent pp.Indent, ipNet ipnet.Type) (net.IP, bool) {
-	switch ipNet {
-	case ipnet.IP4:
-		return p.getIP4(ctx, indent)
-	case ipnet.IP6:
-		return p.getIP6(ctx, indent)
-	default:
-		return nil, false
+func NewCloudflare() Policy {
+	return &DNSOverHTTPS{
+		policyName: "cloudflare",
+		param: map[ipnet.Type]struct {
+			url   string
+			name  string
+			class dnsmessage.Class
+		}{
+			ipnet.IP4: {"https://1.1.1.1/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
+			ipnet.IP6: {"https://[2606:4700:4700::1111]/dns-query", "whoami.cloudflare.", dnsmessage.ClassCHAOS},
+		},
 	}
 }
diff --git a/internal/detector/dns.go b/internal/detector/dns.go
index 6ac2c74..0c0f07e 100644
--- a/internal/detector/dns.go
+++ b/internal/detector/dns.go
@@ -9,6 +9,7 @@ import (
 
 	"golang.org/x/net/dns/dnsmessage"
 
+	"github.com/favonia/cloudflare-ddns/internal/ipnet"
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
@@ -153,3 +154,34 @@ func getIPFromDNS(ctx context.Context, indent pp.Indent,
 
 	return c.getIP(ctx, indent)
 }
+
+type DNSOverHTTPS struct {
+	policyName string
+	param      map[ipnet.Type]struct {
+		url   string
+		name  string
+		class dnsmessage.Class
+	}
+}
+
+func (p *DNSOverHTTPS) IsManaged() bool {
+	return true
+}
+
+func (p *DNSOverHTTPS) String() string {
+	return p.policyName
+}
+
+func (p *DNSOverHTTPS) GetIP(ctx context.Context, indent pp.Indent, ipNet ipnet.Type) (net.IP, bool) {
+	param, found := p.param[ipNet]
+	if !found {
+		return nil, false
+	}
+
+	ip, ok := getIPFromDNS(ctx, indent, param.url, param.name, param.class)
+	if !ok {
+		return nil, false
+	}
+
+	return ipNet.NormalizeIP(ip), true
+}
diff --git a/internal/detector/http.go b/internal/detector/http.go
index f4961c8..c8f21b8 100644
--- a/internal/detector/http.go
+++ b/internal/detector/http.go
@@ -6,6 +6,7 @@ import (
 	"net"
 	"net/http"
 
+	"github.com/favonia/cloudflare-ddns/internal/ipnet"
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
@@ -54,3 +55,42 @@ func (d *httpConn) getIP(ctx context.Context, indent pp.Indent) (net.IP, bool) {
 
 	return ip, true
 }
+
+func getIPFromHTTP(ctx context.Context, indent pp.Indent, url string) (net.IP, bool) {
+	c := httpConn{
+		method:  http.MethodGet,
+		url:     url,
+		reader:  nil,
+		prepare: func(_ pp.Indent, _ *http.Request) bool { return true },
+		extract: func(_ pp.Indent, body []byte) (string, bool) { return string(body), true },
+	}
+
+	return c.getIP(ctx, indent)
+}
+
+type HTTP struct {
+	policyName string
+	url        map[ipnet.Type]string
+}
+
+func (p *HTTP) IsManaged() bool {
+	return true
+}
+
+func (p *HTTP) String() string {
+	return p.policyName
+}
+
+func (p *HTTP) GetIP(ctx context.Context, indent pp.Indent, ipNet ipnet.Type) (net.IP, bool) {
+	url, found := p.url[ipNet]
+	if !found {
+		return nil, false
+	}
+
+	ip, ok := getIPFromHTTP(ctx, indent, url)
+	if !ok {
+		return nil, false
+	}
+
+	return ipNet.NormalizeIP(ip), true
+}
diff --git a/internal/detector/ipify.go b/internal/detector/ipify.go
index dd6e583..c3c61b7 100644
--- a/internal/detector/ipify.go
+++ b/internal/detector/ipify.go
@@ -1,61 +1,15 @@
 package detector
 
 import (
-	"context"
-	"net"
-	"net/http"
-
 	"github.com/favonia/cloudflare-ddns/internal/ipnet"
-	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-func getIPFromHTTP(ctx context.Context, indent pp.Indent, url string) (net.IP, bool) {
-	c := httpConn{
-		method:  http.MethodGet,
-		url:     url,
-		reader:  nil,
-		prepare: func(_ pp.Indent, _ *http.Request) bool { return true },
-		extract: func(_ pp.Indent, body []byte) (string, bool) { return string(body), true },
-	}
-
-	return c.getIP(ctx, indent)
-}
-
-type Ipify struct{}
-
-func (p *Ipify) IsManaged() bool {
-	return true
-}
-
-func (p *Ipify) String() string {
-	return "ipify"
-}
-
-func (p *Ipify) getIP4(ctx context.Context, indent pp.Indent) (net.IP, bool) {
-	ip, ok := getIPFromHTTP(ctx, indent, "https://api4.ipify.org")
-	if !ok {
-		return nil, false
-	}
-
-	return ip.To4(), true
-}
-
-func (p *Ipify) getIP6(ctx context.Context, indent pp.Indent) (net.IP, bool) {
-	ip, ok := getIPFromHTTP(ctx, indent, "https://api6.ipify.org")
-	if !ok {
-		return nil, false
-	}
-
-	return ip.To16(), true
-}
-
-func (p *Ipify) GetIP(ctx context.Context, indent pp.Indent, ipNet ipnet.Type) (net.IP, bool) {
-	switch ipNet {
-	case ipnet.IP4:
-		return p.getIP4(ctx, indent)
-	case ipnet.IP6:
-		return p.getIP6(ctx, indent)
-	default:
-		return nil, false
+func NewIpify() Policy {
+	return &HTTP{
+		policyName: "ipify",
+		url: map[ipnet.Type]string{
+			ipnet.IP4: "https://api4.ipify.org",
+			ipnet.IP6: "https://api6.ipify.org",
+		},
 	}
 }
diff --git a/internal/detector/local.go b/internal/detector/local.go
index 31aa537..9492157 100644
--- a/internal/detector/local.go
+++ b/internal/detector/local.go
@@ -8,45 +8,46 @@ import (
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-type Local struct{}
+type Local struct {
+	policyName    string
+	remoteUDPAddr map[ipnet.Type]string
+}
 
 func (p *Local) IsManaged() bool {
 	return true
 }
 
 func (p *Local) String() string {
-	return "local"
+	return p.policyName
 }
 
-func (p *Local) getIP4(_ context.Context, indent pp.Indent) (net.IP, bool) {
-	conn, err := net.Dial("udp4", "1.1.1.1:443")
-	if err != nil {
-		pp.Printf(indent, pp.EmojiError, "Failed to detect a local IPv4 address: %v", err)
+func (p *Local) GetIP(ctx context.Context, indent pp.Indent, ipNet ipnet.Type) (net.IP, bool) {
+	remoteUDPAddr, found := p.remoteUDPAddr[ipNet]
+	if !found {
 		return nil, false
 	}
-	defer conn.Close()
-
-	return conn.LocalAddr().(*net.UDPAddr).IP.To4(), true
-}
 
-func (p *Local) getIP6(_ context.Context, indent pp.Indent) (net.IP, bool) {
-	conn, err := net.Dial("udp6", "[2606:4700:4700::1111]:443")
+	conn, err := net.Dial(ipNet.UDPNetwork(), remoteUDPAddr)
 	if err != nil {
-		pp.Printf(indent, pp.EmojiError, "Failed to detect a local IPv6 address: %v", err)
+		pp.Printf(indent, pp.EmojiError, "Failed to detect a local %s address: %v", ipNet.String(), err)
 		return nil, false
 	}
 	defer conn.Close()
 
-	return conn.LocalAddr().(*net.UDPAddr).IP.To16(), true
+	ip := ipNet.NormalizeIP(conn.LocalAddr().(*net.UDPAddr).IP)
+	if ip == nil {
+		return nil, false
+	}
+
+	return ip, true
 }
 
-func (p *Local) GetIP(ctx context.Context, indent pp.Indent, ipNet ipnet.Type) (net.IP, bool) {
-	switch ipNet {
-	case ipnet.IP4:
-		return p.getIP4(ctx, indent)
-	case ipnet.IP6:
-		return p.getIP6(ctx, indent)
-	default:
-		return nil, false
+func NewLocal() Policy {
+	return &Local{
+		policyName: "local",
+		remoteUDPAddr: map[ipnet.Type]string{
+			ipnet.IP4: "1.1.1.1:443",
+			ipnet.IP6: "[2606:4700:4700::1111]:443",
+		},
 	}
 }
diff --git a/internal/detector/unmanaged.go b/internal/detector/unmanaged.go
index 6ff8e1c..0fbac92 100644
--- a/internal/detector/unmanaged.go
+++ b/internal/detector/unmanaged.go
@@ -8,16 +8,20 @@ import (
 	"github.com/favonia/cloudflare-ddns/internal/pp"
 )
 
-type Unmanaged struct{}
+type unmanaged struct{}
 
-func (p *Unmanaged) IsManaged() bool {
+func (p *unmanaged) IsManaged() bool {
 	return false
 }
 
-func (p *Unmanaged) String() string {
+func (p *unmanaged) String() string {
 	return "unmanaged"
 }
 
-func (p *Unmanaged) GetIP(_ context.Context, _ pp.Indent, _ ipnet.Type) (net.IP, bool) {
+func (p *unmanaged) GetIP(_ context.Context, _ pp.Indent, _ ipnet.Type) (net.IP, bool) {
 	return nil, false
 }
+
+func NewUnmanaged() Policy {
+	return &unmanaged{}
+}
diff --git a/internal/ipnet/ipnet.go b/internal/ipnet/ipnet.go
index e655e77..b7878dc 100644
--- a/internal/ipnet/ipnet.go
+++ b/internal/ipnet/ipnet.go
@@ -3,6 +3,7 @@ package ipnet
 
 import (
 	"fmt"
+	"net"
 )
 
 // Type is the type of IP networks.
@@ -46,3 +47,26 @@ func (t Type) Int() int {
 		return 0
 	}
 }
+
+func (t Type) NormalizeIP(ip net.IP) net.IP {
+	switch t {
+	case IP4:
+		return ip.To4()
+	case IP6:
+		return ip.To16()
+	default:
+		return ip
+	}
+}
+
+// UDPNetwork gives the network name for net.Dial.
+func (t Type) UDPNetwork() string {
+	switch t {
+	case IP4:
+		return "udp4"
+	case IP6:
+		return "udp6"
+	default:
+		return ""
+	}
+}


Test patch:
diff --git a/internal/config/env_test.go b/internal/config/env_test.go
index 923f8cc..df965e3 100644
--- a/internal/config/env_test.go
+++ b/internal/config/env_test.go
@@ -253,10 +253,10 @@ func TestReadPolicy(t *testing.T) {
 	key := keyPrefix + "POLICY"
 
 	var (
-		cloudflare = &detector.Cloudflare{}
-		local      = &detector.Local{}
-		unmanaged  = &detector.Unmanaged{}
-		ipify      = &detector.Ipify{}
+		cloudflare = detector.NewCloudflare()
+		local      = detector.NewLocal()
+		unmanaged  = detector.NewUnmanaged()
+		ipify      = detector.NewIpify()
 	)
 
 	for name, tc := range map[string]struct {
diff --git a/internal/ipnet/ipnet_test.go b/internal/ipnet/ipnet_test.go
index b1580fb..699fd6a 100644
--- a/internal/ipnet/ipnet_test.go
+++ b/internal/ipnet/ipnet_test.go
@@ -1,9 +1,10 @@
 package ipnet_test
 
 import (
+	"net"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/favonia/cloudflare-ddns/internal/ipnet"
 )
@@ -11,8 +12,8 @@ import (
 func TestString(t *testing.T) {
 	t.Parallel()
 	for name, tc := range map[string]struct {
-		input  ipnet.Type
-		output string
+		input    ipnet.Type
+		expected string
 	}{
 		"4":   {ipnet.IP4, "IPv4"},
 		"6":   {ipnet.IP6, "IPv6"},
@@ -21,7 +22,7 @@ func TestString(t *testing.T) {
 		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			assert.Equal(t, tc.output, tc.input.String())
+			require.Equal(t, tc.expected, tc.input.String())
 		})
 	}
 }
@@ -29,8 +30,8 @@ func TestString(t *testing.T) {
 func TestRecordType(t *testing.T) {
 	t.Parallel()
 	for name, tc := range map[string]struct {
-		input  ipnet.Type
-		output string
+		input    ipnet.Type
+		expected string
 	}{
 		"4":   {ipnet.IP4, "A"},
 		"6":   {ipnet.IP6, "AAAA"},
@@ -39,7 +40,7 @@ func TestRecordType(t *testing.T) {
 		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			assert.Equal(t, tc.output, tc.input.RecordType())
+			require.Equal(t, tc.expected, tc.input.RecordType())
 		})
 	}
 }
@@ -47,8 +48,8 @@ func TestRecordType(t *testing.T) {
 func TestInt(t *testing.T) {
 	t.Parallel()
 	for name, tc := range map[string]struct {
-		input  ipnet.Type
-		output int
+		input    ipnet.Type
+		expected int
 	}{
 		"4":   {ipnet.IP4, 4},
 		"6":   {ipnet.IP6, 6},
@@ -57,7 +58,47 @@ func TestInt(t *testing.T) {
 		tc := tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			assert.Equal(t, tc.output, tc.input.Int())
+			require.Equal(t, tc.expected, tc.input.Int())
+		})
+	}
+}
+
+func TestNormalizeIP(t *testing.T) {
+	t.Parallel()
+	for name, tc := range map[string]struct {
+		ipNet    ipnet.Type
+		ip       net.IP
+		expected net.IP
+	}{
+		"4-1::2":             {ipnet.IP4, net.ParseIP("1::2").To16(), nil},
+		"4-::ffff:0a0a:0a0a": {ipnet.IP4, net.ParseIP("::ffff:0a0a:0a0a").To16(), net.ParseIP("10.10.10.10").To4()},
+		"6-1::2":             {ipnet.IP6, net.ParseIP("1::2").To16(), net.ParseIP("1::2").To16()},
+		"6-10.10.10.10":      {ipnet.IP6, net.ParseIP("10.10.10.10").To4(), net.ParseIP("10.10.10.10").To16()},
+		"100-nil":            {100, nil, nil},
+		"100-10.10.10.10":    {100, net.ParseIP("10.10.10.10").To4(), net.ParseIP("10.10.10.10").To4()},
+	} {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tc.expected, tc.ipNet.NormalizeIP(tc.ip))
+		})
+	}
+}
+
+func TestUDPNetwork(t *testing.T) {
+	t.Parallel()
+	for name, tc := range map[string]struct {
+		input    ipnet.Type
+		expected string
+	}{
+		"4":   {ipnet.IP4, "udp4"},
+		"6":   {ipnet.IP6, "udp6"},
+		"100": {ipnet.Type(100), ""},
+	} {
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tc.expected, tc.input.UDPNetwork())
 		})
 	}
 }