Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] Fix staleness bug in "Add agent" flyout #103095

Merged
merged 5 commits into from
Jun 24, 2021

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Jun 23, 2021

Summary

Fix #102863. Looks like a regression from #101576.

How to test

  1. Start Kibana & ES on basic
  2. Go to fleet and follow in-app instructions for setting up an agent
  3. Go to the "Agents" tab in fleet
  4. Choose "Add agent"
  5. Choose a different policy in the flyout (you may need to create another policy in the "Agent policies" section)
  6. Make sure that the on-screen command (bottom of flyout) changes its enrollment token value when you change the selected policy
  7. Make sure that the enrollment token value changes in the select inside the "Authentication settings" section
  8. Make sure that the service token value changes in the on-screen command changes when choosing a different policy that has the "Fleet server" integration added

See the linked issue for more details.

To reviewers

This contribution refactors how some of the state is managed in the "Add agent" flyout. Currently the state management uses callbacks only with nested child state values that need to call the callbacks to keep the parent component's state in sync. This can lead to staleness bugs if we refactor these components, as was recently done, and do not propagate state correctly back up to the parent (single source of truth).

Happy to leave these changes out though if it introduces too much volatility since it is another refactor!

Checklist

For now, manual testing is done per the PR description.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Refactored state management. Med High This bug could break the information presented via the "Add agent" flyout. Tests need to be added for the flyout.

For maintainers

@jloleysens jloleysens added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 23, 2021
@jloleysens jloleysens requested a review from kpollich June 23, 2021 14:00
@jloleysens jloleysens requested a review from a team as a code owner June 23, 2021 14:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)


setEnrollmentAPIKeys(enrollmentAPIKeysResponse);
// Default to the first the first enrollment key if there is one.
onKeyChange(enrollmentAPIKeysResponse[0]?.id);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is the fix for the bug in the linked issue.

Copy link
Member

@kpollich kpollich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit on a typo otherwise LGTM 🚀

);

setEnrollmentAPIKeys(enrollmentAPIKeysResponse);
// Default to the first the first enrollment key if there is one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment has a typo. "The first" appears twice.

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 691.9KB 692.1KB +212.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@jloleysens jloleysens merged commit 2a8f3eb into elastic:master Jun 24, 2021
@jloleysens jloleysens deleted the fleet/fix-add-agent-flyout branch June 24, 2021 12:45
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 24, 2021
* * 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]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 24, 2021
* * 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]>
@amolnater-qasource
Copy link

Hi @EricDavisX
We have revalidated this issue on 7.14.0 Snapshot and found it fixed now.
Under Add agent flyout:

  • On changing Policy, enrollment token is selected for the respective policy.
  • enrollment token value changes in the select inside the "Authentication settings" section.
  • the service token value changes for the command when the Policy has the "Fleet server" integration added.

Build details:

Build: 42089
Commit: 67a71c75d2da40e49fba2620f488c9b4ce2467d2

Thanks
QAS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet]: Command string is not updated as per Policy selected under Add Agent fly-out.
5 participants