Ensure that manually attached hosts get packs (#922)

Previously, when determining which packs a host should get when it checked in, we were iterating each pack and only checking whether or not the host was apart of a label which was a target of the pack, but we were never checking whether or not the host had been added as a specific target of that pack. This PR makes the necessary modification to `svc.ListPacksForHost`.

server/datastore/datastore_packs_test.go

@@ -65,15 +65,7 @@ func testGetHostsInPack(t *testing.T, ds kolide.Datastore) {
 	err = ds.AddLabelToPack(l1.ID, p1.ID)
 	require.Nil(t, err)
 
-	h1, err := ds.NewHost(&kolide.Host{
+	h1 := test.NewHost(t, ds, "h1.local", "10.10.10.1", "1", "1", mockClock.Now())
-		DetailUpdateTime: mockClock.Now(),
-		SeenTime:         mockClock.Now(),
-		HostName:         "foobar.local",
-		OsqueryHostID:    "1",
-		NodeKey:          "1",
-		UUID:             "1",
-	})
-	require.Nil(t, err)
 
 	err = ds.RecordLabelQueryExecutions(
 		h1,
@@ -86,15 +78,11 @@ func testGetHostsInPack(t *testing.T, ds kolide.Datastore) {
 	require.Nil(t, err)
 	require.Len(t, hostsInPack, 1)
 
-	h2, err := ds.NewHost(&kolide.Host{
+	explicitHostsInPack, err := ds.ListExplicitHostsInPack(p1.ID, kolide.ListOptions{})
-		DetailUpdateTime: mockClock.Now(),
-		SeenTime:         mockClock.Now(),
-		HostName:         "foobaz.local",
-		OsqueryHostID:    "2",
-		NodeKey:          "2",
-		UUID:             "2",
-	})
 	require.Nil(t, err)
+	require.Len(t, explicitHostsInPack, 0)
+
+	h2 := test.NewHost(t, ds, "h2.local", "10.10.10.2", "2", "2", mockClock.Now())
 
 	err = ds.RecordLabelQueryExecutions(
 		h2,
@@ -106,6 +94,19 @@ func testGetHostsInPack(t *testing.T, ds kolide.Datastore) {
 	hostsInPack, err = ds.ListHostsInPack(p1.ID, kolide.ListOptions{})
 	require.Nil(t, err)
 	require.Len(t, hostsInPack, 2)
+
+	h3 := test.NewHost(t, ds, "h3.local", "10.10.10.3", "3", "3", mockClock.Now())
+
+	err = ds.AddHostToPack(h3.ID, p1.ID)
+	require.Nil(t, err)
+
+	hostsInPack, err = ds.ListHostsInPack(p1.ID, kolide.ListOptions{})
+	require.Nil(t, err)
+	require.Len(t, hostsInPack, 3)
+
+	explicitHostsInPack, err = ds.ListExplicitHostsInPack(p1.ID, kolide.ListOptions{})
+	require.Nil(t, err)
+	require.Len(t, explicitHostsInPack, 1)
 }
 
 func testAddLabelToPackTwice(t *testing.T, ds kolide.Datastore) {

server/service/service_packs.go

@@ -268,6 +268,21 @@ func (svc service) ListPacksForHost(ctx context.Context, hid uint) ([]*kolide.Pa
 				break
 			}
 		}
+
+		// for each pack, we must know what host have been assigned to that pack
+		hostsForPack, err := svc.ds.ListExplicitHostsInPack(pack.ID, kolide.ListOptions{})
+		if err != nil {
+			return nil, err
+		}
+
+		// o(n) iteration to determine whether or not a pack is enabled
+		// in this case, n is len(hostsForPack)
+		for _, host := range hostsForPack {
+			if host.ID == hid {
+				packs = append(packs, pack)
+				break
+			}
+		}
 	}
 
 	return packs, nil

server/service/service_packs_test.go

@@ -3,9 +3,11 @@ package service
 import (
 	"testing"
 
+	"github.com/WatchBeam/clock"
 	"github.com/kolide/kolide-ose/server/config"
 	"github.com/kolide/kolide-ose/server/datastore/inmem"
 	"github.com/kolide/kolide-ose/server/kolide"
+	"github.com/kolide/kolide-ose/server/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/net/context"
@@ -168,3 +170,38 @@ func TestDeletePack(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Len(t, queries, 0)
 }
+
+func TestListPacksForHost(t *testing.T) {
+	ds, err := inmem.New(config.TestConfig())
+	assert.Nil(t, err)
+
+	mockClock := clock.NewMockClock()
+
+	svc, err := newTestService(ds, nil)
+	assert.Nil(t, err)
+
+	ctx := context.Background()
+
+	h1 := test.NewHost(t, ds, "h1", "10.10.10.1", "1", "1", mockClock.Now())
+	h2 := test.NewHost(t, ds, "h2", "10.10.10.2", "2", "2", mockClock.Now())
+
+	p1 := test.NewPack(t, ds, "p1")
+	p2 := test.NewPack(t, ds, "p2")
+
+	require.Nil(t, svc.AddHostToPack(ctx, h1.ID, p1.ID))
+	require.Nil(t, svc.AddHostToPack(ctx, h2.ID, p1.ID))
+
+	require.Nil(t, svc.AddHostToPack(ctx, h1.ID, p2.ID))
+
+	{
+		packs, err := svc.ListPacksForHost(ctx, h1.ID)
+		require.Nil(t, err)
+		require.Len(t, packs, 2)
+	}
+	{
+		packs, err := svc.ListPacksForHost(ctx, h2.ID)
+		require.Nil(t, err)
+		require.Len(t, packs, 1)
+	}
+
+}