-
Notifications
You must be signed in to change notification settings - Fork 483
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
Edit software - GitOps #21612
Comments
Did some further research into this and software edits are handled by https://github.com/fleetdm/fleet/blob/main/docs/Contributing/API-for-contributors.md#batch-apply-software in GitOps, and that endpoint handles updates with PUT semantics on the entire collection of software, which works for that use case. With that said, the existing endpoint doesn't cancel pending installs on changes to a title (or deletions), and unless I'm missing something doesn't update any counts. @noahtalerman @lukeheath do we need to add these side effects to the existing batch endpoint? If we do, adding side effects should be the entirety of this issue. If not, I think we can close this as a noop since edits are already possible via GitOps. |
Based on discussions in standup today, adding the side effects seems to be the reasonable thing to do here, so this will stay open with adding side effects as the scope. |
@iansltx I think we want the experience here to be consistent across UI, API, and YAML. As a user editing software, I expect to cancel pending installs. It looks like we went this route which I think is a good call 👍 |
Was going to include this in the PR with everything else, but it can be reviewed etc. independently, and I've already gotten reviews on backend for all other work on the branch, so will set this up as a separate branch/PR (though I'll reuse a lot of work from the main feature branch). This won't affect ETA (tonight/tomorrow morning). |
#22100) #21612 # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated tests --------- Co-authored-by: RachelElysia <rachel@fleetdm.com> Co-authored-by: Luke Heath <luke@fleetdm.com> Co-authored-by: Jacob Shandling <jacob@fleetdm.com> Co-authored-by: Victor Lyuboslavsky <victor.lyuboslavsky@gmail.com>
#22100) #21612 # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated tests --------- Co-authored-by: RachelElysia <rachel@fleetdm.com> Co-authored-by: Luke Heath <luke@fleetdm.com> Co-authored-by: Jacob Shandling <jacob@fleetdm.com> Co-authored-by: Victor Lyuboslavsky <victor.lyuboslavsky@gmail.com>
QA UI/API - @RachelElysia |
QA UI/API checklist finished and can be found in Story description #20404 |
GitOps QA done In addition to the testplan, also tested
|
GitOps shapes our code, Software installer, Script changes, in flight, Contents shift, we adapt, |
Test plan (taken from enterprise_integration_test TestBatchSetSoftwareInstallersSideEffects):
To test: Need to set
TEST_TEAM_NAME
andSOFTWARE_INSTALLER_URL
variables forteam_software_installer_valid.yml
and apply with the following command:The text was updated successfully, but these errors were encountered: