-
Notifications
You must be signed in to change notification settings - Fork 103
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
update windows data dir installation #1510
Conversation
35b2b4c
to
48ee53a
Compare
pkg/packagekit/wix/wix.go
Outdated
// touch these known file names before harvest to ensure they're cleaned up on uninstall | ||
dataFilenames := []string{"launcher.db", "metadata.json"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to self -- think about this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is probably correct, and I love how small it is.
How should we test this? We could build some nabalu packages with the branch maybe
537e470
to
0a8d5f6
Compare
0a8d5f6
to
9e2bc43
Compare
wo.cleanDirs = append(wo.cleanDirs, wo.packageDataRoot) | ||
|
||
fullIdentifier := fmt.Sprintf("Launcher-%s", wo.identifier) | ||
dataFilesPath := filepath.Join(wo.packageDataRoot, fullIdentifier, "data") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a good idea to update the default value here https://github.com/kolide/launcher/blob/main/pkg/launcher/paths.go#L42 also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the documentation in the code + in the PR description
@@ -39,7 +39,7 @@ func DefaultPath(path defaultPath) string { | |||
if runtime.GOOS == "windows" { | |||
switch path { | |||
case RootDirectory: | |||
return "C:\\Program Files\\Kolide\\Launcher-kolide-k2\\data" | |||
return "C:\\ProgramData\\Kolide\\Launcher-kolide-k2\\data" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is this going to work with existing installations? Do we need to do some kind of migration or peek at old dir first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a recent change I can re-run some tests with to verify- but with prior testing this has always worked out regardless of existing/new installation because only the new installations have the new path in launcher.flags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh duh =) ... yea, I forgot this was assigned in launcher.flags
There are two pieces to this update:
C:\Program Files\Kolide\Launcher-[IDENTIFIER]\
bin
andconf
directoriesC:\ProgramData\Kolide\Launcher-[IDENTIFIER]\
data
directoryrootDir
, containing all updateslauncher.db
file in thedata
directoryIdeally we would integrate the wix RemoveFolderEx util extension into the wix-generated files (along with some registry updates) to achieve full cleanup, but this did not work as advertised after several attempts. Our cleanup complications come from arbitrarily nested directories containing filenames which are unknown at the time of harvest (e.g. rotated logs, updates, etc.). But because we are specifically concerned with the launcher.db cleanup, ensuring that file is present is enough for forward progress here.
Another option we can look into in the future if additional cleanup is needed is the wix CustomAction element. There are some reports of successful cleanup on this type of install structure using a separate removal script via CustomAction.
Including a quick logical overview of the changes here because it took me a bit to understand how wix works and how we integrate it within the build process - note that this is especially confusing because wix operates off of relative paths, expanded internally at build time, while the linux and darwin build processes are able to nest full paths within their respective root build directories back at the level of pkg/packaging/packaging.go. This makes it difficult to cleanly share as much logic as we would like between platform builds, and largely shaped this approach methodology in order to prevent excessive changes within the linux/darwin builds.
canonicalizeRootDir
for the flag file generationProgramData
)DATADIR
(expanded by wix into theProgramData
directory) with our pre-created file stubs, and include the newly generated component group from the main wix templateTesting
To test locally (outside of CI builds) I'd recommend using git bash to build on a windows machine directly (otherwise you will need to make some modifications to the commands below).
Navigate to your launcher directory within git bash and pull down this branch. Most build setup should be identical to other platforms (i.e.
make deps && make
).Note your existing enroll secret and replace in the command below:
cat C:/Program\ Files/Kolide/Launcher-kolide-k2/conf/secret
Run the following to build an MSI for local installation:
Uninstall your existing Launcher installation from Control-Panel and ensure a fresh installation by removing all of
C:/Program\ Files/Kolide
.Your new build for testing should be installable from
./build/launcher.windows-service-msi.msi