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.

Sign up for GitHub

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

Jump to bottom

Fleet UI run script disabled when host has scripts enabled and on orbit < 1.24.0 #18382

Closed
georgekarrv opened this issue Apr 17, 2024 · 10 comments
Closed

Fleet UI run script disabled when host has scripts enabled and on orbit < 1.24.0 #18382

georgekarrv opened this issue Apr 17, 2024 · 10 comments
Assignees
@jacobshandling
Labels
bug Something isn't working as documented #g-endpoint-ops Endpoint ops product group :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. ~unreleased bug This bug was found in an unreleased version of Fleet.
Milestone
4.49.0

Comments

@georgekarrv
Copy link
Member

Fleet version: main Fleet 0.0.0-SNAPSHOT-6b6b6e6 • Go go1.21.7
Fleetd version: 1.23.0

Web browser and operating system:

💥  Actual behavior

Screenshot 2024-04-17 at 4 54 17 PM

🧑‍💻  Steps to reproduce

  1. Latest server
  2. host with --scripts_enabled but running fleetd 1.23.0
  3. Open Device page and try to run a script.

🕯️ More info (optional)

The fleet API is not broken since you can still correctly run scripts via fleetctl and the api.

Fix how UI is determining if scripts are enabled to support more than just the latest orbit version.
@georgekarrv georgekarrv added bug Something isn't working as documented :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. #g-endpoint-ops Endpoint ops product group ~unreleased bug This bug was found in an unreleased version of Fleet. labels Apr 17, 2024
@gillespi314
Copy link
Contributor

Here’s at least part of the frontend bug, I think, where the null value would be coerced to false when generating the dropdown options and cause the option to be disabled.

@jacobshandling
Copy link
Contributor

@gillespi314 so null should be interpreted as scripts being enabled?

@jacobshandling
Copy link
Contributor

Looks like @getvictor saw this coming a week ago.

@jacobshandling
Copy link
Contributor

scripts_enabled == null means this agent is not an orbit agent, or this agent is version <=1.23.0 which is not collecting the scripts enabled info

@rachaelshaw I don't see anything in the Figma for these edge cases. How should the UI handle them?

@georgekarrv
Copy link
Member Author

^ need to continue to have our old logic as a fallback

@jacobshandling
Copy link
Contributor

@georgekarrv @rachaelshaw how about in this case we don't disable the option, but add a tooltip to the effect of "This host may not actually be able to run scripts"?

@gillespi314
Copy link
Contributor

@jacobshandling:

so null should be interpreted as scripts being enabled?

Based on this commit, I think so.

@jacobshandling
Copy link
Contributor

Prototype for now:
Screenshot 2024-04-17 at 4 36 07 PM

@jacobshandling jacobshandling mentioned this issue Apr 17, 2024
Merged
3 tasks
@rachaelshaw rachaelshaw added the :product Product Design department (shows up on 🦢 Drafting board) label Apr 18, 2024
@rachaelshaw
Copy link
Member

@jacobshandling discussed @ design review, and decided to just keep the options enabled as before when scripts_enabled == null, no tooltip.

We noticed that the tooltips for the disabled items are missing "and refetch host vitals" though (it was added later as a response to a concern Victor brought up, and the front-end may have been done already). Figma

@rachaelshaw rachaelshaw removed the :product Product Design department (shows up on 🦢 Drafting board) label Apr 18, 2024
jacobshandling added a commit that referenced this issue Apr 18, 2024
@jacobshandling
UI – when host's scripts_enabled is null, allow running scripts w…
5657b2d 
…ith warning (#18386)

## Addresses #18382 

- Do not disable option when `scripts_enabled: null`
- Update disabled tooltip copy for 3 actions
- Generalize dropdown with tooltip component to work for disabled and
non-disabled (now unused, but was used in prototype before product
input) cases
![Screenshot 2024-04-18 at 11 52
15 AM](https://github.com/fleetdm/fleet/assets/61553566/b1a08086-d39e-47ea-af8f-57f4e391e106)
![Screenshot 2024-04-18 at 12 29
18 PM](https://github.com/fleetdm/fleet/assets/61553566/25072573-ee79-45f4-951a-af072645a7d6)

- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
- [x] Confirm previously disabled options with tooltips still work as
expected with generalized dropdown option with tooltip

---------

Co-authored-by: Jacob Shandling <jacob@fleetdm.com>
@lukeheath lukeheath added this to the 4.49.0-tentative milestone Apr 19, 2024
@fleet-release
Copy link
Contributor

Fleet's script can run,
Even when host orbits low.
Cloud city shines on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jacobshandling jacobshandling

Labels
bug Something isn't working as documented #g-endpoint-ops Endpoint ops product group :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. ~unreleased bug This bug was found in an unreleased version of Fleet.
Projects
None yet
Milestone
4.49.0
Development

No branches or pull requests
7 participants
@georgekarrv @lukeheath @rachaelshaw @jacobshandling @gillespi314 @fleet-release @sharon-fdm