Skip to content

Commit

Permalink
[Fleet] Fix staleness bug in "Add agent" flyout (#103095) (#103263)
Browse files Browse the repository at this point in the history 
* * 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 <[email protected]>

Co-authored-by: Jean-Louis Leysens <[email protected]>
  • Loading branch information
@kibanamachine @jloleysens
kibanamachine and jloleysens authored Jun 24, 2021
1 parent 1bbe1e4 commit 6d44019
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 49 deletions.
38 changes: 16 additions & 22 deletions ...leet/public/components/agent_enrollment_flyout/advanced_agent_authentication_settings.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,19 @@ import {

interface Props {
agentPolicyId?: string;
selectedApiKeyId?: string;
onKeyChange: (key?: string) => void;
}

export const AdvancedAgentAuthenticationSettings: FunctionComponent<Props> = ({
agentPolicyId,
selectedApiKeyId,
onKeyChange,
}) => {
const { notifications } = useStartServices();
const [enrollmentAPIKeys, setEnrollmentAPIKeys] = useState<GetEnrollmentAPIKeysResponse['list']>(
[]
);
// 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<undefined | string>();
const [isLoadingEnrollmentKey, setIsLoadingEnrollmentKey] = useState(false);
const [isAuthenticationSettingsOpen, setIsAuthenticationSettingsOpen] = useState<boolean>(false);

Expand All @@ -51,7 +50,7 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent<Props> = ({
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',
Expand All @@ -66,15 +65,6 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent<Props> = ({
}
};

useEffect(
function triggerOnKeyChangeEffect() {
if (onKeyChange) {
onKeyChange(selectedEnrollmentApiKey);
}
},
[onKeyChange, selectedEnrollmentApiKey]
);

useEffect(
function useEnrollmentKeysForAgentPolicyEffect() {
if (!agentPolicyId) {
Expand All @@ -97,9 +87,13 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent<Props> = ({
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',
Expand All @@ -108,21 +102,21 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent<Props> = ({
}
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 (
<>
Expand All @@ -139,14 +133,14 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent<Props> = ({
{isAuthenticationSettingsOpen && (
<>
<EuiSpacer size="m" />
{enrollmentAPIKeys.length && selectedEnrollmentApiKey ? (
{enrollmentAPIKeys.length && selectedApiKeyId ? (
<EuiSelect
fullWidth
options={enrollmentAPIKeys.map((key) => ({
value: key.id,
text: key.name,
}))}
value={selectedEnrollmentApiKey || undefined}
value={selectedApiKeyId || undefined}
prepend={
<EuiText>
<FormattedMessage
Expand All @@ -156,7 +150,7 @@ export const AdvancedAgentAuthenticationSettings: FunctionComponent<Props> = ({
</EuiText>
}
onChange={(e) => {
setSelectedEnrollmentApiKey(e.target.value);
onKeyChange(e.target.value);
}}
/>
) : (
Expand Down
40 changes: 21 additions & 19 deletions x-pack/plugins/fleet/public/components/agent_enrollment_flyout/agent_policy_selection.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type Props = {
} & (
| {
withKeySelection: true;
selectedApiKeyId?: string;
onKeyChange?: (key?: string) => void;
}
| {
Expand All @@ -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;
}
Expand All @@ -44,33 +45,33 @@ const resolveAgentId = (
}
}

return selectedAgentId;
return selectedAgentPolicyId;
};

export const EnrollmentStepAgentPolicy: React.FC<Props> = (props) => {
const { withKeySelection, agentPolicies, onAgentPolicyChange, excludeFleetServer } = props;
const onKeyChange = props.withKeySelection && props.onKeyChange;
const [selectedAgentId, setSelectedAgentId] = useState<undefined | string>(
const { agentPolicies, onAgentPolicyChange, excludeFleetServer } = props;

const [selectedAgentPolicyId, setSelectedAgentPolicyId] = useState<undefined | string>(
() => 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 (
Expand All @@ -90,25 +91,26 @@ export const EnrollmentStepAgentPolicy: React.FC<Props> = (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',
})}
/>
<EuiSpacer size="m" />
{selectedAgentId && (
{selectedAgentPolicyId && (
<AgentPolicyPackageBadges
agentPolicyId={selectedAgentId}
agentPolicyId={selectedAgentPolicyId}
excludeFleetServer={excludeFleetServer}
/>
)}
{withKeySelection && onKeyChange && (
{props.withKeySelection && props.onKeyChange && (
<>
<EuiSpacer />
<AdvancedAgentAuthenticationSettings
onKeyChange={onKeyChange}
agentPolicyId={selectedAgentId}
selectedApiKeyId={props.selectedApiKeyId}
onKeyChange={props.onKeyChange}
agentPolicyId={selectedAgentPolicyId}
/>
</>
)}
Expand Down
11 changes: 6 additions & 5 deletions x-pack/plugins/fleet/public/components/agent_enrollment_flyout/managed_instructions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ export const ManagedInstructions = React.memo<Props>(
({ agentPolicy, agentPolicies, viewDataStepContent }) => {
const fleetStatus = useFleetStatus();

const [selectedAPIKeyId, setSelectedAPIKeyId] = useState<string | undefined>();
const [selectedApiKeyId, setSelectedAPIKeyId] = useState<string | undefined>();
const [isFleetServerPolicySelected, setIsFleetServerPolicySelected] = useState<boolean>(false);

const apiKey = useGetOneEnrollmentAPIKey(selectedAPIKeyId);
const apiKey = useGetOneEnrollmentAPIKey(selectedApiKeyId);
const settings = useGetSettings();
const fleetServerInstructions = useFleetServerInstructions(apiKey?.data?.item?.policy_id);

Expand All @@ -84,10 +84,11 @@ export const ManagedInstructions = React.memo<Props>(
!agentPolicy
? AgentPolicySelectionStep({
agentPolicies,
selectedApiKeyId,
setSelectedAPIKeyId,
setIsFleetServerPolicySelected,
})
: AgentEnrollmentKeySelectionStep({ agentPolicy, setSelectedAPIKeyId }),
: AgentEnrollmentKeySelectionStep({ agentPolicy, selectedApiKeyId, setSelectedAPIKeyId }),
];
if (isFleetServerPolicySelected) {
baseSteps.push(
Expand All @@ -101,7 +102,7 @@ export const ManagedInstructions = React.memo<Props>(
title: i18n.translate('xpack.fleet.agentEnrollment.stepEnrollAndRunAgentTitle', {
defaultMessage: 'Enroll and start the Elastic Agent',
}),
children: selectedAPIKeyId && apiKey.data && (
children: selectedApiKeyId && apiKey.data && (
<ManualInstructions apiKey={apiKey.data.item} fleetServerHosts={fleetServerHosts} />
),
});
Expand All @@ -115,7 +116,7 @@ export const ManagedInstructions = React.memo<Props>(
}, [
agentPolicy,
agentPolicies,
selectedAPIKeyId,
selectedApiKeyId,
apiKey.data,
isFleetServerPolicySelected,
settings.data?.item?.fleet_server_hosts,
Expand Down
12 changes: 9 additions & 3 deletions x-pack/plugins/fleet/public/components/agent_enrollment_flyout/steps.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}) => {
Expand Down Expand Up @@ -99,6 +101,7 @@ export const AgentPolicySelectionStep = ({
<EnrollmentStepAgentPolicy
agentPolicies={regularAgentPolicies}
withKeySelection={setSelectedAPIKeyId ? true : false}
selectedApiKeyId={selectedApiKeyId}
onKeyChange={setSelectedAPIKeyId}
onAgentPolicyChange={onAgentPolicyChange}
excludeFleetServer={excludeFleetServer}
Expand All @@ -109,9 +112,11 @@ export const AgentPolicySelectionStep = ({

export const AgentEnrollmentKeySelectionStep = ({
agentPolicy,
selectedApiKeyId,
setSelectedAPIKeyId,
}: {
agentPolicy: AgentPolicy;
selectedApiKeyId?: string;
setSelectedAPIKeyId: (key?: string) => void;
}) => {
return {
Expand All @@ -132,6 +137,7 @@ export const AgentEnrollmentKeySelectionStep = ({
<EuiSpacer size="l" />
<AdvancedAgentAuthenticationSettings
agentPolicyId={agentPolicy.id}
selectedApiKeyId={selectedApiKeyId}
onKeyChange={setSelectedAPIKeyId}
/>
</>
Expand Down

0 comments on commit 6d44019

Please sign in to comment.