From 3c90d082e02af9ae3a38f6650d65f91f00fdd6f5 Mon Sep 17 00:00:00 2001 From: THoelzel <15081333+THoelzel@users.noreply.github.com> Date: Sun, 21 Apr 2019 12:13:10 -0400 Subject: [PATCH 1/4] Fix blacklistIPs JS configuration Net.IPNet does not support JSON unmarshalling, causing the blacklistIPs configuration to fail when set through JS. Fixed by wrapping net.IPNet for the purpose of JSON unmarshalling. Fixes #973 --- cmd/options.go | 3 +-- js/runner_test.go | 43 ++++++++++++++++++++++++++++++++++++++++--- lib/netext/dialer.go | 5 +++-- lib/options.go | 39 ++++++++++++++++++++++++++++++++++++++- lib/options_test.go | 33 ++++++++++++++++++++++++++++++--- 5 files changed, 112 insertions(+), 11 deletions(-) diff --git a/cmd/options.go b/cmd/options.go index 40164b8efb8..3ab181697d6 100644 --- a/cmd/options.go +++ b/cmd/options.go @@ -22,7 +22,6 @@ package cmd import ( "fmt" - "net" "strings" "time" @@ -122,7 +121,7 @@ func getOptions(flags *pflag.FlagSet) (lib.Options, error) { return opts, err } for _, s := range blacklistIPStrings { - _, net, err := net.ParseCIDR(s) + net, err := lib.ParseCIDR(s) if err != nil { return opts, errors.Wrap(err, "blacklist-ip") } diff --git a/js/runner_test.go b/js/runner_test.go index bd15201293d..916fa840757 100644 --- a/js/runner_test.go +++ b/js/runner_test.go @@ -747,7 +747,7 @@ func TestVUIntegrationInsecureRequests(t *testing.T) { } } -func TestVUIntegrationBlacklist(t *testing.T) { +func TestVUIntegrationBlacklistOption(t *testing.T) { r1, err := New(&lib.SourceData{ Filename: "/script.js", Data: []byte(` @@ -759,13 +759,13 @@ func TestVUIntegrationBlacklist(t *testing.T) { return } - _, cidr, err := net.ParseCIDR("10.0.0.0/8") + cidr, err := lib.ParseCIDR("10.0.0.0/8") if !assert.NoError(t, err) { return } r1.SetOptions(lib.Options{ Throw: null.BoolFrom(true), - BlacklistIPs: []*net.IPNet{cidr}, + BlacklistIPs: []*lib.IPNet{cidr}, }) r2, err := NewFromArchive(r1.MakeArchive(), lib.RuntimeOptions{}) @@ -786,6 +786,43 @@ func TestVUIntegrationBlacklist(t *testing.T) { } } +func TestVUIntegrationBlacklistScript(t *testing.T) { + r1, err := New(&lib.SourceData{ + Filename: "/script.js", + Data: []byte(` + import http from "k6/http"; + + export let options = { + throw: true, + blacklistIPs: ["10.0.0.0/8"], + }; + + export default function() { http.get("http://10.1.2.3/"); } + `), + }, afero.NewMemMapFs(), lib.RuntimeOptions{}) + if !assert.NoError(t, err) { + return + } + + r2, err := NewFromArchive(r1.MakeArchive(), lib.RuntimeOptions{}) + if !assert.NoError(t, err) { + return + } + + runners := map[string]*Runner{"Source": r1, "Archive": r2} + + for name, r := range runners { + t.Run(name, func(t *testing.T) { + vu, err := r.NewVU(make(chan stats.SampleContainer, 100)) + if !assert.NoError(t, err) { + return + } + err = vu.RunOnce(context.Background()) + assert.EqualError(t, err, "GoError: Get http://10.1.2.3/: IP (10.1.2.3) is in a blacklisted range (10.0.0.0/8)") + }) + } +} + func TestVUIntegrationHosts(t *testing.T) { tb := testutils.NewHTTPMultiBin(t) defer tb.Cleanup() diff --git a/lib/netext/dialer.go b/lib/netext/dialer.go index 7026f859961..0954a863b7e 100644 --- a/lib/netext/dialer.go +++ b/lib/netext/dialer.go @@ -28,6 +28,7 @@ import ( "sync/atomic" "time" + "github.com/loadimpact/k6/lib" "github.com/loadimpact/k6/lib/metrics" "github.com/loadimpact/k6/stats" @@ -40,7 +41,7 @@ type Dialer struct { net.Dialer Resolver *dnscache.Resolver - Blacklist []*net.IPNet + Blacklist []*lib.IPNet Hosts map[string]net.IP BytesRead int64 @@ -58,7 +59,7 @@ func NewDialer(dialer net.Dialer) *Dialer { // BlackListedIPError is an error that is returned when a given IP is blacklisted type BlackListedIPError struct { ip net.IP - net *net.IPNet + net *lib.IPNet } func (b BlackListedIPError) Error() string { diff --git a/lib/options.go b/lib/options.go index f4a4657e401..8facde66732 100644 --- a/lib/options.go +++ b/lib/options.go @@ -205,6 +205,43 @@ func (c *TLSAuth) Certificate() (*tls.Certificate, error) { return c.certificate, nil } +// IPNet is a wrapper around net.IPNet for JSON unmarshalling +type IPNet struct { + net.IPNet +} + +// UnmarshalJSON populates the IPNet from the given CIDR JSON +func (ipnet *IPNet) UnmarshalJSON(b []byte) (err error) { + var cidr string + if err := json.Unmarshal(b, &cidr); err != nil { + return err + } + + newIPNet, err := ParseCIDR(cidr) + if err != nil { + return errors.Wrap(err, "Failed to parse CIDR") + } + + *ipnet = *newIPNet + + return nil +} + +// ParseCIDR creates an IPNet out of a CIDR string +func ParseCIDR(s string) (*IPNet, error) { + ip, ipnet, err := net.ParseCIDR(s) + if err != nil { + return nil, err + } + + return &IPNet{ + IPNet: net.IPNet{ + IP: ip, + Mask: ipnet.Mask, + }, + }, nil +} + type Options struct { // Should the test start in a paused state? Paused null.Bool `json:"paused" envconfig:"paused"` @@ -258,7 +295,7 @@ type Options struct { Thresholds map[string]stats.Thresholds `json:"thresholds" envconfig:"thresholds"` // Blacklist IP ranges that tests may not contact. Mainly useful in hosted setups. - BlacklistIPs []*net.IPNet `json:"blacklistIPs" envconfig:"blacklist_ips"` + BlacklistIPs []*IPNet `json:"blacklistIPs" envconfig:"blacklist_ips"` // Hosts overrides dns entries for given hosts Hosts map[string]net.IP `json:"hosts" envconfig:"hosts"` diff --git a/lib/options_test.go b/lib/options_test.go index 63cf4ae891e..4c7e222cd7e 100644 --- a/lib/options_test.go +++ b/lib/options_test.go @@ -308,9 +308,11 @@ func TestOptions(t *testing.T) { }) t.Run("BlacklistIPs", func(t *testing.T) { opts := Options{}.Apply(Options{ - BlacklistIPs: []*net.IPNet{{ - IP: net.IPv4zero, - Mask: net.CIDRMask(1, 1), + BlacklistIPs: []*IPNet{{ + net.IPNet{ + IP: net.IPv4zero, + Mask: net.CIDRMask(1, 1), + }, }}, }) assert.NotNil(t, opts.BlacklistIPs) @@ -513,3 +515,28 @@ func TestTagSetTextUnmarshal(t *testing.T) { require.Equal(t, (map[string]bool)(*set), expected) } } + +func TestCIDRUnmarshal(t *testing.T) { + + var testData = []struct { + input string + expected *IPNet + }{ + {"10.0.0.0/8", &IPNet{IPNet: net.IPNet{ + IP: net.ParseIP("10.0.0.0"), + Mask: net.IPv4Mask(255, 0, 0, 0), + }}}, + {"fc00:1234:5678::/48", &IPNet{IPNet: net.IPNet{ + IP: net.ParseIP("fc00:1234:5678::"), + Mask: net.CIDRMask(48, 128), + }}}, + } + + for _, data := range testData { + actualIPNet := &IPNet{} + err := actualIPNet.UnmarshalJSON([]byte("\"" + data.input + "\"")) + require.NoError(t, err) + + assert.Equal(t, data.expected, actualIPNet) + } +} From 9ec56c8575d5fb836de96f023876a6bf2f9631c7 Mon Sep 17 00:00:00 2001 From: THoelzel <15081333+THoelzel@users.noreply.github.com> Date: Sun, 21 Apr 2019 19:10:19 -0400 Subject: [PATCH 2/4] Improve code coverage --- cmd/options.go | 6 +++--- js/runner_test.go | 1 + lib/options.go | 2 +- lib/options_test.go | 49 +++++++++++++++++++++++++++++++-------------- 4 files changed, 39 insertions(+), 19 deletions(-) diff --git a/cmd/options.go b/cmd/options.go index 3ab181697d6..91db3805f8a 100644 --- a/cmd/options.go +++ b/cmd/options.go @@ -121,9 +121,9 @@ func getOptions(flags *pflag.FlagSet) (lib.Options, error) { return opts, err } for _, s := range blacklistIPStrings { - net, err := lib.ParseCIDR(s) - if err != nil { - return opts, errors.Wrap(err, "blacklist-ip") + net, parseErr := lib.ParseCIDR(s) + if parseErr != nil { + return opts, errors.Wrap(parseErr, "blacklist-ip") } opts.BlacklistIPs = append(opts.BlacklistIPs, net) } diff --git a/js/runner_test.go b/js/runner_test.go index 916fa840757..a19bab7f325 100644 --- a/js/runner_test.go +++ b/js/runner_test.go @@ -812,6 +812,7 @@ func TestVUIntegrationBlacklistScript(t *testing.T) { runners := map[string]*Runner{"Source": r1, "Archive": r2} for name, r := range runners { + r := r t.Run(name, func(t *testing.T) { vu, err := r.NewVU(make(chan stats.SampleContainer, 100)) if !assert.NoError(t, err) { diff --git a/lib/options.go b/lib/options.go index 8facde66732..df23259fdbc 100644 --- a/lib/options.go +++ b/lib/options.go @@ -211,7 +211,7 @@ type IPNet struct { } // UnmarshalJSON populates the IPNet from the given CIDR JSON -func (ipnet *IPNet) UnmarshalJSON(b []byte) (err error) { +func (ipnet *IPNet) UnmarshalJSON(b []byte) error { var cidr string if err := json.Unmarshal(b, &cidr); err != nil { return err diff --git a/lib/options_test.go b/lib/options_test.go index 4c7e222cd7e..d991aa799d0 100644 --- a/lib/options_test.go +++ b/lib/options_test.go @@ -519,24 +519,43 @@ func TestTagSetTextUnmarshal(t *testing.T) { func TestCIDRUnmarshal(t *testing.T) { var testData = []struct { - input string - expected *IPNet + input string + expectedOutput *IPNet + expactFailure bool }{ - {"10.0.0.0/8", &IPNet{IPNet: net.IPNet{ - IP: net.ParseIP("10.0.0.0"), - Mask: net.IPv4Mask(255, 0, 0, 0), - }}}, - {"fc00:1234:5678::/48", &IPNet{IPNet: net.IPNet{ - IP: net.ParseIP("fc00:1234:5678::"), - Mask: net.CIDRMask(48, 128), - }}}, + { + "10.0.0.0/8", + &IPNet{IPNet: net.IPNet{ + IP: net.ParseIP("10.0.0.0"), + Mask: net.IPv4Mask(255, 0, 0, 0), + }}, + false, + }, + { + "fc00:1234:5678::/48", + &IPNet{IPNet: net.IPNet{ + IP: net.ParseIP("fc00:1234:5678::"), + Mask: net.CIDRMask(48, 128), + }}, + false, + }, + {"10.0.0.0", nil, true}, + {"fc00:1234:5678::", nil, true}, + {"fc00::1234::/48", nil, true}, } for _, data := range testData { - actualIPNet := &IPNet{} - err := actualIPNet.UnmarshalJSON([]byte("\"" + data.input + "\"")) - require.NoError(t, err) - - assert.Equal(t, data.expected, actualIPNet) + data := data + t.Run(data.input, func(t *testing.T) { + actualIPNet := &IPNet{} + err := actualIPNet.UnmarshalJSON([]byte("\"" + data.input + "\"")) + + if data.expactFailure { + require.EqualError(t, err, "Failed to parse CIDR: invalid CIDR address: "+data.input) + } else { + require.NoError(t, err) + assert.Equal(t, data.expectedOutput, actualIPNet) + } + }) } } From a8f50f1298f045e815d0c506d7bc1965aa639c9a Mon Sep 17 00:00:00 2001 From: THoelzel <15081333+THoelzel@users.noreply.github.com> Date: Mon, 22 Apr 2019 17:58:58 -0400 Subject: [PATCH 3/4] Code cleanup --- lib/options.go | 9 ++------- lib/options_test.go | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/options.go b/lib/options.go index df23259fdbc..e56a266e0b2 100644 --- a/lib/options.go +++ b/lib/options.go @@ -229,17 +229,12 @@ func (ipnet *IPNet) UnmarshalJSON(b []byte) error { // ParseCIDR creates an IPNet out of a CIDR string func ParseCIDR(s string) (*IPNet, error) { - ip, ipnet, err := net.ParseCIDR(s) + _, ipnet, err := net.ParseCIDR(s) if err != nil { return nil, err } - return &IPNet{ - IPNet: net.IPNet{ - IP: ip, - Mask: ipnet.Mask, - }, - }, nil + return &IPNet{*ipnet}, nil } type Options struct { diff --git a/lib/options_test.go b/lib/options_test.go index d991aa799d0..29e7eb65397 100644 --- a/lib/options_test.go +++ b/lib/options_test.go @@ -526,7 +526,7 @@ func TestCIDRUnmarshal(t *testing.T) { { "10.0.0.0/8", &IPNet{IPNet: net.IPNet{ - IP: net.ParseIP("10.0.0.0"), + IP: net.IP{10, 0, 0, 0}, Mask: net.IPv4Mask(255, 0, 0, 0), }}, false, From 820ff7cf22e6325b0d8cf46c444e5b75f7529359 Mon Sep 17 00:00:00 2001 From: THoelzel <15081333+THoelzel@users.noreply.github.com> Date: Wed, 24 Apr 2019 20:47:36 -0400 Subject: [PATCH 4/4] Switch to UnmarshalText --- lib/netext/dialer.go | 6 +++--- lib/options.go | 21 ++++++++++----------- lib/options_test.go | 16 +++++++--------- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/netext/dialer.go b/lib/netext/dialer.go index 0954a863b7e..f95d91c099b 100644 --- a/lib/netext/dialer.go +++ b/lib/netext/dialer.go @@ -81,9 +81,9 @@ func (d *Dialer) DialContext(ctx context.Context, proto, addr string) (net.Conn, } } - for _, net := range d.Blacklist { - if net.Contains(ip) { - return nil, BlackListedIPError{ip: ip, net: net} + for _, ipnet := range d.Blacklist { + if (*net.IPNet)(ipnet).Contains(ip) { + return nil, BlackListedIPError{ip: ip, net: ipnet} } } ipStr := ip.String() diff --git a/lib/options.go b/lib/options.go index e56a266e0b2..4c743e2d4b9 100644 --- a/lib/options.go +++ b/lib/options.go @@ -206,18 +206,15 @@ func (c *TLSAuth) Certificate() (*tls.Certificate, error) { } // IPNet is a wrapper around net.IPNet for JSON unmarshalling -type IPNet struct { - net.IPNet -} +type IPNet net.IPNet -// UnmarshalJSON populates the IPNet from the given CIDR JSON -func (ipnet *IPNet) UnmarshalJSON(b []byte) error { - var cidr string - if err := json.Unmarshal(b, &cidr); err != nil { - return err - } +func (ipnet *IPNet) String() string { + return (*net.IPNet)(ipnet).String() +} - newIPNet, err := ParseCIDR(cidr) +// UnmarshalText populates the IPNet from the given CIDR +func (ipnet *IPNet) UnmarshalText(b []byte) error { + newIPNet, err := ParseCIDR(string(b)) if err != nil { return errors.Wrap(err, "Failed to parse CIDR") } @@ -234,7 +231,9 @@ func ParseCIDR(s string) (*IPNet, error) { return nil, err } - return &IPNet{*ipnet}, nil + parsedIPNet := IPNet(*ipnet) + + return &parsedIPNet, nil } type Options struct { diff --git a/lib/options_test.go b/lib/options_test.go index 29e7eb65397..0f601ad9ef4 100644 --- a/lib/options_test.go +++ b/lib/options_test.go @@ -309,10 +309,8 @@ func TestOptions(t *testing.T) { t.Run("BlacklistIPs", func(t *testing.T) { opts := Options{}.Apply(Options{ BlacklistIPs: []*IPNet{{ - net.IPNet{ - IP: net.IPv4zero, - Mask: net.CIDRMask(1, 1), - }, + IP: net.IPv4zero, + Mask: net.CIDRMask(1, 1), }}, }) assert.NotNil(t, opts.BlacklistIPs) @@ -525,18 +523,18 @@ func TestCIDRUnmarshal(t *testing.T) { }{ { "10.0.0.0/8", - &IPNet{IPNet: net.IPNet{ + &IPNet{ IP: net.IP{10, 0, 0, 0}, Mask: net.IPv4Mask(255, 0, 0, 0), - }}, + }, false, }, { "fc00:1234:5678::/48", - &IPNet{IPNet: net.IPNet{ + &IPNet{ IP: net.ParseIP("fc00:1234:5678::"), Mask: net.CIDRMask(48, 128), - }}, + }, false, }, {"10.0.0.0", nil, true}, @@ -548,7 +546,7 @@ func TestCIDRUnmarshal(t *testing.T) { data := data t.Run(data.input, func(t *testing.T) { actualIPNet := &IPNet{} - err := actualIPNet.UnmarshalJSON([]byte("\"" + data.input + "\"")) + err := actualIPNet.UnmarshalText([]byte(data.input)) if data.expactFailure { require.EqualError(t, err, "Failed to parse CIDR: invalid CIDR address: "+data.input)