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

[TUF autoupdater] Remove osquery client dependency #1178

Merged
merged 34 commits into from
May 12, 2023

Conversation

RebeccaMahany
Copy link
Contributor

@RebeccaMahany RebeccaMahany commented May 4, 2023

Relates to #954.

This PR extracts the osquerier dependency from the library, so that we can init the library before we have an osquery client.

It also pulls out a shared function for sorting versions in the library for reuse later.

Adds a small amount of documentation about how to generate mocks with mockery, since it's done in a slightly non-standard way in this package.

Future work + linked PRs

Subsequent PRs will tackle the following (order is not set in stone):

  1. A new implementation of findNewest -- already in progress here: [TUF autoupdater] Check out latest #1185
  2. Point to release file <binary>/<os>/<arch>/<channel>/release.json, once available
  3. Trigger a reload/restart after a successful update; launcher init performs version selection using the new update library (potentially falling back to findNew); launcher performs library tidying after version selection
  4. Other improvements/refactors (ship a more up-to-date TUF repo, maybe split library parts of autoupdate/tuf out into its own package)
  5. An "update now" functionality tied to control server
  6. Eventually removing the old notary autoupdater

Previous work:

  1. Run new TUF autoupdater side-by-side with notary autoupdater #1081
  2. Expose data and metrics about new TUF autoupdater #1103
  3. Point to production TUF infra #1108
  4. Perform retry on TUF update #1110
  5. Add library manager to handle TUF downloads #1111
  6. Small autoupdate improvements #1119
  7. Pass knapsack in to tuf autoupdater to simplify configuration #1168

@RebeccaMahany RebeccaMahany changed the title [TUF autoupdater] Split up library manager [TUF autoupdater] Get installed version May 8, 2023
@RebeccaMahany RebeccaMahany marked this pull request as ready for review May 8, 2023 18:29
pkg/autoupdate/tuf/autoupdate.go Show resolved Hide resolved
pkg/autoupdate/tuf/library_manager.go Show resolved Hide resolved

// IsInstallVersion checks whether the version in the given target is the same as the one
// contained in the installation.
func (ulm *updateLibraryManager) IsInstallVersion(binary autoupdatableBinary, targetFilename string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm. So how will this get used?

  1. installed as 1.0.8
  2. upgraded to 1.0.9
  3. downgraded to 1.0.8

So what is executed?

My gut sense is that the complexity isn't worth it, and just downloading 1.0.8 is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pulled it out, more trouble than it was worth!

Copy link
Contributor

Choose a reason for hiding this comment

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

Having slept on it... I think we need something here.

There's a case that's like:

  1. installed as 1.0.8
  2. stable is 1.0.8
  3. download 1.0.8

And that feels kinda -_-

Bt my inclination would be to push the initial versioning to main or potentially even the packaging layer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Main might be clean enough. I think the hardest part here was the find-binary code. And main / extension setup should know that

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like I'm talking myself in circles here. I'm sorry the requirements aren't more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just wrapped up this commit 72db950, which I think handles what we're talking about:

  1. install version is 1.0.8
  2. current running version is therefore 1.0.8
  3. there is no update downloaded for 1.0.8
  4. we see that current running version is 1.0.8 so we don't perform the download

pkg/autoupdate/tuf/library_manager.go Outdated Show resolved Hide resolved
@RebeccaMahany RebeccaMahany changed the title [TUF autoupdater] Get installed version [TUF autoupdater] Remove osquery client dependency May 10, 2023
Copy link
Contributor

@directionless directionless left a comment

Choose a reason for hiding this comment

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

I'm not sure I understand all the nuance here, but the code feels like the right shape.

@RebeccaMahany RebeccaMahany merged commit a3b79a2 into kolide:main May 12, 2023
14 checks passed
@RebeccaMahany RebeccaMahany deleted the becca/tuf-split-up-library branch May 12, 2023 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants