From 2a8f3eb2f921cfb1d6a5faaac59cbac5aac24910 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 24 Jun 2021 14:45:35 +0200 Subject: [PATCH] [Fleet] Fix staleness bug in "Add agent" flyout (#103095) * * Fix stale enrollment api token bug * Refactored naming * raise the state of the selected enrollment api key to parent to avoid state sync issues * removed consts for onKeyChange and selectedApiKeyId * fix typo Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- ...advanced_agent_authentication_settings.tsx | 38 ++++++++---------- .../agent_policy_selection.tsx | 40 ++++++++++--------- .../managed_instructions.tsx | 11 ++--- .../agent_enrollment_flyout/steps.tsx | 12 ++++-- 4 files changed, 52 insertions(+), 49 deletions(-) diff --git a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/advanced_agent_authentication_settings.tsx b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/advanced_agent_authentication_settings.tsx index 25602b7e108fdd..96fab27a550508 100644 --- a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/advanced_agent_authentication_settings.tsx +++ b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/advanced_agent_authentication_settings.tsx @@ -21,20 +21,19 @@ import { interface Props { agentPolicyId?: string; + selectedApiKeyId?: string; onKeyChange: (key?: string) => void; } export const AdvancedAgentAuthenticationSettings: FunctionComponent = ({ agentPolicyId, + selectedApiKeyId, onKeyChange, }) => { const { notifications } = useStartServices(); const [enrollmentAPIKeys, setEnrollmentAPIKeys] = useState( [] ); - // TODO: Remove this piece of state since we don't need it here. The currently selected enrollment API key only - // needs to live on the form - const [selectedEnrollmentApiKey, setSelectedEnrollmentApiKey] = useState(); const [isLoadingEnrollmentKey, setIsLoadingEnrollmentKey] = useState(false); const [isAuthenticationSettingsOpen, setIsAuthenticationSettingsOpen] = useState(false); @@ -51,7 +50,7 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent = ({ return; } setEnrollmentAPIKeys([res.data.item]); - setSelectedEnrollmentApiKey(res.data.item.id); + onKeyChange(res.data.item.id); notifications.toasts.addSuccess( i18n.translate('xpack.fleet.newEnrollmentKey.keyCreatedToasts', { defaultMessage: 'Enrollment token created', @@ -66,15 +65,6 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent = ({ } }; - useEffect( - function triggerOnKeyChangeEffect() { - if (onKeyChange) { - onKeyChange(selectedEnrollmentApiKey); - } - }, - [onKeyChange, selectedEnrollmentApiKey] - ); - useEffect( function useEnrollmentKeysForAgentPolicyEffect() { if (!agentPolicyId) { @@ -97,9 +87,13 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent = ({ throw new Error('No data while fetching enrollment API keys'); } - setEnrollmentAPIKeys( - res.data.list.filter((key) => key.policy_id === agentPolicyId && key.active === true) + const enrollmentAPIKeysResponse = res.data.list.filter( + (key) => key.policy_id === agentPolicyId && key.active === true ); + + setEnrollmentAPIKeys(enrollmentAPIKeysResponse); + // Default to the first enrollment key if there is one. + onKeyChange(enrollmentAPIKeysResponse[0]?.id); } catch (error) { notifications.toasts.addError(error, { title: 'Error', @@ -108,21 +102,21 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent = ({ } fetchEnrollmentAPIKeys(); }, - [agentPolicyId, notifications.toasts] + [onKeyChange, agentPolicyId, notifications.toasts] ); useEffect( function useDefaultEnrollmentKeyForAgentPolicyEffect() { if ( - !selectedEnrollmentApiKey && + !selectedApiKeyId && enrollmentAPIKeys.length > 0 && enrollmentAPIKeys[0].policy_id === agentPolicyId ) { const enrollmentAPIKeyId = enrollmentAPIKeys[0].id; - setSelectedEnrollmentApiKey(enrollmentAPIKeyId); + onKeyChange(enrollmentAPIKeyId); } }, - [enrollmentAPIKeys, selectedEnrollmentApiKey, agentPolicyId] + [enrollmentAPIKeys, selectedApiKeyId, agentPolicyId, onKeyChange] ); return ( <> @@ -139,14 +133,14 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent = ({ {isAuthenticationSettingsOpen && ( <> - {enrollmentAPIKeys.length && selectedEnrollmentApiKey ? ( + {enrollmentAPIKeys.length && selectedApiKeyId ? ( ({ value: key.id, text: key.name, }))} - value={selectedEnrollmentApiKey || undefined} + value={selectedApiKeyId || undefined} prepend={ = ({ } onChange={(e) => { - setSelectedEnrollmentApiKey(e.target.value); + onKeyChange(e.target.value); }} /> ) : ( diff --git a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/agent_policy_selection.tsx b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/agent_policy_selection.tsx index f92b2d48259351..d9d1aa2e77f86f 100644 --- a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/agent_policy_selection.tsx +++ b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/agent_policy_selection.tsx @@ -22,6 +22,7 @@ type Props = { } & ( | { withKeySelection: true; + selectedApiKeyId?: string; onKeyChange?: (key?: string) => void; } | { @@ -31,9 +32,9 @@ type Props = { const resolveAgentId = ( agentPolicies?: AgentPolicy[], - selectedAgentId?: string + selectedAgentPolicyId?: string ): undefined | string => { - if (agentPolicies && agentPolicies.length && !selectedAgentId) { + if (agentPolicies && agentPolicies.length && !selectedAgentPolicyId) { if (agentPolicies.length === 1) { return agentPolicies[0].id; } @@ -44,33 +45,33 @@ const resolveAgentId = ( } } - return selectedAgentId; + return selectedAgentPolicyId; }; export const EnrollmentStepAgentPolicy: React.FC = (props) => { - const { withKeySelection, agentPolicies, onAgentPolicyChange, excludeFleetServer } = props; - const onKeyChange = props.withKeySelection && props.onKeyChange; - const [selectedAgentId, setSelectedAgentId] = useState( + const { agentPolicies, onAgentPolicyChange, excludeFleetServer } = props; + + const [selectedAgentPolicyId, setSelectedAgentPolicyId] = useState( () => resolveAgentId(agentPolicies, undefined) // no agent id selected yet ); useEffect( function triggerOnAgentPolicyChangeEffect() { if (onAgentPolicyChange) { - onAgentPolicyChange(selectedAgentId); + onAgentPolicyChange(selectedAgentPolicyId); } }, - [selectedAgentId, onAgentPolicyChange] + [selectedAgentPolicyId, onAgentPolicyChange] ); useEffect( function useDefaultAgentPolicyEffect() { - const resolvedId = resolveAgentId(agentPolicies, selectedAgentId); - if (resolvedId !== selectedAgentId) { - setSelectedAgentId(resolvedId); + const resolvedId = resolveAgentId(agentPolicies, selectedAgentPolicyId); + if (resolvedId !== selectedAgentPolicyId) { + setSelectedAgentPolicyId(resolvedId); } }, - [agentPolicies, selectedAgentId] + [agentPolicies, selectedAgentPolicyId] ); return ( @@ -90,25 +91,26 @@ export const EnrollmentStepAgentPolicy: React.FC = (props) => { value: agentPolicy.id, text: agentPolicy.name, }))} - value={selectedAgentId || undefined} - onChange={(e) => setSelectedAgentId(e.target.value)} + value={selectedAgentPolicyId || undefined} + onChange={(e) => setSelectedAgentPolicyId(e.target.value)} aria-label={i18n.translate('xpack.fleet.enrollmentStepAgentPolicy.policySelectAriaLabel', { defaultMessage: 'Agent policy', })} /> - {selectedAgentId && ( + {selectedAgentPolicyId && ( )} - {withKeySelection && onKeyChange && ( + {props.withKeySelection && props.onKeyChange && ( <> )} diff --git a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/managed_instructions.tsx b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/managed_instructions.tsx index 919f0c3052db91..efae8db377f7f1 100644 --- a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/managed_instructions.tsx +++ b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/managed_instructions.tsx @@ -62,10 +62,10 @@ export const ManagedInstructions = React.memo( ({ agentPolicy, agentPolicies, viewDataStepContent }) => { const fleetStatus = useFleetStatus(); - const [selectedAPIKeyId, setSelectedAPIKeyId] = useState(); + const [selectedApiKeyId, setSelectedAPIKeyId] = useState(); const [isFleetServerPolicySelected, setIsFleetServerPolicySelected] = useState(false); - const apiKey = useGetOneEnrollmentAPIKey(selectedAPIKeyId); + const apiKey = useGetOneEnrollmentAPIKey(selectedApiKeyId); const settings = useGetSettings(); const fleetServerInstructions = useFleetServerInstructions(apiKey?.data?.item?.policy_id); @@ -84,10 +84,11 @@ export const ManagedInstructions = React.memo( !agentPolicy ? AgentPolicySelectionStep({ agentPolicies, + selectedApiKeyId, setSelectedAPIKeyId, setIsFleetServerPolicySelected, }) - : AgentEnrollmentKeySelectionStep({ agentPolicy, setSelectedAPIKeyId }), + : AgentEnrollmentKeySelectionStep({ agentPolicy, selectedApiKeyId, setSelectedAPIKeyId }), ]; if (isFleetServerPolicySelected) { baseSteps.push( @@ -101,7 +102,7 @@ export const ManagedInstructions = React.memo( title: i18n.translate('xpack.fleet.agentEnrollment.stepEnrollAndRunAgentTitle', { defaultMessage: 'Enroll and start the Elastic Agent', }), - children: selectedAPIKeyId && apiKey.data && ( + children: selectedApiKeyId && apiKey.data && ( ), }); @@ -115,7 +116,7 @@ export const ManagedInstructions = React.memo( }, [ agentPolicy, agentPolicies, - selectedAPIKeyId, + selectedApiKeyId, apiKey.data, isFleetServerPolicySelected, settings.data?.item?.fleet_server_hosts, diff --git a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/steps.tsx b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/steps.tsx index 03cff88e639695..8b12994473e347 100644 --- a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/steps.tsx +++ b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/steps.tsx @@ -49,14 +49,16 @@ export const DownloadStep = () => { export const AgentPolicySelectionStep = ({ agentPolicies, - setSelectedAPIKeyId, setSelectedPolicyId, - setIsFleetServerPolicySelected, + selectedApiKeyId, + setSelectedAPIKeyId, excludeFleetServer, + setIsFleetServerPolicySelected, }: { agentPolicies?: AgentPolicy[]; - setSelectedAPIKeyId?: (key?: string) => void; setSelectedPolicyId?: (policyId?: string) => void; + selectedApiKeyId?: string; + setSelectedAPIKeyId?: (key?: string) => void; setIsFleetServerPolicySelected?: (selected: boolean) => void; excludeFleetServer?: boolean; }) => { @@ -99,6 +101,7 @@ export const AgentPolicySelectionStep = ({ void; }) => { return { @@ -132,6 +137,7 @@ export const AgentEnrollmentKeySelectionStep = ({