From 1eccf9a8742f2dd85f156fe72a422d0a0fd931c2 Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Tue, 13 Aug 2019 09:42:58 -0700 Subject: [PATCH] Add warning in query UI when Redis fails (#2086) - Add warning message when Redis fails - Disable query button when Redis fails - Refactor SMTP warning banner into component for reuse Closes #2073 --- frontend/components/SmtpWarning/index.js | 1 - .../WarningBanner.jsx} | 16 ++++--- .../WarningBanner/WarningBanner.tests.jsx | 46 +++++++++++++++++++ .../_styles.scss | 3 +- frontend/components/WarningBanner/index.js | 1 + .../QueryPageSelectTargets.jsx | 3 ++ .../QueryProgressDetails.jsx | 4 +- frontend/kolide/endpoints.js | 1 + frontend/kolide/index.js | 2 + frontend/kolide/status.js | 12 +++++ .../Admin/AppSettingsPage/AppSettingsPage.jsx | 5 +- .../AppSettingsPage/AppSettingsPage.tests.jsx | 8 ++-- .../UserManagementPage/UserManagementPage.jsx | 5 +- .../UserManagementPage.tests.jsx | 6 +-- .../pages/queries/QueryPage/QueryPage.jsx | 24 +++++++++- frontend/pages/queries/QueryPage/_styles.scss | 4 ++ server/kolide/query_results.go | 4 ++ server/kolide/service.go | 1 + server/kolide/status.go | 9 ++++ server/pubsub/inmem_query_results.go | 4 ++ server/service/endpoint_status.go | 24 ++++++++++ server/service/handler.go | 8 ++++ server/service/service_status.go | 7 +++ 23 files changed, 176 insertions(+), 22 deletions(-) delete mode 100644 frontend/components/SmtpWarning/index.js rename frontend/components/{SmtpWarning/SmtpWarning.jsx => WarningBanner/WarningBanner.jsx} (64%) create mode 100644 frontend/components/WarningBanner/WarningBanner.tests.jsx rename frontend/components/{SmtpWarning => WarningBanner}/_styles.scss (97%) create mode 100644 frontend/components/WarningBanner/index.js create mode 100644 frontend/kolide/status.js create mode 100644 server/kolide/status.go create mode 100644 server/service/endpoint_status.go create mode 100644 server/service/service_status.go diff --git a/frontend/components/SmtpWarning/index.js b/frontend/components/SmtpWarning/index.js deleted file mode 100644 index 891a693420..0000000000 --- a/frontend/components/SmtpWarning/index.js +++ /dev/null @@ -1 +0,0 @@ -export default from './SmtpWarning'; diff --git a/frontend/components/SmtpWarning/SmtpWarning.jsx b/frontend/components/WarningBanner/WarningBanner.jsx similarity index 64% rename from frontend/components/SmtpWarning/SmtpWarning.jsx rename to frontend/components/WarningBanner/WarningBanner.jsx index 02ef81f730..91c00fadae 100644 --- a/frontend/components/SmtpWarning/SmtpWarning.jsx +++ b/frontend/components/WarningBanner/WarningBanner.jsx @@ -5,33 +5,37 @@ import classnames from 'classnames'; import Button from 'components/buttons/Button'; import Icon from 'components/icons/Icon'; -const baseClass = 'smtp-warning'; +const baseClass = 'warning-banner'; -const SmtpWarning = ({ className, onDismiss, onResolve, shouldShowWarning }) => { +const WarningBanner = ({ className, message, labelText, shouldShowWarning, onDismiss, onResolve }) => { if (!shouldShowWarning) { return false; } const fullClassName = classnames(baseClass, className); + const label = labelText || 'Warning!'; + return (
- Warning! + {label}
- Email is not currently configured in Fleet. Many features rely on email to work. + {message} {onDismiss && } {onResolve && }
); }; -SmtpWarning.propTypes = { +WarningBanner.propTypes = { className: PropTypes.string, + message: PropTypes.string.isRequired, + labelText: PropTypes.string, onDismiss: PropTypes.func, onResolve: PropTypes.func, shouldShowWarning: PropTypes.bool.isRequired, }; -export default SmtpWarning; +export default WarningBanner; diff --git a/frontend/components/WarningBanner/WarningBanner.tests.jsx b/frontend/components/WarningBanner/WarningBanner.tests.jsx new file mode 100644 index 0000000000..4cade5f7a9 --- /dev/null +++ b/frontend/components/WarningBanner/WarningBanner.tests.jsx @@ -0,0 +1,46 @@ +import React from 'react'; +import expect, { createSpy } from 'expect'; +import { shallow } from 'enzyme'; + +import WarningBanner from 'components/WarningBanner/WarningBanner'; + +describe('WarningBanner - component', () => { + it('renders default banner', () => { + const props = { shouldShowWarning: true, message: 'message' }; + const component = shallow(); + expect(component.length).toEqual(1); + expect(component.find('Icon').props().name).toEqual('warning-filled'); + expect(component.find('.warning-banner__label').text()).toEqual('Warning!'); + expect(component.find('.warning-banner__text').text()).toEqual('message'); + }); + + it('renders custom label', () => { + const props = { shouldShowWarning: true, message: 'message', labelText: 'label' }; + const component = shallow(); + expect(component.find('.warning-banner__label').text()).toEqual('label'); + }); + + it('renders empty when disabled', () => { + const props = { shouldShowWarning: false, message: 'message' }; + const component = shallow(); + expect(component.html()).toBe(null); + }); + + it('handles dismiss action', () => { + const spy = createSpy(); + const props = { shouldShowWarning: true, message: 'message', onDismiss: spy }; + const component = shallow(); + + component.find('Button').simulate('click'); + expect(spy).toHaveBeenCalled(); + }); + + it('handles resolve action', () => { + const spy = createSpy(); + const props = { shouldShowWarning: true, message: 'message', onResolve: spy }; + const component = shallow(); + + component.find('Button').simulate('click'); + expect(spy).toHaveBeenCalled(); + }); +}); diff --git a/frontend/components/SmtpWarning/_styles.scss b/frontend/components/WarningBanner/_styles.scss similarity index 97% rename from frontend/components/SmtpWarning/_styles.scss rename to frontend/components/WarningBanner/_styles.scss index ea156f53a3..6c3cb0c590 100644 --- a/frontend/components/SmtpWarning/_styles.scss +++ b/frontend/components/WarningBanner/_styles.scss @@ -1,4 +1,4 @@ -.smtp-warning { +.warning-banner { display: flex; justify-content: space-between; align-items: flex-start; @@ -34,4 +34,3 @@ margin-left: 15px; } } - diff --git a/frontend/components/WarningBanner/index.js b/frontend/components/WarningBanner/index.js new file mode 100644 index 0000000000..91c13437eb --- /dev/null +++ b/frontend/components/WarningBanner/index.js @@ -0,0 +1 @@ +export default from './WarningBanner'; diff --git a/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx b/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx index c99490ff57..38b4d54320 100644 --- a/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx +++ b/frontend/components/queries/QueryPageSelectTargets/QueryPageSelectTargets.jsx @@ -20,6 +20,7 @@ class QueryPageSelectTargets extends Component { selectedTargets: PropTypes.arrayOf(targetInterface), targetsCount: PropTypes.number, queryTimerMilliseconds: PropTypes.number, + disableRun: PropTypes.bool, }; render () { @@ -34,6 +35,7 @@ class QueryPageSelectTargets extends Component { onStopQuery, queryIsRunning, queryTimerMilliseconds, + disableRun, } = this.props; return ( @@ -44,6 +46,7 @@ class QueryPageSelectTargets extends Component { onStopQuery={onStopQuery} queryIsRunning={queryIsRunning} queryTimerMilliseconds={queryTimerMilliseconds} + disableRun={disableRun} /> { +const QueryProgressDetails = ({ campaign, className, onRunQuery, onStopQuery, queryIsRunning, queryTimerMilliseconds, disableRun }) => { const { hosts_count: hostsCount } = campaign; const totalHostsCount = get(campaign, ['totals', 'count'], 0); const totalRowsCount = get(campaign, ['query_results', 'length'], 0); @@ -20,6 +20,7 @@ const QueryProgressDetails = ({ campaign, className, onRunQuery, onStopQuery, qu className={`${baseClass}__run-btn`} onClick={onRunQuery} variant="success" + disabled={disableRun} > Run @@ -75,6 +76,7 @@ QueryProgressDetails.propTypes = { onStopQuery: PropTypes.func.isRequired, queryIsRunning: PropTypes.bool, queryTimerMilliseconds: PropTypes.number, + disableRun: PropTypes.bool, }; export default QueryProgressDetails; diff --git a/frontend/kolide/endpoints.js b/frontend/kolide/endpoints.js index b272d91b76..0d3f4f57f3 100644 --- a/frontend/kolide/endpoints.js +++ b/frontend/kolide/endpoints.js @@ -34,4 +34,5 @@ export default { return `/v1/kolide/users/${id}/admin`; }, SSO: '/v1/kolide/sso', + STATUS_RESULT_STORE: '/v1/kolide/status/result_store', }; diff --git a/frontend/kolide/index.js b/frontend/kolide/index.js index c47c32a6c8..febbcbde11 100644 --- a/frontend/kolide/index.js +++ b/frontend/kolide/index.js @@ -13,6 +13,7 @@ import statusLabelMethods from 'kolide/entities/status_labels'; import targetMethods from 'kolide/entities/targets'; import userMethods from 'kolide/entities/users'; import websocketMethods from 'kolide/websockets'; +import statusMethods from 'kolide/status'; const DEFAULT_BODY = JSON.stringify({}); @@ -33,6 +34,7 @@ class Kolide extends Base { this.targets = targetMethods(this); this.users = userMethods(this); this.websockets = websocketMethods(this); + this.status = statusMethods(this); } authenticatedDelete (endpoint, overrideHeaders = {}) { diff --git a/frontend/kolide/status.js b/frontend/kolide/status.js new file mode 100644 index 0000000000..50fe25c872 --- /dev/null +++ b/frontend/kolide/status.js @@ -0,0 +1,12 @@ +import endpoints from 'kolide/endpoints'; + +export default (client) => { + return { + result_store: () => { + const { STATUS_RESULT_STORE } = endpoints; + const endpoint = client.baseURL + STATUS_RESULT_STORE; + + return client.authenticatedGet(endpoint); + }, + }; +}; diff --git a/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx b/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx index 64b3e434ba..03478f85f5 100644 --- a/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx +++ b/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.jsx @@ -7,7 +7,7 @@ import AppConfigForm from 'components/forms/admin/AppConfigForm'; import configInterface from 'interfaces/config'; import deepDifference from 'utilities/deep_difference'; import { renderFlash } from 'redux/nodes/notifications/actions'; -import SmtpWarning from 'components/SmtpWarning'; +import WarningBanner from 'components/WarningBanner'; import { updateConfig } from 'redux/nodes/app/actions'; export const baseClass = 'app-settings'; @@ -68,7 +68,8 @@ class AppSettingsPage extends Component { return (

App Settings

- diff --git a/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.tests.jsx b/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.tests.jsx index 4d3e1d200b..a11b7ae414 100644 --- a/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.tests.jsx +++ b/frontend/pages/Admin/AppSettingsPage/AppSettingsPage.tests.jsx @@ -28,7 +28,7 @@ describe('AppSettingsPage - component', () => { connectedComponent(AppSettingsPage, { mockStore }) ).find('AppSettingsPage'); - const smtpWarning = page.find('SmtpWarning'); + const smtpWarning = page.find('WarningBanner'); expect(smtpWarning.length).toEqual(1); expect(smtpWarning.find('Icon').length).toEqual(1); @@ -41,12 +41,12 @@ describe('AppSettingsPage - component', () => { connectedComponent(AppSettingsPage, { mockStore }) ); - const smtpWarning = page.find('SmtpWarning'); + const smtpWarning = page.find('WarningBanner'); const dismissButton = smtpWarning.find('Button').first(); dismissButton.simulate('click'); - expect(page.find('SmtpWarning').html()).toNotExist(); + expect(page.find('WarningBanner').html()).toNotExist(); }); it('does not render a warning if SMTP has been configured', () => { @@ -55,6 +55,6 @@ describe('AppSettingsPage - component', () => { connectedComponent(AppSettingsPage, { mockStore }) ).find('AppSettingsPage'); - expect(page.find('SmtpWarning').html()).toNotExist(); + expect(page.find('WarningBanner').html()).toNotExist(); }); }); diff --git a/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx b/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx index 4da203a32a..79b40d055e 100644 --- a/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx +++ b/frontend/pages/Admin/UserManagementPage/UserManagementPage.jsx @@ -14,7 +14,7 @@ import InviteUserForm from 'components/forms/InviteUserForm'; import Modal from 'components/modals/Modal'; import paths from 'router/paths'; import { renderFlash } from 'redux/nodes/notifications/actions'; -import SmtpWarning from 'components/SmtpWarning'; +import WarningBanner from 'components/WarningBanner'; import { updateUser } from 'redux/nodes/auth/actions'; import userActions from 'redux/nodes/entities/users/actions'; import UserBlock from 'components/UserBlock'; @@ -249,7 +249,8 @@ export class UserManagementPage extends Component { return (
- diff --git a/frontend/pages/Admin/UserManagementPage/UserManagementPage.tests.jsx b/frontend/pages/Admin/UserManagementPage/UserManagementPage.tests.jsx index 82f3a46ca4..59dc881406 100644 --- a/frontend/pages/Admin/UserManagementPage/UserManagementPage.tests.jsx +++ b/frontend/pages/Admin/UserManagementPage/UserManagementPage.tests.jsx @@ -136,8 +136,8 @@ describe('UserManagementPage - component', () => { mockStore: configuredMockStore, })); - expect(notConfiguredPage.find('SmtpWarning').html()).toExist(); - expect(configuredPage.find('SmtpWarning').html()).toNotExist(); + expect(notConfiguredPage.find('WarningBanner').html()).toExist(); + expect(configuredPage.find('WarningBanner').html()).toNotExist(); }); }); @@ -146,7 +146,7 @@ describe('UserManagementPage - component', () => { const mockStore = reduxMockStore(notConfiguredStore); const page = mount(connectedComponent(ConnectedUserManagementPage, { mockStore })); - const smtpWarning = page.find('SmtpWarning'); + const smtpWarning = page.find('WarningBanner'); smtpWarning.find('Button').simulate('click'); diff --git a/frontend/pages/queries/QueryPage/QueryPage.jsx b/frontend/pages/queries/QueryPage/QueryPage.jsx index 478643c9dc..814e57100f 100644 --- a/frontend/pages/queries/QueryPage/QueryPage.jsx +++ b/frontend/pages/queries/QueryPage/QueryPage.jsx @@ -17,6 +17,7 @@ import { formatSelectedTargetsForApi } from 'kolide/helpers'; import helpers from 'pages/queries/QueryPage/helpers'; import hostActions from 'redux/nodes/entities/hosts/actions'; import hostInterface from 'interfaces/host'; +import WarningBanner from 'components/WarningBanner'; import QueryForm from 'components/forms/queries/QueryForm'; import osqueryTableInterface from 'interfaces/osquery_table'; import queryActions from 'redux/nodes/entities/queries/actions'; @@ -91,6 +92,10 @@ export class QueryPage extends Component { dispatch(hostActions.loadAll()); } + Kolide.status.result_store().catch((response) => { + this.setState({ resultStoreError: response.message.errors[0].reason }); + }); + helpers.selectHosts(dispatch, { hosts: selectedHosts, selectedTargets, @@ -445,6 +450,20 @@ export class QueryPage extends Component { return false; } + renderResultStoreWarning = () => { + const { resultStoreError } = this.state; + + if (!resultStoreError) { + return false; + } + + const message = `Live query disabled due to Redis error: ${resultStoreError}`; + + return ( + + ); + } + renderResultsTable = () => { const { campaign, @@ -485,7 +504,7 @@ export class QueryPage extends Component { renderTargetsInput = () => { const { onFetchTargets, onRunQuery, onStopQuery, onTargetSelect } = this; - const { campaign, queryIsRunning, targetsCount, targetsError, runQueryMilliseconds } = this.state; + const { campaign, queryIsRunning, targetsCount, targetsError, runQueryMilliseconds, resultStoreError } = this.state; const { selectedTargets } = this.props; return ( @@ -500,6 +519,7 @@ export class QueryPage extends Component { selectedTargets={selectedTargets} targetsCount={targetsCount} queryTimerMilliseconds={runQueryMilliseconds} + disableRun={resultStoreError !== undefined} /> ); } @@ -515,6 +535,7 @@ export class QueryPage extends Component { onUpdateQuery, renderResultsTable, renderTargetsInput, + renderResultStoreWarning, } = this; const { queryIsRunning } = this.state; const { @@ -547,6 +568,7 @@ export class QueryPage extends Component { title={title} />
+ {renderResultStoreWarning()} {renderTargetsInput()} {renderResultsTable()}
diff --git a/frontend/pages/queries/QueryPage/_styles.scss b/frontend/pages/queries/QueryPage/_styles.scss index 3698898adb..04bce3122f 100644 --- a/frontend/pages/queries/QueryPage/_styles.scss +++ b/frontend/pages/queries/QueryPage/_styles.scss @@ -12,4 +12,8 @@ position: relative; min-height: 400px; } + + &__warning { + margin: 0px; + } } diff --git a/server/kolide/query_results.go b/server/kolide/query_results.go index f4afc44b71..e0abbca042 100644 --- a/server/kolide/query_results.go +++ b/server/kolide/query_results.go @@ -16,4 +16,8 @@ type QueryResultStore interface { // query results. Channel values should be either // DistributedQueryResult or error ReadChannel(ctx context.Context, query DistributedQueryCampaign) (<-chan interface{}, error) + + // HealthCheck returns nil if the store is functioning properly, or an + // error describing the problem. + HealthCheck() error } diff --git a/server/kolide/service.go b/server/kolide/service.go index 9fd45f85a8..9f76e0f322 100644 --- a/server/kolide/service.go +++ b/server/kolide/service.go @@ -17,4 +17,5 @@ type Service interface { ScheduledQueryService OptionService FileIntegrityMonitoringService + StatusService } diff --git a/server/kolide/status.go b/server/kolide/status.go new file mode 100644 index 0000000000..97f4d3a850 --- /dev/null +++ b/server/kolide/status.go @@ -0,0 +1,9 @@ +package kolide + +import "context" + +type StatusService interface { + // StatusResultStore returns nil if the result store is functioning + // correctly, or an error indicating the problem. + StatusResultStore(ctx context.Context) error +} diff --git a/server/pubsub/inmem_query_results.go b/server/pubsub/inmem_query_results.go index b560538443..cd4ba29c80 100644 --- a/server/pubsub/inmem_query_results.go +++ b/server/pubsub/inmem_query_results.go @@ -60,3 +60,7 @@ func (im *inmemQueryResults) ReadChannel(ctx context.Context, query kolide.Distr }() return channel, nil } + +func (im *inmemQueryResults) HealthCheck() error { + return nil +} diff --git a/server/service/endpoint_status.go b/server/service/endpoint_status.go new file mode 100644 index 0000000000..d13ec148c3 --- /dev/null +++ b/server/service/endpoint_status.go @@ -0,0 +1,24 @@ +package service + +import ( + "context" + + "github.com/go-kit/kit/endpoint" + "github.com/kolide/fleet/server/kolide" +) + +type statusResultStoreResponse struct { + Err error `json:"error,omitempty"` +} + +func (m statusResultStoreResponse) error() error { return m.Err } + +func makeStatusResultStoreEndpoint(svc kolide.Service) endpoint.Endpoint { + return func(ctx context.Context, req interface{}) (interface{}, error) { + var resp statusResultStoreResponse + if err := svc.StatusResultStore(ctx); err != nil { + resp.Err = err + } + return resp, nil + } +} diff --git a/server/service/handler.go b/server/service/handler.go index 06388aa81d..b13e58475c 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -96,6 +96,7 @@ type KolideEndpoints struct { SSOSettings endpoint.Endpoint GetFIM endpoint.Endpoint ModifyFIM endpoint.Endpoint + StatusResultStore endpoint.Endpoint } // MakeKolideServerEndpoints creates the Kolide API endpoints. @@ -188,6 +189,9 @@ func MakeKolideServerEndpoints(svc kolide.Service, jwtKey string) KolideEndpoint GetFIM: authenticatedUser(jwtKey, svc, makeGetFIMEndpoint(svc)), ModifyFIM: authenticatedUser(jwtKey, svc, makeModifyFIMEndpoint(svc)), + // Authenticated status endpoints + StatusResultStore: authenticatedUser(jwtKey, svc, makeStatusResultStoreEndpoint(svc)), + // Osquery endpoints EnrollAgent: makeEnrollAgentEndpoint(svc), GetClientConfig: authenticatedHost(svc, makeGetClientConfigEndpoint(svc)), @@ -279,6 +283,7 @@ type kolideHandlers struct { SettingsSSO http.Handler ModifyFIM http.Handler GetFIM http.Handler + StatusResultStore http.Handler } func makeKolideKitHandlers(e KolideEndpoints, opts []kithttp.ServerOption) *kolideHandlers { @@ -367,6 +372,7 @@ func makeKolideKitHandlers(e KolideEndpoints, opts []kithttp.ServerOption) *koli SettingsSSO: newServer(e.SSOSettings, decodeNoParamsRequest), ModifyFIM: newServer(e.ModifyFIM, decodeModifyFIMRequest), GetFIM: newServer(e.GetFIM, decodeNoParamsRequest), + StatusResultStore: newServer(e.StatusResultStore, decodeNoParamsRequest), } } @@ -496,6 +502,8 @@ func attachKolideAPIRoutes(r *mux.Router, h *kolideHandlers) { r.Handle("/api/v1/kolide/targets", h.SearchTargets).Methods("POST").Name("search_targets") + r.Handle("/api/v1/kolide/status/result_store", h.StatusResultStore).Methods("GET").Name("status_result_store") + r.Handle("/api/v1/osquery/enroll", h.EnrollAgent).Methods("POST").Name("enroll_agent") r.Handle("/api/v1/osquery/config", h.GetClientConfig).Methods("POST").Name("get_client_config") r.Handle("/api/v1/osquery/distributed/read", h.GetDistributedQueries).Methods("POST").Name("get_distributed_queries") diff --git a/server/service/service_status.go b/server/service/service_status.go new file mode 100644 index 0000000000..e278e22c81 --- /dev/null +++ b/server/service/service_status.go @@ -0,0 +1,7 @@ +package service + +import "context" + +func (svc service) StatusResultStore(ctx context.Context) error { + return svc.resultStore.HealthCheck() +}