From 88739ac2b6d37a4066c76702dc23d6038f7b7706 Mon Sep 17 00:00:00 2001 From: PJ Date: Tue, 19 Mar 2024 21:28:33 +0100 Subject: [PATCH] bus: deprecate /hosts endpoint infavour of /search/hosts --- api/host.go | 20 +---- autopilot/autopilot.go | 3 +- autopilot/contractor.go | 2 +- autopilot/scanner.go | 2 +- autopilot/scanner_test.go | 4 +- bus/bus.go | 11 ++- bus/client/hosts.go | 12 ++- internal/test/e2e/blocklist_test.go | 8 +- internal/test/e2e/cluster_test.go | 2 +- internal/test/e2e/pruning_test.go | 4 +- stores/hostdb.go | 5 -- stores/hostdb_test.go | 125 +++++++++++++++------------- 12 files changed, 97 insertions(+), 101 deletions(-) diff --git a/api/host.go b/api/host.go index ba66ffd58d..74a9c05405 100644 --- a/api/host.go +++ b/api/host.go @@ -70,16 +70,12 @@ type ( // Option types. type ( - HostsOptions struct { - Offset int - Limit int - FilterMode string - } HostsForScanningOptions struct { MaxLastScan TimeRFC3339 Limit int Offset int } + SearchHostOptions struct { AddressContains string FilterMode string @@ -92,19 +88,7 @@ type ( func DefaultSearchHostOptions() SearchHostOptions { return SearchHostOptions{ Limit: -1, - FilterMode: HostFilterModeAll, - } -} - -func (opts HostsOptions) Apply(values url.Values) { - if opts.Offset != 0 { - values.Set("offset", fmt.Sprint(opts.Offset)) - } - if opts.Limit != 0 { - values.Set("limit", fmt.Sprint(opts.Limit)) - } - if opts.FilterMode != "" { - values.Set("filterMode", opts.FilterMode) + FilterMode: HostFilterModeAllowed, } } diff --git a/autopilot/autopilot.go b/autopilot/autopilot.go index d8a7602654..3b90a83296 100644 --- a/autopilot/autopilot.go +++ b/autopilot/autopilot.go @@ -54,7 +54,6 @@ type Bus interface { // hostdb Host(ctx context.Context, hostKey types.PublicKey) (hostdb.HostInfo, error) - Hosts(ctx context.Context, opts api.HostsOptions) ([]hostdb.HostInfo, error) HostsForScanning(ctx context.Context, opts api.HostsForScanningOptions) ([]hostdb.HostAddress, error) RemoveOfflineHosts(ctx context.Context, minRecentScanFailures uint64, maxDowntime time.Duration) (uint64, error) SearchHosts(ctx context.Context, opts api.SearchHostOptions) ([]hostdb.HostInfo, error) @@ -196,7 +195,7 @@ func (ap *Autopilot) configHandlerPOST(jc jape.Context) { state := ap.State() // fetch hosts - hosts, err := ap.bus.Hosts(ctx, api.HostsOptions{}) + hosts, err := ap.bus.SearchHosts(ctx, api.DefaultSearchHostOptions()) if jc.Check("failed to get hosts", err) != nil { return } diff --git a/autopilot/contractor.go b/autopilot/contractor.go index 47a03480f7..ef64f630b9 100644 --- a/autopilot/contractor.go +++ b/autopilot/contractor.go @@ -249,7 +249,7 @@ func (c *contractor) performContractMaintenance(ctx context.Context, w Worker) ( } // fetch all hosts - hosts, err := c.ap.bus.Hosts(ctx, api.HostsOptions{}) + hosts, err := c.ap.bus.SearchHosts(ctx, api.DefaultSearchHostOptions()) if err != nil { return false, err } diff --git a/autopilot/scanner.go b/autopilot/scanner.go index 85301822c6..76643e5b57 100644 --- a/autopilot/scanner.go +++ b/autopilot/scanner.go @@ -31,7 +31,7 @@ type ( // a bit, we currently use inline interfaces to avoid having to update the // scanner tests with every interface change bus interface { - Hosts(ctx context.Context, opts api.HostsOptions) ([]hostdb.HostInfo, error) + SearchHosts(ctx context.Context, opts api.SearchHostOptions) ([]hostdb.HostInfo, error) HostsForScanning(ctx context.Context, opts api.HostsForScanningOptions) ([]hostdb.HostAddress, error) RemoveOfflineHosts(ctx context.Context, minRecentScanFailures uint64, maxDowntime time.Duration) (uint64, error) } diff --git a/autopilot/scanner_test.go b/autopilot/scanner_test.go index 481b78046d..1cdd096d23 100644 --- a/autopilot/scanner_test.go +++ b/autopilot/scanner_test.go @@ -19,7 +19,7 @@ type mockBus struct { reqs []string } -func (b *mockBus) Hosts(ctx context.Context, opts api.HostsOptions) ([]hostdb.HostInfo, error) { +func (b *mockBus) SearchHosts(ctx context.Context, opts api.SearchHostOptions) ([]hostdb.HostInfo, error) { b.reqs = append(b.reqs, fmt.Sprintf("%d-%d", opts.Offset, opts.Offset+opts.Limit)) start := opts.Offset @@ -40,7 +40,7 @@ func (b *mockBus) Hosts(ctx context.Context, opts api.HostsOptions) ([]hostdb.Ho } func (b *mockBus) HostsForScanning(ctx context.Context, opts api.HostsForScanningOptions) ([]hostdb.HostAddress, error) { - hosts, err := b.Hosts(ctx, api.HostsOptions{ + hosts, err := b.SearchHosts(ctx, api.SearchHostOptions{ Offset: opts.Offset, Limit: opts.Limit, }) diff --git a/bus/bus.go b/bus/bus.go index 3838a18777..5fae2687ea 100644 --- a/bus/bus.go +++ b/bus/bus.go @@ -92,7 +92,6 @@ type ( // A HostDB stores information about hosts. HostDB interface { Host(ctx context.Context, hostKey types.PublicKey) (hostdb.HostInfo, error) - Hosts(ctx context.Context, filterMode string, offset, limit int) ([]hostdb.HostInfo, error) HostsForScanning(ctx context.Context, maxLastScan time.Time, offset, limit int) ([]hostdb.HostAddress, error) RecordHostScans(ctx context.Context, scans []hostdb.HostScan) error RecordPriceTables(ctx context.Context, priceTableUpdate []hostdb.PriceTableUpdate) error @@ -285,7 +284,7 @@ func (b *bus) Handler() http.Handler { "GET /contract/:id/roots": b.contractIDRootsHandlerGET, "GET /contract/:id/size": b.contractSizeHandlerGET, - "GET /hosts": b.hostsHandlerGET, + "GET /hosts": b.hostsHandlerGETDeprecated, "GET /hosts/allowlist": b.hostsAllowlistHandlerGET, "PUT /hosts/allowlist": b.hostsAllowlistHandlerPUT, "GET /hosts/blocklist": b.hostsBlocklistHandlerGET, @@ -755,7 +754,7 @@ func (b *bus) walletPendingHandler(jc jape.Context) { jc.Encode(relevant) } -func (b *bus) hostsHandlerGET(jc jape.Context) { +func (b *bus) hostsHandlerGETDeprecated(jc jape.Context) { offset := 0 limit := -1 filterMode := api.HostFilterModeAllowed @@ -774,7 +773,7 @@ func (b *bus) hostsHandlerGET(jc jape.Context) { } // fetch hosts - hosts, err := b.hdb.Hosts(jc.Request.Context(), filterMode, offset, limit) + hosts, err := b.hdb.SearchHosts(jc.Request.Context(), filterMode, "", nil, offset, limit) if jc.Check(fmt.Sprintf("couldn't fetch hosts %d-%d", offset, offset+limit), err) != nil { return } @@ -786,6 +785,10 @@ func (b *bus) searchHostsHandlerPOST(jc jape.Context) { if jc.Decode(&req) != nil { return } + + // TODO: on the next major release we should: + // - remove api.DefaultSearchHostOptions and set defaults in the handler + // - validate the filter mode here and return a 400 hosts, err := b.hdb.SearchHosts(jc.Request.Context(), req.FilterMode, req.AddressContains, req.KeyIn, req.Offset, req.Limit) if jc.Check(fmt.Sprintf("couldn't fetch hosts %d-%d", req.Offset, req.Offset+req.Limit), err) != nil { return diff --git a/bus/client/hosts.go b/bus/client/hosts.go index 70c8b3431d..f48bc171ed 100644 --- a/bus/client/hosts.go +++ b/bus/client/hosts.go @@ -30,9 +30,17 @@ func (c *Client) HostBlocklist(ctx context.Context) (blocklist []string, err err } // Hosts returns 'limit' hosts at given 'offset'. -func (c *Client) Hosts(ctx context.Context, opts api.HostsOptions) (hosts []hostdb.HostInfo, err error) { +func (c *Client) Hosts(ctx context.Context, opts struct { + Offset int + Limit int +}) (hosts []hostdb.HostInfo, err error) { values := url.Values{} - opts.Apply(values) + if opts.Offset != 0 { + values.Set("offset", fmt.Sprint(opts.Offset)) + } + if opts.Limit != 0 { + values.Set("limit", fmt.Sprint(opts.Limit)) + } err = c.c.WithContext(ctx).GET("/hosts?"+values.Encode(), &hosts) return } diff --git a/internal/test/e2e/blocklist_test.go b/internal/test/e2e/blocklist_test.go index e371f01d48..06f7e133d6 100644 --- a/internal/test/e2e/blocklist_test.go +++ b/internal/test/e2e/blocklist_test.go @@ -23,6 +23,8 @@ func TestBlocklist(t *testing.T) { hosts: 3, }) defer cluster.Shutdown() + + // convenience variables b := cluster.Bus tt := cluster.tt @@ -117,7 +119,7 @@ func TestBlocklist(t *testing.T) { } // assert we have 4 hosts - hosts, err := b.Hosts(context.Background(), api.HostsOptions{}) + hosts, err := b.SearchHosts(context.Background(), api.DefaultSearchHostOptions()) tt.OK(err) if len(hosts) != 4 { t.Fatal("unexpected number of hosts", len(hosts)) @@ -142,7 +144,7 @@ func TestBlocklist(t *testing.T) { } // assert all others are blocked - hosts, err = b.Hosts(context.Background(), api.HostsOptions{}) + hosts, err = b.SearchHosts(context.Background(), api.DefaultSearchHostOptions()) tt.OK(err) if len(hosts) != 1 { t.Fatal("unexpected number of hosts", len(hosts)) @@ -152,7 +154,7 @@ func TestBlocklist(t *testing.T) { tt.OK(b.UpdateHostAllowlist(context.Background(), nil, nil, true)) // assert no hosts are blocked - hosts, err = b.Hosts(context.Background(), api.HostsOptions{}) + hosts, err = b.SearchHosts(context.Background(), api.DefaultSearchHostOptions()) tt.OK(err) if len(hosts) != 5 { t.Fatal("unexpected number of hosts", len(hosts)) diff --git a/internal/test/e2e/cluster_test.go b/internal/test/e2e/cluster_test.go index 65febbbf7a..69ac903910 100644 --- a/internal/test/e2e/cluster_test.go +++ b/internal/test/e2e/cluster_test.go @@ -146,7 +146,7 @@ func TestNewTestCluster(t *testing.T) { }) // Get host info for every host. - hosts, err := cluster.Bus.Hosts(context.Background(), api.HostsOptions{}) + hosts, err := cluster.Bus.SearchHosts(context.Background(), api.DefaultSearchHostOptions()) tt.OK(err) for _, host := range hosts { hi, err := cluster.Autopilot.HostInfo(host.PublicKey) diff --git a/internal/test/e2e/pruning_test.go b/internal/test/e2e/pruning_test.go index b5f6cccd08..0601521675 100644 --- a/internal/test/e2e/pruning_test.go +++ b/internal/test/e2e/pruning_test.go @@ -84,7 +84,7 @@ func TestHostPruning(t *testing.T) { } // assert the host was not pruned - hostss, err := b.Hosts(context.Background(), api.HostsOptions{}) + hostss, err := b.SearchHosts(context.Background(), api.DefaultSearchHostOptions()) tt.OK(err) if len(hostss) != 1 { t.Fatal("host was pruned") @@ -96,7 +96,7 @@ func TestHostPruning(t *testing.T) { // assert the host was pruned tt.Retry(10, time.Second, func() error { - hostss, err = b.Hosts(context.Background(), api.HostsOptions{}) + hostss, err = b.SearchHosts(context.Background(), api.DefaultSearchHostOptions()) tt.OK(err) if len(hostss) != 0 { return fmt.Errorf("host was not pruned, %+v", hostss[0].Interactions) diff --git a/stores/hostdb.go b/stores/hostdb.go index 101ee298d5..0a6fb00f6e 100644 --- a/stores/hostdb.go +++ b/stores/hostdb.go @@ -530,11 +530,6 @@ func (ss *SQLStore) SearchHosts(ctx context.Context, filterMode, addressContains return hosts, err } -// Hosts returns non-blocked hosts at given offset and limit. -func (ss *SQLStore) Hosts(ctx context.Context, filterMode string, offset, limit int) ([]hostdb.HostInfo, error) { - return ss.SearchHosts(ctx, filterMode, "", nil, offset, limit) -} - func (ss *SQLStore) RemoveOfflineHosts(ctx context.Context, minRecentFailures uint64, maxDowntime time.Duration) (removed uint64, err error) { // sanity check 'maxDowntime' if maxDowntime < 0 { diff --git a/stores/hostdb_test.go b/stores/hostdb_test.go index 5287005029..735ea41908 100644 --- a/stores/hostdb_test.go +++ b/stores/hostdb_test.go @@ -53,7 +53,7 @@ func TestSQLHostDB(t *testing.T) { } // Assert it's returned - allHosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 0, -1) + allHosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 0, -1) if err != nil { t.Fatal(err) } @@ -158,39 +158,65 @@ func (s *SQLStore) addTestScan(hk types.PublicKey, t time.Time, err error, setti }) } -// TestSQLHosts tests the Hosts method of the SQLHostDB type. -func TestSQLHosts(t *testing.T) { +// TestSearchHosts is a unit tests for the SearchHosts method. +func TestSearchHosts(t *testing.T) { ss := newTestSQLStore(t, defaultTestSQLStoreConfig) defer ss.Close() ctx := context.Background() - hks, err := ss.addTestHosts(3) - if err != nil { - t.Fatal(err) + // add 3 hosts + var hks []types.PublicKey + for i := 1; i <= 3; i++ { + if err := ss.addCustomTestHost(types.PublicKey{byte(i)}, fmt.Sprintf("-%v-", i)); err != nil { + t.Fatal(err) + } + hks = append(hks, types.PublicKey{byte(i)}) } hk1, hk2, hk3 := hks[0], hks[1], hks[2] - // assert the hosts method returns the expected hosts - if hosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 0, -1); err != nil || len(hosts) != 3 { + // assert defaults return all hosts + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 0, -1); err != nil || len(hosts) != 3 { t.Fatal("unexpected", len(hosts), err) } - if hosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 0, 1); err != nil || len(hosts) != 1 { + + // assert we can search using offset and limit + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 0, 1); err != nil || len(hosts) != 1 { t.Fatal("unexpected", len(hosts), err) } else if host := hosts[0]; host.PublicKey != hk1 { t.Fatal("unexpected host", hk1, hk2, hk3, host.PublicKey) } - if hosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 1, 1); err != nil || len(hosts) != 1 { + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 1, 1); err != nil || len(hosts) != 1 { t.Fatal("unexpected", len(hosts), err) } else if host := hosts[0]; host.PublicKey != hk2 { t.Fatal("unexpected host", hk1, hk2, hk3, host.PublicKey) } - if hosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 3, 1); err != nil || len(hosts) != 0 { + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 3, 1); err != nil || len(hosts) != 0 { t.Fatal("unexpected", len(hosts), err) } - if _, err := ss.Hosts(ctx, api.HostFilterModeAllowed, -1, -1); err != ErrNegativeOffset { + if _, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, -1, -1); err != ErrNegativeOffset { t.Fatal("unexpected error", err) } + // assert we can search by address + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "1", nil, 0, -1); err != nil || len(hosts) != 1 { + t.Fatal("unexpected", len(hosts), err) + } + + // assert we can search by key + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "", []types.PublicKey{hk1, hk2}, 0, -1); err != nil || len(hosts) != 2 { + t.Fatal("unexpected", len(hosts), err) + } + + // assert we can search by address and key + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "1", []types.PublicKey{hk1, hk2}, 0, -1); err != nil || len(hosts) != 1 { + t.Fatal("unexpected", len(hosts), err) + } + + // assert we can search by key and limit + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "3", []types.PublicKey{hk3}, 0, -1); err != nil || len(hosts) != 1 { + t.Fatal("unexpected", len(hosts), err) + } + // add a custom host and block it hk4 := types.PublicKey{4} if err := ss.addCustomTestHost(hk4, "host4.com"); err != nil { @@ -201,13 +227,27 @@ func TestSQLHosts(t *testing.T) { } // assert host filter mode is applied - if hosts, err := ss.Hosts(ctx, api.HostFilterModeAll, 0, -1); err != nil || len(hosts) != 4 { + if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "", nil, 0, -1); err != nil || len(hosts) != 4 { t.Fatal("unexpected", len(hosts), err) - } else if hosts, err := ss.Hosts(ctx, api.HostFilterModeBlocked, 0, -1); err != nil || len(hosts) != 1 { + } else if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeBlocked, "", nil, 0, -1); err != nil || len(hosts) != 1 { t.Fatal("unexpected", len(hosts), err) - } else if hosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 0, -1); err != nil || len(hosts) != 3 { + } else if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 0, -1); err != nil || len(hosts) != 3 { t.Fatal("unexpected", len(hosts), err) } +} + +// TestHostsForScanning is a unit test for the HostsForScanning method. +func TestHostsForScanning(t *testing.T) { + ss := newTestSQLStore(t, defaultTestSQLStoreConfig) + defer ss.Close() + ctx := context.Background() + + // add 3 hosts + hks, err := ss.addTestHosts(3) + if err != nil { + t.Fatal(err) + } + hk1, hk2, hk3 := hks[0], hks[1], hks[2] // add a scan for every non-blocked host n := time.Now() @@ -222,20 +262,19 @@ func TestSQLHosts(t *testing.T) { } // fetch all hosts using the HostsForScanning method - hostAddresses, err := ss.HostsForScanning(ctx, n, 0, 4) + hostAddresses, err := ss.HostsForScanning(ctx, n, 0, -1) if err != nil { t.Fatal(err) - } else if len(hostAddresses) != 4 { + } else if len(hostAddresses) != 3 { t.Fatal("wrong number of addresses") - } else if hostAddresses[0].PublicKey != hk4 || - hostAddresses[1].PublicKey != hk3 || - hostAddresses[2].PublicKey != hk2 || - hostAddresses[3].PublicKey != hk1 { + } else if hostAddresses[0].PublicKey != hk3 || + hostAddresses[1].PublicKey != hk2 || + hostAddresses[2].PublicKey != hk1 { t.Fatal("wrong key") } - // fetch one host by setting the cutoff exactly to hk3 - hostAddresses, err = ss.HostsForScanning(ctx, n.Add(-3*time.Minute), 0, -1) + // fetch one host by setting the cutoff exactly to hk2 + hostAddresses, err = ss.HostsForScanning(ctx, n.Add(-2*time.Minute), 0, -1) if err != nil { t.Fatal(err) } else if len(hostAddresses) != 1 { @@ -243,7 +282,7 @@ func TestSQLHosts(t *testing.T) { } // fetch no hosts - hostAddresses, err = ss.HostsForScanning(ctx, time.Time{}, 0, 3) + hostAddresses, err = ss.HostsForScanning(ctx, time.Time{}, 0, -1) if err != nil { t.Fatal(err) } else if len(hostAddresses) != 0 { @@ -251,40 +290,6 @@ func TestSQLHosts(t *testing.T) { } } -// TestSearchHosts is a unit test for SearchHosts. -func TestSearchHosts(t *testing.T) { - ss := newTestSQLStore(t, defaultTestSQLStoreConfig) - defer ss.Close() - ctx := context.Background() - - // add 3 hosts - var hks []types.PublicKey - for i := 0; i < 3; i++ { - if err := ss.addCustomTestHost(types.PublicKey{byte(i)}, fmt.Sprintf("-%v-", i+1)); err != nil { - t.Fatal(err) - } - hks = append(hks, types.PublicKey{byte(i)}) - } - hk1, hk2, hk3 := hks[0], hks[1], hks[2] - - // Search by address. - if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "1", nil, 0, -1); err != nil || len(hosts) != 1 { - t.Fatal("unexpected", len(hosts), err) - } - // Filter by key. - if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "", []types.PublicKey{hk1, hk2}, 0, -1); err != nil || len(hosts) != 2 { - t.Fatal("unexpected", len(hosts), err) - } - // Filter by address and key. - if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "1", []types.PublicKey{hk1, hk2}, 0, -1); err != nil || len(hosts) != 1 { - t.Fatal("unexpected", len(hosts), err) - } - // Filter by key and limit results - if hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAll, "3", []types.PublicKey{hk3}, 0, -1); err != nil || len(hosts) != 1 { - t.Fatal("unexpected", len(hosts), err) - } -} - // TestRecordScan is a test for recording scans. func TestRecordScan(t *testing.T) { ss := newTestSQLStore(t, defaultTestSQLStoreConfig) @@ -606,7 +611,7 @@ func TestSQLHostAllowlist(t *testing.T) { numHosts := func() int { t.Helper() - hosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 0, -1) + hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 0, -1) if err != nil { t.Fatal(err) } @@ -778,7 +783,7 @@ func TestSQLHostBlocklist(t *testing.T) { numHosts := func() int { t.Helper() - hosts, err := ss.Hosts(ctx, api.HostFilterModeAllowed, 0, -1) + hosts, err := ss.SearchHosts(ctx, api.HostFilterModeAllowed, "", nil, 0, -1) if err != nil { t.Fatal(err) }