From 103ab810659fc2e50e9b57dd3b13eb883828f4c0 Mon Sep 17 00:00:00 2001 From: Zachary Proser Date: Sun, 26 Jun 2022 12:20:26 -0400 Subject: [PATCH] Refactor tests to use mock requests, not servers --- .circleci/config.yml | 21 +++++ cli.go | 5 +- go.mod | 1 + go.sum | 2 + handler.go | 4 +- list.go | 4 +- procrastiproxy.go | 10 +- server.go | 5 +- server_test.go | 219 ++++++++++++++++++++++--------------------- util.go | 7 +- util_test.go | 8 +- 11 files changed, 158 insertions(+), 128 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 282d38b..434c9ce 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -10,6 +10,7 @@ workflows: - test - test-windows - test-mac + - test-mac-12-go-1-18 - release: # Only run this job on git tag pushes filters: @@ -60,6 +61,26 @@ jobs: command: | mkdir -p /tmp/logs go test -v ./... --timeout 5m | tee /tmp/logs/test.log + + + # Specifically test Go 1.18 on mac to catch syscall issues + test-mac-12-go-1-18: + macos: + xcode: 12.5.1 + steps: + - checkout + - run: + name: install go + command: | + brew install go@1.18 + # Make Go available in the PATH upon first being installed + echo 'export PATH="/usr/local/opt/go@1.18/bin:$PATH"' >> /Users/distiller/.bash_profile + - run: + name: run tests + command: | + mkdir -p /tmp/logs + go test -v ./... --timeout 5m | tee /tmp/logs/test.log + release: docker: - image: cimg/go:1.17 diff --git a/cli.go b/cli.go index ecb986d..2307176 100644 --- a/cli.go +++ b/cli.go @@ -43,7 +43,9 @@ func RunCLI() error { } log.SetLevel(level) - if parseErr := parseBlockListInput(blockList); parseErr != nil { + p := NewProcrastiproxy() + + if parseErr := parseBlockListInput(blockList, p.GetList()); parseErr != nil { return parseErr } @@ -51,7 +53,6 @@ func RunCLI() error { return parseErr } - p := NewProcrastiproxy() // Configure proxy time-based block settings p.ConfigureProxyTimeSettings(*blockStartTime, *blockEndTime) diff --git a/go.mod b/go.mod index 51144e9..1295666 100644 --- a/go.mod +++ b/go.mod @@ -5,4 +5,5 @@ go 1.14 require ( github.com/hashicorp/go-multierror v1.1.1 github.com/sirupsen/logrus v1.8.1 + golang.org/x/sys v0.0.0-20220624220833-87e55d714810 // indirect ) diff --git a/go.sum b/go.sum index 8566648..55c6a3c 100644 --- a/go.sum +++ b/go.sum @@ -12,3 +12,5 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037 h1:YyJpGZS1sBuBCzLAR1VEpK193GlqGZbnPFnPV/5Rsb4= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20220624220833-87e55d714810 h1:rHZQSjJdAI4Xf5Qzeh2bBc5YJIkPFVM6oDtMFYmgws0= +golang.org/x/sys v0.0.0-20220624220833-87e55d714810/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/handler.go b/handler.go index 1a3459d..1fefa0c 100644 --- a/handler.go +++ b/handler.go @@ -77,7 +77,7 @@ func (p *Procrastiproxy) timeAwareHandler(w http.ResponseWriter, r *http.Request func (p *Procrastiproxy) blockListAwareHandler(w http.ResponseWriter, r *http.Request) { host := sanitizeHost(r.URL.Host) - if hostIsBlocked(host) { + if hostIsOnBlockList(host, p.GetList()) { log.Debugf("Blocking request to host: %s. User explicitly blocked and present time is within configured proxy block window", host) blockRequest(w) return @@ -94,7 +94,7 @@ func (p *Procrastiproxy) adminHandler(w http.ResponseWriter, r *http.Request) { log.Println(err) } var respMsg string - list := GetList() + list := p.GetList() if adminCmd.Command == "block" { list.Add(adminCmd.Host) respMsg = fmt.Sprintf("Successfully added: %s to the block list\n", adminCmd.Host) diff --git a/list.go b/list.go index 8caee3f..8d19da1 100644 --- a/list.go +++ b/list.go @@ -4,7 +4,7 @@ import ( "sync" ) -var list = newList() +var list = NewList() type List struct { m sync.Mutex @@ -16,7 +16,7 @@ func GetList() *List { return list } -func newList() *List { +func NewList() *List { return &List{ members: make(map[string]bool), } diff --git a/procrastiproxy.go b/procrastiproxy.go index 7b10f70..7a1b5c7 100644 --- a/procrastiproxy.go +++ b/procrastiproxy.go @@ -14,7 +14,8 @@ var procrastiproxy *Procrastiproxy var DefaultNow = time.Now type Procrastiproxy struct { - Now func() time.Time + Now func() time.Time + List *List ProxyTimeSettings } @@ -23,11 +24,16 @@ func NewProcrastiproxy() *Procrastiproxy { return procrastiproxy } procrastiproxy = &Procrastiproxy{ - Now: DefaultNow, + Now: DefaultNow, + List: NewList(), } return procrastiproxy } +func (p *Procrastiproxy) GetList() *List { + return p.List +} + // custom errors type EmptyBlockListError struct{} diff --git a/server.go b/server.go index 16be9b9..78947d0 100644 --- a/server.go +++ b/server.go @@ -13,10 +13,9 @@ func sanitizeHost(host string) string { return strings.ToLower(strings.TrimSpace(strings.Replace(host, "\n", "", -1))) } -func hostIsBlocked(host string) bool { +func hostIsOnBlockList(host string, list *List) bool { host = sanitizeHost(host) - blockList := GetList() - return blockList.Contains(host) + return list.Contains(host) } func RunServer(args []string) { diff --git a/server_test.go b/server_test.go index 8ace266..b04f2a7 100644 --- a/server_test.go +++ b/server_test.go @@ -4,169 +4,172 @@ import ( "fmt" "net/http" "net/http/httptest" - "net/url" + "strings" "testing" "github.com/stretchr/testify/require" ) -func setupTestServer(t *testing.T, handlerFunc func(http.ResponseWriter, *http.Request)) (*http.Client, *httptest.Server, error) { - // Create a test backend that wraps our blockListAwareHandler. This test backend - // can then be sent various HTTP requests in test cases - backend := httptest.NewServer(http.HandlerFunc(handlerFunc)) +// TestHostBlocking ensures that you can access a host via the block list aware handler before it is added to the block list, +// but not after it is added to the block list +func TestHostBlocking(t *testing.T) { + t.Parallel() - proxyURL, err := url.Parse(backend.URL) - if err != nil { - t.Fatal(err) - } + p := NewProcrastiproxy() - // Create a client that will use our procrastiproxy as a proxy, so that we can - // test our proxy's functionality - client := &http.Client{ - Transport: &http.Transport{Proxy: http.ProxyURL(proxyURL)}, - } + blockedHost := "reddit.com" + testURL := "http://reddit.com" - return client, backend, nil + // First, ensure that the target host can be reached before it is added to the block list + fr := httptest.NewRequest("GET", testURL, strings.NewReader("")) -} + fw := httptest.NewRecorder() -func setupBlockListAwareServer(t *testing.T) (*http.Client, *httptest.Server, error) { - return setupTestServer(t, NewProcrastiproxy().blockListAwareHandler) -} + http.HandlerFunc(p.blockListAwareHandler).ServeHTTP(fw, fr) -func setupProxyTestServer(t *testing.T) (*http.Client, *httptest.Server, error) { - return setupTestServer(t, proxyHandler) -} + if fw.Code != http.StatusOK { + t.Logf("Wanted HTTP StatusCode: %d for URL prior to adding host to block list: %s but got: %d\n", http.StatusOK, testURL, fw.Code) + } -func setupAdminTestServer(t *testing.T) (*http.Client, *httptest.Server, error) { - return setupTestServer(t, NewProcrastiproxy().adminHandler) -} + AddHostToBlockList(p.GetList(), blockedHost) -// TestDeniedBlockHosts adds a host to the block list and then immediately attempts to make -// a request to that host, which should be denied by procrastiproxy -func TestDeniesBlockedHost(t *testing.T) { - client, backend, err := setupBlockListAwareServer(t) - defer backend.Close() - if err != nil { - t.Fatal(err) - } + // Next, ensure the same host cannot be accessed after being added to the block list - blockedHost := "reddit.com" - testURL := "http://reddit.com" + r := httptest.NewRequest("GET", testURL, strings.NewReader("")) - AddHostToBlockList(blockedHost) + w := httptest.NewRecorder() - res, err := client.Get(testURL) - if err != nil { - t.Fatalf("Error attempting to fetch test server URL: %v\n", err) - } - if res.StatusCode != http.StatusForbidden { - t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusForbidden, testURL, res.StatusCode) + http.HandlerFunc(p.blockListAwareHandler).ServeHTTP(w, r) + + if w.Code != http.StatusForbidden { + t.Logf("Wanted HTTP StatusCode: %d for URL after adding host to block list: %s but got: %d\n", http.StatusForbidden, testURL, w.Code) t.Fail() } } -// TestAllowsWhitelistedHost ensures that a host that has not been explicitly blocked -// can be reached through procrastiproxy -func TestAllowsWhitelistedHost(t *testing.T) { +// TestProxiedHost ensures that you can use the proxyHandler to do "pass-through" network requests +func TestProxiedHost(t *testing.T) { + t.Parallel() - client, backend, err := setupBlockListAwareServer(t) - defer backend.Close() - if err != nil { - t.Fatal(err) - } + testURL := "http://reddit.com" - blockedHost := "nytimes.com" - testURL := "http://wikipedia.org" + // First, ensure that the target host can be reached before it is added to the block list + r := httptest.NewRequest("GET", testURL, strings.NewReader("")) - // Set the blocked hosts config variable that is used by the proxy backend - AddHostToBlockList(blockedHost) + w := httptest.NewRecorder() - res, err := client.Get(testURL) - if err != nil { - t.Fatalf("Error attempting to fetch test server URL: %v\n", err) - } - if res.StatusCode != http.StatusOK { - t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testURL, res.StatusCode) + http.HandlerFunc(proxyHandler).ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testURL, w.Code) t.Fail() } } -func TestProxiedHost(t *testing.T) { +// TestAdminHandlerBlocksHostsDynamicallt ensures that you can dynamically add a new host to the block list +// via the admin/block endpoint, that will be respected for all subsequent requests +func TestAdminHandlerBlocksHostsDynamically(t *testing.T) { + t.Parallel() - client, backend, err := setupProxyTestServer(t) - defer backend.Close() - if err != nil { - t.Fatal(err) - } + p := NewProcrastiproxy() - testURL := "http://reddit.com" + // Sanity check the initial block list is empty + require.Equal(t, p.GetList().Length(), 0) - res, err := client.Get(testURL) - if err != nil { - t.Fatalf("Error attempting to fetch proxy test server URL: %v\n", err) - } - if res.StatusCode != http.StatusOK { - t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testURL, res.StatusCode) - t.Fail() - } -} + testHost := "docker.com" -func TestAdminHandlerBlocksHostsDynamically(t *testing.T) { + // Sanity check that we can initially reach the target host because it has not yet been blocked + testHostURL := "http://docker.com" + fr := httptest.NewRequest("GET", testHostURL, strings.NewReader("")) + rw := httptest.NewRecorder() + + http.HandlerFunc(p.blockListAwareHandler).ServeHTTP(rw, fr) - client, backend, err := setupAdminTestServer(t) - defer backend.Close() - if err != nil { - t.Fatal(err) + if rw.Code != http.StatusOK { + t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testHostURL, rw.Code) + t.Fail() } - testHost := "docker.com" + // Now, dynamically block the same target host by making a request to the admin block endpoint, passing the host info testURL := fmt.Sprintf("http://localhost:8000/admin/block/%s", testHost) - res, err := client.Get(testURL) - if err != nil { - t.Fatalf("Error attempting to fetch admin test server URL: %v\n", err) - } - if res.StatusCode != http.StatusOK { - t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testURL, res.StatusCode) + r := httptest.NewRequest("GET", testURL, strings.NewReader("")) + w := httptest.NewRecorder() + + http.HandlerFunc(p.adminHandler).ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testURL, w.Code) t.Fail() } // Finally, ensure the host we just dynamically added to the block list is found - l := GetList() + require.True(t, p.GetList().Contains(testHost)) + require.Equal(t, p.GetList().Length(), 1) + + // Ensure that, attempting to hit the same host now fails because it is blocked at the proxy level + ar := httptest.NewRequest("GET", testHostURL, strings.NewReader("")) + aw := httptest.NewRecorder() - require.True(t, l.Contains(testHost)) + http.HandlerFunc(p.blockListAwareHandler).ServeHTTP(aw, ar) - l.Clear() + if aw.Code != http.StatusForbidden { + t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusForbidden, testHostURL, aw.Code) + t.Fail() + } } +// TestAdminHandlerUnblocksHostsDynamically ensures that a blocked host can be removed from the block list via the admin/unblock endpoint func TestAdminHandlerUnblocksHostsDynamically(t *testing.T) { + t.Parallel() - client, backend, err := setupAdminTestServer(t) - defer backend.Close() - if err != nil { - t.Fatal(err) - } + p := NewProcrastiproxy() - // Start off by pre-populating the list with the test host testHost := "wikipedia.com" - l := GetList() - l.Add(testHost) + testHostURL := fmt.Sprintf("http://%s", testHost) - testURL := fmt.Sprintf("http://localhost:8000/admin/unblock/%s", testHost) + // Pre-populate the list with the test host + p.GetList().Add(testHost) - res, err := client.Get(testURL) - if err != nil { - t.Fatalf("Error attempting to fetch admin test server URL: %v\n", err) + require.True(t, p.GetList().Contains(testHost)) + + // Sanity check that you cannot get the blocked test host to begin with + fr := httptest.NewRequest("GET", testHostURL, strings.NewReader("")) + rw := httptest.NewRecorder() + + http.HandlerFunc(p.blockListAwareHandler).ServeHTTP(rw, fr) + + if rw.Code != http.StatusForbidden { + t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusForbidden, testHostURL, rw.Code) + t.Fail() } - if res.StatusCode != http.StatusOK { - t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testURL, res.StatusCode) + + // Now, dynamically unblock the same test host by making a request to the admin unblock endpoint + testAdminURL := fmt.Sprintf("http://localhost:8000/admin/unblock/%s", testHost) + + r := httptest.NewRequest("GET", testAdminURL, strings.NewReader("")) + w := httptest.NewRecorder() + + http.HandlerFunc(p.adminHandler).ServeHTTP(w, r) + + if w.Code != http.StatusOK { + t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testAdminURL, w.Code) t.Fail() } - // Finally, ensure the host we just dynamically added to the block list is found - require.False(t, l.Contains(testHost)) + // Ensure the host we just dynamically removed from the block list is no longer found on it + require.False(t, p.GetList().Contains(testHost)) + require.Equal(t, p.GetList().Length(), 0) + + // Ensure that we can access the test host now that it has been unblocked via the admin endpoint + ar := httptest.NewRequest("GET", testHostURL, strings.NewReader("")) + aw := httptest.NewRecorder() - l.Clear() + http.HandlerFunc(p.blockListAwareHandler).ServeHTTP(aw, ar) + + if aw.Code != http.StatusOK { + t.Logf("Wanted HTTP StatusCode: %d for URL: %s but got: %d\n", http.StatusOK, testHostURL, aw.Code) + t.Fail() + } } diff --git a/util.go b/util.go index dc17be3..ffa2231 100644 --- a/util.go +++ b/util.go @@ -15,7 +15,7 @@ func validateBlockListInput(blockListMembers []string) error { return nil } -func parseBlockListInput(blockList *string) error { +func parseBlockListInput(blockList *string, list *List) error { var blockListMembers []string var blockListString = *blockList if blockListString != "" { @@ -28,7 +28,7 @@ func parseBlockListInput(blockList *string) error { } for _, host := range blockListMembers { - AddHostToBlockList(host) + AddHostToBlockList(list, host) } return nil @@ -63,8 +63,7 @@ func parseStartAndEndTimes(blockTimeStart, blockTimeEnd string) error { } // Build the fast, in-memory list of blocked hosts from the configured values -func AddHostToBlockList(hosts ...string) { - list := GetList() +func AddHostToBlockList(list *List, hosts ...string) { for _, host := range hosts { list.Add(host) } diff --git a/util_test.go b/util_test.go index 646e67e..8cf6ba6 100644 --- a/util_test.go +++ b/util_test.go @@ -45,19 +45,17 @@ func TestParseBlockListInput(t *testing.T) { for _, tc := range testCases { - l := GetList() - l.Clear() + l := NewList() t.Run(tc.Name, func(t *testing.T) { - err := parseBlockListInput(&tc.InputString) + err := parseBlockListInput(&tc.InputString, l) require.NoError(t, err) // Ensure list has expected members following parsing - l := GetList() for _, member := range tc.Want { require.True(t, l.Contains(member)) //Also sanity check that hostIsBlocked returns true - require.True(t, hostIsBlocked(member)) + require.True(t, hostIsOnBlockList(member, l)) } })