Conversation
mostlikelee
requested review from
a team,
getvictor,
gillespi314,
lucasmrod and
roperzh
as code owners
getvictor
left a comment
getvictor
left a comment
There was a problem hiding this comment.
Looks good. Left a few comments/questions.
cmd/fleetctl/gitops_test.go
Outdated
There was a problem hiding this comment.
Is this top level key supposed to be required or optional for global settings?
There was a problem hiding this comment.
Good question. The PR assumes that software is an optional key. However because of the declarative nature of gitops, declaring empty software will delete any existing installable software out of "no teams". Is that ok @marko-lisica ?
There was a problem hiding this comment.
To note, this mimics the current behavior of teams software
There was a problem hiding this comment.
discussed answer, for reference: #19550 (comment)
server/fleet/software_installer.go
Outdated
There was a problem hiding this comment.
There is a bug with PreInstallQuery: #20747
Please make a note on the bug that it will need to be fixed for No Team software as well.
There was a problem hiding this comment.
This is the breaking contributors API change, right?
There was a problem hiding this comment.
True, i'll update the contributor docs
server/service/client.go
Outdated
getvictor
left a comment
getvictor
left a comment
There was a problem hiding this comment.
Looks good. One question to check.
Since adding software top level key may break existing gitops users (including dogfood), please make sure the right people are notified.
Also, we test against the fleet-gitops repo. Please add the software key to: https://github.com/fleetdm/fleet-gitops/blob/main/default.yml
I assume free users simply ignore the software key if present.
pkg/spec/gitops.go
Outdated
There was a problem hiding this comment.
Is the following syntax supported?
software:
- path: ../lib/eng_software.yml
- path: ../lib/mgr_software.yml
If not, can you file a bug? This is how other top level keys work.
There was a problem hiding this comment.
i ran a quick test and that's still working
mostlikelee
requested review from
a team,
eashaw,
mikermcneil and
noahtalerman
as code owners
This reverts commit 576a94b765111ba96b66023218a0c7af328ea8de.
mostlikelee
force-pushed
the
24064-gitops
branch
from
576a94b to
1312b08
Compare
fleet-release
removed
the
~ceo
server/service/client.go
Outdated
| // Apply "no team" software installers. Running every time to support | ||
| // removal of software installers on an empty config. | ||
| if appconfig != nil && appconfig.License.IsPremium() { | ||
| if len(specs.Teams) == 0 && appconfig != nil && appconfig.License.IsPremium() { |
There was a problem hiding this comment.
What happens if a user adds a single query with fleetctl apply? Will it wipe their No Team software?
There was a problem hiding this comment.
Looks good, if the tests pass. cc: @lucasmrod
Luke also needs to review.
|
Removing Mike as is seems to be an accident. |
|
@lukeheath Can you approve and merge this PR? It will fix failing GitOps tests on main. |
#20464 Adding gitops support for a top level `software` key to be used to manage installable software into "no team". - [ ] 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] Added/updated tests - [X] Manual QA for all new/changed functionality --------- Co-authored-by: Victor Lyuboslavsky <victor.lyuboslavsky@gmail.com>
6 participants
#20464
Adding gitops support for a top level
softwarekey to be used to manage installable software into "no team".changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.