fix(windows): resolve kernel path via %SystemRoot%#288
Conversation
KernelVersion() called GetFileVersionInfo on a hardcoded C:\Windows\System32\ntoskrnl.exe, which fails with ERROR_FILE_NOT_FOUND on Windows hosts whose system drive is not C:\. The failure propagates through sysinfo.Host() and breaks Elastic Agent install/enroll on such hosts. Resolve the kernel path through %SystemRoot% (falling back to %WINDIR% and then the existing C:\Windows default) and add a regression test. Fixes elastic#287
intxgo
left a comment
There was a problem hiding this comment.
Since this PR tries to address totally stripped environment by fallback to "C:\Windows", this can be mitigated in a safer way.
is calling API an option GetSystemWindowsDirectory
or reading a registry HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\SystemRoot
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
Pushed 7a745fb. Switched to reading |
There was a problem hiding this comment.
LGTM, but I have an opinion here. With the fix the behavior will be much better, however should the library decide on an arbitrary fallback const fallbackSystemRoot = C:\Windows ?
I think the client code could decide to do it, the library should return a "trusted" value or empty or error. Current behavior kind of still exhibit "half of" the original bug.
|
I opened #289 to address the unrelated CI failure. That needs merged first to unblock the rest of the CI pipeline in this change. |
|
Thanks, will rebase on top once #289 lands. |
Fixes #287.
KernelVersion()calledGetFileVersionInfoon a hardcodedC:\Windows\System32\ntoskrnl.exe, which fails withERROR_FILE_NOT_FOUNDon Windows hosts whose system drive is notC:\(for example a server installed onW:\). The failure propagates throughsysinfo.Host()and breaks Elastic Agent install/enroll on those hosts, as reported in the issue.This PR resolves the kernel path through
%SystemRoot%(falling back to%WINDIR%, then the historicalC:\Windowsdefault) and adds at.Setenv-driven regression test covering each fallback step.Local results:
GOOS=windows go build ./...clean for amd64 and arm64.go run github.com/elastic/go-licenser@v0.4.2 -dclean.gofmtandgo vetclean.TestKernelExePathtable-test compiles and links cleanly on a Windows test binary (GOOS=windows go test -c).I will add a
.changelog/<pr>.txtentry once GitHub assigns the PR number.