diff --git a/changes/fix-packs-page b/changes/fix-packs-page new file mode 100644 index 000000000..9c6964dac --- /dev/null +++ b/changes/fix-packs-page @@ -0,0 +1 @@ +* Fix bugs with targeting and improve performance on the Packs edit page. diff --git a/cypress/integration/all/app/packflow.spec.ts b/cypress/integration/all/app/packflow.spec.ts index 106f8f891..52fd2f1c9 100644 --- a/cypress/integration/all/app/packflow.spec.ts +++ b/cypress/integration/all/app/packflow.spec.ts @@ -12,6 +12,7 @@ describe( it("Create, edit, and delete a pack and pack query successfully", () => { cy.visit("/packs/manage"); + cy.wait(1000); // eslint-disable-line cypress/no-unnecessary-waiting cy.findByRole("button", { name: /create new pack/i }).click(); cy.wait(1000); // eslint-disable-line cypress/no-unnecessary-waiting @@ -116,7 +117,7 @@ describe( cy.get(".remove-pack-modal__btn-wrap > .button--alert") .contains("button", /delete/i) - .click(); + .click({ force: true }); cy.findByText(/successfully deleted/i).should("be.visible"); diff --git a/frontend/components/forms/packs/EditPackForm/EditPackForm.tsx b/frontend/components/forms/packs/EditPackForm/EditPackForm.tsx index e6dc24c8b..5903b4d9c 100644 --- a/frontend/components/forms/packs/EditPackForm/EditPackForm.tsx +++ b/frontend/components/forms/packs/EditPackForm/EditPackForm.tsx @@ -12,7 +12,6 @@ import InputField from "components/forms/fields/InputField"; import SelectTargetsDropdown from "components/forms/fields/SelectTargetsDropdown"; import PackQueriesListWrapper from "components/queries/PackQueriesListWrapper"; -const fieldNames = ["description", "name", "targets"]; const baseClass = "edit-pack-form"; interface IEditPackForm { diff --git a/frontend/components/forms/packs/PackForm/PackForm.tsx b/frontend/components/forms/packs/PackForm/PackForm.tsx index 4c010919a..30d43a7d6 100644 --- a/frontend/components/forms/packs/PackForm/PackForm.tsx +++ b/frontend/components/forms/packs/PackForm/PackForm.tsx @@ -1,5 +1,4 @@ -import React, { Component, useState } from "react"; -import PropTypes from "prop-types"; +import React, { useState } from "react"; import classnames from "classnames"; import Button from "components/buttons/Button"; @@ -41,7 +40,6 @@ const EditPackForm = ({ onFetchTargets, selectedTargetsCount, isPremiumTier, - formData, baseError, }: IPackForm): JSX.Element => { const [errors, setErrors] = useState<{ [key: string]: any }>({}); diff --git a/frontend/interfaces/pack.ts b/frontend/interfaces/pack.ts index 0f86da627..33c3b56d7 100644 --- a/frontend/interfaces/pack.ts +++ b/frontend/interfaces/pack.ts @@ -1,4 +1,7 @@ import PropTypes from "prop-types"; +import { IHost } from "./host"; +import { ILabel } from "./label"; +import { ITeam } from "./team"; export default PropTypes.shape({ created_at: PropTypes.string, @@ -25,7 +28,10 @@ export interface IPack { disabled?: boolean; query_count: number; total_hosts_count: number; + hosts: IHost[]; host_ids: number[]; + labels: ILabel[]; label_ids: number[]; + teams: ITeam[]; team_ids: number[]; } diff --git a/frontend/pages/packs/EditPackPage/EditPackPage.tsx b/frontend/pages/packs/EditPackPage/EditPackPage.tsx index 5a71a83a8..ba286dd55 100644 --- a/frontend/pages/packs/EditPackPage/EditPackPage.tsx +++ b/frontend/pages/packs/EditPackPage/EditPackPage.tsx @@ -1,8 +1,7 @@ -import React, { useState, useEffect, useCallback, useContext } from "react"; +import React, { useState, useCallback, useContext } from "react"; import { useQuery } from "react-query"; import { Params } from "react-router/lib/Router"; -import { filter, includes } from "lodash"; import { useDispatch } from "react-redux"; import { push } from "react-router-redux"; @@ -18,12 +17,9 @@ import { ITarget, ITargetsAPIResponse } from "interfaces/target"; import { ITeam } from "interfaces/team"; import { AppContext } from "context/app"; -import hostsAPI from "services/entities/hosts"; -import labelsAPI from "services/entities/labels"; import packsAPI from "services/entities/packs"; import queriesAPI from "services/entities/queries"; import scheduledqueriesAPI from "services/entities/scheduled_queries"; -import teamsAPI from "services/entities/teams"; // @ts-ignore import { renderFlash } from "redux/nodes/notifications/actions"; @@ -44,17 +40,6 @@ interface IStoredFleetQueriesResponse { queries: IQuery[]; } -interface IStoredLabelsResponse { - labels: ILabel[]; -} -interface IStoredHostsResponse { - hosts: IHost[]; -} - -interface IStoredTeamsResponse { - teams: ITeam[]; -} - interface IStoredPackResponse { pack: IPack; } @@ -88,22 +73,6 @@ const EditPacksPage = ({ select: (data: IStoredFleetQueriesResponse) => data.queries, }); - const { data: labels } = useQuery( - ["labels"], - () => labelsAPI.loadAll(), - { - select: (data: IStoredLabelsResponse) => data.labels, - } - ); - - const { data: hosts } = useQuery( - ["all hosts"], - () => hostsAPI.loadAll({ perPage: 30000 }), - { - select: (data: IStoredHostsResponse) => data.hosts, - } - ); - const { data: storedPack } = useQuery( ["stored pack"], () => packsAPI.load(packId), @@ -137,58 +106,23 @@ const EditPacksPage = ({ const [selectedPackQueryIds, setSelectedPackQueryIds] = useState< number[] | never[] >([]); - const [storedPackLabels, setStoredPackLabels] = useState([]); - const [storedPackHosts, setStoredPackHosts] = useState([]); - const [storedPackTeams, setStoredPackTeams] = useState([]); - useEffect(() => { - if (labels && storedPack) { - const packLabels = filter(labels, (label) => { - return includes(storedPack.label_ids, label.id); - }); - setStoredPackLabels(packLabels); - } - }, [labels, storedPack]); - - useEffect(() => { - if (hosts && storedPack) { - const packHosts = filter(hosts, (host) => { - return includes(storedPack.host_ids, host.id); - }); - setStoredPackHosts(packHosts); - } - }, [hosts, storedPack]); - - const { data: teams } = useQuery( - ["all teams"], - () => teamsAPI.loadAll(), - { - enabled: !!isPremiumTier, - select: (data: IStoredTeamsResponse) => data.teams, - } - ); - - useEffect(() => { - if (teams && storedPack) { - const packTeams = filter(teams, (team) => { - return includes(storedPack.team_ids, team.id); - }); - setStoredPackTeams(packTeams); - } - }, [teams, storedPack]); - - const packTargets = [ - ...storedPackHosts.map((host) => ({ - ...host, - target_type: "hosts", - })), - ...storedPackLabels, - ...storedPackTeams.map((team) => ({ - ...team, - target_type: "teams", - display_text: team.name, - })), - ]; + const packTargets = storedPack + ? [ + ...storedPack.hosts.map((host) => ({ + ...host, + target_type: "hosts", + })), + ...storedPack.labels.map((label) => ({ + ...label, + target_type: "labels", + })), + ...storedPack.teams.map((team) => ({ + ...team, + target_type: "teams", + })), + ] + : []; const onCancelEditPack = () => { return dispatch(push(PATHS.MANAGE_PACKS)); diff --git a/server/datastore/mysql/packs.go b/server/datastore/mysql/packs.go index 0a08802cf..5212633f7 100644 --- a/server/datastore/mysql/packs.go +++ b/server/datastore/mysql/packs.go @@ -315,21 +315,38 @@ func replacePackTargetsDB(ctx context.Context, tx sqlx.ExecerContext, pack *flee } func loadPackTargetsDB(ctx context.Context, q sqlx.QueryerContext, pack *fleet.Pack) error { - var targets []fleet.PackTarget - sql := `SELECT * FROM pack_targets WHERE pack_id = ?` - if err := sqlx.SelectContext(ctx, q, &targets, sql, pack.ID); err != nil { + var targets []fleet.Target + sql := ` + SELECT type, target_id, + COALESCE( + CASE + WHEN type = ? THEN (SELECT hostname FROM hosts WHERE id = target_id) + WHEN type = ? THEN (SELECT name FROM teams WHERE id = target_id) + WHEN type = ? THEN (SELECT name FROM labels WHERE id = target_id) + END + , '') AS display_text + FROM pack_targets + WHERE pack_id = ?` + if err := sqlx.SelectContext( + ctx, q, &targets, sql, + fleet.TargetHost, fleet.TargetTeam, fleet.TargetLabel, pack.ID, + ); err != nil { return ctxerr.Wrap(ctx, err, "select pack targets") } pack.HostIDs, pack.LabelIDs, pack.TeamIDs = []uint{}, []uint{}, []uint{} + pack.Hosts, pack.Labels, pack.Teams = []fleet.Target{}, []fleet.Target{}, []fleet.Target{} for _, target := range targets { switch target.Type { case fleet.TargetHost: pack.HostIDs = append(pack.HostIDs, target.TargetID) + pack.Hosts = append(pack.Hosts, target) case fleet.TargetLabel: pack.LabelIDs = append(pack.LabelIDs, target.TargetID) + pack.Labels = append(pack.Labels, target) case fleet.TargetTeam: pack.TeamIDs = append(pack.TeamIDs, target.TargetID) + pack.Teams = append(pack.Teams, target) default: return ctxerr.Errorf(ctx, "unknown target type: %d", target.Type) } diff --git a/server/datastore/mysql/packs_test.go b/server/datastore/mysql/packs_test.go index 56ea79c7f..8621ba6d9 100644 --- a/server/datastore/mysql/packs_test.go +++ b/server/datastore/mysql/packs_test.go @@ -65,8 +65,11 @@ func testPacksDelete(t *testing.T, ds *Datastore) { func testPacksSave(t *testing.T, ds *Datastore) { expectedPack := &fleet.Pack{ Name: "foo", + Hosts: []fleet.Target{{TargetID: 1, Type: fleet.TargetHost}}, HostIDs: []uint{1}, + Labels: []fleet.Target{{TargetID: 1, Type: fleet.TargetLabel}}, LabelIDs: []uint{1}, + Teams: []fleet.Target{{TargetID: 1, Type: fleet.TargetTeam}}, TeamIDs: []uint{1}, } @@ -82,8 +85,11 @@ func testPacksSave(t *testing.T, ds *Datastore) { expectedPack = &fleet.Pack{ ID: pack.ID, Name: "bar", + Hosts: []fleet.Target{{TargetID: 3, Type: fleet.TargetHost}}, HostIDs: []uint{3}, + Labels: []fleet.Target{{TargetID: 4, Type: fleet.TargetLabel}, {TargetID: 6, Type: fleet.TargetLabel}}, LabelIDs: []uint{4, 6}, + Teams: []fleet.Target{}, TeamIDs: []uint{}, } diff --git a/server/fleet/packs.go b/server/fleet/packs.go index 765343337..3fdd726d4 100644 --- a/server/fleet/packs.go +++ b/server/fleet/packs.go @@ -10,15 +10,18 @@ type PackListOptions struct { // Pack is the structure which represents an osquery query pack. type Pack struct { UpdateCreateTimestamps - ID uint `json:"id"` - Name string `json:"name"` - Description string `json:"description,omitempty"` - Platform string `json:"platform,omitempty"` - Disabled bool `json:"disabled"` - Type *string `json:"type" db:"pack_type"` - LabelIDs []uint `json:"label_ids"` - HostIDs []uint `json:"host_ids"` - TeamIDs []uint `json:"team_ids"` + ID uint `json:"id"` + Name string `json:"name"` + Description string `json:"description,omitempty"` + Platform string `json:"platform,omitempty"` + Disabled bool `json:"disabled"` + Type *string `json:"type" db:"pack_type"` + Labels []Target `json:"labels"` + LabelIDs []uint `json:"label_ids"` + Hosts []Target `json:"hosts"` + HostIDs []uint `json:"host_ids"` + Teams []Target `json:"teams"` + TeamIDs []uint `json:"team_ids"` } // EditablePackType only returns true when the pack doesn't have a specific Type set, only nil & empty string Pack.Type @@ -75,8 +78,8 @@ type PackSpecQuery struct { // PackTarget targets a pack to a host, label, or team. type PackTarget struct { - ID uint `db:"id"` - PackID uint `db:"pack_id"` + ID uint `db:"id" json:"-"` + PackID uint `db:"pack_id" json:"-"` Target } diff --git a/server/fleet/targets.go b/server/fleet/targets.go index 8cbc35dd9..9a1e42ecf 100644 --- a/server/fleet/targets.go +++ b/server/fleet/targets.go @@ -1,5 +1,10 @@ package fleet +import ( + "encoding/json" + "fmt" +) + type TargetSearchResults struct { Hosts []*Host Labels []*Label @@ -47,9 +52,53 @@ const ( TargetTeam ) +func (t TargetType) String() string { + switch t { + case TargetLabel: + return "label" + case TargetHost: + return "host" + case TargetTeam: + return "team" + default: + return fmt.Sprintf("unknown: %d", t) + } +} + +func ParseTargetType(s string) (TargetType, error) { + switch s { + case "label": + return TargetLabel, nil + case "host": + return TargetHost, nil + case "team": + return TargetTeam, nil + default: + return 0, fmt.Errorf("invalid TargetType: %s", s) + } +} + +func (t TargetType) MarshalJSON() ([]byte, error) { + return json.Marshal(t.String()) +} + +func (t *TargetType) UnmarshalJSON(b []byte) error { + var s string + if err := json.Unmarshal(b, &s); err != nil { + return err + } + parsed, err := ParseTargetType(s) + if err != nil { + return err + } + *t = parsed + return nil +} + type Target struct { - Type TargetType `db:"type"` - TargetID uint `db:"target_id"` + Type TargetType `db:"type" json:"type"` + TargetID uint `db:"target_id" json:"id"` + DisplayText string `db:"display_text" json:"display_text"` } func (t Target) AuthzType() string { diff --git a/server/fleet/targets_test.go b/server/fleet/targets_test.go new file mode 100644 index 000000000..3e24a9980 --- /dev/null +++ b/server/fleet/targets_test.go @@ -0,0 +1,36 @@ +package fleet_test + +import ( + "encoding/json" + "testing" + + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTargetTypeJSON(t *testing.T) { + testCases := []struct { + expected fleet.TargetType + shouldErr bool + }{ + {fleet.TargetLabel, false}, + {fleet.TargetHost, false}, + {fleet.TargetTeam, false}, + {fleet.TargetType(37), true}, + } + for _, tt := range testCases { + t.Run(tt.expected.String(), func(t *testing.T) { + b, err := json.Marshal(tt.expected) + require.NoError(t, err) + var target fleet.TargetType + err = json.Unmarshal(b, &target) + if tt.shouldErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, target) + } + }) + } +}